Fix admin access to registryAdminClientId when they aren't associated with it

An admin that isn't associated with any clientId, will default to the
registryAdminClientId.

However, if we set the registryAdminClientId as the session's
CLIENT_ID_ATTRIBUTE, the next time we access the server we have a client-id
attribute we aren't associated with - which returns a "403 Registrar Console
access revoked" error (the assumption is - we were associated with it before
but aren't anymore)

To fix this - we just add all admins as "hasAccessTo" registryAdminClientId, even if it's not in the contacts. This will let admins stay on the admin registrar, without affecting where they log-in initially if they are also contacts in different registrars.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=200402856

java/google/registry/ui/server/registrar/SessionUtils.java

@@ -42,7 +42,7 @@ public class SessionUtils {
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
-  public static final String CLIENT_ID_ATTRIBUTE = "clientId";
+  private static final String CLIENT_ID_ATTRIBUTE = "clientId";
 
   @Inject
   @Config("registryAdminClientId")
@@ -101,7 +101,7 @@ public class SessionUtils {
 
     // Use the clientId if it exists
     if (clientId != null) {
-      if (!hasAccessToRegistrar(clientId, user.getUserId())) {
+      if (!hasAccessToRegistrar(clientId, user.getUserId(), userAuthInfo.isUserAdmin())) {
         logger.atInfo().log("Registrar Console access revoked: %s", clientId);
         session.invalidate();
         return false;
@@ -114,7 +114,7 @@ public class SessionUtils {
     // The clientId was null, so let's try and find a registrar this user is associated with
     Optional<Registrar> registrar = findRegistrarForUser(user.getUserId());
     if (registrar.isPresent()) {
-      verify(hasAccessToRegistrar(registrar.get(), user.getUserId()));
+      verify(isInAllowedContacts(registrar.get(), user.getUserId()));
       logger.atInfo().log(
           "Associating user %s with found registrar %s.",
           user.getUserId(), registrar.get().getClientId());
@@ -180,18 +180,27 @@ public class SessionUtils {
     return result;
   }
 
-  /** @see #hasAccessToRegistrar(Registrar, String) */
+  /** @see #isInAllowedContacts(Registrar, String) */
-  protected static boolean hasAccessToRegistrar(String clientId, final String gaeUserId) {
+  boolean hasAccessToRegistrar(String clientId, String gaeUserId, boolean isAdmin) {
     Optional<Registrar> registrar = Registrar.loadByClientIdCached(clientId);
     if (!registrar.isPresent()) {
       logger.atWarning().log("Registrar '%s' disappeared from Datastore!", clientId);
       return false;
     }
-    return hasAccessToRegistrar(registrar.get(), gaeUserId);
+    if (isAdmin && clientId.equals(registryAdminClientId)) {
+      return true;
+    }
+    return isInAllowedContacts(registrar.get(), gaeUserId);
   }
 
-  /** Returns {@code true} if {@code gaeUserId} is listed in contacts. */
+  /**
-  private static boolean hasAccessToRegistrar(Registrar registrar, final String gaeUserId) {
+   * Returns {@code true} if {@code gaeUserId} is listed in contacts with access to the registrar.
+   *
+   * <p>Each registrar contact can either have getGaeUserId equals null or the user's gaeUserId.
+   * Null means the contact doesn't have access to the registrar console. None-null means the
+   * contact has access.
+   */
+  private static boolean isInAllowedContacts(Registrar registrar, final String gaeUserId) {
     return registrar
         .getContacts()
         .stream()

javatests/google/registry/ui/server/registrar/SessionUtilsTest.java

@@ -260,6 +260,28 @@ public class SessionUtilsTest {
                 UNAUTHORIZED_ADMIN.user().getUserId(), ADMIN_CLIENT_ID));
   }
 
+  /**
+   * If session clientId points to the adminClientId, and the user is an admin that doesn't have
+   * access to this registrar - it means this is the second (or later) visit of this admin and they
+   * were granted access to the default registrar because they aren't associated with any other
+   * registrar.
+   *
+   * We continue to grant the admin access.
+   */
+  @Test
+  public void testCheckRegistrarConsoleLogin_noSession_noAccess_isAdmin_secondRequest()
+      throws Exception {
+    when(session.getAttribute("clientId")).thenReturn(ADMIN_CLIENT_ID);
+    assertThat(sessionUtils.checkRegistrarConsoleLogin(req, UNAUTHORIZED_ADMIN)).isTrue();
+    assertAboutLogs()
+        .that(testLogHandler)
+        .hasLogAtLevelWithMessage(
+            Level.INFO,
+            String.format(
+                "Associating user %s with given registrar %s.",
+                UNAUTHORIZED_ADMIN.user().getUserId(), ADMIN_CLIENT_ID));
+  }
+
   /**
    * If clientId does not exist in the session and the user is not associated with a registrar, then
    * access should not be granted.
@@ -279,17 +301,37 @@ public class SessionUtilsTest {
   @Test
   public void testHasAccessToRegistrar_orphanedContact_returnsFalse() throws Exception {
     deleteResource(loadRegistrar(DEFAULT_CLIENT_ID));
-    assertThat(SessionUtils.hasAccessToRegistrar(DEFAULT_CLIENT_ID, THE_REGISTRAR_GAE_USER_ID))
+    assertThat(
+            sessionUtils.hasAccessToRegistrar(DEFAULT_CLIENT_ID, THE_REGISTRAR_GAE_USER_ID, false))
         .isFalse();
   }
 
   @Test
   public void testHasAccessToRegistrar_accessRevoked_returnsFalse() throws Exception {
     RegistrarContact.updateContacts(loadRegistrar(DEFAULT_CLIENT_ID), new java.util.HashSet<>());
-    assertThat(SessionUtils.hasAccessToRegistrar(DEFAULT_CLIENT_ID, THE_REGISTRAR_GAE_USER_ID))
+    assertThat(
+            sessionUtils.hasAccessToRegistrar(DEFAULT_CLIENT_ID, THE_REGISTRAR_GAE_USER_ID, false))
         .isFalse();
   }
 
+  @Test
+  public void testHasAccessToRegistrar_orphanedAdmin_notAdminRegistrar_returnsFalse()
+      throws Exception {
+    RegistrarContact.updateContacts(loadRegistrar(DEFAULT_CLIENT_ID), new java.util.HashSet<>());
+    assertThat(
+            sessionUtils.hasAccessToRegistrar(DEFAULT_CLIENT_ID, THE_REGISTRAR_GAE_USER_ID, true))
+        .isFalse();
+  }
+
+  @Test
+  public void testHasAccessToRegistrar_orphanedAdmin_onAdminRegistrar_returnsTrue()
+      throws Exception {
+    RegistrarContact.updateContacts(loadRegistrar(ADMIN_CLIENT_ID), new java.util.HashSet<>());
+    assertThat(
+            sessionUtils.hasAccessToRegistrar(ADMIN_CLIENT_ID, THE_REGISTRAR_GAE_USER_ID, true))
+        .isTrue();
+  }
+
   @Test
   public void testNullness() throws Exception {
     new NullPointerTester()