Move AuthenticatedRegistrarAccessor to request/auth/

It is starting to be used in more places than just ur/server/registrar. Even now it's used in the RDAP, and we are going to start using it for the registrar-xhr endpoint meaning it will be used in EPP flows as well.

Also logically - this is part of the request authentication.

While moving - we also refactor it to make it easier to use in tests. Instead of mocking, we will be able to create instances with arbitrary roles.

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

java/google/registry/rdap/RdapActionBase.java

@@ -47,8 +47,8 @@ import google.registry.request.RequestMethod;
 import google.registry.request.RequestPath;
 import google.registry.request.Response;
 import google.registry.request.auth.AuthResult;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor;
 import google.registry.request.auth.UserAuthInfo;
-import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor;
 import google.registry.util.Clock;
 import java.io.IOException;
 import java.net.URI;

java/google/registry/request/HttpException.java

@@ -115,6 +115,10 @@ public abstract class HttpException extends RuntimeException {
       super(HttpServletResponse.SC_FORBIDDEN, message, null);
     }
 
+    public ForbiddenException(String message, Exception cause) {
+      super(HttpServletResponse.SC_FORBIDDEN, message, cause);
+    }
+
     @Override
     public String getResponseCodeString() {
       return "Forbidden";

java/google/registry/request/auth/AuthenticatedRegistrarAccessor.java

@@ -12,8 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package google.registry.ui.server.registrar;
+package google.registry.request.auth;
 
+import static com.google.common.base.MoreObjects.toStringHelper;
+import static com.google.common.base.Preconditions.checkNotNull;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 
 import com.google.appengine.api.users.User;
@@ -27,19 +29,16 @@ import google.registry.config.RegistryConfig.Config;
 import google.registry.groups.GroupsConnection;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.RegistrarContact;
-import google.registry.request.HttpException.ForbiddenException;
-import google.registry.request.auth.AuthResult;
-import google.registry.request.auth.UserAuthInfo;
 import javax.annotation.concurrent.Immutable;
 import javax.inject.Inject;
 
 /**
  * Allows access only to {@link Registrar}s the current user has access to.
  *
- * <p>A user has OWNER role on a Registrar if there exists a {@link RegistrarContact} with
+ * <p>A user has OWNER role on a Registrar if there exists a {@link RegistrarContact} with that
- * that user's gaeId and the registrar as a parent.
+ * user's gaeId and the registrar as a parent.
  *
- * <p>An admin has in addition OWNER role on {@link #registryAdminClientId}.
+ * <p>An admin has in addition OWNER role on {@code #registryAdminClientId}.
  *
  * <p>An admin also has ADMIN role on ALL registrars.
  */
@@ -54,13 +53,14 @@ public class AuthenticatedRegistrarAccessor {
     ADMIN
   }
 
-  AuthResult authResult;
+  private final String userIdForLogging;
-  String registryAdminClientId;
 
   /**
    * Gives all roles a user has for a given clientId.
    *
    * <p>The order is significant, with "more specific to this user" coming first.
+   *
+   * <p>Logged out users have an empty roleMap.
    */
   private final ImmutableSetMultimap<String, Role> roleMap;
 
@@ -89,28 +89,42 @@ public class AuthenticatedRegistrarAccessor {
         overrideGroupsConnection != null ? overrideGroupsConnection : groupsConnection.get());
   }
 
+  @VisibleForTesting
   AuthenticatedRegistrarAccessor(
       AuthResult authResult,
-      @Config("registryAdminClientId") String registryAdminClientId,
+      String registryAdminClientId,
-      @Config("gSuiteSupportGroupEmailAddress") String gSuiteSupportGroupEmailAddress,
+      String gSuiteSupportGroupEmailAddress,
       GroupsConnection groupsConnection) {
-    this.authResult = authResult;
+    this(
-    this.registryAdminClientId = registryAdminClientId;
+        authResult.userIdForLogging(),
-    this.roleMap =
         createRoleMap(
-            authResult,
+            authResult, registryAdminClientId, groupsConnection, gSuiteSupportGroupEmailAddress));
-            registryAdminClientId,
-            groupsConnection,
-            gSuiteSupportGroupEmailAddress);
 
     logger.atInfo().log(
         "%s has the following roles: %s", authResult.userIdForLogging(), roleMap);
   }
 
