Add tool changes to modify allowedEppActions on allocation tokens (#1970)

* Add tool changes to modify allowedEppActions on allocation tokens

* Change enum value error message

* Remove unnecessary variable

* Prevent UNKNOWN command

* Check command name instead of string
This commit is contained in:
sarahcaseybot 2023-03-31 14:37:19 -04:00 committed by GitHub
parent a549d537db
commit fba137f1bb
5 changed files with 106 additions and 2 deletions

View file

@ -14,6 +14,8 @@
package google.registry.model.domain.fee;
import static com.google.common.base.Preconditions.checkArgument;
import google.registry.model.ImmutableObject;
import google.registry.model.domain.Period;
import java.util.Optional;
@ -37,7 +39,19 @@ public abstract class FeeQueryCommandExtensionItem extends ImmutableObject {
RENEW,
TRANSFER,
RESTORE,
UPDATE
UPDATE;
public static CommandName parseKnownCommand(String string) {
try {
CommandName command = valueOf(string);
checkArgument(!command.equals(UNKNOWN));
return command;
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(
"Invalid EPP action name. Valid actions are CREATE, RENEW, TRANSFER, RESTORE, and"
+ " UPDATE");
}
}
}
/** The default validity period (if not specified) is 1 year for all operations. */

View file

@ -39,6 +39,7 @@ import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.common.io.Files;
import google.registry.model.billing.BillingEvent.RenewalPriceBehavior;
import google.registry.model.domain.fee.FeeQueryCommandExtensionItem.CommandName;
import google.registry.model.domain.token.AllocationToken;
import google.registry.model.domain.token.AllocationToken.RegistrationBehavior;
import google.registry.model.domain.token.AllocationToken.TokenStatus;
@ -114,6 +115,11 @@ class GenerateAllocationTokensCommand implements Command {
description = "Comma-separated list of allowed TLDs, or null if all are allowed")
private List<String> allowedTlds;
@Parameter(
names = {"--allowed_epp_actions"},
description = "Comma-separated list of allowed EPP actions, or null if all are allowed")
private List<String> allowedEppActions;
@Parameter(
names = {"--discount_fraction"},
description =
@ -207,7 +213,13 @@ class GenerateAllocationTokensCommand implements Command {
.setTokenType(tokenType == null ? SINGLE_USE : tokenType)
.setAllowedRegistrarIds(
ImmutableSet.copyOf(nullToEmpty(allowedClientIds)))
.setAllowedTlds(ImmutableSet.copyOf(nullToEmpty(allowedTlds)));
.setAllowedTlds(ImmutableSet.copyOf(nullToEmpty(allowedTlds)))
.setAllowedEppActions(
isNullOrEmpty(allowedEppActions)
? ImmutableSet.of()
: allowedEppActions.stream()
.map(CommandName::parseKnownCommand)
.collect(toImmutableSet()));
Optional.ofNullable(discountFraction).ifPresent(token::setDiscountFraction);
Optional.ofNullable(discountPremiums).ifPresent(token::setDiscountPremiums);
Optional.ofNullable(discountYears).ifPresent(token::setDiscountYears);
@ -255,6 +267,10 @@ class GenerateAllocationTokensCommand implements Command {
!ImmutableList.of("").equals(allowedTlds),
"Either omit --allowed_tlds if all TLDs are allowed, or include a comma-separated list");
if (ImmutableList.of("").equals(allowedEppActions)) {
allowedEppActions = ImmutableList.of();
}
if (!isNullOrEmpty(tokenStatusTransitions)) {
// Don't allow package tokens to be created with a scheduled end time since this could allow
// future domains to be attributed to the package and never be billed. Package promotion

View file

@ -29,6 +29,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import google.registry.model.billing.BillingEvent.RenewalPriceBehavior;
import google.registry.model.domain.fee.FeeQueryCommandExtensionItem.CommandName;
import google.registry.model.domain.token.AllocationToken;
import google.registry.model.domain.token.AllocationToken.RegistrationBehavior;
import google.registry.model.domain.token.AllocationToken.TokenStatus;
@ -63,6 +64,13 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens
+ "existing list.")
private List<String> allowedTlds;
@Parameter(
names = {"--allowed_epp_actions"},
description =
"Comma-separated list of allowed EPP actions. Use an empty string to clear the existing"
+ " list.")
private List<String> allowedEppActions;
@Parameter(
names = {"--discount_fraction"},
description =
@ -128,6 +136,9 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens
if (ImmutableList.of("").equals(allowedTlds)) {
allowedTlds = ImmutableList.of();
}
if (ImmutableList.of("").equals(allowedEppActions)) {
allowedEppActions = ImmutableList.of();
}
if (tokenStatusTransitions != null
&& (tokenStatusTransitions.containsValue(TokenStatus.ENDED)
@ -184,6 +195,14 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens
.ifPresent(clientIds -> builder.setAllowedRegistrarIds(ImmutableSet.copyOf(clientIds)));
Optional.ofNullable(allowedTlds)
.ifPresent(tlds -> builder.setAllowedTlds(ImmutableSet.copyOf(tlds)));
Optional.ofNullable(allowedEppActions)
.ifPresent(
eppActions -> {
builder.setAllowedEppActions(
eppActions.stream()
.map(CommandName::parseKnownCommand)
.collect(toImmutableSet()));
});
Optional.ofNullable(discountFraction).ifPresent(builder::setDiscountFraction);
Optional.ofNullable(discountPremiums).ifPresent(builder::setDiscountPremiums);
Optional.ofNullable(discountYears).ifPresent(builder::setDiscountYears);

View file

@ -35,6 +35,7 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Iterables;
import com.google.common.io.Files;
import google.registry.model.domain.fee.FeeQueryCommandExtensionItem.CommandName;
import google.registry.model.domain.token.AllocationToken;
import google.registry.model.domain.token.AllocationToken.TokenStatus;
import google.registry.model.reporting.HistoryEntry.HistoryEntryId;
@ -136,6 +137,7 @@ class GenerateAllocationTokensCommandTest extends CommandTestCase<GenerateAlloca
"--type", "UNLIMITED_USE",
"--allowed_client_ids", "TheRegistrar,NewRegistrar",
"--allowed_tlds", "tld,example",
"--allowed_epp_actions", "CREATE,RENEW",
"--discount_fraction", "0.5",
"--discount_premiums", "true",
"--discount_years", "6",
@ -148,6 +150,7 @@ class GenerateAllocationTokensCommandTest extends CommandTestCase<GenerateAlloca
.setTokenType(UNLIMITED_USE)
.setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar", "NewRegistrar"))
.setAllowedTlds(ImmutableSet.of("tld", "example"))
.setAllowedEppActions(ImmutableSet.of(CommandName.CREATE, CommandName.RENEW))
.setDiscountFraction(0.5)
.setDiscountPremiums(true)
.setDiscountYears(6)

View file

@ -36,6 +36,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import com.beust.jcommander.ParameterException;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import google.registry.model.domain.fee.FeeQueryCommandExtensionItem.CommandName;
import google.registry.model.domain.token.AllocationToken;
import google.registry.model.domain.token.AllocationToken.RegistrationBehavior;
import google.registry.model.domain.token.AllocationToken.TokenStatus;
@ -80,6 +81,57 @@ class UpdateAllocationTokensCommandTest extends CommandTestCase<UpdateAllocation
assertThat(reloadResource(token).getAllowedRegistrarIds()).isEmpty();
}
@Test
void testUpdateEppActions_setEppActions() throws Exception {
AllocationToken token =
persistResource(
builderWithPromo().setAllowedEppActions(ImmutableSet.of(CommandName.CREATE)).build());
runCommandForced("--prefix", "token", "--allowed_epp_actions", "RENEW,RESTORE");
assertThat(reloadResource(token).getAllowedEppActions())
.containsExactly(CommandName.RENEW, CommandName.RESTORE);
}
@Test
void testUpdateEppActions_clearEppActions() throws Exception {
AllocationToken token =
persistResource(
builderWithPromo()
.setAllowedEppActions(ImmutableSet.of(CommandName.CREATE, CommandName.RENEW))
.build());
runCommandForced("--prefix", "token", "--allowed_epp_actions", "");
assertThat(reloadResource(token).getAllowedEppActions()).isEmpty();
}
@Test
void testUpdateEppActions_invalidEppAction() throws Exception {
persistResource(
builderWithPromo().setAllowedEppActions(ImmutableSet.of(CommandName.CREATE)).build());
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() -> runCommandForced("--prefix", "token", "--allowed_epp_actions", "FAKE"));
assertThat(thrown)
.hasMessageThat()
.isEqualTo(
"Invalid EPP action name. Valid actions are CREATE, RENEW, TRANSFER, RESTORE, and"
+ " UPDATE");
}
@Test
void testUpdateEppActions_unknownEppAction() throws Exception {
persistResource(
builderWithPromo().setAllowedEppActions(ImmutableSet.of(CommandName.CREATE)).build());
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() -> runCommandForced("--prefix", "token", "--allowed_epp_actions", "UNKNOWN"));
assertThat(thrown)
.hasMessageThat()
.isEqualTo(
"Invalid EPP action name. Valid actions are CREATE, RENEW, TRANSFER, RESTORE, and"
+ " UPDATE");
}
@Test
void testUpdateDiscountFraction() throws Exception {
AllocationToken token = persistResource(builderWithPromo().setDiscountFraction(0.5).build());