From c84034b779ac6b78157b92a3a16438eeff2588b6 Mon Sep 17 00:00:00 2001 From: Gus Brodman Date: Tue, 23 Jun 2026 12:52:28 -0400 Subject: [PATCH] Don't allow registrar-based ops on global users --- .../ui/server/console/ConsoleUsersAction.java | 26 +++-- .../console/ConsoleUsersActionTest.java | 97 +++++++++++++++++++ 2 files changed, 116 insertions(+), 7 deletions(-) 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 7b0c58c7d72..3a27e9f4ac4 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 @@ -37,6 +37,7 @@ import google.registry.config.RegistryConfig.Config; import google.registry.model.console.ConsolePermission; import google.registry.model.console.ConsoleUpdateHistory; +import google.registry.model.console.GlobalRole; import google.registry.model.console.RegistrarRole; import google.registry.model.console.User; import google.registry.model.console.UserRoles; @@ -149,10 +150,15 @@ private void runAppendUserToExistingRegistrar() { throw new BadRequestException("Total users amount per registrar is limited to 4"); } + User userToAppend = verifyUserExists(this.userData.get().emailAddress); + if (userToAppend.getUserRoles().isAdmin() + || !userToAppend.getUserRoles().getGlobalRole().equals(GlobalRole.NONE)) { + throw new BadRequestException( + "Cannot append a global administrator or user with a global role to a registrar"); + } + updateUserRegistrarRoles( - this.userData.get().emailAddress, - registrarId, - requestRoleToAllowedRoles(this.userData.get().role)); + userToAppend, registrarId, requestRoleToAllowedRoles(this.userData.get().role)); sendConfirmationEmail(registrarId, this.userData.get().emailAddress, "Added existing user"); consoleApiParams.response().setStatus(SC_OK); @@ -164,7 +170,14 @@ private void runDeleteInTransaction() throws IOException { } String email = this.userData.get().emailAddress; - User updatedUser = updateUserRegistrarRoles(email, registrarId, null); + User userToDelete = verifyUserExists(email); + if (userToDelete.getUserRoles().isAdmin() + || !userToDelete.getUserRoles().getGlobalRole().equals(GlobalRole.NONE)) { + throw new BadRequestException( + "Cannot delete a global administrator or user with a global role"); + } + + User updatedUser = updateUserRegistrarRoles(userToDelete, registrarId, null); // User has no registrars assigned if (updatedUser.getUserRoles().getRegistrarRoles().isEmpty()) { @@ -251,7 +264,7 @@ private void runUpdateInTransaction() { } updateUserRegistrarRoles( - this.userData.get().emailAddress, + verifyUserExists(this.userData.get().emailAddress), registrarId, requestRoleToAllowedRoles(this.userData.get().role)); @@ -297,9 +310,8 @@ private boolean validateUserRegistrarAssociation(User user) { return false; } - private User updateUserRegistrarRoles(String email, String registrarId, RegistrarRole newRole) { + private User updateUserRegistrarRoles(User user, String registrarId, RegistrarRole newRole) { Map updatedRegistrarRoles; - User user = verifyUserExists(email); if (newRole == null) { updatedRegistrarRoles = user.getUserRoles().getRegistrarRoles().entrySet().stream() 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 fc163415465..bd874cb6ea0 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 @@ -407,6 +407,103 @@ void testFailure_noPermissionToUpdateUser() throws IOException { .contains("Can't update user not associated with registrarId TheRegistrar"); } + @Test + void testSuccess_appendUser() throws IOException { + User user = DatabaseHelper.createAdminUser("email@email.com"); + AuthResult authResult = AuthResult.createUser(user); + ConsoleUsersAction action = + createAction( + Optional.of(ConsoleApiParamsUtils.createFake(authResult)), + Optional.of("POST"), + Optional.of( + new UserData("test3@test.com", null, RegistrarRole.TECH_CONTACT.name(), null))); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_OK); + User appendedUser = DatabaseHelper.loadByKey(VKey.create(User.class, "test3@test.com")); + assertThat(appendedUser.getUserRoles().getRegistrarRoles().get("TheRegistrar")) + .isEqualTo(RegistrarRole.TECH_CONTACT); + } + + @Test + void testFailure_appendUser_globalAdmin() throws IOException { + User user = DatabaseHelper.createAdminUser("email@email.com"); + AuthResult authResult = AuthResult.createUser(user); + DatabaseHelper.persistResource( + new User.Builder() + .setEmailAddress("globaladmin@test.com") + .setUserRoles( + new UserRoles.Builder().setIsAdmin(true).setGlobalRole(GlobalRole.NONE).build()) + .build()); + + ConsoleUsersAction action = + createAction( + Optional.of(ConsoleApiParamsUtils.createFake(authResult)), + Optional.of("POST"), + Optional.of( + new UserData( + "globaladmin@test.com", null, RegistrarRole.TECH_CONTACT.name(), null))); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_BAD_REQUEST); + assertThat(response.getPayload()) + .contains("Cannot append a global administrator or user with a global role"); + } + + @Test + void testFailure_appendUser_globalRole() throws IOException { + User user = DatabaseHelper.createAdminUser("email@email.com"); + AuthResult authResult = AuthResult.createUser(user); + DatabaseHelper.persistResource( + new User.Builder() + .setEmailAddress("support@test.com") + .setUserRoles( + new UserRoles.Builder() + .setIsAdmin(false) + .setGlobalRole(GlobalRole.SUPPORT_AGENT) + .build()) + .build()); + + ConsoleUsersAction action = + createAction( + Optional.of(ConsoleApiParamsUtils.createFake(authResult)), + Optional.of("POST"), + Optional.of( + new UserData("support@test.com", null, RegistrarRole.TECH_CONTACT.name(), null))); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_BAD_REQUEST); + assertThat(response.getPayload()) + .contains("Cannot append a global administrator or user with a global role"); + } + + @Test + void testFailure_deleteUser_globalAdmin() throws IOException { + User user = DatabaseHelper.createAdminUser("email@email.com"); + AuthResult authResult = AuthResult.createUser(user); + // Historically associated global admin + DatabaseHelper.persistResource( + new User.Builder() + .setEmailAddress("globaladmin@test.com") + .setUserRoles( + new UserRoles.Builder() + .setIsAdmin(true) + .setGlobalRole(GlobalRole.NONE) + .setRegistrarRoles( + ImmutableMap.of("TheRegistrar", RegistrarRole.PRIMARY_CONTACT)) + .build()) + .build()); + + ConsoleUsersAction action = + createAction( + Optional.of(ConsoleApiParamsUtils.createFake(authResult)), + Optional.of("DELETE"), + Optional.of( + new UserData( + "globaladmin@test.com", null, RegistrarRole.ACCOUNT_MANAGER.toString(), null))); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_BAD_REQUEST); + assertThat(response.getPayload()) + .contains("Cannot delete a global administrator or user with a global role"); + } + private ConsoleUsersAction createAction( Optional maybeConsoleApiParams, Optional method,