Remove GAE Users service API usage

[jianglai]

This is the last remaining GAE API that we depend on. By removing it, we are able to remove all common GAE dependencies as well.

To merge this PR, we need to create console User objects that have the same email address as the RegistrarPoc objects' login_email_address and copy over the existing registry lock hashes and salts.

We are also able to simply the code base by removing some redundant logic like AuthMethod (API is now the only supported one) and UserAuthInfo (console user is now the only supported one)

There are several behavioral changes that are worth noting:

1. The XsrfTokenManager now uses the console user's email address to mint and verify the token. Previously, only email addresses returned by the GAE Users service are used, whereas a blank email address will be used if the user is logged in as a console user. I believe this was an oversight that is now corrected.
2. The legacy console will return 401 when no user is logged in, instead of redirecting to the Users service login flow.
3. The logout URL in the legacy console is changed to use the IAP logout flow. It will clear the cookie and redirect the users to IAP login page (tested on QA).
4. The screenshot changes are mostly due to the console users lacking a display name and therefore showing the email address instead. Some changes are due to using the console user's email address as the registry lock email address, which is being fixed in Add DB column for separate rlock email address #2413 and its follow-up RPs.

This change is 

[jianglai]

Reviewed 201 of 201 files at r1, 13 of 13 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @param)

---

core/src/main/java/google/registry/request/auth/AuthResult.java line 30 at r1 (raw file):

Previously, github-advanced-security[bot] wrote…

Spurious Javadoc @param tags

@param tag "authLevel" does not match any actual type parameter of type "AuthResult".

Show more details

Done.

---

core/src/main/java/google/registry/request/auth/AuthResult.java line 31 at r1 (raw file):

Previously, github-advanced-security[bot] wrote…

Spurious Javadoc @param tags

@param tag "user" does not match any actual type parameter of type "AuthResult".

Show more details

Done.

---

core/src/main/java/google/registry/request/auth/AuthResult.java line 32 at r1 (raw file):

Previously, github-advanced-security[bot] wrote…

Spurious Javadoc @param tags

@param tag "serviceAccountEmail" does not match any actual type parameter of type "AuthResult".

Show more details

Done.

[jianglai]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gbrodman, @param, and @ptkach)

[jianglai]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gbrodman, @param, and @ptkach)

[jianglai]

Reviewed 17 of 17 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gbrodman, @param, and @ptkach)

[jianglai]

Gentle ping. I think we should start reviewing this PR as it is pretty extensive, even though it cannot be submitted due to other dependencies still in flux.

[gbrodman]

Reviewed 170 of 201 files at r1, 11 of 13 files at r2, 1 of 1 files at r3, 3 of 3 files at r4, 17 of 17 files at r5, all commit messages.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @jianglai, @param, and @ptkach)

---

core/src/main/java/google/registry/request/auth/AuthResult.java line 59 at r5 (raw file):

  private static AuthResult create(
      AuthLevel authLevel, @Nullable User user, @Nullable String email) {

can we rename this to serviceAccountEmail or something to make it clear that this should not be the user's email?

---

core/src/main/java/google/registry/ui/server/registrar/ConsoleOteSetupAction.java line 56 at r5 (raw file):

    path = ConsoleOteSetupAction.PATH,
    method = {Method.POST, Method.GET},
    auth = Auth.AUTH_PUBLIC)

I think we want this to be PUBLIC_LOGGIED_IN

---

core/src/main/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorAction.java line 66 at r5 (raw file):

    path = ConsoleRegistrarCreatorAction.PATH,
    method = {Method.POST, Method.GET},
    auth = Auth.AUTH_PUBLIC)

I think we want this to be PUBLIC_LOGGIED_IN

---

core/src/main/java/google/registry/ui/server/registrar/ConsoleUiAction.java line 43 at r5 (raw file):

/** Action that serves Registrar Console single HTML page (SPA). */
@Action(service = Action.Service.DEFAULT, path = ConsoleUiAction.PATH, auth = Auth.AUTH_PUBLIC)

I think we want this to be PUBLIC_LOGGIED_IN

---

core/src/main/java/google/registry/ui/server/registrar/HtmlAction.java line 79 at r5 (raw file):

    data.put("productName", productName);
    data.put("username", user.getEmailAddress());
    data.put("logoutUrl", "/registrar?gcp-iap-mode=CLEAR_LOGIN_COOKIE");

sadly this does not work as well as you might expect, but it's probably still better than nothing

---

core/src/test/java/google/registry/request/auth/AuthenticatedRegistrarAccessorTest.java line 84 at r5 (raw file):

  private static final String REGISTRAR_ID_WITH_ACCESS = "TheRegistrar";

  /** Registrar ID of a REAL registrar with the {@link USER} has no access to. */

nit: "which", not "with"

---

core/src/test/java/google/registry/request/auth/RequestAuthenticatorTest.java line 51 at r5 (raw file):

              .setEmailAddress("user@registry.example")
              .setUserRoles(
                  new UserRoles.Builder().setIsAdmin(false).setGlobalRole(GlobalRole.NONE).build())

fwiw "false" is the default for admin, no need to set it

---

core/src/test/java/google/registry/ui/server/console/ConsoleEppPasswordActionTest.java line 199 at r5 (raw file):

        AuthenticatedRegistrarAccessor.createForTesting(
            ImmutableSetMultimap.of("registrarId", OWNER));
    Cookie cookie =

I think we can remove this cookie-setting code, since the fake params class does it for us? (This comment may apply multiple places in the PR)

---

core/src/test/java/google/registry/ui/server/registrar/ConsoleUiActionTest.java line 157 at r5 (raw file):

  @Test
  void testNoUser_redirect() {

nit, but it's no longer a "redirect"

---

core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java line 402 at r5 (raw file):

    clock.setTo(previousLock.getLockRequestTime().plusHours(2));
    Map<String, ?> response = action.handleJsonRequest(lockRequest());
    assertSuccess(response, "lock", "Marla.Singer.RegistryLock@crr.com");

we'll need to change this, but you already noted that in the commit description

---

core/src/test/java/google/registry/webdriver/RegistrarCreateConsoleScreenshotTest.java line 42 at r5 (raw file):

  @BeforeEach
  void beforeEach() {

can we just put this in WebDriverTestCase so we don't need to repeat it?

---

core/src/test/java/google/registry/testing/ConsoleApiParamsUtil.java line 27 at r5 (raw file):

import org.joda.time.DateTime;

public final class ConsoleApiParamsUtil {

By convention, I think we'd want this to be ConsoleApiParamsUtil*s. But why do we need to change this at all?

[jianglai]

Reviewed 35 of 35 files at r6, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @gbrodman, @param, and @ptkach)

---

core/src/main/java/google/registry/request/auth/AuthResult.java line 59 at r5 (raw file):

Previously, gbrodman wrote…

can we rename this to serviceAccountEmail or something to make it clear that this should not be the user's email?

It was only a private method so I don't think there would be any confusion to the users. But sure.

---

core/src/main/java/google/registry/ui/server/registrar/ConsoleOteSetupAction.java line 56 at r5 (raw file):

Previously, gbrodman wrote…

I think we want this to be PUBLIC_LOGGIED_IN

I think all actions that extends HtmlAction can now be AUTH_PUBLIC_LOGGED_IN, as they are now behind IAP and there is no way for an unauthenticated request to reach them. I left them as AUTH_PUBLIC so that they behave absolutely the same as before (sans the support for legacy auth, of course) and there is one fewer variable to worry about. In the grand scheme of things this probably doesn't matter because we will stop supporting the legacy console soon.

If you feel strongly, I'm OK with changing them to AUTH_PUBLIC_LOGGED_IN, though I don't think it will affect anything in practice.

---

core/src/main/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorAction.java line 66 at r5 (raw file):

Previously, gbrodman wrote…

I think we want this to be PUBLIC_LOGGIED_IN

Ditto.

---

core/src/main/java/google/registry/ui/server/registrar/ConsoleUiAction.java line 43 at r5 (raw file):

Previously, gbrodman wrote…

I think we want this to be PUBLIC_LOGGIED_IN

Ditto.

---

core/src/main/java/google/registry/ui/server/registrar/HtmlAction.java line 79 at r5 (raw file):

Previously, gbrodman wrote…

sadly this does not work as well as you might expect, but it's probably still better than nothing

I tried and it seems to work fine. If you click on the button, it will log you out and take you back to the IAP login screen. I think that is as much as one can do with the endpoints behind IAP. It is also the same behavior as what the logout button does in the new console.

Can you clarify what you meant by "this does not work as well as you might expect"?

---

core/src/test/java/google/registry/request/auth/RequestAuthenticatorTest.java line 51 at r5 (raw file):

Previously, gbrodman wrote…

fwiw "false" is the default for admin, no need to set it

Correct. I think in a lot of places we set it to false unnecessary.

---

core/src/test/java/google/registry/ui/server/console/ConsoleEppPasswordActionTest.java line 199 at r5 (raw file):

Previously, gbrodman wrote…

I think we can remove this cookie-setting code, since the fake params class does it for us? (This comment may apply multiple places in the PR)

Correct. It turned out after rebasing it is gone already :P

---

core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java line 402 at r5 (raw file):

Previously, gbrodman wrote…

we'll need to change this, but you already noted that in the commit description

Yes, just waiting for your PR to get merged and deployed.

---

core/src/test/java/google/registry/webdriver/RegistrarCreateConsoleScreenshotTest.java line 42 at r5 (raw file):

Previously, gbrodman wrote…

can we just put this in WebDriverTestCase so we don't need to repeat it?

That's a good pint, but unfortunately no. It needs access to the server field, which is set by each child test class (as the route is different in each class).

---

core/src/test/java/google/registry/testing/ConsoleApiParamsUtil.java line 27 at r5 (raw file):

Previously, gbrodman wrote…

By convention, I think we'd want this to be ConsoleApiParamsUtil*s. But why do we need to change this at all?

Because FakeConsoleApiParams suggests that the class is a fake implementation of ConsoleApiParams, but in reality it just provides a helper function that returns a real ConsoleApiParams, which some fake/mocked parameters.

And yes, it should have been *Utils.

[gbrodman]

Reviewed 35 of 35 files at r6, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @jianglai, @param, and @ptkach)

---

core/src/main/java/google/registry/request/auth/AuthResult.java line 59 at r5 (raw file):

Previously, jianglai (Lai Jiang) wrote…

It was only a private method so I don't think there would be any confusion to the users. But sure.

i think the order of the words in the variable got mixed up :|

---

core/src/main/java/google/registry/ui/server/registrar/ConsoleOteSetupAction.java line 56 at r5 (raw file):

Previously, jianglai (Lai Jiang) wrote…

I think all actions that extends HtmlAction can now be AUTH_PUBLIC_LOGGED_IN, as they are now behind IAP and there is no way for an unauthenticated request to reach them. I left them as AUTH_PUBLIC so that they behave absolutely the same as before (sans the support for legacy auth, of course) and there is one fewer variable to worry about. In the grand scheme of things this probably doesn't matter because we will stop supporting the legacy console soon.

If you feel strongly, I'm OK with changing them to AUTH_PUBLIC_LOGGED_IN, though I don't think it will affect anything in practice.

What do you mean by "they behave absolutely the same as before"? I think it's probably best to have them be AUTH_PUBLIC_LOGGED_IN just in case we accidentally mess up something with the IAP configuration at some point. There's no downside to setting them to that; it's the more correct option and we're changing this file anyway.

[jianglai]

Reviewed 6 of 6 files at r7, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @gbrodman, @param, and @ptkach)

