Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,33 @@ protected DeleteUsersResult execute() throws FirebaseAuthException {
};
}

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);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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);
      }
    };
  }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good and align with the suggested improvements. Thank you for addressing the feedback.


/**
* Generates the out-of-band email action link for password reset flows for the specified email
* address.
Expand Down
17 changes: 14 additions & 3 deletions src/main/java/com/google/firebase/auth/FirebaseUserManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,20 @@ String createSessionCookie(String idToken,

String getEmailActionLink(EmailLinkType type, String email,
@Nullable ActionCodeSettings settings) throws FirebaseAuthException {
return getEmailActionLink(type, email, null, settings);
}

String getEmailActionLink(EmailLinkType type, String email, @Nullable String newEmail,
@Nullable ActionCodeSettings settings) throws FirebaseAuthException {
ImmutableMap.Builder<String, Object> payload = ImmutableMap.<String, Object>builder()
.put("requestType", type.name())
.put("email", email)
.put("returnOobLink", true);
.put("requestType", type.name())
.put("email", email)
.put("returnOobLink", true);

if (newEmail != null) {
payload.put("newEmail", newEmail);
}

if (settings != null) {
payload.putAll(settings.getProperties());
}
Expand Down Expand Up @@ -389,6 +399,7 @@ enum EmailLinkType {
VERIFY_EMAIL,
EMAIL_SIGNIN,
PASSWORD_RESET,
VERIFY_AND_CHANGE_EMAIL,
}

static FirebaseUserManager createUserManager(FirebaseApp app, String tenantId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
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;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This import appears to be unused and can be removed to keep the code clean.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I saw this and I fixed it bro

import com.google.firebase.internal.FirebaseProcessEnvironment;
import com.google.firebase.internal.SdkUtils;
import com.google.firebase.testing.MultiRequestMockHttpTransport;
Expand Down Expand Up @@ -1510,6 +1511,23 @@ public void testGenerateEmailVerificationLink() throws Exception {
assertEquals("VERIFY_EMAIL", parsed.get("requestType"));
assertTrue((Boolean) parsed.get("returnOobLink"));
}

@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"));
}
Comment on lines +1515 to +1530
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!


@Test
public void testGenerateESignInWithEmailLinkNoEmail() throws Exception {
Expand All @@ -1529,6 +1547,7 @@ public void testGenerateESignInWithEmailLinkNoEmail() throws Exception {
}
}


@Test
public void testGenerateESignInWithEmailLinkNullSettings() throws Exception {
initializeAppForUserManagement();
Expand Down Expand Up @@ -2864,6 +2883,7 @@ public void testTenantAwareDeleteSamlProviderConfig() throws Exception {
checkUrl(interceptor, "DELETE", expectedUrl);
}


@Test
public void testCreateOidcProviderFromEmulatorAuth() throws Exception {
FirebaseProcessEnvironment.setenv("FIREBASE_AUTH_EMULATOR_HOST", AUTH_EMULATOR);
Expand Down