mirror of
https://github.com/google/nomulus.git
synced 2025-07-09 04:33:28 +02:00
Require an override flag to allow updating pending delete domains (#939)
* Require an override flag to allow updating pending delete domains Needing to update pending delete domains is an uncommon situation, yet currently we are allowing superusers to do so without any extra validation (which has led to errors). This adds a new override flag to gate the update of pending delete domains; without it, the update will fail.
This commit is contained in:
parent
306a6dc079
commit
e80f293438
2 changed files with 70 additions and 1 deletions
|
@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
|
||||||
import static com.google.common.collect.ImmutableSet.toImmutableSet;
|
import static com.google.common.collect.ImmutableSet.toImmutableSet;
|
||||||
import static google.registry.model.EppResourceUtils.loadByForeignKey;
|
import static google.registry.model.EppResourceUtils.loadByForeignKey;
|
||||||
import static google.registry.model.domain.rgp.GracePeriodStatus.AUTO_RENEW;
|
import static google.registry.model.domain.rgp.GracePeriodStatus.AUTO_RENEW;
|
||||||
|
import static google.registry.model.eppcommon.StatusValue.PENDING_DELETE;
|
||||||
import static google.registry.model.eppcommon.StatusValue.SERVER_UPDATE_PROHIBITED;
|
import static google.registry.model.eppcommon.StatusValue.SERVER_UPDATE_PROHIBITED;
|
||||||
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
|
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
|
||||||
import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
|
import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
|
||||||
|
@ -139,6 +140,11 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
|
||||||
+ " deleted at the end of its current registration period.")
|
+ " deleted at the end of its current registration period.")
|
||||||
Boolean autorenews;
|
Boolean autorenews;
|
||||||
|
|
||||||
|
@Parameter(
|
||||||
|
names = {"--force_in_pending_delete"},
|
||||||
|
description = "Force a superuser update even on domains that are in pending delete")
|
||||||
|
boolean forceInPendingDelete;
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected void initMutatingEppToolCommand() {
|
protected void initMutatingEppToolCommand() {
|
||||||
if (!nameservers.isEmpty()) {
|
if (!nameservers.isEmpty()) {
|
||||||
|
@ -186,6 +192,12 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
|
||||||
"The domain '%s' has status SERVER_UPDATE_PROHIBITED. Verify that you are allowed "
|
"The domain '%s' has status SERVER_UPDATE_PROHIBITED. Verify that you are allowed "
|
||||||
+ "to make updates, and if so, use the domain_unlock command to enable updates.",
|
+ "to make updates, and if so, use the domain_unlock command to enable updates.",
|
||||||
domain);
|
domain);
|
||||||
|
checkArgument(
|
||||||
|
!domainBase.getStatusValues().contains(PENDING_DELETE) || forceInPendingDelete,
|
||||||
|
"The domain '%s' has status PENDING_DELETE. Verify that you really are intending to "
|
||||||
|
+ "update a domain in pending delete (this is uncommon), and if so, pass the "
|
||||||
|
+ "--force_in_pending_delete parameter to allow this update.",
|
||||||
|
domain);
|
||||||
|
|
||||||
// Use TreeSets so that the results are always in the same order (this makes testing easier).
|
// Use TreeSets so that the results are always in the same order (this makes testing easier).
|
||||||
Set<String> addAdminsThisDomain = new TreeSet<>(addAdmins);
|
Set<String> addAdminsThisDomain = new TreeSet<>(addAdmins);
|
||||||
|
|
|
@ -16,6 +16,7 @@ package google.registry.tools;
|
||||||
|
|
||||||
import static com.google.common.truth.Truth.assertThat;
|
import static com.google.common.truth.Truth.assertThat;
|
||||||
import static google.registry.model.domain.rgp.GracePeriodStatus.AUTO_RENEW;
|
import static google.registry.model.domain.rgp.GracePeriodStatus.AUTO_RENEW;
|
||||||
|
import static google.registry.model.eppcommon.StatusValue.PENDING_DELETE;
|
||||||
import static google.registry.model.eppcommon.StatusValue.SERVER_UPDATE_PROHIBITED;
|
import static google.registry.model.eppcommon.StatusValue.SERVER_UPDATE_PROHIBITED;
|
||||||
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
|
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
|
||||||
import static google.registry.testing.DatabaseHelper.createTld;
|
import static google.registry.testing.DatabaseHelper.createTld;
|
||||||
|
@ -334,6 +335,39 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
|
||||||
assertThat(stdErr).contains("example.tld");
|
assertThat(stdErr).contains("example.tld");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void testSuccess_canUpdatePendingDeleteDomain_whenSuperuserPassesOverrideFlag() throws Exception {
|
||||||
|
ContactResource adminContact1 = persistResource(newContactResource("crr-admin1"));
|
||||||
|
ContactResource adminContact2 = persistResource(newContactResource("crr-admin2"));
|
||||||
|
ContactResource techContact1 = persistResource(newContactResource("crr-tech1"));
|
||||||
|
ContactResource techContact2 = persistResource(newContactResource("crr-tech2"));
|
||||||
|
VKey<ContactResource> adminResourceKey1 = adminContact1.createVKey();
|
||||||
|
VKey<ContactResource> adminResourceKey2 = adminContact2.createVKey();
|
||||||
|
VKey<ContactResource> techResourceKey1 = techContact1.createVKey();
|
||||||
|
VKey<ContactResource> techResourceKey2 = techContact2.createVKey();
|
||||||
|
|
||||||
|
persistResource(
|
||||||
|
newDomainBase("example.tld")
|
||||||
|
.asBuilder()
|
||||||
|
.setContacts(
|
||||||
|
ImmutableSet.of(
|
||||||
|
DesignatedContact.create(DesignatedContact.Type.ADMIN, adminResourceKey1),
|
||||||
|
DesignatedContact.create(DesignatedContact.Type.ADMIN, adminResourceKey2),
|
||||||
|
DesignatedContact.create(DesignatedContact.Type.TECH, techResourceKey1),
|
||||||
|
DesignatedContact.create(DesignatedContact.Type.TECH, techResourceKey2)))
|
||||||
|
.setStatusValues(ImmutableSet.of(PENDING_DELETE))
|
||||||
|
.build());
|
||||||
|
|
||||||
|
runCommandForced(
|
||||||
|
"--client=NewRegistrar",
|
||||||
|
"--admins=crr-admin2,crr-admin3",
|
||||||
|
"--techs=crr-tech2,crr-tech3",
|
||||||
|
"--superuser",
|
||||||
|
"--force_in_pending_delete",
|
||||||
|
"example.tld");
|
||||||
|
eppVerifier.expectSuperuser().verifySent("domain_update_set_contacts.xml");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void testFailure_cantUpdateRegistryLockedDomainEvenAsSuperuser() {
|
void testFailure_cantUpdateRegistryLockedDomainEvenAsSuperuser() {
|
||||||
HostResource host = persistActiveHost("ns1.zdns.google");
|
HostResource host = persistActiveHost("ns1.zdns.google");
|
||||||
|
@ -356,7 +390,30 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
|
||||||
"example.tld"));
|
"example.tld"));
|
||||||
assertThat(e)
|
assertThat(e)
|
||||||
.hasMessageThat()
|
.hasMessageThat()
|
||||||
.containsMatch("The domain 'example.tld' has status SERVER_UPDATE_PROHIBITED");
|
.contains("The domain 'example.tld' has status SERVER_UPDATE_PROHIBITED.");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void testFailure_cantUpdatePendingDeleteDomainEvenAsSuperuser_withoutPassingOverrideFlag() {
|
||||||
|
HostResource host = persistActiveHost("ns1.zdns.google");
|
||||||
|
ImmutableSet<VKey<HostResource>> nameservers = ImmutableSet.of(host.createVKey());
|
||||||
|
persistResource(
|
||||||
|
newDomainBase("example.tld")
|
||||||
|
.asBuilder()
|
||||||
|
.setStatusValues(ImmutableSet.of(PENDING_DELETE))
|
||||||
|
.setNameservers(nameservers)
|
||||||
|
.build());
|
||||||
|
|
||||||
|
Exception e =
|
||||||
|
assertThrows(
|
||||||
|
IllegalArgumentException.class,
|
||||||
|
() ->
|
||||||
|
runCommandForced(
|
||||||
|
"--client=NewRegistrar",
|
||||||
|
"--statuses=clientRenewProhibited,serverHold",
|
||||||
|
"--superuser",
|
||||||
|
"example.tld"));
|
||||||
|
assertThat(e).hasMessageThat().contains("The domain 'example.tld' has status PENDING_DELETE.");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue