mirror of
https://github.com/google/nomulus.git
synced 2025-08-06 01:35:17 +02:00
Prevent ending package tokens with active domains (#1919)
* Prevent ending package tokens with active domains * Fix bad formatting in comments * Fix lots of nits
This commit is contained in:
sarahcaseybot
GitHub
parent
07b87bbb4d
commit
5e081f4692
4 changed files with 132 additions and 2 deletions
|
|
@ -18,9 +18,11 @@ import static com.google.common.base.Preconditions.checkArgument;
|
|||
import static com.google.common.collect.ImmutableSet.toImmutableSet;
|
||||
import static com.google.common.collect.Sets.difference;
|
||||
import static google.registry.model.billing.BillingEvent.RenewalPriceBehavior.DEFAULT;
|
||||
import static google.registry.model.domain.token.AllocationToken.TokenType.PACKAGE;
|
||||
import static google.registry.model.domain.token.AllocationToken.TokenType.SINGLE_USE;
|
||||
import static google.registry.model.domain.token.AllocationToken.TokenType.UNLIMITED_USE;
|
||||
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
|
||||
import static google.registry.util.CollectionUtils.isNullOrEmpty;
|
||||
import static google.registry.util.CollectionUtils.nullToEmpty;
|
||||
import static google.registry.util.StringGenerator.DEFAULT_PASSWORD_LENGTH;
|
||||
import static java.nio.charset.StandardCharsets.UTF_8;
|
||||
|
|
@ -253,6 +255,22 @@ class GenerateAllocationTokensCommand implements Command {
|
|||
!ImmutableList.of("").equals(allowedTlds),
|
||||
"Either omit --allowed_tlds if all TLDs are allowed, or include a comma-separated list");
|
||||
|
||||
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
|
||||
// tokens should only be scheduled to end with a brief time period before the status
|
||||
// transition occurs so that no new domains are registered using that token between when the
|
||||
// status is scheduled and when the transition occurs.
|
||||
// TODO(@sarahbot): Create a cleaner way to handle ending packages once we actually have
|
||||
// customers using them
|
||||
boolean hasEnding =
|
||||
tokenStatusTransitions.containsValue(TokenStatus.ENDED)
|
||||
|| tokenStatusTransitions.containsValue(TokenStatus.CANCELLED);
|
||||
checkArgument(
|
||||
!(PACKAGE.equals(tokenType) && hasEnding),
|
||||
"PACKAGE tokens should not be generated with ENDED or CANCELLED in their transition map");
|
||||
}
|
||||
|
||||
if (tokenStrings != null) {
|
||||
verifyTokenStringsDoNotExist();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@
|
|||
|
||||
package google.registry.tools;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkArgument;
|
||||
import static com.google.common.collect.ImmutableMap.toImmutableMap;
|
||||
import static com.google.common.collect.ImmutableSet.toImmutableSet;
|
||||
import static com.google.common.collect.Iterables.partition;
|
||||
|
|
@ -31,6 +32,7 @@ import google.registry.model.billing.BillingEvent.RenewalPriceBehavior;
|
|||
import google.registry.model.domain.token.AllocationToken;
|
||||
import google.registry.model.domain.token.AllocationToken.RegistrationBehavior;
|
||||
import google.registry.model.domain.token.AllocationToken.TokenStatus;
|
||||
import google.registry.model.domain.token.AllocationToken.TokenType;
|
||||
import google.registry.tools.params.TransitionListParameter.TokenStatusTransitions;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
|
@ -114,6 +116,7 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens
|
|||
private static final Joiner JOINER = Joiner.on(", ");
|
||||
|
||||
private ImmutableSet<AllocationToken> tokensToSave;
|
||||
private boolean endToken = false;
|
||||
|
||||
@Override
|
||||
public void init() {
|
||||
|
|
@ -126,6 +129,12 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens
|
|||
allowedTlds = ImmutableList.of();
|
||||
}
|
||||
|
||||
if (tokenStatusTransitions != null
|
||||
&& (tokenStatusTransitions.containsValue(TokenStatus.ENDED)
|
||||
|| tokenStatusTransitions.containsValue(TokenStatus.CANCELLED))) {
|
||||
endToken = true;
|
||||
}
|
||||
|
||||
tokensToSave =
|
||||
tm().transact(
|
||||
() ->
|
||||
|
|
@ -157,6 +166,19 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens
|
|||
}
|
||||
|
||||
private AllocationToken updateToken(AllocationToken original) {
|
||||
if (endToken && original.getTokenType().equals(TokenType.PACKAGE)) {
|
||||
Long domainsInPackage =
|
||||
tm().query("SELECT COUNT(*) FROM Domain WHERE currentPackageToken = :token", Long.class)
|
||||
.setParameter("token", original.createVKey())
|
||||
.getSingleResult();
|
||||
|
||||
checkArgument(
|
||||
domainsInPackage == 0,
|
||||
"Package token %s can not end its promotion because it still has %s domains in the"
|
||||
+ " package",
|
||||
original.getToken(),
|
||||
domainsInPackage);
|
||||
}
|
||||
AllocationToken.Builder builder = original.asBuilder();
|
||||
Optional.ofNullable(allowedClientIds)
|
||||
.ifPresent(clientIds -> builder.setAllowedRegistrarIds(ImmutableSet.copyOf(clientIds)));
|
||||
|
|
|
|||
|
|
@ -395,6 +395,26 @@ class GenerateAllocationTokensCommandTest extends CommandTestCase<GenerateAlloca
|
|||
.isInstanceOf(IllegalArgumentException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testFailure_invalidPackageTokenStatusTransition() {
|
||||
assertThat(
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() ->
|
||||
runCommand(
|
||||
"--number",
|
||||
"999",
|
||||
"--type",
|
||||
"PACKAGE",
|
||||
String.format(
|
||||
"--token_status_transitions=\"%s=NOT_STARTED,%s=VALID,%s=ENDED\"",
|
||||
START_OF_TIME, fakeClock.nowUtc(), fakeClock.nowUtc().plusDays(1)))))
|
||||
.hasMessageThat()
|
||||
.isEqualTo(
|
||||
"PACKAGE tokens should not be generated with ENDED or CANCELLED in their transition"
|
||||
+ " map");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testFailure_lengthOfZero() {
|
||||
IllegalArgumentException thrown =
|
||||
|
|
|
|||
|
|
@ -22,9 +22,12 @@ import static google.registry.model.domain.token.AllocationToken.TokenStatus.CAN
|
|||
import static google.registry.model.domain.token.AllocationToken.TokenStatus.ENDED;
|
||||
import static google.registry.model.domain.token.AllocationToken.TokenStatus.NOT_STARTED;
|
||||
import static google.registry.model.domain.token.AllocationToken.TokenStatus.VALID;
|
||||
import static google.registry.model.domain.token.AllocationToken.TokenType.PACKAGE;
|
||||
import static google.registry.model.domain.token.AllocationToken.TokenType.SINGLE_USE;
|
||||
import static google.registry.model.domain.token.AllocationToken.TokenType.UNLIMITED_USE;
|
||||
import static google.registry.testing.DatabaseHelper.createTld;
|
||||
import static google.registry.testing.DatabaseHelper.loadByEntity;
|
||||
import static google.registry.testing.DatabaseHelper.persistActiveDomain;
|
||||
import static google.registry.testing.DatabaseHelper.persistResource;
|
||||
import static google.registry.util.DateTimeUtils.START_OF_TIME;
|
||||
import static org.joda.time.DateTimeZone.UTC;
|
||||
|
|
@ -255,7 +258,7 @@ class UpdateAllocationTokensCommandTest extends CommandTestCase<UpdateAllocation
|
|||
|
||||
@Test
|
||||
void testUpdateStatusTransitions() throws Exception {
|
||||
DateTime now = DateTime.now(UTC);
|
||||
DateTime now = fakeClock.nowUtc();
|
||||
AllocationToken token = persistResource(builderWithPromo().build());
|
||||
runCommandForced(
|
||||
"--prefix",
|
||||
|
|
@ -270,7 +273,7 @@ class UpdateAllocationTokensCommandTest extends CommandTestCase<UpdateAllocation
|
|||
|
||||
@Test
|
||||
void testUpdateStatusTransitions_badTransitions() {
|
||||
DateTime now = DateTime.now(UTC);
|
||||
DateTime now = fakeClock.nowUtc();
|
||||
persistResource(builderWithPromo().build());
|
||||
IllegalArgumentException thrown =
|
||||
assertThrows(
|
||||
|
|
@ -288,6 +291,73 @@ class UpdateAllocationTokensCommandTest extends CommandTestCase<UpdateAllocation
|
|||
.isEqualTo("tokenStatusTransitions map cannot transition from NOT_STARTED to ENDED.");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testUpdateStatusTransitions_endPackageTokenNoDomains() throws Exception {
|
||||
DateTime now = fakeClock.nowUtc();
|
||||
AllocationToken token =
|
||||
persistResource(
|
||||
new AllocationToken.Builder()
|
||||
.setToken("token")
|
||||
.setTokenType(PACKAGE)
|
||||
.setRenewalPriceBehavior(SPECIFIED)
|
||||
.setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar"))
|
||||
.setTokenStatusTransitions(
|
||||
ImmutableSortedMap.<DateTime, TokenStatus>naturalOrder()
|
||||
.put(START_OF_TIME, NOT_STARTED)
|
||||
.put(now.minusDays(1), VALID)
|
||||
.build())
|
||||
.build());
|
||||
runCommandForced(
|
||||
"--prefix",
|
||||
"token",
|
||||
"--token_status_transitions",
|
||||
String.format(
|
||||
"\"%s=NOT_STARTED,%s=VALID,%s=ENDED\"", START_OF_TIME, now.minusDays(1), now));
|
||||
token = reloadResource(token);
|
||||
assertThat(token.getTokenStatusTransitions().toValueMap())
|
||||
.containsExactly(START_OF_TIME, NOT_STARTED, now.minusDays(1), VALID, now, ENDED);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testUpdateStatusTransitions_endPackageTokenWithActiveDomainsFails() throws Exception {
|
||||
DateTime now = fakeClock.nowUtc();
|
||||
AllocationToken token =
|
||||
persistResource(
|
||||
new AllocationToken.Builder()
|
||||
.setToken("token")
|
||||
.setTokenType(PACKAGE)
|
||||
.setRenewalPriceBehavior(SPECIFIED)
|
||||
.setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar"))
|
||||
.setTokenStatusTransitions(
|
||||
ImmutableSortedMap.<DateTime, TokenStatus>naturalOrder()
|
||||
.put(START_OF_TIME, NOT_STARTED)
|
||||
.put(now.minusDays(1), VALID)
|
||||
.build())
|
||||
.build());
|
||||
createTld("tld");
|
||||
persistResource(
|
||||
persistActiveDomain("example.tld")
|
||||
.asBuilder()
|
||||
.setCurrentPackageToken(token.createVKey())
|
||||
.build());
|
||||
IllegalArgumentException thrown =
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() ->
|
||||
runCommandForced(
|
||||
"--prefix",
|
||||
"token",
|
||||
"--token_status_transitions",
|
||||
String.format(
|
||||
"\"%s=NOT_STARTED,%s=VALID,%s=ENDED\"",
|
||||
START_OF_TIME, now.minusDays(1), now)));
|
||||
assertThat(thrown)
|
||||
.hasMessageThat()
|
||||
.isEqualTo(
|
||||
"Package token token can not end its promotion because it still has 1 domains in the"
|
||||
+ " package");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testUpdate_onlyWithPrefix() throws Exception {
|
||||
AllocationToken token =
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue