diff --git a/java/google/registry/flows/EppConsoleAction.java b/java/google/registry/flows/EppConsoleAction.java index 88e875c34..a8d586d51 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()), + false, // This endpoint is never a dry run. inputXmlBytes); } } diff --git a/java/google/registry/flows/EppController.java b/java/google/registry/flows/EppController.java index be81eab93..53b3a79f5 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, + boolean isDryRun, byte[] inputXmlBytes) { Trid trid = null; try { @@ -72,6 +73,7 @@ public final class EppController { trid, sessionMetadata, credentials, + isDryRun, inputXmlBytes, metrics, clock); diff --git a/java/google/registry/flows/EppRequestHandler.java b/java/google/registry/flows/EppRequestHandler.java index 8719bd672..b22e72040 100644 --- a/java/google/registry/flows/EppRequestHandler.java +++ b/java/google/registry/flows/EppRequestHandler.java @@ -42,10 +42,12 @@ public class EppRequestHandler { public void executeEpp( SessionMetadata sessionMetadata, TransportCredentials credentials, + boolean isDryRun, byte[] inputXmlBytes) { try { response.setPayload(new String( - eppController.handleEppCommand(sessionMetadata, credentials, inputXmlBytes), UTF_8)); + eppController.handleEppCommand( + sessionMetadata, credentials, isDryRun, 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/EppTlsAction.java b/java/google/registry/flows/EppTlsAction.java index ae4563075..008cb7bc9 100644 --- a/java/google/registry/flows/EppTlsAction.java +++ b/java/google/registry/flows/EppTlsAction.java @@ -46,7 +46,11 @@ public class EppTlsAction implements Runnable { if (!tlsCredentials.hasSni()) { logger.warning("Request did not include required SNI header."); } - eppRequestHandler.executeEpp(new HttpSessionMetadata(session), tlsCredentials, inputXmlBytes); + eppRequestHandler.executeEpp( + new HttpSessionMetadata(session), + tlsCredentials, + false, // This endpoint is never a dry run. + inputXmlBytes); } } diff --git a/java/google/registry/flows/EppToolAction.java b/java/google/registry/flows/EppToolAction.java index 4a8da23d8..345dd143a 100644 --- a/java/google/registry/flows/EppToolAction.java +++ b/java/google/registry/flows/EppToolAction.java @@ -51,10 +51,10 @@ public class EppToolAction implements Runnable { new StatelessRequestSessionMetadata( clientIdentifier, superuser, - dryRun, ProtocolDefinition.getVisibleServiceExtensionUris(), SessionSource.TOOL), new PasswordOnlyTransportCredentials(), + dryRun, xml.getBytes(UTF_8)); } diff --git a/java/google/registry/flows/FlowRunner.java b/java/google/registry/flows/FlowRunner.java index 734778666..90b4d4ae6 100644 --- a/java/google/registry/flows/FlowRunner.java +++ b/java/google/registry/flows/FlowRunner.java @@ -36,7 +36,7 @@ import org.joda.time.DateTime; /** Run a flow, either transactionally or not, with logging and retrying as needed. */ public class FlowRunner { - private static final String COMMAND_LOG_FORMAT = "EPP Command" + Strings.repeat("\n\t%s", 5); + private static final String COMMAND_LOG_FORMAT = "EPP Command" + Strings.repeat("\n\t%s", 6); private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); @@ -44,6 +44,7 @@ public class FlowRunner { private final EppInput eppInput; private final Trid trid; private final SessionMetadata sessionMetadata; + private final boolean isDryRun; private final TransportCredentials credentials; private final byte[] inputXmlBytes; private final EppMetrics metrics; @@ -55,6 +56,7 @@ public class FlowRunner { Trid trid, SessionMetadata sessionMetadata, TransportCredentials credentials, + boolean isDryRun, byte[] inputXmlBytes, final EppMetrics metrics, Clock clock) { @@ -64,6 +66,7 @@ public class FlowRunner { this.trid = trid; this.sessionMetadata = sessionMetadata; this.credentials = credentials; + this.isDryRun = isDryRun; this.inputXmlBytes = inputXmlBytes; this.metrics = metrics; this.clock = clock; @@ -77,7 +80,8 @@ public class FlowRunner { clientId, sessionMetadata, prettyPrint(inputXmlBytes).replaceAll("\n", "\n\t"), - credentials); + credentials, + isDryRun ? "DRY_RUN" : "LIVE"); if (!isTransactional()) { if (metrics != null) { metrics.incrementAttempts(); @@ -100,7 +104,7 @@ public class FlowRunner { } try { EppOutput output = createAndInitFlow(ofy().getTransactionTime()).run(); - if (sessionMetadata.isDryRun()) { + if (isDryRun) { throw new DryRunException(output); } return output; diff --git a/java/google/registry/flows/SessionMetadata.java b/java/google/registry/flows/SessionMetadata.java index 29c1a4b80..e4fcae772 100644 --- a/java/google/registry/flows/SessionMetadata.java +++ b/java/google/registry/flows/SessionMetadata.java @@ -137,12 +137,6 @@ public abstract class SessionMetadata { setPropertyChecked(FAILED_LOGIN_ATTEMPTS_KEY, null); } - // These three methods are here to allow special permissions if a derived class overrides them. - - public boolean isDryRun() { - return false; - } - @Override public String toString() { return toStringHelper(getClass()) diff --git a/java/google/registry/flows/StatelessRequestSessionMetadata.java b/java/google/registry/flows/StatelessRequestSessionMetadata.java index d34224970..78041c611 100644 --- a/java/google/registry/flows/StatelessRequestSessionMetadata.java +++ b/java/google/registry/flows/StatelessRequestSessionMetadata.java @@ -21,19 +21,16 @@ public class StatelessRequestSessionMetadata extends SessionMetadata { private final String clientId; private final boolean isSuperuser; - private final boolean isDryRun; private final Set serviceExtensionUris; private final SessionSource sessionSource; public StatelessRequestSessionMetadata( String clientId, boolean isSuperuser, - boolean isDryRun, Set serviceExtensionUris, SessionSource source) { this.clientId = clientId; this.isSuperuser = isSuperuser; - this.isDryRun = isDryRun; this.serviceExtensionUris = serviceExtensionUris; this.sessionSource = source; } @@ -48,11 +45,6 @@ public class StatelessRequestSessionMetadata extends SessionMetadata { return isSuperuser; } - @Override - public boolean isDryRun() { - return isDryRun; - } - @Override public Set getServiceExtensionUris() { return serviceExtensionUris; diff --git a/java/google/registry/tools/ValidateLoginCredentialsCommand.java b/java/google/registry/tools/ValidateLoginCredentialsCommand.java index 5fbca7eb8..9806ce074 100644 --- a/java/google/registry/tools/ValidateLoginCredentialsCommand.java +++ b/java/google/registry/tools/ValidateLoginCredentialsCommand.java @@ -109,6 +109,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 + false, inputXmlBytes, null, new SystemClock()).run()), UTF_8)); diff --git a/java/google/registry/ui/server/api/CheckApiAction.java b/java/google/registry/ui/server/api/CheckApiAction.java index 2f33a072c..2875851e9 100644 --- a/java/google/registry/ui/server/api/CheckApiAction.java +++ b/java/google/registry/ui/server/api/CheckApiAction.java @@ -81,7 +81,6 @@ public class CheckApiAction implements Runnable { new StatelessRequestSessionMetadata( RegistryEnvironment.get().config().getCheckApiServletRegistrarClientId(), false, - false, ImmutableSet.of(FEE_0_6.getUri()), SessionSource.HTTP); @@ -121,6 +120,7 @@ public class CheckApiAction implements Runnable { Trid.create(getClass().getSimpleName()), sessionMetadata, new PasswordOnlyTransportCredentials(), + false, inputXmlBytes, null, clock) diff --git a/javatests/google/registry/flows/EppConsoleActionTest.java b/javatests/google/registry/flows/EppConsoleActionTest.java index b523f9fda..22a37fd34 100644 --- a/javatests/google/registry/flows/EppConsoleActionTest.java +++ b/javatests/google/registry/flows/EppConsoleActionTest.java @@ -55,12 +55,11 @@ public class EppConsoleActionTest extends ShardableTestCase { ArgumentCaptor.forClass(TransportCredentials.class); ArgumentCaptor metadataCaptor = ArgumentCaptor.forClass(SessionMetadata.class); verify(action.eppRequestHandler).executeEpp( - metadataCaptor.capture(), credentialsCaptor.capture(), eq(INPUT_XML_BYTES)); + metadataCaptor.capture(), credentialsCaptor.capture(), eq(false), eq(INPUT_XML_BYTES)); assertThat(((GaeUserCredentials) credentialsCaptor.getValue()).gaeUser.getEmail()) .isEqualTo("person@example.com"); SessionMetadata sessionMetadata = metadataCaptor.getValue(); assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier"); - assertThat(sessionMetadata.isDryRun()).isFalse(); // Should always be false for console. assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser); } diff --git a/javatests/google/registry/flows/EppTestCase.java b/javatests/google/registry/flows/EppTestCase.java index e5dc72597..5f4041af3 100644 --- a/javatests/google/registry/flows/EppTestCase.java +++ b/javatests/google/registry/flows/EppTestCase.java @@ -118,7 +118,7 @@ public class EppTestCase extends ShardableTestCase { handler.eppController = new EppController(); handler.eppController.clock = clock; handler.eppController.metrics = mock(EppMetrics.class); - handler.executeEpp(sessionMetadata, credentials, inputXml.getBytes(UTF_8)); + handler.executeEpp(sessionMetadata, credentials, false, 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 d1f8c7c03..06276f742 100644 --- a/javatests/google/registry/flows/EppTlsActionTest.java +++ b/javatests/google/registry/flows/EppTlsActionTest.java @@ -49,10 +49,9 @@ public class EppTlsActionTest extends ShardableTestCase { action.run(); ArgumentCaptor captor = ArgumentCaptor.forClass(SessionMetadata.class); verify(action.eppRequestHandler) - .executeEpp(captor.capture(), same(action.tlsCredentials), eq(INPUT_XML_BYTES)); + .executeEpp(captor.capture(), same(action.tlsCredentials), eq(false), eq(INPUT_XML_BYTES)); SessionMetadata sessionMetadata = captor.getValue(); assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier"); - assertThat(sessionMetadata.isDryRun()).isFalse(); // Should always be false for TLS. assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser); } diff --git a/javatests/google/registry/flows/EppToolActionTest.java b/javatests/google/registry/flows/EppToolActionTest.java index fc8cfba2f..d2a297594 100644 --- a/javatests/google/registry/flows/EppToolActionTest.java +++ b/javatests/google/registry/flows/EppToolActionTest.java @@ -42,10 +42,10 @@ public class EppToolActionTest { verify(action.eppRequestHandler).executeEpp( captor.capture(), isA(PasswordOnlyTransportCredentials.class), + eq(dryRun), eq(action.xml.getBytes(UTF_8))); SessionMetadata sessionMetadata = captor.getValue(); assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier"); - assertThat(sessionMetadata.isDryRun()).isEqualTo(dryRun); assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser); } diff --git a/javatests/google/registry/flows/FlowTestCase.java b/javatests/google/registry/flows/FlowTestCase.java index 2d54cc8d2..179e5316a 100644 --- a/javatests/google/registry/flows/FlowTestCase.java +++ b/javatests/google/registry/flows/FlowTestCase.java @@ -87,8 +87,6 @@ public abstract class FlowTestCase { @Rule public final InjectRule inject = new InjectRule(); - private FlowRunner flowRunner; - protected EppLoader eppLoader; protected Class flowClass; protected TestSessionMetadata sessionMetadata; @@ -97,7 +95,6 @@ public abstract class FlowTestCase { @Before public void init() throws Exception { - flowRunner = null; sessionMetadata = new TestSessionMetadata(); sessionMetadata.setClientId("TheRegistrar"); sessionMetadata.setServiceExtensionUris(ProtocolDefinition.getVisibleServiceExtensionUris()); @@ -123,16 +120,8 @@ public abstract class FlowTestCase { return readResourceUtf8(getClass(), "testdata/" + filename); } - /** Lazily load the flow, since it may fail to initialize if the environment isn't set up yet. */ - public FlowRunner getFlowRunner() throws Exception { - if (flowRunner == null) { - flowRunner = createFlowRunner(); - } - return flowRunner; - } - /** Load a flow from an epp object. */ - private FlowRunner createFlowRunner() throws Exception { + private FlowRunner getFlowRunner(CommitMode commitMode) throws Exception { EppInput eppInput = eppLoader.getEpp(); flowClass = firstNonNull(flowClass, FlowPicker.getFlowClass(eppInput)); Class expectedFlowClass = new TypeInstantiator(getClass()){}.getExactType(); @@ -143,6 +132,7 @@ public abstract class FlowTestCase { getTrid(), sessionMetadata, credentials, + commitMode.equals(CommitMode.DRY_RUN), "".getBytes(), null, clock); @@ -163,7 +153,7 @@ public abstract class FlowTestCase { } public void assertTransactionalFlow(boolean isTransactional) throws Exception { - assertThat(getFlowRunner().isTransactional()).isEqualTo(isTransactional); + assertThat(getFlowRunner(CommitMode.LIVE).isTransactional()).isEqualTo(isTransactional); } public void assertNoHistory() throws Exception { @@ -282,8 +272,7 @@ public abstract class FlowTestCase { /** Run a flow, and attempt to marshal the result to EPP or throw if it doesn't validate. */ public EppOutput runFlow(CommitMode commitMode, UserPrivileges userPrivileges) throws Exception { sessionMetadata.setSuperuser(userPrivileges.equals(UserPrivileges.SUPERUSER)); - sessionMetadata.setIsDryRun(commitMode.equals(CommitMode.DRY_RUN)); - EppOutput output = getFlowRunner().run(); + EppOutput output = getFlowRunner(commitMode).run(); marshal(output, ValidationMode.STRICT); return output; } @@ -296,8 +285,7 @@ public abstract class FlowTestCase { CommitMode commitMode, UserPrivileges userPrivileges, String xml, String... ignoredPaths) throws Exception { sessionMetadata.setSuperuser(userPrivileges.equals(UserPrivileges.SUPERUSER)); - sessionMetadata.setIsDryRun(commitMode.equals(CommitMode.DRY_RUN)); - EppOutput eppOutput = getFlowRunner().run(); + EppOutput eppOutput = getFlowRunner(commitMode).run(); if (eppOutput.isResponse()) { assertThat(eppOutput.isSuccess()).isTrue(); } diff --git a/javatests/google/registry/model/EppResourceUtilsTest.java b/javatests/google/registry/model/EppResourceUtilsTest.java index 0e3aa24d3..5af02b8c5 100644 --- a/javatests/google/registry/model/EppResourceUtilsTest.java +++ b/javatests/google/registry/model/EppResourceUtilsTest.java @@ -86,6 +86,7 @@ public class EppResourceUtilsTest { Trid.create(null, "server-trid"), sessionMetadata, new PasswordOnlyTransportCredentials(), + false, "".getBytes(), null, clock) diff --git a/javatests/google/registry/testing/TestSessionMetadata.java b/javatests/google/registry/testing/TestSessionMetadata.java index c243a37d9..5537a71ce 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 boolean isDryRun = false; private SessionSource sessionSource = SessionSource.NONE; @Override @@ -58,15 +57,6 @@ public class TestSessionMetadata extends SessionMetadata { sessionSource = source; } - public void setIsDryRun(boolean isDryRun) { - this.isDryRun = isDryRun; - } - - @Override - public boolean isDryRun() { - return isDryRun; - } - public boolean isValid() { return isValid; }