From 4240be268ae1d7ab82694d83e5c18ac1b5aacc3d Mon Sep 17 00:00:00 2001
From: mcilwain <mcilwain@google.com>
Date: Tue, 26 Mar 2019 10:30:39 -0700
Subject: [PATCH] Check registrar existence prior to verifying access

This way the error messages are more sensible when a registrar doesn't exist
(which realistically shouldn't happen in the typical case anyway).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=240376239
---
 .../request/auth/AuthenticatedRegistrarAccessor.java       | 7 ++-----
 .../request/auth/AuthenticatedRegistrarAccessorTest.java   | 2 +-
 .../registry/ui/server/registrar/OteStatusActionTest.java  | 5 +++--
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/java/google/registry/request/auth/AuthenticatedRegistrarAccessor.java b/java/google/registry/request/auth/AuthenticatedRegistrarAccessor.java
index 1d9412d1a..4fd31b7b3 100644
--- a/java/google/registry/request/auth/AuthenticatedRegistrarAccessor.java
+++ b/java/google/registry/request/auth/AuthenticatedRegistrarAccessor.java
@@ -220,16 +220,13 @@ public class AuthenticatedRegistrarAccessor {
    * @param clientId ID of the registrar we request
    */
   public Registrar getRegistrar(String clientId) throws RegistrarAccessDeniedException {
-    // Verify access before checking if the registrar exists, in order to not leak information
-    // about objects in the system the user doesn't have permissions on.
-    verifyAccess(clientId);
-
     Registrar registrar =
         Registrar.loadByClientId(clientId)
             .orElseThrow(
                 () ->
                     new RegistrarAccessDeniedException(
-                        String.format("Registrar %s not found", clientId)));
+                        String.format("Registrar %s does not exist", clientId)));
+    verifyAccess(clientId);
 
     if (!clientId.equals(registrar.getClientId())) {
       logger.atSevere().log(
diff --git a/javatests/google/registry/request/auth/AuthenticatedRegistrarAccessorTest.java b/javatests/google/registry/request/auth/AuthenticatedRegistrarAccessorTest.java
index c3e4f64f3..293e004a9 100644
--- a/javatests/google/registry/request/auth/AuthenticatedRegistrarAccessorTest.java
+++ b/javatests/google/registry/request/auth/AuthenticatedRegistrarAccessorTest.java
@@ -349,7 +349,7 @@ public class AuthenticatedRegistrarAccessorTest {
     expectGetRegistrarFailure(
         "BadClientId",
         GAE_ADMIN,
-        "admin admin@gmail.com doesn't have access to registrar BadClientId");
+        "Registrar BadClientId does not exist");
     verifyZeroInteractions(lazyGroupsConnection);
   }
 
diff --git a/javatests/google/registry/ui/server/registrar/OteStatusActionTest.java b/javatests/google/registry/ui/server/registrar/OteStatusActionTest.java
index 4fbc935f4..a9948f6bc 100644
--- a/javatests/google/registry/ui/server/registrar/OteStatusActionTest.java
+++ b/javatests/google/registry/ui/server/registrar/OteStatusActionTest.java
@@ -113,14 +113,15 @@ public final class OteStatusActionTest {
   }
 
   @Test
-  public void testFailure_noRegistrar() {
+  public void testFailure_registrarDoesntExist() {
     assertThat(action.handleJsonRequest(ImmutableMap.of("clientId", "nonexistent-3")))
         .containsExactlyEntriesIn(
-            errorResultWithMessage("TestUserId doesn't have access to registrar nonexistent-3"));
+            errorResultWithMessage("Registrar nonexistent-3 does not exist"));
   }
 
   @Test
   public void testFailure_notAuthorized() {
+    persistNewRegistrar(CLIENT_ID, "blobio-1", Type.REAL, 1L);
     action.registrarAccessor =
         AuthenticatedRegistrarAccessor.createForTesting(ImmutableSetMultimap.of());
     assertThat(action.handleJsonRequest(ImmutableMap.of("clientId", CLIENT_ID)))