From 33ad3e10e18ffff2378d6d4a156d87e6d7d24a0e Mon Sep 17 00:00:00 2001 From: Gus Brodman Date: Tue, 21 Jul 2026 11:12:08 -0400 Subject: [PATCH] Only grant IAP access to the console for new users Currently we grant users IAP_SECURED_WEB_APP_USER_ROLE access to the entire project upon creation if there is no preexisting group set up for them to use. These users only need to be able to access the console, so we should restrict it to the console. If there is a group (which we do have in our prod + sandbox configuration) then we just add them to that group instead and that group controls all the privileges. So this is a no-op for our setup. We prioritize the group -- if both the group and the IAP service ID are specified, we'll still just add them to the group b/534931170 --- .../registry/config/RegistryConfig.java | 6 ++ .../config/RegistryConfigSettings.java | 1 + .../registry/config/files/default-config.yaml | 5 ++ .../registry/model/OteAccountBuilder.java | 15 +++- .../google/registry/model/console/User.java | 18 ++--- .../registry/tools/CreateUserCommand.java | 7 +- .../java/google/registry/tools/IamClient.java | 22 +++++- .../registry/tools/SetupOteCommand.java | 7 +- .../ui/server/console/ConsoleOteAction.java | 6 +- .../ui/server/console/ConsoleUsersAction.java | 6 +- .../registry/model/OteAccountBuilderTest.java | 41 ++++++++-- .../registry/model/console/UserTest.java | 79 +++++++++++++++++-- .../registry/tools/CreateUserCommandTest.java | 51 ++++++++++-- .../google/registry/tools/IamClientTest.java | 13 ++- .../registry/tools/SetupOteCommandTest.java | 5 +- .../server/console/ConsoleOteActionTest.java | 2 + .../console/ConsoleUsersActionTest.java | 1 + 17 files changed, 244 insertions(+), 41 deletions(-) diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index cfbf40c122c..2c6aa162e63 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -165,6 +165,12 @@ public static boolean provideIsEmailSendingEnabled(RegistryConfigSettings config return config.misc.isEmailSendingEnabled; } + @Provides + @Config("consoleIapServiceId") + public static Optional provideConsoleIapServiceId(RegistryConfigSettings config) { + return Optional.ofNullable(config.registrarConsole.consoleIapServiceId); + } + /** * The e-mail address for general support. Used in the "contact-us" section of the registrar * console. diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index 455d298f483..5db71b0173a 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -186,6 +186,7 @@ public static class Rde { /** Configuration for the web-based registrar console. */ public static class RegistrarConsole { + public String consoleIapServiceId; public String dumFileName; public String supportPhoneNumber; public String supportEmailAddress; diff --git a/core/src/main/java/google/registry/config/files/default-config.yaml b/core/src/main/java/google/registry/config/files/default-config.yaml index 42270ea5730..4addece2d86 100644 --- a/core/src/main/java/google/registry/config/files/default-config.yaml +++ b/core/src/main/java/google/registry/config/files/default-config.yaml @@ -390,6 +390,11 @@ rde: sshIdentityEmailAddress: rde@example.com registrarConsole: + # IAP service ID of the HTTP(s) load balancer that serves the console-api backend. + # Users created in the console will be bound only to this service to limit access. + # This is only applicable to non-group-based auth (if there is no consoleUserGroupEmailAddress) + consoleIapServiceId: null + # DUM download file name, excluding the extension dumFileName: dum_file_name diff --git a/core/src/main/java/google/registry/model/OteAccountBuilder.java b/core/src/main/java/google/registry/model/OteAccountBuilder.java index 69b943231fb..9bbc33aa6ce 100644 --- a/core/src/main/java/google/registry/model/OteAccountBuilder.java +++ b/core/src/main/java/google/registry/model/OteAccountBuilder.java @@ -197,8 +197,7 @@ public OteAccountBuilder addUser(String email) { registrars.stream() .collect( toImmutableMap( - Registrar::getRegistrarId, - registrar -> RegistrarRole.ACCOUNT_MANAGER))) + Registrar::getRegistrarId, _ -> RegistrarRole.ACCOUNT_MANAGER))) .build()) .build()); return this; @@ -265,10 +264,18 @@ public ImmutableMap getRegistrarIdToTldMap() { /** Grants the users permission to pass IAP. */ public void grantIapPermission( - Optional groupEmailAddress, CloudTasksUtils cloudTasksUtils, IamClient iamClient) { + Optional groupEmailAddress, + Optional consoleIapServiceId, + CloudTasksUtils cloudTasksUtils, + IamClient iamClient) { for (User user : users) { User.grantIapPermission( - user.getEmailAddress(), groupEmailAddress, cloudTasksUtils, null, iamClient); + user.getEmailAddress(), + groupEmailAddress, + consoleIapServiceId, + cloudTasksUtils, + null, + iamClient); } } diff --git a/core/src/main/java/google/registry/model/console/User.java b/core/src/main/java/google/registry/model/console/User.java index 7ce960ab8b9..612906500b1 100644 --- a/core/src/main/java/google/registry/model/console/User.java +++ b/core/src/main/java/google/registry/model/console/User.java @@ -86,6 +86,7 @@ public class User extends UpdateAutoTimestampEntity implements Buildable { public static void grantIapPermission( String emailAddress, Optional groupEmailAddress, + Optional consoleIapServiceId, @Nullable CloudTasksUtils cloudTasksUtils, @Nullable ServiceConnection connection, IamClient iamClient) { @@ -93,14 +94,14 @@ public static void grantIapPermission( return; } checkArgument( - cloudTasksUtils != null || connection != null, - "At least one of cloudTasksUtils or connection must be set"); + cloudTasksUtils == null ^ connection == null, + "Precisely one of cloudTasksUtils or connection can be set"); checkArgument( - cloudTasksUtils == null || connection == null, - "Only one of cloudTasksUtils or connection can be set"); + groupEmailAddress.isPresent() || consoleIapServiceId.isPresent(), + "At least one of groupEmailAddress or consoleIapServiceId must be present"); if (groupEmailAddress.isEmpty()) { logger.atInfo().log("Granting IAP role to user %s", emailAddress); - iamClient.addBinding(emailAddress, IAP_SECURED_WEB_APP_USER_ROLE); + iamClient.addBinding(emailAddress, IAP_SECURED_WEB_APP_USER_ROLE, consoleIapServiceId.get()); } else { logger.atInfo().log("Adding %s to group %s", emailAddress, groupEmailAddress.get()); if (cloudTasksUtils != null) { @@ -129,11 +130,8 @@ public static void revokeIapPermission( return; } checkArgument( - cloudTasksUtils != null || connection != null, - "At least one of cloudTasksUtils or connection must be set"); - checkArgument( - cloudTasksUtils == null || connection == null, - "Only one of cloudTasksUtils or connection can be set"); + cloudTasksUtils == null ^ connection == null, + "Precisely one of cloudTasksUtils or connection can be set"); if (groupEmailAddress.isEmpty()) { logger.atInfo().log("Removing IAP role from user %s", emailAddress); iamClient.removeBinding(emailAddress, IAP_SECURED_WEB_APP_USER_ROLE); diff --git a/core/src/main/java/google/registry/tools/CreateUserCommand.java b/core/src/main/java/google/registry/tools/CreateUserCommand.java index 9caa725e9ef..8ffd20dbb84 100644 --- a/core/src/main/java/google/registry/tools/CreateUserCommand.java +++ b/core/src/main/java/google/registry/tools/CreateUserCommand.java @@ -38,6 +38,10 @@ public class CreateUserCommand extends CreateOrUpdateUserCommand implements Comm @Config("gSuiteConsoleUserGroupEmailAddress") Optional maybeGroupEmailAddress; + @Inject + @Config("consoleIapServiceId") + Optional consoleIapServiceId; + @Nullable @Override User getExistingUser(String email) { @@ -49,7 +53,8 @@ User getExistingUser(String email) { @Override protected String execute() throws Exception { String ret = super.execute(); - grantIapPermission(email, maybeGroupEmailAddress, null, connection, iamClient); + grantIapPermission( + email, maybeGroupEmailAddress, consoleIapServiceId, null, connection, iamClient); return ret; } diff --git a/core/src/main/java/google/registry/tools/IamClient.java b/core/src/main/java/google/registry/tools/IamClient.java index e83232899b0..cbbfc8554b8 100644 --- a/core/src/main/java/google/registry/tools/IamClient.java +++ b/core/src/main/java/google/registry/tools/IamClient.java @@ -16,6 +16,7 @@ import com.google.api.services.cloudresourcemanager.CloudResourceManager; import com.google.api.services.cloudresourcemanager.model.Binding; +import com.google.api.services.cloudresourcemanager.model.Expr; import com.google.api.services.cloudresourcemanager.model.GetIamPolicyRequest; import com.google.api.services.cloudresourcemanager.model.Policy; import com.google.api.services.cloudresourcemanager.model.SetIamPolicyRequest; @@ -31,7 +32,10 @@ @Singleton public class IamClient { + private static final String MEMBER_FORMAT = "user:%s"; + private static final String IAP_ACCESS_EXPRESSION_FORMAT = + "resource.name == 'projects/%s/iap_web/compute/services/%s'"; private final CloudResourceManager resourceManager; private final String projectId; @@ -60,7 +64,7 @@ protected IamClient(CloudResourceManager resourceManager, String projectId) { * *

No-op if the role is already bound to the account. */ - public void addBinding(String account, String role) { + public void addBinding(String account, String role, String consoleIapServiceId) { String member = String.format(MEMBER_FORMAT, account); Policy policy = getPolicy(); Binding binding = @@ -69,7 +73,21 @@ public void addBinding(String account, String role) { .findFirst() .orElseGet( () -> { - Binding newBinding = new Binding().setRole(role).setMembers(new ArrayList<>()); + Binding newBinding = + new Binding() + .setRole(role) + .setMembers(new ArrayList<>()) + .setCondition( + new Expr() + .setTitle("Registrar Console IAP access") + .setDescription( + "Restrict IAP access only to the Registrar Console HTTP(s)" + + " load balancer") + .setExpression( + String.format( + IAP_ACCESS_EXPRESSION_FORMAT, + projectId, + consoleIapServiceId))); policy.getBindings().add(newBinding); return newBinding; }); diff --git a/core/src/main/java/google/registry/tools/SetupOteCommand.java b/core/src/main/java/google/registry/tools/SetupOteCommand.java index 13584c23c70..5ece9b485cf 100644 --- a/core/src/main/java/google/registry/tools/SetupOteCommand.java +++ b/core/src/main/java/google/registry/tools/SetupOteCommand.java @@ -86,6 +86,10 @@ final class SetupOteCommand extends ConfirmingCommand { @Config("gSuiteConsoleUserGroupEmailAddress") Optional maybeGroupEmailAddress; + @Inject + @Config("consoleIapServiceId") + Optional consoleIapServiceId; + OteAccountBuilder oteAccountBuilder; String password; @@ -138,7 +142,8 @@ protected String prompt() { @Override public String execute() { ImmutableMap clientIdToTld = oteAccountBuilder.buildAndPersist(); - oteAccountBuilder.grantIapPermission(maybeGroupEmailAddress, cloudTasksUtils, iamClient); + oteAccountBuilder.grantIapPermission( + maybeGroupEmailAddress, consoleIapServiceId, cloudTasksUtils, iamClient); StringBuilder output = new StringBuilder(); diff --git a/core/src/main/java/google/registry/ui/server/console/ConsoleOteAction.java b/core/src/main/java/google/registry/ui/server/console/ConsoleOteAction.java index 2465a6a91ea..905297170be 100644 --- a/core/src/main/java/google/registry/ui/server/console/ConsoleOteAction.java +++ b/core/src/main/java/google/registry/ui/server/console/ConsoleOteAction.java @@ -62,6 +62,7 @@ public class ConsoleOteAction extends ConsoleApiAction { private final StringGenerator passwordGenerator; private final Optional oteCreateData; private final Optional maybeGroupEmailAddress; + private final Optional consoleIapServiceId; private final IamClient iamClient; private final String registrarId; @@ -71,12 +72,14 @@ public ConsoleOteAction( IamClient iamClient, @Parameter("registrarId") String registrarId, // Get request param @Config("gSuiteConsoleUserGroupEmailAddress") Optional maybeGroupEmailAddress, + @Config("consoleIapServiceId") Optional consoleIapServiceId, @Named("base58StringGenerator") StringGenerator passwordGenerator, @Parameter("oteCreateData") Optional oteCreateData) { super(consoleApiParams); this.passwordGenerator = passwordGenerator; this.oteCreateData = oteCreateData; this.maybeGroupEmailAddress = maybeGroupEmailAddress; + this.consoleIapServiceId = consoleIapServiceId; this.iamClient = iamClient; this.registrarId = registrarId; } @@ -105,7 +108,8 @@ protected void postHandler(User user) { .setPassword(password); ImmutableMap registrarIdToTld = oteAccountBuilder.buildAndPersist(); - oteAccountBuilder.grantIapPermission(maybeGroupEmailAddress, cloudTasksUtils, iamClient); + oteAccountBuilder.grantIapPermission( + maybeGroupEmailAddress, consoleIapServiceId, cloudTasksUtils, iamClient); consoleApiParams.response().setStatus(SC_OK); consoleApiParams .response() diff --git a/core/src/main/java/google/registry/ui/server/console/ConsoleUsersAction.java b/core/src/main/java/google/registry/ui/server/console/ConsoleUsersAction.java index 7b43dd2e678..9b47b0261dd 100644 --- a/core/src/main/java/google/registry/ui/server/console/ConsoleUsersAction.java +++ b/core/src/main/java/google/registry/ui/server/console/ConsoleUsersAction.java @@ -79,6 +79,7 @@ public class ConsoleUsersAction extends ConsoleApiAction { private final StringGenerator passwordGenerator; private final Optional userData; private final Optional maybeGroupEmailAddress; + private final Optional consoleIapServiceId; private final IamClient iamClient; private final String gSuiteDomainName; @@ -89,6 +90,7 @@ public ConsoleUsersAction( IamClient iamClient, @Config("gSuiteDomainName") String gSuiteDomainName, @Config("gSuiteConsoleUserGroupEmailAddress") Optional maybeGroupEmailAddress, + @Config("consoleIapServiceId") Optional consoleIapServiceId, @Named("base58StringGenerator") StringGenerator passwordGenerator, @Parameter("userData") Optional userData, @Parameter("registrarId") String registrarId) { @@ -98,6 +100,7 @@ public ConsoleUsersAction( this.passwordGenerator = passwordGenerator; this.userData = userData; this.maybeGroupEmailAddress = maybeGroupEmailAddress; + this.consoleIapServiceId = consoleIapServiceId; this.iamClient = iamClient; this.gSuiteDomainName = gSuiteDomainName; } @@ -256,7 +259,8 @@ private void runCreate() throws IOException { User.Builder builder = new User.Builder().setUserRoles(userRoles).setEmailAddress(newEmail); tm().put(builder.build()); - User.grantIapPermission(newEmail, maybeGroupEmailAddress, cloudTasksUtils, null, iamClient); + User.grantIapPermission( + newEmail, maybeGroupEmailAddress, consoleIapServiceId, cloudTasksUtils, null, iamClient); sendConfirmationEmail(registrarId, newEmail, "Created user"); consoleApiParams.response().setStatus(SC_CREATED); consoleApiParams diff --git a/core/src/test/java/google/registry/model/OteAccountBuilderTest.java b/core/src/test/java/google/registry/model/OteAccountBuilderTest.java index 8affd6073ea..9ee026cdebc 100644 --- a/core/src/test/java/google/registry/model/OteAccountBuilderTest.java +++ b/core/src/test/java/google/registry/model/OteAccountBuilderTest.java @@ -116,6 +116,7 @@ public static void verifyUser(String registrarId, String email) { public static void verifyIapPermission( @Nullable String emailAddress, Optional maybeGroupEmailAddress, + Optional consoleIapServiceId, CloudTasksHelper cloudTasksHelper, IamClient iamClient) { if (emailAddress == null) { @@ -125,7 +126,8 @@ public static void verifyIapPermission( String groupEmailAddress = maybeGroupEmailAddress.orElse(null); if (groupEmailAddress == null) { cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); - verify(iamClient).addBinding(emailAddress, IAP_SECURED_WEB_APP_USER_ROLE); + verify(iamClient) + .addBinding(emailAddress, IAP_SECURED_WEB_APP_USER_ROLE, consoleIapServiceId.get()); } else { cloudTasksHelper.assertTasksEnqueued( "console-user-group-update", @@ -146,9 +148,32 @@ void testUpdateUserGroup() { CloudTasksUtils cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); OteAccountBuilder.forRegistrarId("myclientid") .addUser("email@example.com") - .grantIapPermission(Optional.of("console@example.com"), cloudTasksUtils, iamClient); + .grantIapPermission( + Optional.of("console@example.com"), + Optional.of("consoleIapServiceId"), + cloudTasksUtils, + iamClient); verifyIapPermission( - "email@example.com", Optional.of("console@example.com"), cloudTasksHelper, iamClient); + "email@example.com", + Optional.of("console@example.com"), + Optional.of("consoleIapServiceId"), + cloudTasksHelper, + iamClient); + } + + @Test + void testUpdateUserGroup_worksWithoutIapServiceId() { + CloudTasksUtils cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); + OteAccountBuilder.forRegistrarId("myclientid") + .addUser("email@example.com") + .grantIapPermission( + Optional.of("console@example.com"), Optional.empty(), cloudTasksUtils, iamClient); + verifyIapPermission( + "email@example.com", + Optional.of("console@example.com"), + Optional.empty(), + cloudTasksHelper, + iamClient); } @Test @@ -156,8 +181,14 @@ void testGrantIndividualPermission() { CloudTasksUtils cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); OteAccountBuilder.forRegistrarId("myclientid") .addUser("email@example.com") - .grantIapPermission(Optional.empty(), cloudTasksUtils, iamClient); - verifyIapPermission("email@example.com", Optional.empty(), cloudTasksHelper, iamClient); + .grantIapPermission( + Optional.empty(), Optional.of("consoleIapServiceId"), cloudTasksUtils, iamClient); + verifyIapPermission( + "email@example.com", + Optional.empty(), + Optional.of("consoleIapServiceId"), + cloudTasksHelper, + iamClient); } @Test diff --git a/core/src/test/java/google/registry/model/console/UserTest.java b/core/src/test/java/google/registry/model/console/UserTest.java index 21d093b2d1e..a67cc9e9f6f 100644 --- a/core/src/test/java/google/registry/model/console/UserTest.java +++ b/core/src/test/java/google/registry/model/console/UserTest.java @@ -66,13 +66,21 @@ void testPersistence_lookupByEmail() { void testFailure_asyncAndSyncModeConflict() { assertThrows( IllegalArgumentException.class, - () -> User.grantIapPermission("email@example.com", Optional.empty(), null, null, null)); + () -> + User.grantIapPermission( + "email@example.com", + Optional.empty(), + Optional.of("consoleIapServiceId"), + null, + null, + null)); assertThrows( IllegalArgumentException.class, () -> User.grantIapPermission( "email@example.com", Optional.empty(), + Optional.of("consoleIapServiceId"), mock(CloudTasksUtils.class), mock(ServiceConnection.class), null)); @@ -90,6 +98,24 @@ void testFailure_asyncAndSyncModeConflict() { null)); } + @Test + void testGrantIapPermission_neitherGroupNorIapServiceId_fails() { + IllegalArgumentException e = + assertThrows( + IllegalArgumentException.class, + () -> + User.grantIapPermission( + "email@example.com", + Optional.empty(), + Optional.empty(), + mock(CloudTasksUtils.class), + null, + mock(IamClient.class))); + assertThat(e) + .hasMessageThat() + .isEqualTo("At least one of groupEmailAddress or consoleIapServiceId must be present"); + } + @Test void testFailure_badInputs() { User.Builder builder = new User.Builder(); @@ -156,13 +182,33 @@ void testGrantIapPermissionAsync() { // Individual permission. User.grantIapPermission( - "email@example.com", Optional.empty(), cloudTasksUtils, null, iamClient); + "email@example.com", + Optional.empty(), + Optional.of("consoleIapServiceId"), + cloudTasksUtils, + null, + iamClient); cloudTasksHelper.assertNoTasksEnqueued(); - verify(iamClient).addBinding("email@example.com", IAP_SECURED_WEB_APP_USER_ROLE); + verify(iamClient) + .addBinding("email@example.com", IAP_SECURED_WEB_APP_USER_ROLE, "consoleIapServiceId"); // Group membership. User.grantIapPermission( - "email@example.com", Optional.of("console@example.com"), cloudTasksUtils, null, iamClient); + "email@example.com", + Optional.of("console@example.com"), + Optional.empty(), + cloudTasksUtils, + null, + iamClient); + // Group membership takes precedence over individual membership + User.grantIapPermission( + "groupemail@example.com", + Optional.of("console@example.com"), + Optional.of("consoleIapServiceId"), + cloudTasksUtils, + null, + iamClient); + cloudTasksHelper.assertTasksEnqueued( "console-user-group-update", new TaskMatcher() @@ -171,6 +217,13 @@ void testGrantIapPermissionAsync() { .path("/_dr/admin/updateUserGroup") .param("userEmailAddress", "email@example.com") .param("groupEmailAddress", "console@example.com") + .param("groupUpdateMode", "ADD"), + new TaskMatcher() + .service("BACKEND") + .method(HttpMethod.POST) + .path("/_dr/admin/updateUserGroup") + .param("userEmailAddress", "groupemail@example.com") + .param("groupEmailAddress", "console@example.com") .param("groupUpdateMode", "ADD")); verifyNoMoreInteractions(iamClient); } @@ -181,13 +234,25 @@ void testGrantIapPermissionSync() throws Exception { IamClient iamClient = mock(IamClient.class); // Individual permission. - User.grantIapPermission("email@example.com", Optional.empty(), null, connection, iamClient); + User.grantIapPermission( + "email@example.com", + Optional.empty(), + Optional.of("consoleIapServiceId"), + null, + connection, + iamClient); verifyNoInteractions(connection); - verify(iamClient).addBinding("email@example.com", IAP_SECURED_WEB_APP_USER_ROLE); + verify(iamClient) + .addBinding("email@example.com", IAP_SECURED_WEB_APP_USER_ROLE, "consoleIapServiceId"); // Group membership. User.grantIapPermission( - "email@example.com", Optional.of("console@example.com"), null, connection, iamClient); + "email@example.com", + Optional.of("console@example.com"), + Optional.empty(), + null, + connection, + iamClient); verify(connection) .sendPostRequest( UpdateUserGroupAction.PATH, diff --git a/core/src/test/java/google/registry/tools/CreateUserCommandTest.java b/core/src/test/java/google/registry/tools/CreateUserCommandTest.java index c281e3909c3..eaef1ef35b9 100644 --- a/core/src/test/java/google/registry/tools/CreateUserCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateUserCommandTest.java @@ -45,6 +45,7 @@ public class CreateUserCommandTest extends CommandTestCase { void beforeEach() { command.iamClient = iamClient; command.maybeGroupEmailAddress = Optional.empty(); + command.consoleIapServiceId = Optional.of("consoleIapServiceId"); command.setConnection(connection); } @@ -56,7 +57,8 @@ void testSuccess() throws Exception { assertThat(onlyUser.getUserRoles().isAdmin()).isFalse(); assertThat(onlyUser.getUserRoles().getGlobalRole()).isEqualTo(GlobalRole.NONE); assertThat(onlyUser.getUserRoles().getRegistrarRoles()).isEmpty(); - verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE); + verify(iamClient) + .addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE, "consoleIapServiceId"); verifyNoMoreInteractions(iamClient); verifyNoInteractions(connection); } @@ -86,6 +88,41 @@ void testSuccess_addToGroup() throws Exception { verifyNoMoreInteractions(connection); } + @Test + void testSuccess_addToGroup_noIapServiceId() throws Exception { + command.maybeGroupEmailAddress = Optional.of("group@example.test"); + command.consoleIapServiceId = Optional.empty(); + runCommandForced("--email", "user@example.test"); + User onlyUser = Iterables.getOnlyElement(DatabaseHelper.loadAllOf(User.class)); + assertThat(onlyUser.getEmailAddress()).isEqualTo("user@example.test"); + verify(connection) + .sendPostRequest( + UpdateUserGroupAction.PATH, + ImmutableMap.of( + "userEmailAddress", + "user@example.test", + "groupEmailAddress", + "group@example.test", + "groupUpdateMode", + "ADD"), + MediaType.PLAIN_TEXT_UTF_8, + new byte[0]); + verifyNoInteractions(iamClient); + verifyNoMoreInteractions(connection); + } + + @Test + void testFailure_neitherGroupNorIapServiceId() { + command.maybeGroupEmailAddress = Optional.empty(); + command.consoleIapServiceId = Optional.empty(); + IllegalArgumentException e = + assertThrows( + IllegalArgumentException.class, () -> runCommandForced("--email", "user@example.test")); + assertThat(e) + .hasMessageThat() + .isEqualTo("At least one of groupEmailAddress or consoleIapServiceId must be present"); + } + @Test void testSuccess_registryLock() throws Exception { runCommandForced( @@ -107,7 +144,8 @@ void testSuccess_registryLock() throws Exception { void testSuccess_admin() throws Exception { runCommandForced("--email", "user@example.test", "--admin", "true"); assertThat(loadExistingUser("user@example.test").getUserRoles().isAdmin()).isTrue(); - verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE); + verify(iamClient) + .addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE, "consoleIapServiceId"); verifyNoMoreInteractions(iamClient); verifyNoInteractions(connection); } @@ -117,7 +155,8 @@ void testSuccess_globalRole() throws Exception { runCommandForced("--email", "user@example.test", "--global_role", "FTE"); assertThat(loadExistingUser("user@example.test").getUserRoles().getGlobalRole()) .isEqualTo(GlobalRole.FTE); - verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE); + verify(iamClient) + .addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE, "consoleIapServiceId"); verifyNoMoreInteractions(iamClient); verifyNoInteractions(connection); } @@ -136,7 +175,8 @@ void testSuccess_registrarRoles() throws Exception { RegistrarRole.ACCOUNT_MANAGER, "NewRegistrar", RegistrarRole.PRIMARY_CONTACT)); - verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE); + verify(iamClient) + .addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE, "consoleIapServiceId"); verifyNoMoreInteractions(iamClient); verifyNoInteractions(connection); } @@ -144,7 +184,8 @@ void testSuccess_registrarRoles() throws Exception { @Test void testFailure_alreadyExists() throws Exception { runCommandForced("--email", "user@example.test"); - verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE); + verify(iamClient) + .addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE, "consoleIapServiceId"); verifyNoMoreInteractions(iamClient); assertThat( assertThrows( diff --git a/core/src/test/java/google/registry/tools/IamClientTest.java b/core/src/test/java/google/registry/tools/IamClientTest.java index 2e91f4dc758..733b7f91ee8 100644 --- a/core/src/test/java/google/registry/tools/IamClientTest.java +++ b/core/src/test/java/google/registry/tools/IamClientTest.java @@ -38,6 +38,7 @@ /** Unit tests for {@link IamClient}. */ public class IamClientTest { private final CloudResourceManager resourceManager = mock(CloudResourceManager.class); + private final String consoleIapServiceId = "123456789"; private final String projectId = "my-project"; private final String account = "test@example.test"; private final String role = "roles/fakeRole"; @@ -89,11 +90,17 @@ void testSuccess_findUser() { void testSuccess_addBinding_noMatchedBindingExists() throws Exception { setupRequests(); assertThat(bindings.size()).isEqualTo(1); - client.addBinding(account, role); + client.addBinding(account, role, consoleIapServiceId); assertThat(bindings.size()).isEqualTo(2); Binding binding = bindings.get(1); assertThat(binding.getRole()).isEqualTo(role); assertThat(binding.getMembers()).containsExactly("user:" + account); + assertThat(binding.getCondition()).isNotNull(); + assertThat(binding.getCondition().getTitle()).isEqualTo("Registrar Console IAP access"); + assertThat(binding.getCondition().getDescription()) + .isEqualTo("Restrict IAP access only to the Registrar Console HTTP(s) load balancer"); + assertThat(binding.getCondition().getExpression()) + .isEqualTo("resource.name == 'projects/my-project/iap_web/compute/services/123456789'"); verifySetPolicyRequest(); } @@ -107,7 +114,7 @@ void testSuccess_addBinding_matchBindingExists() throws Exception { when(matchedBinding.getMembers()).thenReturn(existingMembers); bindings.add(matchedBinding); assertThat(bindings.size()).isEqualTo(2); - client.addBinding(account, role); + client.addBinding(account, role, consoleIapServiceId); assertThat(bindings.size()).isEqualTo(2); assertThat(existingMembers) .containsExactly("serviceAccount:service@example.test", "user:" + account); @@ -125,7 +132,7 @@ void testSuccess_addBinding_matchedBindingExists_memberExists() throws Exception when(matchedBinding.getMembers()).thenReturn(existingMembers); bindings.add(matchedBinding); assertThat(bindings.size()).isEqualTo(2); - client.addBinding(account, role); + client.addBinding(account, role, consoleIapServiceId); assertThat(bindings.size()).isEqualTo(2); assertThat(existingMembers) .containsExactly("serviceAccount:service@example.test", "user:" + account); diff --git a/core/src/test/java/google/registry/tools/SetupOteCommandTest.java b/core/src/test/java/google/registry/tools/SetupOteCommandTest.java index f7984bf4564..b65bdeb35a6 100644 --- a/core/src/test/java/google/registry/tools/SetupOteCommandTest.java +++ b/core/src/test/java/google/registry/tools/SetupOteCommandTest.java @@ -73,6 +73,7 @@ void beforeEach() { command.clock = fakeClock; command.passwordGenerator = passwordGenerator; command.maybeGroupEmailAddress = Optional.of("group@example.com"); + command.consoleIapServiceId = Optional.of("consoleIapServiceId"); command.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); command.iamClient = iamClient; persistPremiumList("default_sandbox_list", USD, "sandbox,USD 1000"); @@ -129,7 +130,9 @@ private void verifyIapPermission(@Nullable String emailAddress) { String groupEmailAddress = command.maybeGroupEmailAddress.orElse(null); if (groupEmailAddress == null) { cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); - verify(iamClient).addBinding(emailAddress, IAP_SECURED_WEB_APP_USER_ROLE); + verify(iamClient) + .addBinding( + emailAddress, IAP_SECURED_WEB_APP_USER_ROLE, command.consoleIapServiceId.get()); } else { cloudTasksHelper.assertTasksEnqueued( "console-user-group-update", diff --git a/core/src/test/java/google/registry/ui/server/console/ConsoleOteActionTest.java b/core/src/test/java/google/registry/ui/server/console/ConsoleOteActionTest.java index 95a17856d78..b3095f9552e 100644 --- a/core/src/test/java/google/registry/ui/server/console/ConsoleOteActionTest.java +++ b/core/src/test/java/google/registry/ui/server/console/ConsoleOteActionTest.java @@ -144,6 +144,7 @@ void testSuccess_oteCreated() { verifyIapPermission( "contact@registry.example", Optional.of("someRandomString@email.test"), + Optional.of("consoleIapServiceId"), cloudTasksHelper, iamClient); } @@ -216,6 +217,7 @@ private ConsoleOteAction createAction( iamClient, registrarId, maybeGroupEmailAddress, + Optional.of("consoleIapServiceId"), passwordGenerator, oteCreateData); } diff --git a/core/src/test/java/google/registry/ui/server/console/ConsoleUsersActionTest.java b/core/src/test/java/google/registry/ui/server/console/ConsoleUsersActionTest.java index f96042bf7dc..61e8deed5cc 100644 --- a/core/src/test/java/google/registry/ui/server/console/ConsoleUsersActionTest.java +++ b/core/src/test/java/google/registry/ui/server/console/ConsoleUsersActionTest.java @@ -663,6 +663,7 @@ private ConsoleUsersAction createAction( iamClient, "email.com", Optional.of("someRandomString"), + Optional.of("consoleIapServiceId"), passwordGenerator, userData, "TheRegistrar");