From 6d66f52359de757abc8c27923a32f8ecd918a22f Mon Sep 17 00:00:00 2001 From: gbrodman Date: Fri, 12 Aug 2022 12:18:09 -0400 Subject: [PATCH] Create a registry lock permission and corresponding account manager role (#1740) * Create a registry lock permission and corresponding account manager role This allows us to distinguish between standard account managers and users that might have the registry lock permission. This will make the registry lock password-setting flow easier (user can reset their password iff they have the REGISTRY_LOCK permission, instead of having a separate boolean) and allows us to easily determine whether or not a user should have access to registry lock views in the UI. --- .../model/console/ConsolePermission.java | 4 +- .../model/console/ConsoleRoleDefinitions.java | 12 ++++- .../registry/model/console/RegistrarRole.java | 3 ++ .../google/registry/model/console/User.java | 48 +++++++++++-------- .../console/ConsoleRoleDefinitionsTest.java | 25 ++++++---- .../registry/model/console/UserTest.java | 29 +++++++++++ 6 files changed, 90 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/google/registry/model/console/ConsolePermission.java b/core/src/main/java/google/registry/model/console/ConsolePermission.java index edcf01edb..0c1f43ace 100644 --- a/core/src/main/java/google/registry/model/console/ConsolePermission.java +++ b/core/src/main/java/google/registry/model/console/ConsolePermission.java @@ -63,5 +63,7 @@ public enum ConsolePermission { /** View announcements in the UI. */ VIEW_ANNOUNCEMENTS, /** Viewing a record of actions performed in the UI for a particular registrar. */ - VIEW_ACTIVITY_LOG + VIEW_ACTIVITY_LOG, + /** View and perform registry locks. */ + REGISTRY_LOCK } diff --git a/core/src/main/java/google/registry/model/console/ConsoleRoleDefinitions.java b/core/src/main/java/google/registry/model/console/ConsoleRoleDefinitions.java index fbf5db433..7d2352a82 100644 --- a/core/src/main/java/google/registry/model/console/ConsoleRoleDefinitions.java +++ b/core/src/main/java/google/registry/model/console/ConsoleRoleDefinitions.java @@ -44,7 +44,8 @@ public class ConsoleRoleDefinitions { ConsolePermission.VIEW_OPERATIONAL_DATA, ConsolePermission.SEND_ANNOUNCEMENTS, ConsolePermission.VIEW_ANNOUNCEMENTS, - ConsolePermission.VIEW_ACTIVITY_LOG); + ConsolePermission.VIEW_ACTIVITY_LOG, + ConsolePermission.REGISTRY_LOCK); /** Permissions for a registry support lead. */ static final ImmutableSet SUPPORT_LEAD_PERMISSIONS = @@ -76,10 +77,17 @@ public class ConsoleRoleDefinitions { ConsolePermission.VIEW_OPERATIONAL_DATA, ConsolePermission.VIEW_ANNOUNCEMENTS); + /** Permissions for a registrar partner account manager that can perform registry locks. */ + static final ImmutableSet ACCOUNT_MANAGER_WITH_REGISTRY_LOCK_PERMISSIONS = + new ImmutableSet.Builder() + .addAll(ACCOUNT_MANAGER_PERMISSIONS) + .add(ConsolePermission.REGISTRY_LOCK) + .build(); + /** Permissions for the tech contact of a registrar. */ static final ImmutableSet TECH_CONTACT_PERMISSIONS = new ImmutableSet.Builder() - .addAll(ACCOUNT_MANAGER_PERMISSIONS) + .addAll(ACCOUNT_MANAGER_WITH_REGISTRY_LOCK_PERMISSIONS) .add( ConsolePermission.MANAGE_ACCREDITATION, ConsolePermission.CONFIGURE_EPP_CONNECTION, diff --git a/core/src/main/java/google/registry/model/console/RegistrarRole.java b/core/src/main/java/google/registry/model/console/RegistrarRole.java index 503f0ca29..fb858a005 100644 --- a/core/src/main/java/google/registry/model/console/RegistrarRole.java +++ b/core/src/main/java/google/registry/model/console/RegistrarRole.java @@ -15,6 +15,7 @@ package google.registry.model.console; import static google.registry.model.console.ConsoleRoleDefinitions.ACCOUNT_MANAGER_PERMISSIONS; +import static google.registry.model.console.ConsoleRoleDefinitions.ACCOUNT_MANAGER_WITH_REGISTRY_LOCK_PERMISSIONS; import static google.registry.model.console.ConsoleRoleDefinitions.PRIMARY_CONTACT_PERMISSIONS; import static google.registry.model.console.ConsoleRoleDefinitions.TECH_CONTACT_PERMISSIONS; @@ -25,6 +26,8 @@ public enum RegistrarRole { /** The user is a standard account manager at a registrar. */ ACCOUNT_MANAGER(ACCOUNT_MANAGER_PERMISSIONS), + /** The user is an account manager that can perform registry locks. */ + ACCOUNT_MANAGER_WITH_REGISTRY_LOCK(ACCOUNT_MANAGER_WITH_REGISTRY_LOCK_PERMISSIONS), /** The user is a technical contact of a registrar. */ TECH_CONTACT(TECH_CONTACT_PERMISSIONS), /** The user is the primary contact at a registrar. */ 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 b47a4fa31..5550307d6 100644 --- a/core/src/main/java/google/registry/model/console/User.java +++ b/core/src/main/java/google/registry/model/console/User.java @@ -40,12 +40,6 @@ public class User extends ImmutableObject implements Buildable { /** Roles (which grant permissions) associated with this user. */ private UserRoles userRoles; - /** - * Whether the contact is allowed to set their registry lock password through the registrar - * console. This will be set to false on contact creation and when the user sets a password. - */ - boolean allowedToSetRegistryLockPassword = false; - /** * A hashed password that exists iff this contact is registry-lock-enabled. The hash is a base64 * encoded SHA256 string. @@ -55,6 +49,10 @@ public class User extends ImmutableObject implements Buildable { /** Randomly generated hash salt. */ String registryLockPasswordSalt; + public long getId() { + return id; + } + public String getGaiaId() { return gaiaId; } @@ -67,11 +65,7 @@ public class User extends ImmutableObject implements Buildable { return userRoles; } - public boolean isAllowedToSetRegistryLockPassword() { - return allowedToSetRegistryLockPassword; - } - - public boolean isRegistryLockAllowed() { + public boolean hasRegistryLockPassword() { return !isNullOrEmpty(registryLockPasswordHash) && !isNullOrEmpty(registryLockPasswordSalt); } @@ -85,6 +79,22 @@ public class User extends ImmutableObject implements Buildable { .equals(registryLockPasswordHash); } + /** + * Whether the user has the registry lock permission on any registrar or globally. + * + *

If so, they should be allowed to (re)set their registry lock password. + */ + public boolean hasAnyRegistryLockPermission() { + if (userRoles == null) { + return false; + } + if (userRoles.isAdmin() || userRoles.hasGlobalPermission(ConsolePermission.REGISTRY_LOCK)) { + return true; + } + return userRoles.getRegistrarRoles().values().stream() + .anyMatch(role -> role.hasPermission(ConsolePermission.REGISTRY_LOCK)); + } + @Override public Builder asBuilder() { return new Builder(clone(this)); @@ -99,6 +109,7 @@ public class User extends ImmutableObject implements Buildable { super(user); } + @Override public User build() { checkArgumentNotNull(getInstance().gaiaId, "Gaia ID cannot be null"); checkArgumentNotNull(getInstance().emailAddress, "Email address cannot be null"); @@ -123,25 +134,22 @@ public class User extends ImmutableObject implements Buildable { return this; } - public Builder setAllowedToSetRegistryLockPassword(boolean allowedToSetRegistryLockPassword) { - if (allowedToSetRegistryLockPassword) { - getInstance().registryLockPasswordSalt = null; - getInstance().registryLockPasswordHash = null; - } - getInstance().allowedToSetRegistryLockPassword = allowedToSetRegistryLockPassword; + public Builder removeRegistryLockPassword() { + getInstance().registryLockPasswordHash = null; + getInstance().registryLockPasswordSalt = null; return this; } public Builder setRegistryLockPassword(String registryLockPassword) { checkArgument( - getInstance().allowedToSetRegistryLockPassword, - "Not allowed to set registry lock password for this user"); + getInstance().hasAnyRegistryLockPermission(), "User has no registry lock permission"); + checkArgument( + !getInstance().hasRegistryLockPassword(), "User already has a password, remove it first"); checkArgument( !isNullOrEmpty(registryLockPassword), "Registry lock password was null or empty"); getInstance().registryLockPasswordSalt = base64().encode(SALT_SUPPLIER.get()); getInstance().registryLockPasswordHash = hashPassword(registryLockPassword, getInstance().registryLockPasswordSalt); - getInstance().allowedToSetRegistryLockPassword = false; return this; } } diff --git a/core/src/test/java/google/registry/model/console/ConsoleRoleDefinitionsTest.java b/core/src/test/java/google/registry/model/console/ConsoleRoleDefinitionsTest.java index 15dcddbd3..be4fa7436 100644 --- a/core/src/test/java/google/registry/model/console/ConsoleRoleDefinitionsTest.java +++ b/core/src/test/java/google/registry/model/console/ConsoleRoleDefinitionsTest.java @@ -49,21 +49,30 @@ public class ConsoleRoleDefinitionsTest { // Note: we can't use Truth's IterableSubject to check all the subset/superset restrictions // because it is generic to iterables and doesn't know about sets. assertThat( - ConsoleRoleDefinitions.SUPPORT_LEAD_PERMISSIONS.containsAll( - ConsoleRoleDefinitions.SUPPORT_AGENT_PERMISSIONS)) + ConsoleRoleDefinitions.ACCOUNT_MANAGER_WITH_REGISTRY_LOCK_PERMISSIONS.containsAll( + ConsoleRoleDefinitions.ACCOUNT_MANAGER_PERMISSIONS)) .isTrue(); assertThat( - ConsoleRoleDefinitions.SUPPORT_AGENT_PERMISSIONS.containsAll( - ConsoleRoleDefinitions.SUPPORT_LEAD_PERMISSIONS)) + ConsoleRoleDefinitions.ACCOUNT_MANAGER_PERMISSIONS.containsAll( + ConsoleRoleDefinitions.ACCOUNT_MANAGER_WITH_REGISTRY_LOCK_PERMISSIONS)) .isFalse(); assertThat( - ConsoleRoleDefinitions.SUPPORT_LEAD_PERMISSIONS.containsAll( - ConsoleRoleDefinitions.SUPPORT_AGENT_PERMISSIONS)) + ConsoleRoleDefinitions.TECH_CONTACT_PERMISSIONS.containsAll( + ConsoleRoleDefinitions.ACCOUNT_MANAGER_WITH_REGISTRY_LOCK_PERMISSIONS)) .isTrue(); assertThat( - ConsoleRoleDefinitions.SUPPORT_AGENT_PERMISSIONS.containsAll( - ConsoleRoleDefinitions.SUPPORT_LEAD_PERMISSIONS)) + ConsoleRoleDefinitions.ACCOUNT_MANAGER_WITH_REGISTRY_LOCK_PERMISSIONS.containsAll( + ConsoleRoleDefinitions.TECH_CONTACT_PERMISSIONS)) + .isFalse(); + + assertThat( + ConsoleRoleDefinitions.PRIMARY_CONTACT_PERMISSIONS.containsAll( + ConsoleRoleDefinitions.TECH_CONTACT_PERMISSIONS)) + .isTrue(); + assertThat( + ConsoleRoleDefinitions.TECH_CONTACT_PERMISSIONS.containsAll( + ConsoleRoleDefinitions.PRIMARY_CONTACT_PERMISSIONS)) .isFalse(); } } 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 596a35482..ac4fb0403 100644 --- a/core/src/test/java/google/registry/model/console/UserTest.java +++ b/core/src/test/java/google/registry/model/console/UserTest.java @@ -58,4 +58,33 @@ public class UserTest { builder.setUserRoles(new UserRoles.Builder().build()); builder.build(); } + + @Test + void testRegistryLockPassword() { + assertThat( + assertThrows( + IllegalArgumentException.class, + () -> + new User.Builder() + .setUserRoles(new UserRoles.Builder().build()) + .setRegistryLockPassword("foobar"))) + .hasMessageThat() + .isEqualTo("User has no registry lock permission"); + + User user = + new User.Builder() + .setGaiaId("gaiaId") + .setEmailAddress("email@email.com") + .setUserRoles(new UserRoles.Builder().setGlobalRole(GlobalRole.FTE).build()) + .build(); + assertThat(user.hasRegistryLockPassword()).isFalse(); + + user = user.asBuilder().setRegistryLockPassword("foobar").build(); + assertThat(user.hasRegistryLockPassword()).isTrue(); + assertThat(user.verifyRegistryLockPassword("foobar")).isTrue(); + + user = user.asBuilder().removeRegistryLockPassword().build(); + assertThat(user.hasRegistryLockPassword()).isFalse(); + assertThat(user.verifyRegistryLockPassword("foobar")).isFalse(); + } }