zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-05-13 07:57:13 +02:00
Code Issues Projects Releases Packages Wiki Activity

Fix DomainTransferRequestFlow to correctly cancel autorenew graces

Browse source 
This fixes longstanding bug b/19430703 in which domain transfers that were
server-approved would only handle the autorenew grace period correctly if
the autorenew grace period was going to start within the transfer window.
If the autorenew grace period was already active (e.g. the domain had
recently autorenewed, before the transfer was requested), the logic would
miss it, even if it was going to be active throughout the transfer window
(i.e. it would still be active at the server-approval time).

When the autorenew grace period is active at the time a transfer is approved
(whether by the server or explicitly via DomainTransferApproveFlow), the
correct behavior is to essentially "cancel" the autorenew - the losing registrar
receives a refund for the autorenew charge, and the gaining registrar's transfer
extended registration years are applied to the expiration time as it was prior
to that autorenew.  The way we implement this is that we just have the transfer
essentially "subsume" the autorenew - we deduct 1 year from the transfer's
extended registration years before extending the registration period from what
the expiration time is post-autorenew at the moment of transfer approval.

See b/19430703#comment17 for details on the policy justification; the only real
ICANN document about this is https://www.icann.org/news/advisory-2002-06-06-en,
but registrars informally document in many places that transfers will trigger
autorenew grace, e.g. see https://support.google.com/domains/answer/3251236

There are still a few parts of this bug that remain unfixed:

  1) RdeDomainImportAction repeats a lot of logic when handling imported domains
     that are in pending transfer, so it will also need to address this case in
     some way, but the policy choices there are unclear so I'm waiting until we
     know more about RDE import goals to figure out how to fix that.

  2) Behavior at the millisecond edge cases is inconsistent - specifically, for
     the case where a transfer is requested such that the automatic transfer
     time is exactly the domain's expiration time (down to the millisecond),
     the correct behavior is a little unclear and this CL for now ignores this
     issue in favor of getting a fix for 99.999% of the issue into prod.  See
     newly created b/35881941 for the gory details.

Also, there are parts of this bug that will be fixed as parts of either
b/25084229 (transfer exDate computations) or b/35110537 (disallowing transfers
with extended registration years other than 1), both of which are less pressing.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=149024269
This commit is contained in:
nickfelt 2017-03-02 11:13:40 -08:00 • committed by Ben McIlwain
parent f663f00251
commit 3a7f67b7f3
7 changed files with 175 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

15
java/google/registry/flows/domain/DomainTransferApproveFlow.java
View file

