feat: implement legacyFetchSignInWithEmail behaviour for Angular and React#1343
feat: implement legacyFetchSignInWithEmail behaviour for Angular and React#1343russellwheatley wants to merge 16 commits into
Conversation
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 robust mechanism for handling authentication errors where a user attempts to sign in with a different provider than previously used for the same email. It provides a new behavior that fetches and presents alternative sign-in methods, along with corresponding UI components for React and Angular, to guide users through the recovery process. This significantly improves the user experience by preventing dead ends during sign-in and offering clear paths to resolve credential conflicts. 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a new legacyFetchSignInWithEmail behavior to handle OAuth account mismatch flows, allowing users to recover their accounts by suggesting previously used sign-in methods. The changes include core logic for error handling and state management, new UI components for React and Angular to display recovery options, and updated translations. My feedback suggests a minor improvement to the attachEmailToError function to ensure safer cloning of the customData object.
| /** | ||
| * Fetches previous sign-in methods for OAuth account mismatch flows. | ||
| * | ||
| * @returns A behavior that populates legacy sign-in recovery state. | ||
| */ |
There was a problem hiding this comment.
Can you please add a bit more context here? Something like:
Adds support for deprecated methods and behavior (like
fetchSignInMethodsForEmail()) when email enumeration protection is disabled.If your web app relies on this legacy behaviors, we recommend that you begin migrating away from doing so, and enable email enumeration protection as soon as you can.
jhuleatt
left a comment
There was a problem hiding this comment.
Overall LGTM, I just want to confirm that all defaults and demos have this legacy behavior disabled (except for the demo that explicitly shows this flow). This won't work by default on any Firebase project created after 2023
| }, | ||
| { | ||
| name: "Legacy Recovery Demo", | ||
| description: "Use this screen to test wrong-provider recovery for email/password and OAuth attempts.", |
There was a problem hiding this comment.
| description: "Use this screen to test wrong-provider recovery for email/password and OAuth attempts.", | |
| description: "Use this screen to test wrong-provider recovery for email/password and OAuth attempts in a project that has email enumeration protection disabled.", |
| expect(redirectErrorElement).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("renders legacy recovery by default", async () => { |
There was a problem hiding this comment.
What does "by default" mean here? legacyFetchSignInWithEmail should be an optional behavior, not the default. From what I can tell, I think that's the case, but just making sure.
| export class SignInAuthScreenComponent { | ||
| private ui = injectUI(); | ||
| /** Whether to show the default legacy sign-in recovery UI. */ | ||
| showLegacySignInRecovery = input(true); |
There was a problem hiding this comment.
Let's make sure all the defaults have legacy recovery disabled
Uh oh!
There was an error while loading. Please reload this page.