From 2b7f78db989d86a5b46f9143e09f981c1e3fbb5f Mon Sep 17 00:00:00 2001 From: mountford Date: Tue, 20 Jun 2017 08:16:43 -0700 Subject: [PATCH] Change registrar console login code in preparation for removing requireLogin We are going to remove the requireLogin attribute from the action attribute, because it is specific to the UserService API. This is used by four actions: ConsoleUIAction RegistrarSettingsAction RegistrarPaymentSetupAction RegistrarPaymentAction Instead, these four actions will now check the login status directly. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=159562335 --- .../frontend/FrontendRequestComponent.java | 2 - .../registry/request/auth/AuthResult.java | 4 +- .../registry/request/auth/UserAuthInfo.java | 4 +- .../ui/server/registrar/ConsoleUiAction.java | 19 +++++--- .../registrar/RegistrarPaymentAction.java | 7 ++- .../RegistrarPaymentSetupAction.java | 8 +++- .../registrar/RegistrarSettingsAction.java | 10 ++--- .../server/registrar/RegistrarUserModule.java | 34 -------------- .../ui/server/registrar/SessionUtils.java | 44 +++++++++++-------- .../google/registry/ui/server/registrar/BUILD | 1 + .../server/registrar/ConsoleUiActionTest.java | 17 ++++--- .../registrar/RegistrarPaymentActionTest.java | 16 ++++++- .../RegistrarPaymentSetupActionTest.java | 33 +++++++++++--- .../RegistrarSettingsActionTest.java | 21 ++++++--- .../RegistrarSettingsActionTestCase.java | 12 +++-- .../registrar/SecuritySettingsTest.java | 10 +++-- .../ui/server/registrar/SessionUtilsTest.java | 21 +++------ 17 files changed, 151 insertions(+), 112 deletions(-) delete mode 100644 java/google/registry/ui/server/registrar/RegistrarUserModule.java diff --git a/java/google/registry/module/frontend/FrontendRequestComponent.java b/java/google/registry/module/frontend/FrontendRequestComponent.java index db02d5862..bd974e40b 100644 --- a/java/google/registry/module/frontend/FrontendRequestComponent.java +++ b/java/google/registry/module/frontend/FrontendRequestComponent.java @@ -41,7 +41,6 @@ import google.registry.ui.server.registrar.ConsoleUiAction; import google.registry.ui.server.registrar.RegistrarPaymentAction; import google.registry.ui.server.registrar.RegistrarPaymentSetupAction; import google.registry.ui.server.registrar.RegistrarSettingsAction; -import google.registry.ui.server.registrar.RegistrarUserModule; import google.registry.whois.WhoisHttpServer; import google.registry.whois.WhoisModule; import google.registry.whois.WhoisServer; @@ -54,7 +53,6 @@ import google.registry.whois.WhoisServer; DnsModule.class, EppTlsModule.class, RdapModule.class, - RegistrarUserModule.class, RequestModule.class, WhiteboxModule.class, WhoisModule.class, diff --git a/java/google/registry/request/auth/AuthResult.java b/java/google/registry/request/auth/AuthResult.java index 23029a8ab..925dd09f8 100644 --- a/java/google/registry/request/auth/AuthResult.java +++ b/java/google/registry/request/auth/AuthResult.java @@ -36,11 +36,11 @@ public abstract class AuthResult { return authLevel() != AuthLevel.NONE; } - static AuthResult create(AuthLevel authLevel) { + public static AuthResult create(AuthLevel authLevel) { return new AutoValue_AuthResult(authLevel, Optional.absent()); } - static AuthResult create(AuthLevel authLevel, @Nullable UserAuthInfo userAuthInfo) { + public static AuthResult create(AuthLevel authLevel, @Nullable UserAuthInfo userAuthInfo) { if (authLevel == AuthLevel.USER) { checkNotNull(userAuthInfo); } diff --git a/java/google/registry/request/auth/UserAuthInfo.java b/java/google/registry/request/auth/UserAuthInfo.java index 64033109e..d171ec4de 100644 --- a/java/google/registry/request/auth/UserAuthInfo.java +++ b/java/google/registry/request/auth/UserAuthInfo.java @@ -37,12 +37,12 @@ public abstract class UserAuthInfo { /** Used by the OAuth authentication mechanism (only) to return information about the session. */ public abstract Optional oauthTokenInfo(); - static UserAuthInfo create( + public static UserAuthInfo create( User user, boolean isUserAdmin) { return new AutoValue_UserAuthInfo(user, isUserAdmin, Optional.absent()); } - static UserAuthInfo create( + public static UserAuthInfo create( User user, boolean isUserAdmin, OAuthTokenInfo oauthTokenInfo) { return new AutoValue_UserAuthInfo(user, isUserAdmin, Optional.of(oauthTokenInfo)); } diff --git a/java/google/registry/ui/server/registrar/ConsoleUiAction.java b/java/google/registry/ui/server/registrar/ConsoleUiAction.java index 8c6ecb9b6..18a6db312 100644 --- a/java/google/registry/ui/server/registrar/ConsoleUiAction.java +++ b/java/google/registry/ui/server/registrar/ConsoleUiAction.java @@ -14,10 +14,13 @@ package google.registry.ui.server.registrar; +import static com.google.common.net.HttpHeaders.LOCATION; import static com.google.common.net.HttpHeaders.X_FRAME_OPTIONS; import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; +import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE; +import com.google.appengine.api.users.User; import com.google.appengine.api.users.UserService; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Supplier; @@ -32,6 +35,7 @@ import google.registry.request.Action; import google.registry.request.Response; import google.registry.request.auth.Auth; import google.registry.request.auth.AuthLevel; +import google.registry.request.auth.AuthResult; import google.registry.security.XsrfTokenManager; import google.registry.ui.server.SoyTemplateUtils; import google.registry.ui.soy.registrar.ConsoleSoyInfo; @@ -69,6 +73,7 @@ public final class ConsoleUiAction implements Runnable { @Inject SessionUtils sessionUtils; @Inject UserService userService; @Inject XsrfTokenManager xsrfTokenManager; + @Inject AuthResult authResult; @Inject @Config("logoFilename") String logoFilename; @Inject @Config("productName") String productName; @Inject @Config("integrationEmail") String integrationEmail; @@ -81,6 +86,12 @@ public final class ConsoleUiAction implements Runnable { @Override public void run() { + if (!authResult.userAuthInfo().isPresent()) { + response.setStatus(SC_MOVED_TEMPORARILY); + response.setHeader(LOCATION, userService.createLoginURL(req.getRequestURI())); + return; + } + User user = authResult.userAuthInfo().get().user(); response.setContentType(MediaType.HTML_UTF_8); response.setHeader(X_FRAME_OPTIONS, "SAMEORIGIN"); // Disallow iframing. response.setHeader("X-Ui-Compatible", "IE=edge"); // Ask IE not to be silly. @@ -102,9 +113,9 @@ public final class ConsoleUiAction implements Runnable { .render()); return; } - data.put("username", userService.getCurrentUser().getNickname()); + data.put("username", user.getNickname()); data.put("logoutUrl", userService.createLogoutURL(PATH)); - if (!sessionUtils.checkRegistrarConsoleLogin(req)) { + if (!sessionUtils.checkRegistrarConsoleLogin(req, user)) { response.setStatus(SC_FORBIDDEN); response.setPayload( TOFU_SUPPLIER.get() @@ -115,9 +126,7 @@ public final class ConsoleUiAction implements Runnable { return; } Registrar registrar = Registrar.loadByClientIdCached(sessionUtils.getRegistrarClientId(req)); - data.put( - "xsrfToken", - xsrfTokenManager.generateToken(userService.getCurrentUser().getEmail())); + data.put("xsrfToken", xsrfTokenManager.generateToken(user.getEmail())); data.put("clientId", registrar.getClientId()); data.put("showPaymentLink", registrar.getBillingMethod() == Registrar.BillingMethod.BRAINTREE); diff --git a/java/google/registry/ui/server/registrar/RegistrarPaymentAction.java b/java/google/registry/ui/server/registrar/RegistrarPaymentAction.java index 0ab56f970..ac5b6f61b 100644 --- a/java/google/registry/ui/server/registrar/RegistrarPaymentAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarPaymentAction.java @@ -37,6 +37,7 @@ import google.registry.request.JsonActionRunner; import google.registry.request.JsonActionRunner.JsonAction; import google.registry.request.auth.Auth; import google.registry.request.auth.AuthLevel; +import google.registry.request.auth.AuthResult; import google.registry.security.JsonResponseHelper; import google.registry.ui.forms.FormField; import google.registry.ui.forms.FormFieldException; @@ -46,6 +47,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; import org.joda.money.CurrencyUnit; import org.joda.money.IllegalCurrencyException; import org.joda.money.Money; @@ -148,9 +150,11 @@ public final class RegistrarPaymentAction implements Runnable, JsonAction { .required() .build(); + @Inject HttpServletRequest request; @Inject BraintreeGateway braintreeGateway; @Inject JsonActionRunner jsonActionRunner; - @Inject Registrar registrar; + @Inject AuthResult authResult; + @Inject SessionUtils sessionUtils; @Inject @Config("braintreeMerchantAccountIds") ImmutableMap accountIds; @Inject RegistrarPaymentAction() {} @@ -161,6 +165,7 @@ public final class RegistrarPaymentAction implements Runnable, JsonAction { @Override public Map handleJsonRequest(Map json) { + Registrar registrar = sessionUtils.getRegistrarForAuthResult(request, authResult); logger.infofmt("Processing payment: %s", json); String paymentMethodNonce; Money amount; diff --git a/java/google/registry/ui/server/registrar/RegistrarPaymentSetupAction.java b/java/google/registry/ui/server/registrar/RegistrarPaymentSetupAction.java index 5932aaff6..c36341bcd 100644 --- a/java/google/registry/ui/server/registrar/RegistrarPaymentSetupAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarPaymentSetupAction.java @@ -30,9 +30,11 @@ import google.registry.request.JsonActionRunner; import google.registry.request.JsonActionRunner.JsonAction; import google.registry.request.auth.Auth; import google.registry.request.auth.AuthLevel; +import google.registry.request.auth.AuthResult; import google.registry.security.JsonResponseHelper; import java.util.Map; import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; import org.joda.money.CurrencyUnit; /** @@ -80,10 +82,12 @@ import org.joda.money.CurrencyUnit; ) public final class RegistrarPaymentSetupAction implements Runnable, JsonAction { + @Inject HttpServletRequest request; @Inject BraintreeGateway braintreeGateway; @Inject BraintreeRegistrarSyncer customerSyncer; @Inject JsonActionRunner jsonActionRunner; - @Inject Registrar registrar; + @Inject AuthResult authResult; + @Inject SessionUtils sessionUtils; @Inject @Config("brainframe") String brainframe; @Inject @Config("braintreeMerchantAccountIds") ImmutableMap accountIds; @Inject RegistrarPaymentSetupAction() {} @@ -95,6 +99,8 @@ public final class RegistrarPaymentSetupAction implements Runnable, JsonAction { @Override public Map handleJsonRequest(Map json) { + Registrar registrar = sessionUtils.getRegistrarForAuthResult(request, authResult); + if (!json.isEmpty()) { return JsonResponseHelper.create(ERROR, "JSON request object must be empty"); } diff --git a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index 5c7fde99e..fb381ebb2 100644 --- a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -42,6 +42,7 @@ import google.registry.request.HttpException.BadRequestException; import google.registry.request.JsonActionRunner; import google.registry.request.auth.Auth; import google.registry.request.auth.AuthLevel; +import google.registry.request.auth.AuthResult; import google.registry.security.JsonResponseHelper; import google.registry.ui.forms.FormException; import google.registry.ui.forms.FormFieldException; @@ -81,7 +82,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA @Inject HttpServletRequest request; @Inject JsonActionRunner jsonActionRunner; - @Inject Registrar initialRegistrar; + @Inject AuthResult authResult; @Inject SendEmailUtils sendEmailUtils; @Inject SessionUtils sessionUtils; @Inject @Config("registrarChangesNotificationEmailAddresses") ImmutableList @@ -105,12 +106,9 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA throw new BadRequestException("Malformed JSON"); } - if (!sessionUtils.checkRegistrarConsoleLogin(request)) { - return JsonResponseHelper.create(ERROR, "Not authorized to access Registrar Console"); - } - + Registrar initialRegistrar = sessionUtils.getRegistrarForAuthResult(request, authResult); // Process the operation. Though originally derived from a CRUD - // handlder, registrar-settings really only supports read and update. + // handler, registrar-settings really only supports read and update. String op = Optional.fromNullable((String) input.get(OP_PARAM)).or("read"); @SuppressWarnings("unchecked") Map args = (Map) diff --git a/java/google/registry/ui/server/registrar/RegistrarUserModule.java b/java/google/registry/ui/server/registrar/RegistrarUserModule.java deleted file mode 100644 index 91f685d63..000000000 --- a/java/google/registry/ui/server/registrar/RegistrarUserModule.java +++ /dev/null @@ -1,34 +0,0 @@ -// 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.ui.server.registrar; - -import dagger.Module; -import dagger.Provides; -import google.registry.model.registrar.Registrar; -import google.registry.request.HttpException.ForbiddenException; -import javax.servlet.http.HttpServletRequest; - -/** Registrar Console module providing reference to logged-in {@link Registrar}. */ -@Module -public final class RegistrarUserModule { - - @Provides - static Registrar provideRegistrarUser(SessionUtils sessionUtils, HttpServletRequest req) { - if (!sessionUtils.checkRegistrarConsoleLogin(req)) { - throw new ForbiddenException("Not authorized to access Registrar Console"); - } - return Registrar.loadByClientId(sessionUtils.getRegistrarClientId(req)); - } -} diff --git a/java/google/registry/ui/server/registrar/SessionUtils.java b/java/google/registry/ui/server/registrar/SessionUtils.java index fc60bcbb7..ab136ac07 100644 --- a/java/google/registry/ui/server/registrar/SessionUtils.java +++ b/java/google/registry/ui/server/registrar/SessionUtils.java @@ -14,19 +14,19 @@ package google.registry.ui.server.registrar; -import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Verify.verify; import static google.registry.model.ofy.ObjectifyService.ofy; import com.google.appengine.api.users.User; -import com.google.appengine.api.users.UserService; import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.collect.FluentIterable; import com.googlecode.objectify.Key; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; +import google.registry.request.HttpException.ForbiddenException; +import google.registry.request.auth.AuthResult; import google.registry.util.FormattingLogger; import javax.annotation.CheckReturnValue; import javax.annotation.Nonnull; @@ -43,38 +43,49 @@ public class SessionUtils { public static final String CLIENT_ID_ATTRIBUTE = "clientId"; - private final UserService userService; + @Inject public SessionUtils() {} - @Inject - public SessionUtils(UserService userService) { - this.userService = checkNotNull(userService); + /** + * Checks that the authentication result indicates a user that has access to the registrar + * console, then gets the associated registrar. + * + *

Throws a {@link ForbiddenException} if the user is not logged in, or not authorized to use + * the registrar console. + */ + @CheckReturnValue + Registrar getRegistrarForAuthResult(HttpServletRequest request, AuthResult authResult) { + if (!authResult.userAuthInfo().isPresent()) { + throw new ForbiddenException("Not logged in"); + } + if (!checkRegistrarConsoleLogin(request, authResult.userAuthInfo().get().user())) { + throw new ForbiddenException("Not authorized to access Registrar Console"); + } + return Registrar.loadByClientIdCached(getRegistrarClientId(request)); } /** - * Checks GAE user has access to Registrar Console. + * Checks that the specified user has access to the Registrar Console. * *

This routine will first check the HTTP session (creating one if it doesn't exist) for the * {@code clientId} attribute: * *

    *
  • If it does not exist, then we will attempt to guess the {@link Registrar} with which the - * user's GAIA ID is associated. The {@code clientId} of the first matching {@code Registrar} will - * then be stored to the HTTP session. + * user is associated. The {@code clientId} of the first matching {@code Registrar} will then + * be stored to the HTTP session. *
  • If it does exist, then we'll fetch the Registrar from Datastore to make sure access * wasn't revoked. *
* - *

Note: You must ensure the user has logged in before calling this method, for example - * by setting {@code @Action(requireLogin = true)}. + *

Note: You must ensure the user has logged in before calling this method. * * @return {@code false} if user does not have access, in which case the caller should write an * error response and abort the request. */ @CheckReturnValue - public boolean checkRegistrarConsoleLogin(HttpServletRequest req) { - HttpSession session = req.getSession(); - User user = userService.getCurrentUser(); + public boolean checkRegistrarConsoleLogin(HttpServletRequest req, User user) { checkState(user != null, "No logged in user found"); + HttpSession session = req.getSession(); String clientId = (String) session.getAttribute(CLIENT_ID_ATTRIBUTE); if (clientId == null) { Optional registrar = guessRegistrar(user.getUserId()); @@ -108,11 +119,6 @@ public class SessionUtils { return clientId; } - /** @see UserService#isUserLoggedIn() */ - public boolean isLoggedIn() { - return userService.isUserLoggedIn(); - } - /** Returns first {@link Registrar} that {@code gaeUserId} is authorized to administer. */ private static Optional guessRegistrar(String gaeUserId) { RegistrarContact contact = ofy().load() diff --git a/javatests/google/registry/ui/server/registrar/BUILD b/javatests/google/registry/ui/server/registrar/BUILD index 48455d62c..cb4a50ed6 100644 --- a/javatests/google/registry/ui/server/registrar/BUILD +++ b/javatests/google/registry/ui/server/registrar/BUILD @@ -17,6 +17,7 @@ java_library( "//java/google/registry/export/sheet", "//java/google/registry/model", "//java/google/registry/request", + "//java/google/registry/request/auth", "//java/google/registry/security", "//java/google/registry/ui/server/registrar", "//java/google/registry/ui/soy/registrar:soy_java_wrappers", diff --git a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java index c427d499c..4e86b62bb 100644 --- a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java +++ b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java @@ -19,8 +19,12 @@ import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.google.appengine.api.users.User; import com.google.appengine.api.users.UserServiceFactory; import com.google.common.net.MediaType; +import google.registry.request.auth.AuthLevel; +import google.registry.request.auth.AuthResult; +import google.registry.request.auth.UserAuthInfo; import google.registry.security.XsrfTokenManager; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; @@ -44,9 +48,10 @@ public class ConsoleUiActionTest { .build(); private final SessionUtils sessionUtils = mock(SessionUtils.class); - + private final HttpServletRequest request = mock(HttpServletRequest.class); private final FakeResponse response = new FakeResponse(); private final ConsoleUiAction action = new ConsoleUiAction(); + private final User user = new User("marla.singer@example.com", "gmail.com", "12345"); @Before public void setUp() throws Exception { @@ -58,13 +63,14 @@ public class ConsoleUiActionTest { action.announcementsEmail = "announcements@example.com"; action.supportPhoneNumber = "1 (888) 555 0123"; action.technicalDocsUrl = "http://example.com/technical-docs"; + action.req = request; action.response = response; action.sessionUtils = sessionUtils; action.userService = UserServiceFactory.getUserService(); action.xsrfTokenManager = new XsrfTokenManager(new FakeClock(), action.userService); - when(sessionUtils.checkRegistrarConsoleLogin(any(HttpServletRequest.class))).thenReturn(true); - when(sessionUtils.getRegistrarClientId(any(HttpServletRequest.class))) - .thenReturn("TheRegistrar"); + action.authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false)); + when(sessionUtils.checkRegistrarConsoleLogin(request, user)).thenReturn(true); + when(sessionUtils.getRegistrarClientId(request)).thenReturn("TheRegistrar"); } @Test @@ -101,7 +107,8 @@ public class ConsoleUiActionTest { @Test public void testUserDoesntHaveAccessToAnyRegistrar_showsWhoAreYouPage() throws Exception { - when(sessionUtils.checkRegistrarConsoleLogin(any(HttpServletRequest.class))).thenReturn(false); + when(sessionUtils.checkRegistrarConsoleLogin(any(HttpServletRequest.class), any(User.class))) + .thenReturn(false); action.run(); assertThat(response.getPayload()).contains("

You need permission

"); } diff --git a/javatests/google/registry/ui/server/registrar/RegistrarPaymentActionTest.java b/javatests/google/registry/ui/server/registrar/RegistrarPaymentActionTest.java index ae6d347f6..d5dfdf462 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarPaymentActionTest.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarPaymentActionTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.ReflectiveFieldExtractor.extractField; import static java.util.Arrays.asList; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -30,10 +31,15 @@ import com.braintreegateway.TransactionRequest; import com.braintreegateway.ValidationError; import com.braintreegateway.ValidationErrorCode; import com.braintreegateway.ValidationErrors; +import com.google.appengine.api.users.User; import com.google.common.collect.ImmutableMap; import google.registry.model.registrar.Registrar; +import google.registry.request.auth.AuthLevel; +import google.registry.request.auth.AuthResult; +import google.registry.request.auth.UserAuthInfo; import google.registry.testing.AppEngineRule; import java.math.BigDecimal; +import javax.servlet.http.HttpServletRequest; import org.joda.money.CurrencyUnit; import org.junit.Before; import org.junit.Rule; @@ -66,6 +72,10 @@ public class RegistrarPaymentActionTest { @Mock private ValidationErrors validationErrors; + private final SessionUtils sessionUtils = mock(SessionUtils.class); + + private final User user = new User("marla.singer@example.com", "gmail.com", "12345"); + @Captor private ArgumentCaptor transactionRequestCaptor; @@ -73,7 +83,8 @@ public class RegistrarPaymentActionTest { @Before public void before() throws Exception { - paymentAction.registrar = Registrar.loadByClientId("TheRegistrar"); + paymentAction.sessionUtils = sessionUtils; + paymentAction.authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false)); paymentAction.accountIds = ImmutableMap.of( CurrencyUnit.USD, "merchant-account-usd", @@ -81,6 +92,9 @@ public class RegistrarPaymentActionTest { paymentAction.braintreeGateway = braintreeGateway; when(braintreeGateway.transaction()).thenReturn(transactionGateway); when(transactionGateway.sale(any(TransactionRequest.class))).thenReturn(result); + when(sessionUtils.getRegistrarForAuthResult( + any(HttpServletRequest.class), any(AuthResult.class))) + .thenReturn(Registrar.loadByClientId("TheRegistrar")); } @Test diff --git a/javatests/google/registry/ui/server/registrar/RegistrarPaymentSetupActionTest.java b/javatests/google/registry/ui/server/registrar/RegistrarPaymentSetupActionTest.java index 4cfbc3427..763081ab1 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarPaymentSetupActionTest.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarPaymentSetupActionTest.java @@ -15,7 +15,9 @@ package google.registry.ui.server.registrar; import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.DatastoreHelper.persistResource; import static java.util.Arrays.asList; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -23,10 +25,15 @@ import static org.mockito.Mockito.when; import com.braintreegateway.BraintreeGateway; import com.braintreegateway.ClientTokenGateway; +import com.google.appengine.api.users.User; import com.google.common.collect.ImmutableMap; import google.registry.braintree.BraintreeRegistrarSyncer; import google.registry.model.registrar.Registrar; +import google.registry.request.auth.AuthLevel; +import google.registry.request.auth.AuthResult; +import google.registry.request.auth.UserAuthInfo; import google.registry.testing.AppEngineRule; +import javax.servlet.http.HttpServletRequest; import org.joda.money.CurrencyUnit; import org.junit.Before; import org.junit.Rule; @@ -46,17 +53,25 @@ public class RegistrarPaymentSetupActionTest { private final BraintreeGateway braintreeGateway = mock(BraintreeGateway.class); private final ClientTokenGateway clientTokenGateway = mock(ClientTokenGateway.class); private final BraintreeRegistrarSyncer customerSyncer = mock(BraintreeRegistrarSyncer.class); + private final SessionUtils sessionUtils = mock(SessionUtils.class); + private final User user = new User("marla.singer@example.com", "gmail.com", "12345"); private final RegistrarPaymentSetupAction action = new RegistrarPaymentSetupAction(); @Before public void before() throws Exception { + action.sessionUtils = sessionUtils; + action.authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false)); action.braintreeGateway = braintreeGateway; action.customerSyncer = customerSyncer; - action.registrar = - Registrar.loadByClientId("TheRegistrar").asBuilder() + Registrar registrar = persistResource( + Registrar.loadByClientId("TheRegistrar") + .asBuilder() .setBillingMethod(Registrar.BillingMethod.BRAINTREE) - .build(); + .build()); + when(sessionUtils.getRegistrarForAuthResult( + any(HttpServletRequest.class), any(AuthResult.class))) + .thenReturn(registrar); when(braintreeGateway.clientToken()).thenReturn(clientTokenGateway); } @@ -78,7 +93,7 @@ public class RegistrarPaymentSetupActionTest { "token", blanketsOfSadness, "currencies", asList("USD", "JPY"), "brainframe", "/doodle"))); - verify(customerSyncer).sync(eq(action.registrar)); + verify(customerSyncer).sync(eq(Registrar.loadByClientId("TheRegistrar"))); } @Test @@ -92,10 +107,14 @@ public class RegistrarPaymentSetupActionTest { @Test public void testNotOnCreditCardBillingTerms_showsErrorPage() throws Exception { - action.registrar = - Registrar.loadByClientId("TheRegistrar").asBuilder() + Registrar registrar = persistResource( + Registrar.loadByClientId("TheRegistrar") + .asBuilder() .setBillingMethod(Registrar.BillingMethod.EXTERNAL) - .build(); + .build()); + when(sessionUtils.getRegistrarForAuthResult( + any(HttpServletRequest.class), any(AuthResult.class))) + .thenReturn(registrar); assertThat(action.handleJsonRequest(ImmutableMap.of())) .containsExactly( "status", "ERROR", diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java index b54fe1a65..95f7b0d99 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java @@ -19,7 +19,9 @@ import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued; import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; import static google.registry.util.ResourceUtils.readResourceUtf8; import static java.util.Arrays.asList; -import static org.mockito.Mockito.anyInt; +import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -27,9 +29,12 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableMap; import google.registry.export.sheet.SyncRegistrarsSheetAction; import google.registry.model.registrar.Registrar; +import google.registry.request.HttpException.ForbiddenException; +import google.registry.request.auth.AuthResult; import google.registry.testing.TaskQueueHelper.TaskMatcher; import java.util.Map; import javax.mail.internet.InternetAddress; +import javax.servlet.http.HttpServletRequest; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -65,11 +70,15 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase @Test public void testRead_notAuthorized_failure() throws Exception { - when(sessionUtils.checkRegistrarConsoleLogin(req)).thenReturn(false); - Map response = action.handleJsonRequest(ImmutableMap.of()); - assertThat(response).containsEntry("status", "ERROR"); - assertThat((String) response.get("message")).startsWith("Not authorized"); - assertNoTasksEnqueued("sheet"); + when(sessionUtils.getRegistrarForAuthResult( + any(HttpServletRequest.class), any(AuthResult.class))) + .thenThrow(new ForbiddenException("Not authorized to access Registrar Console")); + try { + action.handleJsonRequest(ImmutableMap.of()); + fail("expected ForbiddenException"); + } catch (ForbiddenException ex) { + assertNoTasksEnqueued("sheet"); + } } /** diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index add01adc9..d7b7771dd 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -23,6 +23,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.google.appengine.api.modules.ModulesService; +import com.google.appengine.api.users.User; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -32,6 +33,9 @@ import google.registry.model.registrar.Registrar; import google.registry.request.JsonActionRunner; import google.registry.request.JsonResponse; import google.registry.request.ResponseImpl; +import google.registry.request.auth.AuthLevel; +import google.registry.request.auth.AuthResult; +import google.registry.request.auth.UserAuthInfo; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.InjectRule; @@ -73,6 +77,7 @@ public class RegistrarSettingsActionTestCase { final SendEmailService emailService = mock(SendEmailService.class); final ModulesService modulesService = mock(ModulesService.class); final SessionUtils sessionUtils = mock(SessionUtils.class); + final User user = new User("user", "gmail.com"); Message message; @@ -85,7 +90,7 @@ public class RegistrarSettingsActionTestCase { public void setUp() throws Exception { action.request = req; action.sessionUtils = sessionUtils; - action.initialRegistrar = Registrar.loadByClientId(CLIENT_ID); + action.authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false)); action.jsonActionRunner = new JsonActionRunner( ImmutableMap.of(), new JsonResponse(new ResponseImpl(rsp))); action.registrarChangesNotificationEmailAddresses = ImmutableList.of( @@ -101,9 +106,8 @@ public class RegistrarSettingsActionTestCase { when(rsp.getWriter()).thenReturn(new PrintWriter(writer)); when(req.getContentType()).thenReturn("application/json"); when(req.getReader()).thenReturn(createJsonPayload(ImmutableMap.of("op", "read"))); - when(sessionUtils.isLoggedIn()).thenReturn(true); - when(sessionUtils.checkRegistrarConsoleLogin(req)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(req)).thenReturn(CLIENT_ID); + when(sessionUtils.getRegistrarForAuthResult(req, action.authResult)) + .thenReturn(Registrar.loadByClientId(CLIENT_ID)); when(modulesService.getVersionHostname("backend", null)).thenReturn("backend.hostname"); } diff --git a/javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java b/javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java index aad2ee4cc..50c872140 100644 --- a/javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java +++ b/javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java @@ -21,8 +21,10 @@ import static google.registry.testing.CertificateSamples.SAMPLE_CERT; import static google.registry.testing.CertificateSamples.SAMPLE_CERT2; import static google.registry.testing.CertificateSamples.SAMPLE_CERT2_HASH; import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH; +import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static java.util.Arrays.asList; +import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableMap; import google.registry.model.registrar.Registrar; @@ -100,12 +102,14 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase { @Test public void testEmptyOrNullCertificate_doesNotClearOutCurrentOne() throws Exception { - action.initialRegistrar = + Registrar initialRegistrar = persistResource( Registrar.loadByClientId(CLIENT_ID).asBuilder() .setClientCertificate(SAMPLE_CERT, START_OF_TIME) .setFailoverClientCertificate(SAMPLE_CERT2, START_OF_TIME) - .build(); - Map jsonMap = action.initialRegistrar.toJsonMap(); + .build()); + when(sessionUtils.getRegistrarForAuthResult(req, action.authResult)) + .thenReturn(initialRegistrar); + Map jsonMap = initialRegistrar.toJsonMap(); jsonMap.put("clientCertificate", null); jsonMap.put("failoverClientCertificate", ""); Map response = action.handleJsonRequest(ImmutableMap.of( diff --git a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java b/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java index f160f3cab..142136967 100644 --- a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java +++ b/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java @@ -24,7 +24,6 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import com.google.appengine.api.users.User; -import com.google.appengine.api.users.UserService; import com.google.common.testing.NullPointerTester; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; @@ -55,7 +54,6 @@ public class SessionUtilsTest { @Rule public final InjectRule inject = new InjectRule(); - private final UserService userService = mock(UserService.class); private final HttpServletRequest req = mock(HttpServletRequest.class); private final HttpServletResponse rsp = mock(HttpServletResponse.class); private final HttpSession session = mock(HttpSession.class); @@ -66,22 +64,20 @@ public class SessionUtilsTest { @Before public void before() throws Exception { - sessionUtils = new SessionUtils(userService); + sessionUtils = new SessionUtils(); when(req.getSession()).thenReturn(session); } @Test public void testCheckRegistrarConsoleLogin_authedButNoSession_createsSession() throws Exception { - when(userService.getCurrentUser()).thenReturn(jart); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req)).isTrue(); + assertThat(sessionUtils.checkRegistrarConsoleLogin(req, jart)).isTrue(); verify(session).setAttribute(eq("clientId"), eq("TheRegistrar")); } @Test public void testCheckRegistrarConsoleLogin_authedWithValidSession_doesNothing() throws Exception { when(session.getAttribute("clientId")).thenReturn("TheRegistrar"); - when(userService.getCurrentUser()).thenReturn(jart); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req)).isTrue(); + assertThat(sessionUtils.checkRegistrarConsoleLogin(req, jart)).isTrue(); verify(session).getAttribute("clientId"); verifyNoMoreInteractions(session); } @@ -92,16 +88,14 @@ public class SessionUtilsTest { Registrar.loadByClientId("TheRegistrar"), new java.util.HashSet()); when(session.getAttribute("clientId")).thenReturn("TheRegistrar"); - when(userService.getCurrentUser()).thenReturn(jart); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req)).isFalse(); + assertThat(sessionUtils.checkRegistrarConsoleLogin(req, jart)).isFalse(); verify(session).invalidate(); } @Test public void testCheckRegistrarConsoleLogin_orphanedContactIsDenied() throws Exception { deleteResource(Registrar.loadByClientId("TheRegistrar")); - when(userService.getCurrentUser()).thenReturn(jart); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req)).isFalse(); + assertThat(sessionUtils.checkRegistrarConsoleLogin(req, jart)).isFalse(); } @Test @@ -109,13 +103,12 @@ public class SessionUtilsTest { throws Exception { thrown.expect(IllegalStateException.class); @SuppressWarnings("unused") - boolean unused = sessionUtils.checkRegistrarConsoleLogin(req); + boolean unused = sessionUtils.checkRegistrarConsoleLogin(req, null); } @Test public void testCheckRegistrarConsoleLogin_notAllowed_returnsFalse() throws Exception { - when(userService.getCurrentUser()).thenReturn(bozo); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req)).isFalse(); + assertThat(sessionUtils.checkRegistrarConsoleLogin(req, bozo)).isFalse(); } @Test