diff --git a/java/google/registry/loadtest/LoadTestAction.java b/java/google/registry/loadtest/LoadTestAction.java index 60b5fbfee..c059d91a7 100644 --- a/java/google/registry/loadtest/LoadTestAction.java +++ b/java/google/registry/loadtest/LoadTestAction.java @@ -146,6 +146,8 @@ public class LoadTestAction implements Runnable { @Inject TaskEnqueuer taskEnqueuer; + @Inject XsrfTokenManager xsrfTokenManager; + private final String xmlContactCreateTmpl; private final String xmlContactCreateFail; private final String xmlContactInfo; @@ -163,7 +165,7 @@ public class LoadTestAction implements Runnable { *

Note that the email address is set to empty, because the logged-in user hitting this * endpoint will not be the same as when the tasks themselves fire and hit the epptool endpoint. */ - private final String xsrfToken = XsrfTokenManager.generateToken("admin", ""); + private final String xsrfToken; @Inject LoadTestAction(@Parameter("tld") String tld) { @@ -183,6 +185,7 @@ public class LoadTestAction implements Runnable { xmlHostCreateTmpl = loadXml("host_create"); xmlHostCreateFail = xmlHostCreateTmpl.replace("%host%", EXISTING_HOST); xmlHostInfo = loadXml("host_info").replace("%host%", EXISTING_HOST); + xsrfToken = xsrfTokenManager.generateToken("admin", ""); } @Override diff --git a/java/google/registry/module/backend/BUILD b/java/google/registry/module/backend/BUILD index b332a4a1c..be891abeb 100644 --- a/java/google/registry/module/backend/BUILD +++ b/java/google/registry/module/backend/BUILD @@ -32,6 +32,7 @@ java_library( "//java/google/registry/request", "//java/google/registry/request:modules", "//java/google/registry/request/auth", + "//java/google/registry/security", "//java/google/registry/tmch", "//java/google/registry/util", "@com_google_appengine_api_1_0_sdk", diff --git a/java/google/registry/module/backend/BackendRequestHandler.java b/java/google/registry/module/backend/BackendRequestHandler.java index c6b9036cc..b4faf38b4 100644 --- a/java/google/registry/module/backend/BackendRequestHandler.java +++ b/java/google/registry/module/backend/BackendRequestHandler.java @@ -17,6 +17,7 @@ package google.registry.module.backend; import com.google.appengine.api.users.UserService; import google.registry.request.RequestHandler; import google.registry.request.auth.RequestAuthenticator; +import google.registry.security.XsrfTokenManager; import javax.inject.Inject; import javax.inject.Provider; @@ -27,7 +28,8 @@ public class BackendRequestHandler @Inject BackendRequestHandler( Provider componentBuilderProvider, UserService userService, - RequestAuthenticator requestAuthenticator) { - super(componentBuilderProvider, userService, requestAuthenticator); + RequestAuthenticator requestAuthenticator, + XsrfTokenManager xsrfTokenManager) { + super(componentBuilderProvider, userService, requestAuthenticator, xsrfTokenManager); } } diff --git a/java/google/registry/module/frontend/BUILD b/java/google/registry/module/frontend/BUILD index d10807a80..bfe6b9658 100644 --- a/java/google/registry/module/frontend/BUILD +++ b/java/google/registry/module/frontend/BUILD @@ -19,6 +19,7 @@ java_library( "//java/google/registry/request", "//java/google/registry/request:modules", "//java/google/registry/request/auth", + "//java/google/registry/security", "//java/google/registry/ui", "//java/google/registry/ui/server/registrar", "//java/google/registry/util", diff --git a/java/google/registry/module/frontend/FrontendRequestHandler.java b/java/google/registry/module/frontend/FrontendRequestHandler.java index 3e00fa63f..edcb07ccd 100644 --- a/java/google/registry/module/frontend/FrontendRequestHandler.java +++ b/java/google/registry/module/frontend/FrontendRequestHandler.java @@ -17,6 +17,7 @@ package google.registry.module.frontend; import com.google.appengine.api.users.UserService; import google.registry.request.RequestHandler; import google.registry.request.auth.RequestAuthenticator; +import google.registry.security.XsrfTokenManager; import javax.inject.Inject; import javax.inject.Provider; @@ -27,7 +28,8 @@ public class FrontendRequestHandler @Inject FrontendRequestHandler( Provider componentBuilderProvider, UserService userService, - RequestAuthenticator requestAuthenticator) { - super(componentBuilderProvider, userService, requestAuthenticator); + RequestAuthenticator requestAuthenticator, + XsrfTokenManager xsrfTokenManager) { + super(componentBuilderProvider, userService, requestAuthenticator, xsrfTokenManager); } } diff --git a/java/google/registry/module/tools/BUILD b/java/google/registry/module/tools/BUILD index 5574a6d6a..57eb1e8e7 100644 --- a/java/google/registry/module/tools/BUILD +++ b/java/google/registry/module/tools/BUILD @@ -21,6 +21,7 @@ java_library( "//java/google/registry/request", "//java/google/registry/request:modules", "//java/google/registry/request/auth", + "//java/google/registry/security", "//java/google/registry/tools/server", "//java/google/registry/tools/server/javascrap", "//java/google/registry/util", diff --git a/java/google/registry/module/tools/ToolsRequestHandler.java b/java/google/registry/module/tools/ToolsRequestHandler.java index 4aecd8b2c..d21a4c402 100644 --- a/java/google/registry/module/tools/ToolsRequestHandler.java +++ b/java/google/registry/module/tools/ToolsRequestHandler.java @@ -17,6 +17,7 @@ package google.registry.module.tools; import com.google.appengine.api.users.UserService; import google.registry.request.RequestHandler; import google.registry.request.auth.RequestAuthenticator; +import google.registry.security.XsrfTokenManager; import javax.inject.Inject; import javax.inject.Provider; @@ -27,7 +28,8 @@ public class ToolsRequestHandler @Inject ToolsRequestHandler( Provider componentBuilderProvider, UserService userService, - RequestAuthenticator requestAuthenticator) { - super(componentBuilderProvider, userService, requestAuthenticator); + RequestAuthenticator requestAuthenticator, + XsrfTokenManager xsrfTokenManager) { + super(componentBuilderProvider, userService, requestAuthenticator, xsrfTokenManager); } } diff --git a/java/google/registry/request/RequestHandler.java b/java/google/registry/request/RequestHandler.java index f29319120..336c7c991 100644 --- a/java/google/registry/request/RequestHandler.java +++ b/java/google/registry/request/RequestHandler.java @@ -19,7 +19,6 @@ import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.net.HttpHeaders.LOCATION; import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; import static google.registry.security.XsrfTokenManager.X_CSRF_TOKEN; -import static google.registry.security.XsrfTokenManager.validateToken; import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; import static javax.servlet.http.HttpServletResponse.SC_METHOD_NOT_ALLOWED; import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; @@ -29,6 +28,7 @@ import com.google.appengine.api.users.UserService; import com.google.common.base.Optional; import google.registry.request.auth.AuthResult; import google.registry.request.auth.RequestAuthenticator; +import google.registry.security.XsrfTokenManager; import google.registry.util.FormattingLogger; import google.registry.util.TypeUtils.TypeInstantiator; import java.io.IOException; @@ -36,7 +36,6 @@ import javax.annotation.Nullable; import javax.inject.Provider; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.joda.time.Duration; /** * Dagger-based request processor. @@ -74,12 +73,11 @@ public class RequestHandler> { private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); - private static final Duration XSRF_VALIDITY = Duration.standardDays(1); - private final Router router; private final Provider requestComponentBuilderProvider; private final UserService userService; private final RequestAuthenticator requestAuthenticator; + private final XsrfTokenManager xsrfTokenManager; /** * Constructor for subclasses to create a new request handler for a specific request component. @@ -92,13 +90,16 @@ public class RequestHandler> { * be used to construct new instances of the request component (with the required * request-derived modules provided by this class) * @param userService an instance of the App Engine UserService API - * @param requestAuthenticator an instance of the RequestAuthenticator class + * @param requestAuthenticator an instance of the {@link RequestAuthenticator} class + * @param xsrfTokenManager an instance of the {@link XsrfTokenManager} class */ protected RequestHandler( Provider requestComponentBuilderProvider, UserService userService, - RequestAuthenticator requestAuthenticator) { - this(null, requestComponentBuilderProvider, userService, requestAuthenticator); + RequestAuthenticator requestAuthenticator, + XsrfTokenManager xsrfTokenManager) { + this(null, requestComponentBuilderProvider, userService, requestAuthenticator, + xsrfTokenManager); } /** Creates a new RequestHandler with an explicit component class for test purposes. */ @@ -106,19 +107,22 @@ public class RequestHandler> { Class component, Provider requestComponentBuilderProvider, UserService userService, - RequestAuthenticator requestAuthenticator) { + RequestAuthenticator requestAuthenticator, + XsrfTokenManager xsrfTokenManager) { return new RequestHandler<>( checkNotNull(component), requestComponentBuilderProvider, userService, - requestAuthenticator); + requestAuthenticator, + xsrfTokenManager); } private RequestHandler( @Nullable Class component, Provider requestComponentBuilderProvider, UserService userService, - RequestAuthenticator requestAuthenticator) { + RequestAuthenticator requestAuthenticator, + XsrfTokenManager xsrfTokenManager) { // If the component class isn't explicitly provided, infer it from the class's own typing. // This is safe only for use by subclasses of RequestHandler where the generic parameter is // preserved at runtime, so only expose that option via the protected constructor. @@ -127,6 +131,7 @@ public class RequestHandler> { this.requestComponentBuilderProvider = checkNotNull(requestComponentBuilderProvider); this.userService = checkNotNull(userService); this.requestAuthenticator = checkNotNull(requestAuthenticator); + this.xsrfTokenManager = checkNotNull(xsrfTokenManager); } /** Runs the appropriate action for a servlet request. */ @@ -160,10 +165,9 @@ public class RequestHandler> { return; } if (route.get().shouldXsrfProtect(method) - && !validateToken( + && !xsrfTokenManager.validateToken( nullToEmpty(req.getHeader(X_CSRF_TOKEN)), - route.get().action().xsrfScope(), - XSRF_VALIDITY)) { + route.get().action().xsrfScope())) { rsp.sendError(SC_FORBIDDEN, "Invalid " + X_CSRF_TOKEN); return; } diff --git a/java/google/registry/security/BUILD b/java/google/registry/security/BUILD index a2c54aed3..4d6155398 100644 --- a/java/google/registry/security/BUILD +++ b/java/google/registry/security/BUILD @@ -18,6 +18,7 @@ java_library( "@com_google_code_findbugs_jsr305", "@com_google_guava", "@com_googlecode_json_simple", + "@javax_inject", "@javax_servlet_api", "@joda_time", ], diff --git a/java/google/registry/security/XsrfTokenManager.java b/java/google/registry/security/XsrfTokenManager.java index eb8207bc2..22d141b4c 100644 --- a/java/google/registry/security/XsrfTokenManager.java +++ b/java/google/registry/security/XsrfTokenManager.java @@ -14,7 +14,6 @@ package google.registry.security; -import static com.google.appengine.api.users.UserServiceFactory.getUserService; import static com.google.common.io.BaseEncoding.base64Url; import static google.registry.model.server.ServerSecret.getServerSecret; import static java.nio.charset.StandardCharsets.UTF_8; @@ -25,9 +24,8 @@ import com.google.common.base.Splitter; import com.google.common.hash.Hashing; import google.registry.util.Clock; import google.registry.util.FormattingLogger; -import google.registry.util.NonFinalForTesting; -import google.registry.util.SystemClock; import java.util.List; +import javax.inject.Inject; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -37,10 +35,18 @@ public final class XsrfTokenManager { /** HTTP header used for transmitting XSRF tokens. */ public static final String X_CSRF_TOKEN = "X-CSRF-Token"; + private static final Duration XSRF_VALIDITY = Duration.standardDays(1); + private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); - @NonFinalForTesting - private static Clock clock = new SystemClock(); + private final Clock clock; + private final UserService userService; + + @Inject + public XsrfTokenManager(Clock clock, UserService userService) { + this.clock = clock; + this.userService = userService; + } private static String encodeToken(long creationTime, String scope, String userEmail) { String token = Joiner.on('\t').join(getServerSecret(), userEmail, scope, creationTime); @@ -58,23 +64,22 @@ public final class XsrfTokenManager { * any callback that doesn't have a user shouldn't be able to access any per-user resources * anyways. */ - public static String generateToken(String scope) { + public String generateToken(String scope) { return generateToken(scope, getLoggedInEmailOrEmpty()); } /** Generate an xsrf token for a given scope and user. */ - public static String generateToken(String scope, String email) { + public String generateToken(String scope, String email) { long now = clock.nowUtc().getMillis(); return Joiner.on(':').join(encodeToken(now, scope, email), now); } - private static String getLoggedInEmailOrEmpty() { - UserService userService = getUserService(); + private String getLoggedInEmailOrEmpty() { return userService.isUserLoggedIn() ? userService.getCurrentUser().getEmail() : ""; } - /** Validate an xsrf token, given the scope it was used for and an expiration duration. */ - public static boolean validateToken(String token, String scope, Duration validLifetime) { + /** Validate an xsrf token, given the scope it was used for. */ + public boolean validateToken(String token, String scope) { List tokenParts = Splitter.on(':').splitToList(token); if (tokenParts.size() != 2) { logger.warningfmt("Malformed XSRF token: %s", token); @@ -89,7 +94,7 @@ public final class XsrfTokenManager { logger.warningfmt("Bad timestamp in XSRF token: %s", token); return false; } - if (new DateTime(creationTime).plus(validLifetime).isBefore(clock.nowUtc())) { + if (new DateTime(creationTime).plus(XSRF_VALIDITY).isBefore(clock.nowUtc())) { logger.infofmt("Expired timestamp in XSRF token: %s", token); return false; } @@ -100,6 +105,4 @@ public final class XsrfTokenManager { } return true; } - - private XsrfTokenManager() {} } diff --git a/java/google/registry/tools/AppEngineConnection.java b/java/google/registry/tools/AppEngineConnection.java index d9dc72aa3..5024787ee 100644 --- a/java/google/registry/tools/AppEngineConnection.java +++ b/java/google/registry/tools/AppEngineConnection.java @@ -52,6 +52,7 @@ class AppEngineConnection implements Connection { @Inject HttpRequestFactory requestFactory; @Inject AppEngineConnectionFlags flags; + @Inject XsrfTokenManager xsrfTokenManager; @Inject AppEngineConnection() {} @@ -65,7 +66,7 @@ class AppEngineConnection implements Connection { memoize(new Supplier() { @Override public String get() { - return XsrfTokenManager.generateToken("admin", getUserId()); + return xsrfTokenManager.generateToken("admin", getUserId()); }}); @Override diff --git a/java/google/registry/tools/RegistryToolComponent.java b/java/google/registry/tools/RegistryToolComponent.java index d325e452b..7b26bfc1a 100644 --- a/java/google/registry/tools/RegistryToolComponent.java +++ b/java/google/registry/tools/RegistryToolComponent.java @@ -24,6 +24,7 @@ import google.registry.keyring.api.KeyModule; import google.registry.request.Modules.DatastoreServiceModule; import google.registry.request.Modules.Jackson2Module; import google.registry.request.Modules.URLFetchServiceModule; +import google.registry.request.Modules.UserServiceModule; import google.registry.util.SystemClock.SystemClockModule; import google.registry.util.SystemSleeper.SystemSleeperModule; import google.registry.whois.WhoisModule; @@ -54,6 +55,7 @@ import javax.inject.Singleton; SystemClockModule.class, SystemSleeperModule.class, URLFetchServiceModule.class, + UserServiceModule.class, VoidDnsWriterModule.class, WhoisModule.class, } diff --git a/java/google/registry/ui/server/registrar/ConsoleUiAction.java b/java/google/registry/ui/server/registrar/ConsoleUiAction.java index d4b9bd80b..f627ffb1a 100644 --- a/java/google/registry/ui/server/registrar/ConsoleUiAction.java +++ b/java/google/registry/ui/server/registrar/ConsoleUiAction.java @@ -58,6 +58,7 @@ public final class ConsoleUiAction implements Runnable { @Inject Response response; @Inject SessionUtils sessionUtils; @Inject UserService userService; + @Inject XsrfTokenManager xsrfTokenManager; @Inject @Config("logoFilename") String logoFilename; @Inject @Config("productName") String productName; @Inject @Config("integrationEmail") String integrationEmail; @@ -104,7 +105,7 @@ public final class ConsoleUiAction implements Runnable { return; } Registrar registrar = Registrar.loadByClientId(sessionUtils.getRegistrarClientId(req)); - data.put("xsrfToken", XsrfTokenManager.generateToken(EppConsoleAction.XSRF_SCOPE)); + data.put("xsrfToken", xsrfTokenManager.generateToken(EppConsoleAction.XSRF_SCOPE)); data.put("clientId", registrar.getClientId()); data.put("showPaymentLink", registrar.getBillingMethod() == Registrar.BillingMethod.BRAINTREE); diff --git a/javatests/google/registry/request/RequestHandlerTest.java b/javatests/google/registry/request/RequestHandlerTest.java index 70edf81b8..da2125588 100644 --- a/javatests/google/registry/request/RequestHandlerTest.java +++ b/javatests/google/registry/request/RequestHandlerTest.java @@ -17,7 +17,6 @@ package google.registry.request; import static com.google.common.truth.Truth.assertThat; import static google.registry.request.Action.Method.GET; import static google.registry.request.Action.Method.POST; -import static google.registry.security.XsrfTokenManager.generateToken; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; @@ -25,7 +24,6 @@ import static org.mockito.Mockito.when; import com.google.appengine.api.oauth.OAuthServiceFactory; import com.google.appengine.api.users.User; -import com.google.appengine.api.users.UserService; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.testing.NullPointerTester; @@ -38,7 +36,10 @@ import google.registry.request.auth.AuthenticationMechanism; import google.registry.request.auth.LegacyAuthenticationMechanism; import google.registry.request.auth.OAuthAuthenticationMechanism; import google.registry.request.auth.RequestAuthenticator; +import google.registry.security.XsrfTokenManager; import google.registry.testing.AppEngineRule; +import google.registry.testing.FakeClock; +import google.registry.testing.FakeUserService; import google.registry.testing.Providers; import google.registry.testing.UserInfo; import java.io.PrintWriter; @@ -207,9 +208,6 @@ public final class RequestHandlerTest { @Mock private HttpServletResponse rsp; - @Mock - private UserService userService; - @Mock private BumblebeeTask bumblebeeTask; @@ -228,9 +226,13 @@ public final class RequestHandlerTest { private AuthResult providedAuthResult = null; private final User testUser = new User("test@example.com", "test@example.com"); private RequestAuthenticator requestAuthenticator; + private XsrfTokenManager xsrfTokenManager; + private FakeUserService userService; @Before public void before() throws Exception { + userService = new FakeUserService(); + xsrfTokenManager = new XsrfTokenManager(new FakeClock(), userService); requestAuthenticator = new RequestAuthenticator( new AppEngineInternalAuthenticationMechanism(), ImmutableList.of( @@ -252,7 +254,8 @@ public final class RequestHandlerTest { } }), userService, - requestAuthenticator); + requestAuthenticator, + xsrfTokenManager); when(rsp.getWriter()).thenReturn(new PrintWriter(httpOutput)); } @@ -361,6 +364,7 @@ public final class RequestHandlerTest { public void testNullness() { NullPointerTester tester = new NullPointerTester(); tester.setDefault(RequestAuthenticator.class, requestAuthenticator); + tester.setDefault(XsrfTokenManager.class, xsrfTokenManager); tester.testAllPublicStaticMethods(RequestHandler.class); tester.testAllPublicInstanceMethods(handler); } @@ -375,8 +379,9 @@ public final class RequestHandlerTest { @Test public void testXsrfProtection_validTokenProvided_runsAction() throws Exception { + userService.setUser(testUser, false); when(req.getMethod()).thenReturn("POST"); - when(req.getHeader("X-CSRF-Token")).thenReturn(generateToken("vampire")); + when(req.getHeader("X-CSRF-Token")).thenReturn(xsrfTokenManager.generateToken("vampire")); when(req.getRequestURI()).thenReturn("/safe-sloth"); handler.handleRequest(req, rsp); verify(safeSlothTask).run(); @@ -384,8 +389,9 @@ public final class RequestHandlerTest { @Test public void testXsrfProtection_tokenWithInvalidScopeProvided_returns403() throws Exception { + userService.setUser(testUser, false); when(req.getMethod()).thenReturn("POST"); - when(req.getHeader("X-CSRF-Token")).thenReturn(generateToken("blood")); + when(req.getHeader("X-CSRF-Token")).thenReturn(xsrfTokenManager.generateToken("blood")); when(req.getRequestURI()).thenReturn("/safe-sloth"); handler.handleRequest(req, rsp); verify(rsp).sendError(403, "Invalid X-CSRF-Token"); @@ -401,19 +407,16 @@ public final class RequestHandlerTest { @Test public void testMustBeLoggedIn_notLoggedIn_redirectsToLoginPage() throws Exception { - when(userService.isUserLoggedIn()).thenReturn(false); - when(userService.createLoginURL("/users-only")).thenReturn("/login"); when(req.getMethod()).thenReturn("GET"); when(req.getRequestURI()).thenReturn("/users-only"); handler.handleRequest(req, rsp); verify(rsp).setStatus(302); - verify(rsp).setHeader("Location", "/login"); + verify(rsp).setHeader("Location", "/login?dest=/users-only"); } @Test public void testMustBeLoggedIn_loggedIn_runsAction() throws Exception { - when(userService.isUserLoggedIn()).thenReturn(true); - when(userService.getCurrentUser()).thenReturn(testUser); + userService.setUser(testUser, false); when(req.getMethod()).thenReturn("GET"); when(req.getRequestURI()).thenReturn("/users-only"); handler.handleRequest(req, rsp); @@ -441,9 +444,7 @@ public final class RequestHandlerTest { @Test public void testAuthNeeded_notAuthorized() throws Exception { - when(userService.isUserLoggedIn()).thenReturn(true); - when(userService.getCurrentUser()).thenReturn(testUser); - when(userService.isUserAdmin()).thenReturn(false); + userService.setUser(testUser, false); when(req.getMethod()).thenReturn("GET"); when(req.getRequestURI()).thenReturn("/auth/adminUserAnyMethod"); handler.handleRequest(req, rsp); @@ -453,9 +454,7 @@ public final class RequestHandlerTest { @Test public void testAuthNeeded_success() throws Exception { - when(userService.isUserLoggedIn()).thenReturn(true); - when(userService.getCurrentUser()).thenReturn(testUser); - when(userService.isUserAdmin()).thenReturn(true); + userService.setUser(testUser, true); when(req.getMethod()).thenReturn("GET"); when(req.getRequestURI()).thenReturn("/auth/adminUserAnyMethod"); handler.handleRequest(req, rsp); diff --git a/javatests/google/registry/security/XsrfTokenManagerTest.java b/javatests/google/registry/security/XsrfTokenManagerTest.java index a5dce9e51..51a8a13c0 100644 --- a/javatests/google/registry/security/XsrfTokenManagerTest.java +++ b/javatests/google/registry/security/XsrfTokenManagerTest.java @@ -15,16 +15,14 @@ package google.registry.security; import static com.google.common.truth.Truth.assertThat; -import static google.registry.security.XsrfTokenManager.generateToken; -import static google.registry.security.XsrfTokenManager.validateToken; import static google.registry.util.DateTimeUtils.START_OF_TIME; import com.google.common.base.Splitter; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; +import google.registry.testing.FakeUserService; import google.registry.testing.InjectRule; import google.registry.testing.UserInfo; -import org.joda.time.Duration; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -35,9 +33,10 @@ import org.mockito.runners.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) public class XsrfTokenManagerTest { - private static final Duration ONE_DAY = Duration.standardDays(1); - FakeClock clock = new FakeClock(START_OF_TIME); + FakeUserService userService = new FakeUserService(); + + XsrfTokenManager xsrfTokenManager = new XsrfTokenManager(clock, userService); @Rule public final AppEngineRule appEngine = AppEngineRule.builder() @@ -50,46 +49,51 @@ public class XsrfTokenManagerTest { @Before public void init() { - inject.setStaticField(XsrfTokenManager.class, "clock", clock); + // inject.setStaticField(XsrfTokenManager.class, "clock", clock); } @Test public void testSuccess() { - assertThat(validateToken(generateToken("console"), "console", ONE_DAY)).isTrue(); + assertThat(xsrfTokenManager.validateToken(xsrfTokenManager.generateToken("console"), "console")) + .isTrue(); } @Test public void testNoTimestamp() { - assertThat(validateToken("foo", "console", ONE_DAY)).isFalse(); + assertThat(xsrfTokenManager.validateToken("foo", "console")).isFalse(); } @Test public void testBadNumberTimestamp() { - assertThat(validateToken("foo:bar", "console", ONE_DAY)).isFalse(); + assertThat(xsrfTokenManager.validateToken("foo:bar", "console")).isFalse(); } @Test public void testExpired() { - String token = generateToken("console"); + String token = xsrfTokenManager.generateToken("console"); clock.setTo(START_OF_TIME.plusDays(2)); - assertThat(validateToken(token, "console", ONE_DAY)).isFalse(); + assertThat(xsrfTokenManager.validateToken(token, "console")).isFalse(); } @Test public void testTimestampTamperedWith() { - String encodedPart = Splitter.on(':').splitToList(generateToken("console")).get(0); + String encodedPart = + Splitter.on(':').splitToList(xsrfTokenManager.generateToken("console")).get(0); long tamperedTimestamp = clock.nowUtc().plusMillis(1).getMillis(); - assertThat(validateToken(encodedPart + ":" + tamperedTimestamp, "console", ONE_DAY)).isFalse(); - } - - @Test - public void testDifferentUser() { - assertThat(validateToken(generateToken("console", "b@example.com"), "console", ONE_DAY)) + assertThat(xsrfTokenManager.validateToken(encodedPart + ":" + tamperedTimestamp, "console")) .isFalse(); } + @Test + public void testDifferentUser() { + assertThat(xsrfTokenManager + .validateToken(xsrfTokenManager.generateToken("console", "b@example.com"), "console")) + .isFalse(); + } + @Test public void testDifferentScope() { - assertThat(validateToken(generateToken("console"), "foobar", ONE_DAY)).isFalse(); + assertThat(xsrfTokenManager.validateToken(xsrfTokenManager.generateToken("console"), "foobar")) + .isFalse(); } } diff --git a/javatests/google/registry/testing/FakeUserService.java b/javatests/google/registry/testing/FakeUserService.java index 043a0c195..068d8f0aa 100644 --- a/javatests/google/registry/testing/FakeUserService.java +++ b/javatests/google/registry/testing/FakeUserService.java @@ -34,12 +34,12 @@ public class FakeUserService implements UserService { @Override public String createLoginURL(String destinationURL) { - throw new UnsupportedOperationException(); + return String.format("/login?dest=%s", destinationURL); } @Override public String createLoginURL(String destinationURL, String authDomain) { - throw new UnsupportedOperationException(); + return createLoginURL(destinationURL); } @Deprecated @@ -51,12 +51,12 @@ public class FakeUserService implements UserService { @Override public String createLogoutURL(String destinationURL) { - throw new UnsupportedOperationException(); + return String.format("/logout?dest=%s", destinationURL); } @Override public String createLogoutURL(String destinationURL, String authDomain) { - throw new UnsupportedOperationException(); + return createLogoutURL(destinationURL); } @Override diff --git a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java index 4a18bc026..b80de02c2 100644 --- a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java +++ b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java @@ -20,7 +20,9 @@ import static org.mockito.Mockito.when; import com.google.appengine.api.users.UserServiceFactory; import com.google.common.net.MediaType; +import google.registry.security.XsrfTokenManager; import google.registry.testing.AppEngineRule; +import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.UserInfo; import javax.servlet.http.HttpServletRequest; @@ -60,6 +62,7 @@ public class ConsoleUiActionTest { action.response = response; action.sessionUtils = sessionUtils; action.userService = UserServiceFactory.getUserService(); + action.xsrfTokenManager = new XsrfTokenManager(new FakeClock(), action.userService); when(sessionUtils.checkRegistrarConsoleLogin(any(HttpServletRequest.class))).thenReturn(true); when(sessionUtils.getRegistrarClientId(any(HttpServletRequest.class))) .thenReturn("TheRegistrar"); diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index b9bfc17c1..ff9596634 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -18,7 +18,6 @@ import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailAddres import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailDisplayName; import static google.registry.security.JsonHttpTestUtils.createJsonPayload; import static google.registry.security.JsonHttpTestUtils.createJsonResponseSupplier; -import static google.registry.security.XsrfTokenManager.generateToken; import static google.registry.util.ResourceUtils.readResourceUtf8; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.when; @@ -33,8 +32,10 @@ import google.registry.model.registrar.Registrar; import google.registry.request.JsonActionRunner; import google.registry.request.JsonResponse; import google.registry.request.ResponseImpl; +import google.registry.security.XsrfTokenManager; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; +import google.registry.testing.FakeUserService; import google.registry.testing.InjectRule; import google.registry.util.SendEmailService; import java.io.PrintWriter; @@ -91,6 +92,7 @@ public class RegistrarSettingsActionTestCase { final StringWriter writer = new StringWriter(); final Supplier> json = createJsonResponseSupplier(writer); final FakeClock clock = new FakeClock(DateTime.parse("2014-01-01T00:00:00Z")); + final XsrfTokenManager xsrfTokenManager = new XsrfTokenManager(clock, new FakeUserService()); @Before public void setUp() throws Exception { @@ -111,7 +113,7 @@ public class RegistrarSettingsActionTestCase { when(req.getMethod()).thenReturn("POST"); when(rsp.getWriter()).thenReturn(new PrintWriter(writer)); when(req.getContentType()).thenReturn("application/json"); - when(req.getHeader(eq("X-CSRF-Token"))).thenReturn(generateToken("console")); + when(req.getHeader(eq("X-CSRF-Token"))).thenReturn(xsrfTokenManager.generateToken("console")); when(req.getReader()).thenReturn(createJsonPayload(ImmutableMap.of("op", "read"))); when(sessionUtils.isLoggedIn()).thenReturn(true); when(sessionUtils.checkRegistrarConsoleLogin(req)).thenReturn(true);