Clean up RegistryLockPostAction (#470)

* Clean up RegistryLockPostAction

* pocId -> userEmail when appropriate

* Merge remote-tracking branch 'origin/master' into lockPostImprovements

* Remove pocId
This commit is contained in:
gbrodman 2020-02-11 08:43:44 -07:00 committed by GitHub
parent 79b46001b6
commit a076b746a3
2 changed files with 48 additions and 61 deletions

View file

@ -21,7 +21,6 @@ import static google.registry.security.JsonResponseHelper.Status.ERROR;
import static google.registry.security.JsonResponseHelper.Status.SUCCESS; import static google.registry.security.JsonResponseHelper.Status.SUCCESS;
import static google.registry.ui.server.registrar.RegistrarConsoleModule.PARAM_CLIENT_ID; import static google.registry.ui.server.registrar.RegistrarConsoleModule.PARAM_CLIENT_ID;
import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.base.Throwables; import com.google.common.base.Throwables;
@ -34,6 +33,7 @@ import google.registry.model.registrar.Registrar;
import google.registry.model.registrar.RegistrarContact; import google.registry.model.registrar.RegistrarContact;
import google.registry.request.Action; import google.registry.request.Action;
import google.registry.request.Action.Method; import google.registry.request.Action.Method;
import google.registry.request.HttpException.ForbiddenException;
import google.registry.request.JsonActionRunner; import google.registry.request.JsonActionRunner;
import google.registry.request.auth.Auth; import google.registry.request.auth.Auth;
import google.registry.request.auth.AuthResult; import google.registry.request.auth.AuthResult;
@ -123,10 +123,16 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
!Strings.isNullOrEmpty(postInput.fullyQualifiedDomainName), !Strings.isNullOrEmpty(postInput.fullyQualifiedDomainName),
"Missing key for fullyQualifiedDomainName"); "Missing key for fullyQualifiedDomainName");
checkNotNull(postInput.isLock, "Missing key for isLock"); checkNotNull(postInput.isLock, "Missing key for isLock");
checkArgumentPresent(authResult.userAuthInfo(), "User is not logged in"); UserAuthInfo userAuthInfo =
authResult
.userAuthInfo()
.orElseThrow(() -> new ForbiddenException("User is not logged in"));
boolean isAdmin = authResult.userAuthInfo().get().isUserAdmin(); boolean isAdmin = userAuthInfo.isUserAdmin();
verifyRegistryLockPassword(postInput); String userEmail = userAuthInfo.user().getEmail();
if (!isAdmin) {
verifyRegistryLockPassword(postInput, userEmail);
}
jpaTm() jpaTm()
.transact( .transact(
() -> { () -> {
@ -135,12 +141,12 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
? domainLockUtils.createRegistryLockRequest( ? domainLockUtils.createRegistryLockRequest(
postInput.fullyQualifiedDomainName, postInput.fullyQualifiedDomainName,
postInput.clientId, postInput.clientId,
postInput.pocId, userEmail,
isAdmin, isAdmin,
clock) clock)
: domainLockUtils.createRegistryUnlockRequest( : domainLockUtils.createRegistryUnlockRequest(
postInput.fullyQualifiedDomainName, postInput.clientId, isAdmin, clock); postInput.fullyQualifiedDomainName, postInput.clientId, isAdmin, clock);
sendVerificationEmail(registryLock, postInput.isLock); sendVerificationEmail(registryLock, userEmail, postInput.isLock);
}); });
String action = postInput.isLock ? "lock" : "unlock"; String action = postInput.isLock ? "lock" : "unlock";
return JsonResponseHelper.create(SUCCESS, String.format("Successful %s", action)); return JsonResponseHelper.create(SUCCESS, String.format("Successful %s", action));
@ -152,7 +158,7 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
} }
} }
private void sendVerificationEmail(RegistryLock lock, boolean isLock) { private void sendVerificationEmail(RegistryLock lock, String userEmail, boolean isLock) {
try { try {
String url = String url =
new URIBuilder() new URIBuilder()
@ -165,8 +171,7 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
.toString(); .toString();
String body = String.format(VERIFICATION_EMAIL_TEMPLATE, lock.getDomainName(), url); String body = String.format(VERIFICATION_EMAIL_TEMPLATE, lock.getDomainName(), url);
ImmutableList<InternetAddress> recipients = ImmutableList<InternetAddress> recipients =
ImmutableList.of( ImmutableList.of(new InternetAddress(userEmail, true));
new InternetAddress(authResult.userAuthInfo().get().user().getEmail(), true));
String action = isLock ? "lock" : "unlock"; String action = isLock ? "lock" : "unlock";
sendEmailService.sendEmail( sendEmailService.sendEmail(
EmailMessage.newBuilder() EmailMessage.newBuilder()
@ -180,30 +185,25 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
} }
} }
private void verifyRegistryLockPassword(RegistryLockPostInput postInput) private void verifyRegistryLockPassword(RegistryLockPostInput postInput, String userEmail)
throws RegistrarAccessDeniedException { throws RegistrarAccessDeniedException {
// Verify that the user can access the registrar and that the user is either an admin or has // Verify that the user can access the registrar and that the user has
// registry lock enabled and provided a correct password // registry lock enabled and provided a correct password
checkArgument(authResult.userAuthInfo().isPresent(), "Auth result not present");
Registrar registrar = registrarAccessor.getRegistrar(postInput.clientId); Registrar registrar = registrarAccessor.getRegistrar(postInput.clientId);
checkArgument( checkArgument(
registrar.isRegistryLockAllowed(), "Registry lock not allowed for this registrar"); registrar.isRegistryLockAllowed(), "Registry lock not allowed for this registrar");
UserAuthInfo userAuthInfo = authResult.userAuthInfo().get(); checkArgument(!Strings.isNullOrEmpty(postInput.password), "Missing key for password");
if (!userAuthInfo.isUserAdmin()) { RegistrarContact registrarContact =
checkArgument(!Strings.isNullOrEmpty(postInput.pocId), "Missing key for pocId"); registrar.getContacts().stream()
checkArgument(!Strings.isNullOrEmpty(postInput.password), "Missing key for password"); .filter(contact -> contact.getEmailAddress().equals(userEmail))
RegistrarContact registrarContact = .findFirst()
registrar.getContacts().stream() .orElseThrow(
.filter(contact -> contact.getEmailAddress().equals(postInput.pocId)) () ->
.findFirst() new IllegalArgumentException(
.orElseThrow( String.format("Unknown user email %s", userEmail)));
() -> checkArgument(
new IllegalArgumentException( registrarContact.verifyRegistryLockPassword(postInput.password),
String.format("Unknown registrar POC ID %s", postInput.pocId))); "Incorrect registry lock password for contact");
checkArgument(
registrarContact.verifyRegistryLockPassword(postInput.password),
"Incorrect registry lock password for contact");
}
} }
/** Value class that represents the expected input body from the UI request. */ /** Value class that represents the expected input body from the UI request. */
@ -211,7 +211,6 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
private String clientId; private String clientId;
private String fullyQualifiedDomainName; private String fullyQualifiedDomainName;
private Boolean isLock; private Boolean isLock;
private String pocId;
private String password; private String password;
} }
} }

