From 7b34659a8fd5df7adde3e38681ca17a5e080008b Mon Sep 17 00:00:00 2001 From: gbrodman Date: Thu, 9 May 2024 17:42:45 -0400 Subject: [PATCH] Add registryLockEmailAddress field to User object (#2418) We've added the field in the database in a previous PR. This is only used in the old console for now because the new console does not have registry lock functionality yet --- .../google/registry/model/console/User.java | 2 - .../registry/model/console/UserBase.java | 15 ++++++++ .../tools/CreateOrUpdateUserCommand.java | 18 +++++++++ .../console/ConsoleRegistryLockAction.java | 13 +++++-- .../registrar/RegistryLockPostAction.java | 3 +- .../registry/tools/CreateUserCommandTest.java | 36 ++++++++++++++++++ .../registry/tools/UpdateUserCommandTest.java | 38 +++++++++++++++++++ .../ConsoleRegistryLockActionTest.java | 12 +++++- .../registrar/RegistryLockPostActionTest.java | 3 +- .../sql/er_diagram/brief_er_diagram.html | 12 +++--- .../sql/er_diagram/full_er_diagram.html | 4 +- .../sql/schema/db-schema.sql.generated | 2 + 12 files changed, 142 insertions(+), 16 deletions(-) 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 70e407924..f94e8d297 100644 --- a/core/src/main/java/google/registry/model/console/User.java +++ b/core/src/main/java/google/registry/model/console/User.java @@ -14,7 +14,6 @@ package google.registry.model.console; - import google.registry.persistence.VKey; import javax.persistence.Access; import javax.persistence.AccessType; @@ -58,6 +57,5 @@ public class User extends UserBase { public Builder(User user) { super(user); } - } } diff --git a/core/src/main/java/google/registry/model/console/UserBase.java b/core/src/main/java/google/registry/model/console/UserBase.java index 364bc5301..9c2890bfb 100644 --- a/core/src/main/java/google/registry/model/console/UserBase.java +++ b/core/src/main/java/google/registry/model/console/UserBase.java @@ -25,6 +25,8 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import google.registry.model.Buildable; import google.registry.model.UpdateAutoTimestampEntity; import google.registry.util.PasswordUtils; +import java.util.Optional; +import javax.annotation.Nullable; import javax.persistence.Access; import javax.persistence.AccessType; import javax.persistence.Column; @@ -54,6 +56,9 @@ public class UserBase extends UpdateAutoTimestampEntity implements Buildable { @Column(nullable = false) String emailAddress; + /** Optional external email address to use for registry lock confirmation emails. */ + @Column String registryLockEmailAddress; + /** Roles (which grant permissions) associated with this user. */ @Column(nullable = false) UserRoles userRoles; @@ -89,6 +94,10 @@ public class UserBase extends UpdateAutoTimestampEntity implements Buildable { return emailAddress; } + public Optional getRegistryLockEmailAddress() { + return Optional.ofNullable(registryLockEmailAddress); + } + public UserRoles getUserRoles() { return userRoles; } @@ -150,6 +159,12 @@ public class UserBase extends UpdateAutoTimestampEntity implements Buildable { return thisCastToDerived(); } + public B setRegistryLockEmailAddress(@Nullable String registryLockEmailAddress) { + getInstance().registryLockEmailAddress = + registryLockEmailAddress == null ? null : checkValidEmail(registryLockEmailAddress); + return thisCastToDerived(); + } + public B setUserRoles(UserRoles userRoles) { checkArgumentNotNull(userRoles, "User roles cannot be null"); getInstance().userRoles = userRoles; diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateUserCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateUserCommand.java index d30b0f68e..02c0bf2d8 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateUserCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateUserCommand.java @@ -34,6 +34,14 @@ public abstract class CreateOrUpdateUserCommand extends ConfirmingCommand { @Parameter(names = "--email", description = "Email address of the user", required = true) String email; + @Nullable + @Parameter( + names = "--registry_lock_email_address", + description = + "Optional external email address to use for registry lock confirmation emails, or empty" + + " to remove the field.") + private String registryLockEmailAddress; + @Nullable @Parameter( names = "--admin", @@ -78,6 +86,16 @@ public abstract class CreateOrUpdateUserCommand extends ConfirmingCommand { User.Builder builder = (user == null) ? new User.Builder().setEmailAddress(email) : user.asBuilder(); builder.setUserRoles(userRolesBuilder.build()); + + // An empty registryLockEmailAddress indicates that we should remove the field + if (registryLockEmailAddress != null) { + if (registryLockEmailAddress.isEmpty()) { + builder.setRegistryLockEmailAddress(null); + } else { + builder.setRegistryLockEmailAddress(registryLockEmailAddress); + } + } + User newUser = builder.build(); UserDao.saveUser(newUser); } diff --git a/core/src/main/java/google/registry/ui/server/console/ConsoleRegistryLockAction.java b/core/src/main/java/google/registry/ui/server/console/ConsoleRegistryLockAction.java index 430f091b7..af3bbea88 100644 --- a/core/src/main/java/google/registry/ui/server/console/ConsoleRegistryLockAction.java +++ b/core/src/main/java/google/registry/ui/server/console/ConsoleRegistryLockAction.java @@ -150,20 +150,27 @@ public class ConsoleRegistryLockAction extends ConsoleApiAction { } } - String userEmail = user.getEmailAddress(); + Optional maybeRegistryLockEmail = user.getRegistryLockEmailAddress(); + if (maybeRegistryLockEmail.isEmpty()) { + setFailedResponse( + "User has no registry lock email address", HttpStatusCodes.STATUS_CODE_BAD_REQUEST); + return; + } + String registryLockEmail = maybeRegistryLockEmail.get(); + try { tm().transact( () -> { RegistryLock registryLock = isLock ? domainLockUtils.saveNewRegistryLockRequest( - domainName, registrarId, userEmail, isAdmin) + domainName, registrarId, registryLockEmail, isAdmin) : domainLockUtils.saveNewRegistryUnlockRequest( domainName, registrarId, isAdmin, relockDurationMillis.map(Duration::new)); - sendVerificationEmail(registryLock, userEmail, isLock); + sendVerificationEmail(registryLock, registryLockEmail, isLock); }); } catch (IllegalArgumentException e) { // Catch IllegalArgumentExceptions separately to give a nicer error message and code diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java index bfb0bbad9..ff3a12111 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java +++ b/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java @@ -199,7 +199,8 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc checkArgument( user.verifyRegistryLockPassword(postInput.password), "Incorrect registry lock password for user"); - return user.getEmailAddress(); + return user.getRegistryLockEmailAddress() + .orElseThrow(() -> new IllegalArgumentException("User has no registry lock email address")); } private String verifyPasswordAndGetEmailLegacyUser(User user, RegistryLockPostInput postInput) diff --git a/core/src/test/java/google/registry/tools/CreateUserCommandTest.java b/core/src/test/java/google/registry/tools/CreateUserCommandTest.java index 8d7dfdbda..da1f1ff07 100644 --- a/core/src/test/java/google/registry/tools/CreateUserCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateUserCommandTest.java @@ -53,6 +53,17 @@ public class CreateUserCommandTest extends CommandTestCase { verifyNoMoreInteractions(iamClient); } + @Test + void testSuccess_registryLock() throws Exception { + runCommandForced( + "--email", + "user@example.test", + "--registry_lock_email_address", + "registrylockemail@otherexample.test"); + assertThat(UserDao.loadUser("user@example.test").get().getRegistryLockEmailAddress()) + .hasValue("registrylockemail@otherexample.test"); + } + @Test void testSuccess_admin() throws Exception { runCommandForced("--email", "user@example.test", "--admin", "true"); @@ -101,4 +112,29 @@ public class CreateUserCommandTest extends CommandTestCase { .isEqualTo("A user with email user@example.test already exists"); verifyNoMoreInteractions(iamClient); } + + @Test + void testFailure_badEmail() throws Exception { + assertThat( + assertThrows( + IllegalArgumentException.class, + () -> runCommandForced("--email", "this is not valid"))) + .hasMessageThat() + .isEqualTo("Provided email this is not valid is not a valid email address"); + } + + @Test + void testFailure_badRegistryLockEmail() throws Exception { + assertThat( + assertThrows( + IllegalArgumentException.class, + () -> + runCommandForced( + "--email", + "user@example.test", + "--registry_lock_email_address", + "this is not valid"))) + .hasMessageThat() + .isEqualTo("Provided email this is not valid is not a valid email address"); + } } diff --git a/core/src/test/java/google/registry/tools/UpdateUserCommandTest.java b/core/src/test/java/google/registry/tools/UpdateUserCommandTest.java index aa7bc07b4..80145d59e 100644 --- a/core/src/test/java/google/registry/tools/UpdateUserCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateUserCommandTest.java @@ -38,6 +38,29 @@ public class UpdateUserCommandTest extends CommandTestCase { .build()); } + @Test + void testSuccess_registryLockEmail() throws Exception { + runCommandForced( + "--email", + "user@example.test", + "--registry_lock_email_address", + "registrylockemail@otherexample.test"); + assertThat(UserDao.loadUser("user@example.test").get().getRegistryLockEmailAddress()) + .hasValue("registrylockemail@otherexample.test"); + } + + @Test + void testSuccess_removeRegistryLockEmail() throws Exception { + UserDao.saveUser( + UserDao.loadUser("user@example.test") + .get() + .asBuilder() + .setRegistryLockEmailAddress("registrylock@otherexample.test") + .build()); + runCommandForced("--email", "user@example.test", "--registry_lock_email_address", ""); + assertThat(UserDao.loadUser("user@example.test").get().getRegistryLockEmailAddress()).isEmpty(); + } + @Test void testSuccess_admin() throws Exception { assertThat(UserDao.loadUser("user@example.test").get().getUserRoles().isAdmin()).isFalse(); @@ -86,4 +109,19 @@ public class UpdateUserCommandTest extends CommandTestCase { .hasMessageThat() .isEqualTo("User nonexistent@example.test not found"); } + + @Test + void testFailure_badRegistryLockEmail() { + assertThat( + assertThrows( + IllegalArgumentException.class, + () -> + runCommandForced( + "--email", + "user@example.test", + "--registry_lock_email_address", + "this is not valid"))) + .hasMessageThat() + .isEqualTo("Provided email this is not valid is not a valid email address"); + } } diff --git a/core/src/test/java/google/registry/ui/server/console/ConsoleRegistryLockActionTest.java b/core/src/test/java/google/registry/ui/server/console/ConsoleRegistryLockActionTest.java index 7f104ce1f..4b2b42a0a 100644 --- a/core/src/test/java/google/registry/ui/server/console/ConsoleRegistryLockActionTest.java +++ b/core/src/test/java/google/registry/ui/server/console/ConsoleRegistryLockActionTest.java @@ -100,6 +100,7 @@ public class ConsoleRegistryLockActionTest { user = new User.Builder() .setEmailAddress("user@theregistrar.com") + .setRegistryLockEmailAddress("registrylock@theregistrar.com") .setUserRoles( new UserRoles.Builder() .setRegistrarRoles( @@ -436,6 +437,15 @@ public class ConsoleRegistryLockActionTest { .isEqualTo("Registry lock not allowed for registrar TheRegistrar"); } + @Test + void testPost_failure_noRegistryLockEmail() { + user = user.asBuilder().setRegistryLockEmailAddress(null).build(); + action = createDefaultPostAction(true); + action.run(); + assertThat(response.getStatus()).isEqualTo(HttpStatusCodes.STATUS_CODE_BAD_REQUEST); + assertThat(response.getPayload()).isEqualTo("User has no registry lock email address"); + } + @Test void testPost_failure_badPassword() throws Exception { action = createPostAction("example.test", true, "badPassword", Optional.empty()); @@ -534,6 +544,6 @@ public class ConsoleRegistryLockActionTest { assertThat(sentMessage.subject()).matches("Registry (un)?lock verification"); assertThat(sentMessage.body()).matches(EMAIL_MESSAGE_TEMPLATE); assertThat(sentMessage.recipients()) - .containsExactly(new InternetAddress("user@theregistrar.com")); + .containsExactly(new InternetAddress("registrylock@theregistrar.com")); } } diff --git a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java index 544d73aeb..27b7f6f06 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java +++ b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java @@ -218,6 +218,7 @@ final class RegistryLockPostActionTest { google.registry.model.console.User consoleUser = new google.registry.model.console.User.Builder() .setEmailAddress("johndoe@theregistrar.com") + .setRegistryLockEmailAddress("johndoe.registrylock@theregistrar.com") .setUserRoles( new UserRoles.Builder() .setRegistrarRoles( @@ -229,7 +230,7 @@ final class RegistryLockPostActionTest { AuthResult consoleAuthResult = AuthResult.createUser(UserAuthInfo.create(consoleUser)); action = createAction(consoleAuthResult); Map response = action.handleJsonRequest(lockRequest()); - assertSuccess(response, "lock", "johndoe@theregistrar.com"); + assertSuccess(response, "lock", "johndoe.registrylock@theregistrar.com"); } @Test diff --git a/db/src/main/resources/sql/er_diagram/brief_er_diagram.html b/db/src/main/resources/sql/er_diagram/brief_er_diagram.html index e0d05bbfc..1068f2218 100644 --- a/db/src/main/resources/sql/er_diagram/brief_er_diagram.html +++ b/db/src/main/resources/sql/er_diagram/brief_er_diagram.html @@ -261,7 +261,7 @@ td.section { generated on - 2024-05-01 21:04:48.5296048 + 2024-05-09 17:59:02.29076074 last flyway file @@ -277,11 +277,11 @@ td.section { SchemaCrawler_Diagram - generated by - SchemaCrawler 16.10.1 - generated on - 2024-05-01 21:04:48.5296048 - + generated by + SchemaCrawler 16.10.1 + generated on + 2024-05-09 17:59:02.29076074 + allocationtoken_a08ccbef diff --git a/db/src/main/resources/sql/er_diagram/full_er_diagram.html b/db/src/main/resources/sql/er_diagram/full_er_diagram.html index 85675f080..e15275274 100644 --- a/db/src/main/resources/sql/er_diagram/full_er_diagram.html +++ b/db/src/main/resources/sql/er_diagram/full_er_diagram.html @@ -261,7 +261,7 @@ td.section { </tr> <tr> <td class="property_name">generated on</td> - <td class="property_value">2024-05-01 21:04:45.792776047</td> + <td class="property_value">2024-05-09 17:58:59.761656939</td> </tr> <tr> <td class="property_name">last flyway file</td> @@ -280,7 +280,7 @@ td.section { <text text-anchor="start" x="4443.5" y="-29.8" font-family="Helvetica,sans-Serif" font-size="14.00">generated by</text> <text text-anchor="start" x="4526.5" y="-29.8" font-family="Helvetica,sans-Serif" font-size="14.00">SchemaCrawler 16.10.1</text> <text text-anchor="start" x="4442.5" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">generated on</text> - <text text-anchor="start" x="4526.5" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">2024-05-01 21:04:45.792776047</text> + <text text-anchor="start" x="4526.5" y="-10.8" font-family="Helvetica,sans-Serif" font-size="14.00">2024-05-09 17:58:59.761656939</text> <polygon fill="none" stroke="#888888" points="4439,-4 4439,-44 4726,-44 4726,-4 4439,-4" /> <!-- allocationtoken_a08ccbef --> <g id="node1" class="node"> <title> diff --git a/db/src/main/resources/sql/schema/db-schema.sql.generated b/db/src/main/resources/sql/schema/db-schema.sql.generated index 4c70f9a63..093b0bd0f 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -882,6 +882,7 @@ id bigserial not null, update_timestamp timestamptz, email_address text not null, + registry_lock_email_address text, registry_lock_password_hash text, registry_lock_password_salt text, global_role text not null, @@ -899,6 +900,7 @@ history_url text not null, user_id int8 not null, email_address text not null, + registry_lock_email_address text, registry_lock_password_hash text, registry_lock_password_salt text, global_role text not null,