---

core/src/main/java/google/registry/request/auth/AuthResult.java line 59 at r5 (raw file):

Previously, gbrodman wrote…

i think the order of the words in the variable got mixed up :|

oops...

---

core/src/main/java/google/registry/ui/server/registrar/ConsoleOteSetupAction.java line 56 at r5 (raw file):

Previously, gbrodman wrote…

What do you mean by "they behave absolutely the same as before"? I think it's probably best to have them be AUTH_PUBLIC_LOGGED_IN just in case we accidentally mess up something with the IAP configuration at some point. There's no downside to setting them to that; it's the more correct option and we're changing this file anyway.

What I meant is that these endpoints used to allow unlogged-in users because they would redirect them to the login page. Now they would just return 401, but it should be a condition that is never true because the endpoints are behind IAP.

[gbrodman]

Looks good to me, waiting to approve until we can get all the dependencies through.

Reviewed 6 of 6 files at r7, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @param and @ptkach)

---

core/src/main/java/google/registry/ui/server/registrar/HtmlAction.java line 79 at r5 (raw file):

Previously, jianglai (Lai Jiang) wrote…

I tried and it seems to work fine. If you click on the button, it will log you out and take you back to the IAP login screen. I think that is as much as one can do with the endpoints behind IAP. It is also the same behavior as what the logout button does in the new console.

Can you clarify what you meant by "this does not work as well as you might expect"?

I don't remember exactly but if you try reloading / refreshing it might not actually fully log you out, if you have IAP sessions open on other sites (like gmail). It's not a huge deal.

---

core/src/test/java/google/registry/webdriver/RegistrarCreateConsoleScreenshotTest.java line 42 at r5 (raw file):

Previously, jianglai (Lai Jiang) wrote…

That's a good pint, but unfortunately no. It needs access to the server field, which is set by each child test class (as the route is different in each class).

womp :(

[jianglai]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @param and @ptkach)

---

core/src/main/java/google/registry/ui/server/registrar/HtmlAction.java line 79 at r5 (raw file):

Previously, gbrodman wrote…

I don't remember exactly but if you try reloading / refreshing it might not actually fully log you out, if you have IAP sessions open on other sites (like gmail). It's not a huge deal.

Is it specific to the old console? I copy & pasted the URL from the new console, so I would expect it to behave the same?

[jianglai]

Reviewed all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @param and @ptkach)

[gbrodman]

Reviewed 20 of 20 files at r9, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @param and @ptkach)

[jianglai]

Reviewed all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @param and @ptkach)

[jianglai]

Reviewed 14 of 14 files at r10, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @param and @ptkach)

[gbrodman]

Reviewed 14 of 14 files at r10, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @param and @ptkach)

[jianglai]

Dismissed @Github-advanced-security[bot] from 3 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ptkach)

[gbrodman]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ptkach)

[gbrodman]

Reviewed 1 of 1 files at r11, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ptkach)

[gbrodman]

Reviewed 22 of 23 files at r12, 1 of 1 files at r13, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ptkach)

[jianglai]

Reviewed 23 of 23 files at r12, 5 of 5 files at r14, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ptkach)

[jianglai]

Reviewed 2 of 2 files at r15, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ptkach)