diff --git a/java/google/registry/request/Route.java b/java/google/registry/request/Route.java index e52a0cdac..8741a2c32 100644 --- a/java/google/registry/request/Route.java +++ b/java/google/registry/request/Route.java @@ -42,6 +42,7 @@ abstract class Route { } boolean shouldXsrfProtect(Action.Method requestMethod) { - return action().xsrfProtection() && requestMethod != Action.Method.GET; + return action().xsrfProtection() + && (requestMethod != Action.Method.GET) && (requestMethod != Action.Method.HEAD); } } diff --git a/java/google/registry/request/auth/AppEngineInternalAuthenticationMechanism.java b/java/google/registry/request/auth/AppEngineInternalAuthenticationMechanism.java index 0750cb215..af79a48b7 100644 --- a/java/google/registry/request/auth/AppEngineInternalAuthenticationMechanism.java +++ b/java/google/registry/request/auth/AppEngineInternalAuthenticationMechanism.java @@ -17,6 +17,7 @@ package google.registry.request.auth; import static google.registry.request.auth.AuthLevel.APP; import static google.registry.request.auth.AuthLevel.NONE; +import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; /** @@ -56,6 +57,9 @@ public class AppEngineInternalAuthenticationMechanism implements AuthenticationM // As defined in the App Engine request header documentation. private static final String QUEUE_NAME_HEADER = "X-AppEngine-QueueName"; + @Inject + public AppEngineInternalAuthenticationMechanism() {} + @Override public AuthResult authenticate(HttpServletRequest request) { if (request.getHeader(QUEUE_NAME_HEADER) == null) { diff --git a/java/google/registry/request/auth/AuthModule.java b/java/google/registry/request/auth/AuthModule.java index ee8eef21e..53f5cb67b 100644 --- a/java/google/registry/request/auth/AuthModule.java +++ b/java/google/registry/request/auth/AuthModule.java @@ -16,12 +16,9 @@ package google.registry.request.auth; import com.google.appengine.api.oauth.OAuthService; import com.google.appengine.api.oauth.OAuthServiceFactory; -import com.google.appengine.api.users.UserService; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import dagger.Module; import dagger.Provides; -import google.registry.config.RegistryConfig.Config; /** * Dagger module for authentication routines. @@ -29,28 +26,12 @@ import google.registry.config.RegistryConfig.Config; @Module public class AuthModule { - /** Provides the internal authentication mechanism. */ - @Provides - AppEngineInternalAuthenticationMechanism provideAppEngineInternalAuthenticationMechanism() { - return new AppEngineInternalAuthenticationMechanism(); - } - /** Provides the custom authentication mechanisms (including OAuth). */ @Provides ImmutableList provideApiAuthenticationMechanisms( - OAuthService oauthService, - @Config("availableOauthScopes") ImmutableSet availableOauthScopes, - @Config("requiredOauthScopes") ImmutableSet requiredOauthScopes, - @Config("allowedOauthClientIds") ImmutableSet allowedOauthClientIds) { + OAuthAuthenticationMechanism oauthAuthenticationMechanism) { return ImmutableList.of( - new OAuthAuthenticationMechanism( - oauthService, availableOauthScopes, requiredOauthScopes, allowedOauthClientIds)); - } - - /** Provides the legacy authentication mechanism. */ - @Provides - LegacyAuthenticationMechanism provideLegacyAuthenticationMechanism(UserService userService) { - return new LegacyAuthenticationMechanism(userService); + oauthAuthenticationMechanism); } /** Provides the OAuthService instance. */ diff --git a/java/google/registry/request/auth/BUILD b/java/google/registry/request/auth/BUILD index fb701cb33..26d201e3e 100644 --- a/java/google/registry/request/auth/BUILD +++ b/java/google/registry/request/auth/BUILD @@ -9,6 +9,7 @@ java_library( srcs = glob(["*.java"]), deps = [ "//java/google/registry/config", + "//java/google/registry/security", "//java/google/registry/util", "@com_google_appengine_api_1_0_sdk", "@com_google_auto_value", diff --git a/java/google/registry/request/auth/LegacyAuthenticationMechanism.java b/java/google/registry/request/auth/LegacyAuthenticationMechanism.java index 5f6259b0f..e93924b6d 100644 --- a/java/google/registry/request/auth/LegacyAuthenticationMechanism.java +++ b/java/google/registry/request/auth/LegacyAuthenticationMechanism.java @@ -14,11 +14,15 @@ package google.registry.request.auth; +import static com.google.common.base.Strings.nullToEmpty; import static google.registry.request.auth.AuthLevel.NONE; import static google.registry.request.auth.AuthLevel.USER; +import static google.registry.security.XsrfTokenManager.X_CSRF_TOKEN; import com.google.appengine.api.users.UserService; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableSet; +import google.registry.security.XsrfTokenManager; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; @@ -27,25 +31,36 @@ import javax.servlet.http.HttpServletRequest; * *

Just use the values returned by UserService. */ -// TODO(mountford) Add XSRF protection here or elsewhere, from RequestHandler public class LegacyAuthenticationMechanism implements AuthenticationMechanism { private final UserService userService; + private final XsrfTokenManager xsrfTokenManager; + + /** HTTP methods which are considered safe, and do not require XSRF protection. */ + private static final ImmutableSet SAFE_METHODS = ImmutableSet.of("GET", "HEAD"); @VisibleForTesting @Inject - public LegacyAuthenticationMechanism(UserService userService) { + public LegacyAuthenticationMechanism(UserService userService, XsrfTokenManager xsrfTokenManager) { this.userService = userService; + this.xsrfTokenManager = xsrfTokenManager; } @Override public AuthResult authenticate(HttpServletRequest request) { if (!userService.isUserLoggedIn()) { return AuthResult.create(NONE); - } else { - return AuthResult.create( - USER, - UserAuthInfo.create(userService.getCurrentUser(), userService.isUserAdmin())); } + + if (!SAFE_METHODS.contains(request.getMethod()) + && !xsrfTokenManager.validateToken( + nullToEmpty(request.getHeader(X_CSRF_TOKEN)), + "console")) { // hard-coded for now; in the long run, this will be removed + return AuthResult.create(NONE); + } + + return AuthResult.create( + USER, + UserAuthInfo.create(userService.getCurrentUser(), userService.isUserAdmin())); } } diff --git a/java/google/registry/security/XsrfTokenManager.java b/java/google/registry/security/XsrfTokenManager.java index 22d141b4c..4d9796c12 100644 --- a/java/google/registry/security/XsrfTokenManager.java +++ b/java/google/registry/security/XsrfTokenManager.java @@ -25,6 +25,7 @@ import com.google.common.hash.Hashing; import google.registry.util.Clock; import google.registry.util.FormattingLogger; import java.util.List; +import javax.annotation.Nullable; import javax.inject.Inject; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -32,6 +33,8 @@ import org.joda.time.Duration; /** Helper class for generating and validate XSRF tokens. */ public final class XsrfTokenManager { + // TODO(b/35388772): remove the scope parameter + /** HTTP header used for transmitting XSRF tokens. */ public static final String X_CSRF_TOKEN = "X-CSRF-Token"; @@ -48,8 +51,16 @@ public final class XsrfTokenManager { this.userService = userService; } - private static String encodeToken(long creationTime, String scope, String userEmail) { - String token = Joiner.on('\t').join(getServerSecret(), userEmail, scope, creationTime); + /** + * Encode a token. + * + *

The token is a Base64-encoded SHA-256 hash of a string containing the secret, email, scope + * and creation time, separated by tabs. If the scope is null, the string is secret, email, + * creation time. In the future, the scope option will be removed. + */ + private static String encodeToken(long creationTime, @Nullable String scope, String userEmail) { + String token = + Joiner.on('\t').skipNulls().join(getServerSecret(), userEmail, scope, creationTime); return base64Url().encode(Hashing.sha256() .newHasher(token.length()) .putString(token, UTF_8) @@ -58,28 +69,69 @@ public final class XsrfTokenManager { } /** - * Generate an xsrf token for a given scope using the logged in user or else no user. + * Generate an xsrf token for a given scope using the email of the logged in user or else no user. * *

If there is no user, the entire xsrf check becomes basically a no-op, but that's ok because * any callback that doesn't have a user shouldn't be able to access any per-user resources * anyways. + * + *

The scope (or lack thereof) is passed to {@link #encodeToken}. Use of a scope in xsrf tokens + * is deprecated; instead, use the no-argument version. */ - public String generateToken(String scope) { - return generateToken(scope, getLoggedInEmailOrEmpty()); + @Deprecated + public String generateTokenWithCurrentUser(@Nullable String scope) { + return generateTokenSub(scope, getLoggedInEmailOrEmpty()); } - /** Generate an xsrf token for a given scope and user. */ - public String generateToken(String scope, String email) { + /** + * Generate an xsrf token for a given scope and user. + * + *

The scope (or lack thereof) is passed to {@link #encodeToken}. Use of a scope in xsrf tokens + * is deprecated; instead, use the no-argument version. + */ + @Deprecated + public String generateToken(@Nullable String scope, String email) { long now = clock.nowUtc().getMillis(); return Joiner.on(':').join(encodeToken(now, scope, email), now); } + /** Generate an xsrf token for a given user. */ + public String generateToken(String email) { + return generateTokenSub(null, email); + } + private String getLoggedInEmailOrEmpty() { return userService.isUserLoggedIn() ? userService.getCurrentUser().getEmail() : ""; } - /** Validate an xsrf token, given the scope it was used for. */ - public boolean validateToken(String token, String scope) { + private String generateTokenSub(@Nullable String scope, String email) { + long now = clock.nowUtc().getMillis(); + return Joiner.on(':').join(encodeToken(now, scope, email), now); + } + + /** + * Validate an xsrf token, given the scope it was used for. + * + *

We plan to remove the scope parameter. As a first step, the method first checks for the + * existence of a token with no scope. If that is not found, it then looks for the existence of a + * token with the specified scope. Our next step will be to have clients pass in a null scope. + * Finally, we will remove scopes from this code altogether. + */ + @Deprecated + public boolean validateToken(String token, @Nullable String scope) { + return validateTokenSub(token, scope); + } + + /** + * Validate an xsrf token. + * + *

This is the non-scoped version to which we will transition in the future. + */ + public boolean validateToken(String token) { + return validateTokenSub(token, null); + } + + private boolean validateTokenSub(String token, @Nullable String scope) { List tokenParts = Splitter.on(':').splitToList(token); if (tokenParts.size() != 2) { logger.warningfmt("Malformed XSRF token: %s", token); @@ -98,11 +150,21 @@ public final class XsrfTokenManager { logger.infofmt("Expired timestamp in XSRF token: %s", token); return false; } - String reconstructedToken = encodeToken(creationTime, scope, getLoggedInEmailOrEmpty()); - if (!reconstructedToken.equals(encodedPart)) { - logger.warningfmt("Reconstructed XSRF mismatch: %s ≠ %s", encodedPart, reconstructedToken); - return false; + // First, check for a scopeless token, because that's the token of the future. + String reconstructedToken = encodeToken(creationTime, null, getLoggedInEmailOrEmpty()); + if (reconstructedToken.equals(encodedPart)) { + return true; } - return true; + + // If we don't find one, look for one with the specified scope. + if (scope != null) { + reconstructedToken = encodeToken(creationTime, scope, getLoggedInEmailOrEmpty()); + if (reconstructedToken.equals(encodedPart)) { + return true; + } + } + + logger.warningfmt("Reconstructed XSRF mismatch: %s ≠ %s", encodedPart, reconstructedToken); + return false; } } diff --git a/java/google/registry/ui/server/registrar/ConsoleUiAction.java b/java/google/registry/ui/server/registrar/ConsoleUiAction.java index f627ffb1a..3f0d3d14b 100644 --- a/java/google/registry/ui/server/registrar/ConsoleUiAction.java +++ b/java/google/registry/ui/server/registrar/ConsoleUiAction.java @@ -105,7 +105,10 @@ 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, userService.getCurrentUser().getEmail())); 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 8dac65512..e04e13287 100644 --- a/javatests/google/registry/request/RequestHandlerTest.java +++ b/javatests/google/registry/request/RequestHandlerTest.java @@ -241,7 +241,7 @@ public final class RequestHandlerTest { ImmutableSet.of("https://www.googleapis.com/auth/userinfo.email"), ImmutableSet.of("https://www.googleapis.com/auth/userinfo.email"), ImmutableSet.of("proxy-client-id", "regtool-client-id"))), - new LegacyAuthenticationMechanism(userService)); + new LegacyAuthenticationMechanism(userService, xsrfTokenManager)); // Initialize here, not inline, so that we pick up the mocked UserService. handler = RequestHandler.createForTest( @@ -382,7 +382,8 @@ public final class RequestHandlerTest { public void testXsrfProtection_validTokenProvided_runsAction() throws Exception { userService.setUser(testUser, false); when(req.getMethod()).thenReturn("POST"); - when(req.getHeader("X-CSRF-Token")).thenReturn(xsrfTokenManager.generateToken("vampire")); + when(req.getHeader("X-CSRF-Token")) + .thenReturn(xsrfTokenManager.generateTokenWithCurrentUser("vampire")); when(req.getRequestURI()).thenReturn("/safe-sloth"); handler.handleRequest(req, rsp); verify(safeSlothTask).run(); @@ -392,7 +393,8 @@ public final class RequestHandlerTest { public void testXsrfProtection_tokenWithInvalidScopeProvided_returns403() throws Exception { userService.setUser(testUser, false); when(req.getMethod()).thenReturn("POST"); - when(req.getHeader("X-CSRF-Token")).thenReturn(xsrfTokenManager.generateToken("blood")); + when(req.getHeader("X-CSRF-Token")) + .thenReturn(xsrfTokenManager.generateTokenWithCurrentUser("blood")); when(req.getRequestURI()).thenReturn("/safe-sloth"); handler.handleRequest(req, rsp); verify(rsp).sendError(403, "Invalid X-CSRF-Token"); diff --git a/javatests/google/registry/request/auth/BUILD b/javatests/google/registry/request/auth/BUILD index 48c58f749..d9ab332f7 100644 --- a/javatests/google/registry/request/auth/BUILD +++ b/javatests/google/registry/request/auth/BUILD @@ -14,6 +14,7 @@ java_library( deps = [ "//java/google/registry/request", "//java/google/registry/request/auth", + "//java/google/registry/security", "//javatests/google/registry/testing", "//third_party/java/objectify:objectify-v4_1", "@com_google_appengine_api_1_0_sdk//:testonly", diff --git a/javatests/google/registry/request/auth/RequestAuthenticatorTest.java b/javatests/google/registry/request/auth/RequestAuthenticatorTest.java index bd716a897..735df8096 100644 --- a/javatests/google/registry/request/auth/RequestAuthenticatorTest.java +++ b/javatests/google/registry/request/auth/RequestAuthenticatorTest.java @@ -26,11 +26,14 @@ import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import google.registry.request.Action; +import google.registry.security.XsrfTokenManager; import google.registry.testing.AppEngineRule; import google.registry.testing.ExceptionRule; +import google.registry.testing.FakeClock; import google.registry.testing.FakeOAuthService; import google.registry.testing.FakeUserService; import javax.servlet.http.HttpServletRequest; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -125,6 +128,8 @@ public class RequestAuthenticatorTest { private final User testUser = new User("test@google.com", "test@google.com"); private final FakeUserService fakeUserService = new FakeUserService(); + private final XsrfTokenManager xsrfTokenManager = + new XsrfTokenManager(new FakeClock(), fakeUserService); private final FakeOAuthService fakeOAuthService = new FakeOAuthService( false /* isOAuthEnabled */, testUser, @@ -132,6 +137,11 @@ public class RequestAuthenticatorTest { "test-client-id", ImmutableList.of("test-scope1", "test-scope2", "nontest-scope")); + @Before + public void before() throws Exception { + when(req.getMethod()).thenReturn("POST"); + } + private RequestAuthenticator createRequestAuthenticator(UserService userService) { return new RequestAuthenticator( new AppEngineInternalAuthenticationMechanism(), @@ -141,7 +151,7 @@ public class RequestAuthenticatorTest { ImmutableSet.of("test-scope1", "test-scope2", "test-scope3"), ImmutableSet.of("test-scope1", "test-scope2"), ImmutableSet.of("test-client-id", "other-test-client-id"))), - new LegacyAuthenticationMechanism(userService)); + new LegacyAuthenticationMechanism(userService, xsrfTokenManager)); } private Optional runTest(UserService userService, Class clazz) { @@ -152,19 +162,20 @@ public class RequestAuthenticatorTest { @Test public void testNoAuthNeeded_noneFound() throws Exception { Optional authResult = runTest(mockUserService, AuthNone.class); + verifyZeroInteractions(mockUserService); assertThat(authResult).isPresent(); - assertThat(authResult.get()).isNotNull(); assertThat(authResult.get().authLevel()).isEqualTo(AuthLevel.NONE); } @Test public void testNoAuthNeeded_internalFound() throws Exception { when(req.getHeader("X-AppEngine-QueueName")).thenReturn("__cron"); + Optional authResult = runTest(mockUserService, AuthNone.class); + verifyZeroInteractions(mockUserService); assertThat(authResult).isPresent(); - assertThat(authResult.get()).isNotNull(); assertThat(authResult.get().authLevel()).isEqualTo(AuthLevel.APP); assertThat(authResult.get().userAuthInfo()).isAbsent(); } @@ -172,6 +183,7 @@ public class RequestAuthenticatorTest { @Test public void testInternalAuth_notInvokedInternally() throws Exception { Optional authResult = runTest(mockUserService, AuthInternalOnly.class); + verifyZeroInteractions(mockUserService); assertThat(authResult).isAbsent(); } @@ -179,10 +191,11 @@ public class RequestAuthenticatorTest { @Test public void testInternalAuth_success() throws Exception { when(req.getHeader("X-AppEngine-QueueName")).thenReturn("__cron"); + Optional authResult = runTest(mockUserService, AuthInternalOnly.class); + verifyZeroInteractions(mockUserService); assertThat(authResult).isPresent(); - assertThat(authResult.get()).isNotNull(); assertThat(authResult.get().authLevel()).isEqualTo(AuthLevel.APP); assertThat(authResult.get().userAuthInfo()).isAbsent(); } @@ -190,15 +203,28 @@ public class RequestAuthenticatorTest { @Test public void testAnyUserAnyMethod_notLoggedIn() throws Exception { Optional authResult = runTest(fakeUserService, AuthAnyUserAnyMethod.class); + + assertThat(authResult).isAbsent(); + } + + @Test + public void testAnyUserAnyMethod_xsrfFailure() throws Exception { + fakeUserService.setUser(testUser, false); + + Optional authResult = runTest(fakeUserService, AuthAnyUserAnyMethod.class); + assertThat(authResult).isAbsent(); } @Test public void testAnyUserAnyMethod_success() throws Exception { fakeUserService.setUser(testUser, false /* isAdmin */); + when(req.getHeader(XsrfTokenManager.X_CSRF_TOKEN)) + .thenReturn(xsrfTokenManager.generateTokenWithCurrentUser("console")); + Optional authResult = runTest(fakeUserService, AuthAnyUserAnyMethod.class); + assertThat(authResult).isPresent(); - assertThat(authResult.get()).isNotNull(); assertThat(authResult.get().authLevel()).isEqualTo(AuthLevel.USER); assertThat(authResult.get().userAuthInfo()).isPresent(); assertThat(authResult.get().userAuthInfo().get().user()).isEqualTo(testUser); @@ -206,23 +232,53 @@ public class RequestAuthenticatorTest { assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo()).isAbsent(); } + @Test + public void testAnyUserAnyMethod_xsrfNotRequiredForGet() throws Exception { + fakeUserService.setUser(testUser, false); + when(req.getMethod()).thenReturn("GET"); + + Optional authResult = runTest(fakeUserService, AuthAnyUserAnyMethod.class); + + assertThat(authResult).isPresent(); + assertThat(authResult.get().authLevel()).isEqualTo(AuthLevel.USER); + assertThat(authResult.get().userAuthInfo()).isPresent(); + assertThat(authResult.get().userAuthInfo().get().user()).isEqualTo(testUser); + assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo()).isAbsent(); + } + @Test public void testAdminUserAnyMethod_notLoggedIn() throws Exception { Optional authResult = runTest(fakeUserService, AuthAdminUserAnyMethod.class); + assertThat(authResult).isAbsent(); } @Test public void testAdminUserAnyMethod_notAdminUser() throws Exception { fakeUserService.setUser(testUser, false /* isAdmin */); + Optional authResult = runTest(fakeUserService, AuthAdminUserAnyMethod.class); + + assertThat(authResult).isAbsent(); + } + + @Test + public void testAdminUserAnyMethod_xsrfFailure() throws Exception { + fakeUserService.setUser(testUser, true); + + Optional authResult = runTest(fakeUserService, AuthAdminUserAnyMethod.class); + assertThat(authResult).isAbsent(); } @Test public void testAdminUserAnyMethod_success() throws Exception { fakeUserService.setUser(testUser, true /* isAdmin */); + when(req.getHeader(XsrfTokenManager.X_CSRF_TOKEN)) + .thenReturn(xsrfTokenManager.generateTokenWithCurrentUser("console")); + Optional authResult = runTest(fakeUserService, AuthAdminUserAnyMethod.class); + assertThat(authResult).isPresent(); assertThat(authResult.get().authLevel()).isEqualTo(AuthLevel.USER); assertThat(authResult.get().userAuthInfo()).isPresent(); @@ -236,7 +292,9 @@ public class RequestAuthenticatorTest { fakeOAuthService.setUser(testUser); fakeOAuthService.setOAuthEnabled(true); when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN"); + Optional authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class); + assertThat(authResult).isPresent(); assertThat(authResult.get().authLevel()).isEqualTo(AuthLevel.USER); assertThat(authResult.get().userAuthInfo()).isPresent(); @@ -257,7 +315,9 @@ public class RequestAuthenticatorTest { fakeOAuthService.setUserAdmin(true); fakeOAuthService.setOAuthEnabled(true); when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN"); + Optional authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class); + assertThat(authResult).isPresent(); assertThat(authResult.get().authLevel()).isEqualTo(AuthLevel.USER); assertThat(authResult.get().userAuthInfo()).isPresent(); @@ -276,7 +336,9 @@ public class RequestAuthenticatorTest { public void testOAuthMissingAuthenticationToken_failure() throws Exception { fakeOAuthService.setUser(testUser); fakeOAuthService.setOAuthEnabled(true); + Optional authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class); + assertThat(authResult).isAbsent(); } @@ -286,7 +348,9 @@ public class RequestAuthenticatorTest { fakeOAuthService.setOAuthEnabled(true); fakeOAuthService.setClientId("wrong-client-id"); when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN"); + Optional authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class); + assertThat(authResult).isAbsent(); } @@ -296,7 +360,9 @@ public class RequestAuthenticatorTest { fakeOAuthService.setOAuthEnabled(true); fakeOAuthService.setAuthorizedScopes(); when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN"); + Optional authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class); + assertThat(authResult).isAbsent(); } @@ -306,7 +372,9 @@ public class RequestAuthenticatorTest { fakeOAuthService.setOAuthEnabled(true); fakeOAuthService.setAuthorizedScopes("test-scope1", "test-scope3"); when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN"); + Optional authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class); + assertThat(authResult).isAbsent(); } @@ -316,7 +384,9 @@ public class RequestAuthenticatorTest { fakeOAuthService.setOAuthEnabled(true); fakeOAuthService.setAuthorizedScopes("test-scope1", "test-scope2", "test-scope3"); when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN"); + Optional authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class); + assertThat(authResult).isPresent(); assertThat(authResult.get().authLevel()).isEqualTo(AuthLevel.USER); assertThat(authResult.get().userAuthInfo()).isPresent(); @@ -334,13 +404,16 @@ public class RequestAuthenticatorTest { @Test public void testAnyUserNoLegacy_failureWithLegacyUser() throws Exception { fakeUserService.setUser(testUser, false /* isAdmin */); + Optional authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class); + assertThat(authResult).isAbsent(); } @Test public void testNoMethods_failure() throws Exception { thrown.expect(IllegalArgumentException.class, "Must specify at least one auth method"); + runTest(fakeUserService, AuthNoMethods.class); } @@ -348,6 +421,7 @@ public class RequestAuthenticatorTest { public void testMissingInternal_failure() throws Exception { thrown.expect(IllegalArgumentException.class, "Auth method INTERNAL must always be specified, and as the first auth method"); + runTest(fakeUserService, AuthMissingInternal.class); } @@ -355,6 +429,7 @@ public class RequestAuthenticatorTest { public void testWrongMethodOrdering_failure() throws Exception { thrown.expect(IllegalArgumentException.class, "Auth methods must be unique and strictly in order - INTERNAL, API, LEGACY"); + runTest(fakeUserService, AuthWrongMethodOrdering.class); } @@ -362,6 +437,7 @@ public class RequestAuthenticatorTest { public void testDuplicateMethods_failure() throws Exception { thrown.expect(IllegalArgumentException.class, "Auth methods must be unique and strictly in order - INTERNAL, API, LEGACY"); + runTest(fakeUserService, AuthDuplicateMethods.class); } @@ -369,6 +445,7 @@ public class RequestAuthenticatorTest { public void testInternalWithUser_failure() throws Exception { thrown.expect(IllegalArgumentException.class, "Actions with only INTERNAL auth may not require USER auth level"); + runTest(fakeUserService, AuthInternalWithUser.class); } @@ -376,6 +453,7 @@ public class RequestAuthenticatorTest { public void testWronglyIgnoringUser_failure() throws Exception { thrown.expect(IllegalArgumentException.class, "Actions with auth methods beyond INTERNAL must not specify the IGNORED user policy"); + runTest(fakeUserService, AuthWronglyIgnoringUser.class); } } diff --git a/javatests/google/registry/security/XsrfTokenManagerTest.java b/javatests/google/registry/security/XsrfTokenManagerTest.java index 51a8a13c0..efbbd02bd 100644 --- a/javatests/google/registry/security/XsrfTokenManagerTest.java +++ b/javatests/google/registry/security/XsrfTokenManagerTest.java @@ -54,7 +54,9 @@ public class XsrfTokenManagerTest { @Test public void testSuccess() { - assertThat(xsrfTokenManager.validateToken(xsrfTokenManager.generateToken("console"), "console")) + assertThat( + xsrfTokenManager.validateToken( + xsrfTokenManager.generateTokenWithCurrentUser("console"), "console")) .isTrue(); } @@ -70,7 +72,7 @@ public class XsrfTokenManagerTest { @Test public void testExpired() { - String token = xsrfTokenManager.generateToken("console"); + String token = xsrfTokenManager.generateTokenWithCurrentUser("console"); clock.setTo(START_OF_TIME.plusDays(2)); assertThat(xsrfTokenManager.validateToken(token, "console")).isFalse(); } @@ -78,7 +80,9 @@ public class XsrfTokenManagerTest { @Test public void testTimestampTamperedWith() { String encodedPart = - Splitter.on(':').splitToList(xsrfTokenManager.generateToken("console")).get(0); + Splitter.on(':') + .splitToList(xsrfTokenManager.generateTokenWithCurrentUser("console")) + .get(0); long tamperedTimestamp = clock.nowUtc().plusMillis(1).getMillis(); assertThat(xsrfTokenManager.validateToken(encodedPart + ":" + tamperedTimestamp, "console")) .isFalse(); @@ -93,7 +97,26 @@ public class XsrfTokenManagerTest { @Test public void testDifferentScope() { - assertThat(xsrfTokenManager.validateToken(xsrfTokenManager.generateToken("console"), "foobar")) + assertThat( + xsrfTokenManager.validateToken( + xsrfTokenManager.generateTokenWithCurrentUser("console"), "foobar")) .isFalse(); } + + @Test + public void testNullScope() { + assertThat( + xsrfTokenManager.validateToken( + xsrfTokenManager.generateTokenWithCurrentUser(null), null)) + .isTrue(); + } + + // This test checks that the server side will pass when we switch the client to use a null scope. + @Test + public void testNullScopePassesWhenTestedWithNonNullScope() { + assertThat( + xsrfTokenManager.validateToken( + xsrfTokenManager.generateTokenWithCurrentUser(null), "console")) + .isTrue(); + } } diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index ff9596634..1504dd3b5 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -113,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(xsrfTokenManager.generateToken("console")); + when(req.getHeader(eq("X-CSRF-Token"))).thenReturn(xsrfTokenManager.generateTokenWithCurrentUser("console")); when(req.getReader()).thenReturn(createJsonPayload(ImmutableMap.of("op", "read"))); when(sessionUtils.isLoggedIn()).thenReturn(true); when(sessionUtils.checkRegistrarConsoleLogin(req)).thenReturn(true);