Make not logged in errors take precedence over extension errors (#1483 )

* Make not logged in errors take precedence over extension errors This is the right order to do the checks in, because if the registrar isn't logged in (or their login failed) then they will have an empty set of declared extensions, so any attempt to use an extension will throw a "Service extension(s) must be declared at login" error. This is potentially misleading because the actual error in this situation is that the registrar isn't logged in at all. This also fixes some flows that weren't declared final (but should be), or methods declared final on final classes, which is superfluous.
Don't throw errors when existing premium list is empty (#1479 )
2026-07-08 17:16:54 +00:00 · 2021-12-30 17:23:14 -05:00 · 2021-12-30 17:22:43 -05:00 · 2021-12-30 12:53:39 -05:00 · 2021-12-28 15:45:40 -05:00
41 changed files with 167 additions and 85 deletions
@@ -56,9 +56,9 @@ public final class ContactCheckFlow implements Flow {
  @Inject ContactCheckFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
-    extensionManager.validate();  // There are no legal extensions for this flow.
+  public EppResponse run() throws EppException {
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate(); // There are no legal extensions for this flow.
    ImmutableList<String> targetIds = ((Check) resourceCommand).getTargetIds();
    verifyTargetIdCount(targetIds, maxChecks);
    ImmutableSet<String> existingIds =
@@ -70,10 +70,10 @@ public final class ContactCreateFlow implements TransactionalFlow {
  @Inject ContactCreateFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    Create command = (Create) resourceCommand;
    DateTime now = tm().getTransactionTime();
    verifyResourceDoesNotExist(ContactResource.class, targetId, now, registrarId);
@@ -90,10 +90,10 @@ public final class ContactDeleteFlow implements TransactionalFlow {
  ContactDeleteFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    checkLinkedDomains(targetId, now, ContactResource.class, DomainBase::getReferencedContacts);
    ContactResource existingContact = loadAndVerifyExistence(ContactResource.class, targetId, now);
@@ -65,10 +65,10 @@ public final class ContactInfoFlow implements Flow {
  ContactInfoFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    DateTime now = clock.nowUtc();
-    extensionManager.validate(); // There are no legal extensions for this flow.
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate(); // There are no legal extensions for this flow.
    ContactResource contact = loadAndVerifyExistence(ContactResource.class, targetId, now);
    if (!isSuperuser) {
      verifyResourceOwnership(registrarId, contact);
@@ -74,14 +74,14 @@ public final class ContactTransferApproveFlow implements TransactionalFlow {
  @Inject ContactTransferApproveFlow() {}

  /**
-   * <p>The logic in this flow, which handles client approvals, very closely parallels the logic in
+   * The logic in this flow, which handles client approvals, very closely parallels the logic in
   * {@link ContactResource#cloneProjectedAtTime} which handles implicit server approvals.
   */
  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    ContactResource existingContact = loadAndVerifyExistence(ContactResource.class, targetId, now);
    verifyOptionalAuthInfo(authInfo, existingContact);
@@ -76,8 +76,8 @@ public final class ContactTransferCancelFlow implements TransactionalFlow {
  @Override
  public final EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    ContactResource existingContact = loadAndVerifyExistence(ContactResource.class, targetId, now);
    verifyOptionalAuthInfo(authInfo, existingContact);
@@ -63,9 +63,9 @@ public final class ContactTransferQueryFlow implements Flow {
  @Inject ContactTransferQueryFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
-    extensionManager.validate();  // There are no legal extensions for this flow.
+  public EppResponse run() throws EppException {
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate(); // There are no legal extensions for this flow.
    ContactResource contact =
        loadAndVerifyExistence(ContactResource.class, targetId, clock.nowUtc());
    verifyOptionalAuthInfo(authInfo, contact);
@@ -72,10 +72,10 @@ public final class ContactTransferRejectFlow implements TransactionalFlow {
  @Inject ContactTransferRejectFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    ContactResource existingContact = loadAndVerifyExistence(ContactResource.class, targetId, now);
    verifyOptionalAuthInfo(authInfo, existingContact);
@@ -92,10 +92,10 @@ public final class ContactTransferRequestFlow implements TransactionalFlow {
  @Inject ContactTransferRequestFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(gainingClientId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    ContactResource existingContact = loadAndVerifyExistence(ContactResource.class, targetId, now);
    verifyAuthInfoPresentForResourceTransfer(authInfo);
@@ -89,10 +89,10 @@ public final class ContactUpdateFlow implements TransactionalFlow {
  @Inject ContactUpdateFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    Update command = (Update) resourceCommand;
    DateTime now = tm().getTransactionTime();
    ContactResource existingContact = loadAndVerifyExistence(ContactResource.class, targetId, now);
@@ -135,8 +135,8 @@ public final class DomainCheckFlow implements Flow {
    extensionManager.register(
        FeeCheckCommandExtension.class, LaunchCheckExtension.class, AllocationTokenExtension.class);
    flowCustomLogic.beforeValidation();
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    ImmutableList<String> domainNames = ((Check) resourceCommand).getTargetIds();
    verifyTargetIdCount(domainNames, maxChecks);
    DateTime now = clock.nowUtc();
@@ -82,8 +82,8 @@ public final class DomainClaimsCheckFlow implements Flow {
  @Override
  public EppResponse run() throws EppException {
    extensionManager.register(LaunchCheckExtension.class, AllocationTokenExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    if (eppInput.getSingleExtension(AllocationTokenExtension.class).isPresent()) {
      throw new DomainClaimsCheckNotAllowedWithAllocationTokens();
    }
@@ -198,7 +198,7 @@ import org.joda.time.Duration;
 * @error {@link DomainPricingLogic.AllocationTokenInvalidForPremiumNameException}
 */
@ReportingSpec(ActivityReportField.DOMAIN_CREATE)
-public class DomainCreateFlow implements TransactionalFlow {
+public final class DomainCreateFlow implements TransactionalFlow {

  /** Anchor tenant creates should always be for 2 years, since they get 2 years free. */
  private static final int ANCHOR_TENANT_CREATE_VALID_YEARS = 2;
@@ -219,7 +219,7 @@ public class DomainCreateFlow implements TransactionalFlow {
  @Inject DomainCreateFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(
        FeeCreateCommandExtension.class,
        SecDnsCreateExtension.class,
@@ -227,9 +227,9 @@ public class DomainCreateFlow implements TransactionalFlow {
        LaunchCreateExtension.class,
        AllocationTokenExtension.class);
    flowCustomLogic.beforeValidation();
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
    verifyRegistrarIsActive(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    DomainCommand.Create command = cloneAndLinkReferences((Create) resourceCommand, now);
    Period period = command.getPeriod();
@@ -138,12 +138,12 @@ public final class DomainDeleteFlow implements TransactionalFlow {
  @Inject DomainDeleteFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(
        MetadataExtension.class, SecDnsCreateExtension.class, DomainDeleteSuperuserExtension.class);
    flowCustomLogic.beforeValidation();
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    // Loads the target resource if it exists
    DomainBase existingDomain = loadAndVerifyExistence(DomainBase.class, targetId, now);
@@ -91,11 +91,11 @@ public final class DomainInfoFlow implements Flow {
  DomainInfoFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(FeeInfoCommandExtensionV06.class);
    flowCustomLogic.beforeValidation();
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = clock.nowUtc();
    DomainBase domain = verifyExistence(
        DomainBase.class, targetId, loadByForeignKey(DomainBase.class, targetId, now));
@@ -135,12 +135,12 @@ public final class DomainRenewFlow implements TransactionalFlow {
  @Inject DomainRenewFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(FeeRenewCommandExtension.class, MetadataExtension.class);
    flowCustomLogic.beforeValidation();
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
    verifyRegistrarIsActive(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    Renew command = (Renew) resourceCommand;
    // Loads the target resource if it exists
@@ -128,14 +128,14 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow {
  @Inject DomainRestoreRequestFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(
        FeeUpdateCommandExtension.class,
        MetadataExtension.class,
        RgpUpdateExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
    verifyRegistrarIsActive(registrarId);
+    extensionManager.validate();
    Update command = (Update) resourceCommand;
    DateTime now = tm().getTransactionTime();
    DomainBase existingDomain = loadAndVerifyExistence(DomainBase.class, targetId, now);
@@ -99,14 +99,14 @@ public final class DomainTransferApproveFlow implements TransactionalFlow {
  @Inject DomainTransferApproveFlow() {}

  /**
-   * <p>The logic in this flow, which handles client approvals, very closely parallels the logic in
+   * The logic in this flow, which handles client approvals, very closely parallels the logic in
   * {@link DomainBase#cloneProjectedAtTime} which handles implicit server approvals.
   */
  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    DomainBase existingDomain = loadAndVerifyExistence(DomainBase.class, targetId, now);
    verifyOptionalAuthInfo(authInfo, existingDomain);
@@ -86,10 +86,10 @@ public final class DomainTransferCancelFlow implements TransactionalFlow {
  @Inject DomainTransferCancelFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    DomainBase existingDomain = loadAndVerifyExistence(DomainBase.class, targetId, now);
    verifyOptionalAuthInfo(authInfo, existingDomain);
@@ -67,9 +67,9 @@ public final class DomainTransferQueryFlow implements Flow {
  @Inject DomainTransferQueryFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
-    extensionManager.validate();  // There are no legal extensions for this flow.
+  public EppResponse run() throws EppException {
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate(); // There are no legal extensions for this flow.
    DateTime now = clock.nowUtc();
    DomainBase domain = loadAndVerifyExistence(DomainBase.class, targetId, now);
    verifyOptionalAuthInfo(authInfo, domain);
@@ -88,10 +88,10 @@ public final class DomainTransferRejectFlow implements TransactionalFlow {
  @Inject DomainTransferRejectFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    DomainBase existingDomain = loadAndVerifyExistence(DomainBase.class, targetId, now);
    Registry registry = Registry.get(existingDomain.getTld());
@@ -139,14 +139,14 @@ public final class DomainTransferRequestFlow implements TransactionalFlow {
  @Inject DomainTransferRequestFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(
        DomainTransferRequestSuperuserExtension.class,
        FeeTransferCommandExtension.class,
        MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(gainingClientId);
    verifyRegistrarIsActive(gainingClientId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    DomainBase existingDomain = loadAndVerifyExistence(DomainBase.class, targetId, now);
    Optional<DomainTransferRequestSuperuserExtension> superuserExtension =
@@ -43,6 +43,7 @@ import static google.registry.flows.domain.DomainFlowUtils.verifyNotInPendingDel
 import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_UPDATE;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;

+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Ordering;
@@ -77,9 +78,11 @@ import google.registry.model.domain.secdns.SecDnsUpdateExtension;
 import google.registry.model.domain.superuser.DomainUpdateSuperuserExtension;
 import google.registry.model.eppcommon.AuthInfo;
 import google.registry.model.eppcommon.StatusValue;
+import google.registry.model.eppcommon.Trid;
 import google.registry.model.eppinput.EppInput;
 import google.registry.model.eppinput.ResourceCommand;
 import google.registry.model.eppoutput.EppResponse;
+import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.reporting.IcannReportingTypes.ActivityReportField;
 import google.registry.model.tld.Registry;
@@ -149,6 +152,7 @@ public final class DomainUpdateFlow implements TransactionalFlow {
  @Inject @RegistrarId String registrarId;
  @Inject @TargetId String targetId;
  @Inject @Superuser boolean isSuperuser;
+  @Inject Trid trid;
  @Inject DomainHistory.Builder historyBuilder;
  @Inject DnsQueue dnsQueue;
  @Inject EppResponse.Builder responseBuilder;
@@ -164,8 +168,8 @@ public final class DomainUpdateFlow implements TransactionalFlow {
        SecDnsUpdateExtension.class,
        DomainUpdateSuperuserExtension.class);
    flowCustomLogic.beforeValidation();
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    Update command = cloneAndLinkReferences((Update) resourceCommand, now);
    DomainBase existingDomain = loadAndVerifyExistence(DomainBase.class, targetId, now);
@@ -360,6 +364,10 @@ public final class DomainUpdateFlow implements TransactionalFlow {
            .setEventTime(now)
            .setRegistrarId(existingDomain.getCurrentSponsorRegistrarId())
            .setMsg(msg)
+            .setResponseData(
+                ImmutableList.of(
+                    DomainPendingActionNotificationResponse.create(
+                        existingDomain.getDomainName(), true, trid, now)))
            .build());
  }
 }
@@ -56,9 +56,9 @@ public final class HostCheckFlow implements Flow {
  @Inject HostCheckFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
-    extensionManager.validate();  // There are no legal extensions for this flow.
+  public EppResponse run() throws EppException {
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate(); // There are no legal extensions for this flow.
    ImmutableList<String> hostnames = ((Check) resourceCommand).getTargetIds();
    verifyTargetIdCount(hostnames, maxChecks);
    ImmutableSet<String> existingIds =
@@ -100,10 +100,10 @@ public final class HostCreateFlow implements TransactionalFlow {
  HostCreateFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    Create command = (Create) resourceCommand;
    DateTime now = tm().getTransactionTime();
    verifyResourceDoesNotExist(HostResource.class, targetId, now, registrarId);
@@ -92,10 +92,10 @@ public final class HostDeleteFlow implements TransactionalFlow {
  HostDeleteFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    DateTime now = tm().getTransactionTime();
    validateHostName(targetId);
    checkLinkedDomains(targetId, now, HostResource.class, DomainBase::getNameservers);
@@ -62,8 +62,8 @@ public final class HostInfoFlow implements Flow {

  @Override
  public EppResponse run() throws EppException {
-    extensionManager.validate();  // There are no legal extensions for this flow.
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate(); // There are no legal extensions for this flow.
    validateHostName(targetId);
    DateTime now = clock.nowUtc();
    HostResource host = loadAndVerifyExistence(HostResource.class, targetId, now);
@@ -125,10 +125,10 @@ public final class HostUpdateFlow implements TransactionalFlow {
  @Inject HostUpdateFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
+  public EppResponse run() throws EppException {
    extensionManager.register(MetadataExtension.class);
-    extensionManager.validate();
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate();
    Update command = (Update) resourceCommand;
    Change change = command.getInnerChange();
    String suppliedNewHostName = change.getFullyQualifiedHostName();
@@ -45,17 +45,17 @@ import org.joda.time.DateTime;
 /**
 * An EPP flow for acknowledging {@link PollMessage}s.
 *
- * <p>Registrars refer to poll messages using an externally visible id generated by
- * {@link PollMessageExternalKeyConverter}. One-time poll messages are deleted from Datastore once
- * they are ACKed, whereas autorenew poll messages are simply marked as read, and won't be delivered
- * again until the next year of their recurrence.
+ * <p>Registrars refer to poll messages using an externally visible id generated by {@link
+ * PollMessageExternalKeyConverter}. One-time poll messages are deleted from Datastore once they are
+ * ACKed, whereas autorenew poll messages are simply marked as read, and won't be delivered again
+ * until the next year of their recurrence.
 *
 * @error {@link PollAckFlow.InvalidMessageIdException}
 * @error {@link PollAckFlow.MessageDoesNotExistException}
 * @error {@link PollAckFlow.MissingMessageIdException}
 * @error {@link PollAckFlow.NotAuthorizedToAckMessageException}
 */
-public class PollAckFlow implements TransactionalFlow {
+public final class PollAckFlow implements TransactionalFlow {

  @Inject ExtensionManager extensionManager;
  @Inject @RegistrarId String registrarId;
@@ -65,9 +65,9 @@ public class PollAckFlow implements TransactionalFlow {
  @Inject PollAckFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
-    extensionManager.validate();  // There are no legal extensions for this flow.
+  public EppResponse run() throws EppException {
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate(); // There are no legal extensions for this flow.
    if (messageId.isEmpty()) {
      throw new MissingMessageIdException();
    }
@@ -47,7 +47,7 @@ import org.joda.time.DateTime;
 *
 * @error {@link PollRequestFlow.UnexpectedMessageIdException}
 */
-public class PollRequestFlow implements Flow {
+public final class PollRequestFlow implements Flow {

  @Inject ExtensionManager extensionManager;
  @Inject @RegistrarId String registrarId;
@@ -57,9 +57,9 @@ public class PollRequestFlow implements Flow {
  @Inject PollRequestFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
-    extensionManager.validate();  // There are no legal extensions for this flow.
+  public EppResponse run() throws EppException {
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate(); // There are no legal extensions for this flow.
    if (!messageId.isEmpty()) {
      throw new UnexpectedMessageIdException();
    }
@@ -27,7 +27,7 @@ import javax.inject.Inject;
 *
 * @error {@link google.registry.flows.FlowUtils.GenericXmlSyntaxErrorException}
 */
-public class HelloFlow implements Flow {
+public final class HelloFlow implements Flow {

  @Inject ExtensionManager extensionManager;
  @Inject Clock clock;
@@ -90,7 +90,7 @@ public class LoginFlow implements Flow {
  }

  /** Run the flow without bothering to log errors. The {@link #run} method will do that for us. */
-  private final EppResponse runWithoutLogging() throws EppException {
+  private EppResponse runWithoutLogging() throws EppException {
    extensionManager.validate();  // There are no legal extensions for this flow.
    Login login = (Login) eppInput.getCommandWrapper().getCommand();
    if (!registrarId.isEmpty()) {
@@ -30,7 +30,7 @@ import javax.inject.Inject;
 *
 * @error {@link google.registry.flows.FlowUtils.NotLoggedInException}
 */
-public class LogoutFlow implements Flow {
+public final class LogoutFlow implements Flow {

  @Inject ExtensionManager extensionManager;
  @Inject @RegistrarId String registrarId;
@@ -39,9 +39,9 @@ public class LogoutFlow implements Flow {
  @Inject LogoutFlow() {}

  @Override
-  public final EppResponse run() throws EppException {
-    extensionManager.validate();  // There are no legal extensions for this flow.
+  public EppResponse run() throws EppException {
    validateRegistrarIsLoggedIn(registrarId);
+    extensionManager.validate(); // There are no legal extensions for this flow.
    sessionMetadata.invalidate();
    return responseBuilder.setResultFromCode(SUCCESS_AND_CLOSE).build();
  }
@@ -14,11 +14,13 @@

 package google.registry.model.tld.label;

+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration;
 import static google.registry.config.RegistryConfig.getSingletonCachePersistDuration;
 import static google.registry.config.RegistryConfig.getStaticPremiumListMaxCachedEntries;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.util.CollectionUtils.isNullOrEmpty;

 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
@@ -153,27 +155,26 @@ public class PremiumListDao {
  }

  public static PremiumList save(String name, CurrencyUnit currencyUnit, List<String> inputData) {
+    checkArgument(!inputData.isEmpty(), "New premium list data cannot be empty");
    return save(PremiumListUtils.parseToPremiumList(name, currencyUnit, inputData));
  }

+  /** Saves the given premium list (and its premium list entries) to Cloud SQL. */
  public static PremiumList save(PremiumList premiumList) {
-    jpaTm().transact(() -> jpaTm().insert(premiumList));
-    premiumListCache.invalidate(premiumList.getName());
    jpaTm()
        .transact(
            () -> {
-              if (premiumList.getLabelsToPrices() != null) {
-                Optional<PremiumList> savedPremiumList =
-                    PremiumListDao.getLatestRevision(premiumList.getName());
+              jpaTm().insert(premiumList);
+              jpaTm().getEntityManager().flush(); // This populates the revisionId.
+              long revisionId = premiumList.getRevisionId();
+
+              if (!isNullOrEmpty(premiumList.getLabelsToPrices())) {
                ImmutableSet.Builder<PremiumEntry> entries = new ImmutableSet.Builder<>();
                premiumList.getLabelsToPrices().entrySet().stream()
                    .forEach(
                        entry ->
                            entries.add(
-                                PremiumEntry.create(
-                                    savedPremiumList.get().getRevisionId(),
-                                    entry.getValue(),
-                                    entry.getKey())));
+                                PremiumEntry.create(revisionId, entry.getValue(), entry.getKey())));
                jpaTm().insertAll(entries.build());
              }
            });
@@ -14,7 +14,6 @@

 package google.registry.model.tld.label;

-import static com.google.common.base.Preconditions.checkArgument;
 import static org.joda.time.DateTimeZone.UTC;

 import com.google.common.collect.ImmutableMap;
@@ -38,7 +37,6 @@ public class PremiumListUtils {
            .setCreationTimestamp(DateTime.now(UTC))
            .build();
    ImmutableMap<String, PremiumEntry> prices = partialPremiumList.parse(inputData);
-    checkArgument(inputData.size() > 0, "Input cannot be empty");
    Map<String, BigDecimal> priceAmounts = Maps.transformValues(prices, PremiumEntry::getValue);
    return partialPremiumList.asBuilder().setLabelsToPrices(priceAmounts).build();
  }
@@ -16,6 +16,7 @@ package google.registry.tools;

 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static google.registry.model.tld.label.PremiumListUtils.parseToPremiumList;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.util.ListNamingUtils.convertFilePathToName;
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -27,7 +28,6 @@ import com.google.common.collect.Streams;
 import google.registry.model.tld.label.PremiumList;
 import google.registry.model.tld.label.PremiumList.PremiumEntry;
 import google.registry.model.tld.label.PremiumListDao;
-import google.registry.model.tld.label.PremiumListUtils;
 import java.nio.file.Files;
 import java.util.List;
 import java.util.Optional;
@@ -45,11 +45,11 @@ class UpdatePremiumListCommand extends CreateOrUpdatePremiumListCommand {
        String.format("Could not update premium list %s because it doesn't exist.", name));
    List<String> existingEntry = getExistingPremiumEntry(list.get()).asList();
    inputData = Files.readAllLines(inputFile, UTF_8);
+    checkArgument(!inputData.isEmpty(), "New premium list data cannot be empty");
    currency = list.get().getCurrency();
    // reconstructing existing premium list to bypass Hibernate lazy initialization exception
-    PremiumList existingPremiumList =
-        PremiumListUtils.parseToPremiumList(name, currency, existingEntry);
-    PremiumList updatedPremiumList = PremiumListUtils.parseToPremiumList(name, currency, inputData);
+    PremiumList existingPremiumList = parseToPremiumList(name, currency, existingEntry);
+    PremiumList updatedPremiumList = parseToPremiumList(name, currency, inputData);

    return String.format(
        "Update premium list for %s?\n Old List: %s\n New List: %s",
@@ -130,6 +130,17 @@ class DomainCheckFlowTest extends ResourceCheckFlowTestCase<DomainCheckFlow, Dom
    assertAboutEppExceptions().that(thrown).marshalsToXml();
  }

+  @TestOfyAndSql
+  void testNotLoggedIn_takesPrecedenceOverUndeclaredExtensions() {
+    // Attempt to use the fee extension, but there is no login session and no supported extensions.
+    setEppInput("domain_check_fee_v06.xml", ImmutableMap.of("CURRENCY", "USD"));
+    sessionMetadata.setRegistrarId(null);
+    sessionMetadata.setServiceExtensionUris(ImmutableSet.of());
+    // NotLoggedIn should be thrown, not UndeclaredServiceExtensionException.
+    EppException thrown = assertThrows(NotLoggedInException.class, this::runFlow);
+    assertAboutEppExceptions().that(thrown).marshalsToXml();
+  }
+
  @TestOfyAndSql
  void testSuccess_nothingExists() throws Exception {
    doCheckTest(
@@ -99,7 +99,9 @@ import google.registry.model.domain.DomainBase;
 import google.registry.model.domain.DomainHistory;
 import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.eppcommon.StatusValue;
+import google.registry.model.eppcommon.Trid;
 import google.registry.model.host.HostResource;
+import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.tld.Registry;
 import google.registry.persistence.VKey;
@@ -949,6 +951,13 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
            .setParent(getOnlyHistoryEntryOfType(updatedDomain, DOMAIN_UPDATE))
            .setRegistrarId("NewRegistrar")
            .setMsg("The registry administrator has added the status(es) [serverHold].")
+            .setResponseData(
+                ImmutableList.of(
+                    DomainPendingActionNotificationResponse.create(
+                        "example.tld",
+                        true,
+                        Trid.create("ABC-12345", "server-trid"),
+                        clock.nowUtc())))
            .build());
  }

@@ -983,6 +992,13 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
            .setMsg(
                "The registry administrator has removed the status(es) [serverHold,"
                    + " serverTransferProhibited, serverUpdateProhibited].")
+            .setResponseData(
+                ImmutableList.of(
+                    DomainPendingActionNotificationResponse.create(
+                        "example.tld",
+                        true,
+                        Trid.create("ABC-12345", "server-trid"),
+                        clock.nowUtc())))
            .build());
  }

@@ -1015,6 +1031,13 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
                "The registry administrator has added the status(es) [serverHold,"
                    + " serverRenewProhibited] and removed the status(es)"
                    + " [serverTransferProhibited].")
+            .setResponseData(
+                ImmutableList.of(
+                    DomainPendingActionNotificationResponse.create(
+                        "example.tld",
+                        true,
+                        Trid.create("ABC-12345", "server-trid"),
+                        clock.nowUtc())))
            .build());
  }

@@ -22,6 +22,7 @@ import static google.registry.testing.DatabaseHelper.persistResource;
 import static org.joda.money.CurrencyUnit.JPY;
 import static org.joda.money.CurrencyUnit.USD;
 import static org.joda.time.Duration.standardDays;
+import static org.junit.jupiter.api.Assertions.assertThrows;

 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -239,6 +240,15 @@ public class PremiumListDaoTest {
        .hasValue(moneyOf(JPY, 15000));
  }

+  @Test
+  void testSave_throwsOnEmptyInputData() {
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> PremiumListDao.save("test-list", CurrencyUnit.GBP, ImmutableList.of()));
+    assertThat(thrown).hasMessageThat().isEqualTo("New premium list data cannot be empty");
+  }
+
  @Test
  void test_savePremiumList_clearsCache() {
    assertThat(PremiumListDao.premiumListCache.getIfPresent("testname")).isNull();
@@ -84,6 +84,27 @@ class UpdatePremiumListCommandTest<C extends UpdatePremiumListCommand>
    assertThat(entries.contains("eth,USD 9999.00")).isTrue();
  }

+  @Test
+  void commandRun_successUpdateList_whenExistingListIsEmpty() throws Exception {
+    File existingPremiumFile = tmpDir.resolve(TLD_TEST + ".txt").toFile();
+    Files.asCharSink(existingPremiumFile, UTF_8).write("");
+
+    File newPremiumFile = tmpDir.resolve(String.format("%s.txt", TLD_TEST)).toFile();
+    String newPremiumListData = "eth,USD 9999";
+    Files.asCharSink(newPremiumFile, UTF_8).write(newPremiumListData);
+
+    UpdatePremiumListCommand command = new UpdatePremiumListCommand();
+    // data come from @beforeEach of CreateOrUpdatePremiumListCommandTestCase.java
+    command.inputFile = Paths.get(newPremiumFile.getPath());
+    runCommandForced("--name=" + TLD_TEST, "--input=" + command.inputFile);
+
+    ImmutableSet<String> entries =
+        command.getExistingPremiumEntry(PremiumListDao.getLatestRevision(TLD_TEST).get());
+    assertThat(entries.size()).isEqualTo(1);
+    // verify that list is updated; cannot use only string since price is formatted;
+    assertThat(entries.contains("eth,USD 9999.00")).isTrue();
+  }
+
  @Test
  void commandRun_successUpdateMultiLineList() throws Exception {
    File tmpFile = tmpDir.resolve(TLD_TEST + ".txt").toFile();
@@ -112,7 +133,7 @@ class UpdatePremiumListCommandTest<C extends UpdatePremiumListCommand>
    command.inputFile = tmpPath;
    command.name = TLD_TEST;
    IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, command::prompt);
-    assertThat(thrown).hasMessageThat().contains("Input cannot be empty");
+    assertThat(thrown).hasMessageThat().isEqualTo("New premium list data cannot be empty");
  }

  @Test
@@ -8,6 +8,16 @@
      <msg>The registry administrator has added the status(es) [serverHold].
      </msg>
    </msgQ>
+    <resData>
+      <domain:panData xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
+        <domain:name paResult="1">example.tld</domain:name>
+        <domain:paTRID>
+          <clTRID>ABC-12345</clTRID>
+          <svTRID>server-trid</svTRID>
+        </domain:paTRID>
+        <domain:paDate>2000-06-02T13:00:00Z</domain:paDate>
+      </domain:panData>
+    </resData>
    <trID>
      <clTRID>ABC-12345</clTRID>
      <svTRID>server-trid</svTRID>
Author	SHA1	Message	Date
Ben McIlwain	7a174e3ffa	Make not logged in errors take precedence over extension errors (#1483 ) * Make not logged in errors take precedence over extension errors This is the right order to do the checks in, because if the registrar isn't logged in (or their login failed) then they will have an empty set of declared extensions, so any attempt to use an extension will throw a "Service extension(s) must be declared at login" error. This is potentially misleading because the actual error in this situation is that the registrar isn't logged in at all. This also fixes some flows that weren't declared final (but should be), or methods declared final on final classes, which is superfluous.	2021-12-30 17:23:14 -05:00
Ben McIlwain	2b38ad8a25	Don't throw errors when existing premium list is empty (#1479 ) * Don't throw errors when existing premium list is empty This state is possible to get into when things go wrong and it shouldn't prevent saving new revisions of the list. Note that it will continue to throw errors if you attempt to save a new revision that is blank (which is usually a mistake). See http://b/211774375	2021-12-30 17:22:43 -05:00
Ben McIlwain	eefb4c71aa	Make premium list saving run as a single transaction (#1480 ) * Make premium list saving run as a single transaction This fixes the bug where the new revision is saved, but then execution gets halted for some reason (e.g. request timeout) before the entries finish saving, which leaves the DB in a bad state with a new top revision containing zero entries, thus making everything standard.	2021-12-30 12:53:39 -05:00
Ben McIlwain	9d3cbd07fd	Add pending action extension to server update poll messages (#1478 ) * Add pending action extension to server update poll messages This is necessary for the poll messages to contain the necessary context explaining what domain name the relevant statuses were being added/removed to/from.	2021-12-28 15:45:40 -05:00