+  private AuthenticatedRegistrarAccessor(
+      String userIdForLogging, ImmutableSetMultimap<String, Role> roleMap) {
+    this.userIdForLogging = checkNotNull(userIdForLogging);
+    this.roleMap = checkNotNull(roleMap);
+  }
+
+  /**
+   * Creates a "logged-in user" accessor with a given role map, used for tests.
+   *
+   * <p>The user's "name" in logs and exception messages is "TestUserId".
+   */
+  @VisibleForTesting
+  public static AuthenticatedRegistrarAccessor createForTesting(
+      ImmutableSetMultimap<String, Role> roleMap) {
+    return new AuthenticatedRegistrarAccessor("TestUserId", roleMap);
+  }
+
   /**
    * A map that gives all roles a user has for a given clientId.
    *
-   * <p>Throws a {@link ForbiddenException} if the user is not logged in.
+   * <p>Throws a {@link RegistrarAccessDeniedException} if the user is not logged in.
    *
    * <p>The result is ordered starting from "most specific to this user".
    *
@@ -129,7 +143,7 @@ public class AuthenticatedRegistrarAccessor {
   /**
    * "Guesses" which client ID the user wants from all those they have access to.
    *
-   * <p>If no such ClientIds exist, throws a ForbiddenException.
+   * <p>If no such ClientIds exist, throws a RegistrarAccessDeniedException.
    *
    * <p>This should be the ClientId "most likely wanted by the user".
    *
@@ -141,56 +155,59 @@ public class AuthenticatedRegistrarAccessor {
    * other clue as to the requested {@code clientId}. It is perfectly OK to get a {@code clientId}
    * from any other source, as long as the registrar is then loaded using {@link #getRegistrar}.
    */
-  public String guessClientId() {
+  public String guessClientId() throws RegistrarAccessDeniedException {
-    verifyLoggedIn();
     return getAllClientIdWithRoles().keySet().stream()
         .findFirst()
         .orElseThrow(
             () ->
-                new ForbiddenException(
+                new RegistrarAccessDeniedException(
-                    String.format(
+                    String.format("%s isn't associated with any registrar", userIdForLogging)));
-                        "%s isn't associated with any registrar",
-                        authResult.userIdForLogging())));
   }
 
   /**
    * Loads a Registrar IFF the user is authorized.
    *
-   * <p>Throws a {@link ForbiddenException} if the user is not logged in, or not authorized to
+   * <p>Throws a {@link RegistrarAccessDeniedException} if the user is not logged in, or not
-   * access the requested registrar.
+   * authorized to access the requested registrar.
    *
    * @param clientId ID of the registrar we request
    */
-  public Registrar getRegistrar(String clientId) {
+  public Registrar getRegistrar(String clientId) throws RegistrarAccessDeniedException {
-    verifyLoggedIn();
+    verifyAccess(clientId);
-
-    ImmutableSet<Role> roles = getAllClientIdWithRoles().get(clientId);
-
-    if (roles.isEmpty()) {
-      throw new ForbiddenException(
-          String.format(
-              "%s doesn't have access to registrar %s",
-              authResult.userIdForLogging(), clientId));
-    }
 
     Registrar registrar =
         Registrar.loadByClientId(clientId)
             .orElseThrow(
-                () -> new ForbiddenException(String.format("Registrar %s not found", clientId)));
+                () ->
+                    new RegistrarAccessDeniedException(
+                        String.format("Registrar %s not found", clientId)));
 
     if (!clientId.equals(registrar.getClientId())) {
       logger.atSevere().log(
           "registrarLoader.apply(clientId) returned a Registrar with a different clientId. "
               + "Requested: %s, returned: %s.",
           clientId, registrar.getClientId());
-      throw new ForbiddenException("Internal error - please check logs");
+      throw new RegistrarAccessDeniedException("Internal error - please check logs");
     }
 
-    logger.atInfo().log(
-        "%s has %s access to registrar %s.", authResult.userIdForLogging(), roles, clientId);
     return registrar;
   }
 
