Add more tests to new authentication framework

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

java/google/registry/request/auth/RequestAuthenticator.java

@@ -154,7 +154,7 @@ public class RequestAuthenticator {
     checkArgument(
         Ordering.explicit(Auth.AuthMethod.INTERNAL, Auth.AuthMethod.API, Auth.AuthMethod.LEGACY)
             .isStrictlyOrdered(authMethods),
-        "Auth methods must be strictly in order - INTERNAL, API, LEGACY");
+        "Auth methods must be unique and strictly in order - INTERNAL, API, LEGACY");
     checkArgument(
         authMethods.contains(Auth.AuthMethod.INTERNAL),
         "Auth method INTERNAL must always be specified, and as the first auth method");

javatests/google/registry/request/auth/BUILD

@@ -18,6 +18,7 @@ java_library(
         "//third_party/java/objectify:objectify-v4_1",
         "@com_google_appengine_api_1_0_sdk//:testonly",
         "@com_google_appengine_tools_appengine_gcs_client",
+        "@com_google_appengine_tools_sdk",
         "@com_google_code_findbugs_jsr305",
         "@com_google_guava",
         "@com_google_truth",

javatests/google/registry/request/auth/RequestAuthenticatorTest.java

@@ -14,36 +14,37 @@
 
 package google.registry.request.auth;
 
+import static com.google.common.net.HttpHeaders.AUTHORIZATION;
 import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verifyZeroInteractions;
 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.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import google.registry.request.Action;
+import google.registry.testing.AppEngineRule;
+import google.registry.testing.ExceptionRule;
+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;
-import org.mockito.Mock;
+import org.junit.runners.JUnit4;
-import org.mockito.runners.MockitoJUnitRunner;
 
 /** Unit tests for {@link RequestAuthenticator}. */
-@RunWith(MockitoJUnitRunner.class)
+@RunWith(JUnit4.class)
 public class RequestAuthenticatorTest {
 
-  private FakeUserService fakeUserService;
+  @Rule
+  public final AppEngineRule appEngine = AppEngineRule.builder().build();
 
-  @Mock
+  @Rule
-  private UserService mockUserService;
+  public final ExceptionRule thrown = new ExceptionRule();
-
-  @Mock
-  private HttpServletRequest req;
 
   @Action(
       path = "/auth/none",
@@ -69,6 +70,15 @@ public class RequestAuthenticatorTest {
       method = Action.Method.GET)
   public static class AuthAnyUserAnyMethod {}
 
+  @Action(
+      path = "/auth/anyUserNoLegacy",
+      auth = @Auth(
+          methods = {Auth.AuthMethod.INTERNAL, Auth.AuthMethod.API},
+          minimumLevel = AuthLevel.USER,
+          userPolicy = Auth.UserPolicy.PUBLIC),
+      method = Action.Method.GET)
+  public static class AuthAnyUserNoLegacy {}
+
   @Action(
       path = "/auth/adminUserAnyMethod",
       auth = @Auth(
@@ -76,25 +86,61 @@ public class RequestAuthenticatorTest {
           minimumLevel = AuthLevel.USER,
           userPolicy = Auth.UserPolicy.ADMIN),
       method = Action.Method.GET)
-  public static class AuthAdminUserAnyMethod {
+  public static class AuthAdminUserAnyMethod {}
-  }
 
-  private final User testUser = new User("test@example.com", "test@example.com");
+  @Action(
+      path = "/auth/noMethods",
+      auth = @Auth(methods = {}))
+  public static class AuthNoMethods {}
 
-  @Before
+  @Action(
-  public void before() throws Exception {
+      path = "/auth/missingInternal",
-    fakeUserService = new FakeUserService();
+      auth = @Auth(methods = {Auth.AuthMethod.API, Auth.AuthMethod.LEGACY}))
-  }
+  public static class AuthMissingInternal {}
 
-  private static RequestAuthenticator createRequestAuthenticator(UserService userService) {
+  @Action(
+      path = "/auth/wrongMethodOrdering",
+      auth = @Auth(methods = {Auth.AuthMethod.API, Auth.AuthMethod.INTERNAL}))
+  public static class AuthWrongMethodOrdering {}
+
+  @Action(
+      path = "/auth/duplicateMethods",
+      auth = @Auth(methods = {Auth.AuthMethod.INTERNAL, Auth.AuthMethod.API, Auth.AuthMethod.API}))
+  public static class AuthDuplicateMethods {}
+
+  @Action(
+      path = "/auth/internalWithUser",
+      auth = @Auth(methods = {Auth.AuthMethod.INTERNAL}, minimumLevel = AuthLevel.USER))
+  public static class AuthInternalWithUser {}
+
+  @Action(
+      path = "/auth/wronglyIgnoringUser",
+      auth = @Auth(
+          methods = {Auth.AuthMethod.INTERNAL, Auth.AuthMethod.API},
+          userPolicy = Auth.UserPolicy.IGNORED))
+  public static class AuthWronglyIgnoringUser {}
+
+  private final UserService mockUserService = mock(UserService.class);
+  private final HttpServletRequest req = mock(HttpServletRequest.class);
+
+  private final User testUser = new User("test@google.com", "test@google.com");
+  private final FakeUserService fakeUserService = new FakeUserService();
+  private final FakeOAuthService fakeOAuthService = new FakeOAuthService(
+      false /* isOAuthEnabled */,
+      testUser,
+      false /* isUserAdmin */,
+      "test-client-id",
+      ImmutableList.of("test-scope1", "test-scope2", "nontest-scope"));
+
+  private RequestAuthenticator createRequestAuthenticator(UserService userService) {
     return new RequestAuthenticator(
         new AppEngineInternalAuthenticationMechanism(),
         ImmutableList.<AuthenticationMechanism>of(
             new OAuthAuthenticationMechanism(
-                OAuthServiceFactory.getOAuthService(),
+                fakeOAuthService,
-                ImmutableSet.of("https://www.googleapis.com/auth/userinfo.email"),
+                ImmutableSet.of("test-scope1", "test-scope2", "test-scope3"),
-                ImmutableSet.of("https://www.googleapis.com/auth/userinfo.email"),
+                ImmutableSet.of("test-scope1", "test-scope2"),
-                ImmutableSet.of("proxy-client-id", "regtool-client-id"))),
+                ImmutableSet.of("test-client-id", "other-test-client-id"))),
         new LegacyAuthenticationMechanism(userService));
   }
 
@@ -149,13 +195,14 @@ public class RequestAuthenticatorTest {
 
   @Test
   public void testAnyUserAnyMethod_success() throws Exception {
-    fakeUserService.setUser(testUser, false);
+    fakeUserService.setUser(testUser, false /* isAdmin */);
     Optional<AuthResult> 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);
+    assertThat(authResult.get().userAuthInfo().get().isUserAdmin()).isFalse();
     assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo()).isAbsent();
   }
 
@@ -167,21 +214,168 @@ public class RequestAuthenticatorTest {
 
   @Test
   public void testAdminUserAnyMethod_notAdminUser() throws Exception {
-    fakeUserService.setUser(testUser, false);
+    fakeUserService.setUser(testUser, false /* isAdmin */);
     Optional<AuthResult> authResult = runTest(fakeUserService, AuthAdminUserAnyMethod.class);
     assertThat(authResult).isAbsent();
   }
 
   @Test
   public void testAdminUserAnyMethod_success() throws Exception {
-    fakeUserService.setUser(testUser, true);
+    fakeUserService.setUser(testUser, true /* isAdmin */);
     Optional<AuthResult> authResult = runTest(fakeUserService, AuthAdminUserAnyMethod.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().isUserAdmin()).isTrue();
     assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo()).isAbsent();
   }
 
-  // TODO(mountford) Add more tests for OAuth, misconfiguration, etc., either here or separately
+  @Test
+  public void testOAuth_success() throws Exception {
+    fakeOAuthService.setUser(testUser);
+    fakeOAuthService.setOAuthEnabled(true);
+    when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN");
+    Optional<AuthResult> authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.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().isUserAdmin()).isFalse();
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo()).isPresent();
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo().get().authorizedScopes())
+      .containsAllOf("test-scope1", "test-scope2");
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo().get().oauthClientId())
+      .isEqualTo("test-client-id");
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo().get().rawAccessToken())
+      .isEqualTo("TOKEN");
+  }
+
+  @Test
+  public void testOAuthAdmin_success() throws Exception {
+    fakeOAuthService.setUser(testUser);
+    fakeOAuthService.setUserAdmin(true);
+    fakeOAuthService.setOAuthEnabled(true);
+    when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN");
+    Optional<AuthResult> authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.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().isUserAdmin()).isTrue();
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo()).isPresent();
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo().get().authorizedScopes())
+      .containsAllOf("test-scope1", "test-scope2");
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo().get().oauthClientId())
+      .isEqualTo("test-client-id");
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo().get().rawAccessToken())
+      .isEqualTo("TOKEN");
+  }
+
+  @Test
+  public void testOAuthMissingAuthenticationToken_failure() throws Exception {
+    fakeOAuthService.setUser(testUser);
+    fakeOAuthService.setOAuthEnabled(true);
+    Optional<AuthResult> authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class);
+    assertThat(authResult).isAbsent();
+  }
+
+  @Test
+  public void testOAuthClientIdMismatch_failure() throws Exception {
+    fakeOAuthService.setUser(testUser);
+    fakeOAuthService.setOAuthEnabled(true);
+    fakeOAuthService.setClientId("wrong-client-id");
+    when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN");
+    Optional<AuthResult> authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class);
+    assertThat(authResult).isAbsent();
+  }
+
+  @Test
+  public void testOAuthNoScopes_failure() throws Exception {
+    fakeOAuthService.setUser(testUser);
+    fakeOAuthService.setOAuthEnabled(true);
+    fakeOAuthService.setAuthorizedScopes();
+    when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN");
+    Optional<AuthResult> authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class);
+    assertThat(authResult).isAbsent();
+  }
+
+  @Test
+  public void testOAuthMissingScope_failure() throws Exception {
+    fakeOAuthService.setUser(testUser);
+    fakeOAuthService.setOAuthEnabled(true);
+    fakeOAuthService.setAuthorizedScopes("test-scope1", "test-scope3");
+    when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN");
+    Optional<AuthResult> authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.class);
+    assertThat(authResult).isAbsent();
+  }
+
+  @Test
+  public void testOAuthExtraScope_success() throws Exception {
+    fakeOAuthService.setUser(testUser);
+    fakeOAuthService.setOAuthEnabled(true);
+    fakeOAuthService.setAuthorizedScopes("test-scope1", "test-scope2", "test-scope3");
+    when(req.getHeader(AUTHORIZATION)).thenReturn("Bearer TOKEN");
+    Optional<AuthResult> authResult = runTest(fakeUserService, AuthAnyUserNoLegacy.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().isUserAdmin()).isFalse();
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo()).isPresent();
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo().get().authorizedScopes())
+      .containsAllOf("test-scope1", "test-scope2", "test-scope3");
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo().get().oauthClientId())
+      .isEqualTo("test-client-id");
+    assertThat(authResult.get().userAuthInfo().get().oauthTokenInfo().get().rawAccessToken())
+      .isEqualTo("TOKEN");
+  }
+
+  @Test
+  public void testAnyUserNoLegacy_failureWithLegacyUser() throws Exception {
+    fakeUserService.setUser(testUser, false /* isAdmin */);
+    Optional<AuthResult> 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);
+  }
+
+  @Test
+  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);
+  }
+
+  @Test
+  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);
+  }
+
+  @Test
+  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);
+  }
+
+  @Test
+  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);
+  }
+
+  @Test
+  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);
+  }
 }

javatests/google/registry/testing/FakeOAuthService.java

@@ -0,0 +1,130 @@
+// Copyright 2017 The Nomulus 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 com.google.appengine.api.oauth.OAuthRequestException;
+import com.google.appengine.api.oauth.OAuthService;
+import com.google.appengine.api.users.User;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+
+/** A fake {@link OAuthService} implementation for testing. */
+public class FakeOAuthService implements OAuthService {
+
+  private boolean isOAuthEnabled;
+  private User currentUser;
+  private boolean isUserAdmin;
+  private String clientId;
+  private ImmutableList<String> authorizedScopes;
+
+  public FakeOAuthService(
+      boolean isOAuthEnabled,
+      User currentUser,
+      boolean isUserAdmin,
+      String clientId,
+      List<String> authorizedScopes) {
+    this.isOAuthEnabled = isOAuthEnabled;
+    this.currentUser = currentUser;
+    this.isUserAdmin = isUserAdmin;
+    this.clientId = clientId;
+    this.authorizedScopes = ImmutableList.copyOf(authorizedScopes);
+  }
+
+  public void setOAuthEnabled(boolean isOAuthEnabled) {
+    this.isOAuthEnabled = isOAuthEnabled;
+  }
+
+  public void setUser(User currentUser) {
+    this.currentUser = currentUser;
+  }
+
+  public void setUserAdmin(boolean isUserAdmin) {
+    this.isUserAdmin = isUserAdmin;
+  }
+
+  public void setClientId(String clientId) {
+    this.clientId = clientId;
+  }
+
+  public void setAuthorizedScopes(String... scopes) {
+    this.authorizedScopes = ImmutableList.<String>copyOf(scopes);
+  }
+
+  @Override
+  public User getCurrentUser() throws OAuthRequestException {
+    if (!isOAuthEnabled) {
+      throw new OAuthRequestException("invalid OAuth request");
+    }
+    return currentUser;
+  }
+
+  @Override
+  public User getCurrentUser(String scope) throws OAuthRequestException {
+    return getCurrentUser();
+  }
+
+  @Override
+  public User getCurrentUser(String... scopes) throws OAuthRequestException {
+    return getCurrentUser();
+  }
+
+  @Override
+  public boolean isUserAdmin() throws OAuthRequestException {
+    if (!isOAuthEnabled) {
+      throw new OAuthRequestException("invalid OAuth request");
+    }
+    return isUserAdmin;
+  }
+
+  @Override
+  public boolean isUserAdmin(String scope) throws OAuthRequestException {
+    return isUserAdmin();
+  }
+
+  @Override
+  public boolean isUserAdmin(String... scopes) throws OAuthRequestException {
+    return isUserAdmin();
+  }
+
+  @Override
+  public String getClientId(String scope) throws OAuthRequestException {
+    if (!isOAuthEnabled) {
+      throw new OAuthRequestException("invalid OAuth request");
+    }
+    return clientId;
+  }
+
+  @Override
+  public String getClientId(String... scopes) throws OAuthRequestException {
+    if (!isOAuthEnabled) {
+      throw new OAuthRequestException("invalid OAuth request");
+    }
+    return clientId;
+  }
+
+  @Override
+  public String[] getAuthorizedScopes(String... scopes) throws OAuthRequestException {
+    if (!isOAuthEnabled) {
+      throw new OAuthRequestException("invalid OAuth request");
+    }
+    return authorizedScopes.toArray(new String[0]);
+  }
+
+  @Deprecated
+  @Override
+  public String getOAuthConsumerKey() throws OAuthRequestException {
+    throw new UnsupportedOperationException();
+  }
+}