From 63bb2dd79b3b0f68320cd1f6ae4fa5873191a544 Mon Sep 17 00:00:00 2001 From: gbrodman Date: Wed, 23 Oct 2019 10:57:46 -0700 Subject: [PATCH] Don't include password hash + salt in visible diffs (#322) We don't want to override toDiffableFieldMap because (per the javadoc) that is supposed to contain sensitive information. So, we should just remove it before sending it out. --- .../ui/server/registrar/RegistrarSettingsAction.java | 11 +++++++++-- .../ui/server/registrar/update_registrar_email.txt | 8 ++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index a8e201a97..285e9692c 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -240,11 +240,18 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA }); } - private Map expandRegistrarWithContacts(Iterable contacts, - Registrar registrar) { + private Map expandRegistrarWithContacts( + Iterable contacts, Registrar registrar) { ImmutableSet> expandedContacts = Streams.stream(contacts) .map(RegistrarContact::toDiffableFieldMap) + // Note: per the javadoc, toDiffableFieldMap includes sensitive data but we don't want + // to display it here + .peek( + map -> { + map.remove("registryLockPasswordHash"); + map.remove("registryLockPasswordSalt"); + }) .collect(toImmutableSet()); // Use LinkedHashMap here to preserve ordering; null values mean we can't use ImmutableMap. LinkedHashMap result = new LinkedHashMap<>(registrar.toDiffableFieldMap()); diff --git a/core/src/test/resources/google/registry/ui/server/registrar/update_registrar_email.txt b/core/src/test/resources/google/registry/ui/server/registrar/update_registrar_email.txt index 0fe61c218..e1b2cf009 100644 --- a/core/src/test/resources/google/registry/ui/server/registrar/update_registrar_email.txt +++ b/core/src/test/resources/google/registry/ui/server/registrar/update_registrar_email.txt @@ -11,9 +11,9 @@ emailAddress: the.registrar@example.com -> thase@the.registrar url: http://my.fake.url -> http://my.new.url contacts: ADDED: - {parent=Key(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false, registryLockPasswordHash=null, registryLockPasswordSalt=null} + {parent=Key(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false} REMOVED: - {parent=Key(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=John Doe, emailAddress=johndoe@theregistrar.com, phoneNumber=+1.1234567890, faxNumber=null, types=[ADMIN], gaeUserId=31337, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false, registryLockPasswordHash=null, registryLockPasswordSalt=null}, - {parent=Key(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Jian-Yang, emailAddress=jyang@bachman.accelerator, phoneNumber=+1.1234567890, faxNumber=null, types=[TECH], gaeUserId=null, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false, registryLockPasswordHash=null, registryLockPasswordSalt=null} + {parent=Key(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=John Doe, emailAddress=johndoe@theregistrar.com, phoneNumber=+1.1234567890, faxNumber=null, types=[ADMIN], gaeUserId=31337, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}, + {parent=Key(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Jian-Yang, emailAddress=jyang@bachman.accelerator, phoneNumber=+1.1234567890, faxNumber=null, types=[TECH], gaeUserId=null, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false} FINAL CONTENTS: - {parent=Key(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false, registryLockPasswordHash=null, registryLockPasswordSalt=null} + {parent=Key(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}