From d3a1d0709ea6868ca0a7f6a7ccdce2e229b744e1 Mon Sep 17 00:00:00 2001 From: gbrodman Date: Wed, 22 Jul 2026 16:53:41 +0000 Subject: [PATCH] Only grant IAP access to the console for new users (#3167) 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 3f7683834..793b341a4 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 final class RegistryConfig { 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 e4b7b9398..d6bda8f0e 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -187,6 +187,7 @@ public class RegistryConfigSettings { /** 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 8cfdcf20e..322d6ca87 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 69b943231..9bbc33aa6 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 final class OteAccountBuilder { registrars.stream() .collect( toImmutableMap( - Registrar::getRegistrarId, - registrar -> RegistrarRole.ACCOUNT_MANAGER))) + Registrar::getRegistrarId, _ -> RegistrarRole.ACCOUNT_MANAGER))) .build()) .build()); return this; @@ -265,10 +264,18 @@ public final class OteAccountBuilder { /** 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 7ce960ab8..612906500 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 class User extends UpdateAutoTimestampEntity implements Buildable { 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 class User extends UpdateAutoTimestampEntity implements Buildable { 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 9caa725e9..8ffd20dbb 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 @@ public class CreateUserCommand extends CreateOrUpdateUserCommand implements Comm @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 e83232899..cbbfc8554 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 @@ package google.registry.tools; 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 @@ import java.util.Optional; @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 @@ public class IamClient { * *

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 class IamClient { .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 13584c23c..5ece9b485 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 @@ you sure you didn't mean to run this against sandbox (e.g. "-e SANDBOX")?\ @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 2465a6a91..905297170 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 class ConsoleOteAction extends ConsoleApiAction { 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 @@ public class ConsoleOteAction extends ConsoleApiAction { .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 7b43dd2e6..9b47b0261 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 class ConsoleUsersAction extends ConsoleApiAction { 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 class ConsoleUsersAction extends ConsoleApiAction { this.passwordGenerator = passwordGenerator; this.userData = userData; this.maybeGroupEmailAddress = maybeGroupEmailAddress; + this.consoleIapServiceId = consoleIapServiceId; this.iamClient = iamClient; this.gSuiteDomainName = gSuiteDomainName; } @@ -256,7 +259,8 @@ public class ConsoleUsersAction extends ConsoleApiAction { 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 8affd6073..9ee026cde 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 final class OteAccountBuilderTest { public static void verifyIapPermission( @Nullable String emailAddress, Optional maybeGroupEmailAddress, + Optional consoleIapServiceId, CloudTasksHelper cloudTasksHelper, IamClient iamClient) { if (emailAddress == null) { @@ -125,7 +126,8 @@ public final class OteAccountBuilderTest { 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 @@ public final class OteAccountBuilderTest { 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 @@ public final class OteAccountBuilderTest { 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 21d093b2d..a67cc9e9f 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 @@ public class UserTest extends EntityTestCase { 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 @@ public class UserTest extends EntityTestCase { 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 @@ public class UserTest extends EntityTestCase { // 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 @@ public class UserTest extends EntityTestCase { .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 @@ public class UserTest extends EntityTestCase { 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 c281e3909..eaef1ef35 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 @@ public class CreateUserCommandTest extends CommandTestCase { 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 @@ public class CreateUserCommandTest extends CommandTestCase { 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 @@ public class CreateUserCommandTest extends CommandTestCase { 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 @@ public class CreateUserCommandTest extends CommandTestCase { 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 @@ public class CreateUserCommandTest extends CommandTestCase { 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 @@ public class CreateUserCommandTest extends CommandTestCase { @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 2e91f4dc7..733b7f91e 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 @@ import org.mockito.ArgumentCaptor; /** 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 @@ public class IamClientTest { 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 @@ public class IamClientTest { 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 @@ public class IamClientTest { 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 f7984bf45..b65bdeb35 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 @@ class SetupOteCommandTest extends CommandTestCase { 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 @@ class SetupOteCommandTest extends CommandTestCase { 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 95a17856d..b3095f955 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 @@ class ConsoleOteActionTest extends ConsoleActionBaseTestCase { verifyIapPermission( "contact@registry.example", Optional.of("someRandomString@email.test"), + Optional.of("consoleIapServiceId"), cloudTasksHelper, iamClient); } @@ -216,6 +217,7 @@ class ConsoleOteActionTest extends ConsoleActionBaseTestCase { 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 f96042bf7..61e8deed5 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 @@ class ConsoleUsersActionTest extends ConsoleActionBaseTestCase { iamClient, "email.com", Optional.of("someRandomString"), + Optional.of("consoleIapServiceId"), passwordGenerator, userData, "TheRegistrar");