@ -14,7 +14,6 @@
package google.registry.flows.domain;
import static com.google.common.collect.Iterables.filter;
import static com.google.common.collect.Iterables.getOnlyElement;
import static google.registry.flows.FlowUtils.validateClientIsLoggedIn;
import static google.registry.flows.ResourceFlowUtils.approvePendingTransfer;
@ -32,7 +31,6 @@ import static google.registry.pricing.PricingEngineProxy.getDomainRenewCost;
import static google.registry.util.DateTimeUtils.END_OF_TIME;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableSet;
import com.googlecode.objectify.Key;
import google.registry.flows.EppException;
@ -123,16 +121,9 @@ public final class DomainTransferApproveFlow implements TransactionalFlow {
.setParent(historyEntry)
.build();
// If we are within an autorenew grace period, cancel the autorenew billing event and reduce
// the number of years to extend the registration by one.
GracePeriod autorenewGrace = getOnlyElement(
filter(
existingDomain.getGracePeriods(),
new Predicate<GracePeriod>() {
@Override
public boolean apply(GracePeriod gracePeriod) {
return GracePeriodStatus.AUTO_RENEW.equals(gracePeriod.getType());
}}),
null);
// the number of years to extend the registration by one to "subsume" the autorenew.
GracePeriod autorenewGrace =
getOnlyElement(existingDomain.getGracePeriodsOfType(GracePeriodStatus.AUTO_RENEW), null);
if (autorenewGrace != null) {
extraYears--;
ofy().save().entity(

23
java/google/registry/flows/domain/DomainTransferRequestFlow.java
View file

@ -50,6 +50,7 @@ import google.registry.model.domain.Period;
import google.registry.model.domain.fee.FeeTransferCommandExtension;
import google.registry.model.domain.fee.FeeTransformResponseExtension;
import google.registry.model.domain.metadata.MetadataExtension;
import google.registry.model.domain.rgp.GracePeriodStatus;
import google.registry.model.eppcommon.AuthInfo;
import google.registry.model.eppcommon.StatusValue;
import google.registry.model.eppcommon.Trid;
@ -138,9 +139,27 @@ public final class DomainTransferRequestFlow implements TransactionalFlow {
validateFeeChallenge(targetId, tld, now, feeTransfer, feesAndCredits);
HistoryEntry historyEntry = buildHistory(period, existingDomain, now);
DateTime automaticTransferTime = now.plus(registry.getAutomaticTransferLength());
// If the domain will be in the auto-renew grace period at the moment of transfer, the transfer
// will subsume the autorenew, so we reduce by 1 the number of years to extend the registration.
// Note that the regular "years" remains the same since it affects the transfer charge amount.
// The gaining registrar is still billed for the full years; the losing registrar will get a
// cancellation for the autorenew written out within createTransferServerApproveEntities().
//
// See b/19430703#comment17 and https://www.icann.org/news/advisory-2002-06-06-en for the
// policy documentation for transfers subsuming autorenews within the autorenew grace period.
int registrationExtensionYears = years;
DomainResource domainAtTransferTime =
existingDomain.cloneProjectedAtTime(automaticTransferTime);
if (!domainAtTransferTime.getGracePeriodsOfType(GracePeriodStatus.AUTO_RENEW).isEmpty()) {
registrationExtensionYears--;
}
// The new expiration time if there is a server approval.
DateTime serverApproveNewExpirationTime = extendRegistrationWithCap(
automaticTransferTime, existingDomain.getRegistrationExpirationTime(), years);
DateTime serverApproveNewExpirationTime =
extendRegistrationWithCap(
automaticTransferTime,
domainAtTransferTime.getRegistrationExpirationTime(),
registrationExtensionYears);
// Create speculative entities in anticipation of an automatic server approval.
ImmutableSet<TransferServerApproveEntity> serverApproveEntities =
createTransferServerApproveEntities(

46
java/google/registry/flows/domain/DomainTransferUtils.java
View file

@ -26,6 +26,8 @@ import google.registry.model.billing.BillingEvent;
import google.registry.model.billing.BillingEvent.Flag;
import google.registry.model.billing.BillingEvent.Reason;
import google.registry.model.domain.DomainResource;
import google.registry.model.domain.GracePeriod;
import google.registry.model.domain.rgp.GracePeriodStatus;
import google.registry.model.eppcommon.Trid;
import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse;
import google.registry.model.poll.PollMessage;
@ -39,7 +41,6 @@ import google.registry.model.transfer.TransferStatus;
import javax.annotation.Nullable;
import org.joda.money.Money;
import org.joda.time.DateTime;
import org.joda.time.Duration;
/**
* Utility logic for facilitating domain transfers.
@ -217,34 +218,33 @@ public final class DomainTransferUtils {
/**
* Creates an optional autorenew cancellation if one would apply to the server-approved transfer.
*
* <p>If there will be an autorenew between now and the automatic transfer time, and if the
* autorenew grace period length is long enough that the domain will still be within it at the
* automatic transfer time, then the transfer will subsume the autorenew and we need to write out
* a cancellation for it.
* <p>If the domain will be in the auto-renew grace period at the automatic transfer time, then
* the transfer will subsume the autorenew. This means that we "cancel" the 1-year extension of
* the autorenew before applying the extra transfer years, which in effect means reducing the
* transfer extended registration years by one. Since the gaining registrar will still be billed
* for the full extended registration years, we must issue a cancellation for the autorenew, so
* that the losing registrar will not be charged (essentially, the gaining registrar takes on the
* cost of the year of registration that the autorenew just added).
*
* <p>For details on the policy justification, see b/19430703#comment17 and
* <a href="https://www.icann.org/news/advisory-2002-06-06-en">this ICANN advisory</a>.
*/
// TODO(b/19430703): the above logic is incomplete; it doesn't handle a grace period that started
// before the transfer was requested and continues through the automatic transfer time.
private static Optional<BillingEvent.Cancellation> createOptionalAutorenewCancellation(
DateTime automaticTransferTime,
HistoryEntry historyEntry,
String targetId,
DomainResource existingDomain) {
Registry registry = Registry.get(existingDomain.getTld());
DateTime oldExpirationTime = existingDomain.getRegistrationExpirationTime();
Duration autoRenewGracePeriodLength = registry.getAutoRenewGracePeriodLength();
if (automaticTransferTime.isAfter(oldExpirationTime)
&& automaticTransferTime.isBefore(oldExpirationTime.plus(autoRenewGracePeriodLength))) {
return Optional.of(new BillingEvent.Cancellation.Builder()
.setReason(Reason.RENEW)
.setFlags(ImmutableSet.of(Flag.AUTO_RENEW))
.setTargetId(targetId)
.setClientId(existingDomain.getCurrentSponsorClientId())
.setEventTime(automaticTransferTime)
.setBillingTime(existingDomain.getRegistrationExpirationTime()
.plus(registry.getAutoRenewGracePeriodLength()))
.setRecurringEventKey(existingDomain.getAutorenewBillingEvent())
.setParent(historyEntry)
.build());
DomainResource domainAtTransferTime =
existingDomain.cloneProjectedAtTime(automaticTransferTime);
GracePeriod autorenewGracePeriod =
getOnlyElement(
domainAtTransferTime.getGracePeriodsOfType(GracePeriodStatus.AUTO_RENEW), null);
if (autorenewGracePeriod != null) {
return Optional.of(
BillingEvent.Cancellation.forGracePeriod(autorenewGracePeriod, historyEntry, targetId)
.asBuilder()
.setEventTime(automaticTransferTime)
.build());
}
return Optional.absent();
}

91
javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java
View file

@ -39,6 +39,7 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Iterables;
import com.googlecode.objectify.Key;
import google.registry.flows.ResourceFlowUtils.BadAuthInfoForResourceException;
import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
import google.registry.flows.domain.DomainFlowUtils.BadPeriodUnitException;
@ -55,7 +56,6 @@ import google.registry.flows.exceptions.ObjectAlreadySponsoredException;
import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
import google.registry.model.billing.BillingEvent;
import google.registry.model.billing.BillingEvent.Cancellation.Builder;
import google.registry.model.billing.BillingEvent.Flag;
import google.registry.model.billing.BillingEvent.Reason;
import google.registry.model.contact.ContactAuthInfo;
import google.registry.model.domain.DomainAuthInfo;
@ -579,28 +579,97 @@ public class DomainTransferRequestFlowTest
}
@Test
public void testSuccess_autorenewBeforeAutomaticTransfer() throws Exception {
public void testSuccess_autorenewGraceActive_onlyAtTransferRequestTime() throws Exception {
setupDomain("example", "tld");
DomainResource oldResource = persistResource(reloadResourceByForeignKey().asBuilder()
.setRegistrationExpirationTime(clock.nowUtc().plusDays(1).plus(1))
// Set the domain to have auto-renewed long enough ago that it is still in the autorenew grace
// period at the transfer request time, but will have exited it by the automatic transfer time.
DateTime autorenewTime =
clock.nowUtc().minus(Registry.get("tld").getAutoRenewGracePeriodLength()).plusDays(1);
DateTime expirationTime = autorenewTime.plusYears(1);
domain = persistResource(domain.asBuilder()
.setRegistrationExpirationTime(expirationTime)
.addGracePeriod(GracePeriod.createForRecurring(
GracePeriodStatus.AUTO_RENEW,
autorenewTime.plus(Registry.get("tld").getAutoRenewGracePeriodLength()),
"TheRegistrar",
domain.getAutorenewBillingEvent()))
.build());
clock.advanceOneMilli();
// The autorenew should be subsumed into the transfer resulting in 2 years of renewal in total.
// Since the autorenew grace period will have ended by the automatic transfer time, subsuming
// will not occur in the server-approve case, so the transfer will add 1 year to the current
// expiration time as usual, and no Cancellation will be issued. Note however that if the
// transfer were to be manually approved before the autorenew grace period ends, then the
// DomainTransferApproveFlow will still issue a Cancellation.
doSuccessfulTest(
"domain_transfer_request_2_years.xml",
"domain_transfer_request_response_2_years.xml",
clock.nowUtc().plusDays(1).plusYears(2),
"domain_transfer_request.xml",
"domain_transfer_request_response_autorenew_grace_at_request_only.xml",
expirationTime.plusYears(1));
}
@Test
public void testSuccess_autorenewGraceActive_throughoutTransferWindow() throws Exception {
setupDomain("example", "tld");
Key<BillingEvent.Recurring> existingAutorenewEvent =
domain.getAutorenewBillingEvent();
// Set domain to have auto-renewed just before the transfer request, so that it will have an
// active autorenew grace period spanning the entire transfer window.
DateTime autorenewTime = clock.nowUtc().minusDays(1);
DateTime expirationTime = autorenewTime.plusYears(1);
domain = persistResource(domain.asBuilder()
.setRegistrationExpirationTime(expirationTime)
.addGracePeriod(GracePeriod.createForRecurring(
GracePeriodStatus.AUTO_RENEW,
autorenewTime.plus(Registry.get("tld").getAutoRenewGracePeriodLength()),
"TheRegistrar",
existingAutorenewEvent))
.build());
clock.advanceOneMilli();
// The transfer will subsume the recent autorenew, so there will be no net change in expiration
// time caused by the transfer, but we must write a Cancellation.
doSuccessfulTest(
"domain_transfer_request.xml",
"domain_transfer_request_response_autorenew_grace_throughout_transfer_window.xml",
expirationTime,
new BillingEvent.Cancellation.Builder()
.setReason(Reason.RENEW)
.setFlags(ImmutableSet.of(Flag.AUTO_RENEW))
.setTargetId("example.tld")
.setClientId("TheRegistrar")
// The cancellation happens at the moment of transfer.
.setEventTime(clock.nowUtc().plus(Registry.get("tld").getAutomaticTransferLength()))
.setBillingTime(oldResource.getRegistrationExpirationTime().plus(
.setBillingTime(autorenewTime.plus(
Registry.get("tld").getAutoRenewGracePeriodLength()))
// The cancellation should refer to the old autorenew billing event.
.setRecurringEventKey(oldResource.getAutorenewBillingEvent()));
.setRecurringEventKey(existingAutorenewEvent));
}
@Test
public void testSuccess_autorenewGraceActive_onlyAtAutomaticTransferTime() throws Exception {
setupDomain("example", "tld");
Key<BillingEvent.Recurring> existingAutorenewEvent =
domain.getAutorenewBillingEvent();
// Set domain to expire in 1 day, so that it will be in the autorenew grace period by the
// automatic transfer time, even though it isn't yet.
DateTime expirationTime = clock.nowUtc().plusDays(1);
domain = persistResource(domain.asBuilder()
.setRegistrationExpirationTime(expirationTime)
.build());
clock.advanceOneMilli();
// The transfer will subsume the future autorenew, meaning that the expected server-approve
// expiration time will be 1 year beyond the current one, and we must write a Cancellation.
doSuccessfulTest(
"domain_transfer_request.xml",
"domain_transfer_request_response_autorenew_grace_at_transfer_only.xml",
expirationTime.plusYears(1),
new BillingEvent.Cancellation.Builder()
.setReason(Reason.RENEW)
.setTargetId("example.tld")
.setClientId("TheRegistrar")
// The cancellation happens at the moment of transfer.
.setEventTime(clock.nowUtc().plus(Registry.get("tld").getAutomaticTransferLength()))
.setBillingTime(
expirationTime.plus(Registry.get("tld").getAutoRenewGracePeriodLength()))
// The cancellation should refer to the old autorenew billing event.
.setRecurringEventKey(existingAutorenewEvent));
}
@Test

2
javatests/google/registry/flows/domain/testdata/domain_transfer_request_response_2_years.xml → javatests/google/registry/flows/domain/testdata/domain_transfer_request_response_autorenew_grace_at_request_only.xml vendored
View file

@ -12,7 +12,7 @@
<domain:reDate>2000-06-09T22:00:00.0Z</domain:reDate>
<domain:acID>TheRegistrar</domain:acID>
<domain:acDate>2000-06-14T22:00:00.0Z</domain:acDate>
<domain:exDate>2002-06-10T22:00:00.0Z</domain:exDate>
<domain:exDate>2002-04-26T22:00:00.0Z</domain:exDate>
</domain:trnData>
</resData>
<trID>

23
javatests/google/registry/flows/domain/testdata/domain_transfer_request_response_autorenew_grace_at_transfer_only.xml vendored Normal file
View file

@ -0,0 +1,23 @@
<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
<response>
<result code="1001">
<msg>Command completed successfully; action pending</msg>
</result>
<resData>
<domain:trnData
xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
<domain:name>example.tld</domain:name>
<domain:trStatus>pending</domain:trStatus>
<domain:reID>NewRegistrar</domain:reID>
<domain:reDate>2000-06-09T22:00:00.0Z</domain:reDate>
<domain:acID>TheRegistrar</domain:acID>
<domain:acDate>2000-06-14T22:00:00.0Z</domain:acDate>
<domain:exDate>2001-06-10T22:00:00.0Z</domain:exDate>
</domain:trnData>
</resData>
<trID>
<clTRID>ABC-12345</clTRID>
<svTRID>server-trid</svTRID>
</trID>
</response>
</epp>

24
javatests/google/registry/flows/domain/testdata/domain_transfer_request_response_autorenew_grace_throughout_transfer_window.xml vendored Normal file
View file

@ -0,0 +1,24 @@
<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
<response>
<result code="1001">
<msg>Command completed successfully; action pending</msg>
</result>
<resData>
<domain:trnData
xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
<domain:name>example.tld</domain:name>
<domain:trStatus>pending</domain:trStatus>
<domain:reID>NewRegistrar</domain:reID>
<domain:reDate>2000-06-09T22:00:00.0Z</domain:reDate>
<domain:acID>TheRegistrar</domain:acID>
<domain:acDate>2000-06-14T22:00:00.0Z</domain:acDate>
<!-- TODO(b/25084229): Exdate should be 2001; needs fixing. -->
<domain:exDate>2002-06-08T22:00:00.0Z</domain:exDate>
</domain:trnData>
</resData>
<trID>
<clTRID>ABC-12345</clTRID>
<svTRID>server-trid</svTRID>
</trID>
</response>
</epp>