From 2a3a3fbc304cc86ff4b0a8c13679370c219ae6b3 Mon Sep 17 00:00:00 2001 From: cgoldfeder Date: Sun, 19 Jun 2016 18:12:57 -0700 Subject: [PATCH] Break SessionSource out of SessionMetadata and rename it EppRequestSource. The "SessionSource" has nothing to do with sessions (and it's often used in sessionless contexts). What it does indicate is the endpoint used to make the request. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=125295224 --- .../registry/flows/EppConsoleAction.java | 1 + java/google/registry/flows/EppController.java | 2 ++ .../registry/flows/EppRequestHandler.java | 8 ++++- .../registry/flows/EppRequestSource.java | 26 ++++++++++++++++ java/google/registry/flows/EppTlsAction.java | 1 + java/google/registry/flows/EppToolAction.java | 5 ++- java/google/registry/flows/Flow.java | 3 ++ java/google/registry/flows/FlowRunner.java | 9 ++++-- .../registry/flows/HttpSessionMetadata.java | 5 --- .../flows/ResourceCreateOrMutateFlow.java | 4 +-- .../registry/flows/SessionMetadata.java | 31 ------------------- .../StatelessRequestSessionMetadata.java | 10 +----- .../ValidateLoginCredentialsCommand.java | 2 ++ .../ui/server/api/CheckApiAction.java | 6 ++-- .../registry/flows/EppConsoleActionTest.java | 3 +- .../google/registry/flows/EppTestCase.java | 8 ++++- .../registry/flows/EppTlsActionTest.java | 1 + .../registry/flows/EppToolActionTest.java | 1 + .../google/registry/flows/FlowTestCase.java | 4 +-- .../flows/domain/DomainCreateFlowTest.java | 24 +++++++------- .../flows/domain/DomainDeleteFlowTest.java | 4 +-- .../flows/domain/DomainUpdateFlowTest.java | 14 ++++----- .../flows/host/HostUpdateFlowTest.java | 4 +-- .../registry/model/EppResourceUtilsTest.java | 2 ++ .../registry/testing/TestSessionMetadata.java | 11 ------- 25 files changed, 94 insertions(+), 95 deletions(-) create mode 100644 java/google/registry/flows/EppRequestSource.java diff --git a/java/google/registry/flows/EppConsoleAction.java b/java/google/registry/flows/EppConsoleAction.java index 20aa52d94..2bc1ad20c 100644 --- a/java/google/registry/flows/EppConsoleAction.java +++ b/java/google/registry/flows/EppConsoleAction.java @@ -43,6 +43,7 @@ public class EppConsoleAction implements Runnable { eppRequestHandler.executeEpp( new HttpSessionMetadata(session), new GaeUserCredentials(getUserService().getCurrentUser()), + EppRequestSource.CONSOLE, false, // This endpoint is never a dry run. false, // This endpoint is never a superuser. inputXmlBytes); diff --git a/java/google/registry/flows/EppController.java b/java/google/registry/flows/EppController.java index bb02f6f5f..f6a334323 100644 --- a/java/google/registry/flows/EppController.java +++ b/java/google/registry/flows/EppController.java @@ -54,6 +54,7 @@ public final class EppController { public byte[] handleEppCommand( SessionMetadata sessionMetadata, TransportCredentials credentials, + EppRequestSource eppRequestSource, boolean isDryRun, boolean isSuperuser, byte[] inputXmlBytes) { @@ -74,6 +75,7 @@ public final class EppController { trid, sessionMetadata, credentials, + eppRequestSource, isDryRun, isSuperuser, inputXmlBytes, diff --git a/java/google/registry/flows/EppRequestHandler.java b/java/google/registry/flows/EppRequestHandler.java index 3ea855e21..974c3598c 100644 --- a/java/google/registry/flows/EppRequestHandler.java +++ b/java/google/registry/flows/EppRequestHandler.java @@ -42,13 +42,19 @@ public class EppRequestHandler { public void executeEpp( SessionMetadata sessionMetadata, TransportCredentials credentials, + EppRequestSource eppRequestSource, boolean isDryRun, boolean isSuperuser, byte[] inputXmlBytes) { try { response.setPayload(new String( eppController.handleEppCommand( - sessionMetadata, credentials, isDryRun, isSuperuser, inputXmlBytes), UTF_8)); + sessionMetadata, + credentials, + eppRequestSource, + isDryRun, + isSuperuser, + inputXmlBytes), UTF_8)); response.setContentType(APPLICATION_EPP_XML); // Note that we always return 200 (OK) even if the EppController returns an error response. // This is because returning an non-OK HTTP status code will cause the proxy server to diff --git a/java/google/registry/flows/EppRequestSource.java b/java/google/registry/flows/EppRequestSource.java new file mode 100644 index 000000000..6c7b86d81 --- /dev/null +++ b/java/google/registry/flows/EppRequestSource.java @@ -0,0 +1,26 @@ +// Copyright 2016 The Domain Registry Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.flows; + +/** + * An enum that identifies the origin of the session. + */ +public enum EppRequestSource { + CONSOLE, + TLS, + TOOL, + CHECK_API, + UNIT_TEST +} diff --git a/java/google/registry/flows/EppTlsAction.java b/java/google/registry/flows/EppTlsAction.java index 55e00cfc1..b1bdf4b99 100644 --- a/java/google/registry/flows/EppTlsAction.java +++ b/java/google/registry/flows/EppTlsAction.java @@ -49,6 +49,7 @@ public class EppTlsAction implements Runnable { eppRequestHandler.executeEpp( new HttpSessionMetadata(session), tlsCredentials, + EppRequestSource.TLS, false, // This endpoint is never a dry run. false, // This endpoint is never a superuser. inputXmlBytes); diff --git a/java/google/registry/flows/EppToolAction.java b/java/google/registry/flows/EppToolAction.java index 36c456d4e..ac2ebb985 100644 --- a/java/google/registry/flows/EppToolAction.java +++ b/java/google/registry/flows/EppToolAction.java @@ -21,7 +21,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import dagger.Module; import dagger.Provides; -import google.registry.flows.SessionMetadata.SessionSource; import google.registry.model.eppcommon.ProtocolDefinition; import google.registry.request.Action; import google.registry.request.Action.Method; @@ -50,9 +49,9 @@ public class EppToolAction implements Runnable { eppRequestHandler.executeEpp( new StatelessRequestSessionMetadata( clientIdentifier, - ProtocolDefinition.getVisibleServiceExtensionUris(), - SessionSource.TOOL), + ProtocolDefinition.getVisibleServiceExtensionUris()), new PasswordOnlyTransportCredentials(), + EppRequestSource.TOOL, isDryRun, isSuperuser, xml.getBytes(UTF_8)); diff --git a/java/google/registry/flows/Flow.java b/java/google/registry/flows/Flow.java index b46a4402a..b56325c50 100644 --- a/java/google/registry/flows/Flow.java +++ b/java/google/registry/flows/Flow.java @@ -43,6 +43,7 @@ public abstract class Flow { protected EppInput eppInput; protected SessionMetadata sessionMetadata; protected TransportCredentials credentials; + protected EppRequestSource eppRequestSource; protected Trid trid; protected DateTime now; protected byte[] inputXmlBytes; @@ -103,6 +104,7 @@ public abstract class Flow { Trid trid, SessionMetadata sessionMetadata, TransportCredentials credentials, + EppRequestSource eppRequestSource, boolean isSuperuser, DateTime now, byte[] inputXmlBytes) throws EppException { @@ -110,6 +112,7 @@ public abstract class Flow { this.trid = trid; this.sessionMetadata = sessionMetadata; this.credentials = credentials; + this.eppRequestSource = eppRequestSource; this.now = now; this.isSuperuser = isSuperuser; this.inputXmlBytes = inputXmlBytes; diff --git a/java/google/registry/flows/FlowRunner.java b/java/google/registry/flows/FlowRunner.java index 2486b2005..892b068d1 100644 --- a/java/google/registry/flows/FlowRunner.java +++ b/java/google/registry/flows/FlowRunner.java @@ -44,30 +44,33 @@ public class FlowRunner { private final EppInput eppInput; private final Trid trid; private final SessionMetadata sessionMetadata; + private final TransportCredentials credentials; + private final EppRequestSource eppRequestSource; private final boolean isDryRun; private final boolean isSuperuser; - private final TransportCredentials credentials; private final byte[] inputXmlBytes; private final EppMetrics metrics; private final Clock clock; + public FlowRunner( Class flowClass, EppInput eppInput, Trid trid, SessionMetadata sessionMetadata, TransportCredentials credentials, + EppRequestSource eppRequestSource, boolean isDryRun, boolean isSuperuser, byte[] inputXmlBytes, final EppMetrics metrics, Clock clock) { - credentials.toString(); this.flowClass = flowClass; this.eppInput = eppInput; this.trid = trid; this.sessionMetadata = sessionMetadata; this.credentials = credentials; + this.eppRequestSource = eppRequestSource; this.isDryRun = isDryRun; this.isSuperuser = isSuperuser; this.inputXmlBytes = inputXmlBytes; @@ -84,6 +87,7 @@ public class FlowRunner { sessionMetadata, prettyPrint(inputXmlBytes).replaceAll("\n", "\n\t"), credentials, + eppRequestSource, isDryRun ? "DRY_RUN" : "LIVE", isSuperuser ? "SUPERUSER" : "NORMAL"); if (!isTransactional()) { @@ -138,6 +142,7 @@ public class FlowRunner { trid, sessionMetadata, credentials, + eppRequestSource, isSuperuser, now, inputXmlBytes); diff --git a/java/google/registry/flows/HttpSessionMetadata.java b/java/google/registry/flows/HttpSessionMetadata.java index 37b6d439e..cb93ec1ed 100644 --- a/java/google/registry/flows/HttpSessionMetadata.java +++ b/java/google/registry/flows/HttpSessionMetadata.java @@ -52,9 +52,4 @@ public class HttpSessionMetadata extends SessionMetadata { protected Object getProperty(String key) { return session.getAttribute(key); } - - @Override - public SessionSource getSessionSource() { - return SessionSource.HTTP; - } } diff --git a/java/google/registry/flows/ResourceCreateOrMutateFlow.java b/java/google/registry/flows/ResourceCreateOrMutateFlow.java index 7f230df6a..02c196c89 100644 --- a/java/google/registry/flows/ResourceCreateOrMutateFlow.java +++ b/java/google/registry/flows/ResourceCreateOrMutateFlow.java @@ -20,7 +20,6 @@ import static google.registry.model.ofy.ObjectifyService.ofy; import com.googlecode.objectify.Key; import google.registry.flows.EppException.AuthorizationErrorException; -import google.registry.flows.SessionMetadata.SessionSource; import google.registry.model.EppResource; import google.registry.model.domain.Period; import google.registry.model.domain.metadata.MetadataExtension; @@ -124,8 +123,7 @@ public abstract class ResourceCreateOrMutateFlow /** Ensure that, if a metadata command exists, it is being passed from a tool-created session. */ void validateMetadataExtension() throws EppException { - if (!(metadataExtension == null - || sessionMetadata.getSessionSource().equals(SessionSource.TOOL))) { + if (!(metadataExtension == null || eppRequestSource.equals(EppRequestSource.TOOL))) { throw new OnlyToolCanPassMetadataException(); } } diff --git a/java/google/registry/flows/SessionMetadata.java b/java/google/registry/flows/SessionMetadata.java index 8cc6f8290..a5e262581 100644 --- a/java/google/registry/flows/SessionMetadata.java +++ b/java/google/registry/flows/SessionMetadata.java @@ -26,26 +26,6 @@ import java.util.Set; /** Class to allow setting and retrieving session information in flows. */ public abstract class SessionMetadata { - /** - * An enum that identifies the origin of the session. - */ - public enum SessionSource { - /** e.g. {@code EppConsoleServlet} */ - CONSOLE, - - /** e.g. {@code EppTlsServlet} */ - HTTP, - - /** e.g. {@code EppToolServlet} */ - TOOL, - - /** e.g. {@code LoadTestAction} */ - LOADTEST, - - /** Direct flow runs (default for e.g. testing) */ - NONE - } - /** The key used for looking up the current client id on the session object. */ protected static final String CLIENT_ID_KEY = "CLIENT_ID"; @@ -95,16 +75,6 @@ public abstract class SessionMetadata { return getProperty(Set.class, SERVICE_EXTENSIONS_KEY); } - public abstract SessionSource getSessionSource(); - - /** - * Subclasses can override if they present a need to change the session - * source at runtime (e.g. anonymous classes created for testing) - */ - public void setSessionSource(@SuppressWarnings("unused") SessionSource source) { - throw new UnsupportedOperationException(); - } - public void setClientId(String clientId) { setPropertyChecked(CLIENT_ID_KEY, clientId); } @@ -132,7 +102,6 @@ public abstract class SessionMetadata { .add("system hash code", System.identityHashCode(this)) .add("clientId", getClientId()) .add("failedLoginAttempts", getFailedLoginAttempts()) - .add("sessionSource", getSessionSource()) .add("serviceExtensionUris", Joiner.on('.').join(nullToEmpty(getServiceExtensionUris()))) .toString(); } diff --git a/java/google/registry/flows/StatelessRequestSessionMetadata.java b/java/google/registry/flows/StatelessRequestSessionMetadata.java index c12a7d43e..80d6e9295 100644 --- a/java/google/registry/flows/StatelessRequestSessionMetadata.java +++ b/java/google/registry/flows/StatelessRequestSessionMetadata.java @@ -21,15 +21,12 @@ public class StatelessRequestSessionMetadata extends SessionMetadata { private final String clientId; private final Set serviceExtensionUris; - private final SessionSource sessionSource; public StatelessRequestSessionMetadata( String clientId, - Set serviceExtensionUris, - SessionSource source) { + Set serviceExtensionUris) { this.clientId = clientId; this.serviceExtensionUris = serviceExtensionUris; - this.sessionSource = source; } @Override @@ -42,11 +39,6 @@ public class StatelessRequestSessionMetadata extends SessionMetadata { return serviceExtensionUris; } - @Override - public SessionSource getSessionSource() { - return sessionSource; - } - @Override public void invalidate() { throw new UnsupportedOperationException(); diff --git a/java/google/registry/tools/ValidateLoginCredentialsCommand.java b/java/google/registry/tools/ValidateLoginCredentialsCommand.java index edd8cac14..3aacb7aed 100644 --- a/java/google/registry/tools/ValidateLoginCredentialsCommand.java +++ b/java/google/registry/tools/ValidateLoginCredentialsCommand.java @@ -31,6 +31,7 @@ import com.google.template.soy.data.SoyMapData; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; +import google.registry.flows.EppRequestSource; import google.registry.flows.FlowRunner; import google.registry.flows.HttpSessionMetadata; import google.registry.flows.TlsCredentials; @@ -109,6 +110,7 @@ final class ValidateLoginCredentialsCommand implements RemoteApiCommand, GtechCo clientCertificateHash, Optional.of(clientIpAddress), "placeholder"), // behave as if we have SNI on, since we're validating a cert + EppRequestSource.TOOL, false, false, inputXmlBytes, diff --git a/java/google/registry/ui/server/api/CheckApiAction.java b/java/google/registry/ui/server/api/CheckApiAction.java index 84889bf67..1c5b7a0d3 100644 --- a/java/google/registry/ui/server/api/CheckApiAction.java +++ b/java/google/registry/ui/server/api/CheckApiAction.java @@ -37,9 +37,9 @@ import dagger.Provides; import google.registry.config.RegistryEnvironment; import google.registry.flows.EppException; +import google.registry.flows.EppRequestSource; import google.registry.flows.FlowRunner; import google.registry.flows.PasswordOnlyTransportCredentials; -import google.registry.flows.SessionMetadata.SessionSource; import google.registry.flows.StatelessRequestSessionMetadata; import google.registry.flows.domain.DomainCheckFlow; import google.registry.model.domain.fee.FeeCheckResponseExtension; @@ -80,8 +80,7 @@ public class CheckApiAction implements Runnable { private final StatelessRequestSessionMetadata sessionMetadata = new StatelessRequestSessionMetadata( RegistryEnvironment.get().config().getCheckApiServletRegistrarClientId(), - ImmutableSet.of(FEE_0_6.getUri()), - SessionSource.HTTP); + ImmutableSet.of(FEE_0_6.getUri())); @Inject @Parameter("domain") String domain; @Inject Response response; @@ -119,6 +118,7 @@ public class CheckApiAction implements Runnable { Trid.create(getClass().getSimpleName()), sessionMetadata, new PasswordOnlyTransportCredentials(), + EppRequestSource.CHECK_API, false, false, inputXmlBytes, diff --git a/javatests/google/registry/flows/EppConsoleActionTest.java b/javatests/google/registry/flows/EppConsoleActionTest.java index 23be9de00..ceda30306 100644 --- a/javatests/google/registry/flows/EppConsoleActionTest.java +++ b/javatests/google/registry/flows/EppConsoleActionTest.java @@ -44,7 +44,7 @@ public class EppConsoleActionTest extends ShardableTestCase { .build(); @Test - public void testPassesArgumentsThrough() { + public void doTest() { EppConsoleAction action = new EppConsoleAction(); action.inputXmlBytes = INPUT_XML_BYTES; action.session = new BasicHttpSession(); @@ -57,6 +57,7 @@ public class EppConsoleActionTest extends ShardableTestCase { verify(action.eppRequestHandler).executeEpp( metadataCaptor.capture(), credentialsCaptor.capture(), + eq(EppRequestSource.CONSOLE), eq(false), eq(false), eq(INPUT_XML_BYTES)); diff --git a/javatests/google/registry/flows/EppTestCase.java b/javatests/google/registry/flows/EppTestCase.java index d67776257..04dc2b8d7 100644 --- a/javatests/google/registry/flows/EppTestCase.java +++ b/javatests/google/registry/flows/EppTestCase.java @@ -117,7 +117,13 @@ public class EppTestCase extends ShardableTestCase { handler.eppController = new EppController(); handler.eppController.clock = clock; handler.eppController.metrics = mock(EppMetrics.class); - handler.executeEpp(sessionMetadata, credentials, false, isSuperuser, inputXml.getBytes(UTF_8)); + handler.executeEpp( + sessionMetadata, + credentials, + EppRequestSource.UNIT_TEST, + false, + isSuperuser, + inputXml.getBytes(UTF_8)); assertThat(response.getStatus()).isEqualTo(SC_OK); assertThat(response.getContentType()).isEqualTo(APPLICATION_EPP_XML_UTF8); String result = response.getPayload(); diff --git a/javatests/google/registry/flows/EppTlsActionTest.java b/javatests/google/registry/flows/EppTlsActionTest.java index 8895a9f79..859b675bc 100644 --- a/javatests/google/registry/flows/EppTlsActionTest.java +++ b/javatests/google/registry/flows/EppTlsActionTest.java @@ -51,6 +51,7 @@ public class EppTlsActionTest extends ShardableTestCase { verify(action.eppRequestHandler).executeEpp( captor.capture(), same(action.tlsCredentials), + eq(EppRequestSource.TLS), eq(false), eq(false), eq(INPUT_XML_BYTES)); diff --git a/javatests/google/registry/flows/EppToolActionTest.java b/javatests/google/registry/flows/EppToolActionTest.java index bfa1df7a9..62331c453 100644 --- a/javatests/google/registry/flows/EppToolActionTest.java +++ b/javatests/google/registry/flows/EppToolActionTest.java @@ -42,6 +42,7 @@ public class EppToolActionTest { verify(action.eppRequestHandler).executeEpp( captor.capture(), isA(PasswordOnlyTransportCredentials.class), + eq(EppRequestSource.TOOL), eq(isDryRun), eq(isSuperuser), eq(action.xml.getBytes(UTF_8))); diff --git a/javatests/google/registry/flows/FlowTestCase.java b/javatests/google/registry/flows/FlowTestCase.java index 3a30872b0..b20a4ea03 100644 --- a/javatests/google/registry/flows/FlowTestCase.java +++ b/javatests/google/registry/flows/FlowTestCase.java @@ -34,7 +34,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; -import google.registry.flows.SessionMetadata.SessionSource; import google.registry.flows.picker.FlowPicker; import google.registry.model.billing.BillingEvent; import google.registry.model.domain.GracePeriod; @@ -92,13 +91,13 @@ public abstract class FlowTestCase { protected SessionMetadata sessionMetadata; protected FakeClock clock = new FakeClock(DateTime.now(UTC)); protected TransportCredentials credentials = new PasswordOnlyTransportCredentials(); + protected EppRequestSource eppRequestSource = EppRequestSource.UNIT_TEST; @Before public void init() throws Exception { sessionMetadata = new TestSessionMetadata(); sessionMetadata.setClientId("TheRegistrar"); sessionMetadata.setServiceExtensionUris(ProtocolDefinition.getVisibleServiceExtensionUris()); - sessionMetadata.setSessionSource(SessionSource.NONE); ofy().saveWithoutBackup().entity(new ClaimsListSingleton()).now(); inject.setStaticField(Ofy.class, "clock", clock); // For transactional flows. } @@ -133,6 +132,7 @@ public abstract class FlowTestCase { getTrid(), sessionMetadata, credentials, + eppRequestSource, commitMode.equals(CommitMode.DRY_RUN), userPrivileges.equals(UserPrivileges.SUPERUSER), "".getBytes(), diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java index 5f9903202..317f0ae3a 100644 --- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java @@ -47,12 +47,12 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import google.registry.flows.EppException.UnimplementedExtensionException; +import google.registry.flows.EppRequestSource; import google.registry.flows.LoggedInFlow.UndeclaredServiceExtensionException; import google.registry.flows.ResourceCreateFlow.ResourceAlreadyExistsException; import google.registry.flows.ResourceCreateOrMutateFlow.OnlyToolCanPassMetadataException; import google.registry.flows.ResourceFlow.BadCommandForRegistryPhaseException; import google.registry.flows.ResourceFlowTestCase; -import google.registry.flows.SessionMetadata.SessionSource; import google.registry.flows.domain.BaseDomainCreateFlow.AcceptedTooLongAgoException; import google.registry.flows.domain.BaseDomainCreateFlow.ClaimsPeriodEndedException; import google.registry.flows.domain.BaseDomainCreateFlow.ExpiredClaimException; @@ -287,7 +287,7 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase".getBytes(), diff --git a/javatests/google/registry/testing/TestSessionMetadata.java b/javatests/google/registry/testing/TestSessionMetadata.java index 5537a71ce..0d3ef7c24 100644 --- a/javatests/google/registry/testing/TestSessionMetadata.java +++ b/javatests/google/registry/testing/TestSessionMetadata.java @@ -25,7 +25,6 @@ public class TestSessionMetadata extends SessionMetadata { private final Map properties = new HashMap<>(); private boolean isValid = true; - private SessionSource sessionSource = SessionSource.NONE; @Override protected void setProperty(String key, Object value) { @@ -47,16 +46,6 @@ public class TestSessionMetadata extends SessionMetadata { isValid = false; } - @Override - public SessionSource getSessionSource() { - return sessionSource; - } - - @Override - public void setSessionSource(SessionSource source) { - sessionSource = source; - } - public boolean isValid() { return isValid; }