From 21c0b43af0019560b8b9b6c1534cd0862793a2fc Mon Sep 17 00:00:00 2001 From: ctingue Date: Wed, 26 Oct 2016 14:40:20 -0700 Subject: [PATCH] Fix missing LRP token during LRP period behavior ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=137321673 --- .../domain/DomainApplicationCreateFlow.java | 9 +++---- .../flows/domain/DomainCreateFlow.java | 13 ++++------ .../flows/domain/TldSpecificLogicProxy.java | 5 ++-- .../DomainApplicationCreateFlowTest.java | 25 ++++++++++--------- 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/java/google/registry/flows/domain/DomainApplicationCreateFlow.java b/java/google/registry/flows/domain/DomainApplicationCreateFlow.java index b683e8c88..87deff8fb 100644 --- a/java/google/registry/flows/domain/DomainApplicationCreateFlow.java +++ b/java/google/registry/flows/domain/DomainApplicationCreateFlow.java @@ -239,13 +239,10 @@ public final class DomainApplicationCreateFlow extends LoggedInFlow implements T DomainApplicationIndex.createUpdatedInstance(newApplication), EppResourceIndex.create(Key.create(newApplication))); // Anchor tenant registrations override LRP, and landrush applications can skip it. + // If a token is passed in outside of an LRP phase, it is simply ignored (i.e. never redeemed). if (registry.getLrpPeriod().contains(now) && !isAnchorTenant) { - // TODO(b/32059212): This is a bug: empty tokens should still fail. Preserving to fix in a - // separate targeted change. - if (!authInfo.getPw().getValue().isEmpty()) { - entitiesToSave.add( - prepareMarkedLrpTokenEntity(authInfo.getPw().getValue(), domainName, historyEntry)); - } + entitiesToSave.add( + prepareMarkedLrpTokenEntity(authInfo.getPw().getValue(), domainName, historyEntry)); } ofy().save().entities(entitiesToSave.build()); return createOutput( diff --git a/java/google/registry/flows/domain/DomainCreateFlow.java b/java/google/registry/flows/domain/DomainCreateFlow.java index 6b7fd80c7..92a421109 100644 --- a/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/java/google/registry/flows/domain/DomainCreateFlow.java @@ -267,15 +267,12 @@ public class DomainCreateFlow extends LoggedInFlow implements TransactionalFlow newDomain, ForeignKeyIndex.create(newDomain, newDomain.getDeletionTime()), EppResourceIndex.create(Key.create(newDomain))); + // Anchor tenant registrations override LRP, and landrush applications can skip it. // If a token is passed in outside of an LRP phase, it is simply ignored (i.e. never redeemed). - if (hasLrpToken(registry, isAnchorTenant)) { - // TODO(b/32059212): This is a bug: empty tokens should still fail. Preserving to fix in a - // separate targeted change. - if (!authInfo.getPw().getValue().isEmpty()) { - entitiesToSave.add( - prepareMarkedLrpTokenEntity(authInfo.getPw().getValue(), domainName, historyEntry)); - } + if (isLrpCreate(registry, isAnchorTenant)) { + entitiesToSave.add( + prepareMarkedLrpTokenEntity(authInfo.getPw().getValue(), domainName, historyEntry)); } enqueueTasks(hasSignedMarks, hasClaimsNotice, newDomain); ofy().save().entities(entitiesToSave.build()); @@ -386,7 +383,7 @@ public class DomainCreateFlow extends LoggedInFlow implements TransactionalFlow .build(); } - private boolean hasLrpToken(Registry registry, boolean isAnchorTenant) { + private boolean isLrpCreate(Registry registry, boolean isAnchorTenant) { return registry.getLrpPeriod().contains(now) && !isAnchorTenant; } diff --git a/java/google/registry/flows/domain/TldSpecificLogicProxy.java b/java/google/registry/flows/domain/TldSpecificLogicProxy.java index 72d5c37f4..831a54b64 100644 --- a/java/google/registry/flows/domain/TldSpecificLogicProxy.java +++ b/java/google/registry/flows/domain/TldSpecificLogicProxy.java @@ -278,12 +278,11 @@ public final class TldSpecificLogicProxy { // domain-name-to-assignee match. if (!lrpToken.isEmpty()) { LrpTokenEntity token = ofy().load().key(Key.create(LrpTokenEntity.class, lrpToken)).now(); - if (token != null) { - if (token.getAssignee().equalsIgnoreCase(domainName.toString()) + if (token != null + && token.getAssignee().equalsIgnoreCase(domainName.toString()) && token.getRedemptionHistoryEntry() == null && token.getValidTlds().contains(domainName.parent().toString())) { return Optional.of(token); - } } } return Optional.absent(); diff --git a/javatests/google/registry/flows/domain/DomainApplicationCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainApplicationCreateFlowTest.java index 118e055a7..7ace3bfa8 100644 --- a/javatests/google/registry/flows/domain/DomainApplicationCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainApplicationCreateFlowTest.java @@ -893,18 +893,6 @@ public class DomainApplicationCreateFlowTest assertThat(ofy().load().entity(token).now().getRedemptionHistoryEntry()).isNotNull(); } - @Test - public void testSuccess_landrush_duringLrpWithMissingToken() throws Exception { - createTld("tld", TldState.LANDRUSH); - persistResource(Registry.get("tld").asBuilder() - .setLrpPeriod(new Interval(clock.nowUtc().minusDays(1), clock.nowUtc().plusDays(1))) - .build()); - setEppInput("domain_create_landrush.xml"); - persistContactsAndHosts(); - clock.advanceOneMilli(); - doSuccessfulTest("domain_create_landrush_response.xml", false); - } - @Test public void testSuccess_landrushLrpApplication_superuser() throws Exception { // Using an LRP token as superuser should still mark the token as redeemed (i.e. same effect @@ -1028,6 +1016,19 @@ public class DomainApplicationCreateFlowTest assertThat(ofy().load().entity(token).now().getRedemptionHistoryEntry()).isNull(); } + @Test + public void testFailure_landrush_duringLrpWithMissingToken() throws Exception { + createTld("tld", TldState.LANDRUSH); + persistResource(Registry.get("tld").asBuilder() + .setLrpPeriod(new Interval(clock.nowUtc().minusDays(1), clock.nowUtc().plusDays(1))) + .build()); + setEppInput("domain_create_landrush.xml"); + persistContactsAndHosts(); + clock.advanceOneMilli(); + thrown.expect(InvalidLrpTokenException.class); + runFlow(); + } + @Test public void testFailure_landrushWithPeriodInMonths() throws Exception { createTld("tld", TldState.LANDRUSH);