Refactor a few new XsrfTokenManager methods

Followup to comments on []

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=148792464

java/google/registry/security/XsrfTokenManager.java

@@ -68,24 +68,13 @@ public final class XsrfTokenManager {
         .asBytes());
   }
 
-  /**
-   * Generate an xsrf token for a given scope using the email of the logged in user or else no user.
-   *
-   * <p>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.
-   *
-   * <p>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 generateTokenWithCurrentUser(@Nullable String scope) {
-    return generateTokenSub(scope, getLoggedInEmailOrEmpty());
-  }
-
   /**
    * Generate an xsrf token for a given scope and user.
    *
+   * <p>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.
+   *
    * <p>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.
    */
@@ -97,18 +86,13 @@ public final class XsrfTokenManager {
 
   /** Generate an xsrf token for a given user. */
   public String generateToken(String email) {
-    return generateTokenSub(null, email);
+    return generateToken(null, email);
   }
 
   private String getLoggedInEmailOrEmpty() {
     return userService.isUserLoggedIn() ? userService.getCurrentUser().getEmail() : "";
   }
 
-  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.
    *

javatests/google/registry/request/RequestHandlerTest.java

@@ -383,7 +383,7 @@ public final class RequestHandlerTest {
     userService.setUser(testUser,  false);
     when(req.getMethod()).thenReturn("POST");
     when(req.getHeader("X-CSRF-Token"))
-        .thenReturn(xsrfTokenManager.generateTokenWithCurrentUser("vampire"));
+        .thenReturn(xsrfTokenManager.generateToken("vampire", testUser.getEmail()));
     when(req.getRequestURI()).thenReturn("/safe-sloth");
     handler.handleRequest(req, rsp);
     verify(safeSlothTask).run();
@@ -394,7 +394,7 @@ public final class RequestHandlerTest {
     userService.setUser(testUser,  false);
     when(req.getMethod()).thenReturn("POST");
     when(req.getHeader("X-CSRF-Token"))
-        .thenReturn(xsrfTokenManager.generateTokenWithCurrentUser("blood"));
+        .thenReturn(xsrfTokenManager.generateToken("blood", testUser.getEmail()));
     when(req.getRequestURI()).thenReturn("/safe-sloth");
     handler.handleRequest(req, rsp);
     verify(rsp).sendError(403, "Invalid X-CSRF-Token");

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.generateTokenWithCurrentUser("console"));
+        .thenReturn(xsrfTokenManager.generateToken("console", testUser.getEmail()));
 
     Optional<AuthResult> 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.generateTokenWithCurrentUser("console"));
+        .thenReturn(xsrfTokenManager.generateToken("console", testUser.getEmail()));
 
     Optional<AuthResult> authResult = runTest(fakeUserService, AuthAdminUserAnyMethod.class);
 

javatests/google/registry/security/XsrfTokenManagerTest.java

@@ -17,12 +17,12 @@ package google.registry.security;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 
+import com.google.appengine.api.users.User;
 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.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -33,31 +33,30 @@ import org.mockito.runners.MockitoJUnitRunner;
 @RunWith(MockitoJUnitRunner.class)
 public class XsrfTokenManagerTest {
 
-  FakeClock clock = new FakeClock(START_OF_TIME);
-  FakeUserService userService = new FakeUserService();
-
-  XsrfTokenManager xsrfTokenManager = new XsrfTokenManager(clock, userService);
-
   @Rule
   public final AppEngineRule appEngine = AppEngineRule.builder()
       .withDatastore()
-      .withUserService(UserInfo.createAdmin("a@example.com", "user1"))
       .build();
 
   @Rule
   public InjectRule inject = new InjectRule();
 
+  private final User testUser = new User("test@example.com", "test@example.com");
+  private final FakeClock clock = new FakeClock(START_OF_TIME);
+  private final FakeUserService userService = new FakeUserService();
+  private final XsrfTokenManager xsrfTokenManager = new XsrfTokenManager(clock, userService);
+
+  String realToken;
+
   @Before
   public void init() {
-    // inject.setStaticField(XsrfTokenManager.class, "clock", clock);
+    userService.setUser(testUser, false);
+    realToken = xsrfTokenManager.generateToken("console", testUser.getEmail());
   }
 
   @Test
   public void testSuccess() {
-    assertThat(
+    assertThat(xsrfTokenManager.validateToken(realToken, "console")).isTrue();
-            xsrfTokenManager.validateToken(
-                xsrfTokenManager.generateTokenWithCurrentUser("console"), "console"))
-        .isTrue();
   }
 
   @Test
@@ -72,17 +71,13 @@ public class XsrfTokenManagerTest {
 
   @Test
   public void testExpired() {
-    String token = xsrfTokenManager.generateTokenWithCurrentUser("console");
     clock.setTo(START_OF_TIME.plusDays(2));
-    assertThat(xsrfTokenManager.validateToken(token, "console")).isFalse();
+    assertThat(xsrfTokenManager.validateToken(realToken, "console")).isFalse();
   }
 
   @Test
   public void testTimestampTamperedWith() {
-    String encodedPart =
+    String encodedPart = Splitter.on(':').splitToList(realToken).get(0);
-        Splitter.on(':')
-            .splitToList(xsrfTokenManager.generateTokenWithCurrentUser("console"))
-            .get(0);
     long tamperedTimestamp = clock.nowUtc().plusMillis(1).getMillis();
     assertThat(xsrfTokenManager.validateToken(encodedPart + ":" + tamperedTimestamp, "console"))
         .isFalse();
@@ -91,32 +86,25 @@ public class XsrfTokenManagerTest {
   @Test
   public void testDifferentUser() {
     assertThat(xsrfTokenManager
-        .validateToken(xsrfTokenManager.generateToken("console", "b@example.com"), "console"))
+        .validateToken(xsrfTokenManager.generateToken("console", "eve@example.com"), "console"))
             .isFalse();
   }
 
   @Test
   public void testDifferentScope() {
-    assertThat(
+    assertThat(xsrfTokenManager.validateToken(realToken, "foobar")).isFalse();
-            xsrfTokenManager.validateToken(
-                xsrfTokenManager.generateTokenWithCurrentUser("console"), "foobar"))
-        .isFalse();
   }
 
   @Test
   public void testNullScope() {
-    assertThat(
+    String tokenWithNullScope = xsrfTokenManager.generateToken(null, testUser.getEmail());
-            xsrfTokenManager.validateToken(
+    assertThat(xsrfTokenManager.validateToken(tokenWithNullScope, null)).isTrue();
-                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(
+    String tokenWithNullScope = xsrfTokenManager.generateToken(null, testUser.getEmail());
-            xsrfTokenManager.validateToken(
+    assertThat(xsrfTokenManager.validateToken(tokenWithNullScope, "console")).isTrue();
-                xsrfTokenManager.generateTokenWithCurrentUser(null), "console"))
-        .isTrue();
   }
 }

javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java

@@ -19,7 +19,6 @@ import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailDispla
 import static google.registry.security.JsonHttpTestUtils.createJsonPayload;
 import static google.registry.security.JsonHttpTestUtils.createJsonResponseSupplier;
 import static google.registry.util.ResourceUtils.readResourceUtf8;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.when;
 
 import com.google.appengine.api.modules.ModulesService;
@@ -32,10 +31,8 @@ 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;
@@ -92,7 +89,6 @@ public class RegistrarSettingsActionTestCase {
   final StringWriter writer = new StringWriter();
   final Supplier<Map<String, Object>> 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 {
@@ -113,7 +109,6 @@ 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.generateTokenWithCurrentUser("console"));
     when(req.getReader()).thenReturn(createJsonPayload(ImmutableMap.of("op", "read")));
     when(sessionUtils.isLoggedIn()).thenReturn(true);
     when(sessionUtils.checkRegistrarConsoleLogin(req)).thenReturn(true);