View file

@ -132,6 +132,7 @@ public final class RegistryLockPostActionTest {
createAction( createAction(
AuthResult.create(AuthLevel.USER, UserAuthInfo.create(userWithoutPermission, true))); AuthResult.create(AuthLevel.USER, UserAuthInfo.create(userWithoutPermission, true)));
Map<String, ?> response = action.handleJsonRequest(unlockRequest()); Map<String, ?> response = action.handleJsonRequest(unlockRequest());
// we should still email the admin user's email address
assertSuccess(response, "unlock", "johndoe@theregistrar.com"); assertSuccess(response, "unlock", "johndoe@theregistrar.com");
} }
@ -171,7 +172,7 @@ public final class RegistryLockPostActionTest {
@Test @Test
public void testSuccess_adminUser() throws Exception { public void testSuccess_adminUser() throws Exception {
// Admin user should be able to lock/unlock regardless // Admin user should be able to lock/unlock regardless -- and we use the admin user's email
action = action =
createAction( createAction(
AuthResult.create(AuthLevel.USER, UserAuthInfo.create(userWithoutPermission, true))); AuthResult.create(AuthLevel.USER, UserAuthInfo.create(userWithoutPermission, true)));
@ -179,6 +180,20 @@ public final class RegistryLockPostActionTest {
assertSuccess(response, "lock", "johndoe@theregistrar.com"); assertSuccess(response, "lock", "johndoe@theregistrar.com");
} }
@Test
public void testSuccess_adminUser_doesNotRequirePassword() throws Exception {
action =
createAction(
AuthResult.create(AuthLevel.USER, UserAuthInfo.create(userWithoutPermission, true)));
Map<String, ?> response =
action.handleJsonRequest(
ImmutableMap.of(
"clientId", "TheRegistrar",
"fullyQualifiedDomainName", "example.tld",
"isLock", true));
assertSuccess(response, "lock", "johndoe@theregistrar.com");
}
@Test @Test
public void testFailure_noInput() { public void testFailure_noInput() {
Map<String, ?> response = action.handleJsonRequest(null); Map<String, ?> response = action.handleJsonRequest(null);
@ -231,20 +246,21 @@ public final class RegistryLockPostActionTest {
ImmutableMap.of( ImmutableMap.of(
"clientId", "TheRegistrar", "clientId", "TheRegistrar",
"fullyQualifiedDomainName", "example.tld", "fullyQualifiedDomainName", "example.tld",
"isLock", true, "isLock", true));
"pocId", "Marla.Singer@crr.com"));
assertFailureWithMessage(response, "Missing key for password"); assertFailureWithMessage(response, "Missing key for password");
} }
@Test @Test
public void testFailure_notEnabledForRegistrarContact() { public void testFailure_notEnabledForRegistrarContact() {
action =
createAction(
AuthResult.create(AuthLevel.USER, UserAuthInfo.create(userWithoutPermission, false)));
Map<String, ?> response = Map<String, ?> response =
action.handleJsonRequest( action.handleJsonRequest(
ImmutableMap.of( ImmutableMap.of(
"clientId", "TheRegistrar", "clientId", "TheRegistrar",
"fullyQualifiedDomainName", "example.tld", "fullyQualifiedDomainName", "example.tld",
"isLock", true, "isLock", true,
"pocId", "johndoe@theregistrar.com",
"password", "hi")); "password", "hi"));
assertFailureWithMessage(response, "Incorrect registry lock password for contact"); assertFailureWithMessage(response, "Incorrect registry lock password for contact");
} }
@ -257,7 +273,6 @@ public final class RegistryLockPostActionTest {
"clientId", "TheRegistrar", "clientId", "TheRegistrar",
"fullyQualifiedDomainName", "example.tld", "fullyQualifiedDomainName", "example.tld",
"isLock", true, "isLock", true,
"pocId", "Marla.Singer@crr.com",
"password", "badPassword")); "password", "badPassword"));
assertFailureWithMessage(response, "Incorrect registry lock password for contact"); assertFailureWithMessage(response, "Incorrect registry lock password for contact");
} }
@ -270,36 +285,10 @@ public final class RegistryLockPostActionTest {
"clientId", "TheRegistrar", "clientId", "TheRegistrar",
"fullyQualifiedDomainName", "bad.tld", "fullyQualifiedDomainName", "bad.tld",
"isLock", true, "isLock", true,
"pocId", "Marla.Singer@crr.com",
"password", "hi")); "password", "hi"));
assertFailureWithMessage(response, "Unknown domain bad.tld"); assertFailureWithMessage(response, "Unknown domain bad.tld");
} }
@Test
public void testFailure_noPocId() {
Map<String, ?> response =
action.handleJsonRequest(
ImmutableMap.of(
"clientId", "TheRegistrar",
"fullyQualifiedDomainName", "bad.tld",
"isLock", true,
"password", "hi"));
assertFailureWithMessage(response, "Missing key for pocId");
}
@Test
public void testFailure_invalidPocId() {
Map<String, ?> response =
action.handleJsonRequest(
ImmutableMap.of(
"clientId", "TheRegistrar",
"fullyQualifiedDomainName", "bad.tld",
"isLock", true,
"pocId", "someotherpoc@crr.com",
"password", "hi"));
assertFailureWithMessage(response, "Unknown registrar POC ID someotherpoc@crr.com");
}
@Test @Test
public void testSuccess_previousLockUnlocked() throws Exception { public void testSuccess_previousLockUnlocked() throws Exception {
RegistryLockDao.save( RegistryLockDao.save(
@ -357,7 +346,6 @@ public final class RegistryLockPostActionTest {
"isLock", lock, "isLock", lock,
"clientId", "TheRegistrar", "clientId", "TheRegistrar",
"fullyQualifiedDomainName", "example.tld", "fullyQualifiedDomainName", "example.tld",
"pocId", "Marla.Singer@crr.com",
"password", "hi"); "password", "hi");
} }