diff --git a/java/google/registry/flows/EppConsoleAction.java b/java/google/registry/flows/EppConsoleAction.java index 6e0061d31..8bffa4be9 100644 --- a/java/google/registry/flows/EppConsoleAction.java +++ b/java/google/registry/flows/EppConsoleAction.java @@ -14,8 +14,7 @@ package google.registry.flows; -import static com.google.appengine.api.users.UserServiceFactory.getUserService; - +import com.google.appengine.api.users.UserService; import google.registry.request.Action; import google.registry.request.Action.Method; import google.registry.request.Payload; @@ -35,13 +34,14 @@ public class EppConsoleAction implements Runnable { @Inject @Payload byte[] inputXmlBytes; @Inject HttpSession session; @Inject EppRequestHandler eppRequestHandler; + @Inject UserService userService; @Inject EppConsoleAction() {} @Override public void run() { eppRequestHandler.executeEpp( new HttpSessionMetadata(session), - new GaeUserCredentials(getUserService().getCurrentUser()), + GaeUserCredentials.forCurrentUser(userService), EppRequestSource.CONSOLE, false, // This endpoint is never a dry run. false, // This endpoint is never a superuser. diff --git a/java/google/registry/flows/GaeUserCredentials.java b/java/google/registry/flows/GaeUserCredentials.java index 2d4d91345..62908d59e 100644 --- a/java/google/registry/flows/GaeUserCredentials.java +++ b/java/google/registry/flows/GaeUserCredentials.java @@ -14,11 +14,12 @@ package google.registry.flows; -import static com.google.appengine.api.users.UserServiceFactory.getUserService; import static com.google.common.base.MoreObjects.toStringHelper; import static com.google.common.base.Strings.nullToEmpty; +import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.google.appengine.api.users.User; +import com.google.appengine.api.users.UserService; import com.google.common.annotations.VisibleForTesting; import google.registry.flows.EppException.AuthenticationErrorException; import google.registry.model.registrar.Registrar; @@ -28,11 +29,41 @@ import javax.annotation.Nullable; /** Credentials provided by {@link com.google.appengine.api.users.UserService}. */ public class GaeUserCredentials implements TransportCredentials { - final User gaeUser; + private final User gaeUser; + private final Boolean isAdmin; + + /** + * Create an instance for the current user, as determined by {@code UserService}. + * + *

Note that the current user may be null (i.e. there is no logged in user). + */ + public static GaeUserCredentials forCurrentUser(UserService userService) { + User user = userService.getCurrentUser(); + return new GaeUserCredentials(user, user != null ? userService.isUserAdmin() : null); + } + + /** Create an instance that represents an explicit user (for testing purposes). */ + @VisibleForTesting + public static GaeUserCredentials forTestingUser(User gaeUser, Boolean isAdmin) { + checkArgumentNotNull(gaeUser); + checkArgumentNotNull(isAdmin); + return new GaeUserCredentials(gaeUser, isAdmin); + } + + /** Create an instance that represents a non-logged in user (for testing purposes). */ + @VisibleForTesting + public static GaeUserCredentials forLoggedOutUser() { + return new GaeUserCredentials(null, null); + } + + private GaeUserCredentials(@Nullable User gaeUser, @Nullable Boolean isAdmin) { + this.gaeUser = gaeUser; + this.isAdmin = isAdmin; + } @VisibleForTesting - public GaeUserCredentials(@Nullable User gaeUser) { - this.gaeUser = gaeUser; + User getUser() { + return gaeUser; } @Override @@ -42,7 +73,7 @@ public class GaeUserCredentials implements TransportCredentials { throw new UserNotLoggedInException(); } // Allow admins to act as any registrar. - if (getUserService().isUserAdmin()) { + if (Boolean.TRUE.equals(isAdmin)) { return; } // Check Registrar's contacts to see if any are associated with this gaeUserId. @@ -59,6 +90,7 @@ public class GaeUserCredentials implements TransportCredentials { public String toString() { return toStringHelper(getClass()) .add("gaeUser", gaeUser) + .add("isAdmin", isAdmin) .toString(); } diff --git a/javatests/google/registry/flows/EppConsoleActionTest.java b/javatests/google/registry/flows/EppConsoleActionTest.java index d03892689..daf937433 100644 --- a/javatests/google/registry/flows/EppConsoleActionTest.java +++ b/javatests/google/registry/flows/EppConsoleActionTest.java @@ -14,7 +14,7 @@ package google.registry.flows; - +import static com.google.appengine.api.users.UserServiceFactory.getUserService; import static com.google.common.truth.Truth.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; import static org.mockito.Matchers.eq; @@ -49,6 +49,7 @@ public class EppConsoleActionTest extends ShardableTestCase { action.session = new FakeHttpSession(); action.session.setAttribute("CLIENT_ID", "ClientIdentifier"); action.eppRequestHandler = mock(EppRequestHandler.class); + action.userService = getUserService(); action.run(); ArgumentCaptor credentialsCaptor = ArgumentCaptor.forClass(TransportCredentials.class); @@ -60,7 +61,7 @@ public class EppConsoleActionTest extends ShardableTestCase { eq(false), eq(false), eq(INPUT_XML_BYTES)); - assertThat(((GaeUserCredentials) credentialsCaptor.getValue()).gaeUser.getEmail()) + assertThat(((GaeUserCredentials) credentialsCaptor.getValue()).getUser().getEmail()) .isEqualTo("person@example.com"); assertThat(metadataCaptor.getValue().getClientId()).isEqualTo("ClientIdentifier"); } diff --git a/javatests/google/registry/flows/EppLoginAdminUserTest.java b/javatests/google/registry/flows/EppLoginAdminUserTest.java index b7bc34a41..9ca703028 100644 --- a/javatests/google/registry/flows/EppLoginAdminUserTest.java +++ b/javatests/google/registry/flows/EppLoginAdminUserTest.java @@ -36,7 +36,7 @@ public class EppLoginAdminUserTest extends EppTestCase { @Before public void initTransportCredentials() { - setTransportCredentials(new GaeUserCredentials(getUserService().getCurrentUser())); + setTransportCredentials(GaeUserCredentials.forCurrentUser(getUserService())); } @Test diff --git a/javatests/google/registry/flows/EppLoginUserTest.java b/javatests/google/registry/flows/EppLoginUserTest.java index c10a0043b..affa90d16 100644 --- a/javatests/google/registry/flows/EppLoginUserTest.java +++ b/javatests/google/registry/flows/EppLoginUserTest.java @@ -48,7 +48,7 @@ public class EppLoginUserTest extends EppTestCase { .setGaeUserId(user.getUserId()) .setTypes(ImmutableSet.of(RegistrarContact.Type.ADMIN)) .build()); - setTransportCredentials(new GaeUserCredentials(user)); + setTransportCredentials(GaeUserCredentials.forCurrentUser(getUserService())); } @Test diff --git a/javatests/google/registry/flows/FlowRunnerTest.java b/javatests/google/registry/flows/FlowRunnerTest.java index 7141ea34b..0caf5a94e 100644 --- a/javatests/google/registry/flows/FlowRunnerTest.java +++ b/javatests/google/registry/flows/FlowRunnerTest.java @@ -178,10 +178,11 @@ public class FlowRunnerTest extends ShardableTestCase { @Test public void testRun_legacyLoggingStatement_gaeUserCredentials() throws Exception { - flowRunner.credentials = new GaeUserCredentials(new User("user@example.com", "authDomain")); + flowRunner.credentials = + GaeUserCredentials.forTestingUser(new User("user@example.com", "authDomain"), false); flowRunner.run(); assertThat(Splitter.on("\n\t").split(findLogMessageByPrefix(handler, "EPP Command\n\t"))) - .contains("GaeUserCredentials{gaeUser=user@example.com}"); + .contains("GaeUserCredentials{gaeUser=user@example.com, isAdmin=false}"); } @Test diff --git a/javatests/google/registry/flows/session/LoginFlowViaConsoleTest.java b/javatests/google/registry/flows/session/LoginFlowViaConsoleTest.java index 8db4726d3..b3c97a4a1 100644 --- a/javatests/google/registry/flows/session/LoginFlowViaConsoleTest.java +++ b/javatests/google/registry/flows/session/LoginFlowViaConsoleTest.java @@ -15,19 +15,15 @@ package google.registry.flows.session; -import static com.google.appengine.api.users.UserServiceFactory.getUserService; import static google.registry.testing.DatastoreHelper.persistResource; -import com.google.apphosting.api.ApiProxy; -import com.google.apphosting.api.ApiProxy.Environment; +import com.google.appengine.api.users.User; import com.google.common.collect.ImmutableSet; import google.registry.flows.GaeUserCredentials; import google.registry.flows.GaeUserCredentials.BadGaeUserIdException; import google.registry.flows.GaeUserCredentials.UserNotLoggedInException; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; -import java.util.HashMap; -import java.util.Map; import org.junit.Test; /** @@ -39,123 +35,43 @@ public class LoginFlowViaConsoleTest extends LoginFlowTestCase { private static final String GAE_USER_ID1 = "12345"; private static final String GAE_USER_ID2 = "54321"; - Environment oldEnv = null; - @Test public void testSuccess_withLoginAndLinkedAccount() throws Exception { persistLinkedAccount("person@example.com", GAE_USER_ID1); - login("person", "example.com", GAE_USER_ID1); - try { - doSuccessfulTest("login_valid.xml"); - } finally { - ApiProxy.setEnvironmentForCurrentThread(oldEnv); - } + credentials = + GaeUserCredentials.forTestingUser(new User("person", "example.com", GAE_USER_ID1), false); + doSuccessfulTest("login_valid.xml"); } @Test public void testFailure_withoutLoginAndLinkedAccount() throws Exception { persistLinkedAccount("person@example.com", GAE_USER_ID1); - noLogin(); + credentials = GaeUserCredentials.forLoggedOutUser(); doFailingTest("login_valid.xml", UserNotLoggedInException.class); } @Test public void testFailure_withoutLoginAndWithoutLinkedAccount() throws Exception { - noLogin(); + credentials = GaeUserCredentials.forLoggedOutUser(); doFailingTest("login_valid.xml", UserNotLoggedInException.class); } @Test public void testFailure_withLoginAndWithoutLinkedAccount() throws Exception { - login("person", "example.com", GAE_USER_ID1); - try { - doFailingTest("login_valid.xml", BadGaeUserIdException.class); - } finally { - ApiProxy.setEnvironmentForCurrentThread(oldEnv); - } + credentials = + GaeUserCredentials.forTestingUser(new User("person", "example.com", GAE_USER_ID1), false); + doFailingTest("login_valid.xml", BadGaeUserIdException.class); } @Test public void testFailure_withLoginAndNoMatchingLinkedAccount() throws Exception { persistLinkedAccount("joe@example.com", GAE_USER_ID2); - login("person", "example.com", GAE_USER_ID1); - try { - doFailingTest("login_valid.xml", BadGaeUserIdException.class); - } finally { - ApiProxy.setEnvironmentForCurrentThread(oldEnv); - } + credentials = + GaeUserCredentials.forTestingUser(new User("person", "example.com", GAE_USER_ID1), false); + doFailingTest("login_valid.xml", BadGaeUserIdException.class); } - Environment login(final String name, final String authDomain, final String gaeUserId) { - oldEnv = ApiProxy.getCurrentEnvironment(); - // This envAttr thing is the only way to set userId. - // see https://code.google.com/p/googleappengine/issues/detail?id=3579 - final HashMap envAttr = new HashMap<>(oldEnv.getAttributes()); - envAttr.put("com.google.appengine.api.users.UserService.user_id_key", gaeUserId); - - final Environment e = oldEnv; - ApiProxy.setEnvironmentForCurrentThread(new Environment() { - @Override - public String getAppId() { - return e.getAppId(); - } - - @Override - public String getModuleId() { - return e.getModuleId(); - } - - @Override - public String getVersionId() { - return e.getVersionId(); - } - - @Override - public String getEmail() { - return name + "@" + authDomain; - } - - @Override - public boolean isLoggedIn() { - return true; - } - - @Override - public boolean isAdmin() { - return e.isAdmin(); - } - - @Override - public String getAuthDomain() { - return authDomain; - } - - @Override - @SuppressWarnings("deprecation") - public String getRequestNamespace() { - return e.getRequestNamespace(); - } - - @Override - public long getRemainingMillis() { - return e.getRemainingMillis(); - } - - @Override - public Map getAttributes() { - return envAttr; - } - }); - credentials = new GaeUserCredentials(getUserService().getCurrentUser()); - return oldEnv; - } - - void noLogin() { - oldEnv = ApiProxy.getCurrentEnvironment(); - credentials = new GaeUserCredentials(getUserService().getCurrentUser()); - } - - void persistLinkedAccount(String email, String gaeUserId) { + private void persistLinkedAccount(String email, String gaeUserId) { Registrar registrar = Registrar.loadByClientId("NewRegistrar"); RegistrarContact c = new RegistrarContact.Builder() .setParent(registrar)