From 557984bb7549aeb3312da5cee34b43e728d85ba1 Mon Sep 17 00:00:00 2001 From: guyben Date: Fri, 9 Nov 2018 08:11:26 -0800 Subject: [PATCH] Add support G-Suite group whose members have ADMIN access to registrar console After this CL, "support" accounts (accounts that are part of the "support" G-Suite group) will the same access to the registrar console as GCP "admins". However, they don't won't have access to the GCP project itself. We could give them their own Role in the future (say SUPPORT) and give them different access than "admins", but right now we don't need it and YAGNI or something :) NOTE: we identify users by their email (they need to be logged in to a google account). I don't know if that's best practice, since I guess different google accounts might have the same email address. However, G-Suite groups' membership is by email so there's not much we can do about it if we want to use G-Suite groups. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=220804273 --- .../registry/config/RegistryConfig.java | 13 +++ .../config/RegistryConfigSettings.java | 1 + .../registry/config/files/default-config.yaml | 4 + .../nomulus-config-production-sample.yaml | 1 + .../groups/DirectoryGroupsConnection.java | 57 +++++++++++ .../registry/groups/GroupsConnection.java | 3 + java/google/registry/module/frontend/BUILD | 1 + .../module/frontend/FrontendComponent.java | 6 ++ java/google/registry/module/pubapi/BUILD | 1 + .../module/pubapi/PubApiComponent.java | 6 ++ .../AuthenticatedRegistrarAccessor.java | 76 +++++++++++--- .../google/registry/ui/server/registrar/BUILD | 2 + .../groups/DirectoryGroupsConnectionTest.java | 39 ++++++++ .../AuthenticatedRegistrarAccessorTest.java | 99 ++++++++++++++++--- .../google/registry/ui/server/registrar/BUILD | 1 + 15 files changed, 285 insertions(+), 25 deletions(-) diff --git a/java/google/registry/config/RegistryConfig.java b/java/google/registry/config/RegistryConfig.java index 551a7d184..91500a4ee 100644 --- a/java/google/registry/config/RegistryConfig.java +++ b/java/google/registry/config/RegistryConfig.java @@ -392,6 +392,19 @@ public final class RegistryConfig { return config.gSuite.adminAccountEmailAddress; } + /** + * Returns the email address of the group containing emails of support accounts. + * + *

These accounts will have "ADMIN" access to the registrar console. + * + * @see google.registry.groups.DirectoryGroupsConnection + */ + @Provides + @Config("gSuiteSupportGroupEmailAddress") + public static String provideGSuiteSupportGroupEmailAddress(RegistryConfigSettings config) { + return config.gSuite.supportGroupEmailAddress; + } + /** * Returns the email address(es) that notifications of registrar and/or registrar contact * updates should be sent to, or the empty list if updates should not be sent. diff --git a/java/google/registry/config/RegistryConfigSettings.java b/java/google/registry/config/RegistryConfigSettings.java index 404ce1254..95289073c 100644 --- a/java/google/registry/config/RegistryConfigSettings.java +++ b/java/google/registry/config/RegistryConfigSettings.java @@ -66,6 +66,7 @@ public class RegistryConfigSettings { public String outgoingEmailAddress; public String outgoingEmailDisplayName; public String adminAccountEmailAddress; + public String supportGroupEmailAddress; } /** Configuration options for registry policy. */ diff --git a/java/google/registry/config/files/default-config.yaml b/java/google/registry/config/files/default-config.yaml index b5deec22f..25323259d 100644 --- a/java/google/registry/config/files/default-config.yaml +++ b/java/google/registry/config/files/default-config.yaml @@ -32,6 +32,10 @@ gSuite: # logging in to perform administrative actions, not sending emails. adminAccountEmailAddress: admin@example.com + # Group containing the emails of the support accounts. These accounts will be + # given "ADMIN" role on the registrar console. + supportGroupEmailAddress: support@example.com + registryPolicy: # Repository identifier (ROID) suffix for contacts and hosts. contactAndHostRoidSuffix: ROID diff --git a/java/google/registry/config/files/nomulus-config-production-sample.yaml b/java/google/registry/config/files/nomulus-config-production-sample.yaml index 9d2e9ca44..4d065acbc 100644 --- a/java/google/registry/config/files/nomulus-config-production-sample.yaml +++ b/java/google/registry/config/files/nomulus-config-production-sample.yaml @@ -18,6 +18,7 @@ gSuite: outgoingEmailDisplayName: placeholder outgoingEmailAddress: placeholder adminAccountEmailAddress: placeholder + supportGroupEmailAddress: placeholder registryPolicy: contactAndHostRoidSuffix: placeholder diff --git a/java/google/registry/groups/DirectoryGroupsConnection.java b/java/google/registry/groups/DirectoryGroupsConnection.java index 7c4e0a91a..5cf0f35ed 100644 --- a/java/google/registry/groups/DirectoryGroupsConnection.java +++ b/java/google/registry/groups/DirectoryGroupsConnection.java @@ -46,6 +46,22 @@ public class DirectoryGroupsConnection implements GroupsConnection { private static final String MEMBER_NOT_FOUND_MSG = "Resource Not Found: memberKey"; private static final String MEMBER_ALREADY_EXISTS_MSG = "Member already exists."; + /** + * All possible errors from {@link Directory.Members#get} when an email doesn't belong to a group. + * + *

