-
Notifications
You must be signed in to change notification settings - Fork 485
feat: Facebook auth re-authentication to allow deleting user #1256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1ed8dcc
323eef6
37970aa
48c1b24
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ public protocol GoogleProviderAuthUIProtocol: ExternalAuthProvider { | |
|
||
public protocol FacebookProviderAuthUIProtocol: ExternalAuthProvider { | ||
@MainActor func signInWithFacebook(isLimitedLogin: Bool) async throws -> AuthCredential | ||
@MainActor func deleteUser(user: User) async throws | ||
} | ||
|
||
public protocol PhoneAuthProviderAuthUIProtocol: ExternalAuthProvider { | ||
|
@@ -82,17 +83,28 @@ public final class AuthService { | |
public var authenticationFlow: AuthenticationFlow = .login | ||
public var errorMessage = "" | ||
public let passwordPrompt: PasswordPromptCoordinator = .init() | ||
|
||
public var googleProvider: (any GoogleProviderAuthUIProtocol)? | ||
public var facebookProvider: (any FacebookProviderAuthUIProtocol)? | ||
public var phoneAuthProvider: (any PhoneAuthProviderAuthUIProtocol)? | ||
private var googleProvider: (any GoogleProviderAuthUIProtocol)? | ||
private var facebookProvider: (any FacebookProviderAuthUIProtocol)? | ||
private var phoneAuthProvider: (any PhoneAuthProviderAuthUIProtocol)? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are private now because they shouldn't be accessed, should use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I'd rather have the private apis be named |
||
|
||
private var listenerManager: AuthListenerManager? | ||
private var signedInCredential: AuthCredential? | ||
|
||
private var providers: [ExternalAuthProvider] = [] | ||
public func register(provider: ExternalAuthProvider) { | ||
providers.append(provider) | ||
switch provider { | ||
case let google as GoogleProviderAuthUIProtocol: | ||
googleProvider = google | ||
providers.append(provider) | ||
case let facebook as FacebookProviderAuthUIProtocol: | ||
facebookProvider = facebook | ||
providers.append(provider) | ||
case let phone as PhoneAuthProviderAuthUIProtocol: | ||
phoneAuthProvider = phone | ||
providers.append(provider) | ||
default: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This now assigns providers correctly without having to manually set + call |
||
break | ||
} | ||
} | ||
|
||
public func renderButtons(spacing: CGFloat = 16) -> AnyView { | ||
|
@@ -119,7 +131,7 @@ public final class AuthService { | |
get throws { | ||
guard let provider = facebookProvider else { | ||
throw AuthServiceError | ||
.notConfiguredProvider("`FacebookProviderSwift` has not been configured") | ||
.notConfiguredProvider("`FacebookProviderAuthUI` has not been configured") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this not just be a |
||
} | ||
return provider | ||
} | ||
|
@@ -215,7 +227,8 @@ public final class AuthService { | |
if shouldHandleAnonymousUpgrade { | ||
try await handleAutoUpgradeAnonymousUser(credentials: credentials) | ||
} else { | ||
try await auth.signIn(with: credentials) | ||
let result = try await auth.signIn(with: credentials) | ||
signedInCredential = result.credential | ||
} | ||
updateAuthenticationState() | ||
} catch { | ||
|
@@ -247,9 +260,13 @@ public final class AuthService { | |
public extension AuthService { | ||
func deleteUser() async throws { | ||
do { | ||
if let user = auth.currentUser { | ||
let operation = EmailPasswordDeleteUserOperation(passwordPrompt: passwordPrompt) | ||
try await operation(on: user) | ||
if let user = auth.currentUser, let providerId = signedInCredential?.provider { | ||
if providerId == "password" { | ||
let operation = EmailPasswordDeleteUserOperation(passwordPrompt: passwordPrompt) | ||
try await operation(on: user) | ||
} else if providerId == "facebook.com" { | ||
try await safeFacebookProvider.deleteUser(user: user) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bit tricky deleting user with Facebook re-authentication but I found the best way was to have it on the provider itself which makes sense to me anyway. |
||
} | ||
} | ||
|
||
} catch { | ||
|
@@ -295,7 +312,8 @@ public extension AuthService { | |
let credential = EmailAuthProvider.credential(withEmail: email, password: password) | ||
try await handleAutoUpgradeAnonymousUser(credentials: credential) | ||
} else { | ||
try await auth.createUser(withEmail: email, password: password) | ||
let result = try await auth.createUser(withEmail: email, password: password) | ||
signedInCredential = result.credential | ||
} | ||
updateAuthenticationState() | ||
} catch { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
// | ||
// AccountService+Facebook.swift | ||
// FirebaseUI | ||
// | ||
// Created by Russell Wheatley on 14/05/2025. | ||
// | ||
|
||
@preconcurrency import FirebaseAuth | ||
import FirebaseAuthSwiftUI | ||
import Observation | ||
|
||
protocol FacebookOperationReauthentication { | ||
var facebookProvider: FacebookProviderAuthUI { get } | ||
} | ||
|
||
extension FacebookOperationReauthentication { | ||
@MainActor func reauthenticate() async throws -> AuthenticationToken { | ||
guard let user = Auth.auth().currentUser else { | ||
throw AuthServiceError.reauthenticationRequired("No user currently signed-in") | ||
} | ||
|
||
do { | ||
let credential = try await facebookProvider | ||
.signInWithFacebook(isLimitedLogin: facebookProvider.isLimitedLogin) | ||
try await user.reauthenticate(with: credential) | ||
|
||
return .firebase("") | ||
} catch { | ||
throw AuthServiceError.signInFailed(underlying: error) | ||
} | ||
} | ||
} | ||
|
||
@MainActor | ||
class FacebookDeleteUserOperation: AuthenticatedOperation, | ||
@preconcurrency FacebookOperationReauthentication { | ||
let facebookProvider: FacebookProviderAuthUI | ||
init(facebookProvider: FacebookProviderAuthUI) { | ||
self.facebookProvider = facebookProvider | ||
} | ||
|
||
func callAsFunction(on user: User) async throws { | ||
try await callAsFunction(on: user) { | ||
try await user.delete() | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,8 +10,8 @@ import FirebaseAuthSwiftUI | |
public extension AuthService { | ||
@discardableResult | ||
func withFacebookSignIn(scopes scopes: [String]? = nil) -> AuthService { | ||
facebookProvider = FacebookProviderAuthUI(scopes: scopes) | ||
register(provider: facebookProvider!) | ||
FacebookProviderAuthUI.configureProvider(scopes: scopes) | ||
register(provider: FacebookProviderAuthUI.shared) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched to having a private init() for Facebook provider and making it a singleton which is kept on |
||
return self | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,11 +9,6 @@ let kFacebookEmailScope = "email" | |
let kFacebookProfileScope = "public_profile" | ||
let kDefaultFacebookScopes = [kFacebookEmailScope, kFacebookProfileScope] | ||
|
||
public enum FacebookLoginType { | ||
case classic | ||
case limitedLogin | ||
} | ||
|
||
public enum FacebookProviderError: Error { | ||
case signInCancelled(String) | ||
case configurationInvalid(String) | ||
|
@@ -27,35 +22,52 @@ public class FacebookProviderAuthUI: FacebookProviderAuthUIProtocol { | |
let shortName = "Facebook" | ||
let providerId = "facebook.com" | ||
private let loginManager = LoginManager() | ||
private var rawNonce: String | ||
private var shaNonce: String | ||
private var rawNonce: String? | ||
private var shaNonce: String? | ||
// Needed for reauthentication | ||
var isLimitedLogin: Bool = true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need this to track the initial login for re-authentication. |
||
|
||
@MainActor private static var _shared: FacebookProviderAuthUI = | ||
.init(scopes: kDefaultFacebookScopes) | ||
|
||
@MainActor public static var shared: FacebookProviderAuthUI { | ||
return _shared | ||
} | ||
|
||
public init(scopes: [String]? = nil) { | ||
@MainActor public static func configureProvider(scopes: [String]? = nil) { | ||
_shared = FacebookProviderAuthUI(scopes: scopes) | ||
} | ||
|
||
private init(scopes: [String]? = nil) { | ||
self.scopes = scopes ?? kDefaultFacebookScopes | ||
rawNonce = CommonUtils.randomNonce() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved this logic so it recreates every time for limited login otherwise reauthentication will fail as the credential is the same. |
||
shaNonce = CommonUtils.sha256Hash(of: rawNonce) | ||
} | ||
|
||
@MainActor public func authButton() -> AnyView { | ||
AnyView(SignInWithFacebookButton()) | ||
} | ||
|
||
public func deleteUser(user: User) async throws { | ||
let operation = FacebookDeleteUserOperation(facebookProvider: self) | ||
try await operation(on: user) | ||
} | ||
|
||
@MainActor public func signInWithFacebook(isLimitedLogin: Bool) async throws -> AuthCredential { | ||
let trackingStatus = ATTrackingManager.trackingAuthorizationStatus | ||
let tracking: LoginTracking = trackingStatus != .authorized ? .limited : | ||
(isLimitedLogin ? .limited : .enabled) | ||
let loginType: LoginTracking = isLimitedLogin ? .limited : .enabled | ||
self.isLimitedLogin = isLimitedLogin | ||
|
||
guard let configuration: LoginConfiguration = { | ||
if tracking == .limited { | ||
if loginType == .limited { | ||
rawNonce = CommonUtils.randomNonce() | ||
shaNonce = CommonUtils.sha256Hash(of: rawNonce!) | ||
return LoginConfiguration( | ||
permissions: scopes, | ||
tracking: tracking, | ||
nonce: shaNonce | ||
tracking: loginType, | ||
nonce: shaNonce! | ||
) | ||
} else { | ||
return LoginConfiguration( | ||
permissions: scopes, | ||
tracking: tracking | ||
tracking: loginType | ||
) | ||
} | ||
}() else { | ||
|
@@ -74,10 +86,8 @@ public class FacebookProviderAuthUI: FacebookProviderAuthUIProtocol { | |
case .cancelled: | ||
continuation | ||
.resume(throwing: FacebookProviderError.signInCancelled("User cancelled sign-in")) | ||
// showCanceledAlert = true | ||
case let .failed(error): | ||
continuation.resume(throwing: error) | ||
// errorMessage = authService.string.localizedErrorMessage(for: error) | ||
Comment on lines
-77
to
-80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. errorMessage and alert are handled upstream, removed here. |
||
case .success: | ||
continuation.resume() | ||
} | ||
|
@@ -109,7 +119,7 @@ public class FacebookProviderAuthUI: FacebookProviderAuthUIProtocol { | |
if let idToken = AuthenticationToken.current { | ||
let credential = OAuthProvider.credential(withProviderID: providerId, | ||
idToken: idToken.tokenString, | ||
rawNonce: rawNonce) | ||
rawNonce: rawNonce!) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not immediately clear from the code that this optional will never be |
||
return credential | ||
} else { | ||
throw FacebookProviderError | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made a number of API
@MainActor
to stop Xcode compiler error for passing User around which is non-sendable at the moment.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM, you could add TODOs here since User will eventually become Sendable but there's a strong argument for just keeping this API
@MainActor
regardless.