+  public void verifyAccess(String clientId) throws RegistrarAccessDeniedException {
+    ImmutableSet<Role> roles = getAllClientIdWithRoles().get(clientId);
+
+    if (roles.isEmpty()) {
+      throw new RegistrarAccessDeniedException(
+          String.format("%s doesn't have access to registrar %s", userIdForLogging, clientId));
+    }
+    logger.atInfo().log("%s has %s access to registrar %s.", userIdForLogging, roles, clientId);
+  }
+
+  @Override
+  public String toString() {
+    return toStringHelper(getClass()).add("authResult", userIdForLogging).toString();
+  }
+
   private static boolean checkIsSupport(
       GroupsConnection groupsConnection, String userEmail, String supportEmail) {
     if (Strings.isNullOrEmpty(supportEmail)) {
@@ -246,9 +263,10 @@ public class AuthenticatedRegistrarAccessor {
     return builder.build();
   }
 
-  private void verifyLoggedIn() {
+  /** Exception thrown when the current user doesn't have access to the requested Registrar. */
-    if (!authResult.userAuthInfo().isPresent()) {
+  public static class RegistrarAccessDeniedException extends Exception {
-      throw new ForbiddenException("Not logged in");
+    RegistrarAccessDeniedException(String message) {
+      super(message);
     }
   }
 }

java/google/registry/request/auth/BUILD

@@ -9,6 +9,8 @@ java_library(
     srcs = glob(["*.java"]),
     deps = [
         "//java/google/registry/config",
+        "//java/google/registry/groups",
+        "//java/google/registry/model",
         "//java/google/registry/security",
         "@com_google_appengine_api_1_0_sdk",
         "@com_google_auto_value",

java/google/registry/ui/server/registrar/BUILD

@@ -15,7 +15,6 @@ java_library(
         "//java/google/registry/config",
         "//java/google/registry/export/sheet",
         "//java/google/registry/flows",
-        "//java/google/registry/groups",
         "//java/google/registry/model",
         "//java/google/registry/request",
         "//java/google/registry/request/auth",

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

@@ -16,7 +16,7 @@ package google.registry.ui.server.registrar;
 
 import static com.google.common.net.HttpHeaders.LOCATION;
 import static com.google.common.net.HttpHeaders.X_FRAME_OPTIONS;
-import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role.ADMIN;
+import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.ADMIN;
 import static google.registry.ui.server.registrar.RegistrarConsoleModule.PARAM_CLIENT_ID;
 import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
 import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY;
@@ -36,14 +36,15 @@ import com.google.template.soy.tofu.SoyTofu;
 import google.registry.config.RegistryConfig.Config;
 import google.registry.model.registrar.Registrar;
 import google.registry.request.Action;
-import google.registry.request.HttpException.ForbiddenException;
 import google.registry.request.Parameter;
 import google.registry.request.Response;
 import google.registry.request.auth.Auth;
 import google.registry.request.auth.AuthResult;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor.RegistrarAccessDeniedException;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor.Role;
 import google.registry.security.XsrfTokenManager;
 import google.registry.ui.server.SoyTemplateUtils;
-import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role;
 import google.registry.ui.soy.registrar.ConsoleSoyInfo;
 import java.util.Optional;
 import javax.inject.Inject;
@@ -151,7 +152,7 @@ public final class ConsoleUiAction implements Runnable {
       // because the requests come from the browser, and can easily be faked)
       Registrar registrar = registrarAccessor.getRegistrar(clientId);
       data.put("requireFeeExtension", registrar.getPremiumPriceAckRequired());
-    } catch (ForbiddenException e) {
+    } catch (RegistrarAccessDeniedException e) {
       logger.atWarning().withCause(e).log(
           "User %s doesn't have access to registrar console.", authResult.userIdForLogging());
       response.setStatus(SC_FORBIDDEN);

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

@@ -18,7 +18,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.monitoring.metrics.IncrementableMetric;
 import com.google.monitoring.metrics.LabelDescriptor;
 import com.google.monitoring.metrics.MetricRegistryImpl;
-import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor.Role;
 import javax.inject.Inject;
 
 final class RegistrarConsoleMetrics {

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

@@ -42,11 +42,13 @@ import google.registry.request.HttpException.ForbiddenException;
 import google.registry.request.JsonActionRunner;
 import google.registry.request.auth.Auth;
 import google.registry.request.auth.AuthResult;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor.RegistrarAccessDeniedException;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor.Role;
 import google.registry.security.JsonResponseHelper;
 import google.registry.ui.forms.FormException;
 import google.registry.ui.forms.FormFieldException;
 import google.registry.ui.server.RegistrarFormFields;
-import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role;
 import google.registry.util.AppEngineServiceUtils;
 import google.registry.util.CollectionUtils;
 import google.registry.util.DiffUtils;
@@ -161,7 +163,11 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
   }
 
   private RegistrarResult read(String clientId) {
+    try {
       return RegistrarResult.create("Success", registrarAccessor.getRegistrar(clientId));
+    } catch (RegistrarAccessDeniedException e) {
+      throw new ForbiddenException(e.getMessage(), e);
+    }
   }
 
   private RegistrarResult update(final Map<String, ?> args, String clientId) {
@@ -171,7 +177,12 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
               // We load the registrar here rather than outside of the transaction - to make
               // sure we have the latest version. This one is loaded inside the transaction, so it's
               // guaranteed to not change before we update it.
-              Registrar registrar = registrarAccessor.getRegistrar(clientId);
+              Registrar registrar;
+              try {
+                registrar = registrarAccessor.getRegistrar(clientId);
+              } catch (RegistrarAccessDeniedException e) {
+                throw new ForbiddenException(e.getMessage(), e);
+              }
               // Verify that the registrar hasn't been changed.
               // To do that - we find the latest update time (or null if the registrar has been
               // deleted) and compare to the update time from the args. The update time in the args

javatests/google/registry/rdap/RdapActionBaseTestCase.java

@@ -19,7 +19,7 @@ import static google.registry.rdap.RdapAuthorization.Role.PUBLIC;
 import static google.registry.rdap.RdapAuthorization.Role.REGISTRAR;
 import static google.registry.request.Action.Method.GET;
 import static google.registry.request.Action.Method.HEAD;
-import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role.OWNER;
+import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.OWNER;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -29,12 +29,12 @@ import google.registry.model.ofy.Ofy;
 import google.registry.request.Action;
 import google.registry.request.auth.AuthLevel;
 import google.registry.request.auth.AuthResult;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor;
 import google.registry.request.auth.UserAuthInfo;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeResponse;
 import google.registry.testing.InjectRule;
-import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor;
 import google.registry.util.TypeUtils;
 import java.util.Optional;
 import org.joda.time.DateTime;

javatests/google/registry/request/auth/AuthenticatedRegistrarAccessorTest.java

@@ -12,16 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package google.registry.ui.server.registrar;
+package google.registry.request.auth;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.ADMIN;
+import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.OWNER;
 import static google.registry.testing.AppEngineRule.THE_REGISTRAR_GAE_USER_ID;
 import static google.registry.testing.DatastoreHelper.loadRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.JUnitBackports.assertThrows;
 import static google.registry.testing.LogsSubject.assertAboutLogs;
-import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role.ADMIN;
-import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role.OWNER;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -33,10 +33,7 @@ import com.google.common.flogger.LoggerConfig;
 import com.google.common.testing.NullPointerTester;
 import com.google.common.testing.TestLogHandler;
 import google.registry.groups.GroupsConnection;
-import google.registry.request.HttpException.ForbiddenException;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor.RegistrarAccessDeniedException;
-import google.registry.request.auth.AuthLevel;
-import google.registry.request.auth.AuthResult;
-import google.registry.request.auth.UserAuthInfo;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.InjectRule;
 import java.util.logging.Level;
@@ -76,7 +73,8 @@ public class AuthenticatedRegistrarAccessorTest {
         UserAuthInfo.create(
             new User(
                 String.format(
-                    "%s_%s@gmail.com", isAuthorized ? "good" : "evil", isAdmin ? "admin" : "user"),
+                    "%s_%s@gmail.com",
+                    isAuthorized ? "auth" : "unauth", isAdmin ? "admin" : "user"),
                 "gmail.com",
                 isAuthorized ? THE_REGISTRAR_GAE_USER_ID : "badGaeUserId"),
             isAdmin));
@@ -94,12 +92,6 @@ public class AuthenticatedRegistrarAccessorTest {
     LoggerConfig.getConfig(AuthenticatedRegistrarAccessor.class).removeHandler(testLogHandler);
   }
 
-  private String formatMessage(String message, AuthResult authResult, String clientId) {
-    return message
-        .replace("{user}", authResult.userIdForLogging())
-        .replace("{clientId}", String.valueOf(clientId));
-  }
-
   /** Users only have access to the registrars they are a contact for. */
   @Test
   public void getAllClientIdWithAccess_authorizedUser() {
@@ -114,7 +106,7 @@ public class AuthenticatedRegistrarAccessorTest {
   /** Users in support group have admin access to everything. */
   @Test
   public void getAllClientIdWithAccess_authorizedUser_isSupportGroup() {
-    when(groupsConnection.isMemberOfGroup("good_user@gmail.com", SUPPORT_GROUP)).thenReturn(true);
+    when(groupsConnection.isMemberOfGroup("auth_user@gmail.com", SUPPORT_GROUP)).thenReturn(true);
     AuthenticatedRegistrarAccessor registrarAccessor =
         new AuthenticatedRegistrarAccessor(
             AUTHORIZED_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection);
@@ -149,7 +141,7 @@ public class AuthenticatedRegistrarAccessorTest {
   /** Unauthorized users who are in support group have admin access. */
   @Test
   public void getAllClientIdWithAccess_unauthorizedUser_inSupportGroup() {
-    when(groupsConnection.isMemberOfGroup("evil_user@gmail.com", SUPPORT_GROUP)).thenReturn(true);
+    when(groupsConnection.isMemberOfGroup("unauth_user@gmail.com", SUPPORT_GROUP)).thenReturn(true);
     AuthenticatedRegistrarAccessor registrarAccessor =
         new AuthenticatedRegistrarAccessor(
             UNAUTHORIZED_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection);
@@ -179,7 +171,7 @@ public class AuthenticatedRegistrarAccessorTest {
         new AuthenticatedRegistrarAccessor(
             AUTHORIZED_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection);
 
-    verify(groupsConnection).isMemberOfGroup("good_user@gmail.com", SUPPORT_GROUP);
+    verify(groupsConnection).isMemberOfGroup("auth_user@gmail.com", SUPPORT_GROUP);
     assertThat(registrarAccessor.getAllClientIdWithRoles())
         .containsExactly(DEFAULT_CLIENT_ID, OWNER);
   }
@@ -223,34 +215,39 @@ public class AuthenticatedRegistrarAccessorTest {
     expectGetRegistrarFailure(
         DEFAULT_CLIENT_ID,
         UNAUTHORIZED_USER,
-        "{user} doesn't have access to registrar {clientId}");
+        "user unauth_user@gmail.com doesn't have access to registrar TheRegistrar");
   }
 
   /** Fail loading registrar if there's no user associated with the request. */
   @Test
   public void testGetRegistrarForUser_noUser() {
-    expectGetRegistrarFailure(DEFAULT_CLIENT_ID, NO_USER, "Not logged in");
+    expectGetRegistrarFailure(
+        DEFAULT_CLIENT_ID,
+        NO_USER,
+        "<logged-out user> doesn't have access to registrar TheRegistrar");
   }
 
   /** Succeed loading registrar if user has access to it. */
   @Test
-  public void testGetRegistrarForUser_hasAccess_isNotAdmin() {
+  public void testGetRegistrarForUser_hasAccess_isNotAdmin() throws Exception {
     expectGetRegistrarSuccess(
-        AUTHORIZED_USER, "{user} has [OWNER] access to registrar {clientId}");
+        AUTHORIZED_USER, "user auth_user@gmail.com has [OWNER] access to registrar TheRegistrar");
   }
 
   /** Succeed loading registrar if admin with access. */
   @Test
-  public void testGetRegistrarForUser_hasAccess_isAdmin() {
+  public void testGetRegistrarForUser_hasAccess_isAdmin() throws Exception {
     expectGetRegistrarSuccess(
-        AUTHORIZED_ADMIN, "{user} has [OWNER, ADMIN] access to registrar {clientId}");
+        AUTHORIZED_ADMIN,
+        "admin auth_admin@gmail.com has [OWNER, ADMIN] access to registrar TheRegistrar");
   }
 
   /** Succeed loading registrar for admin even if they aren't on the approved contacts list. */
   @Test
-  public void testGetRegistrarForUser_noAccess_isAdmin() {
+  public void testGetRegistrarForUser_noAccess_isAdmin() throws Exception {
     expectGetRegistrarSuccess(
-        UNAUTHORIZED_ADMIN, "{user} has [ADMIN] access to registrar {clientId}.");
+        UNAUTHORIZED_ADMIN,
+        "admin unauth_admin@gmail.com has [ADMIN] access to registrar TheRegistrar.");
   }
 
   /** Fail loading registrar even if admin, if registrar doesn't exist. */
@@ -259,20 +256,17 @@ public class AuthenticatedRegistrarAccessorTest {
     expectGetRegistrarFailure(
         "BadClientId",
         AUTHORIZED_ADMIN,
-        "{user} doesn't have access to registrar {clientId}");
+        "admin auth_admin@gmail.com doesn't have access to registrar BadClientId");
   }
 
-  private void expectGetRegistrarSuccess(
+  private void expectGetRegistrarSuccess(AuthResult authResult, String message) throws Exception {
-      AuthResult authResult, String message) {
     AuthenticatedRegistrarAccessor registrarAccessor =
         new AuthenticatedRegistrarAccessor(
             authResult, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection);
 
+    // make sure loading the registrar succeeds and returns a value
     assertThat(registrarAccessor.getRegistrar(DEFAULT_CLIENT_ID)).isNotNull();
-    assertAboutLogs()
+    assertAboutLogs().that(testLogHandler).hasLogAtLevelWithMessage(Level.INFO, message);
-        .that(testLogHandler)
-        .hasLogAtLevelWithMessage(
-            Level.INFO, formatMessage(message, authResult, DEFAULT_CLIENT_ID));
   }
 
   private void expectGetRegistrarFailure(
@@ -281,16 +275,17 @@ public class AuthenticatedRegistrarAccessorTest {
         new AuthenticatedRegistrarAccessor(
             authResult, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection);
 
-    ForbiddenException exception =
+    // make sure getRegistrar fails
+    RegistrarAccessDeniedException exception =
         assertThrows(
-            ForbiddenException.class, () -> registrarAccessor.getRegistrar(clientId));
+            RegistrarAccessDeniedException.class, () -> registrarAccessor.getRegistrar(clientId));
 
-    assertThat(exception).hasMessageThat().contains(formatMessage(message, authResult, clientId));
+    assertThat(exception).hasMessageThat().contains(message);
   }
 
   /** If a user has access to a registrar, we should guess that registrar. */
   @Test
-  public void testGuessClientIdForUser_hasAccess_isNotAdmin() {
+  public void testGuessClientIdForUser_hasAccess_isNotAdmin() throws Exception {
     AuthenticatedRegistrarAccessor registrarAccessor =
         new AuthenticatedRegistrarAccessor(
             AUTHORIZED_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection);
@@ -301,7 +296,8 @@ public class AuthenticatedRegistrarAccessorTest {
   /** If a user doesn't have access to any registrars, guess returns nothing. */
   @Test
   public void testGuessClientIdForUser_noAccess_isNotAdmin() {
-    expectGuessRegistrarFailure(UNAUTHORIZED_USER, "{user} isn't associated with any registrar");
+    expectGuessRegistrarFailure(
+        UNAUTHORIZED_USER, "user unauth_user@gmail.com isn't associated with any registrar");
   }
 
   /**
@@ -309,7 +305,7 @@ public class AuthenticatedRegistrarAccessorTest {
    * ADMIN_CLIENT_ID).
    */
   @Test
-  public void testGuessClientIdForUser_hasAccess_isAdmin() {
+  public void testGuessClientIdForUser_hasAccess_isAdmin() throws Exception {
     AuthenticatedRegistrarAccessor registrarAccessor =
         new AuthenticatedRegistrarAccessor(
             AUTHORIZED_ADMIN, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection);
@@ -319,7 +315,7 @@ public class AuthenticatedRegistrarAccessorTest {
 
   /** If an admin doesn't have access to a registrar, we should guess the ADMIN_CLIENT_ID. */
   @Test
-  public void testGuessClientIdForUser_noAccess_isAdmin() {
+  public void testGuessClientIdForUser_noAccess_isAdmin() throws Exception {
     AuthenticatedRegistrarAccessor registrarAccessor =
         new AuthenticatedRegistrarAccessor(
             UNAUTHORIZED_ADMIN, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection);
@@ -333,7 +329,7 @@ public class AuthenticatedRegistrarAccessorTest {
    * registrars.
    */
   @Test
-  public void testGuessClientIdForUser_noAccess_isAdmin_adminClientIdEmpty() {
+  public void testGuessClientIdForUser_noAccess_isAdmin_adminClientIdEmpty() throws Exception {
     AuthenticatedRegistrarAccessor registrarAccessor =
         new AuthenticatedRegistrarAccessor(UNAUTHORIZED_ADMIN, "", SUPPORT_GROUP, groupsConnection);
 
@@ -345,7 +341,7 @@ public class AuthenticatedRegistrarAccessorTest {
    * non-existent registrar, we still guess it (we will later fail loading the registrar).
    */
   @Test
-  public void testGuessClientIdForUser_noAccess_isAdmin_adminClientIdInvalid() {
+  public void testGuessClientIdForUser_noAccess_isAdmin_adminClientIdInvalid() throws Exception {
     AuthenticatedRegistrarAccessor registrarAccessor =
         new AuthenticatedRegistrarAccessor(
             UNAUTHORIZED_ADMIN, "NonexistentRegistrar", SUPPORT_GROUP, groupsConnection);
@@ -358,11 +354,9 @@ public class AuthenticatedRegistrarAccessorTest {
         new AuthenticatedRegistrarAccessor(
             authResult, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection);
 
-    ForbiddenException exception =
+    RegistrarAccessDeniedException exception =
-        assertThrows(ForbiddenException.class, () -> registrarAccessor.guessClientId());
+        assertThrows(RegistrarAccessDeniedException.class, () -> registrarAccessor.guessClientId());
-    assertThat(exception)
+    assertThat(exception).hasMessageThat().contains(message);
-        .hasMessageThat()
-        .contains(formatMessage(message, authResult, null));
   }
 
   @Test

javatests/google/registry/request/auth/BUILD

@@ -12,6 +12,7 @@ java_library(
     srcs = glob(["*.java"]),
     resources = glob(["testdata/*"]),
     deps = [
+        "//java/google/registry/groups",
         "//java/google/registry/request/auth",
         "//java/google/registry/security",
         "//javatests/google/registry/testing",
@@ -19,7 +20,10 @@ java_library(
         "@com_google_appengine_api_1_0_sdk",
         "@com_google_appengine_tools_appengine_gcs_client",
         "@com_google_appengine_tools_sdk",
+        "@com_google_flogger",
+        "@com_google_flogger_system_backend",
         "@com_google_guava",
+        "@com_google_guava_testlib",
         "@com_google_truth",
         "@com_google_truth_extensions_truth_java8_extension",
         "@javax_servlet_api",

javatests/google/registry/ui/server/BUILD

@@ -14,6 +14,7 @@ java_library(
         "//java/google/registry/ui/forms",
         "//java/google/registry/ui/server",
         "//javatests/google/registry/testing",
+        "@com_google_guava",
         "@com_google_truth",
         "@com_google_truth_extensions_truth_java8_extension",
         "@junit",

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

@@ -17,9 +17,8 @@ package google.registry.ui.server.registrar;
 import static com.google.common.net.HttpHeaders.LOCATION;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.monitoring.metrics.contrib.LongMetricSubject.assertThat;
-import static google.registry.testing.DatastoreHelper.loadRegistrar;
+import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.ADMIN;
-import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role.ADMIN;
+import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.OWNER;
-import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role.OWNER;
 import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -28,9 +27,9 @@ import com.google.appengine.api.users.User;
 import com.google.appengine.api.users.UserServiceFactory;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.net.MediaType;
-import google.registry.request.HttpException.ForbiddenException;
 import google.registry.request.auth.AuthLevel;
 import google.registry.request.auth.AuthResult;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor;
 import google.registry.request.auth.UserAuthInfo;
 import google.registry.security.XsrfTokenManager;
 import google.registry.testing.AppEngineRule;
@@ -56,8 +55,6 @@ public class ConsoleUiActionTest {
       .withUserService(UserInfo.create("marla.singer@example.com", "12345"))
       .build();
 
-  private final AuthenticatedRegistrarAccessor registrarAccessor =
-      mock(AuthenticatedRegistrarAccessor.class);
   private final HttpServletRequest request = mock(HttpServletRequest.class);
   private final FakeResponse response = new FakeResponse();
   private final ConsoleUiAction action = new ConsoleUiAction();
@@ -76,24 +73,19 @@ public class ConsoleUiActionTest {
     action.req = request;
     action.response = response;
     action.registrarConsoleMetrics = new RegistrarConsoleMetrics();
-    action.registrarAccessor = registrarAccessor;
     action.userService = UserServiceFactory.getUserService();
     action.xsrfTokenManager = new XsrfTokenManager(new FakeClock(), action.userService);
     action.paramClientId = Optional.empty();
     AuthResult authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false));
     action.authResult = authResult;
-    when(registrarAccessor.getRegistrar("TheRegistrar"))
+
-        .thenReturn(loadRegistrar("TheRegistrar"));
+    action.registrarAccessor =
-    when(registrarAccessor.getAllClientIdWithRoles())
+        AuthenticatedRegistrarAccessor.createForTesting(
-        .thenReturn(
             ImmutableSetMultimap.of(
                 "TheRegistrar", OWNER,
-                "OtherRegistrar", OWNER,
+                "NewRegistrar", OWNER,
-                "OtherRegistrar", ADMIN,
+                "NewRegistrar", ADMIN,
                 "AdminRegistrar", ADMIN));
-    when(registrarAccessor.guessClientId()).thenCallRealMethod();
-    // Used for error message in guessClientId
-    registrarAccessor.authResult = authResult;
     RegistrarConsoleMetrics.consoleRequestMetric.reset();
   }
 
@@ -146,7 +138,8 @@ public class ConsoleUiActionTest {
 
   @Test
   public void testUserDoesntHaveAccessToAnyRegistrar_showsWhoAreYouPage() {
-    when(registrarAccessor.getAllClientIdWithRoles()).thenReturn(ImmutableSetMultimap.of());
+    action.registrarAccessor =
+        AuthenticatedRegistrarAccessor.createForTesting(ImmutableSetMultimap.of());
     action.run();
     assertThat(response.getPayload()).contains("<h1>You need permission</h1>");
     assertThat(response.getPayload()).contains("not associated with Nomulus.");
@@ -175,8 +168,6 @@ public class ConsoleUiActionTest {
   public void testSettingClientId_notAllowed_showsNeedPermissionPage() {
     // Behaves the same way if fakeRegistrar exists, but we don't have access to it
     action.paramClientId = Optional.of("fakeRegistrar");
-    when(registrarAccessor.getRegistrar("fakeRegistrar"))
-        .thenThrow(new ForbiddenException("forbidden"));
     action.run();
     assertThat(response.getPayload()).contains("<h1>You need permission</h1>");
     assertThat(response.getPayload()).contains("not associated with the registrar fakeRegistrar.");
@@ -185,20 +176,18 @@ public class ConsoleUiActionTest {
 
   @Test
   public void testSettingClientId_allowed_showsRegistrarConsole() {
-    action.paramClientId = Optional.of("OtherRegistrar");
+    action.paramClientId = Optional.of("NewRegistrar");
-    when(registrarAccessor.getRegistrar("OtherRegistrar"))
-        .thenReturn(loadRegistrar("TheRegistrar"));
     action.run();
     assertThat(response.getPayload()).contains("Registrar Console");
     assertThat(response.getPayload()).contains("reg-content-and-footer");
-    assertMetric("OtherRegistrar", "true", "[OWNER, ADMIN]", "SUCCESS");
+    assertMetric("NewRegistrar", "true", "[OWNER, ADMIN]", "SUCCESS");
   }
 
   @Test
   public void testUserHasAccessAsTheRegistrar_showsClientIdChooser() {
     action.run();
     assertThat(response.getPayload()).contains("<option value=\"TheRegistrar\" selected>");
-    assertThat(response.getPayload()).contains("<option value=\"OtherRegistrar\">");
+    assertThat(response.getPayload()).contains("<option value=\"NewRegistrar\">");
     assertThat(response.getPayload()).contains("<option value=\"AdminRegistrar\">");
     assertMetric("TheRegistrar", "false", "[OWNER]", "SUCCESS");
   }

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

@@ -86,10 +86,11 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
   public void testFailure_readRegistrarInfo_notAuthorized() {
     setUserWithoutAccess();
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of("id", CLIENT_ID));
-    assertThat(response).containsExactly(
+    assertThat(response)
+        .containsExactly(
             "status", "ERROR",
             "results", ImmutableList.of(),
-        "message", "forbidden test error");
+            "message", "TestUserId doesn't have access to registrar TheRegistrar");
     assertNoTasksEnqueued("sheet");
     assertMetric(CLIENT_ID, "read", "[]", "ERROR: ForbiddenException");
   }
@@ -160,10 +161,11 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
         "op", "update",
         "id", CLIENT_ID,
         "args", ImmutableMap.of("lastUpdateTime", getLastUpdateTime())));
-    assertThat(response).containsExactly(
+    assertThat(response)
+        .containsExactly(
             "status", "ERROR",
             "results", ImmutableList.of(),
-        "message", "forbidden test error");
+            "message", "TestUserId doesn't have access to registrar TheRegistrar");
     assertNoTasksEnqueued("sheet");
     assertMetric(CLIENT_ID, "update", "[]", "ERROR: ForbiddenException");
   }

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

@@ -17,13 +17,11 @@ package google.registry.ui.server.registrar;
 import static com.google.monitoring.metrics.contrib.LongMetricSubject.assertThat;
 import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailAddress;
 import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailDisplayName;
+import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.ADMIN;
+import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.OWNER;
 import static google.registry.security.JsonHttpTestUtils.createJsonPayload;
 import static google.registry.testing.DatastoreHelper.createTlds;
 import static google.registry.testing.DatastoreHelper.disallowRegistrarAccess;
-import static google.registry.testing.DatastoreHelper.loadRegistrar;
-import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role.ADMIN;
-import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role.OWNER;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import com.google.appengine.api.users.User;
@@ -32,12 +30,12 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSetMultimap;
 import google.registry.config.RegistryEnvironment;
 import google.registry.model.ofy.Ofy;
-import google.registry.request.HttpException.ForbiddenException;
 import google.registry.request.JsonActionRunner;
 import google.registry.request.JsonResponse;
 import google.registry.request.ResponseImpl;
 import google.registry.request.auth.AuthLevel;
 import google.registry.request.auth.AuthResult;
+import google.registry.request.auth.AuthenticatedRegistrarAccessor;
 import google.registry.request.auth.UserAuthInfo;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
@@ -134,34 +132,25 @@ public class RegistrarSettingsActionTestCase {
     RegistrarConsoleMetrics.settingsRequestMetric.reset(clientId, op, roles, status);
   }
 
-  /** Sets registrarAccessor.getRegistrar to succeed for all AccessTypes. */
+  /** Sets registrarAccessor.getRegistrar to succeed for CLIENT_ID only. */
   protected void setUserWithAccess() {
-    action.registrarAccessor = mock(AuthenticatedRegistrarAccessor.class);
+    action.registrarAccessor =
-
+        AuthenticatedRegistrarAccessor.createForTesting(
-    when(action.registrarAccessor.getAllClientIdWithRoles())
+            ImmutableSetMultimap.of(CLIENT_ID, OWNER));
-        .thenReturn(ImmutableSetMultimap.of(CLIENT_ID, OWNER));
-    when(action.registrarAccessor.getRegistrar(CLIENT_ID))
-        .thenAnswer(x -> loadRegistrar(CLIENT_ID));
   }
 
   /** Sets registrarAccessor.getRegistrar to always fail. */
   protected void setUserWithoutAccess() {
-    action.registrarAccessor = mock(AuthenticatedRegistrarAccessor.class);
+    action.registrarAccessor =
-
+        AuthenticatedRegistrarAccessor.createForTesting(ImmutableSetMultimap.of());
-    when(action.registrarAccessor.getAllClientIdWithRoles()).thenReturn(ImmutableSetMultimap.of());
-    when(action.registrarAccessor.getRegistrar(CLIENT_ID))
-        .thenThrow(new ForbiddenException("forbidden test error"));
   }
 
   /**
    * Sets registrarAccessor.getAllClientIdWithRoles to return a map with admin role for CLIENT_ID
    */
   protected void setUserAdmin() {
-    action.registrarAccessor = mock(AuthenticatedRegistrarAccessor.class);
+    action.registrarAccessor =
-
+        AuthenticatedRegistrarAccessor.createForTesting(
-    when(action.registrarAccessor.getAllClientIdWithRoles())
+            ImmutableSetMultimap.of(CLIENT_ID, ADMIN));
-        .thenReturn(ImmutableSetMultimap.of(CLIENT_ID, ADMIN));
-    when(action.registrarAccessor.getRegistrar(CLIENT_ID))
-        .thenAnswer(x -> loadRegistrar(CLIENT_ID));
   }
 }