diff --git a/core/src/main/java/google/registry/tools/DomainLockUtils.java b/core/src/main/java/google/registry/tools/DomainLockUtils.java index 8c64e3e31..5ffd1a3ff 100644 --- a/core/src/main/java/google/registry/tools/DomainLockUtils.java +++ b/core/src/main/java/google/registry/tools/DomainLockUtils.java @@ -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 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); diff --git a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java index ebe34e988..53abc5ff3 100644 --- a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java +++ b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java @@ -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 =