1
0
mirror of https://github.com/google/nomulus synced 2026-07-16 04:52:25 +00:00

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.
This commit is contained in:
Ben McIlwain
2021-12-30 17:23:14 -05:00
committed by GitHub
parent 2b38ad8a25
commit 7a174e3ffa
34 changed files with 80 additions and 69 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 =
@@ -168,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);
@@ -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();
}
@@ -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(