mirrors/nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus synced 2026-06-09 16:33:02 +00:00
Code Issues Releases Wiki Activity

Enforce additional pw reset permissions (#3098)

Browse Source 
This should not have been an issue in practice due to defense in depth,
but we should check these just in case.
This commit is contained in:
gbrodman
2026-06-25 16:45:58 -04:00
committed by GitHub
parent 6a47287da7
commit 11d625b837
5 changed files with 84 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/google/registry/ui/server/console/ConsoleApiAction.java
+2 -1
View File
@@ -105,7 +105,8 @@ public abstract class ConsoleApiAction implements Runnable {
}
}
protected void checkPermission(User user, String registrarId, ConsolePermission permission) {
protected static void checkPermission(
User user, String registrarId, ConsolePermission permission) {
if (!user.getUserRoles().hasPermission(registrarId, permission)) {
throw new ConsolePermissionForbiddenException(
String.format(
core/src/main/java/google/registry/ui/server/console/PasswordResetRequestAction.java
+6 -7
View File
@@ -131,13 +131,12 @@ public class PasswordResetRequestAction extends ConsoleApiAction {
new IllegalArgumentException(
"Unknown user with lock email " + destinationEmail));
// Prevent IDOR: Ensure the resolved user actually belongs to the registrar the requester
// has permissions for, or is a global admin.
if (!targetUser.getUserRoles().isAdmin()
&& !targetUser.getUserRoles().getRegistrarRoles().containsKey(registrarId)) {
throw new IllegalArgumentException(
"User with lock email " + destinationEmail + " is not associated with " + registrarId);
}
// Prevent IDOR: Ensure the resolved user has the right permission
checkArgument(
targetUser.getUserRoles().hasPermission(registrarId, ConsolePermission.REGISTRY_LOCK),
"User %s does not have permission REGISTRY_LOCK on registrar %s",
targetUser.getEmailAddress(),
registrarId);
return targetUser;
}
core/src/main/java/google/registry/ui/server/console/PasswordResetVerifyAction.java
+9 -1
View File
@@ -121,7 +121,15 @@ public class PasswordResetVerifyAction extends ConsoleApiAction {
ConsolePermission requiredVerifyPermission =
switch (request.getType()) {
case EPP -> ConsolePermission.MANAGE_USERS;
case REGISTRY_LOCK -> ConsolePermission.REGISTRY_LOCK;
case REGISTRY_LOCK -> {
checkArgument(
user.getRegistryLockEmailAddress()
.map(address -> address.equals(request.getDestinationEmail()))
.orElse(false),
"User %s has the wrong registry lock email address",
user.getEmailAddress());
yield ConsolePermission.REGISTRY_LOCK;
}
};
checkPermission(user, request.getRegistrarId(), requiredVerifyPermission);
if (plusHours(request.getRequestTime(), 1).isBefore(tm().getTxTime())) {
core/src/test/java/google/registry/ui/server/console/PasswordResetRequestActionTest.java
+25
View File
@@ -158,6 +158,31 @@ public class PasswordResetRequestActionTest extends ConsoleActionBaseTestCase {
.isEqualTo("Unknown user with lock email nonexistent@email.com");
}
@Test
void testFailure_registryLock_userNoLockPermission() throws Exception {
DatabaseHelper.persistResource(
new User.Builder()
.setEmailAddress("email@registry.tld")
.setUserRoles(
new UserRoles.Builder()
.setRegistrarRoles(
ImmutableMap.of("TheRegistrar", RegistrarRole.ACCOUNT_MANAGER))
.build())
.setRegistryLockEmailAddress("registrylock@theregistrar.com")
.build());
PasswordResetRequestAction action =
createAction(
PasswordResetRequest.Type.REGISTRY_LOCK,
"TheRegistrar",
"registrylock@theregistrar.com");
action.run();
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_BAD_REQUEST);
assertThat(response.getPayload())
.isEqualTo(
"User email@registry.tld does not have permission REGISTRY_LOCK on registrar"
+ " TheRegistrar");
}
@Test
@Disabled("Enable when testing is done in sandbox and isAdmin check is removed")
void testFailure_epp_noPermission() throws Exception {
core/src/test/java/google/registry/ui/server/console/PasswordResetVerifyActionTest.java
+42
View File
@@ -157,6 +157,48 @@ public class PasswordResetVerifyActionTest extends ConsoleActionBaseTestCase {
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_BAD_REQUEST);
}
@Test
void testFailure_post_lock_differentRegistryLockEmail() throws Exception {
User user =
persistResource(
new User.Builder()
.setEmailAddress("anotheruser@example.tld")
.setUserRoles(
new UserRoles.Builder()
.setRegistrarRoles(
ImmutableMap.of(
"TheRegistrar", RegistrarRole.ACCOUNT_MANAGER_WITH_REGISTRY_LOCK))
.build())
.setRegistryLockEmailAddress("anotherregistrylock@theregistrar.com")
.build());
verificationCode = saveRequest(PasswordResetRequest.Type.REGISTRY_LOCK).getVerificationCode();
createAction(user, "POST", verificationCode, "newPassword").run();
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_BAD_REQUEST);
assertThat(response.getPayload())
.isEqualTo("User anotheruser@example.tld has the wrong registry lock email address");
}
@Test
void testFailure_get_lock_differentRegistryLockEmail() throws Exception {
User user =
persistResource(
new User.Builder()
.setEmailAddress("anotheruser@example.tld")
.setUserRoles(
new UserRoles.Builder()
.setRegistrarRoles(
ImmutableMap.of(
"TheRegistrar", RegistrarRole.ACCOUNT_MANAGER_WITH_REGISTRY_LOCK))
.build())
.setRegistryLockEmailAddress("anotherregistrylock@theregistrar.com")
.build());
verificationCode = saveRequest(PasswordResetRequest.Type.REGISTRY_LOCK).getVerificationCode();
createAction(user, "GET", verificationCode, null).run();
assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_BAD_REQUEST);
assertThat(response.getPayload())
.isEqualTo("User anotheruser@example.tld has the wrong registry lock email address");
}
private User createTechUser() {
return new User.Builder()
.setEmailAddress("tech@example.tld")