From ec4ffe53f0f0c5baf060a533973ab3a0804d7148 Mon Sep 17 00:00:00 2001 From: nickfelt Date: Thu, 23 Mar 2017 07:51:08 -0700 Subject: [PATCH] Clean up flow validation of registration periods This tidies up some logic in the flows that checks registration periods, so that in the create flows we're consistently checking that the requested number of years is <= 10 right away (DomainCreateFlow was deferring it until very late, including after custom logic ran, for no good reason I can see). It also refactors the validateRegistrationPeriod() overload used by DomainRenewFlow to take the newExpirationTime directly, and just check to ensure that it's >= to now.plusYears(10) (with leap-safety just in case). This is a much simpler check than before, which recomputed the newExpirationTime separately from the logic used by DomainRenewFlow itself (always dangerous) and did a more convoluted and unnecessary comparison involving extendRegistrationWithCap(). ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=151002960 --- .../registry/flows/domain/DomainAllocateFlow.java | 2 +- .../flows/domain/DomainApplicationCreateFlow.java | 4 ++-- .../registry/flows/domain/DomainCreateFlow.java | 2 +- .../registry/flows/domain/DomainFlowUtils.java | 14 +++++--------- .../registry/flows/domain/DomainRenewFlow.java | 6 +++--- 5 files changed, 12 insertions(+), 16 deletions(-) diff --git a/java/google/registry/flows/domain/DomainAllocateFlow.java b/java/google/registry/flows/domain/DomainAllocateFlow.java index 6837915ea..a89b8ca34 100644 --- a/java/google/registry/flows/domain/DomainAllocateFlow.java +++ b/java/google/registry/flows/domain/DomainAllocateFlow.java @@ -144,8 +144,8 @@ public class DomainAllocateFlow implements TransactionalFlow { InternetDomainName domainName = validateDomainName(command.getFullyQualifiedDomainName()); Registry registry = Registry.get(domainName.parent().toString()); Period period = command.getPeriod(); - Integer years = period.getValue(); verifyUnitIsYears(period); + int years = period.getValue(); validateRegistrationPeriod(years); validateCreateCommandContactsAndNameservers(command, registry, domainName); SecDnsCreateExtension secDnsCreate = diff --git a/java/google/registry/flows/domain/DomainApplicationCreateFlow.java b/java/google/registry/flows/domain/DomainApplicationCreateFlow.java index 094cb69da..729af9dc9 100644 --- a/java/google/registry/flows/domain/DomainApplicationCreateFlow.java +++ b/java/google/registry/flows/domain/DomainApplicationCreateFlow.java @@ -208,8 +208,6 @@ public final class DomainApplicationCreateFlow implements TransactionalFlow { Registry registry = Registry.get(tld); FeesAndCredits feesAndCredits = pricingLogic.getCreatePrice(registry, targetId, now, command.getPeriod().getValue()); - // Superusers can create reserved domains, force creations on domains that require a claims - // notice without specifying a claims key, and override blocks on registering premium domains. verifyUnitIsYears(command.getPeriod()); int years = command.getPeriod().getValue(); validateRegistrationPeriod(years); @@ -220,6 +218,8 @@ public final class DomainApplicationCreateFlow implements TransactionalFlow { } boolean isAnchorTenant = matchesAnchorTenantReservation(domainName, authInfo.getPw().getValue()); + // Superusers can create reserved domains, force creations on domains that require a claims + // notice without specifying a claims key, and override blocks on registering premium domains. if (!isSuperuser) { verifyPremiumNameIsNotBlocked(targetId, now, clientId); prohibitLandrushIfExactlyOneSunrise(registry, now); diff --git a/java/google/registry/flows/domain/DomainCreateFlow.java b/java/google/registry/flows/domain/DomainCreateFlow.java index b1de7ca98..e5f238329 100644 --- a/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/java/google/registry/flows/domain/DomainCreateFlow.java @@ -190,6 +190,7 @@ public class DomainCreateFlow implements TransactionalFlow { Period period = command.getPeriod(); verifyUnitIsYears(period); int years = period.getValue(); + validateRegistrationPeriod(years); failfastForCreate(targetId, now); verifyResourceDoesNotExist(DomainResource.class, targetId, now); // Validate that this is actually a legal domain name on a TLD that the registrar has access to. @@ -248,7 +249,6 @@ public class DomainCreateFlow implements TransactionalFlow { validateSecDnsExtension(eppInput.getSingleExtension(SecDnsCreateExtension.class)); String repoId = createDomainRepoId(ObjectifyService.allocateId(), registry.getTldStr()); DateTime registrationExpirationTime = leapSafeAddYears(now, years); - validateRegistrationPeriod(years); HistoryEntry historyEntry = buildHistory(repoId, period, now); // Bill for the create. BillingEvent.OneTime createBillingEvent = diff --git a/java/google/registry/flows/domain/DomainFlowUtils.java b/java/google/registry/flows/domain/DomainFlowUtils.java index 5727a86fd..cd4bec2d3 100644 --- a/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/java/google/registry/flows/domain/DomainFlowUtils.java @@ -24,7 +24,6 @@ import static com.google.common.collect.Sets.union; import static google.registry.flows.domain.DomainPricingLogic.getMatchingLrpToken; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.domain.DomainResource.MAX_REGISTRATION_YEARS; -import static google.registry.model.domain.DomainResource.extendRegistrationWithCap; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.registry.Registries.findTldForName; import static google.registry.model.registry.label.ReservationType.NAMESERVER_RESTRICTED; @@ -668,17 +667,14 @@ public class DomainFlowUtils { } /** - * Check whether a new registration period (via a renew) does not extend beyond a maximum number - * of years (e.g. {@link DomainResource#MAX_REGISTRATION_YEARS}). + * Check whether a new expiration time (via a renew) does not extend beyond a maximum number of + * years (e.g. {@link DomainResource#MAX_REGISTRATION_YEARS}) from "now". * * @throws ExceedsMaxRegistrationYearsException if the new registration period is too long */ - public static void validateRegistrationPeriod( - DateTime now, - DateTime oldExpirationTime, - int years) throws EppException { - DateTime newExpirationTime = leapSafeAddYears(oldExpirationTime, years); // uncapped - if (extendRegistrationWithCap(now, oldExpirationTime, years).isBefore(newExpirationTime)) { + public static void validateRegistrationPeriod(DateTime now, DateTime newExpirationTime) + throws EppException { + if (leapSafeAddYears(now, MAX_REGISTRATION_YEARS).isBefore(newExpirationTime)) { throw new ExceedsMaxRegistrationYearsException(); } } diff --git a/java/google/registry/flows/domain/DomainRenewFlow.java b/java/google/registry/flows/domain/DomainRenewFlow.java index 528c50faa..b4ef74d86 100644 --- a/java/google/registry/flows/domain/DomainRenewFlow.java +++ b/java/google/registry/flows/domain/DomainRenewFlow.java @@ -133,6 +133,9 @@ public final class DomainRenewFlow implements TransactionalFlow { DomainResource existingDomain = loadAndVerifyExistence(DomainResource.class, targetId, now); verifyRenewAllowed(authInfo, existingDomain, command); int years = command.getPeriod().getValue(); + DateTime newExpirationTime = + leapSafeAddYears(existingDomain.getRegistrationExpirationTime(), years); // Uncapped + validateRegistrationPeriod(now, newExpirationTime); FeeRenewCommandExtension feeRenew = eppInput.getSingleExtension(FeeRenewCommandExtension.class); FeesAndCredits feesAndCredits = @@ -150,9 +153,6 @@ public final class DomainRenewFlow implements TransactionalFlow { .setModificationTime(now) .setParent(Key.create(existingDomain)) .build(); - DateTime oldExpirationTime = existingDomain.getRegistrationExpirationTime(); - DateTime newExpirationTime = leapSafeAddYears(oldExpirationTime, years); // Uncapped - validateRegistrationPeriod(now, oldExpirationTime, years); String tld = existingDomain.getTld(); // Bill for this explicit renew itself. BillingEvent.OneTime explicitRenewEvent =