Allow registrars to be completely DISABLED

Disabled registrar cannot perform any actions via EPP and cannot log in to the
registrar web console.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=239606389
This commit is contained in:
mcilwain 2019-03-21 09:07:17 -07:00 committed by jianglai
parent e4ac18ec31
commit d7306652eb
15 changed files with 138 additions and 46 deletions

View file

@ -130,7 +130,7 @@ public class LoginFlow implements Flow {
throw e; throw e;
} }
} }
if (registrar.get().getState().equals(Registrar.State.PENDING)) { if (!registrar.get().isLive()) {
throw new RegistrarAccountNotActiveException(); throw new RegistrarAccountNotActiveException();
} }
if (login.getNewPassword() != null) { // We don't support in-band password changes. if (login.getNewPassword() != null) { // We don't support in-band password changes.

View file

@ -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 * This is a suspended account which is disallowed from provisioning new domains, but can
* otherwise still perform other operations to continue operations. * 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}. */ /** Regex for E.164 phone number format specified by {@code contact.xsd}. */

View file

@ -16,18 +16,22 @@ package google.registry.request.auth;
import static com.google.common.base.MoreObjects.toStringHelper; import static com.google.common.base.MoreObjects.toStringHelper;
import static com.google.common.base.Preconditions.checkNotNull; 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 static google.registry.model.ofy.ObjectifyService.ofy;
import com.google.appengine.api.users.User; import com.google.appengine.api.users.User;
import com.google.common.annotations.VisibleForTesting; 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.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.flogger.FluentLogger; import com.google.common.flogger.FluentLogger;
import com.googlecode.objectify.Key;
import dagger.Lazy; import dagger.Lazy;
import google.registry.config.RegistryConfig.Config; import google.registry.config.RegistryConfig.Config;
import google.registry.groups.GroupsConnection; import google.registry.groups.GroupsConnection;
import google.registry.model.registrar.Registrar; import google.registry.model.registrar.Registrar;
import google.registry.model.registrar.Registrar.State;
import google.registry.model.registrar.RegistrarContact; import google.registry.model.registrar.RegistrarContact;
import java.util.Optional; import java.util.Optional;
import javax.annotation.concurrent.Immutable; import javax.annotation.concurrent.Immutable;
@ -297,6 +301,9 @@ public class AuthenticatedRegistrarAccessor {
lazyGroupsConnection, user.getEmail(), gSuiteSupportGroupEmailAddress); 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<String, Role> createRoleMap( private static ImmutableSetMultimap<String, Role> createRoleMap(
AuthResult authResult, AuthResult authResult,
boolean isAdmin, boolean isAdmin,
@ -305,36 +312,36 @@ public class AuthenticatedRegistrarAccessor {
if (!authResult.userAuthInfo().isPresent()) { if (!authResult.userAuthInfo().isPresent()) {
return ImmutableSetMultimap.of(); return ImmutableSetMultimap.of();
} }
UserAuthInfo userAuthInfo = authResult.userAuthInfo().get(); UserAuthInfo userAuthInfo = authResult.userAuthInfo().get();
User user = userAuthInfo.user(); User user = userAuthInfo.user();
ImmutableSetMultimap.Builder<String, Role> builder = new ImmutableSetMultimap.Builder<>(); ImmutableSetMultimap.Builder<String, Role> builder = new ImmutableSetMultimap.Builder<>();
logger.atInfo().log("Checking registrar contacts for user ID %s", user.getUserId()); logger.atInfo().log("Checking registrar contacts for user ID %s", user.getUserId());
ofy() // Find all registrars that have a registrar contact with this user's ID.
.load() ImmutableList<Key<Registrar>> accessibleClientIds =
.type(RegistrarContact.class) stream(ofy().load().type(RegistrarContact.class).filter("gaeUserId", user.getUserId()))
.filter("gaeUserId", user.getUserId()) .map(RegistrarContact::getParent)
.forEach(contact -> builder.put(contact.getParent().getName(), Role.OWNER)); .collect(toImmutableList());
if (isAdmin && !Strings.isNullOrEmpty(registryAdminClientId)) { // Filter out disabled registrars (note that pending registrars still allow console login).
builder.put(registryAdminClientId, Role.OWNER); 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) { if (isAdmin) {
// Admins have ADMIN access to all registrars, and OWNER access to all non-REAL registrars
ofy() ofy()
.load() .load()
.type(Registrar.class) .type(Registrar.class)
.forEach(registrar -> { .forEach(
if (registrar.getType() != Registrar.Type.REAL registrar -> {
|| registrar.getState() == Registrar.State.PENDING) { if (registrar.getType() != Registrar.Type.REAL
builder.put(registrar.getClientId(), Role.OWNER); || !registrar.isLive()
} || registrar.getClientId().equals(registryAdminClientId)) {
builder.put(registrar.getClientId(), Role.ADMIN); builder.put(registrar.getClientId(), Role.OWNER);
}); }
builder.put(registrar.getClientId(), Role.ADMIN);
});
} }
return builder.build(); return builder.build();

View file

@ -12,6 +12,7 @@ java_library(
"//java/google/registry/groups", "//java/google/registry/groups",
"//java/google/registry/model", "//java/google/registry/model",
"//java/google/registry/security", "//java/google/registry/security",
"//third_party/objectify:objectify-v4_1",
"@com_google_appengine_api_1_0_sdk", "@com_google_appengine_api_1_0_sdk",
"@com_google_auto_value", "@com_google_auto_value",
"@com_google_code_findbugs_jsr305", "@com_google_code_findbugs_jsr305",

View file

@ -80,6 +80,7 @@ final class ValidateLoginCredentialsCommand implements CommandWithRemoteApi {
Registrar.loadByClientId(clientId), "Registrar %s not found", clientId); Registrar.loadByClientId(clientId), "Registrar %s not found", clientId);
new TlsCredentials(true, clientCertificateHash, Optional.of(clientIpAddress)) new TlsCredentials(true, clientCertificateHash, Optional.of(clientIpAddress))
.validate(registrar, password); .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());
} }
} }

View file

@ -1467,26 +1467,26 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow,
@Test @Test
public void testFailure_suspendedRegistrarCantCreateDomain() { public void testFailure_suspendedRegistrarCantCreateDomain() {
persistContactsAndHosts(); doFailingTest_invalidRegistrarState(State.SUSPENDED);
persistResource(
Registrar.loadByClientId("TheRegistrar")
.get()
.asBuilder()
.setState(State.SUSPENDED)
.build());
EppException thrown =
assertThrows(RegistrarMustBeActiveForThisOperationException.class, this::runFlow);
assertAboutEppExceptions().that(thrown).marshalsToXml();
} }
@Test @Test
public void testFailure_pendingRegistrarCantCreateDomain() { public void testFailure_pendingRegistrarCantCreateDomain() {
doFailingTest_invalidRegistrarState(State.PENDING);
}
@Test
public void testFailure_disabledRegistrarCantCreateDomain() {
doFailingTest_invalidRegistrarState(State.DISABLED);
}
private void doFailingTest_invalidRegistrarState(State registrarState) {
persistContactsAndHosts(); persistContactsAndHosts();
persistResource( persistResource(
Registrar.loadByClientId("TheRegistrar") Registrar.loadByClientId("TheRegistrar")
.get() .get()
.asBuilder() .asBuilder()
.setState(State.PENDING) .setState(registrarState)
.build()); .build());
EppException thrown = EppException thrown =
assertThrows(RegistrarMustBeActiveForThisOperationException.class, this::runFlow); assertThrows(RegistrarMustBeActiveForThisOperationException.class, this::runFlow);

View file

@ -393,24 +393,25 @@ public class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, D
@Test @Test
public void testFailure_suspendedRegistrarCantRenewDomain() { public void testFailure_suspendedRegistrarCantRenewDomain() {
persistResource( doFailingTest_invalidRegistrarState(State.SUSPENDED);
Registrar.loadByClientId("TheRegistrar")
.get()
.asBuilder()
.setState(State.SUSPENDED)
.build());
EppException thrown =
assertThrows(RegistrarMustBeActiveForThisOperationException.class, this::runFlow);
assertAboutEppExceptions().that(thrown).marshalsToXml();
} }
@Test @Test
public void testFailure_pendingRegistrarCantRenewDomain() { public void testFailure_pendingRegistrarCantRenewDomain() {
doFailingTest_invalidRegistrarState(State.PENDING);
}
@Test
public void testFailure_disabledRegistrarCantRenewDomain() {
doFailingTest_invalidRegistrarState(State.DISABLED);
}
private void doFailingTest_invalidRegistrarState(State registrarState) {
persistResource( persistResource(
Registrar.loadByClientId("TheRegistrar") Registrar.loadByClientId("TheRegistrar")
.get() .get()
.asBuilder() .asBuilder()
.setState(State.PENDING) .setState(registrarState)
.build()); .build());
EppException thrown = EppException thrown =
assertThrows(RegistrarMustBeActiveForThisOperationException.class, this::runFlow); assertThrows(RegistrarMustBeActiveForThisOperationException.class, this::runFlow);

View file

@ -123,6 +123,12 @@ public abstract class LoginFlowTestCase extends FlowTestCase<LoginFlow> {
doFailingTest("login_valid.xml", RegistrarAccountNotActiveException.class); doFailingTest("login_valid.xml", RegistrarAccountNotActiveException.class);
} }
@Test
public void testFailure_disabledRegistrar() {
persistResource(getRegistrarBuilder().setState(State.DISABLED).build());
doFailingTest("login_valid.xml", RegistrarAccountNotActiveException.class);
}
@Test @Test
public void testFailure_incorrectPassword() { public void testFailure_incorrectPassword() {
persistResource(getRegistrarBuilder().setPassword("diff password").build()); persistResource(getRegistrarBuilder().setPassword("diff password").build());

View file

@ -424,6 +424,7 @@ class google.registry.model.registrar.Registrar$BillingAccountEntry {
} }
enum google.registry.model.registrar.Registrar$State { enum google.registry.model.registrar.Registrar$State {
ACTIVE; ACTIVE;
DISABLED;
PENDING; PENDING;
SUSPENDED; SUSPENDED;
} }

View file

@ -35,6 +35,7 @@ import com.google.common.testing.TestLogHandler;
import dagger.Lazy; import dagger.Lazy;
import google.registry.groups.GroupsConnection; import google.registry.groups.GroupsConnection;
import google.registry.model.registrar.Registrar; import google.registry.model.registrar.Registrar;
import google.registry.model.registrar.Registrar.State;
import google.registry.request.auth.AuthenticatedRegistrarAccessor.RegistrarAccessDeniedException; import google.registry.request.auth.AuthenticatedRegistrarAccessor.RegistrarAccessDeniedException;
import google.registry.testing.AppEngineRule; import google.registry.testing.AppEngineRule;
import google.registry.testing.InjectRule; import google.registry.testing.InjectRule;
@ -252,6 +253,21 @@ public class AuthenticatedRegistrarAccessorTest {
verify(lazyGroupsConnection).get(); 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. */ /** Fail loading registrar if user doesn't have access to it, even if it's not REAL. */
@Test @Test
public void testGetRegistrarForUser_noAccess_isNotAdmin_notReal() { public void testGetRegistrarForUser_noAccess_isNotAdmin_notReal() {
@ -302,6 +318,21 @@ public class AuthenticatedRegistrarAccessorTest {
verifyZeroInteractions(lazyGroupsConnection); 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. */ /** Succeed loading non-REAL registrar for admin. */
@Test @Test
public void testGetRegistrarForUser_notInContacts_isAdmin_notReal() throws Exception { public void testGetRegistrarForUser_notInContacts_isAdmin_notReal() throws Exception {

View file

@ -14,6 +14,7 @@
package google.registry.tools; 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.model.registrar.Registrar.State.ACTIVE;
import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.createTld;
import static google.registry.testing.DatastoreHelper.loadRegistrar; import static google.registry.testing.DatastoreHelper.loadRegistrar;
@ -26,6 +27,8 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import google.registry.flows.EppException; import google.registry.flows.EppException;
import google.registry.flows.TransportCredentials.BadRegistrarPasswordException; 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.testing.CertificateSamples;
import google.registry.util.CidrAddressBlock; import google.registry.util.CidrAddressBlock;
import org.junit.Before; import org.junit.Before;
@ -62,6 +65,28 @@ public class ValidateLoginCredentialsCommandTest
"--ip_address=" + CLIENT_IP); "--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 @Test
public void testFailure_loginWithBadPassword() { public void testFailure_loginWithBadPassword() {
EppException thrown = EppException thrown =

View file

@ -27,6 +27,7 @@ java_library(
"//javatests/google/registry/testing", "//javatests/google/registry/testing",
"//third_party/objectify:objectify-v4_1", "//third_party/objectify:objectify-v4_1",
"@com_google_appengine_api_1_0_sdk", "@com_google_appengine_api_1_0_sdk",
"@com_google_dagger",
"@com_google_flogger", "@com_google_flogger",
"@com_google_flogger_system_backend", "@com_google_flogger_system_backend",
"@com_google_guava", "@com_google_guava",

View file

@ -92,7 +92,7 @@ public class ConsoleUiActionTest {
} }
@After @After
public void tearDown() throws Exception { public void tearDown() {
assertThat(RegistrarConsoleMetrics.consoleRequestMetric).hasNoOtherValues(); assertThat(RegistrarConsoleMetrics.consoleRequestMetric).hasNoOtherValues();
} }

View file

@ -23,6 +23,7 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.googlecode.objectify.ObjectifyFilter; import com.googlecode.objectify.ObjectifyFilter;
import google.registry.model.ofy.OfyFilter; import google.registry.model.ofy.OfyFilter;
import google.registry.model.registrar.Registrar.State;
import google.registry.module.frontend.FrontendServlet; import google.registry.module.frontend.FrontendServlet;
import google.registry.server.RegistryTestServer; import google.registry.server.RegistryTestServer;
import google.registry.testing.CertificateSamples; import google.registry.testing.CertificateSamples;
@ -257,6 +258,17 @@ public class RegistrarConsoleScreenshotTest {
driver.diffPage("edit"); 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 @Test
public void settingsWhois() throws Throwable { public void settingsWhois() throws Throwable {
driver.get(server.getUrl("/registrar#whois-settings")); driver.get(server.getUrl("/registrar#whois-settings"));