diff --git a/java/google/registry/flows/HttpSessionMetadata.java b/java/google/registry/flows/HttpSessionMetadata.java index cb93ec1ed..e17c110de 100644 --- a/java/google/registry/flows/HttpSessionMetadata.java +++ b/java/google/registry/flows/HttpSessionMetadata.java @@ -14,42 +14,77 @@ package google.registry.flows; -import static com.google.common.base.Preconditions.checkState; +import static com.google.common.base.MoreObjects.toStringHelper; +import static google.registry.util.CollectionUtils.nullToEmpty; + +import com.google.common.base.Joiner; +import com.google.common.base.Optional; + +import java.util.Set; import javax.servlet.http.HttpSession; /** A metadata class that is a wrapper around {@link HttpSession}. */ -public class HttpSessionMetadata extends SessionMetadata { +public class HttpSessionMetadata implements SessionMetadata { + + private static final String CLIENT_ID = "CLIENT_ID"; + private static final String SERVICE_EXTENSIONS = "SERVICE_EXTENSIONS"; + private static final String FAILED_LOGIN_ATTEMPTS = "FAILED_LOGIN_ATTEMPTS"; private final HttpSession session; - private boolean isValid = true; public HttpSessionMetadata(HttpSession session) { this.session = session; } - @Override - protected void checkValid() { - checkState(isValid, "This session has been invalidated."); - } - @Override public void invalidate() { session.invalidate(); - isValid = false; } @Override - protected void setProperty(String key, Object value) { - if (value == null) { - session.removeAttribute(key); - } else { - session.setAttribute(key, value); - } + public String getClientId() { + return (String) session.getAttribute(CLIENT_ID); } @Override - protected Object getProperty(String key) { - return session.getAttribute(key); + @SuppressWarnings("unchecked") + public Set getServiceExtensionUris() { + return (Set) session.getAttribute(SERVICE_EXTENSIONS); + } + + @Override + public void setClientId(String clientId) { + session.setAttribute(CLIENT_ID, clientId); + } + + @Override + public void setServiceExtensionUris(Set serviceExtensionUris) { + session.setAttribute(SERVICE_EXTENSIONS, serviceExtensionUris); + } + + @Override + public int getFailedLoginAttempts() { + return Optional.fromNullable((Integer) session.getAttribute(FAILED_LOGIN_ATTEMPTS)).or(0); + } + + @Override + public void incrementFailedLoginAttempts() { + session.setAttribute(FAILED_LOGIN_ATTEMPTS, getFailedLoginAttempts() + 1); + } + + @Override + public void resetFailedLoginAttempts() { + session.removeAttribute(FAILED_LOGIN_ATTEMPTS); + } + + @Override + public String toString() { + return toStringHelper(getClass()) + .add("system hash code", System.identityHashCode(this)) + .add("clientId", getClientId()) + .add("failedLoginAttempts", getFailedLoginAttempts()) + .add("serviceExtensionUris", Joiner.on('.').join(nullToEmpty(getServiceExtensionUris()))) + .toString(); } } diff --git a/java/google/registry/flows/SessionMetadata.java b/java/google/registry/flows/SessionMetadata.java index a5e262581..f85bd691d 100644 --- a/java/google/registry/flows/SessionMetadata.java +++ b/java/google/registry/flows/SessionMetadata.java @@ -14,95 +14,29 @@ package google.registry.flows; -import static com.google.common.base.MoreObjects.toStringHelper; -import static com.google.common.base.Preconditions.checkNotNull; -import static google.registry.util.CollectionUtils.nullToEmpty; - -import com.google.common.base.Joiner; -import com.google.common.base.Optional; - import java.util.Set; -/** Class to allow setting and retrieving session information in flows. */ -public abstract class SessionMetadata { - - /** The key used for looking up the current client id on the session object. */ - protected static final String CLIENT_ID_KEY = "CLIENT_ID"; - - /** The key used for looking up the service extensions on the session object. */ - protected static final String SERVICE_EXTENSIONS_KEY = "SERVICE_EXTENSIONS"; - - /** The key used for looking up the number of failed login attempts. */ - protected static final String FAILED_LOGIN_ATTEMPTS_KEY = "FAILED_LOGIN_ATTEMPTS"; - - protected abstract void setProperty(String key, Object value); - - protected abstract Object getProperty(String key); +/** Object to allow setting and retrieving session information in flows. */ +public interface SessionMetadata { /** * Invalidates the session. A new instance must be created after this for future sessions. * Attempts to invoke methods of this class after this method has been called will throw * {@code IllegalStateException}. */ - public abstract void invalidate(); + void invalidate(); - /** Subclasses can override this to verify that this is a valid session. */ - protected void checkValid() {} + String getClientId(); - /** Check that the session is valid and set a property. */ - private void setPropertyChecked(String key, Object value) { - checkValid(); - setProperty(key, value); - } + Set getServiceExtensionUris(); - /** - * Check that the session is valid and get a property as a given type. - * - * @param clazz type to return, specified as a param to enforce typesafe generics - * @see "http://errorprone.info/bugpattern/TypeParameterUnusedInFormals" - */ - private T getProperty(Class clazz, String key) { - checkValid(); - return clazz.cast(getProperty(key)); - } + void setClientId(String clientId); - public String getClientId() { - return getProperty(String.class, CLIENT_ID_KEY); - } + void setServiceExtensionUris(Set serviceExtensionUris); - @SuppressWarnings("unchecked") - public Set getServiceExtensionUris() { - return getProperty(Set.class, SERVICE_EXTENSIONS_KEY); - } + int getFailedLoginAttempts(); - public void setClientId(String clientId) { - setPropertyChecked(CLIENT_ID_KEY, clientId); - } + void incrementFailedLoginAttempts(); - public void setServiceExtensionUris(Set serviceExtensionUris) { - setPropertyChecked(SERVICE_EXTENSIONS_KEY, checkNotNull(serviceExtensionUris)); - } - - public int getFailedLoginAttempts() { - return Optional.fromNullable(getProperty(Integer.class, FAILED_LOGIN_ATTEMPTS_KEY)) - .or(0); - } - - public void incrementFailedLoginAttempts() { - setPropertyChecked(FAILED_LOGIN_ATTEMPTS_KEY, getFailedLoginAttempts() + 1); - } - - public void resetFailedLoginAttempts() { - setPropertyChecked(FAILED_LOGIN_ATTEMPTS_KEY, null); - } - - @Override - public String toString() { - return toStringHelper(getClass()) - .add("system hash code", System.identityHashCode(this)) - .add("clientId", getClientId()) - .add("failedLoginAttempts", getFailedLoginAttempts()) - .add("serviceExtensionUris", Joiner.on('.').join(nullToEmpty(getServiceExtensionUris()))) - .toString(); - } + void resetFailedLoginAttempts(); } diff --git a/java/google/registry/flows/StatelessRequestSessionMetadata.java b/java/google/registry/flows/StatelessRequestSessionMetadata.java index 80d6e9295..035fb4712 100644 --- a/java/google/registry/flows/StatelessRequestSessionMetadata.java +++ b/java/google/registry/flows/StatelessRequestSessionMetadata.java @@ -14,19 +14,25 @@ package google.registry.flows; +import static com.google.common.base.MoreObjects.toStringHelper; +import static dagger.internal.Preconditions.checkNotNull; +import static google.registry.util.CollectionUtils.nullToEmpty; + +import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableSet; + import java.util.Set; /** A read-only {@link SessionMetadata} that doesn't support login/logout. */ -public class StatelessRequestSessionMetadata extends SessionMetadata { +public class StatelessRequestSessionMetadata implements SessionMetadata { private final String clientId; - private final Set serviceExtensionUris; + private final ImmutableSet serviceExtensionUris; public StatelessRequestSessionMetadata( - String clientId, - Set serviceExtensionUris) { - this.clientId = clientId; - this.serviceExtensionUris = serviceExtensionUris; + String clientId, ImmutableSet serviceExtensionUris) { + this.clientId = checkNotNull(clientId); + this.serviceExtensionUris = checkNotNull(serviceExtensionUris); } @Override @@ -45,14 +51,38 @@ public class StatelessRequestSessionMetadata extends SessionMetadata { } @Override - protected void setProperty(String key, Object value) { + public void setClientId(String clientId) { throw new UnsupportedOperationException(); } @Override - protected Object getProperty(String key) { - // We've overridden the getters of all of the properties that we care about. Return null for - // everything else so that toString() continues to work. - return null; + public void setServiceExtensionUris(Set serviceExtensionUris) { + throw new UnsupportedOperationException(); + } + + @Override + public int getFailedLoginAttempts() { + throw new UnsupportedOperationException(); + } + + @Override + public void incrementFailedLoginAttempts() { + throw new UnsupportedOperationException(); + } + + @Override + public void resetFailedLoginAttempts() { + throw new UnsupportedOperationException(); + } + + + @Override + public String toString() { + return toStringHelper(getClass()) + .add("system hash code", System.identityHashCode(this)) + .add("clientId", getClientId()) + .add("serviceExtensionUris", Joiner.on('.').join(nullToEmpty(getServiceExtensionUris()))) + .toString(); } } + diff --git a/java/google/registry/tools/ValidateLoginCredentialsCommand.java b/java/google/registry/tools/ValidateLoginCredentialsCommand.java index 3aacb7aed..591659aba 100644 --- a/java/google/registry/tools/ValidateLoginCredentialsCommand.java +++ b/java/google/registry/tools/ValidateLoginCredentialsCommand.java @@ -33,7 +33,7 @@ import com.beust.jcommander.Parameters; import google.registry.flows.EppRequestSource; import google.registry.flows.FlowRunner; -import google.registry.flows.HttpSessionMetadata; +import google.registry.flows.SessionMetadata; import google.registry.flows.TlsCredentials; import google.registry.flows.session.LoginFlow; import google.registry.model.eppcommon.Trid; @@ -42,11 +42,11 @@ import google.registry.tools.Command.GtechCommand; import google.registry.tools.Command.RemoteApiCommand; import google.registry.tools.params.PathParameter; import google.registry.tools.soy.LoginSoyInfo; -import google.registry.util.BasicHttpSession; import google.registry.util.SystemClock; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Set; import javax.annotation.Nullable; @@ -100,12 +100,13 @@ final class ValidateLoginCredentialsCommand implements RemoteApiCommand, GtechCo .setData(new SoyMapData("clientIdentifier", clientIdentifier, "password", password)) .render() .getBytes(UTF_8); + System.out.println(new String(marshalWithLenientRetry( new FlowRunner( LoginFlow.class, unmarshal(EppInput.class, inputXmlBytes), Trid.create(null), - new HttpSessionMetadata(new BasicHttpSession()), + new StubSessionMetadata(), new TlsCredentials( clientCertificateHash, Optional.of(clientIpAddress), @@ -117,4 +118,38 @@ final class ValidateLoginCredentialsCommand implements RemoteApiCommand, GtechCo null, new SystemClock()).run()), UTF_8)); } + + /** A {@link SessionMetadata} that ignores setters rather than throwing exceptions. */ + private static class StubSessionMetadata implements SessionMetadata { + + @Override + public void setClientId(String clientId) {} + + @Override + public void setServiceExtensionUris(Set serviceExtensionUris) {} + + @Override + public void incrementFailedLoginAttempts() {} + + @Override + public void resetFailedLoginAttempts() {} + + @Override + public void invalidate() {} + + @Override + public String getClientId() { + return null; + } + + @Override + public Set getServiceExtensionUris() { + return null; + } + + @Override + public int getFailedLoginAttempts() { + return 0; + } + } } diff --git a/javatests/google/registry/flows/EppConsoleActionTest.java b/javatests/google/registry/flows/EppConsoleActionTest.java index ceda30306..3039a5296 100644 --- a/javatests/google/registry/flows/EppConsoleActionTest.java +++ b/javatests/google/registry/flows/EppConsoleActionTest.java @@ -22,9 +22,9 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import google.registry.testing.AppEngineRule; +import google.registry.testing.FakeHttpSession; import google.registry.testing.ShardableTestCase; import google.registry.testing.UserInfo; -import google.registry.util.BasicHttpSession; import org.junit.Rule; import org.junit.Test; @@ -47,7 +47,7 @@ public class EppConsoleActionTest extends ShardableTestCase { public void doTest() { EppConsoleAction action = new EppConsoleAction(); action.inputXmlBytes = INPUT_XML_BYTES; - action.session = new BasicHttpSession(); + action.session = new FakeHttpSession(); action.session.setAttribute("CLIENT_ID", "ClientIdentifier"); action.eppRequestHandler = mock(EppRequestHandler.class); action.run(); diff --git a/javatests/google/registry/flows/EppTestCase.java b/javatests/google/registry/flows/EppTestCase.java index 04dc2b8d7..2a53eecea 100644 --- a/javatests/google/registry/flows/EppTestCase.java +++ b/javatests/google/registry/flows/EppTestCase.java @@ -28,10 +28,10 @@ import com.google.common.net.MediaType; import google.registry.model.ofy.Ofy; import google.registry.monitoring.whitebox.EppMetrics; import google.registry.testing.FakeClock; +import google.registry.testing.FakeHttpSession; import google.registry.testing.FakeResponse; import google.registry.testing.InjectRule; import google.registry.testing.ShardableTestCase; -import google.registry.testing.TestSessionMetadata; import org.joda.time.DateTime; import org.junit.Before; @@ -49,7 +49,7 @@ public class EppTestCase extends ShardableTestCase { private final FakeClock clock = new FakeClock(); - private TestSessionMetadata sessionMetadata; + private SessionMetadata sessionMetadata; private TransportCredentials credentials = new PasswordOnlyTransportCredentials(); private boolean isSuperuser; @@ -94,12 +94,16 @@ public class EppTestCase extends ShardableTestCase { String expectedOutput = loadFileWithSubstitutions(getClass(), outputFilename, outputSubstitutions); if (sessionMetadata == null) { - sessionMetadata = new TestSessionMetadata(); + sessionMetadata = new HttpSessionMetadata(new FakeHttpSession()) { + @Override + public void invalidate() { + // When a session is invalidated, reset the sessionMetadata field. + super.invalidate(); + EppTestCase.this.sessionMetadata = null; + } + }; } String actualOutput = executeXmlCommand(input); - if (!sessionMetadata.isValid()) { - sessionMetadata = null; - } assertXmlEqualsWithMessage( expectedOutput, actualOutput, diff --git a/javatests/google/registry/flows/EppTlsActionTest.java b/javatests/google/registry/flows/EppTlsActionTest.java index 859b675bc..a965622d5 100644 --- a/javatests/google/registry/flows/EppTlsActionTest.java +++ b/javatests/google/registry/flows/EppTlsActionTest.java @@ -23,8 +23,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import google.registry.testing.FakeHttpSession; import google.registry.testing.ShardableTestCase; -import google.registry.util.BasicHttpSession; import org.junit.Test; import org.junit.runner.RunWith; @@ -43,7 +43,7 @@ public class EppTlsActionTest extends ShardableTestCase { action.inputXmlBytes = INPUT_XML_BYTES; action.tlsCredentials = mock(TlsCredentials.class); when(action.tlsCredentials.hasSni()).thenReturn(true); - action.session = new BasicHttpSession(); + action.session = new FakeHttpSession(); action.session.setAttribute("CLIENT_ID", "ClientIdentifier"); action.eppRequestHandler = mock(EppRequestHandler.class); action.run(); diff --git a/javatests/google/registry/flows/FlowTestCase.java b/javatests/google/registry/flows/FlowTestCase.java index b20a4ea03..b1f8bb7ec 100644 --- a/javatests/google/registry/flows/FlowTestCase.java +++ b/javatests/google/registry/flows/FlowTestCase.java @@ -49,8 +49,8 @@ import google.registry.model.tmch.ClaimsListShard.ClaimsListSingleton; import google.registry.testing.AppEngineRule; import google.registry.testing.EppLoader; import google.registry.testing.FakeClock; +import google.registry.testing.FakeHttpSession; import google.registry.testing.InjectRule; -import google.registry.testing.TestSessionMetadata; import google.registry.util.TypeUtils.TypeInstantiator; import google.registry.xml.ValidationMode; @@ -95,7 +95,7 @@ public abstract class FlowTestCase { @Before public void init() throws Exception { - sessionMetadata = new TestSessionMetadata(); + sessionMetadata = new HttpSessionMetadata(new FakeHttpSession()); sessionMetadata.setClientId("TheRegistrar"); sessionMetadata.setServiceExtensionUris(ProtocolDefinition.getVisibleServiceExtensionUris()); ofy().saveWithoutBackup().entity(new ClaimsListSingleton()).now(); diff --git a/javatests/google/registry/model/EppResourceUtilsTest.java b/javatests/google/registry/model/EppResourceUtilsTest.java index 368d2e0ee..f858a1b6c 100644 --- a/javatests/google/registry/model/EppResourceUtilsTest.java +++ b/javatests/google/registry/model/EppResourceUtilsTest.java @@ -32,6 +32,7 @@ import com.googlecode.objectify.Key; import google.registry.flows.EppRequestSource; import google.registry.flows.FlowRunner; +import google.registry.flows.HttpSessionMetadata; import google.registry.flows.PasswordOnlyTransportCredentials; import google.registry.flows.SessionMetadata; import google.registry.model.domain.DomainResource; @@ -42,8 +43,8 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.EppLoader; import google.registry.testing.ExceptionRule; import google.registry.testing.FakeClock; +import google.registry.testing.FakeHttpSession; import google.registry.testing.InjectRule; -import google.registry.testing.TestSessionMetadata; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -79,7 +80,7 @@ public class EppResourceUtilsTest { } private void runFlow() throws Exception { - SessionMetadata sessionMetadata = new TestSessionMetadata(); + SessionMetadata sessionMetadata = new HttpSessionMetadata(new FakeHttpSession()); sessionMetadata.setClientId("TheRegistrar"); new FlowRunner( getFlowClass(eppLoader.getEpp()), diff --git a/java/google/registry/util/BasicHttpSession.java b/javatests/google/registry/testing/FakeHttpSession.java similarity index 86% rename from java/google/registry/util/BasicHttpSession.java rename to javatests/google/registry/testing/FakeHttpSession.java index 2deb14eba..f305b0de1 100644 --- a/java/google/registry/util/BasicHttpSession.java +++ b/javatests/google/registry/testing/FakeHttpSession.java @@ -12,7 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -package google.registry.util; +package google.registry.testing; + +import static com.google.common.base.Preconditions.checkState; import java.util.Enumeration; import java.util.HashMap; @@ -23,11 +25,12 @@ import javax.servlet.ServletContext; import javax.servlet.http.HttpSession; import javax.servlet.http.HttpSessionContext; -/** An {@link HttpSession} that only provides support for getting/setting attributes. */ +/** A fake {@link HttpSession} that only provides support for getting/setting attributes. */ @SuppressWarnings("deprecation") -public class BasicHttpSession implements HttpSession { +public class FakeHttpSession implements HttpSession { private final Map map = new HashMap<>(); + boolean isValid = true; @Override @@ -67,7 +70,7 @@ public class BasicHttpSession implements HttpSession { @Override public Object getAttribute(@Nullable String name) { - checkValid(); + checkState(isValid, "This session has been invalidated."); return map.get(name); } @@ -88,7 +91,7 @@ public class BasicHttpSession implements HttpSession { @Override public void setAttribute(@Nullable String name, @Nullable Object value) { - checkValid(); + checkState(isValid, "This session has been invalidated."); map.put(name, value); } @@ -99,7 +102,7 @@ public class BasicHttpSession implements HttpSession { @Override public void removeAttribute(@Nullable String name) { - checkValid(); + checkState(isValid, "This session has been invalidated."); map.remove(name); } @@ -118,10 +121,4 @@ public class BasicHttpSession implements HttpSession { public boolean isNew() { throw new UnsupportedOperationException(); } - - private void checkValid() { - if (!isValid) { - throw new IllegalStateException("This session has been invalidated."); - } - } } diff --git a/javatests/google/registry/testing/TestSessionMetadata.java b/javatests/google/registry/testing/TestSessionMetadata.java deleted file mode 100644 index 0d3ef7c24..000000000 --- a/javatests/google/registry/testing/TestSessionMetadata.java +++ /dev/null @@ -1,52 +0,0 @@ -// 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.testing; - -import static com.google.common.base.Preconditions.checkState; - -import google.registry.flows.SessionMetadata; - -import java.util.HashMap; -import java.util.Map; - -public class TestSessionMetadata extends SessionMetadata { - - private final Map properties = new HashMap<>(); - private boolean isValid = true; - - @Override - protected void setProperty(String key, Object value) { - properties.put(key, value); - } - - @Override - protected Object getProperty(String key) { - return properties.get(key); - } - - @Override - public void checkValid() { - checkState(isValid, "Session was invalidated"); - } - - @Override - public void invalidate() { - isValid = false; - } - - public boolean isValid() { - return isValid; - } -}