diff --git a/java/google/registry/loadtest/LoadTestAction.java b/java/google/registry/loadtest/LoadTestAction.java index cc4ffd0fa..636a40035 100644 --- a/java/google/registry/loadtest/LoadTestAction.java +++ b/java/google/registry/loadtest/LoadTestAction.java @@ -183,7 +183,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.generateLegacyToken("admin", ""); + xsrfToken = xsrfTokenManager.generateToken(""); } @Override diff --git a/java/google/registry/security/XsrfTokenManager.java b/java/google/registry/security/XsrfTokenManager.java index 778ab7cc4..376aa8dcf 100644 --- a/java/google/registry/security/XsrfTokenManager.java +++ b/java/google/registry/security/XsrfTokenManager.java @@ -99,25 +99,6 @@ public final class XsrfTokenManager { .asBytes()); } - /** - * Generates a legacy-style XSRF token for a given scope and user. - * - *

If there is no user (email is an empty string), 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 is passed to {@link #computeLegacyHash}. Use of a scope in xsrf tokens is - * deprecated; instead, use {@link #generateToken}. - */ - // TODO(b/35388772): remove this in favor of generateToken() - @Deprecated - public String generateLegacyToken(String scope, String email) { - checkArgumentNotNull(scope); - checkArgumentNotNull(email); - long now = clock.nowUtc().getMillis(); - return Joiner.on(':').join(computeLegacyHash(now, scope, email), now); - } - /** * Validates an XSRF token against the current logged-in user. * @@ -157,6 +138,7 @@ public final class XsrfTokenManager { } return true; } else { + // TODO(b/35388772): remove this fallback once we no longer generate legacy tokens. // Fall back to the legacy format, and try the few possible scopes. String hash = tokenParts.get(0); ImmutableSet.Builder reconstructedTokenCandidates = new ImmutableSet.Builder<>(); diff --git a/java/google/registry/tools/AppEngineConnection.java b/java/google/registry/tools/AppEngineConnection.java index 6cfa4b857..144fb3385 100644 --- a/java/google/registry/tools/AppEngineConnection.java +++ b/java/google/registry/tools/AppEngineConnection.java @@ -66,7 +66,7 @@ class AppEngineConnection implements Connection { memoize(new Supplier() { @Override public String get() { - return xsrfTokenManager.generateLegacyToken("admin", getUserId()); + return xsrfTokenManager.generateToken(getUserId()); }}); @Override diff --git a/java/google/registry/ui/server/registrar/ConsoleUiAction.java b/java/google/registry/ui/server/registrar/ConsoleUiAction.java index dcbcab784..420d64c2a 100644 --- a/java/google/registry/ui/server/registrar/ConsoleUiAction.java +++ b/java/google/registry/ui/server/registrar/ConsoleUiAction.java @@ -27,7 +27,6 @@ import com.google.template.soy.data.SoyMapData; import com.google.template.soy.shared.SoyCssRenamingMap; import com.google.template.soy.tofu.SoyTofu; import google.registry.config.RegistryConfig.Config; -import google.registry.flows.EppConsoleAction; import google.registry.model.registrar.Registrar; import google.registry.request.Action; import google.registry.request.Response; @@ -119,8 +118,7 @@ public final class ConsoleUiAction implements Runnable { Registrar registrar = Registrar.loadByClientId(sessionUtils.getRegistrarClientId(req)); data.put( "xsrfToken", - xsrfTokenManager.generateLegacyToken( - EppConsoleAction.XSRF_SCOPE, userService.getCurrentUser().getEmail())); + xsrfTokenManager.generateToken(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 62c3afa32..e07c9463a 100644 --- a/javatests/google/registry/request/RequestHandlerTest.java +++ b/javatests/google/registry/request/RequestHandlerTest.java @@ -403,7 +403,7 @@ public final class RequestHandlerTest { userService.setUser(testUser, false); when(req.getMethod()).thenReturn("POST"); when(req.getHeader("X-CSRF-Token")) - .thenReturn(xsrfTokenManager.generateLegacyToken("admin", testUser.getEmail())); + .thenReturn(xsrfTokenManager.generateToken(testUser.getEmail())); when(req.getRequestURI()).thenReturn("/safe-sloth"); handler.handleRequest(req, rsp); verify(safeSlothTask).run(); @@ -414,7 +414,7 @@ public final class RequestHandlerTest { userService.setUser(testUser, false); when(req.getMethod()).thenReturn("POST"); when(req.getHeader("X-CSRF-Token")) - .thenReturn(xsrfTokenManager.generateLegacyToken("admin", "wrong@example.com")); + .thenReturn(xsrfTokenManager.generateToken("wrong@example.com")); 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/RequestAuthenticatorTest.java b/javatests/google/registry/request/auth/RequestAuthenticatorTest.java index 366e25617..799825b94 100644 --- a/javatests/google/registry/request/auth/RequestAuthenticatorTest.java +++ b/javatests/google/registry/request/auth/RequestAuthenticatorTest.java @@ -220,7 +220,7 @@ public class RequestAuthenticatorTest { public void testAnyUserAnyMethod_success() throws Exception { fakeUserService.setUser(testUser, false /* isAdmin */); when(req.getHeader(XsrfTokenManager.X_CSRF_TOKEN)) - .thenReturn(xsrfTokenManager.generateLegacyToken("console", testUser.getEmail())); + .thenReturn(xsrfTokenManager.generateToken(testUser.getEmail())); Optional authResult = runTest(fakeUserService, AuthAnyUserAnyMethod.class); @@ -275,7 +275,7 @@ public class RequestAuthenticatorTest { public void testAdminUserAnyMethod_success() throws Exception { fakeUserService.setUser(testUser, true /* isAdmin */); when(req.getHeader(XsrfTokenManager.X_CSRF_TOKEN)) - .thenReturn(xsrfTokenManager.generateLegacyToken("console", testUser.getEmail())); + .thenReturn(xsrfTokenManager.generateToken(testUser.getEmail())); Optional authResult = runTest(fakeUserService, AuthAdminUserAnyMethod.class); diff --git a/javatests/google/registry/security/XsrfTokenManagerTest.java b/javatests/google/registry/security/XsrfTokenManagerTest.java index 9ce4862e2..2b6c04738 100644 --- a/javatests/google/registry/security/XsrfTokenManagerTest.java +++ b/javatests/google/registry/security/XsrfTokenManagerTest.java @@ -16,7 +16,6 @@ package google.registry.security; import static com.google.common.truth.Truth.assertThat; import static google.registry.util.DateTimeUtils.START_OF_TIME; -import static org.junit.Assert.fail; import com.google.appengine.api.users.User; import com.google.common.base.Splitter; @@ -49,52 +48,30 @@ public class XsrfTokenManagerTest { private final XsrfTokenManager xsrfTokenManager = new XsrfTokenManager(clock, userService); private String token; - private String legacyToken; @Before public void init() { userService.setUser(testUser, false); token = xsrfTokenManager.generateToken(testUser.getEmail()); - legacyToken = xsrfTokenManager.generateLegacyToken("console", testUser.getEmail()); } @Test - public void testGenerateLegacyToken_invalidScope() { - try { - xsrfTokenManager.generateLegacyToken("foo", testUser.getEmail()); - fail("Expected IllegalArgumentException"); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessageThat().isEqualTo("Invalid scope value: foo"); - } - } - - @Test - public void testValidate_token() { + public void testValidate_validToken() { assertThat(xsrfTokenManager.validateToken(token)).isTrue(); } @Test - public void testValidate_legacyToken() { - assertThat(xsrfTokenManager.validateToken(legacyToken)).isTrue(); - } - - @Test - public void testValidate_token_missingParts() { + public void testValidate_tokenWithMissingParts() { assertThat(xsrfTokenManager.validateToken("foo")).isFalse(); } @Test - public void testValidate_token_badNumberTimestamp() { + public void testValidate_tokenWithBadNumberTimestamp() { assertThat(xsrfTokenManager.validateToken("1:notanumber:base64")).isFalse(); } @Test - public void testValidate_legacyToken_badNumberTimestamp() { - assertThat(xsrfTokenManager.validateToken("base64:notanumber")).isFalse(); - } - - @Test - public void testValidate_token_expiresAfterOneDay() { + public void testValidate_tokenExpiresAfterOneDay() { clock.advanceBy(Duration.standardDays(1)); assertThat(xsrfTokenManager.validateToken(token)).isTrue(); clock.advanceOneMilli(); @@ -102,48 +79,15 @@ public class XsrfTokenManagerTest { } @Test - public void testValidate_legacyToken_expiresAfterOneDay() { - clock.advanceBy(Duration.standardDays(1)); - assertThat(xsrfTokenManager.validateToken(legacyToken)).isTrue(); - clock.advanceOneMilli(); - assertThat(xsrfTokenManager.validateToken(legacyToken)).isFalse(); - } - - @Test - public void testValidate_token_timestampTamperedWith() { + public void testValidate_tokenTimestampTamperedWith() { String encodedPart = Splitter.on(':').splitToList(token).get(2); long fakeTimestamp = clock.nowUtc().plusMillis(1).getMillis(); assertThat(xsrfTokenManager.validateToken("1:" + fakeTimestamp + ":" + encodedPart)).isFalse(); } @Test - public void testValidate_legacyToken_timestampTamperedWith() { - String encodedPart = Splitter.on(':').splitToList(legacyToken).get(0); - long tamperedTimestamp = clock.nowUtc().plusMillis(1).getMillis(); - assertThat(xsrfTokenManager.validateToken(encodedPart + ":" + tamperedTimestamp)).isFalse(); - } - - @Test - public void testValidate_token_differentUser() { + public void testValidate_tokenForDifferentUser() { String otherToken = xsrfTokenManager.generateToken("eve@example.com"); assertThat(xsrfTokenManager.validateToken(otherToken)).isFalse(); } - - @Test - public void testValidate_legacyToken_differentUser() { - String otherToken = xsrfTokenManager.generateLegacyToken("console", "eve@example.com"); - assertThat(xsrfTokenManager.validateToken(otherToken)).isFalse(); - } - - @Test - public void testValidate_legacyToken_adminScope() { - String adminToken = xsrfTokenManager.generateLegacyToken("admin", testUser.getEmail()); - assertThat(xsrfTokenManager.validateToken(adminToken)).isTrue(); - } - - @Test - public void testValidate_legacyToken_consoleScope() { - String consoleToken = xsrfTokenManager.generateLegacyToken("console", testUser.getEmail()); - assertThat(xsrfTokenManager.validateToken(consoleToken)).isTrue(); - } }