feat(auth): legacyFetchSignInWithEmail config option for users still wishing to use auth.fetchSignInWithEmail()#1330
feat(auth): legacyFetchSignInWithEmail config option for users still wishing to use auth.fetchSignInWithEmail()#1330russellwheatley wants to merge 2 commits intomainfrom
legacyFetchSignInWithEmail config option for users still wishing to use auth.fetchSignInWithEmail()#1330Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a legacy sign-in recovery feature to FirebaseUI, allowing users to recover their accounts when they attempt to sign in with an incorrect provider. It adds a new configuration option to enable this feature, modifies error handling to accommodate the new recovery flow, and introduces a new UI component to display the recovery options. Additionally, it includes new tests to ensure the feature functions as expected. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces a new legacy sign-in recovery feature for Firebase SwiftUI authentication. This includes adding a legacySignInRecoveryPresented error type, new data structures (LegacySignInOption, LegacySignInRecoveryContext), and methods within AuthService to manage and present recovery options. The AuthConfiguration now includes a legacyFetchSignInWithEmail flag to enable this feature. A new LegacySignInRecoveryView is added to display these options, and AuthPickerView is updated to present this view as a sheet. Error handling in various sign-in flows is adjusted to correctly trigger this recovery mechanism. A critical fix ensures that the legacySignInRecoveryPresented error, which is an internal signal, does not trigger a user-facing error alert.
| func legacySignInRecoveryResolvesEmailLinkProvider() async throws { | ||
| let email = createEmail() | ||
| try await clearAuthEmulatorState() | ||
| try await createEmailLinkOnlyUser(email: email) | ||
|
|
||
| let service = try await prepareLegacyRecoveryService( | ||
| legacyFetchSignInWithEmail: true, | ||
| includeEmailLinkProvider: true | ||
| ) | ||
|
|
||
| do { | ||
| try await service.signIn(email: email, password: kPassword) | ||
| Issue.record("Expected email/password sign-in to fail for an email-link account") | ||
| } catch { | ||
| if case .legacySignInRecoveryPresented = error as? AuthServiceError { | ||
| // Expected path. | ||
| } else { | ||
| Issue.record("Expected legacy recovery error, got: \(error)") | ||
| } | ||
| } | ||
|
|
||
| let recovery = service.legacySignInRecovery | ||
| #expect(recovery?.email == email) | ||
| #expect(recovery?.options.contains(where: { $0.id == "emailLink" }) == true) | ||
| } | ||
|
|
||
| @Test | ||
| @MainActor | ||
| func legacySignInRecoveryFallsBackWhenProviderNotEnabled() async throws { | ||
| let email = createEmail() | ||
| try await clearAuthEmulatorState() | ||
| try await createEmailLinkOnlyUser(email: email) | ||
|
|
||
| let service = try await prepareLegacyRecoveryService( | ||
| legacyFetchSignInWithEmail: true, | ||
| includeEmailLinkProvider: false | ||
| ) | ||
|
|
||
| do { | ||
| try await service.signIn(email: email, password: kPassword) | ||
| Issue.record("Expected email/password sign-in to fail for an email-link account") | ||
| } catch { | ||
| #expect((error as? AuthServiceError) == nil || { | ||
| if case .legacySignInRecoveryPresented = error as? AuthServiceError { | ||
| return false | ||
| } | ||
| return true | ||
| }()) | ||
| } | ||
|
|
||
| #expect(service.legacySignInRecovery == nil) | ||
| } | ||
|
|
||
| @Test | ||
| @MainActor | ||
| func legacySignInRecoveryDisabledPreservesExistingFailure() async throws { | ||
| let email = createEmail() | ||
| try await clearAuthEmulatorState() | ||
| try await createEmailLinkOnlyUser(email: email) | ||
|
|
||
| let service = try await prepareLegacyRecoveryService( | ||
| legacyFetchSignInWithEmail: false, | ||
| includeEmailLinkProvider: true | ||
| ) | ||
|
|
||
| do { | ||
| try await service.signIn(email: email, password: kPassword) | ||
| Issue.record("Expected email/password sign-in to fail for an email-link account") | ||
| } catch { | ||
| #expect((error as? AuthServiceError) == nil || { | ||
| if case .legacySignInRecoveryPresented = error as? AuthServiceError { | ||
| return false | ||
| } | ||
| return true | ||
| }()) | ||
| } | ||
|
|
||
| #expect(service.legacySignInRecovery == nil) | ||
| } |
There was a problem hiding this comment.
The new unit tests (legacySignInRecoveryResolvesEmailLinkProvider, legacySignInRecoveryFallsBackWhenProviderNotEnabled, legacySignInRecoveryDisabledPreservesExistingFailure) provide good coverage for the core logic and edge cases of the legacy sign-in recovery feature, ensuring its reliability.
@Test
@MainActor
func legacySignInRecoveryResolvesEmailLinkProvider() async throws {
let email = createEmail()
try await clearAuthEmulatorState()
try await createEmailLinkOnlyUser(email: email)
let service = try await prepareLegacyRecoveryService(
legacyFetchSignInWithEmail: true,
includeEmailLinkProvider: true
)
do {
try await service.signIn(email: email, password: kPassword)
Issue.record("Expected email/password sign-in to fail for an email-link account")
} catch {
if case .legacySignInRecoveryPresented = error as? AuthServiceError {
// Expected path.
} else {
Issue.record("Expected legacy recovery error, got: \(error)")
}
}
let recovery = service.legacySignInRecovery
#expect(recovery?.email == email)
#expect(recovery?.options.contains(where: { $0.id == "emailLink" }) == true)
}
@Test
@MainActor
func legacySignInRecoveryFallsBackWhenProviderNotEnabled() async throws {
let email = createEmail()
try await clearAuthEmulatorState()
try await createEmailLinkOnlyUser(email: email)
let service = try await prepareLegacyRecoveryService(
legacyFetchSignInWithEmail: true,
includeEmailLinkProvider: false
)
do {
try await service.signIn(email: email, password: kPassword)
Issue.record("Expected email/password sign-in to fail for an email-link account")
} catch {
#expect((error as? AuthServiceError) == nil || {
if case .legacySignInRecoveryPresented = error as? AuthServiceError {
return false
}
return true
}())
}
#expect(service.legacySignInRecovery == nil)
}
@Test
@MainActor
func legacySignInRecoveryDisabledPreservesExistingFailure() async throws {
let email = createEmail()
try await clearAuthEmulatorState()
try await createEmailLinkOnlyUser(email: email)
let service = try await prepareLegacyRecoveryService(
legacyFetchSignInWithEmail: false,
includeEmailLinkProvider: true
)
do {
try await service.signIn(email: email, password: kPassword)
Issue.record("Expected email/password sign-in to fail for an email-link account")
} catch {
#expect((error as? AuthServiceError) == nil || {
if case .legacySignInRecoveryPresented = error as? AuthServiceError {
return false
}
return true
}())
}
#expect(service.legacySignInRecovery == nil)
}
FirebaseSwiftUI/FirebaseAppleSwiftUI/Sources/Views/SignInWithAppleButton.swift
Show resolved
Hide resolved
| public struct LegacySignInOption: Identifiable, Equatable { | ||
| public let id: String | ||
| public let displayName: String | ||
|
|
||
| public init(id: String, displayName: String) { | ||
| self.id = id | ||
| self.displayName = displayName | ||
| } | ||
| } | ||
|
|
||
| public struct LegacySignInRecoveryContext: Identifiable, Equatable { | ||
| public let id = UUID() | ||
| public let email: String | ||
| public let options: [LegacySignInOption] | ||
| public let unavailableProviders: [String] | ||
|
|
||
| public init(email: String, | ||
| options: [LegacySignInOption], | ||
| unavailableProviders: [String] = []) { | ||
| self.email = email | ||
| self.options = options | ||
| self.unavailableProviders = unavailableProviders | ||
| } | ||
|
|
||
| public static func == (lhs: LegacySignInRecoveryContext, | ||
| rhs: LegacySignInRecoveryContext) -> Bool { | ||
| lhs.id == rhs.id | ||
| } |
There was a problem hiding this comment.
The introduction of LegacySignInOption and LegacySignInRecoveryContext provides clear and structured data models for managing the new sign-in recovery flow. Conforming to Identifiable and Equatable is good practice for SwiftUI integration.
public struct LegacySignInOption: Identifiable, Equatable {
public let id: String
public let displayName: String
public init(id: String, displayName: String) {
self.id = id
self.displayName = displayName
}
}
public struct LegacySignInRecoveryContext: Identifiable, Equatable {
public let id = UUID()
public let email: String
public let options: [LegacySignInOption]
public let unavailableProviders: [String]
public init(email: String,
options: [LegacySignInOption],
unavailableProviders: [String] = []) {
self.email = email
self.options = options
self.unavailableProviders = unavailableProviders
}
public static func == (lhs: LegacySignInRecoveryContext,
rhs: LegacySignInRecoveryContext) -> Bool {
lhs.id == rhs.id
}
}
e2eTest/FirebaseSwiftUIExample/FirebaseSwiftUIExampleUITests/TestUtils.swift
Show resolved
Hide resolved
russellwheatley
force-pushed
the
fetchsigninwithemail-feature
branch
from
bd1e9fd to
179f5ba
Compare
russellwheatley
changed the title
legacyFetchSignInWithEmail config option for users still wishing to use auth.fetchSignInWithEmail()
1 participant
Hey there! So you want to contribute to FirebaseUI? Before you file this pull request, follow these steps: