Skip to content

Commit

Permalink
Upgrade Spezi and SpeziAccount (#23)
Browse files Browse the repository at this point in the history
# Upgrade Spezi and SpeziAccount

## ♻️ Current situation & Problem
This PR updates to the latest releases of Spezi and SpeziAccount. By
employing the newly release framework features we are resolving #14, #15
and removing the modifier workaround to retrieve the authorization
controller again.

For more information see
StanfordSpezi/SpeziAccount#36.
## ⚙️ Release Notes 
* Upgrade SpeziAccount.
* Upgrade Spezi.
* Removed the `firebaseAccount(_:)` modifier.
* Migrated to String Catalogs


## 📚 Documentation
--


## ✅ Testing
--


## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [x] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).
Fo
  • Loading branch information
Supereg authored Nov 13, 2023
1 parent 187d1ed commit 084d697
Show file tree
Hide file tree
Showing 30 changed files with 1,251 additions and 391 deletions.
19 changes: 19 additions & 0 deletions .github/workflows/markdown-lint-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#
# This source file is part of the Stanford Spezi open source project
#
# SPDX-FileCopyrightText: 2023 Stanford University and the project authors (see CONTRIBUTORS.md)
#
# SPDX-License-Identifier: MIT
#

name: Monthly Markdown Link Check

on:
# Runs at midnight on the first of every month
schedule:
- cron: "0 0 1 * *"

jobs:
markdown_link_check:
name: Markdown Link Check
uses: StanfordBDHG/.github/.github/workflows/markdown-link-check.yml@v2
3 changes: 3 additions & 0 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@ jobs:
swiftlint:
name: SwiftLint
uses: StanfordSpezi/.github/.github/workflows/swiftlint.yml@v2
markdown_link_check:
name: Markdown Link Check
uses: StanfordBDHG/.github/.github/workflows/markdown-link-check.yml@v2
8 changes: 5 additions & 3 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ let package = Package(
.library(name: "SpeziFirebaseStorage", targets: ["SpeziFirebaseStorage"])
],
dependencies: [
.package(url: "https://github.com/StanfordSpezi/Spezi", .upToNextMinor(from: "0.7.0")),
.package(url: "https://github.com/StanfordSpezi/SpeziAccount", .upToNextMinor(from: "0.6.1")),
.package(url: "https://github.com/StanfordSpezi/SpeziStorage", .upToNextMinor(from: "0.4.2")),
.package(url: "https://github.com/StanfordSpezi/Spezi", .upToNextMinor(from: "0.8.0")),
.package(url: "https://github.com/StanfordSpezi/SpeziViews.git", .upToNextMinor(from: "0.6.1")),
.package(url: "https://github.com/StanfordSpezi/SpeziAccount", .upToNextMinor(from: "0.7.1")),
.package(url: "https://github.com/StanfordSpezi/SpeziStorage", .upToNextMinor(from: "0.5.0")),
.package(url: "https://github.com/firebase/firebase-ios-sdk", from: "10.13.0")
],
targets: [
Expand All @@ -35,6 +36,7 @@ let package = Package(
dependencies: [
.target(name: "SpeziFirebaseConfiguration"),
.product(name: "Spezi", package: "Spezi"),
.product(name: "SpeziValidation", package: "SpeziViews"),
.product(name: "SpeziAccount", package: "SpeziAccount"),
.product(name: "SpeziLocalStorage", package: "SpeziStorage"),
.product(name: "SpeziSecureStorage", package: "SpeziStorage"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ import OSLog
import SpeziAccount
import SwiftUI

enum ReauthenticationOperationResult {
case cancelled
case success
}


protocol FirebaseAccountService: AnyActor, AccountService {
static var logger: Logger { get }
Expand All @@ -26,18 +31,12 @@ protocol FirebaseAccountService: AnyActor, AccountService {
/// - Parameter context: The global firebase context
func configure(with context: FirebaseContext) async

func inject(authorizationController: AuthorizationController) async

/// This method is called once the account for the given user was removed.
///
/// This allows for additional cleanup tasks to be performed.
/// - Parameter userId: The userId which was removed, or nil if we couldn't retrieve the last user.
func handleAccountRemoval(userId: String?) async

/// This method is called to re-authenticate the current user credentials.
/// - Parameters:
/// - user: The User instance.
func reauthenticateUser(user: User) async throws
///
/// - Parameter user: The user instance to reauthenticate.
/// - Returns: `true` if authentication was successful, `false` if authentication was cancelled by the user.
/// - Throws: If authentication failed.
func reauthenticateUser(user: User) async throws -> ReauthenticationOperationResult
}


Expand Down Expand Up @@ -74,7 +73,12 @@ extension FirebaseAccountService {
}

try await context.dispatchFirebaseAuthAction(on: self) {
try await reauthenticateUser(user: currentUser) // delete requires a recent sign in
let result = try await reauthenticateUser(user: currentUser) // delete requires a recent sign in
guard case .success = result else {
Self.logger.debug("Re-authentication was cancelled. Not deleting the account.")
return // cancelled
}

try await currentUser.delete()
Self.logger.debug("delete() for user.")
}
Expand All @@ -90,12 +94,16 @@ extension FirebaseAccountService {

var changes = false

// if we modify sensitive credentials and require a recent login
if modifications.modifiedDetails.storage[UserIdKey.self] != nil || modifications.modifiedDetails.password != nil {
try await reauthenticateUser(user: currentUser)
}

do {
// if we modify sensitive credentials and require a recent login
if modifications.modifiedDetails.storage[UserIdKey.self] != nil || modifications.modifiedDetails.password != nil {
let result = try await reauthenticateUser(user: currentUser)
guard case .success = result else {
Self.logger.debug("Re-authentication was cancelled. Not deleting the account.")
return // got cancelled!
}
}

if let userId = modifications.modifiedDetails.storage[UserIdKey.self] {
Self.logger.debug("updateEmail(to:) for user.")
try await currentUser.updateEmail(to: userId)
Expand All @@ -105,10 +113,6 @@ extension FirebaseAccountService {
if let password = modifications.modifiedDetails.password {
Self.logger.debug("updatePassword(to:) for user.")
try await currentUser.updatePassword(to: password)

if let userId = currentUser.email { // make sure we save the new password
await context.persistCurrentCredentials(userId: userId, password: password, server: StorageKeys.emailPasswordCredentials)
}
}

if let name = modifications.modifiedDetails.name {
Expand All @@ -125,8 +129,10 @@ extension FirebaseAccountService {
try await context.notifyUserSignIn(user: currentUser, for: self)
}
} catch let error as NSError {
Self.logger.error("Received NSError on firebase dispatch: \(error)")
throw FirebaseAccountError(authErrorCode: AuthErrorCode(_nsError: error))
} catch {
Self.logger.error("Received error on firebase dispatch: \(error)")
throw FirebaseAccountError.unknown(.internalError)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,19 @@ import FirebaseAuth
import OSLog
import SpeziAccount
import SpeziSecureStorage
import SpeziValidation
import SwiftUI


struct EmailPasswordViewStyle: UserIdPasswordAccountSetupViewStyle {
let service: FirebaseEmailPasswordAccountService

var securityRelatedViewModifier: any ViewModifier {
ReauthenticationAlertModifier()
}
}


actor FirebaseEmailPasswordAccountService: UserIdPasswordAccountService, FirebaseAccountService {
static let logger = Logger(subsystem: "edu.stanford.spezi.firebase", category: "AccountService")

Expand All @@ -24,26 +34,19 @@ actor FirebaseEmailPasswordAccountService: UserIdPasswordAccountService, Firebas
\.name
}

static var minimumFirebasePassword: ValidationRule {
// Firebase as a non-configurable limit of 6 characters for an account password.
// Refer to https://stackoverflow.com/questions/38064248/firebase-password-validation-allowed-regex
guard let regex = try? Regex(#"(?=.*[0-9a-zA-Z]).{6,}"#) else {
fatalError("Invalid minimumFirebasePassword regex at construction.")
}

return ValidationRule(
regex: regex,
message: "FIREBASE_ACCOUNT_DEFAULT_PASSWORD_RULE_ERROR \(6)",
bundle: .module
)
}

@AccountReference var account: Account
@_WeakInjectable var context: FirebaseContext

let configuration: AccountServiceConfiguration
let firebaseModel: FirebaseAccountModel

nonisolated var viewStyle: EmailPasswordViewStyle {
EmailPasswordViewStyle(service: self)
}

init(passwordValidationRules: [ValidationRule] = [minimumFirebasePassword]) {

init(_ model: FirebaseAccountModel, passwordValidationRules: [ValidationRule] = [.minimumFirebasePassword]) {
self.configuration = AccountServiceConfiguration(
name: LocalizedStringResource("FIREBASE_EMAIL_AND_PASSWORD", bundle: .atURL(from: .module)),
supportedKeys: .exactly(Self.supportedKeys)
Expand All @@ -58,28 +61,22 @@ actor FirebaseEmailPasswordAccountService: UserIdPasswordAccountService, Firebas
FieldValidationRules(for: \.userId, rules: .minimalEmail)
FieldValidationRules(for: \.password, rules: passwordValidationRules)
}
self.firebaseModel = model
}


func configure(with context: FirebaseContext) async {
self._context.inject(context)
await context.share(account: account)
}

func handleAccountRemoval(userId: String?) {
if let userId {
context.removeCredentials(userId: userId, server: StorageKeys.emailPasswordCredentials)
}
}

func login(userId: String, password: String) async throws {
Self.logger.debug("Received new login request...")

try await context.dispatchFirebaseAuthAction(on: self) {
try await Auth.auth().signIn(withEmail: userId, password: password)
Self.logger.debug("signIn(withEmail:password:)")
}

context.persistCurrentCredentials(userId: userId, password: password, server: StorageKeys.emailPasswordCredentials)
}

func signUp(signupDetails: SignupDetails) async throws {
Expand All @@ -103,8 +100,6 @@ actor FirebaseEmailPasswordAccountService: UserIdPasswordAccountService, Firebas
try await changeRequest.commitChanges()
}
}

context.persistCurrentCredentials(userId: signupDetails.userId, password: password, server: StorageKeys.emailPasswordCredentials)
}

func resetPassword(userId: String) async throws {
Expand All @@ -123,20 +118,36 @@ actor FirebaseEmailPasswordAccountService: UserIdPasswordAccountService, Firebas
}
}

func reauthenticateUser(user: User) async {
func reauthenticateUser(user: User) async throws -> ReauthenticationOperationResult {
guard let userId = user.email else {
return
return .cancelled
}

// with a future version of SpeziAccount we want to get rid of this workaround and request the password from the user on the fly.
guard let password = context.retrieveCredential(userId: userId, server: StorageKeys.emailPasswordCredentials) else {
return // nothing we can do
Self.logger.debug("Requesting credentials for re-authentication...")
let passwordQuery = await firebaseModel.reauthenticateUser(userId: userId)
guard case let .password(password) = passwordQuery else {
return .cancelled
}

do {
try await user.reauthenticate(with: EmailAuthProvider.credential(withEmail: userId, password: password))
} catch {
Self.logger.debug("Credential change might fail. Failed to reauthenticate with firebase: \(error)")
Self.logger.debug("Re-authenticating password-based user now ...")
try await user.reauthenticate(with: EmailAuthProvider.credential(withEmail: userId, password: password))
return .success
}
}


extension ValidationRule {
static var minimumFirebasePassword: ValidationRule {
// Firebase as a non-configurable limit of 6 characters for an account password.
// Refer to https://stackoverflow.com/questions/38064248/firebase-password-validation-allowed-regex
guard let regex = try? Regex(#"(?=.*[0-9a-zA-Z]).{6,}"#) else {
fatalError("Invalid minimumFirebasePassword regex at construction.")
}

return ValidationRule(
regex: regex,
message: "FIREBASE_ACCOUNT_DEFAULT_PASSWORD_RULE_ERROR \(6)",
bundle: .module
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,14 @@ actor FirebaseIdentityProviderAccountService: IdentityProvider, FirebaseAccountS
}

let configuration: AccountServiceConfiguration

private var authorizationController: AuthorizationController?
let firebaseModel: FirebaseAccountModel

@MainActor @AccountReference var account: Account // property wrappers cannot be non-isolated, so we isolate it to main actor
@MainActor private var lastNonce: String?

@_WeakInjectable var context: FirebaseContext

init() {
init(_ model: FirebaseAccountModel) {
self.configuration = AccountServiceConfiguration(
name: LocalizedStringResource("FIREBASE_IDENTITY_PROVIDER", bundle: .atURL(from: .module)),
supportedKeys: .exactly(Self.supportedKeys)
Expand All @@ -60,6 +59,7 @@ actor FirebaseIdentityProviderAccountService: IdentityProvider, FirebaseAccountS
}
UserIdConfiguration(type: .emailAddress, keyboardType: .emailAddress)
}
self.firebaseModel = model
}


Expand All @@ -68,23 +68,15 @@ actor FirebaseIdentityProviderAccountService: IdentityProvider, FirebaseAccountS
await context.share(account: account)
}

func inject(authorizationController: AuthorizationController) {
Self.logger.debug("Received authorization controller injection ...")
self.authorizationController = authorizationController
}

func handleAccountRemoval(userId: String?) async {
// nothing we are doing here
}

func reauthenticateUser(user: User) async throws {
func reauthenticateUser(user: User) async throws -> ReauthenticationOperationResult {
guard let appleIdCredential = try await requestAppleSignInCredential() else {
return // user canceled
return .cancelled
}

let credential = try await oAuthCredential(from: appleIdCredential)

try await user.reauthenticate(with: credential)
return .success
}

func signUp(signupDetails: SignupDetails) async throws {
Expand All @@ -108,7 +100,7 @@ actor FirebaseIdentityProviderAccountService: IdentityProvider, FirebaseAccountS
throw FirebaseAccountError.notSignedIn
}

try await context.dispatchFirebaseAuthAction(on: self) { () -> Void in
try await context.dispatchFirebaseAuthAction(on: self) {
guard let credential = try await requestAppleSignInCredential() else {
return // user canceled
}
Expand Down Expand Up @@ -239,11 +231,8 @@ actor FirebaseIdentityProviderAccountService: IdentityProvider, FirebaseAccountS
}

private func performRequest(_ request: ASAuthorizationAppleIDRequest) async throws -> ASAuthorizationResult? {
guard let authorizationController else {
Self.logger.error("""
Failed to perform AppleID request. We are missing access to the AuthorizationController. \
Did you set up the .firebaseAccount() modifier?
""")
guard let authorizationController = firebaseModel.authorizationController else {
Self.logger.error("Failed to perform AppleID request. We are missing access to the AuthorizationController.")
throw FirebaseAccountError.setupError
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ extension FirebaseEmailVerifiedKey {
public typealias Key = FirebaseEmailVerifiedKey

public var body: some View {
Text("The FirebaseEmailVerifiedKey cannot be set!")
Text(verbatim: "The FirebaseEmailVerifiedKey cannot be set!")
}

public init(_ value: Binding<Value>) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ extension FirebaseOAuthCredentialKey {
public typealias Key = FirebaseOAuthCredentialKey

public var body: some View {
Text("The FirebaseOAuthCredentialKey cannot be set!")
Text(verbatim: "The FirebaseOAuthCredentialKey cannot be set!")
}

public init(_ value: Binding<Value>) {}
Expand All @@ -69,7 +69,7 @@ extension FirebaseOAuthCredentialKey {
public typealias Key = FirebaseOAuthCredentialKey

public var body: some View {
Text("The FirebaseOAuthCredentialKey cannot be displayed!")
Text(verbatim: "The FirebaseOAuthCredentialKey cannot be displayed!")
}

public init(_ value: Value) {}
Expand Down
Loading

0 comments on commit 084d697

Please sign in to comment.