From 06ce429c5ad4ced2625e473851dd586b8fecb01d Mon Sep 17 00:00:00 2001 From: guyben Date: Wed, 17 Oct 2018 10:10:32 -0700 Subject: [PATCH] Include the performing user in the "Registrar updated" emails Whenever a registrar is changed via the registrar console, we send out a notification of that change. Since we're going to allow Admins and soon Vendors to use the console in addition to the registrars, it becomes important to know who actually performed the changes if the registrars complain. In addition, we will now send notifications for changes in Sandbox since we're going to actually allow registrars to update sandbox data. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=217539534 --- .../registry/request/auth/AuthResult.java | 6 +++- .../registrar/RegistrarSettingsAction.java | 29 ++++++++++++++----- .../RegistrarSettingsActionTestCase.java | 9 ++++++ .../testdata/update_registrar_email.txt | 3 +- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/java/google/registry/request/auth/AuthResult.java b/java/google/registry/request/auth/AuthResult.java index 24291ee74..c2e4bc585 100644 --- a/java/google/registry/request/auth/AuthResult.java +++ b/java/google/registry/request/auth/AuthResult.java @@ -38,7 +38,11 @@ public abstract class AuthResult { public String userIdForLogging() { return userAuthInfo() - .map(userAuthInfo -> userAuthInfo.user().getEmail()) + .map( + userAuthInfo -> + String.format( + "%s %s", + userAuthInfo.isUserAdmin() ? "admin" : "user", userAuthInfo.user().getEmail())) .orElse(""); } diff --git a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index d6ce6a466..f7cc41d61 100644 --- a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -23,6 +23,7 @@ import static google.registry.security.JsonResponseHelper.Status.SUCCESS; import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.READ; import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.UPDATE; +import com.google.common.base.Ascii; import com.google.common.base.Strings; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableList; @@ -32,6 +33,7 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; import google.registry.config.RegistryConfig.Config; +import google.registry.config.RegistryEnvironment; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; import google.registry.model.registrar.RegistrarContact.Type; @@ -39,6 +41,7 @@ import google.registry.request.Action; import google.registry.request.HttpException.BadRequestException; import google.registry.request.JsonActionRunner; import google.registry.request.auth.Auth; +import google.registry.request.auth.AuthResult; import google.registry.security.JsonResponseHelper; import google.registry.ui.forms.FormException; import google.registry.ui.forms.FormFieldException; @@ -79,6 +82,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA @Inject AppEngineServiceUtils appEngineServiceUtils; @Inject SendEmailUtils sendEmailUtils; @Inject AuthenticatedRegistrarAccessor registrarAccessor; + @Inject AuthResult authResult; + @Inject RegistryEnvironment registryEnvironment; @Inject @Config("registrarChangesNotificationEmailAddresses") ImmutableList registrarChangesNotificationEmailAddresses; @@ -126,7 +131,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA } catch (Throwable e) { logger.atWarning().withCause(e).log( "Failed to perform operation '%s' on registrar '%s' for args %s", op, clientId, args); - return JsonResponseHelper.create(ERROR, e.getMessage()); + return JsonResponseHelper.create( + ERROR, Optional.ofNullable(e.getMessage()).orElse("Unspecified error")); } } @@ -376,13 +382,20 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA return; } enqueueRegistrarSheetSync(appEngineServiceUtils.getCurrentVersionHostname("backend")); - if (!registrarChangesNotificationEmailAddresses.isEmpty()) { - sendEmailUtils.sendEmail( - registrarChangesNotificationEmailAddresses, - String.format("Registrar %s updated", existingRegistrar.getRegistrarName()), - "The following changes were made to the registrar:\n" - + DiffUtils.prettyPrintDiffedMap(diffs, null)); - } + String environment = Ascii.toLowerCase(String.valueOf(registryEnvironment)); + sendEmailUtils.sendEmail( + registrarChangesNotificationEmailAddresses, + String.format( + "Registrar %s (%s) updated in %s", + existingRegistrar.getRegistrarName(), + existingRegistrar.getClientId(), + environment), + String.format( + "The following changes were made on %s to the registrar %s by %s:\n\n%s", + environment, + existingRegistrar.getClientId(), + authResult.userIdForLogging(), + DiffUtils.prettyPrintDiffedMap(diffs, null))); } /** Thrown when a set of contacts doesn't meet certain constraints. */ diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index c65d43d18..95864dee4 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -26,11 +26,15 @@ import static org.mockito.Mockito.when; import com.google.appengine.api.users.User; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import google.registry.config.RegistryEnvironment; import google.registry.model.ofy.Ofy; import google.registry.request.HttpException.ForbiddenException; import google.registry.request.JsonActionRunner; import google.registry.request.JsonResponse; import google.registry.request.ResponseImpl; +import google.registry.request.auth.AuthLevel; +import google.registry.request.auth.AuthResult; +import google.registry.request.auth.UserAuthInfo; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.InjectRule; @@ -88,6 +92,11 @@ public class RegistrarSettingsActionTestCase { "notification@test.example", "notification2@test.example"); action.sendEmailUtils = new SendEmailUtils(getGSuiteOutgoingEmailAddress(), getGSuiteOutgoingEmailDisplayName()); + action.registryEnvironment = RegistryEnvironment.get(); + action.authResult = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("user@email.com", "email.com", "12345"), false)); inject.setStaticField(Ofy.class, "clock", clock); inject.setStaticField(SendEmailUtils.class, "emailService", emailService); message = new MimeMessage(Session.getDefaultInstance(new Properties(), null)); diff --git a/javatests/google/registry/ui/server/registrar/testdata/update_registrar_email.txt b/javatests/google/registry/ui/server/registrar/testdata/update_registrar_email.txt index 660c73cd5..609fbbad3 100644 --- a/javatests/google/registry/ui/server/registrar/testdata/update_registrar_email.txt +++ b/javatests/google/registry/ui/server/registrar/testdata/update_registrar_email.txt @@ -1,4 +1,5 @@ -The following changes were made to the registrar: +The following changes were made on unittest to the registrar TheRegistrar by user user@email.com: + whoisServer: null -> foo.bar.baz ipAddressWhitelist: null -> [1.1.1.1/32, 2.2.2.2/32, 4.4.4.4/32] localizedAddress.street.0: 123 Example Bőulevard -> 123 Street Rd