diff --git a/java/google/registry/rdap/RdapActionBase.java b/java/google/registry/rdap/RdapActionBase.java index bfbc8e984..c08a71f78 100644 --- a/java/google/registry/rdap/RdapActionBase.java +++ b/java/google/registry/rdap/RdapActionBase.java @@ -16,7 +16,6 @@ package google.registry.rdap; import static com.google.common.base.Charsets.UTF_8; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkState; import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.util.DateTimeUtils.END_OF_TIME; @@ -57,7 +56,6 @@ import java.util.List; import java.util.Optional; import javax.annotation.Nullable; import javax.inject.Inject; -import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; import org.json.simple.JSONValue; @@ -87,7 +85,6 @@ public abstract class RdapActionBase implements Runnable { INCLUDE } - @Inject HttpServletRequest request; @Inject Response response; @Inject @RequestMethod Action.Method requestMethod; @Inject @RequestPath String requestPath; @@ -176,6 +173,23 @@ public abstract class RdapActionBase implements Runnable { } } + /** + * Returns a clientId the given user has console access on, or Optional.empty if there is none. + */ + private Optional getAuthorizedClientId(AuthResult authResult) { + try { + String clientId = sessionUtils.guessClientIdForUser(authResult); + // We load the Registrar to make sure the user has access to it. We don't actually need it, + // we're just checking if an exception is thrown. + sessionUtils.getRegistrarForUserCached(clientId, authResult); + return Optional.of(clientId); + } catch (Exception e) { + logger.atWarning().withCause(e).log( + "Couldn't find registrar for User %s.", authResult.userIdForLogging()); + return Optional.empty(); + } + } + void setPayload(ImmutableMap rdapJson) { if (requestMethod == Action.Method.HEAD) { return; @@ -200,15 +214,11 @@ public abstract class RdapActionBase implements Runnable { if (userAuthInfo.isUserAdmin()) { return RdapAuthorization.ADMINISTRATOR_AUTHORIZATION; } - if (!sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)) { + Optional clientId = getAuthorizedClientId(authResult); + if (!clientId.isPresent()) { return RdapAuthorization.PUBLIC_AUTHORIZATION; } - String clientId = sessionUtils.getRegistrarClientId(request); - Optional registrar = Registrar.loadByClientIdCached(clientId); - if (!registrar.isPresent()) { - return RdapAuthorization.PUBLIC_AUTHORIZATION; - } - return RdapAuthorization.create(RdapAuthorization.Role.REGISTRAR, clientId); + return RdapAuthorization.create(RdapAuthorization.Role.REGISTRAR, clientId.get()); } /** Returns the registrar on which results should be filtered, or absent(). */ @@ -238,14 +248,9 @@ public abstract class RdapActionBase implements Runnable { if (userAuthInfo.isUserAdmin()) { return true; } - if (!sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)) { + if (!getAuthorizedClientId(authResult).isPresent()) { return false; } - String clientId = sessionUtils.getRegistrarClientId(request); - checkState( - Registrar.loadByClientIdCached(clientId).isPresent(), - "Registrar with clientId %s doesn't exist", - clientId); return true; } diff --git a/java/google/registry/request/auth/AuthResult.java b/java/google/registry/request/auth/AuthResult.java index d64b23c34..0903f26a0 100644 --- a/java/google/registry/request/auth/AuthResult.java +++ b/java/google/registry/request/auth/AuthResult.java @@ -36,6 +36,12 @@ public abstract class AuthResult { return authLevel() != AuthLevel.NONE; } + public String userIdForLogging() { + return userAuthInfo() + .map(userAuthInfo -> userAuthInfo.user().getUserId()) + .orElse(""); + } + public static AuthResult create(AuthLevel authLevel) { return new AutoValue_AuthResult(authLevel, Optional.empty()); } diff --git a/java/google/registry/ui/server/registrar/ConsoleUiAction.java b/java/google/registry/ui/server/registrar/ConsoleUiAction.java index e21a33131..11dbf510c 100644 --- a/java/google/registry/ui/server/registrar/ConsoleUiAction.java +++ b/java/google/registry/ui/server/registrar/ConsoleUiAction.java @@ -16,14 +16,15 @@ 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 google.registry.util.PreconditionsUtils.checkArgumentPresent; 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; +import com.google.common.flogger.FluentLogger; import com.google.common.io.Resources; import com.google.common.net.MediaType; import com.google.template.soy.data.SoyMapData; @@ -32,10 +33,10 @@ import com.google.template.soy.tofu.SoyTofu; import google.registry.config.RegistryConfig.Config; import google.registry.model.registrar.Registrar; import google.registry.request.Action; +import google.registry.request.HttpException.ForbiddenException; import google.registry.request.Response; import google.registry.request.auth.Auth; import google.registry.request.auth.AuthResult; -import google.registry.request.auth.UserAuthInfo; import google.registry.security.XsrfTokenManager; import google.registry.ui.server.SoyTemplateUtils; import google.registry.ui.soy.registrar.ConsoleSoyInfo; @@ -49,6 +50,8 @@ import javax.servlet.http.HttpServletRequest; ) public final class ConsoleUiAction implements Runnable { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + public static final String PATH = "/registrar"; private static final Supplier TOFU_SUPPLIER = @@ -97,7 +100,7 @@ public final class ConsoleUiAction implements Runnable { response.setHeader(LOCATION, location); return; } - UserAuthInfo userAuthInfo = authResult.userAuthInfo().get(); + 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. @@ -119,9 +122,24 @@ public final class ConsoleUiAction implements Runnable { .render()); return; } - data.put("username", userAuthInfo.user().getNickname()); + data.put("username", user.getNickname()); data.put("logoutUrl", userService.createLogoutURL(PATH)); - if (!sessionUtils.checkRegistrarConsoleLogin(req, userAuthInfo)) { + data.put("xsrfToken", xsrfTokenManager.generateToken(user.getEmail())); + try { + String clientId = sessionUtils.guessClientIdForUser(authResult); + data.put("clientId", clientId); + // We want to load the registrar even if we won't use it later (even if we remove the + // requireFeeExtension) - to make sure the user indeed has access to the guessed registrar. + // + // Note that not doing so (and just passing the "clientId" as given) isn't a security issue + // since we double check the access to the registrar on any read / update request. We have to + // - since the access might get revoked between the initial page load and the request! (also + // because the requests come from the browser, and can easily be faked) + Registrar registrar = sessionUtils.getRegistrarForUser(clientId, authResult); + data.put("requireFeeExtension", registrar.getPremiumPriceAckRequired()); + } catch (ForbiddenException e) { + logger.atWarning().withCause(e).log( + "User %s doesn't have access to registrar console.", authResult.userIdForLogging()); response.setStatus(SC_FORBIDDEN); response.setPayload( TOFU_SUPPLIER.get() @@ -131,13 +149,6 @@ public final class ConsoleUiAction implements Runnable { .render()); return; } - String clientId = sessionUtils.getRegistrarClientId(req); - Registrar registrar = - checkArgumentPresent( - Registrar.loadByClientIdCached(clientId), "Registrar %s does not exist", clientId); - data.put("xsrfToken", xsrfTokenManager.generateToken(userAuthInfo.user().getEmail())); - data.put("clientId", clientId); - data.put("requireFeeExtension", registrar.getPremiumPriceAckRequired()); String payload = TOFU_SUPPLIER.get() .newRenderer(ConsoleSoyInfo.MAIN) diff --git a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index a3fd9cb5e..74f890d4a 100644 --- a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -54,7 +54,6 @@ import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import javax.inject.Inject; -import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; /** @@ -76,7 +75,6 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA static final String ARGS_PARAM = "args"; static final String ID_PARAM = "id"; - @Inject HttpServletRequest request; @Inject JsonActionRunner jsonActionRunner; @Inject AppEngineServiceUtils appEngineServiceUtils; @Inject AuthResult authResult; @@ -100,60 +98,51 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA throw new BadRequestException("Malformed JSON"); } - Registrar initialRegistrar = sessionUtils.getRegistrarForAuthResult(request, authResult); - // Check that the clientId requested is the same as the one we get in the - // getRegistrarForAuthResult. - // TODO(b/113925293): remove this check, and instead use the requested clientId to select the - // registrar (in a secure way making sure authResult has access to that registrar!) String clientId = (String) input.get(ID_PARAM); if (Strings.isNullOrEmpty(clientId)) { throw new BadRequestException(String.format("Missing key for resource ID: %s", ID_PARAM)); } - if (!clientId.equals(initialRegistrar.getClientId())) { - throw new BadRequestException( - String.format( - "User's clientId changed from %s to %s. Please reload page", - clientId, initialRegistrar.getClientId())); - } + // Process the operation. Though originally derived from a CRUD // handler, registrar-settings really only supports read and update. String op = Optional.ofNullable((String) input.get(OP_PARAM)).orElse("read"); @SuppressWarnings("unchecked") Map args = (Map) Optional.ofNullable(input.get(ARGS_PARAM)).orElse(ImmutableMap.of()); - logger.atInfo().log( - "Received request '%s' on registrar '%s' with args %s", - op, initialRegistrar.getClientId(), args); + logger.atInfo().log("Received request '%s' on registrar '%s' with args %s", op, clientId, args); try { switch (op) { case "update": - return update(args, initialRegistrar.getClientId()); + return update(args, clientId); case "read": - return JsonResponseHelper.create(SUCCESS, "Success", initialRegistrar.toJsonMap()); + return read(clientId); default: return JsonResponseHelper.create(ERROR, "Unknown or unsupported operation: " + op); } } catch (FormFieldException e) { logger.atWarning().withCause(e).log( - "Failed to perform operation '%s' on registrar '%s' for args %s", - op, initialRegistrar.getClientId(), args); + "Failed to perform operation '%s' on registrar '%s' for args %s", op, clientId, args); return JsonResponseHelper.createFormFieldError(e.getMessage(), e.getFieldName()); } catch (FormException e) { logger.atWarning().withCause(e).log( - "Failed to perform operation '%s' on registrar '%s' for args %s", - op, initialRegistrar.getClientId(), args); + "Failed to perform operation '%s' on registrar '%s' for args %s", op, clientId, args); return JsonResponseHelper.create(ERROR, e.getMessage()); } } + Map read(String clientId) { + return JsonResponseHelper.create( + SUCCESS, "Success", sessionUtils.getRegistrarForUser(clientId, authResult).toJsonMap()); + } + Map update(final Map args, String clientId) { return ofy() .transact( () -> { - // We load the registrar here rather than use the initialRegistrar above - to make + // We load the registrar here rather than outside of the transaction - to make // sure we have the latest version. This one is loaded inside the transaction, so it's // guaranteed to not change before we update it. - Registrar registrar = Registrar.loadByClientId(clientId).get(); + Registrar registrar = sessionUtils.getRegistrarForUser(clientId, authResult); // Verify that the registrar hasn't been changed. // To do that - we find the latest update time (or null if the registrar has been // deleted) and compare to the update time from the args. The update time in the args diff --git a/java/google/registry/ui/server/registrar/SessionUtils.java b/java/google/registry/ui/server/registrar/SessionUtils.java index 38eeecc5d..91a375e03 100644 --- a/java/google/registry/ui/server/registrar/SessionUtils.java +++ b/java/google/registry/ui/server/registrar/SessionUtils.java @@ -14,15 +14,11 @@ package google.registry.ui.server.registrar; -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 static google.registry.util.PreconditionsUtils.checkArgumentPresent; import com.google.appengine.api.users.User; import com.google.common.base.Strings; import com.google.common.flogger.FluentLogger; -import com.googlecode.objectify.Key; import google.registry.config.RegistryConfig.Config; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; @@ -30,20 +26,16 @@ import google.registry.request.HttpException.ForbiddenException; import google.registry.request.auth.AuthResult; import google.registry.request.auth.UserAuthInfo; import java.util.Optional; -import javax.annotation.CheckReturnValue; +import java.util.function.Function; import javax.annotation.concurrent.Immutable; import javax.inject.Inject; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpSession; -/** HTTP session management helper class. */ +/** Authenticated Registrar access helper class. */ @Immutable public class SessionUtils { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private static final String CLIENT_ID_ATTRIBUTE = "clientId"; - @Inject @Config("registryAdminClientId") String registryAdminClientId; @@ -52,145 +44,118 @@ public class SessionUtils { public SessionUtils() {} /** - * Checks that the authentication result indicates a user that has access to the registrar - * console, then gets the associated registrar. + * Loads Registrar on behalf of an authorised user. * - *

Throws a {@link ForbiddenException} if the user is not logged in, or not authorized to use - * the registrar console. + *

Throws a {@link ForbiddenException} if the user is not logged in, or not authorized to + * access the requested registrar. + * + * @param clientId ID of the registrar we request + * @param authResult AuthResult of the user on behalf of which we want to access the data */ - @CheckReturnValue - Registrar getRegistrarForAuthResult(HttpServletRequest request, AuthResult authResult) { - if (!authResult.userAuthInfo().isPresent()) { - throw new ForbiddenException("Not logged in"); - } - if (!checkRegistrarConsoleLogin(request, authResult.userAuthInfo().get())) { - throw new ForbiddenException("Not authorized to access Registrar Console"); - } - String clientId = getRegistrarClientId(request); - return checkArgumentPresent( - Registrar.loadByClientId(clientId), - "Registrar %s not found", - clientId); + public Registrar getRegistrarForUser(String clientId, AuthResult authResult) { + return getAndAuthorize(Registrar::loadByClientId, clientId, authResult); } /** - * Checks that the specified user has access to the Registrar Console. + * Loads a Registrar from the cache on behalf of an authorised user. * - *

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

Throws a {@link ForbiddenException} if the user is not logged in, or not authorized to + * access the requested registrar. * - *

    - *
  • If it does not exist, then we will attempt to guess the {@link Registrar} with which the - * 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. - * - * @return {@code false} if user does not have access, in which case the caller should write an - * error response and abort the request. + * @param clientId ID of the registrar we request + * @param authResult AuthResult of the user on behalf of which we want to access the data */ - @CheckReturnValue - public boolean checkRegistrarConsoleLogin(HttpServletRequest req, UserAuthInfo userAuthInfo) { - checkState(userAuthInfo != null, "No logged in user found"); + public Registrar getRegistrarForUserCached(String clientId, AuthResult authResult) { + return getAndAuthorize(Registrar::loadByClientIdCached, clientId, authResult); + } + + Registrar getAndAuthorize( + Function> registrarLoader, + String clientId, + AuthResult authResult) { + UserAuthInfo userAuthInfo = + authResult.userAuthInfo().orElseThrow(() -> new ForbiddenException("Not logged in")); + boolean isAdmin = userAuthInfo.isUserAdmin(); User user = userAuthInfo.user(); - HttpSession session = req.getSession(); - String clientId = (String) session.getAttribute(CLIENT_ID_ATTRIBUTE); + String gaeUserId = user.getUserId(); - // Use the clientId if it exists - if (clientId != null) { - if (!hasAccessToRegistrar(clientId, user.getUserId(), userAuthInfo.isUserAdmin())) { - logger.atInfo().log("Registrar Console access revoked: %s", clientId); - session.invalidate(); - return false; - } - logger.atInfo().log( - "Associating user %s with given registrar %s.", user.getUserId(), clientId); - return true; + Registrar registrar = + registrarLoader + .apply(clientId) + .orElseThrow( + () -> new ForbiddenException(String.format("Registrar %s not found", clientId))); + + if (isInAllowedContacts(registrar, gaeUserId)) { + logger.atInfo().log("User %s has access to registrar %s.", gaeUserId, clientId); + return registrar; } - // The clientId was null, so let's try and find a registrar this user is associated with - Optional registrar = findRegistrarForUser(user.getUserId()); - if (registrar.isPresent()) { - verify(isInAllowedContacts(registrar.get(), user.getUserId())); - logger.atInfo().log( - "Associating user %s with found registrar %s.", - user.getUserId(), registrar.get().getClientId()); - session.setAttribute(CLIENT_ID_ATTRIBUTE, registrar.get().getClientId()); - return true; + if (isAdmin && clientId.equals(registryAdminClientId)) { + // Admins have access to the registryAdminClientId even if they aren't explicitly in the + // allowed contacts + logger.atInfo().log("Allowing admin %s access to registrar %s.", gaeUserId, clientId); + return registrar; } - // We couldn't guess the registrar, but maybe the user is an admin and we can use the + throw new ForbiddenException( + String.format("User %s doesn't have access to registrar %s", gaeUserId, clientId)); + } + + /** + * Tries to guess the {@link Registrar} with which the user is associated. + * + *

Returns the {@code clientId} of a {@link Registrar} the user has access to (is on the + * contact list). If the user has access to multiple {@link Registrar}s, an arbitrary one is + * selected. If the user is an admin without access to any {@link Registrar}s, {@link + * #registryAdminClientId} is returned if it is defined. + * + *

If no {@code clientId} is found, throws a {@link ForbiddenException}. + * + *

If you want to load the {@link Registrar} object from this (or any other) {@code clientId}, + * in order to perform actions on behalf of a user, you must use {@link #getRegistrarForUser} + * which makes sure the user has permissions. + * + *

Note that this is an OPTIONAL step in the authentication - only used if we don't have any + * other clue as to the requested {@code clientId}. It is perfectly OK to get a {@code clientId} + * from any other source, as long as the registrar is then loaded using {@link + * #getRegistrarForUser}. + */ + public String guessClientIdForUser(AuthResult authResult) { + + UserAuthInfo userAuthInfo = + authResult.userAuthInfo().orElseThrow(() -> new ForbiddenException("No logged in")); + boolean isAdmin = userAuthInfo.isUserAdmin(); + User user = userAuthInfo.user(); + String gaeUserId = user.getUserId(); + + RegistrarContact contact = + ofy().load().type(RegistrarContact.class).filter("gaeUserId", gaeUserId).first().now(); + if (contact != null) { + String registrarClientId = contact.getParent().getName(); + logger.atInfo().log( + "Associating user %s with found registrar %s.", gaeUserId, registrarClientId); + return registrarClientId; + } + + // We couldn't find the registrar, but maybe the user is an admin and we can use the // registryAdminClientId - if (userAuthInfo.isUserAdmin()) { - if (Strings.isNullOrEmpty(registryAdminClientId)) { + if (isAdmin) { + if (!Strings.isNullOrEmpty(registryAdminClientId)) { logger.atInfo().log( - "Cannot associate admin user %s with configured client Id." - + " ClientId is null or empty.", - user.getUserId()); - return false; - } - if (!Registrar.loadByClientIdCached(registryAdminClientId).isPresent()) { - logger.atInfo().log( - "Cannot associate admin user %s with configured client Id %s." - + " Registrar does not exist.", - user.getUserId(), registryAdminClientId); - return false; + "User %s is an admin with no associated registrar." + + " Automatically associating the user with configured client Id %s.", + gaeUserId, registryAdminClientId); + return registryAdminClientId; } logger.atInfo().log( - "User %s is an admin with no associated registrar." - + " Automatically associating the user with configured client Id %s.", - user.getUserId(), registryAdminClientId); - session.setAttribute(CLIENT_ID_ATTRIBUTE, registryAdminClientId); - return true; + "Cannot associate admin user %s with configured client Id." + + " ClientId is null or empty.", + gaeUserId); } // We couldn't find any relevant clientId - logger.atInfo().log("User not associated with any Registrar: %s", user.getUserId()); - return false; - } - - /** - * Returns {@link Registrar} clientId associated with HTTP session. - * - * @throws IllegalStateException if you forgot to call {@link #checkRegistrarConsoleLogin}. - */ - @CheckReturnValue - public String getRegistrarClientId(HttpServletRequest req) { - String clientId = (String) req.getSession().getAttribute(CLIENT_ID_ATTRIBUTE); - checkState(clientId != null, "You forgot to call checkRegistrarConsoleLogin()"); - return clientId; - } - - /** Returns first {@link Registrar} that {@code gaeUserId} is authorized to administer. */ - private static Optional findRegistrarForUser(String gaeUserId) { - RegistrarContact contact = - ofy().load().type(RegistrarContact.class).filter("gaeUserId", gaeUserId).first().now(); - if (contact == null) { - return Optional.empty(); - } - String registrarClientId = contact.getParent().getName(); - Optional result = Registrar.loadByClientIdCached(registrarClientId); - if (!result.isPresent()) { - logger.atSevere().log( - "A contact record exists for non-existent registrar: %s.", Key.create(contact)); - } - return result; - } - - /** @see #isInAllowedContacts(Registrar, String) */ - boolean hasAccessToRegistrar(String clientId, String gaeUserId, boolean isAdmin) { - Optional registrar = Registrar.loadByClientIdCached(clientId); - if (!registrar.isPresent()) { - logger.atWarning().log("Registrar '%s' disappeared from Datastore!", clientId); - return false; - } - if (isAdmin && clientId.equals(registryAdminClientId)) { - return true; - } - return isInAllowedContacts(registrar.get(), gaeUserId); + throw new ForbiddenException( + String.format("User %s isn't associated with any registrar", gaeUserId)); } /** diff --git a/javatests/google/registry/rdap/RdapDomainActionTest.java b/javatests/google/registry/rdap/RdapDomainActionTest.java index af19848e9..00f8f784c 100644 --- a/javatests/google/registry/rdap/RdapDomainActionTest.java +++ b/javatests/google/registry/rdap/RdapDomainActionTest.java @@ -58,7 +58,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import javax.annotation.Nullable; -import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; import org.json.simple.JSONObject; import org.json.simple.JSONValue; @@ -81,17 +80,23 @@ public class RdapDomainActionTest { @Rule public final InjectRule inject = new InjectRule(); - private final HttpServletRequest request = mock(HttpServletRequest.class); private final FakeResponse response = new FakeResponse(); private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01TZ")); private final SessionUtils sessionUtils = mock(SessionUtils.class); - private final User user = new User("rdap.user@example.com", "gmail.com", "12345"); - private final UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); - private final UserAuthInfo adminUserAuthInfo = UserAuthInfo.create(user, true); private final RdapMetrics rdapMetrics = mock(RdapMetrics.class); private RdapDomainAction action; + private static final AuthResult AUTH_RESULT = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@user.com", "gmail.com", "12345"), false)); + + private static final AuthResult AUTH_RESULT_ADMIN = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@google.com", "gmail.com", "12345"), true)); + @Before public void setUp() { inject.setStaticField(Ofy.class, "clock", clock); @@ -262,7 +267,6 @@ public class RdapDomainActionTest { action = new RdapDomainAction(); action.clock = clock; - action.request = request; action.requestMethod = Action.Method.GET; action.fullServletPath = "https://example.com/rdap"; action.response = response; @@ -272,19 +276,17 @@ public class RdapDomainActionTest { action.rdapJsonFormatter = RdapTestHelper.getTestRdapJsonFormatter(); action.rdapWhoisServer = null; action.sessionUtils = sessionUtils; - action.authResult = AuthResult.create(AuthLevel.USER, userAuthInfo); + action.authResult = AUTH_RESULT; action.rdapMetrics = rdapMetrics; } private void login(String clientId) { - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn(clientId); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn(clientId); } private void loginAsAdmin() { - when(sessionUtils.checkRegistrarConsoleLogin(request, adminUserAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("irrelevant"); - action.authResult = AuthResult.create(AuthLevel.USER, adminUserAuthInfo); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT_ADMIN)).thenReturn("irrelevant"); + action.authResult = AUTH_RESULT_ADMIN; } private Object generateActualJson(String domainName) { diff --git a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java index b2bb88b84..2684e72ce 100644 --- a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java +++ b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java @@ -71,7 +71,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import javax.annotation.Nullable; -import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; import org.json.simple.JSONArray; import org.json.simple.JSONObject; @@ -91,14 +90,21 @@ public class RdapDomainSearchActionTest extends RdapSearchActionTestCase { @Rule public final InjectRule inject = new InjectRule(); - private final HttpServletRequest request = mock(HttpServletRequest.class); private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01T00:00:00Z")); private final SessionUtils sessionUtils = mock(SessionUtils.class); - private final User user = new User("rdap.user@example.com", "gmail.com", "12345"); - private final UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); - private final UserAuthInfo adminUserAuthInfo = UserAuthInfo.create(user, true); private final RdapDomainSearchAction action = new RdapDomainSearchAction(); + private static final AuthResult AUTH_RESULT = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@user.com", "gmail.com", "12345"), false)); + + private static final AuthResult AUTH_RESULT_ADMIN = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@google.com", "gmail.com", "12345"), true)); + + private FakeResponse response = new FakeResponse(); private Registrar registrar; @@ -388,7 +394,6 @@ public class RdapDomainSearchActionTest extends RdapSearchActionTestCase { clock.nowUtc())); action.clock = clock; - action.request = request; action.requestMethod = Action.Method.GET; action.fullServletPath = "https://example.com/rdap"; action.requestUrl = "https://example.com/rdap/domains"; @@ -401,22 +406,20 @@ public class RdapDomainSearchActionTest extends RdapSearchActionTestCase { action.rdapJsonFormatter = RdapTestHelper.getTestRdapJsonFormatter(); action.rdapWhoisServer = null; action.sessionUtils = sessionUtils; - action.authResult = AuthResult.create(AuthLevel.USER, userAuthInfo); + action.authResult = AUTH_RESULT; action.rdapMetrics = rdapMetrics; action.cursorTokenParam = Optional.empty(); action.rdapResultSetMaxSize = 4; } private void login(String clientId) { - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn(clientId); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn(clientId); metricRole = REGISTRAR; } private void loginAsAdmin() { - when(sessionUtils.checkRegistrarConsoleLogin(request, adminUserAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("irrelevant"); - action.authResult = AuthResult.create(AuthLevel.USER, adminUserAuthInfo); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn("irrelevant"); + action.authResult = AUTH_RESULT_ADMIN; metricRole = ADMINISTRATOR; } diff --git a/javatests/google/registry/rdap/RdapEntityActionTest.java b/javatests/google/registry/rdap/RdapEntityActionTest.java index ae5ef267d..f79d216b7 100644 --- a/javatests/google/registry/rdap/RdapEntityActionTest.java +++ b/javatests/google/registry/rdap/RdapEntityActionTest.java @@ -53,7 +53,6 @@ import google.registry.ui.server.registrar.SessionUtils; import java.util.Map; import java.util.Optional; import javax.annotation.Nullable; -import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; import org.json.simple.JSONValue; import org.junit.Before; @@ -74,13 +73,9 @@ public class RdapEntityActionTest { @Rule public final InjectRule inject = new InjectRule(); - private final HttpServletRequest request = mock(HttpServletRequest.class); private final FakeResponse response = new FakeResponse(); private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01TZ")); private final SessionUtils sessionUtils = mock(SessionUtils.class); - private final User user = new User("rdap.user@example.com", "gmail.com", "12345"); - private final UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); - private final UserAuthInfo adminUserAuthInfo = UserAuthInfo.create(user, true); private final RdapMetrics rdapMetrics = mock(RdapMetrics.class); private RdapEntityAction action; @@ -92,6 +87,16 @@ public class RdapEntityActionTest { private ContactResource disconnectedContact; private ContactResource deletedContact; + private static final AuthResult AUTH_RESULT = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@user.com", "gmail.com", "12345"), false)); + + private static final AuthResult AUTH_RESULT_ADMIN = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@google.com", "gmail.com", "12345"), true)); + @Before public void setUp() { inject.setStaticField(Ofy.class, "clock", clock); @@ -163,7 +168,6 @@ public class RdapEntityActionTest { clock.nowUtc().minusMonths(6)); action = new RdapEntityAction(); action.clock = clock; - action.request = request; action.requestMethod = Action.Method.GET; action.fullServletPath = "https://example.com/rdap"; action.response = response; @@ -173,19 +177,17 @@ public class RdapEntityActionTest { action.rdapJsonFormatter = RdapTestHelper.getTestRdapJsonFormatter(); action.rdapWhoisServer = null; action.sessionUtils = sessionUtils; - action.authResult = AuthResult.create(AuthLevel.USER, userAuthInfo); + action.authResult = AUTH_RESULT; action.rdapMetrics = rdapMetrics; } private void login(String registrar) { - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn(registrar); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn(registrar); } private void loginAsAdmin() { - action.authResult = AuthResult.create(AuthLevel.USER, adminUserAuthInfo); - when(sessionUtils.checkRegistrarConsoleLogin(request, adminUserAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("irrelevant"); + action.authResult = AUTH_RESULT_ADMIN; + when(sessionUtils.guessClientIdForUser(AUTH_RESULT_ADMIN)).thenReturn("irrelevant"); } private Object generateActualJson(String name) { diff --git a/javatests/google/registry/rdap/RdapEntitySearchActionTest.java b/javatests/google/registry/rdap/RdapEntitySearchActionTest.java index b5c4a576e..42932e373 100644 --- a/javatests/google/registry/rdap/RdapEntitySearchActionTest.java +++ b/javatests/google/registry/rdap/RdapEntitySearchActionTest.java @@ -59,7 +59,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import javax.annotation.Nullable; -import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; import org.json.simple.JSONArray; import org.json.simple.JSONObject; @@ -82,12 +81,8 @@ public class RdapEntitySearchActionTest extends RdapSearchActionTestCase { HANDLE } - private final HttpServletRequest request = mock(HttpServletRequest.class); private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01T00:00:00Z")); private final SessionUtils sessionUtils = mock(SessionUtils.class); - private final User user = new User("rdap.user@example.com", "gmail.com", "12345"); - private final UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); - private final UserAuthInfo adminUserAuthInfo = UserAuthInfo.create(user, true); private final RdapEntitySearchAction action = new RdapEntitySearchAction(); private FakeResponse response = new FakeResponse(); @@ -132,6 +127,16 @@ public class RdapEntitySearchActionTest extends RdapSearchActionTestCase { return JSONValue.parse(response.getPayload()); } + private static final AuthResult AUTH_RESULT = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@user.com", "gmail.com", "12345"), false)); + + private static final AuthResult AUTH_RESULT_ADMIN = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@google.com", "gmail.com", "12345"), true)); + @Before public void setUp() { inject.setStaticField(Ofy.class, "clock", clock); @@ -182,7 +187,6 @@ public class RdapEntitySearchActionTest extends RdapSearchActionTestCase { clock.nowUtc().minusMonths(6)); action.clock = clock; - action.request = request; action.requestMethod = Action.Method.GET; action.fullServletPath = "https://example.com/rdap"; action.requestUrl = "https://example.com/rdap/entities"; @@ -199,21 +203,19 @@ public class RdapEntitySearchActionTest extends RdapSearchActionTestCase { action.includeDeletedParam = Optional.empty(); action.formatOutputParam = Optional.empty(); action.sessionUtils = sessionUtils; - action.authResult = AuthResult.create(AuthLevel.USER, userAuthInfo); + action.authResult = AUTH_RESULT; action.rdapMetrics = rdapMetrics; action.cursorTokenParam = Optional.empty(); } private void login(String registrar) { - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn(registrar); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn(registrar); metricRole = REGISTRAR; } private void loginAsAdmin() { - action.authResult = AuthResult.create(AuthLevel.USER, adminUserAuthInfo); - when(sessionUtils.checkRegistrarConsoleLogin(request, adminUserAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("noregistrar"); + action.authResult = AUTH_RESULT_ADMIN; + when(sessionUtils.guessClientIdForUser(AUTH_RESULT_ADMIN)).thenReturn("irrelevant"); metricRole = ADMINISTRATOR; } diff --git a/javatests/google/registry/rdap/RdapNameserverActionTest.java b/javatests/google/registry/rdap/RdapNameserverActionTest.java index 7a3196a0b..de8fc7b98 100644 --- a/javatests/google/registry/rdap/RdapNameserverActionTest.java +++ b/javatests/google/registry/rdap/RdapNameserverActionTest.java @@ -35,6 +35,7 @@ import google.registry.rdap.RdapMetrics.SearchType; import google.registry.rdap.RdapMetrics.WildcardType; import google.registry.rdap.RdapSearchResults.IncompletenessWarningType; import google.registry.request.Action; +import google.registry.request.HttpException.ForbiddenException; import google.registry.request.auth.AuthLevel; import google.registry.request.auth.AuthResult; import google.registry.request.auth.UserAuthInfo; @@ -46,7 +47,6 @@ import google.registry.ui.server.registrar.SessionUtils; import java.util.Map; import java.util.Optional; import javax.annotation.Nullable; -import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; import org.json.simple.JSONValue; import org.junit.Before; @@ -67,15 +67,21 @@ public class RdapNameserverActionTest { @Rule public final InjectRule inject = new InjectRule(); - private final HttpServletRequest request = mock(HttpServletRequest.class); private final FakeResponse response = new FakeResponse(); private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01TZ")); private final SessionUtils sessionUtils = mock(SessionUtils.class); - private final User user = new User("rdap.user@example.com", "gmail.com", "12345"); - private final UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); - private final UserAuthInfo adminUserAuthInfo = UserAuthInfo.create(user, true); private final RdapMetrics rdapMetrics = mock(RdapMetrics.class); + private static final AuthResult AUTH_RESULT = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@user.com", "gmail.com", "12345"), false)); + + private static final AuthResult AUTH_RESULT_ADMIN = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@google.com", "gmail.com", "12345"), true)); + @Before public void setUp() { inject.setStaticField(Ofy.class, "clock", clock); @@ -106,8 +112,7 @@ public class RdapNameserverActionTest { private RdapNameserverAction newRdapNameserverAction( String input, Optional desiredRegistrar, Optional includeDeleted) { - return newRdapNameserverAction( - input, desiredRegistrar, includeDeleted, AuthResult.create(AuthLevel.USER, userAuthInfo)); + return newRdapNameserverAction(input, desiredRegistrar, includeDeleted, AUTH_RESULT); } private RdapNameserverAction newRdapNameserverAction( @@ -117,7 +122,6 @@ public class RdapNameserverActionTest { AuthResult authResult) { RdapNameserverAction action = new RdapNameserverAction(); action.clock = clock; - action.request = request; action.requestMethod = Action.Method.GET; action.fullServletPath = "https://example.tld/rdap"; action.response = response; @@ -358,23 +362,21 @@ public class RdapNameserverActionTest { @Test public void testDeletedNameserver_notFound_notLoggedIn() { - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(false); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenThrow(new ForbiddenException("blah")); generateActualJson("nsdeleted.cat.lol", Optional.empty(), Optional.of(true)); assertThat(response.getStatus()).isEqualTo(404); } @Test public void testDeletedNameserver_notFound_loggedInAsDifferentRegistrar() { - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("otherregistrar"); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn("otherregistrar"); generateActualJson("nsdeleted.cat.lol", Optional.empty(), Optional.of(true)); assertThat(response.getStatus()).isEqualTo(404); } @Test public void testDeletedNameserver_found_loggedInAsCorrectRegistrar() { - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("TheRegistrar"); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn("TheRegistrar"); assertThat( generateActualJson("nsdeleted.cat.lol", Optional.empty(), Optional.of(true))) .isEqualTo( @@ -391,13 +393,12 @@ public class RdapNameserverActionTest { @Test public void testDeletedNameserver_found_loggedInAsAdmin() { - when(sessionUtils.checkRegistrarConsoleLogin(request, adminUserAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("irrelevant"); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT_ADMIN)).thenReturn("irrelevant"); newRdapNameserverAction( "nsdeleted.cat.lol", Optional.empty(), Optional.of(true), - AuthResult.create(AuthLevel.USER, adminUserAuthInfo)) + AUTH_RESULT_ADMIN) .run(); assertThat(JSONValue.parse(response.getPayload())) .isEqualTo( @@ -414,8 +415,7 @@ public class RdapNameserverActionTest { @Test public void testDeletedNameserver_found_sameRegistrarRequested() { - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("TheRegistrar"); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn("TheRegistrar"); assertThat( generateActualJson("nsdeleted.cat.lol", Optional.of("TheRegistrar"), Optional.of(true))) .isEqualTo( @@ -432,8 +432,7 @@ public class RdapNameserverActionTest { @Test public void testDeletedNameserver_notFound_differentRegistrarRequested() { - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("TheRegistrar"); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn("TheRegistrar"); generateActualJson("ns1.cat.lol", Optional.of("otherregistrar"), Optional.of(false)); assertThat(response.getStatus()).isEqualTo(404); } diff --git a/javatests/google/registry/rdap/RdapNameserverSearchActionTest.java b/javatests/google/registry/rdap/RdapNameserverSearchActionTest.java index 2b477e6bd..9499fc8f4 100644 --- a/javatests/google/registry/rdap/RdapNameserverSearchActionTest.java +++ b/javatests/google/registry/rdap/RdapNameserverSearchActionTest.java @@ -58,7 +58,6 @@ import google.registry.ui.server.registrar.SessionUtils; import java.net.URLDecoder; import java.util.Optional; import javax.annotation.Nullable; -import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; import org.json.simple.JSONArray; import org.json.simple.JSONObject; @@ -77,15 +76,22 @@ public class RdapNameserverSearchActionTest extends RdapSearchActionTestCase { @Rule public final InjectRule inject = new InjectRule(); - private final HttpServletRequest request = mock(HttpServletRequest.class); private FakeResponse response = new FakeResponse(); private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01T00:00:00Z")); private final SessionUtils sessionUtils = mock(SessionUtils.class); - private final User user = new User("rdap.user@example.com", "gmail.com", "12345"); - private final UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); - private final UserAuthInfo adminUserAuthInfo = UserAuthInfo.create(user, true); private final RdapNameserverSearchAction action = new RdapNameserverSearchAction(); + private static final AuthResult AUTH_RESULT = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@user.com", "gmail.com", "12345"), false)); + + private static final AuthResult AUTH_RESULT_ADMIN = + AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create(new User("rdap.user@google.com", "gmail.com", "12345"), true)); + + private DomainResource domainCatLol; private HostResource hostNs1CatLol; private HostResource hostNs2CatLol; @@ -183,7 +189,6 @@ public class RdapNameserverSearchActionTest extends RdapSearchActionTestCase { action.requestUrl = "https://example.tld/rdap/nameservers"; action.requestPath = RdapNameserverSearchAction.PATH; action.parameterMap = ImmutableListMultimap.of(); - action.request = request; action.requestMethod = Action.Method.GET; action.response = response; action.rdapJsonFormatter = RdapTestHelper.getTestRdapJsonFormatter(); @@ -194,22 +199,19 @@ public class RdapNameserverSearchActionTest extends RdapSearchActionTestCase { action.registrarParam = Optional.empty(); action.includeDeletedParam = Optional.empty(); action.formatOutputParam = Optional.empty(); - action.authResult = AuthResult.create(AuthLevel.USER, userAuthInfo); + action.authResult = AUTH_RESULT; action.sessionUtils = sessionUtils; action.rdapMetrics = rdapMetrics; action.cursorTokenParam = Optional.empty(); } - private void login(String clientId) { - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn(clientId); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn(clientId); metricRole = REGISTRAR; } private void loginAsAdmin() { - when(sessionUtils.checkRegistrarConsoleLogin(request, adminUserAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("irrelevant"); - action.authResult = AuthResult.create(AuthLevel.USER, adminUserAuthInfo); + when(sessionUtils.guessClientIdForUser(AUTH_RESULT_ADMIN)).thenReturn("irrelevant"); + action.authResult = AUTH_RESULT_ADMIN; metricRole = ADMINISTRATOR; } diff --git a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java index 9c7e39986..c259af5df 100644 --- a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java +++ b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java @@ -16,6 +16,7 @@ package google.registry.ui.server.registrar; import static com.google.common.net.HttpHeaders.LOCATION; import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.DatastoreHelper.loadRegistrar; import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; @@ -24,6 +25,7 @@ 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.HttpException.ForbiddenException; import google.registry.request.auth.AuthLevel; import google.registry.request.auth.AuthResult; import google.registry.request.auth.UserAuthInfo; @@ -70,10 +72,11 @@ public class ConsoleUiActionTest { action.sessionUtils = sessionUtils; action.userService = UserServiceFactory.getUserService(); action.xsrfTokenManager = new XsrfTokenManager(new FakeClock(), action.userService); - UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); - action.authResult = AuthResult.create(AuthLevel.USER, userAuthInfo); - when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); - when(sessionUtils.getRegistrarClientId(request)).thenReturn("TheRegistrar"); + AuthResult authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false)); + action.authResult = authResult; + when(sessionUtils.guessClientIdForUser(authResult)).thenReturn("TheRegistrar"); + when(sessionUtils.getRegistrarForUser("TheRegistrar", authResult)) + .thenReturn(loadRegistrar("TheRegistrar")); } @Test @@ -110,9 +113,8 @@ public class ConsoleUiActionTest { @Test public void testUserDoesntHaveAccessToAnyRegistrar_showsWhoAreYouPage() { - when(sessionUtils.checkRegistrarConsoleLogin( - any(HttpServletRequest.class), any(UserAuthInfo.class))) - .thenReturn(false); + when(sessionUtils.guessClientIdForUser(any(AuthResult.class))) + .thenThrow(new ForbiddenException("forbidden")); action.run(); assertThat(response.getPayload()).contains("

You need permission

"); } diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java index d84a386e9..68efd8c58 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java @@ -22,19 +22,15 @@ import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued; import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; import static google.registry.testing.TestDataHelper.loadFile; import static java.util.Arrays.asList; -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; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import google.registry.export.sheet.SyncRegistrarsSheetAction; import google.registry.model.registrar.Registrar; -import google.registry.request.HttpException.BadRequestException; import google.registry.request.HttpException.ForbiddenException; -import google.registry.request.auth.AuthResult; import google.registry.testing.CertificateSamples; import google.registry.testing.TaskQueueHelper.TaskMatcher; import google.registry.util.CidrAddressBlock; @@ -42,7 +38,6 @@ import java.util.Map; import java.util.function.BiFunction; import java.util.function.Function; import javax.mail.internet.InternetAddress; -import javax.servlet.http.HttpServletRequest; import org.json.simple.JSONValue; import org.json.simple.parser.ParseException; import org.junit.Test; @@ -82,39 +77,27 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase assertNoTasksEnqueued("sheet"); } + /** + * Make sure that if someone spoofs a different registrar (they don't have access to), we fail. + * Also relevant if the person's privilege were revoked after the page load. + */ @Test - public void testRead_notAuthorized_failure() { - when(sessionUtils.getRegistrarForAuthResult( - any(HttpServletRequest.class), any(AuthResult.class))) - .thenThrow(new ForbiddenException("Not authorized to access Registrar Console")); - assertThrows(ForbiddenException.class, () -> action.handleJsonRequest(ImmutableMap.of())); + public void testFailure_readRegistrarInfo_notAuthorized() { + action.authResult = USER_UNAUTHORIZED; + assertThrows( + ForbiddenException.class, () -> action.handleJsonRequest(ImmutableMap.of("id", CLIENT_ID))); assertNoTasksEnqueued("sheet"); } - /** - * This is the default read test for the registrar settings actions. - */ + /** This is the default read test for the registrar settings actions. */ @Test public void testSuccess_readRegistrarInfo_authorized() { Map response = action.handleJsonRequest(ImmutableMap.of("id", CLIENT_ID)); - assertThat(response).containsExactly( - "status", "SUCCESS", - "message", "Success", - "results", asList(loadRegistrar(CLIENT_ID).toJsonMap())); - } - - /** - * We got a different CLIENT_ID from the JS than the one we find ourself. - * - *

This might happen if the user's "guessed" registrar changes after the initial page load. For - * example, if the user was added as contact to a different registrar, or removed as contact from - * the current registrar (but is still a contact of a different one, so the "guessing" works). - */ - @Test - public void testFailure_readRegistrarInfo_differentClientId() { - assertThrows( - BadRequestException.class, - () -> action.handleJsonRequest(ImmutableMap.of("id", "different"))); + assertThat(response) + .containsExactly( + "status", "SUCCESS", + "message", "Success", + "results", asList(loadRegistrar(CLIENT_ID).toJsonMap())); } @Test @@ -159,6 +142,19 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase "results", asList(loadRegistrar(CLIENT_ID).toJsonMap())); } + @Test + public void testFailute_updateRegistrarInfo_notAuthorized() { + action.authResult = USER_UNAUTHORIZED; + assertThrows( + ForbiddenException.class, + () -> + action.handleJsonRequest( + ImmutableMap.of( + "op", "update", + "id", CLIENT_ID, + "args", ImmutableMap.of("lastUpdateTime", getLastUpdateTime())))); + } + @Test public void testUpdate_badEmail_errorEmailField() { Map response = action.handleJsonRequest(ImmutableMap.of( diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index 78118b608..7c4e4c8ca 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -24,6 +24,7 @@ import com.google.appengine.api.users.User; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import google.registry.model.ofy.Ofy; +import google.registry.request.HttpException.ForbiddenException; import google.registry.request.JsonActionRunner; import google.registry.request.JsonResponse; import google.registry.request.ResponseImpl; @@ -57,6 +58,13 @@ public class RegistrarSettingsActionTestCase { static final String CLIENT_ID = "TheRegistrar"; + static final AuthResult USER_AUTHORIZED = + AuthResult.create(AuthLevel.USER, UserAuthInfo.create(new User("user", "gmail.com"), false)); + + static final AuthResult USER_UNAUTHORIZED = + AuthResult.create( + AuthLevel.USER, UserAuthInfo.create(new User("unauthorized", "gmail.com"), false)); + @Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().withTaskQueue().build(); @@ -79,11 +87,10 @@ public class RegistrarSettingsActionTestCase { @Before public void setUp() throws Exception { - action.request = req; action.sessionUtils = sessionUtils; action.appEngineServiceUtils = appEngineServiceUtils; when(appEngineServiceUtils.getCurrentVersionHostname("backend")).thenReturn("backend.hostname"); - action.authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false)); + action.authResult = USER_AUTHORIZED; action.jsonActionRunner = new JsonActionRunner( ImmutableMap.of(), new JsonResponse(new ResponseImpl(rsp))); action.registrarChangesNotificationEmailAddresses = ImmutableList.of( @@ -102,7 +109,9 @@ public class RegistrarSettingsActionTestCase { // the result is out of date after mutations. // (for example, if someone wants to change the registrar to prepare for a test, the function // would still return the old value) - when(sessionUtils.getRegistrarForAuthResult(req, action.authResult)) + when(sessionUtils.getRegistrarForUser(CLIENT_ID, USER_AUTHORIZED)) .thenAnswer(x -> loadRegistrar(CLIENT_ID)); + when(sessionUtils.getRegistrarForUser(CLIENT_ID, USER_UNAUTHORIZED)) + .thenThrow(new ForbiddenException("forbidden")); } } diff --git a/javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java b/javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java index d48b6bc14..1720bf788 100644 --- a/javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java +++ b/javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java @@ -24,7 +24,6 @@ import static google.registry.testing.DatastoreHelper.loadRegistrar; 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; @@ -112,8 +111,6 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase { .setClientCertificate(SAMPLE_CERT, START_OF_TIME) .setFailoverClientCertificate(SAMPLE_CERT2, START_OF_TIME) .build()); - when(sessionUtils.getRegistrarForAuthResult(req, action.authResult)) - .thenReturn(initialRegistrar); Map jsonMap = initialRegistrar.toJsonMap(); jsonMap.put("clientCertificate", null); jsonMap.put("failoverClientCertificate", ""); diff --git a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java b/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java index 905de12a4..3a5137b1b 100644 --- a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java +++ b/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java @@ -16,29 +16,25 @@ package google.registry.ui.server.registrar; import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.AppEngineRule.THE_REGISTRAR_GAE_USER_ID; -import static google.registry.testing.DatastoreHelper.deleteResource; import static google.registry.testing.DatastoreHelper.loadRegistrar; import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.JUnitBackports.assertThrows; import static google.registry.testing.LogsSubject.assertAboutLogs; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; import com.google.appengine.api.users.User; import com.google.common.flogger.LoggerConfig; import com.google.common.testing.NullPointerTester; import com.google.common.testing.TestLogHandler; -import google.registry.model.registrar.RegistrarContact; +import google.registry.request.HttpException.ForbiddenException; +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.InjectRule; import java.util.logging.Level; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -55,25 +51,27 @@ public class SessionUtilsTest { private final HttpServletRequest req = mock(HttpServletRequest.class); private final HttpServletResponse rsp = mock(HttpServletResponse.class); - private final HttpSession session = mock(HttpSession.class); private final TestLogHandler testLogHandler = new TestLogHandler(); private SessionUtils sessionUtils; - private static final UserAuthInfo AUTHORIZED_USER = createAuthInfo(true, false); - private static final UserAuthInfo UNAUTHORIZED_USER = createAuthInfo(false, false); - private static final UserAuthInfo AUTHORIZED_ADMIN = createAuthInfo(true, true); - private static final UserAuthInfo UNAUTHORIZED_ADMIN = createAuthInfo(false, true); + private static final AuthResult AUTHORIZED_USER = createAuthResult(true, false); + private static final AuthResult UNAUTHORIZED_USER = createAuthResult(false, false); + private static final AuthResult AUTHORIZED_ADMIN = createAuthResult(true, true); + private static final AuthResult UNAUTHORIZED_ADMIN = createAuthResult(false, true); + private static final AuthResult NO_USER = AuthResult.create(AuthLevel.NONE); private static final String DEFAULT_CLIENT_ID = "TheRegistrar"; private static final String ADMIN_CLIENT_ID = "NewRegistrar"; - private static UserAuthInfo createAuthInfo(boolean isAuthorized, boolean isAdmin) { - return UserAuthInfo.create( - new User( - "user1@google.com", - "google.com", - isAuthorized ? THE_REGISTRAR_GAE_USER_ID : "badGaeUserId"), - isAdmin); + private static AuthResult createAuthResult(boolean isAuthorized, boolean isAdmin) { + return AuthResult.create( + AuthLevel.USER, + UserAuthInfo.create( + new User( + "user1@google.com", + "google.com", + isAuthorized ? THE_REGISTRAR_GAE_USER_ID : "badGaeUserId"), + isAdmin)); } @Before @@ -82,7 +80,6 @@ public class SessionUtilsTest { sessionUtils = new SessionUtils(); sessionUtils.registryAdminClientId = ADMIN_CLIENT_ID; persistResource(loadRegistrar(ADMIN_CLIENT_ID)); - when(req.getSession()).thenReturn(session); } @After @@ -90,240 +87,161 @@ public class SessionUtilsTest { LoggerConfig.getConfig(SessionUtils.class).removeHandler(testLogHandler); } - /** User needs to be logged in before calling checkRegistrarConsoleLogin */ - @Test - public void testCheckRegistrarConsoleLogin_notLoggedIn_throwsIllegalStateException() { - assertThrows( - IllegalStateException.class, - () -> { - @SuppressWarnings("unused") - boolean unused = sessionUtils.checkRegistrarConsoleLogin(req, null); - }); + private String formatMessage(String message, AuthResult authResult, String clientId) { + return message + .replace("{user}", authResult.userIdForLogging()) + .replace("{clientId}", String.valueOf(clientId)); } - /** - * If clientId exists in the session and the user does not have access to that registrar, then no - * access should be granted. - */ + /** Fail loading registrar if user doesn't have access to it. */ @Test - public void testCheckRegistrarConsoleLogin_hasSession_noAccess_isNotAdmin() { - when(session.getAttribute("clientId")).thenReturn(DEFAULT_CLIENT_ID); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, UNAUTHORIZED_USER)).isFalse(); - verify(session).invalidate(); - assertAboutLogs() - .that(testLogHandler) - .hasLogAtLevelWithMessage(Level.INFO, "Registrar Console access revoked"); + public void testGetRegistrarForUser_noAccess_isNotAdmin() { + expectGetRegistrarFailure( + DEFAULT_CLIENT_ID, + UNAUTHORIZED_USER, + "User {user} doesn't have access to registrar {clientId}"); } - /** - * If clientId exists in the session and the user does not have access to that registrar, then - * access should be revoked. The admin flag should be ignored. - */ + /** Fail loading registrar if there's no user associated with the request. */ @Test - public void testCheckRegistrarConsoleLogin_hasSession_noAccess_isAdmin() { - when(session.getAttribute("clientId")).thenReturn(DEFAULT_CLIENT_ID); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, UNAUTHORIZED_ADMIN)).isFalse(); - verify(session).invalidate(); - assertAboutLogs() - .that(testLogHandler) - .hasLogAtLevelWithMessage(Level.INFO, "Registrar Console access revoked"); + public void testGetRegistrarForUser_noUser() { + expectGetRegistrarFailure(DEFAULT_CLIENT_ID, NO_USER, "Not logged in"); } - /** - * If clientId exists in the session and the user does have access to that registrar, then access - * should be allowed. - */ + /** Succeed loading registrar if user has access to it. */ @Test - public void testCheckRegistrarConsoleLogin_hasSession_hasAccess_isNotAdmin() { - when(session.getAttribute("clientId")).thenReturn(DEFAULT_CLIENT_ID); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, AUTHORIZED_USER)).isTrue(); - verify(session).getAttribute("clientId"); - verifyNoMoreInteractions(session); + public void testGetRegistrarForUser_hasAccess_isNotAdmin() { + expectGetRegistrarSuccess(AUTHORIZED_USER, "User {user} has access to registrar {clientId}"); + } + + /** Succeed loading registrar if admin. */ + @Test + public void testGetRegistrarForUser_hasAccess_isAdmin() { + expectGetRegistrarSuccess(AUTHORIZED_ADMIN, "User {user} has access to registrar {clientId}"); + } + + /** Fail loading registrar if admin isn't on the approved contacts list. */ + @Test + public void testGetRegistrarForUser_noAccess_isAdmin() { + expectGetRegistrarFailure( + DEFAULT_CLIENT_ID, + UNAUTHORIZED_ADMIN, + "User {user} doesn't have access to registrar {clientId}"); + } + + /** Succeed loading registrarAdmin even if unauthorized admin. */ + @Test + public void testGetRegistrarForUser_registrarAdminClientId() { + sessionUtils.registryAdminClientId = DEFAULT_CLIENT_ID; + expectGetRegistrarSuccess( + UNAUTHORIZED_ADMIN, "Allowing admin {user} access to registrar {clientId}."); + } + + /** Fail loading registrar even if admin, if registrar doesn't exist. */ + @Test + public void testGetRegistrarForUser_doesntExist_isAdmin() { + expectGetRegistrarFailure("BadClientId", UNAUTHORIZED_ADMIN, "Registrar {clientId} not found"); + } + + private void expectGetRegistrarSuccess(AuthResult authResult, String message) { + assertThat(sessionUtils.getRegistrarForUser(DEFAULT_CLIENT_ID, authResult)).isNotNull(); assertAboutLogs() .that(testLogHandler) .hasLogAtLevelWithMessage( - Level.INFO, - String.format( - "Associating user %s with given registrar %s.", - AUTHORIZED_USER.user().getUserId(), DEFAULT_CLIENT_ID)); + Level.INFO, formatMessage(message, authResult, DEFAULT_CLIENT_ID)); + } + + private void expectGetRegistrarFailure(String clientId, AuthResult authResult, String message) { + ForbiddenException exception = + assertThrows( + ForbiddenException.class, () -> sessionUtils.getRegistrarForUser(clientId, authResult)); + + assertThat(exception).hasMessageThat().contains(formatMessage(message, authResult, clientId)); + assertAboutLogs().that(testLogHandler).hasNoLogsAtLevel(Level.INFO); + } + + /** If a user has access to a registrar, we should guess that registrar. */ + @Test + public void testGuessClientIdForUser_hasAccess_isNotAdmin() { + expectGuessRegistrarSuccess( + AUTHORIZED_USER, + DEFAULT_CLIENT_ID, + "Associating user {user} with found registrar {clientId}."); + } + + /** If a user doesn't have access to any registrars, guess returns nothing. */ + @Test + public void testGuessClientIdForUser_noAccess_isNotAdmin() { + expectGuessRegistrarFailure( + UNAUTHORIZED_USER, "User {user} isn't associated with any registrar"); } /** - * If clientId exists in the session and the user does have access to that registrar, then access - * should be allowed. The admin flag should be ignored. + * If an admin has access to a registrar, we should guess that registrar (rather than the + * ADMIN_CLIENT_ID). */ @Test - public void testCheckRegistrarConsoleLogin_hasSession_hasAccess_isAdmin() { - when(session.getAttribute("clientId")).thenReturn(DEFAULT_CLIENT_ID); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, AUTHORIZED_ADMIN)).isTrue(); - verify(session).getAttribute("clientId"); - verifyNoMoreInteractions(session); - assertAboutLogs() - .that(testLogHandler) - .hasLogAtLevelWithMessage( - Level.INFO, - String.format( - "Associating user %s with given registrar %s.", - AUTHORIZED_ADMIN.user().getUserId(), DEFAULT_CLIENT_ID)); + public void testGuessClientIdForUser_hasAccess_isAdmin() { + expectGuessRegistrarSuccess( + AUTHORIZED_ADMIN, + DEFAULT_CLIENT_ID, + "Associating user {user} with found registrar {clientId}."); + } + + /** If an admin doesn't have access to a registrar, we should guess the ADMIN_CLIENT_ID. */ + @Test + public void testGuessClientIdForUser_noAccess_isAdmin() { + expectGuessRegistrarSuccess( + UNAUTHORIZED_ADMIN, + ADMIN_CLIENT_ID, + "User {user} is an admin with no associated registrar." + + " Automatically associating the user with configured client Id {clientId}."); } /** - * If clientId does not exist in the session and the user has access to a registrar, then access - * should be granted to that registrar. + * If an admin is not associated with a registrar and there is no configured adminClientId, we + * can't guess the clientId. */ @Test - public void testCheckRegistrarConsoleLogin_noSession_hasAccess_isNotAdmin() { - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, AUTHORIZED_USER)).isTrue(); - verify(session).setAttribute(eq("clientId"), eq(DEFAULT_CLIENT_ID)); - assertAboutLogs() - .that(testLogHandler) - .hasLogAtLevelWithMessage( - Level.INFO, - String.format( - "Associating user %s with found registrar %s.", - AUTHORIZED_USER.user().getUserId(), DEFAULT_CLIENT_ID)); - } - - /** - * If clientId does not exist in the session and the user has access to a registrar, then access - * should be granted to that registrar. The admin flag should be ignored. - */ - @Test - public void testCheckRegistrarConsoleLogin_noSession_hasAccess_isAdmin() { - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, AUTHORIZED_ADMIN)).isTrue(); - verify(session).setAttribute(eq("clientId"), eq(DEFAULT_CLIENT_ID)); - assertAboutLogs() - .that(testLogHandler) - .hasLogAtLevelWithMessage( - Level.INFO, - String.format( - "Associating user %s with found registrar %s.", - AUTHORIZED_ADMIN.user().getUserId(), DEFAULT_CLIENT_ID)); - } - - /** - * If clientId does not exist in the session, the user is not associated with a registrar and the - * user is an admin, then access could be granted to the configured adminClientId. But if the - * configured adminClientId is empty or null, no access is granted. - */ - @Test - public void testCheckRegistrarConsoleLogin_noSession_noAccess_isAdmin_adminClientIdEmpty() { + public void testGuessClientIdForUser_noAccess_isAdmin_adminClientIdEmpty() { sessionUtils.registryAdminClientId = ""; - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, UNAUTHORIZED_ADMIN)).isFalse(); + expectGuessRegistrarFailure( + UNAUTHORIZED_ADMIN, "User {user} isn't associated with any registrar"); assertAboutLogs() .that(testLogHandler) .hasLogAtLevelWithMessage( Level.INFO, - String.format( - "Cannot associate admin user %s with configured client Id." - + " ClientId is null or empty.", - UNAUTHORIZED_ADMIN.user().getUserId())); + "Cannot associate admin user badGaeUserId with configured client Id." + + " ClientId is null or empty."); } /** - * If clientId does not exist in the session, the user is not associated with a registrar and the - * user is an admin, then access could be granted to the configured adminClientId. But if the - * configured adminClientId does not reference a registry, then no access is granted. + * If an admin is not associated with a registrar and the configured adminClientId points to a + * non-existent registrar, we still guess it (we will later failing loading the registrar). */ @Test - public void testCheckRegistrarConsoleLogin_noSession_noAccess_isAdmin_adminClientIdInvalid() { - sessionUtils.registryAdminClientId = "NonexistentRegistry"; - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, UNAUTHORIZED_ADMIN)).isFalse(); + public void testGuessClientIdForUser_noAccess_isAdmin_adminClientIdInvalid() { + sessionUtils.registryAdminClientId = "NonexistentRegistrar"; + expectGuessRegistrarSuccess( + UNAUTHORIZED_ADMIN, + "NonexistentRegistrar", + "User {user} is an admin with no associated registrar." + + " Automatically associating the user with configured client Id {clientId}."); + } + + private void expectGuessRegistrarSuccess(AuthResult authResult, String clientId, String message) { + assertThat(sessionUtils.guessClientIdForUser(authResult)).isEqualTo(clientId); assertAboutLogs() .that(testLogHandler) - .hasLogAtLevelWithMessage( - Level.INFO, - String.format( - "Cannot associate admin user %s with configured client Id %s." - + " Registrar does not exist.", - UNAUTHORIZED_ADMIN.user().getUserId(), "NonexistentRegistry")); + .hasLogAtLevelWithMessage(Level.INFO, formatMessage(message, authResult, clientId)); } - /** - * If clientId does not exist in the session, the user does not have access to a registrar and the - * user is an admin, then grant the user access to the validated configured adminClientId. - */ - @Test - public void testCheckRegistrarConsoleLogin_noSession_noAccess_isAdmin() { - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, UNAUTHORIZED_ADMIN)).isTrue(); - verify(session).setAttribute(eq("clientId"), eq(ADMIN_CLIENT_ID)); - assertAboutLogs() - .that(testLogHandler) - .hasLogAtLevelWithMessage( - Level.INFO, - String.format( - "User %s is an admin with no associated registrar." - + " Automatically associating the user with configured client Id %s.", - UNAUTHORIZED_ADMIN.user().getUserId(), ADMIN_CLIENT_ID)); - } - - /** - * If session clientId points to the adminClientId, and the user is an admin that doesn't have - * access to this registrar - it means this is the second (or later) visit of this admin and they - * were granted access to the default registrar because they aren't associated with any other - * registrar. - * - * We continue to grant the admin access. - */ - @Test - public void testCheckRegistrarConsoleLogin_noSession_noAccess_isAdmin_secondRequest() { - when(session.getAttribute("clientId")).thenReturn(ADMIN_CLIENT_ID); - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, UNAUTHORIZED_ADMIN)).isTrue(); - assertAboutLogs() - .that(testLogHandler) - .hasLogAtLevelWithMessage( - Level.INFO, - String.format( - "Associating user %s with given registrar %s.", - UNAUTHORIZED_ADMIN.user().getUserId(), ADMIN_CLIENT_ID)); - } - - /** - * If clientId does not exist in the session and the user is not associated with a registrar, then - * access should not be granted. - */ - @Test - public void testCheckRegistrarConsoleLogin_noSession_noAccess_isNotAdmin() { - assertThat(sessionUtils.checkRegistrarConsoleLogin(req, UNAUTHORIZED_USER)).isFalse(); - assertAboutLogs() - .that(testLogHandler) - .hasLogAtLevelWithMessage( - Level.INFO, - String.format( - "User not associated with any Registrar: %s", - UNAUTHORIZED_USER.user().getUserId())); - } - - @Test - public void testHasAccessToRegistrar_orphanedContact_returnsFalse() { - deleteResource(loadRegistrar(DEFAULT_CLIENT_ID)); - assertThat( - sessionUtils.hasAccessToRegistrar(DEFAULT_CLIENT_ID, THE_REGISTRAR_GAE_USER_ID, false)) - .isFalse(); - } - - @Test - public void testHasAccessToRegistrar_accessRevoked_returnsFalse() { - RegistrarContact.updateContacts(loadRegistrar(DEFAULT_CLIENT_ID), new java.util.HashSet<>()); - assertThat( - sessionUtils.hasAccessToRegistrar(DEFAULT_CLIENT_ID, THE_REGISTRAR_GAE_USER_ID, false)) - .isFalse(); - } - - @Test - public void testHasAccessToRegistrar_orphanedAdmin_notAdminRegistrar_returnsFalse() { - RegistrarContact.updateContacts(loadRegistrar(DEFAULT_CLIENT_ID), new java.util.HashSet<>()); - assertThat( - sessionUtils.hasAccessToRegistrar(DEFAULT_CLIENT_ID, THE_REGISTRAR_GAE_USER_ID, true)) - .isFalse(); - } - - @Test - public void testHasAccessToRegistrar_orphanedAdmin_onAdminRegistrar_returnsTrue() { - RegistrarContact.updateContacts(loadRegistrar(ADMIN_CLIENT_ID), new java.util.HashSet<>()); - assertThat( - sessionUtils.hasAccessToRegistrar(ADMIN_CLIENT_ID, THE_REGISTRAR_GAE_USER_ID, true)) - .isTrue(); + private void expectGuessRegistrarFailure(AuthResult authResult, String message) { + ForbiddenException exception = + assertThrows(ForbiddenException.class, () -> sessionUtils.guessClientIdForUser(authResult)); + assertThat(exception) + .hasMessageThat() + .contains(formatMessage(message, UNAUTHORIZED_USER, null)); } @Test