See {@link #isMemberOfGroup} for details. + * + *

TODO(b/119220829): remove once we transition to using hasMember + * + *

TODO(b/119221854): update error messages if and when they change + */ + private static final ImmutableSet ERROR_MESSAGES_MEMBER_NOT_FOUND = + ImmutableSet.of( + // The given email corresponds to an actual account, but isn't part of this group + "Resource Not Found: memberKey", + // There's no account corresponding to this email + "Missing required field: memberKey"); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final Groups defaultGroupPermissions = getDefaultGroupPermissions(); @@ -163,4 +179,45 @@ public class DirectoryGroupsConnection implements GroupsConnection { } } } + + @Override + public boolean isMemberOfGroup(String memberEmail, String groupKey) { + // We're using "get" instead of "hasMember" because "hasMember" fails for emails that don't + // belong to the G-Suite domain. + // + // "get" fails for users that aren't part of the group, but it also might fail for other + // reasons (no access, group doesn't exist etc.). + // Which error is caused by "user isn't in that group" isn't documented, and was found using + // trial and error. + // + // TODO(b/119221676): transition to using hasMember + // + // Documentation for the API of "get": + // https://developers.google.com/admin-sdk/directory/v1/reference/members/get + // + // Documentation for the API of "hasMember": + // https://developers.google.com/admin-sdk/directory/v1/reference/members/hasMember + try { + Directory.Members.Get getRequest = directory.members().get(groupKey, memberEmail); + Member getReply = getRequest.execute(); + logger.atInfo().log( + "%s is a member of the group %s. Got reply: %s", memberEmail, groupKey, getReply); + return true; + } catch (GoogleJsonResponseException e) { + if (ERROR_MESSAGES_MEMBER_NOT_FOUND.contains(e.getDetails().getMessage())) { + // This means the "get" request failed because the email wasn't part of the group. + // This is expected behavior for any visitor that isn't a support group member. + logger.atInfo().log( + "%s isn't a member of the group %s. Got reply %s", + memberEmail, groupKey, e.getMessage()); + return false; + } + // If we got here - we had an unexpected error. Rethrow. + throw new RuntimeException( + String.format("Error checking whether %s is in group %s", memberEmail, groupKey), e); + } catch (IOException e) { + throw new RuntimeException( + String.format("Error checking whether %s is in group %s", memberEmail, groupKey), e); + } + } } diff --git a/java/google/registry/groups/GroupsConnection.java b/java/google/registry/groups/GroupsConnection.java index 23ed0af26..5f7aaf0f9 100644 --- a/java/google/registry/groups/GroupsConnection.java +++ b/java/google/registry/groups/GroupsConnection.java @@ -58,4 +58,7 @@ public interface GroupsConnection { * automatically added as an owner. */ Group createGroup(String groupKey) throws IOException; + + /** Checks whether the given email belongs to the "support" group. */ + boolean isMemberOfGroup(String memberEmail, String groupKey); } diff --git a/java/google/registry/module/frontend/BUILD b/java/google/registry/module/frontend/BUILD index 88f243784..796bee607 100644 --- a/java/google/registry/module/frontend/BUILD +++ b/java/google/registry/module/frontend/BUILD @@ -11,6 +11,7 @@ java_library( "//java/google/registry/config", "//java/google/registry/dns", "//java/google/registry/flows", + "//java/google/registry/groups", "//java/google/registry/keyring", "//java/google/registry/keyring/api", "//java/google/registry/keyring/kms", diff --git a/java/google/registry/module/frontend/FrontendComponent.java b/java/google/registry/module/frontend/FrontendComponent.java index 1c96edc5b..04314dbe2 100644 --- a/java/google/registry/module/frontend/FrontendComponent.java +++ b/java/google/registry/module/frontend/FrontendComponent.java @@ -21,6 +21,9 @@ import google.registry.config.CredentialModule; import google.registry.config.RegistryConfig.ConfigModule; import google.registry.flows.ServerTridProviderModule; import google.registry.flows.custom.CustomLogicFactoryModule; +import google.registry.groups.DirectoryModule; +import google.registry.groups.GroupsModule; +import google.registry.groups.GroupssettingsModule; import google.registry.keyring.KeyringModule; import google.registry.keyring.api.DummyKeyringModule; import google.registry.keyring.api.KeyModule; @@ -48,8 +51,11 @@ import javax.inject.Singleton; ConsoleConfigModule.class, CredentialModule.class, CustomLogicFactoryModule.class, + DirectoryModule.class, DummyKeyringModule.class, FrontendRequestComponentModule.class, + GroupsModule.class, + GroupssettingsModule.class, Jackson2Module.class, KeyModule.class, KeyringModule.class, diff --git a/java/google/registry/module/pubapi/BUILD b/java/google/registry/module/pubapi/BUILD index 40d4b233b..5d0307c85 100644 --- a/java/google/registry/module/pubapi/BUILD +++ b/java/google/registry/module/pubapi/BUILD @@ -11,6 +11,7 @@ java_library( "//java/google/registry/config", "//java/google/registry/dns", "//java/google/registry/flows", + "//java/google/registry/groups", "//java/google/registry/keyring", "//java/google/registry/keyring/api", "//java/google/registry/keyring/kms", diff --git a/java/google/registry/module/pubapi/PubApiComponent.java b/java/google/registry/module/pubapi/PubApiComponent.java index 966ad54f0..1b65390a0 100644 --- a/java/google/registry/module/pubapi/PubApiComponent.java +++ b/java/google/registry/module/pubapi/PubApiComponent.java @@ -21,6 +21,9 @@ import google.registry.config.CredentialModule; import google.registry.config.RegistryConfig.ConfigModule; import google.registry.flows.ServerTridProviderModule; import google.registry.flows.custom.CustomLogicFactoryModule; +import google.registry.groups.DirectoryModule; +import google.registry.groups.GroupsModule; +import google.registry.groups.GroupssettingsModule; import google.registry.keyring.KeyringModule; import google.registry.keyring.api.DummyKeyringModule; import google.registry.keyring.api.KeyModule; @@ -46,7 +49,10 @@ import javax.inject.Singleton; ConfigModule.class, CredentialModule.class, CustomLogicFactoryModule.class, + DirectoryModule.class, DummyKeyringModule.class, + GroupsModule.class, + GroupssettingsModule.class, Jackson2Module.class, KeyModule.class, KeyringModule.class, diff --git a/java/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessor.java b/java/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessor.java index f1a531d93..c2d4bd77c 100644 --- a/java/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessor.java +++ b/java/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessor.java @@ -17,11 +17,14 @@ package google.registry.ui.server.registrar; 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.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.flogger.FluentLogger; +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.RegistrarContact; import google.registry.request.HttpException.ForbiddenException; @@ -61,12 +64,44 @@ public class AuthenticatedRegistrarAccessor { */ private final ImmutableSetMultimap roleMap; + /** + * Overriding the injected {@link GroupsConnection} for tests. + * + *

{@link GroupsConnection} needs the injected DelegatedCredential GoogleCredential. However, + * this can't be initialized in the test environment. + * + *

The test server used in the javatests/google/registry/webdriver/ tests will hang if we try + * to instantiate the {@link GroupsConnection}. So instead we inject a {@link Lazy} version and + * allow tests to override the injected instace with (presumabley) a mock insteance. + */ + @VisibleForTesting public static GroupsConnection overrideGroupsConnection = null; + @Inject public AuthenticatedRegistrarAccessor( - AuthResult authResult, @Config("registryAdminClientId") String registryAdminClientId) { + AuthResult authResult, + @Config("registryAdminClientId") String registryAdminClientId, + @Config("gSuiteSupportGroupEmailAddress") String gSuiteSupportGroupEmailAddress, + Lazy groupsConnection) { + this( + authResult, + registryAdminClientId, + gSuiteSupportGroupEmailAddress, + overrideGroupsConnection != null ? overrideGroupsConnection : groupsConnection.get()); + } + + AuthenticatedRegistrarAccessor( + AuthResult authResult, + @Config("registryAdminClientId") String registryAdminClientId, + @Config("gSuiteSupportGroupEmailAddress") String gSuiteSupportGroupEmailAddress, + GroupsConnection groupsConnection) { this.authResult = authResult; this.registryAdminClientId = registryAdminClientId; - this.roleMap = createRoleMap(authResult, registryAdminClientId); + this.roleMap = + createRoleMap( + authResult, + registryAdminClientId, + groupsConnection, + gSuiteSupportGroupEmailAddress); logger.atInfo().log( "%s has the following roles: %s", authResult.userIdForLogging(), roleMap); @@ -156,8 +191,27 @@ public class AuthenticatedRegistrarAccessor { return registrar; } + private static boolean checkIsSupport( + GroupsConnection groupsConnection, String userEmail, String supportEmail) { + if (Strings.isNullOrEmpty(supportEmail)) { + return false; + } + try { + return groupsConnection.isMemberOfGroup(userEmail, supportEmail); + } catch (RuntimeException e) { + logger.atSevere().withCause(e).log( + "Error checking whether email %s belongs to support group %s." + + " Skipping support role check", + userEmail, supportEmail); + return false; + } + } + private static ImmutableSetMultimap createRoleMap( - AuthResult authResult, String registryAdminClientId) { + AuthResult authResult, + String registryAdminClientId, + GroupsConnection groupsConnection, + String gSuiteSupportGroupEmailAddress) { if (!authResult.userAuthInfo().isPresent()) { return ImmutableSetMultimap.of(); @@ -165,8 +219,10 @@ public class AuthenticatedRegistrarAccessor { UserAuthInfo userAuthInfo = authResult.userAuthInfo().get(); - boolean isAdmin = userAuthInfo.isUserAdmin(); User user = userAuthInfo.user(); + boolean isAdmin = userAuthInfo.isUserAdmin(); + boolean isSupport = + checkIsSupport(groupsConnection, user.getEmail(), gSuiteSupportGroupEmailAddress); ImmutableSetMultimap.Builder builder = new ImmutableSetMultimap.Builder<>(); @@ -174,17 +230,13 @@ public class AuthenticatedRegistrarAccessor { .load() .type(RegistrarContact.class) .filter("gaeUserId", user.getUserId()) - .forEach( - contact -> - builder - .put(contact.getParent().getName(), Role.OWNER)); + .forEach(contact -> builder.put(contact.getParent().getName(), Role.OWNER)); if (isAdmin && !Strings.isNullOrEmpty(registryAdminClientId)) { - builder - .put(registryAdminClientId, Role.OWNER); + builder.put(registryAdminClientId, Role.OWNER); } - if (isAdmin) { - // Admins have access to all registrars + if (isAdmin || isSupport) { + // Admins and support have access to all registrars ofy() .load() .type(Registrar.class) diff --git a/java/google/registry/ui/server/registrar/BUILD b/java/google/registry/ui/server/registrar/BUILD index e815fa505..a84e33db0 100644 --- a/java/google/registry/ui/server/registrar/BUILD +++ b/java/google/registry/ui/server/registrar/BUILD @@ -15,6 +15,7 @@ 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", @@ -24,6 +25,7 @@ java_library( "//java/google/registry/ui/soy:soy_java_wrappers", "//java/google/registry/ui/soy/registrar:soy_java_wrappers", "//java/google/registry/util", + "//third_party/java/inject_common", "//third_party/objectify:objectify-v4_1", "@com_google_appengine_api_1_0_sdk", "@com_google_auto_value", diff --git a/javatests/google/registry/groups/DirectoryGroupsConnectionTest.java b/javatests/google/registry/groups/DirectoryGroupsConnectionTest.java index f557f88a8..efe06cfb4 100644 --- a/javatests/google/registry/groups/DirectoryGroupsConnectionTest.java +++ b/javatests/google/registry/groups/DirectoryGroupsConnectionTest.java @@ -216,6 +216,45 @@ public class DirectoryGroupsConnectionTest { assertThat(connection.getMembersOfGroup("nonexistent_group@fake.notreal")).isEmpty(); } + @Test + public void test_isMemberOfSupportGroup_userInGroup_True() throws Exception { + when(members.get("support@domain-registry.example", "user@example.com")).thenReturn(membersGet); + when(membersGet.execute()).thenReturn(new Member()); + assertThat(connection.isMemberOfGroup("user@example.com", "support@domain-registry.example")) + .isTrue(); + } + + @Test + public void test_isMemberOfSupportGroup_userExistButNotInGroup_returnsFalse() throws Exception { + when(members.get("support@domain-registry.example", "user@example.com")) + .thenThrow(makeResponseException(0, "Resource Not Found: memberKey")); + when(membersGet.execute()).thenReturn(new Member()); + assertThat(connection.isMemberOfGroup("user@example.com", "support@domain-registry.example")) + .isFalse(); + } + + @Test + public void test_isMemberOfSupportGroup_userDoesntExist_returnsFalse() throws Exception { + when(members.get("support@domain-registry.example", "user@example.com")) + .thenThrow(makeResponseException(0, "Missing required field: memberKey")); + when(membersGet.execute()).thenReturn(new Member()); + assertThat(connection.isMemberOfGroup("user@example.com", "support@domain-registry.example")) + .isFalse(); + } + + @Test + public void test_isMemberOfSupportGroup_otherError_throws() throws Exception { + when(members.get("support@domain-registry.example", "user@example.com")) + .thenThrow(makeResponseException(0, "some error")); + when(membersGet.execute()).thenReturn(new Member()); + RuntimeException e = + assertThrows( + RuntimeException.class, + () -> + connection.isMemberOfGroup("user@example.com", "support@domain-registry.example")); + assertThat(e).hasCauseThat().isInstanceOf(GoogleJsonResponseException.class); + } + /** Runs a full test to add a member to the group and returns the expected Member. */ private Member runAddMemberTest() throws Exception { connection.addMemberToGroup("spam@example.com", "jim@example.com", Role.MEMBER); diff --git a/javatests/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessorTest.java b/javatests/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessorTest.java index dda2fde6c..8c6341d04 100644 --- a/javatests/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessorTest.java +++ b/javatests/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessorTest.java @@ -22,12 +22,17 @@ 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; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import com.google.appengine.api.users.User; 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.AuthLevel; import google.registry.request.auth.AuthResult; @@ -53,6 +58,7 @@ public class AuthenticatedRegistrarAccessorTest { private final HttpServletRequest req = mock(HttpServletRequest.class); private final HttpServletResponse rsp = mock(HttpServletResponse.class); + private final GroupsConnection groupsConnection = mock(GroupsConnection.class); private final TestLogHandler testLogHandler = new TestLogHandler(); private static final AuthResult AUTHORIZED_USER = createAuthResult(true, false); @@ -60,6 +66,7 @@ public class AuthenticatedRegistrarAccessorTest { private static final AuthResult AUTHORIZED_ADMIN = createAuthResult(true, true); private static final AuthResult UNAUTHORIZED_ADMIN = createAuthResult(false, true); private static final AuthResult NO_USER = AuthResult.create(AuthLevel.NONE); + private static final String SUPPORT_GROUP = "support@registry.example"; private static final String DEFAULT_CLIENT_ID = "TheRegistrar"; private static final String ADMIN_CLIENT_ID = "NewRegistrar"; @@ -79,6 +86,7 @@ public class AuthenticatedRegistrarAccessorTest { public void before() { LoggerConfig.getConfig(AuthenticatedRegistrarAccessor.class).addHandler(testLogHandler); persistResource(loadRegistrar(ADMIN_CLIENT_ID)); + when(groupsConnection.isMemberOfGroup(any(), any())).thenReturn(false); } @After @@ -96,17 +104,34 @@ public class AuthenticatedRegistrarAccessorTest { @Test public void getAllClientIdWithAccess_authorizedUser() { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(AUTHORIZED_USER, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + AUTHORIZED_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.getAllClientIdWithRoles()) .containsExactly(DEFAULT_CLIENT_ID, OWNER); } + /** 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); + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor( + AUTHORIZED_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); + + assertThat(registrarAccessor.getAllClientIdWithRoles()) + .containsExactly( + DEFAULT_CLIENT_ID, OWNER, + DEFAULT_CLIENT_ID, ADMIN, + ADMIN_CLIENT_ID, ADMIN); + } + /** Logged out users don't have access to anything. */ @Test public void getAllClientIdWithAccess_loggedOutUser() { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(NO_USER, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + NO_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.getAllClientIdWithRoles()).isEmpty(); } @@ -115,16 +140,56 @@ public class AuthenticatedRegistrarAccessorTest { @Test public void getAllClientIdWithAccess_unauthorizedUser() { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(UNAUTHORIZED_USER, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + UNAUTHORIZED_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.getAllClientIdWithRoles()).isEmpty(); } + /** 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); + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor( + UNAUTHORIZED_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); + + assertThat(registrarAccessor.getAllClientIdWithRoles()) + .containsExactly( + DEFAULT_CLIENT_ID, ADMIN, + ADMIN_CLIENT_ID, ADMIN); + } + + /** Empty Support group email - skips check. */ + @Test + public void getAllClientIdWithAccess_emptySupportEmail_works() { + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(AUTHORIZED_USER, ADMIN_CLIENT_ID, "", groupsConnection); + + verifyNoMoreInteractions(groupsConnection); + assertThat(registrarAccessor.getAllClientIdWithRoles()) + .containsExactly(DEFAULT_CLIENT_ID, OWNER); + } + + /** Support group check throws - continue anyway. */ + @Test + public void getAllClientIdWithAccess_throwingGroupCheck_stillWorks() { + when(groupsConnection.isMemberOfGroup(any(), any())).thenThrow(new RuntimeException("blah")); + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor( + AUTHORIZED_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); + + verify(groupsConnection).isMemberOfGroup("good_user@gmail.com", SUPPORT_GROUP); + assertThat(registrarAccessor.getAllClientIdWithRoles()) + .containsExactly(DEFAULT_CLIENT_ID, OWNER); + } + /** Admins have read/write access to the authorized registrars, AND the admin registrar. */ @Test public void getAllClientIdWithAccess_authorizedAdmin() { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(AUTHORIZED_ADMIN, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + AUTHORIZED_ADMIN, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.getAllClientIdWithRoles()) .containsExactly( @@ -141,7 +206,8 @@ public class AuthenticatedRegistrarAccessorTest { @Test public void getAllClientIdWithAccess_unauthorizedAdmin() { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(UNAUTHORIZED_ADMIN, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + UNAUTHORIZED_ADMIN, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.getAllClientIdWithRoles()) .containsExactly( @@ -199,7 +265,8 @@ public class AuthenticatedRegistrarAccessorTest { private void expectGetRegistrarSuccess( AuthResult authResult, String message) { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(authResult, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + authResult, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.getRegistrar(DEFAULT_CLIENT_ID)).isNotNull(); assertAboutLogs() @@ -211,7 +278,8 @@ public class AuthenticatedRegistrarAccessorTest { private void expectGetRegistrarFailure( String clientId, AuthResult authResult, String message) { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(authResult, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + authResult, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); ForbiddenException exception = assertThrows( @@ -224,7 +292,8 @@ public class AuthenticatedRegistrarAccessorTest { @Test public void testGuessClientIdForUser_hasAccess_isNotAdmin() { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(AUTHORIZED_USER, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + AUTHORIZED_USER, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.guessClientId()).isEqualTo(DEFAULT_CLIENT_ID); } @@ -242,7 +311,8 @@ public class AuthenticatedRegistrarAccessorTest { @Test public void testGuessClientIdForUser_hasAccess_isAdmin() { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(AUTHORIZED_ADMIN, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + AUTHORIZED_ADMIN, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.guessClientId()).isEqualTo(DEFAULT_CLIENT_ID); } @@ -251,7 +321,8 @@ public class AuthenticatedRegistrarAccessorTest { @Test public void testGuessClientIdForUser_noAccess_isAdmin() { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(UNAUTHORIZED_ADMIN, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + UNAUTHORIZED_ADMIN, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.guessClientId()).isEqualTo(ADMIN_CLIENT_ID); } @@ -264,7 +335,7 @@ public class AuthenticatedRegistrarAccessorTest { @Test public void testGuessClientIdForUser_noAccess_isAdmin_adminClientIdEmpty() { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(UNAUTHORIZED_ADMIN, ""); + new AuthenticatedRegistrarAccessor(UNAUTHORIZED_ADMIN, "", SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.guessClientId()).isAnyOf(ADMIN_CLIENT_ID, DEFAULT_CLIENT_ID); } @@ -276,14 +347,16 @@ public class AuthenticatedRegistrarAccessorTest { @Test public void testGuessClientIdForUser_noAccess_isAdmin_adminClientIdInvalid() { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(UNAUTHORIZED_ADMIN, "NonexistentRegistrar"); + new AuthenticatedRegistrarAccessor( + UNAUTHORIZED_ADMIN, "NonexistentRegistrar", SUPPORT_GROUP, groupsConnection); assertThat(registrarAccessor.guessClientId()).isEqualTo("NonexistentRegistrar"); } private void expectGuessRegistrarFailure(AuthResult authResult, String message) { AuthenticatedRegistrarAccessor registrarAccessor = - new AuthenticatedRegistrarAccessor(authResult, ADMIN_CLIENT_ID); + new AuthenticatedRegistrarAccessor( + authResult, ADMIN_CLIENT_ID, SUPPORT_GROUP, groupsConnection); ForbiddenException exception = assertThrows(ForbiddenException.class, () -> registrarAccessor.guessClientId()); diff --git a/javatests/google/registry/ui/server/registrar/BUILD b/javatests/google/registry/ui/server/registrar/BUILD index e7a04c602..7e2aa43eb 100644 --- a/javatests/google/registry/ui/server/registrar/BUILD +++ b/javatests/google/registry/ui/server/registrar/BUILD @@ -14,6 +14,7 @@ java_library( deps = [ "//java/google/registry/config", "//java/google/registry/export/sheet", + "//java/google/registry/groups", "//java/google/registry/model", "//java/google/registry/request", "//java/google/registry/request/auth",