feat(auth): implement generateVerifyAndChangeEmailLink API#1195
feat(auth): implement generateVerifyAndChangeEmailLink API#1195siddharthmohan619-eng wants to merge 4 commits intofirebase:mainfrom
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 new feature to programmatically generate out-of-band email action links specifically for the "Verify and Change Email" flow. This allows developers to create links that users can use to verify a new email address before it is officially updated on their profile, enhancing the flexibility of email management within the authentication system. 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. Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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
This pull request introduces a new API for generating a 'verify and change email' link. The implementation is sound, but I've provided feedback to improve consistency with existing code patterns in AbstractFirebaseAuth.java by adding Javadoc, argument validation, and refactoring to a common helper method pattern. I've also suggested improvements to the test coverage to ensure the new API is fully tested, and pointed out a minor cleanup in the test file.
| public String generateVerifyAndChangeEmailLink(@NonNull String email, @NonNull String newEmail) | ||
| throws FirebaseAuthException { | ||
| return generateVerifyAndChangeEmailLink(email, newEmail, null); | ||
| } | ||
|
|
||
| public String generateVerifyAndChangeEmailLink( | ||
| @NonNull String email, @NonNull String newEmail, @Nullable ActionCodeSettings settings) | ||
| throws FirebaseAuthException { | ||
| return getUserManager().getEmailActionLink( | ||
| EmailLinkType.VERIFY_AND_CHANGE_EMAIL, email, newEmail, settings); | ||
| } | ||
|
|
||
| public ApiFuture<String> generateVerifyAndChangeEmailLinkAsync( | ||
| @NonNull String email, @NonNull String newEmail) { | ||
| return generateVerifyAndChangeEmailLinkAsync(email, newEmail, null); | ||
| } | ||
|
|
||
| public ApiFuture<String> generateVerifyAndChangeEmailLinkAsync( | ||
| @NonNull String email, @NonNull String newEmail, @Nullable ActionCodeSettings settings) { | ||
| return new CallableOperation<String, FirebaseAuthException>() { | ||
| @Override | ||
| protected String execute() throws FirebaseAuthException { | ||
| return generateVerifyAndChangeEmailLink(email, newEmail, settings); | ||
| } | ||
| }.callAsync(firebaseApp); | ||
| } |
There was a problem hiding this comment.
The new generateVerifyAndChangeEmailLink methods are a great addition. To align them with the existing patterns in this class and improve maintainability, I suggest adding Javadoc for the new public methods and refactoring to use a private ...Op helper method. This helper would centralize the logic and argument validation (e.g., using checkArgument for email and newEmail), which is a common pattern in this file.
/**
* Generates the out-of-band email action link for the verify and change email flow.
*
* @param email The user's current email.
* @param newEmail The user's new email.
* @return A verify and change email link.
* @throws IllegalArgumentException If email or newEmail are null or empty.
* @throws FirebaseAuthException If an error occurs while generating the link.
*/
public String generateVerifyAndChangeEmailLink(@NonNull String email, @NonNull String newEmail)
throws FirebaseAuthException {
return generateVerifyAndChangeEmailLink(email, newEmail, null);
}
/**
* Generates the out-of-band email action link for the verify and change email flow.
*
* @param email The user's current email.
* @param newEmail The user's new email.
* @param settings The action code settings.
* @return A verify and change email link.
* @throws IllegalArgumentException If email or newEmail are null or empty.
* @throws FirebaseAuthException If an error occurs while generating the link.
*/
public String generateVerifyAndChangeEmailLink(
@NonNull String email, @NonNull String newEmail, @Nullable ActionCodeSettings settings)
throws FirebaseAuthException {
return generateVerifyAndChangeEmailLinkOp(email, newEmail, settings).call();
}
/**
* Asynchronously generates the out-of-band email action link for the verify and change email
* flow.
*
* @param email The user's current email.
* @param newEmail The user's new email.
* @return An {@code ApiFuture} which will complete with the generated link.
* @throws IllegalArgumentException If email or newEmail are null or empty.
*/
public ApiFuture<String> generateVerifyAndChangeEmailLinkAsync(
@NonNull String email, @NonNull String newEmail) {
return generateVerifyAndChangeEmailLinkAsync(email, newEmail, null);
}
/**
* Asynchronously generates the out-of-band email action link for the verify and change email
* flow.
*
* @param email The user's current email.
* @param newEmail The user's new email.
* @param settings The action code settings.
* @return An {@code ApiFuture} which will complete with the generated link.
* @throws IllegalArgumentException If email or newEmail are null or empty.
*/
public ApiFuture<String> generateVerifyAndChangeEmailLinkAsync(
@NonNull String email, @NonNull String newEmail, @Nullable ActionCodeSettings settings) {
return generateVerifyAndChangeEmailLinkOp(email, newEmail, settings).callAsync(firebaseApp);
}
private CallableOperation<String, FirebaseAuthException> generateVerifyAndChangeEmailLinkOp(
final String email, final String newEmail, final ActionCodeSettings settings) {
checkArgument(!Strings.isNullOrEmpty(email), "email must not be null or empty");
checkArgument(!Strings.isNullOrEmpty(newEmail), "newEmail must not be null or empty");
final FirebaseUserManager userManager = getUserManager();
return new CallableOperation<String, FirebaseAuthException>() {
@Override
protected String execute() throws FirebaseAuthException {
return userManager.getEmailActionLink(
EmailLinkType.VERIFY_AND_CHANGE_EMAIL, email, newEmail, settings);
}
};
}There was a problem hiding this comment.
@gemini-code-assist Thanks for the detailed review! I've updated the PR to fully align with the existing patterns in this class:
Refactored Logic: Centralized the logic into a private CallableOperation helper method (generateVerifyAndChangeEmailLinkOp).
Validation: Added checkArgument to validate that email and newEmail are not null or empty.
Documentation: Added the requested Javadoc for the new public and async methods.
The build is fully passing locally. Ready for the next stage of review lets goo
There was a problem hiding this comment.
The changes look good and align with the suggested improvements. Thank you for addressing the feedback.
| import com.google.firebase.auth.multitenancy.TenantAwareFirebaseAuth; | ||
| import com.google.firebase.auth.multitenancy.TenantManager; | ||
| import com.google.firebase.internal.ApiClientUtils; | ||
| import com.google.firebase.internal.CallableOperation; |
There was a problem hiding this comment.
This import appears to be unused and can be removed to keep the code clean.
There was a problem hiding this comment.
Yep, I saw this and I fixed it bro
| @Test | ||
| public void testGenerateVerifyAndChangeEmailLink() throws Exception { | ||
| TestResponseInterceptor interceptor = initializeAppForUserManagement( | ||
| TestUtils.loadResource("generateEmailLink.json")); | ||
| String link = FirebaseAuth.getInstance() | ||
| .generateVerifyAndChangeEmailLinkAsync("test@example.com", "new@example.com").get(); | ||
| assertEquals("https://mock-oob-link.for.auth.tests", link); | ||
| checkRequestHeaders(interceptor); | ||
|
|
||
| GenericJson parsed = parseRequestContent(interceptor); | ||
| assertEquals(4, parsed.size()); | ||
| assertEquals("test@example.com", parsed.get("email")); | ||
| assertEquals("new@example.com", parsed.get("newEmail")); | ||
| assertEquals("VERIFY_AND_CHANGE_EMAIL", parsed.get("requestType")); | ||
| assertTrue((Boolean) parsed.get("returnOobLink")); | ||
| } |
There was a problem hiding this comment.
This is a good test for the success case. To make the tests more comprehensive, please consider adding another test case that calls generateVerifyAndChangeEmailLinkAsync with an ActionCodeSettings object. This will ensure that the settings are correctly passed in the request payload, similar to how it's tested for other email action links (e.g., testGeneratePasswordResetLinkWithSettings).
There was a problem hiding this comment.
@gemini-code-assist Thank you for the review! I have addressed all the feedback in the latest commit:
Refactored Logic: Updated AbstractFirebaseAuth.java to use the internal getEmailActionLink and getEmailActionLinkAsync helper methods.
Validation & Javadoc: Added checkArgument to ensure emails are not null/empty, and included the requested Javadoc.
Imports: Removed the unused CallableOperation import in the test file.
Test Coverage: Added testGenerateVerifyAndChangeEmailLinkWithSettings to verify that ActionCodeSettings are correctly parsed and passed in the payload.
Public API: Exposed the new methods in FirebaseAuth.java so they are accessible.
Linting: Fixed all Checkstyle line-length and formatting requirements.
There was a problem hiding this comment.
It's great to see that you've added testGenerateVerifyAndChangeEmailLinkWithSettings to ensure ActionCodeSettings are correctly handled. This significantly improves the test coverage for the new API. Well done!
1 participant
This PR adds support for generating out-of-band email action links for the Verify and Change Email flow. This allows developers to programmatically generate links for users to verify a new email address before it is updated on their profile.
Key Changes:
AbstractFirebaseAuth.java: Added generateVerifyAndChangeEmailLink() and the asynchronous generateVerifyAndChangeEmailLinkAsync() implementation using CallableOperation.
FirebaseAuth.java: Exposed the new methods to the public API.
FirebaseUserManager.java: Updated the backend manager to handle the VERIFY_AND_CHANGE_EMAIL request type.
FirebaseUserManagerTest.java: Added comprehensive unit tests to verify:
Correct request payload (email, newEmail, and requestType).
Proper handling of ActionCodeSettings.
Correct parsing of the returned OOB link.
Testing:
Verified via mvn test -Dtest=FirebaseUserManagerTest. All 147 tests passed, including the new test case.