diff --git a/java/google/registry/flows/session/LoginFlow.java b/java/google/registry/flows/session/LoginFlow.java index ea02e8438..bf78555ca 100644 --- a/java/google/registry/flows/session/LoginFlow.java +++ b/java/google/registry/flows/session/LoginFlow.java @@ -130,7 +130,7 @@ public class LoginFlow implements Flow { throw e; } } - if (registrar.get().getState().equals(Registrar.State.PENDING)) { + if (!registrar.get().isLive()) { throw new RegistrarAccountNotActiveException(); } if (login.getNewPassword() != null) { // We don't support in-band password changes. diff --git a/java/google/registry/model/registrar/Registrar.java b/java/google/registry/model/registrar/Registrar.java index 0fb34cf63..019ccd549 100644 --- a/java/google/registry/model/registrar/Registrar.java +++ b/java/google/registry/model/registrar/Registrar.java @@ -159,7 +159,13 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable * This is a suspended account which is disallowed from provisioning new domains, but can * otherwise still perform other operations to continue operations. */ - SUSPENDED; + SUSPENDED, + + /** + * This registrar is completely disabled and cannot perform any EPP actions whatsoever, nor log + * in to the registrar console. + */ + DISABLED; } /** Regex for E.164 phone number format specified by {@code contact.xsd}. */ diff --git a/java/google/registry/request/auth/AuthenticatedRegistrarAccessor.java b/java/google/registry/request/auth/AuthenticatedRegistrarAccessor.java index 51ec7415b..520638d50 100644 --- a/java/google/registry/request/auth/AuthenticatedRegistrarAccessor.java +++ b/java/google/registry/request/auth/AuthenticatedRegistrarAccessor.java @@ -16,18 +16,22 @@ package google.registry.request.auth; import static com.google.common.base.MoreObjects.toStringHelper; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.Streams.stream; import static google.registry.model.ofy.ObjectifyService.ofy; import com.google.appengine.api.users.User; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.flogger.FluentLogger; +import com.googlecode.objectify.Key; import dagger.Lazy; import google.registry.config.RegistryConfig.Config; import google.registry.groups.GroupsConnection; import google.registry.model.registrar.Registrar; +import google.registry.model.registrar.Registrar.State; import google.registry.model.registrar.RegistrarContact; import java.util.Optional; import javax.annotation.concurrent.Immutable; @@ -297,6 +301,9 @@ public class AuthenticatedRegistrarAccessor { lazyGroupsConnection, user.getEmail(), gSuiteSupportGroupEmailAddress); } + /** + * Returns a map of registrar client IDs to roles for all registrars that the user has access to. + */ private static ImmutableSetMultimap createRoleMap( AuthResult authResult, boolean isAdmin, @@ -305,36 +312,36 @@ public class AuthenticatedRegistrarAccessor { if (!authResult.userAuthInfo().isPresent()) { return ImmutableSetMultimap.of(); } - UserAuthInfo userAuthInfo = authResult.userAuthInfo().get(); - User user = userAuthInfo.user(); - ImmutableSetMultimap.Builder builder = new ImmutableSetMultimap.Builder<>(); - logger.atInfo().log("Checking registrar contacts for user ID %s", user.getUserId()); - ofy() - .load() - .type(RegistrarContact.class) - .filter("gaeUserId", user.getUserId()) - .forEach(contact -> builder.put(contact.getParent().getName(), Role.OWNER)); - if (isAdmin && !Strings.isNullOrEmpty(registryAdminClientId)) { - builder.put(registryAdminClientId, Role.OWNER); - } + // Find all registrars that have a registrar contact with this user's ID. + ImmutableList> accessibleClientIds = + stream(ofy().load().type(RegistrarContact.class).filter("gaeUserId", user.getUserId())) + .map(RegistrarContact::getParent) + .collect(toImmutableList()); + // Filter out disabled registrars (note that pending registrars still allow console login). + ofy().load().keys(accessibleClientIds).values().stream() + .filter(registrar -> registrar.getState() != State.DISABLED) + .forEach(registrar -> builder.put(registrar.getClientId(), Role.OWNER)); + // Admins have ADMIN access to all registrars, and also OWNER access to the registry registrar + // and all non-REAL or non-live registrars. if (isAdmin) { - // Admins have ADMIN access to all registrars, and OWNER access to all non-REAL registrars ofy() .load() .type(Registrar.class) - .forEach(registrar -> { - if (registrar.getType() != Registrar.Type.REAL - || registrar.getState() == Registrar.State.PENDING) { - builder.put(registrar.getClientId(), Role.OWNER); - } - builder.put(registrar.getClientId(), Role.ADMIN); - }); + .forEach( + registrar -> { + if (registrar.getType() != Registrar.Type.REAL + || !registrar.isLive() + || registrar.getClientId().equals(registryAdminClientId)) { + builder.put(registrar.getClientId(), Role.OWNER); + } + builder.put(registrar.getClientId(), Role.ADMIN); + }); } return builder.build(); diff --git a/java/google/registry/request/auth/BUILD b/java/google/registry/request/auth/BUILD index ac1ff20de..f32f990fe 100644 --- a/java/google/registry/request/auth/BUILD +++ b/java/google/registry/request/auth/BUILD @@ -12,6 +12,7 @@ java_library( "//java/google/registry/groups", "//java/google/registry/model", "//java/google/registry/security", + "//third_party/objectify:objectify-v4_1", "@com_google_appengine_api_1_0_sdk", "@com_google_auto_value", "@com_google_code_findbugs_jsr305", diff --git a/java/google/registry/tools/ValidateLoginCredentialsCommand.java b/java/google/registry/tools/ValidateLoginCredentialsCommand.java index b4c771217..8e53a5fcc 100644 --- a/java/google/registry/tools/ValidateLoginCredentialsCommand.java +++ b/java/google/registry/tools/ValidateLoginCredentialsCommand.java @@ -80,6 +80,7 @@ final class ValidateLoginCredentialsCommand implements CommandWithRemoteApi { Registrar.loadByClientId(clientId), "Registrar %s not found", clientId); new TlsCredentials(true, clientCertificateHash, Optional.of(clientIpAddress)) .validate(registrar, password); - checkState(!registrar.getState().equals(Registrar.State.PENDING), "Account pending"); + checkState( + registrar.isLive(), "Registrar %s has non-live state: %s", clientId, registrar.getState()); } } diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java index 32df35918..74e7cc73f 100644 --- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java @@ -1467,26 +1467,26 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase { doFailingTest("login_valid.xml", RegistrarAccountNotActiveException.class); } + @Test + public void testFailure_disabledRegistrar() { + persistResource(getRegistrarBuilder().setState(State.DISABLED).build()); + doFailingTest("login_valid.xml", RegistrarAccountNotActiveException.class); + } + @Test public void testFailure_incorrectPassword() { persistResource(getRegistrarBuilder().setPassword("diff password").build()); diff --git a/javatests/google/registry/model/testdata/schema.txt b/javatests/google/registry/model/testdata/schema.txt index fb6342cf4..bc48d9813 100644 --- a/javatests/google/registry/model/testdata/schema.txt +++ b/javatests/google/registry/model/testdata/schema.txt @@ -424,6 +424,7 @@ class google.registry.model.registrar.Registrar$BillingAccountEntry { } enum google.registry.model.registrar.Registrar$State { ACTIVE; + DISABLED; PENDING; SUSPENDED; } diff --git a/javatests/google/registry/request/auth/AuthenticatedRegistrarAccessorTest.java b/javatests/google/registry/request/auth/AuthenticatedRegistrarAccessorTest.java index 4912222fd..c3e4f64f3 100644 --- a/javatests/google/registry/request/auth/AuthenticatedRegistrarAccessorTest.java +++ b/javatests/google/registry/request/auth/AuthenticatedRegistrarAccessorTest.java @@ -35,6 +35,7 @@ import com.google.common.testing.TestLogHandler; import dagger.Lazy; import google.registry.groups.GroupsConnection; import google.registry.model.registrar.Registrar; +import google.registry.model.registrar.Registrar.State; import google.registry.request.auth.AuthenticatedRegistrarAccessor.RegistrarAccessDeniedException; import google.registry.testing.AppEngineRule; import google.registry.testing.InjectRule; @@ -252,6 +253,21 @@ public class AuthenticatedRegistrarAccessorTest { verify(lazyGroupsConnection).get(); } + @Test + public void testGetRegistrarForUser_registrarIsDisabled_isNotAdmin() { + persistResource( + Registrar.loadByClientId("TheRegistrar") + .get() + .asBuilder() + .setState(State.DISABLED) + .build()); + expectGetRegistrarFailure( + CLIENT_ID_WITH_CONTACT, + USER, + "user user@gmail.com doesn't have access to registrar TheRegistrar"); + verify(lazyGroupsConnection).get(); + } + /** Fail loading registrar if user doesn't have access to it, even if it's not REAL. */ @Test public void testGetRegistrarForUser_noAccess_isNotAdmin_notReal() { @@ -302,6 +318,21 @@ public class AuthenticatedRegistrarAccessorTest { verifyZeroInteractions(lazyGroupsConnection); } + @Test + public void testGetRegistrarForUser_registrarIsDisabled_isAdmin() throws Exception { + persistResource( + Registrar.loadByClientId("NewRegistrar") + .get() + .asBuilder() + .setState(State.DISABLED) + .build()); + expectGetRegistrarSuccess( + REAL_CLIENT_ID_WITHOUT_CONTACT, + GAE_ADMIN, + "admin admin@gmail.com has [OWNER, ADMIN] access to registrar NewRegistrar."); + verifyZeroInteractions(lazyGroupsConnection); + } + /** Succeed loading non-REAL registrar for admin. */ @Test public void testGetRegistrarForUser_notInContacts_isAdmin_notReal() throws Exception { diff --git a/javatests/google/registry/tools/ValidateLoginCredentialsCommandTest.java b/javatests/google/registry/tools/ValidateLoginCredentialsCommandTest.java index c715a051e..366c85f41 100644 --- a/javatests/google/registry/tools/ValidateLoginCredentialsCommandTest.java +++ b/javatests/google/registry/tools/ValidateLoginCredentialsCommandTest.java @@ -14,6 +14,7 @@ package google.registry.tools; +import static com.google.common.truth.Truth.assertThat; import static google.registry.model.registrar.Registrar.State.ACTIVE; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.loadRegistrar; @@ -26,6 +27,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import google.registry.flows.EppException; import google.registry.flows.TransportCredentials.BadRegistrarPasswordException; +import google.registry.model.registrar.Registrar; +import google.registry.model.registrar.Registrar.State; import google.registry.testing.CertificateSamples; import google.registry.util.CidrAddressBlock; import org.junit.Before; @@ -62,6 +65,28 @@ public class ValidateLoginCredentialsCommandTest "--ip_address=" + CLIENT_IP); } + @Test + public void testFailure_registrarIsDisabled() { + persistResource( + Registrar.loadByClientId("NewRegistrar") + .get() + .asBuilder() + .setState(State.DISABLED) + .build()); + IllegalStateException thrown = + assertThrows( + IllegalStateException.class, + () -> + runCommand( + "--client=NewRegistrar", + "--password=" + PASSWORD, + "--cert_hash=" + CERT_HASH, + "--ip_address=" + CLIENT_IP)); + assertThat(thrown) + .hasMessageThat() + .isEqualTo("Registrar NewRegistrar has non-live state: DISABLED"); + } + @Test public void testFailure_loginWithBadPassword() { EppException thrown = diff --git a/javatests/google/registry/ui/server/registrar/BUILD b/javatests/google/registry/ui/server/registrar/BUILD index abe4269cd..b8ac359a1 100644 --- a/javatests/google/registry/ui/server/registrar/BUILD +++ b/javatests/google/registry/ui/server/registrar/BUILD @@ -27,6 +27,7 @@ java_library( "//javatests/google/registry/testing", "//third_party/objectify:objectify-v4_1", "@com_google_appengine_api_1_0_sdk", + "@com_google_dagger", "@com_google_flogger", "@com_google_flogger_system_backend", "@com_google_guava", diff --git a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java index 29a6c389e..c82a5153a 100644 --- a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java +++ b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java @@ -92,7 +92,7 @@ public class ConsoleUiActionTest { } @After - public void tearDown() throws Exception { + public void tearDown() { assertThat(RegistrarConsoleMetrics.consoleRequestMetric).hasNoOtherValues(); } diff --git a/javatests/google/registry/webdriver/RegistrarConsoleScreenshotTest.java b/javatests/google/registry/webdriver/RegistrarConsoleScreenshotTest.java index 3c0bb608b..8eb46bb93 100644 --- a/javatests/google/registry/webdriver/RegistrarConsoleScreenshotTest.java +++ b/javatests/google/registry/webdriver/RegistrarConsoleScreenshotTest.java @@ -23,6 +23,7 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME; import com.google.common.collect.ImmutableMap; import com.googlecode.objectify.ObjectifyFilter; import google.registry.model.ofy.OfyFilter; +import google.registry.model.registrar.Registrar.State; import google.registry.module.frontend.FrontendServlet; import google.registry.server.RegistryTestServer; import google.registry.testing.CertificateSamples; @@ -257,6 +258,17 @@ public class RegistrarConsoleScreenshotTest { driver.diffPage("edit"); } + @Test + public void index_registrarDisabled() throws Throwable { + server.runInAppEngineEnvironment( + () -> + persistResource( + loadRegistrar("TheRegistrar").asBuilder().setState(State.DISABLED).build())); + driver.get(server.getUrl("/registrar")); + driver.waitForElement(By.tagName("h1")); + driver.diffPage("view"); + } + @Test public void settingsWhois() throws Throwable { driver.get(server.getUrl("/registrar#whois-settings")); diff --git a/javatests/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_index_registrarDisabled_view.png b/javatests/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_index_registrarDisabled_view.png new file mode 100644 index 000000000..cf40cb49c Binary files /dev/null and b/javatests/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_index_registrarDisabled_view.png differ