From b5ae37c5ccfa3f19b32fbc3a0a13721b4e8641b3 Mon Sep 17 00:00:00 2001 From: guyben Date: Thu, 22 Mar 2018 08:39:47 -0700 Subject: [PATCH] Return more informative errors when signed mark is invalid at this time A "mark" tells us that the holder owns the trademark for a given domain name. It is signed for authentication. If the signature's certificate is either "not yet valid" or "expired", we return explicit errors to that effect. But in addition to the signature's certificate, the mark itself might not be valid yet or already expired. Right now if that happens - we return an error saying "the mark doesn't match the domain name". That is wrong - as the mark can match the domain name, just be expired. Returning "the mark doesn't match the domain name" in that case is misleading. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=190069976 --- docs/flows.md | 4 +++ .../domain/DomainApplicationCreateFlow.java | 4 ++- .../flows/domain/DomainCreateFlow.java | 2 ++ .../flows/domain/DomainFlowTmchUtils.java | 28 ++++++++++++++++--- .../DomainApplicationCreateFlowTest.java | 6 ++-- .../flows/domain/DomainCreateFlowTest.java | 24 ++++++++++++++++ 6 files changed, 61 insertions(+), 7 deletions(-) diff --git a/docs/flows.md b/docs/flows.md index d0bc2ff51..263075e55 100644 --- a/docs/flows.md +++ b/docs/flows.md @@ -330,6 +330,8 @@ An EPP flow that creates a new application for a domain resource. phase. * Domain labels cannot begin with a dash. * Missing type attribute for contact. + * The provided mark is not yet valid. + * The provided mark has expired. * Signed marks must be encoded. * Certificate used in signed mark signature has expired. * Certificate parsing error, or possibly a bad provider or algorithm. @@ -580,6 +582,8 @@ An EPP flow that creates a new domain resource. * 2305 * The allocation token was already redeemed. * 2306 + * The provided mark is not yet valid. + * The provided mark has expired. * Domain names can only contain a-z, 0-9, '.' and '-'. * Periods for domain registrations must be specified in years. * The requested fees cannot be provided in the requested currency. diff --git a/java/google/registry/flows/domain/DomainApplicationCreateFlow.java b/java/google/registry/flows/domain/DomainApplicationCreateFlow.java index f55e6643a..3139d0325 100644 --- a/java/google/registry/flows/domain/DomainApplicationCreateFlow.java +++ b/java/google/registry/flows/domain/DomainApplicationCreateFlow.java @@ -144,6 +144,8 @@ import org.joda.time.DateTime; * @error {@link NameserversNotSpecifiedForNameserverRestrictedDomainException} * @error {@link NameserversNotSpecifiedForTldWithNameserverWhitelistException} * @error {@link DomainFlowTmchUtils.NoMarksFoundMatchingDomainException} + * @error {@link DomainFlowTmchUtils.FoundMarkNotYetValidException} + * @error {@link DomainFlowTmchUtils.FoundMarkExpiredException} * @error {@link DomainFlowUtils.NotAuthorizedForTldException} * @error {@link DomainFlowUtils.PremiumNameBlockedException} * @error {@link DomainFlowUtils.RegistrantNotAllowedException} @@ -167,7 +169,7 @@ import org.joda.time.DateTime; * @error {@link DomainFlowUtils.UnsupportedFeeAttributeException} * @error {@link DomainFlowUtils.UnsupportedMarkTypeException} */ -@ReportingSpec(ActivityReportField.DOMAIN_CREATE) // Applications are technically domains in EPP. +@ReportingSpec(ActivityReportField.DOMAIN_CREATE) // Applications are technically domains in EPP. public final class DomainApplicationCreateFlow implements TransactionalFlow { @Inject ExtensionManager extensionManager; diff --git a/java/google/registry/flows/domain/DomainCreateFlow.java b/java/google/registry/flows/domain/DomainCreateFlow.java index a2837d259..a3bbcde66 100644 --- a/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/java/google/registry/flows/domain/DomainCreateFlow.java @@ -124,6 +124,8 @@ import org.joda.time.Duration; * @error {@link DomainCreateFlow.MustHaveSignedMarksInCurrentPhaseException} * @error {@link DomainCreateFlow.NoGeneralRegistrationsInCurrentPhaseException} * @error {@link DomainFlowTmchUtils.NoMarksFoundMatchingDomainException} + * @error {@link DomainFlowTmchUtils.FoundMarkNotYetValidException} + * @error {@link DomainFlowTmchUtils.FoundMarkExpiredException} * @error {@link DomainFlowUtils.NotAuthorizedForTldException} * @error {@link DomainFlowUtils.AcceptedTooLongAgoException} * @error {@link DomainFlowUtils.BadDomainNameCharacterException} diff --git a/java/google/registry/flows/domain/DomainFlowTmchUtils.java b/java/google/registry/flows/domain/DomainFlowTmchUtils.java index 7224c70ae..7b9d99703 100644 --- a/java/google/registry/flows/domain/DomainFlowTmchUtils.java +++ b/java/google/registry/flows/domain/DomainFlowTmchUtils.java @@ -16,7 +16,6 @@ package google.registry.flows.domain; import static com.google.common.collect.Iterables.concat; import static google.registry.flows.EppXmlTransformer.unmarshal; -import static google.registry.util.DateTimeUtils.isAtOrAfter; import com.google.common.collect.ImmutableList; import google.registry.flows.EppException; @@ -111,11 +110,18 @@ public final class DomainFlowTmchUtils { throw new SignedMarkParsingErrorException(); } - if (!(isAtOrAfter(now, signedMark.getCreationTime()) - && now.isBefore(signedMark.getExpirationTime()) - && containsMatchingLabel(signedMark.getMark(), domainLabel))) { + if (now.isBefore(signedMark.getCreationTime())) { + throw new FoundMarkNotYetValidException(); + } + + if (now.isAfter(signedMark.getExpirationTime())) { + throw new FoundMarkExpiredException(); + } + + if (!containsMatchingLabel(signedMark.getMark(), domainLabel)) { throw new NoMarksFoundMatchingDomainException(); } + return signedMark; } @@ -150,6 +156,20 @@ public final class DomainFlowTmchUtils { } } + /** The provided mark is not yet valid. */ + static class FoundMarkNotYetValidException extends ParameterValuePolicyErrorException { + public FoundMarkNotYetValidException() { + super("The provided mark is not yet valid"); + } + } + + /** The provided mark has expired. */ + static class FoundMarkExpiredException extends ParameterValuePolicyErrorException { + public FoundMarkExpiredException() { + super("The provided mark has expired"); + } + } + /** Certificate used in signed mark signature was revoked by ICANN. */ static class SignedMarkCertificateRevokedException extends ParameterValuePolicyErrorException { public SignedMarkCertificateRevokedException() { diff --git a/javatests/google/registry/flows/domain/DomainApplicationCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainApplicationCreateFlowTest.java index c1f9679ea..b431feb0b 100644 --- a/javatests/google/registry/flows/domain/DomainApplicationCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainApplicationCreateFlowTest.java @@ -51,6 +51,8 @@ import google.registry.flows.domain.DomainApplicationCreateFlow.NoticeCannotBeUs import google.registry.flows.domain.DomainApplicationCreateFlow.SunriseApplicationDisallowedDuringLandrushException; import google.registry.flows.domain.DomainApplicationCreateFlow.UncontestedSunriseApplicationBlockedInLandrushException; import google.registry.flows.domain.DomainFlowTmchUtils.Base64RequiredForEncodedSignedMarksException; +import google.registry.flows.domain.DomainFlowTmchUtils.FoundMarkExpiredException; +import google.registry.flows.domain.DomainFlowTmchUtils.FoundMarkNotYetValidException; import google.registry.flows.domain.DomainFlowTmchUtils.NoMarksFoundMatchingDomainException; import google.registry.flows.domain.DomainFlowTmchUtils.SignedMarkCertificateExpiredException; import google.registry.flows.domain.DomainFlowTmchUtils.SignedMarkCertificateInvalidException; @@ -1301,7 +1303,7 @@ public class DomainApplicationCreateFlowTest clock.setTo(DateTime.parse("2013-08-09T10:05:59Z").minusSeconds(1)); persistContactsAndHosts(); clock.advanceOneMilli(); - EppException thrown = assertThrows(NoMarksFoundMatchingDomainException.class, this::runFlow); + EppException thrown = assertThrows(FoundMarkNotYetValidException.class, this::runFlow); assertAboutEppExceptions().that(thrown).marshalsToXml(); } @@ -1311,7 +1313,7 @@ public class DomainApplicationCreateFlowTest clock.setTo(DateTime.parse("2017-07-23T22:00:00.000Z")); persistContactsAndHosts(); clock.advanceOneMilli(); - EppException thrown = assertThrows(NoMarksFoundMatchingDomainException.class, this::runFlow); + EppException thrown = assertThrows(FoundMarkExpiredException.class, this::runFlow); assertAboutEppExceptions().that(thrown).marshalsToXml(); } diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java index 541099c1d..68c43a6f1 100644 --- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java @@ -68,6 +68,8 @@ import google.registry.flows.ResourceFlowTestCase; import google.registry.flows.domain.DomainCreateFlow.DomainHasOpenApplicationsException; import google.registry.flows.domain.DomainCreateFlow.MustHaveSignedMarksInCurrentPhaseException; import google.registry.flows.domain.DomainCreateFlow.NoGeneralRegistrationsInCurrentPhaseException; +import google.registry.flows.domain.DomainFlowTmchUtils.FoundMarkExpiredException; +import google.registry.flows.domain.DomainFlowTmchUtils.FoundMarkNotYetValidException; import google.registry.flows.domain.DomainFlowTmchUtils.NoMarksFoundMatchingDomainException; import google.registry.flows.domain.DomainFlowUtils.AcceptedTooLongAgoException; import google.registry.flows.domain.DomainFlowUtils.BadDomainNameCharacterException; @@ -1820,6 +1822,28 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase