1
0
mirror of https://github.com/google/nomulus synced 2026-07-19 06:22:33 +00:00

Handle RegistryLock unlock edge case with doubly-applied lock (#3159)

We allow admin locks to overwrite already-applied locks. In the case
where that happens in between an unlock request and an unlock
completion, we shouldn't allow the unlock completion to go through.
This commit is contained in:
gbrodman
2026-07-17 13:36:26 -04:00
committed by GitHub
parent d6dd95b052
commit d29a98bdd3
2 changed files with 74 additions and 0 deletions
@@ -241,6 +241,15 @@ public final class DomainLockUtils {
lock.getRegistrarId());
checkArgument(!lock.isSuperuser() || isAdmin, "Non-admin user cannot complete admin unlock");
// The pending unlock must still be the most recent RegistryLock for this domain. If a newer
// lock exists (e.g. an admin/superuser lock applied after this unlock was requested or a
// different unlock+relock), the verification code being redeemed is for a superseded row and
// must not be allowed to strip the lock statuses that the newer row applied.
Optional<RegistryLock> mostRecent = RegistryLockDao.getMostRecentByRepoId(lock.getRepoId());
checkArgument(
mostRecent.isPresent() && mostRecent.get().getRevisionId().equals(lock.getRevisionId()),
"The pending unlock on %s has been superseded; please request a new unlock",
lock.getDomainName());
RegistryLock newLock =
RegistryLockDao.save(lock.asBuilder().setUnlockCompletionTime(now).build());
removeLockStatuses(newLock, isAdmin, now);
@@ -512,6 +512,71 @@ public final class DomainLockUtilsTest {
assertNoDomainChanges();
}
@Test
void testFailure_applyUnlock_supersededByNewerLock() {
domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false);
RegistryLock unlockRequest =
domainLockUtils.saveNewRegistryUnlockRequest(
DOMAIN_NAME, "TheRegistrar", false, Optional.empty());
clock.advanceOneMilli();
domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", null, true);
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() ->
domainLockUtils.verifyVerificationCode(
unlockRequest.getVerificationCode(), NON_ADMIN_USER));
assertThat(thrown)
.hasMessageThat()
.isEqualTo(
"The pending unlock on example.tld has been superseded; please request a new unlock");
}
@Test
void testFailure_applyUnlock_supersededByInterleavedUnlockAndLock() {
// Lock the domain initially
domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", null, true);
// 1. Unlock A requested
clock.advanceOneMilli();
RegistryLock unlockA =
domainLockUtils.saveNewRegistryUnlockRequest(
DOMAIN_NAME, "TheRegistrar", true, Optional.empty());
// 2. Unlock B requested
clock.advanceOneMilli();
RegistryLock unlockB =
domainLockUtils.saveNewRegistryUnlockRequest(
DOMAIN_NAME, "TheRegistrar", true, Optional.empty());
// 3. Unlock B completed
clock.advanceOneMilli();
domainLockUtils.verifyVerificationCode(unlockB.getVerificationCode(), ADMIN_USER);
// 4. Lock C requested
clock.advanceOneMilli();
RegistryLock lockC =
domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, true);
// 5. Lock C applied
clock.advanceOneMilli();
domainLockUtils.verifyVerificationCode(lockC.getVerificationCode(), ADMIN_USER);
// 6. Unlock A completion attempt should fail since it has been superseded -- the second unlock
// request rewrote the verification code
clock.advanceOneMilli();
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() ->
domainLockUtils.verifyVerificationCode(unlockA.getVerificationCode(), ADMIN_USER));
assertThat(thrown)
.hasMessageThat()
.isEqualTo(
String.format("Invalid verification code \"%s\"", unlockA.getVerificationCode()));
}
@Test
void testFailure_applyLock_alreadyLocked() {
RegistryLock lock =