From edb8b737e7869c5890dd510743923a0e4685d448 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 30 Oct 2025 11:52:37 +0000 Subject: [PATCH 1/4] fix: allow reauth on sensitive operations --- .../Sources/Services/AuthService.swift | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift index 712ffe0668..2ea0394ee3 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift @@ -188,7 +188,13 @@ public final class AuthService { public func linkAccounts(credentials credentials: AuthCredential) async throws { authenticationState = .authenticating do { - try await currentUser?.link(with: credentials) + guard let user = currentUser else { + throw AuthServiceError.noCurrentUser + } + + try await withReauthenticationIfNeeded(on: user) { + try await user.link(with: credentials) + } updateAuthenticationState() } catch { authenticationState = .unauthenticated @@ -702,7 +708,9 @@ public extension AuthService { } // Complete the enrollment - try await user.multiFactor.enroll(with: assertion, displayName: displayName) + try await withReauthenticationIfNeeded(on: user) { + try await user.multiFactor.enroll(with: assertion, displayName: displayName) + } currentUser = auth.currentUser } catch { updateError(message: string.localizedErrorMessage(for: error)) @@ -731,6 +739,22 @@ public extension AuthService { } } + private func withReauthenticationIfNeeded(on user: User, + operation: () async throws -> Void) async throws { + do { + try await operation() + } catch let error as NSError { + if error.domain == AuthErrorDomain, + error.code == AuthErrorCode.requiresRecentLogin.rawValue || error.code == AuthErrorCode + .userTokenExpired.rawValue { + try await reauthenticateCurrentUser(on: user) + try await operation() + } else { + throw error + } + } + } + func unenrollMFA(_ factorUid: String) async throws -> [MultiFactorInfo] { do { guard let user = auth.currentUser else { @@ -739,20 +763,8 @@ public extension AuthService { let multiFactorUser = user.multiFactor - do { + try await withReauthenticationIfNeeded(on: user) { try await multiFactorUser.unenroll(withFactorUID: factorUid) - } catch let error as NSError { - if error.domain == AuthErrorDomain, - error.code == AuthErrorCode.requiresRecentLogin.rawValue || error.code == AuthErrorCode - .userTokenExpired.rawValue { - try await reauthenticateCurrentUser(on: user) - try await multiFactorUser.unenroll(withFactorUID: factorUid) - } else { - throw AuthServiceError - .multiFactorAuth( - "Invalid second factor: \(error.localizedDescription)" - ) - } } // This is the only we to get the actual latest enrolledFactors From a0a1435974bca489b4d0bd283b7a68eae238e35e Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 30 Oct 2025 11:56:24 +0000 Subject: [PATCH 2/4] refactor: cleaner and simpler reauth implementation --- .../Services/AccountService+Email.swift | 61 +------------------ .../Sources/Services/AuthService.swift | 31 ++++------ 2 files changed, 13 insertions(+), 79 deletions(-) diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AccountService+Email.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AccountService+Email.swift index 2d9848cdc7..6f77ef6d67 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AccountService+Email.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AccountService+Email.swift @@ -12,68 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -@preconcurrency import FirebaseAuth import Observation -@MainActor -protocol EmailPasswordOperationReauthentication { - var passwordPrompt: PasswordPromptCoordinator { get } -} - -extension EmailPasswordOperationReauthentication { - func reauthenticate() async throws { - guard let user = Auth.auth().currentUser else { - throw AuthServiceError.reauthenticationRequired("No user currently signed-in") - } - - guard let email = user.email else { - throw AuthServiceError.invalidCredentials("User does not have an email address") - } - - do { - let password = try await passwordPrompt.confirmPassword() - - let credential = EmailAuthProvider.credential(withEmail: email, password: password) - _ = try await Auth.auth().currentUser?.reauthenticate(with: credential) - } catch { - throw AuthServiceError.signInFailed(underlying: error) - } - } -} - -@MainActor -class EmailPasswordDeleteUserOperation: AuthenticatedOperation, - EmailPasswordOperationReauthentication { - let passwordPrompt: PasswordPromptCoordinator - - init(passwordPrompt: PasswordPromptCoordinator) { - self.passwordPrompt = passwordPrompt - } - - func callAsFunction(on user: User) async throws { - try await callAsFunction(on: user) { - try await user.delete() - } - } -} - -class EmailPasswordUpdatePasswordOperation: AuthenticatedOperation, - EmailPasswordOperationReauthentication { - let passwordPrompt: PasswordPromptCoordinator - let newPassword: String - - init(passwordPrompt: PasswordPromptCoordinator, newPassword: String) { - self.passwordPrompt = passwordPrompt - self.newPassword = newPassword - } - - func callAsFunction(on user: User) async throws { - try await callAsFunction(on: user) { - try await user.updatePassword(to: newPassword) - } - } -} - +/// Coordinator for prompting users to enter their password during reauthentication flows @MainActor @Observable public final class PasswordPromptCoordinator { diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift index 2ea0394ee3..6e5737613e 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift @@ -283,18 +283,12 @@ public final class AuthService { public extension AuthService { func deleteUser() async throws { do { - if let user = auth.currentUser, let providerId = signedInCredential?.provider { - if providerId == EmailAuthProviderID { - let operation = EmailPasswordDeleteUserOperation(passwordPrompt: passwordPrompt) - try await operation(on: user) - } else { - // Find provider by matching ID - guard let matchingProvider = providers.first(where: { $0.id == providerId }) else { - throw AuthServiceError.providerNotFound("No provider found for \(providerId)") - } - - try await matchingProvider.provider.deleteUser(user: user) - } + guard let user = auth.currentUser else { + throw AuthServiceError.noCurrentUser + } + + try await withReauthenticationIfNeeded(on: user) { + try await user.delete() } } catch { updateError(message: string.localizedErrorMessage(for: error)) @@ -304,14 +298,13 @@ public extension AuthService { func updatePassword(to password: String) async throws { do { - if let user = auth.currentUser { - let operation = EmailPasswordUpdatePasswordOperation( - passwordPrompt: passwordPrompt, - newPassword: password - ) - try await operation(on: user) + guard let user = auth.currentUser else { + throw AuthServiceError.noCurrentUser + } + + try await withReauthenticationIfNeeded(on: user) { + try await user.updatePassword(to: password) } - } catch { updateError(message: string.localizedErrorMessage(for: error)) throw error From 974491707602de2c88412b0f05f339d49a7be719 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 30 Oct 2025 11:56:47 +0000 Subject: [PATCH 3/4] test: fix tests with MainActor annotation --- .../FirebaseSwiftUIExampleUITests/MFAEnrolmentUITests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/swiftui/FirebaseSwiftUIExample/FirebaseSwiftUIExampleUITests/MFAEnrolmentUITests.swift b/samples/swiftui/FirebaseSwiftUIExample/FirebaseSwiftUIExampleUITests/MFAEnrolmentUITests.swift index f1d5c93a77..931ace293e 100644 --- a/samples/swiftui/FirebaseSwiftUIExample/FirebaseSwiftUIExampleUITests/MFAEnrolmentUITests.swift +++ b/samples/swiftui/FirebaseSwiftUIExample/FirebaseSwiftUIExampleUITests/MFAEnrolmentUITests.swift @@ -400,7 +400,7 @@ final class MFAEnrollmentUITests: XCTestCase { } // MARK: - Helper Methods - + @MainActor private func signInToApp(app: XCUIApplication, email: String) throws { let password = "123456" @@ -436,7 +436,7 @@ final class MFAEnrollmentUITests: XCTestCase { XCTAssertTrue(signedInText.waitForExistence(timeout: 30), "SignedInView should be visible after login") XCTAssertTrue(signedInText.exists, "SignedInView should be visible after login") } - + @MainActor private func navigateToMFAEnrollment(app: XCUIApplication) throws { // Navigate to MFA management app.buttons["mfa-management-button"].tap() From 046e936a7fcaf6da63dc608fe6cb4a2c2c473795 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 30 Oct 2025 12:17:05 +0000 Subject: [PATCH 4/4] refactor: remove obsolete deleteUser from every provider and AccountService file --- .../Services/AppleProviderAuthUI.swift | 5 -- .../Sources/Services/AccountService.swift | 85 ------------------- .../Sources/Services/AuthService.swift | 5 +- .../Services/FacebookProviderAuthUI.swift | 5 -- .../Services/GoogleProviderAuthUI.swift | 5 -- .../Sources/Services/OAuthProviderSwift.swift | 5 -- .../Services/PhoneAuthProviderAuthUI.swift | 5 -- .../Services/TwitterProviderAuthUI.swift | 5 -- 8 files changed, 2 insertions(+), 118 deletions(-) delete mode 100644 FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AccountService.swift diff --git a/FirebaseSwiftUI/FirebaseAppleSwiftUI/Sources/Services/AppleProviderAuthUI.swift b/FirebaseSwiftUI/FirebaseAppleSwiftUI/Sources/Services/AppleProviderAuthUI.swift index a3d829e532..14bc54cc82 100644 --- a/FirebaseSwiftUI/FirebaseAppleSwiftUI/Sources/Services/AppleProviderAuthUI.swift +++ b/FirebaseSwiftUI/FirebaseAppleSwiftUI/Sources/Services/AppleProviderAuthUI.swift @@ -137,11 +137,6 @@ public class AppleProviderSwift: AuthProviderSwift { return credential } - - public func deleteUser(user: User) async throws { - let operation = ProviderDeleteUserOperation(provider: self) - try await operation(on: user) - } } public class AppleProviderAuthUI: AuthProviderUI { diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AccountService.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AccountService.swift deleted file mode 100644 index 954eccf1b4..0000000000 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AccountService.swift +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright 2025 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import AuthenticationServices -import FirebaseAuth - -extension NSError { - var requiresReauthentication: Bool { - domain == AuthErrorDomain && code == AuthErrorCode.requiresRecentLogin.rawValue - } - - var credentialAlreadyInUse: Bool { - domain == AuthErrorDomain && code == AuthErrorCode.credentialAlreadyInUse.rawValue - } -} - -@MainActor -public protocol AuthenticatedOperation { - func callAsFunction(on user: User) async throws - func reauthenticate() async throws -} - -public extension AuthenticatedOperation { - func callAsFunction(on _: User, - _ performOperation: @MainActor () async throws -> Void) async throws { - do { - try await performOperation() - } catch let error as NSError where error.requiresReauthentication { - let token = try await reauthenticate() - try await performOperation() - } catch AuthServiceError.reauthenticationRequired { - let token = try await reauthenticate() - try await performOperation() - } - } -} - -@MainActor -public protocol ProviderOperationReauthentication { - var authProvider: AuthProviderSwift { get } -} - -public extension ProviderOperationReauthentication { - func reauthenticate() async throws { - guard let user = Auth.auth().currentUser else { - throw AuthServiceError.reauthenticationRequired("No user currently signed-in") - } - - do { - let credential = try await authProvider.createAuthCredential() - _ = try await user.reauthenticate(with: credential) - } catch { - throw AuthServiceError.signInFailed(underlying: error) - } - } -} - -@MainActor -public class ProviderDeleteUserOperation: AuthenticatedOperation, - @preconcurrency ProviderOperationReauthentication { - let provider: Provider - - public var authProvider: AuthProviderSwift { provider } - - public init(provider: Provider) { - self.provider = provider - } - - public func callAsFunction(on user: User) async throws { - try await callAsFunction(on: user) { - try await user.delete() - } - } -} diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift index 6e5737613e..b3a32d38d8 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Services/AuthService.swift @@ -18,7 +18,6 @@ import SwiftUI public protocol AuthProviderSwift { @MainActor func createAuthCredential() async throws -> AuthCredential - @MainActor func deleteUser(user: User) async throws } public protocol AuthProviderUI { @@ -286,7 +285,7 @@ public extension AuthService { guard let user = auth.currentUser else { throw AuthServiceError.noCurrentUser } - + try await withReauthenticationIfNeeded(on: user) { try await user.delete() } @@ -301,7 +300,7 @@ public extension AuthService { guard let user = auth.currentUser else { throw AuthServiceError.noCurrentUser } - + try await withReauthenticationIfNeeded(on: user) { try await user.updatePassword(to: password) } diff --git a/FirebaseSwiftUI/FirebaseFacebookSwiftUI/Sources/Services/FacebookProviderAuthUI.swift b/FirebaseSwiftUI/FirebaseFacebookSwiftUI/Sources/Services/FacebookProviderAuthUI.swift index f123f04229..f069551fbb 100644 --- a/FirebaseSwiftUI/FirebaseFacebookSwiftUI/Sources/Services/FacebookProviderAuthUI.swift +++ b/FirebaseSwiftUI/FirebaseFacebookSwiftUI/Sources/Services/FacebookProviderAuthUI.swift @@ -116,11 +116,6 @@ public class FacebookProviderSwift: AuthProviderSwift { ) } } - - public func deleteUser(user: User) async throws { - let operation = ProviderDeleteUserOperation(provider: self) - try await operation(on: user) - } } public class FacebookProviderAuthUI: AuthProviderUI { diff --git a/FirebaseSwiftUI/FirebaseGoogleSwiftUI/Sources/Services/GoogleProviderAuthUI.swift b/FirebaseSwiftUI/FirebaseGoogleSwiftUI/Sources/Services/GoogleProviderAuthUI.swift index c3edb91956..907b683c74 100644 --- a/FirebaseSwiftUI/FirebaseGoogleSwiftUI/Sources/Services/GoogleProviderAuthUI.swift +++ b/FirebaseSwiftUI/FirebaseGoogleSwiftUI/Sources/Services/GoogleProviderAuthUI.swift @@ -68,11 +68,6 @@ public class GoogleProviderSwift: AuthProviderSwift { } } } - - public func deleteUser(user: User) async throws { - let operation = ProviderDeleteUserOperation(provider: self) - try await operation(on: user) - } } public class GoogleProviderAuthUI: AuthProviderUI { diff --git a/FirebaseSwiftUI/FirebaseOAuthSwiftUI/Sources/Services/OAuthProviderSwift.swift b/FirebaseSwiftUI/FirebaseOAuthSwiftUI/Sources/Services/OAuthProviderSwift.swift index 0a39a5fd1d..abdccbcd9b 100644 --- a/FirebaseSwiftUI/FirebaseOAuthSwiftUI/Sources/Services/OAuthProviderSwift.swift +++ b/FirebaseSwiftUI/FirebaseOAuthSwiftUI/Sources/Services/OAuthProviderSwift.swift @@ -113,11 +113,6 @@ public class OAuthProviderSwift: AuthProviderSwift { } } } - - public func deleteUser(user: User) async throws { - let operation = ProviderDeleteUserOperation(provider: self) - try await operation(on: user) - } } public class OAuthProviderAuthUI: AuthProviderUI { diff --git a/FirebaseSwiftUI/FirebasePhoneAuthSwiftUI/Sources/Services/PhoneAuthProviderAuthUI.swift b/FirebaseSwiftUI/FirebasePhoneAuthSwiftUI/Sources/Services/PhoneAuthProviderAuthUI.swift index 77b040b2d1..bf054c2f9c 100644 --- a/FirebaseSwiftUI/FirebasePhoneAuthSwiftUI/Sources/Services/PhoneAuthProviderAuthUI.swift +++ b/FirebaseSwiftUI/FirebasePhoneAuthSwiftUI/Sources/Services/PhoneAuthProviderAuthUI.swift @@ -63,11 +63,6 @@ public class PhoneProviderSwift: PhoneAuthProviderSwift { presentingViewController.present(hostingController, animated: true) } } - - public func deleteUser(user: User) async throws { - let operation = ProviderDeleteUserOperation(provider: self) - try await operation(on: user) - } } public class PhoneAuthProviderAuthUI: AuthProviderUI { diff --git a/FirebaseSwiftUI/FirebaseTwitterSwiftUI/Sources/Services/TwitterProviderAuthUI.swift b/FirebaseSwiftUI/FirebaseTwitterSwiftUI/Sources/Services/TwitterProviderAuthUI.swift index 693d72f0e6..1afb97d1e5 100644 --- a/FirebaseSwiftUI/FirebaseTwitterSwiftUI/Sources/Services/TwitterProviderAuthUI.swift +++ b/FirebaseSwiftUI/FirebaseTwitterSwiftUI/Sources/Services/TwitterProviderAuthUI.swift @@ -42,11 +42,6 @@ public class TwitterProviderSwift: AuthProviderSwift { } } } - - public func deleteUser(user: User) async throws { - let operation = ProviderDeleteUserOperation(provider: self) - try await operation(on: user) - } } public class TwitterProviderAuthUI: AuthProviderUI {