diff --git a/java/google/registry/rdap/RdapActionBase.java b/java/google/registry/rdap/RdapActionBase.java index 6fe80ef49..7dfefa84a 100644 --- a/java/google/registry/rdap/RdapActionBase.java +++ b/java/google/registry/rdap/RdapActionBase.java @@ -18,7 +18,7 @@ import static com.google.common.base.Charsets.UTF_8; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN; import static google.registry.model.ofy.ObjectifyService.ofy; -import static google.registry.ui.server.registrar.SessionUtils.AccessType.READ_ONLY; +import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.READ; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.DomainNameUtils.canonicalizeDomainName; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; @@ -48,7 +48,7 @@ import google.registry.request.RequestPath; import google.registry.request.Response; import google.registry.request.auth.AuthResult; import google.registry.request.auth.UserAuthInfo; -import google.registry.ui.server.registrar.SessionUtils; +import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; @@ -91,7 +91,7 @@ public abstract class RdapActionBase implements Runnable { @Inject @RequestPath String requestPath; @Inject @FullServletPath String fullServletPath; @Inject AuthResult authResult; - @Inject SessionUtils sessionUtils; + @Inject AuthenticatedRegistrarAccessor registrarAccessor; @Inject RdapJsonFormatter rdapJsonFormatter; @Inject @Parameter("registrar") Optional registrarParam; @Inject @Parameter("includeDeleted") Optional includeDeletedParam; @@ -177,12 +177,12 @@ 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) { + private Optional getAuthorizedClientId() { try { - String clientId = sessionUtils.guessClientIdForUser(authResult); + String clientId = registrarAccessor.guessClientId(); // 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, READ_ONLY, authResult); + registrarAccessor.getRegistrarForUserCached(clientId, READ); return Optional.of(clientId); } catch (Exception e) { logger.atWarning().withCause(e).log( @@ -215,7 +215,7 @@ public abstract class RdapActionBase implements Runnable { if (userAuthInfo.isUserAdmin()) { return RdapAuthorization.ADMINISTRATOR_AUTHORIZATION; } - Optional clientId = getAuthorizedClientId(authResult); + Optional clientId = getAuthorizedClientId(); if (!clientId.isPresent()) { return RdapAuthorization.PUBLIC_AUTHORIZATION; } @@ -249,7 +249,7 @@ public abstract class RdapActionBase implements Runnable { if (userAuthInfo.isUserAdmin()) { return true; } - if (!getAuthorizedClientId(authResult).isPresent()) { + if (!getAuthorizedClientId().isPresent()) { return false; } return true; diff --git a/java/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessor.java b/java/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessor.java new file mode 100644 index 000000000..44334676d --- /dev/null +++ b/java/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessor.java @@ -0,0 +1,234 @@ +// Copyright 2018 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 static google.registry.model.ofy.ObjectifyService.ofy; + +import com.google.appengine.api.users.User; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableSetMultimap; +import com.google.common.flogger.FluentLogger; +import google.registry.config.RegistryConfig.Config; +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.request.auth.UserAuthInfo; +import java.util.Optional; +import java.util.function.Function; +import javax.annotation.concurrent.Immutable; +import javax.inject.Inject; + +/** + * Allows access only to {@link Registrar}s the current user has access to. + * + *

A user has read+write access to a Registrar if there exists a {@link RegistrarContact} with + * that user's gaeId and the registrar as a parent. + * + *

An admin has in addition read+write access to {@link #registryAdminClientId}. + * + *

An admin also has read access to ALL registrars. + */ +@Immutable +public class AuthenticatedRegistrarAccessor { + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + /** Type of access we're requesting. */ + public enum AccessType { + READ, + UPDATE + } + + AuthResult authResult; + String registryAdminClientId; + + /** + * For any AccessType requested - all clientIDs this user is allowed that AccessType on. + * + *

The order is significant, with "more specific to this user" coming first. + */ + private final ImmutableSetMultimap accessMap; + + @Inject + public AuthenticatedRegistrarAccessor( + AuthResult authResult, @Config("registryAdminClientId") String registryAdminClientId) { + this.authResult = authResult; + this.registryAdminClientId = registryAdminClientId; + this.accessMap = createAccessMap(authResult, registryAdminClientId); + + logger.atInfo().log( + "User %s has the following accesses: %s", authResult.userIdForLogging(), accessMap); + } + + /** + * Loads a Registrar IFF the user is authorized. + * + *

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 accessType what kind of access do we want for this registrar - just read it or write as + * well? (different users might have different access levels) + */ + public Registrar getRegistrar(String clientId, AccessType accessType) { + return getAndAuthorize(Registrar::loadByClientId, clientId, accessType); + } + + /** + * Loads a Registrar from the cache IFF the user is authorized. + * + *

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 accessType what kind of access do we want for this registrar - just read it or write as + * well? (different users might have different access levels) + */ + public Registrar getRegistrarForUserCached(String clientId, AccessType accessType) { + return getAndAuthorize(Registrar::loadByClientIdCached, clientId, accessType); + } + + /** + * For all {@link AccessType}s, Returns all ClientIds this user is allowed this access. + * + *

Throws a {@link ForbiddenException} if the user is not logged in. + * + *

The result is ordered starting from "most specific to this user". + * + *

If you want to load the {@link Registrar} object from these (or any other) {@code clientId}, + * in order to perform actions on behalf of a user, you must use {@link #getRegistrar} 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 #getRegistrar}. + */ + public ImmutableSetMultimap getAllClientIdWithAccess() { + verifyLoggedIn(); + return accessMap; + } + + /** + * "Guesses" which client ID the user wants from all those they have access to. + * + *

If no such ClientIds exist, throws a ForbiddenException. + * + *

This should be the ClientId "most likely wanted by the user". + * + *

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 #getRegistrar} 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 #getRegistrar}. + */ + public String guessClientId() { + verifyLoggedIn(); + return getAllClientIdWithAccess().values().stream() + .findFirst() + .orElseThrow( + () -> + new ForbiddenException( + String.format( + "User %s isn't associated with any registrar", + authResult.userIdForLogging()))); + } + + private Registrar getAndAuthorize( + Function> registrarLoader, + String clientId, + AccessType accessType) { + verifyLoggedIn(); + + if (!accessMap.containsEntry(accessType, clientId)) { + throw new ForbiddenException( + String.format( + "User %s doesn't have %s access to registrar %s", + authResult.userIdForLogging(), accessType, clientId)); + } + + Registrar registrar = + registrarLoader + .apply(clientId) + .orElseThrow( + () -> new ForbiddenException(String.format("Registrar %s not found", clientId))); + + if (!clientId.equals(registrar.getClientId())) { + logger.atSevere().log( + "registrarLoader.apply(clientId) returned a Registrar with a different clientId. " + + "Requested: %s, returned: %s.", + clientId, registrar.getClientId()); + throw new ForbiddenException("Internal error - please check logs"); + } + + logger.atInfo().log( + "User %s has %s access to registrar %s.", + authResult.userIdForLogging(), accessType, clientId); + return registrar; + } + + private static ImmutableSetMultimap createAccessMap( + AuthResult authResult, String registryAdminClientId) { + + if (!authResult.userAuthInfo().isPresent()) { + return ImmutableSetMultimap.of(); + } + + UserAuthInfo userAuthInfo = authResult.userAuthInfo().get(); + + boolean isAdmin = userAuthInfo.isUserAdmin(); + User user = userAuthInfo.user(); + + ImmutableSetMultimap.Builder builder = new ImmutableSetMultimap.Builder<>(); + + ofy() + .load() + .type(RegistrarContact.class) + .filter("gaeUserId", user.getUserId()) + .forEach( + contact -> + builder + .put(AccessType.UPDATE, contact.getParent().getName()) + .put(AccessType.READ, contact.getParent().getName())); + if (isAdmin && !Strings.isNullOrEmpty(registryAdminClientId)) { + logger.atInfo().log( + "Giving admin %s read+write access to admin registrar %s", + authResult.userIdForLogging(), registryAdminClientId); + builder + .put(AccessType.UPDATE, registryAdminClientId) + .put(AccessType.READ, registryAdminClientId); + } + + if (isAdmin) { + // Admins have READ access to all registrars + logger.atInfo().log( + "Giving admin %s read-only access to all registrars", authResult.userIdForLogging()); + ofy() + .load() + .type(Registrar.class) + .forEach(registrar -> builder.put(AccessType.READ, registrar.getClientId())); + } + + return builder.build(); + } + + private void verifyLoggedIn() { + if (!authResult.userAuthInfo().isPresent()) { + throw new ForbiddenException("Not logged in"); + } + } +} diff --git a/java/google/registry/ui/server/registrar/ConsoleUiAction.java b/java/google/registry/ui/server/registrar/ConsoleUiAction.java index c3d682a3f..6e5d9ff55 100644 --- a/java/google/registry/ui/server/registrar/ConsoleUiAction.java +++ b/java/google/registry/ui/server/registrar/ConsoleUiAction.java @@ -16,8 +16,9 @@ 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.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.READ; +import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.UPDATE; import static google.registry.ui.server.registrar.RegistrarConsoleModule.PARAM_CLIENT_ID; -import static google.registry.ui.server.registrar.SessionUtils.AccessType.READ_ONLY; 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; @@ -26,6 +27,7 @@ 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.collect.Sets; import com.google.common.flogger.FluentLogger; import com.google.common.io.Resources; import com.google.common.net.MediaType; @@ -71,7 +73,7 @@ public final class ConsoleUiAction implements Runnable { @Inject HttpServletRequest req; @Inject Response response; - @Inject SessionUtils sessionUtils; + @Inject AuthenticatedRegistrarAccessor registrarAccessor; @Inject UserService userService; @Inject XsrfTokenManager xsrfTokenManager; @Inject AuthResult authResult; @@ -131,9 +133,16 @@ public final class ConsoleUiAction implements Runnable { data.put("logoutUrl", userService.createLogoutURL(PATH)); data.put("xsrfToken", xsrfTokenManager.generateToken(user.getEmail())); try { - String clientId = - paramClientId.orElseGet(() -> sessionUtils.guessClientIdForUser(authResult)); + String clientId = paramClientId.orElse(registrarAccessor.guessClientId()); data.put("clientId", clientId); + + data.put("readWriteClientIds", registrarAccessor.getAllClientIdWithAccess().get(UPDATE)); + data.put( + "readOnlyClientIds", + Sets.difference( + registrarAccessor.getAllClientIdWithAccess().get(READ), + registrarAccessor.getAllClientIdWithAccess().get(UPDATE))); + // 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. // @@ -141,7 +150,7 @@ public final class ConsoleUiAction implements Runnable { // 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, READ_ONLY, authResult); + Registrar registrar = registrarAccessor.getRegistrar(clientId, READ); data.put("requireFeeExtension", registrar.getPremiumPriceAckRequired()); } catch (ForbiddenException e) { logger.atWarning().withCause(e).log( diff --git a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index cbd054243..d6ce6a466 100644 --- a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -20,8 +20,8 @@ import static google.registry.export.sheet.SyncRegistrarsSheetAction.enqueueRegi import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.security.JsonResponseHelper.Status.ERROR; import static google.registry.security.JsonResponseHelper.Status.SUCCESS; -import static google.registry.ui.server.registrar.SessionUtils.AccessType.READ_ONLY; -import static google.registry.ui.server.registrar.SessionUtils.AccessType.READ_WRITE; +import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.READ; +import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.UPDATE; import com.google.common.base.Strings; import com.google.common.collect.HashMultimap; @@ -34,13 +34,11 @@ import com.google.common.flogger.FluentLogger; import google.registry.config.RegistryConfig.Config; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; -import google.registry.model.registrar.RegistrarContact.Builder; import google.registry.model.registrar.RegistrarContact.Type; import google.registry.request.Action; import google.registry.request.HttpException.BadRequestException; import google.registry.request.JsonActionRunner; import google.registry.request.auth.Auth; -import google.registry.request.auth.AuthResult; import google.registry.security.JsonResponseHelper; import google.registry.ui.forms.FormException; import google.registry.ui.forms.FormFieldException; @@ -79,9 +77,9 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA @Inject JsonActionRunner jsonActionRunner; @Inject AppEngineServiceUtils appEngineServiceUtils; - @Inject AuthResult authResult; @Inject SendEmailUtils sendEmailUtils; - @Inject SessionUtils sessionUtils; + @Inject AuthenticatedRegistrarAccessor registrarAccessor; + @Inject @Config("registrarChangesNotificationEmailAddresses") ImmutableList registrarChangesNotificationEmailAddresses; @Inject RegistrarSettingsAction() {} @@ -134,9 +132,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA Map read(String clientId) { return JsonResponseHelper.create( - SUCCESS, - "Success", - sessionUtils.getRegistrarForUser(clientId, READ_ONLY, authResult).toJsonMap()); + SUCCESS, "Success", registrarAccessor.getRegistrar(clientId, READ).toJsonMap()); } Map update(final Map args, String clientId) { @@ -146,8 +142,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA // 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 = - sessionUtils.getRegistrarForUser(clientId, READ_WRITE, authResult); + Registrar registrar = registrarAccessor.getRegistrar(clientId, UPDATE); // 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 @@ -263,7 +258,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA Registrar registrar, Map args) { ImmutableSet.Builder contacts = new ImmutableSet.Builder<>(); - Optional> builders = RegistrarFormFields.CONTACTS_FIELD.extractUntyped(args); + Optional> builders = + RegistrarFormFields.CONTACTS_FIELD.extractUntyped(args); if (builders.isPresent()) { builders.get().forEach(c -> contacts.add(c.setParent(registrar).build())); } diff --git a/java/google/registry/ui/server/registrar/SessionUtils.java b/java/google/registry/ui/server/registrar/SessionUtils.java deleted file mode 100644 index 57a63048f..000000000 --- a/java/google/registry/ui/server/registrar/SessionUtils.java +++ /dev/null @@ -1,199 +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 static google.registry.model.ofy.ObjectifyService.ofy; - -import com.google.appengine.api.users.User; -import com.google.common.base.Strings; -import com.google.common.flogger.FluentLogger; -import google.registry.config.RegistryConfig.Config; -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.request.auth.UserAuthInfo; -import java.util.Optional; -import java.util.function.Function; -import javax.annotation.concurrent.Immutable; -import javax.inject.Inject; - -/** HTTP session management helper class. */ -@Immutable -public class SessionUtils { - - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - @Inject - @Config("registryAdminClientId") - String registryAdminClientId; - - @Inject - public SessionUtils() {} - - /** Type of access we're requesting. */ - public enum AccessType {READ_ONLY, READ_WRITE} - - /** - * Loads Registrar on behalf of an authorised user. - * - *

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 accessType what kind of access do we want for this registrar - just read it or write as - * well? (different users might have different access levels) - * @param authResult AuthResult of the user on behalf of which we want to access the data - */ - public Registrar getRegistrarForUser( - String clientId, AccessType accessType, AuthResult authResult) { - return getAndAuthorize(Registrar::loadByClientId, clientId, accessType, authResult); - } - - /** - * Loads a Registrar from the cache on behalf of an authorised user. - * - *

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 accessType what kind of access do we want for this registrar - just read it or write as - * well? (different users might have different access levels) - * @param authResult AuthResult of the user on behalf of which we want to access the data - */ - public Registrar getRegistrarForUserCached( - String clientId, AccessType accessType, AuthResult authResult) { - return getAndAuthorize(Registrar::loadByClientIdCached, clientId, accessType, authResult); - } - - private Registrar getAndAuthorize( - Function> registrarLoader, - String clientId, - AccessType accessType, - AuthResult authResult) { - UserAuthInfo userAuthInfo = - authResult.userAuthInfo().orElseThrow(() -> new ForbiddenException("Not logged in")); - boolean isAdmin = userAuthInfo.isUserAdmin(); - User user = userAuthInfo.user(); - String userIdForLogging = authResult.userIdForLogging(); - - Registrar registrar = - registrarLoader - .apply(clientId) - .orElseThrow( - () -> new ForbiddenException(String.format("Registrar %s not found", clientId))); - - if (isInAllowedContacts(registrar, user)) { - logger.atInfo().log("User %s has access to registrar %s.", userIdForLogging, clientId); - return registrar; - } - - 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.", userIdForLogging, clientId); - return registrar; - } - - if (isAdmin && accessType == AccessType.READ_ONLY) { - // Admins have read-only access to all registrars - logger.atInfo().log( - "Allowing admin %s read-only access to registrar %s.", userIdForLogging, clientId); - return registrar; - } - - throw new ForbiddenException( - String.format( - "User %s doesn't have %s access to registrar %s", - userIdForLogging, accessType, 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 userIdForLogging = authResult.userIdForLogging(); - - RegistrarContact contact = - ofy() - .load() - .type(RegistrarContact.class) - .filter("gaeUserId", user.getUserId()) - .first() - .now(); - if (contact != null) { - String registrarClientId = contact.getParent().getName(); - logger.atInfo().log( - "Associating user %s with found registrar %s.", userIdForLogging, registrarClientId); - return registrarClientId; - } - - // We couldn't find the registrar, but maybe the user is an admin and we can use the - // registryAdminClientId - if (isAdmin) { - if (!Strings.isNullOrEmpty(registryAdminClientId)) { - logger.atInfo().log( - "User %s is an admin with no associated registrar." - + " Automatically associating the user with configured client Id %s.", - userIdForLogging, registryAdminClientId); - return registryAdminClientId; - } - logger.atInfo().log( - "Cannot associate admin user %s with configured client Id." - + " ClientId is null or empty.", - userIdForLogging); - } - - // We couldn't find any relevant clientId - throw new ForbiddenException( - String.format("User %s isn't associated with any registrar", userIdForLogging)); - } - - /** - * Returns {@code true} if {@code user} is listed in contacts with access to the registrar. - * - *

Each registrar contact can either have getGaeUserId equals null or the user's gaeUserId. - * Null means the contact doesn't have access to the registrar console. None-null means the - * contact has access. - */ - private static boolean isInAllowedContacts(Registrar registrar, User user) { - String gaeUserId = user.getUserId(); - return registrar - .getContacts() - .stream() - .anyMatch(contact -> gaeUserId.equals(contact.getGaeUserId())); - } -} diff --git a/java/google/registry/ui/soy/registrar/Console.soy b/java/google/registry/ui/soy/registrar/Console.soy index 7beb32b87..516a0212a 100644 --- a/java/google/registry/ui/soy/registrar/Console.soy +++ b/java/google/registry/ui/soy/registrar/Console.soy @@ -23,6 +23,8 @@ {template .main} {@param xsrfToken: string} /** Security token. */ {@param clientId: string} /** Registrar client identifier. */ + {@param readWriteClientIds: list} + {@param readOnlyClientIds: list} {@param username: string} /** Arbitrary username to display. */ {@param logoutUrl: string} /** Generated URL for logging out of Google. */ {@param productName: string} /** Name to display for this software product. */ @@ -75,7 +77,33 @@ /** Sidebar nav. Ids on each elt for testing only. */ {template .navbar_ visibility="private"} + {@param clientId: string} /** Registrar client identifier. */ + {@param readWriteClientIds: list} + {@param readOnlyClientIds: list} +

+
+ +
{/template} diff --git a/javatests/google/registry/rdap/RdapActionBaseTest.java b/javatests/google/registry/rdap/RdapActionBaseTest.java index e2143861d..5a84b6c96 100644 --- a/javatests/google/registry/rdap/RdapActionBaseTest.java +++ b/javatests/google/registry/rdap/RdapActionBaseTest.java @@ -40,7 +40,7 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.InjectRule; -import google.registry.ui.server.registrar.SessionUtils; +import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor; import java.util.Optional; import org.joda.time.DateTime; import org.json.simple.JSONValue; @@ -64,7 +64,8 @@ public class RdapActionBaseTest { 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 AuthenticatedRegistrarAccessor registrarAccessor = + mock(AuthenticatedRegistrarAccessor.class); private final User user = new User("rdap.user@example.com", "gmail.com", "12345"); private final UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); private final RdapMetrics rdapMetrics = mock(RdapMetrics.class); @@ -119,7 +120,7 @@ public class RdapActionBaseTest { createTld("thing"); inject.setStaticField(Ofy.class, "clock", clock); action = new RdapTestAction(); - action.sessionUtils = sessionUtils; + action.registrarAccessor = registrarAccessor; action.authResult = AuthResult.create(AuthLevel.USER, userAuthInfo); action.includeDeletedParam = Optional.empty(); action.registrarParam = Optional.empty(); diff --git a/javatests/google/registry/rdap/RdapDomainActionTest.java b/javatests/google/registry/rdap/RdapDomainActionTest.java index 00f8f784c..fd0d4cb65 100644 --- a/javatests/google/registry/rdap/RdapDomainActionTest.java +++ b/javatests/google/registry/rdap/RdapDomainActionTest.java @@ -53,7 +53,7 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.InjectRule; -import google.registry.ui.server.registrar.SessionUtils; +import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor; import java.util.List; import java.util.Map; import java.util.Optional; @@ -82,7 +82,8 @@ public class RdapDomainActionTest { 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 AuthenticatedRegistrarAccessor registrarAccessor = + mock(AuthenticatedRegistrarAccessor.class); private final RdapMetrics rdapMetrics = mock(RdapMetrics.class); private RdapDomainAction action; @@ -275,17 +276,17 @@ public class RdapDomainActionTest { action.formatOutputParam = Optional.empty(); action.rdapJsonFormatter = RdapTestHelper.getTestRdapJsonFormatter(); action.rdapWhoisServer = null; - action.sessionUtils = sessionUtils; + action.registrarAccessor = registrarAccessor; action.authResult = AUTH_RESULT; action.rdapMetrics = rdapMetrics; } private void login(String clientId) { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn(clientId); + when(registrarAccessor.guessClientId()).thenReturn(clientId); } private void loginAsAdmin() { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT_ADMIN)).thenReturn("irrelevant"); + when(registrarAccessor.guessClientId()).thenReturn("irrelevant"); action.authResult = AUTH_RESULT_ADMIN; } diff --git a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java index 2684e72ce..20af008a8 100644 --- a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java +++ b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java @@ -62,7 +62,7 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.InjectRule; -import google.registry.ui.server.registrar.SessionUtils; +import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor; import google.registry.util.Idn; import java.net.IDN; import java.net.URLDecoder; @@ -91,7 +91,8 @@ public class RdapDomainSearchActionTest extends RdapSearchActionTestCase { @Rule public final InjectRule inject = new InjectRule(); private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01T00:00:00Z")); - private final SessionUtils sessionUtils = mock(SessionUtils.class); + private final AuthenticatedRegistrarAccessor registrarAccessor = + mock(AuthenticatedRegistrarAccessor.class); private final RdapDomainSearchAction action = new RdapDomainSearchAction(); private static final AuthResult AUTH_RESULT = @@ -405,7 +406,7 @@ public class RdapDomainSearchActionTest extends RdapSearchActionTestCase { action.formatOutputParam = Optional.empty(); action.rdapJsonFormatter = RdapTestHelper.getTestRdapJsonFormatter(); action.rdapWhoisServer = null; - action.sessionUtils = sessionUtils; + action.registrarAccessor = registrarAccessor; action.authResult = AUTH_RESULT; action.rdapMetrics = rdapMetrics; action.cursorTokenParam = Optional.empty(); @@ -413,12 +414,12 @@ public class RdapDomainSearchActionTest extends RdapSearchActionTestCase { } private void login(String clientId) { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn(clientId); + when(registrarAccessor.guessClientId()).thenReturn(clientId); metricRole = REGISTRAR; } private void loginAsAdmin() { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn("irrelevant"); + when(registrarAccessor.guessClientId()).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 f79d216b7..808b479e0 100644 --- a/javatests/google/registry/rdap/RdapEntityActionTest.java +++ b/javatests/google/registry/rdap/RdapEntityActionTest.java @@ -49,7 +49,7 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.InjectRule; -import google.registry.ui.server.registrar.SessionUtils; +import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor; import java.util.Map; import java.util.Optional; import javax.annotation.Nullable; @@ -75,7 +75,8 @@ public class RdapEntityActionTest { 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 AuthenticatedRegistrarAccessor registrarAccessor = + mock(AuthenticatedRegistrarAccessor.class); private final RdapMetrics rdapMetrics = mock(RdapMetrics.class); private RdapEntityAction action; @@ -176,18 +177,18 @@ public class RdapEntityActionTest { action.formatOutputParam = Optional.empty(); action.rdapJsonFormatter = RdapTestHelper.getTestRdapJsonFormatter(); action.rdapWhoisServer = null; - action.sessionUtils = sessionUtils; + action.registrarAccessor = registrarAccessor; action.authResult = AUTH_RESULT; action.rdapMetrics = rdapMetrics; } private void login(String registrar) { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn(registrar); + when(registrarAccessor.guessClientId()).thenReturn(registrar); } private void loginAsAdmin() { action.authResult = AUTH_RESULT_ADMIN; - when(sessionUtils.guessClientIdForUser(AUTH_RESULT_ADMIN)).thenReturn("irrelevant"); + when(registrarAccessor.guessClientId()).thenReturn("irrelevant"); } private Object generateActualJson(String name) { diff --git a/javatests/google/registry/rdap/RdapEntitySearchActionTest.java b/javatests/google/registry/rdap/RdapEntitySearchActionTest.java index 42932e373..7d9ad6626 100644 --- a/javatests/google/registry/rdap/RdapEntitySearchActionTest.java +++ b/javatests/google/registry/rdap/RdapEntitySearchActionTest.java @@ -53,7 +53,7 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.InjectRule; -import google.registry.ui.server.registrar.SessionUtils; +import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor; import java.net.URLDecoder; import java.util.List; import java.util.Map; @@ -82,7 +82,8 @@ public class RdapEntitySearchActionTest extends RdapSearchActionTestCase { } private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01T00:00:00Z")); - private final SessionUtils sessionUtils = mock(SessionUtils.class); + private final AuthenticatedRegistrarAccessor registrarAccessor = + mock(AuthenticatedRegistrarAccessor.class); private final RdapEntitySearchAction action = new RdapEntitySearchAction(); private FakeResponse response = new FakeResponse(); @@ -202,20 +203,20 @@ public class RdapEntitySearchActionTest extends RdapSearchActionTestCase { action.registrarParam = Optional.empty(); action.includeDeletedParam = Optional.empty(); action.formatOutputParam = Optional.empty(); - action.sessionUtils = sessionUtils; + action.registrarAccessor = registrarAccessor; action.authResult = AUTH_RESULT; action.rdapMetrics = rdapMetrics; action.cursorTokenParam = Optional.empty(); } private void login(String registrar) { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn(registrar); + when(registrarAccessor.guessClientId()).thenReturn(registrar); metricRole = REGISTRAR; } private void loginAsAdmin() { action.authResult = AUTH_RESULT_ADMIN; - when(sessionUtils.guessClientIdForUser(AUTH_RESULT_ADMIN)).thenReturn("irrelevant"); + when(registrarAccessor.guessClientId()).thenReturn("irrelevant"); metricRole = ADMINISTRATOR; } diff --git a/javatests/google/registry/rdap/RdapHelpActionTest.java b/javatests/google/registry/rdap/RdapHelpActionTest.java index e808daf04..33262a0cc 100644 --- a/javatests/google/registry/rdap/RdapHelpActionTest.java +++ b/javatests/google/registry/rdap/RdapHelpActionTest.java @@ -33,7 +33,7 @@ import google.registry.request.auth.UserAuthInfo; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.InjectRule; -import google.registry.ui.server.registrar.SessionUtils; +import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor; import java.util.Optional; import org.joda.time.DateTime; import org.json.simple.JSONValue; @@ -52,7 +52,8 @@ public class RdapHelpActionTest { 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 AuthenticatedRegistrarAccessor registrarAccessor = + mock(AuthenticatedRegistrarAccessor.class); private final User user = new User("rdap.user@example.com", "gmail.com", "12345"); private final UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); private final RdapMetrics rdapMetrics = mock(RdapMetrics.class); @@ -67,7 +68,7 @@ public class RdapHelpActionTest { action.clock = clock; action.fullServletPath = "https://example.tld/rdap"; action.requestMethod = Action.Method.GET; - action.sessionUtils = sessionUtils; + action.registrarAccessor = registrarAccessor; action.authResult = AuthResult.create(AuthLevel.USER, userAuthInfo); action.includeDeletedParam = Optional.empty(); action.registrarParam = Optional.empty(); diff --git a/javatests/google/registry/rdap/RdapNameserverActionTest.java b/javatests/google/registry/rdap/RdapNameserverActionTest.java index de8fc7b98..340ba3bf7 100644 --- a/javatests/google/registry/rdap/RdapNameserverActionTest.java +++ b/javatests/google/registry/rdap/RdapNameserverActionTest.java @@ -43,7 +43,7 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.InjectRule; -import google.registry.ui.server.registrar.SessionUtils; +import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor; import java.util.Map; import java.util.Optional; import javax.annotation.Nullable; @@ -69,7 +69,8 @@ public class RdapNameserverActionTest { 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 AuthenticatedRegistrarAccessor registrarAccessor = + mock(AuthenticatedRegistrarAccessor.class); private final RdapMetrics rdapMetrics = mock(RdapMetrics.class); private static final AuthResult AUTH_RESULT = @@ -132,7 +133,7 @@ public class RdapNameserverActionTest { action.rdapJsonFormatter = RdapTestHelper.getTestRdapJsonFormatter(); action.rdapWhoisServer = null; action.authResult = authResult; - action.sessionUtils = sessionUtils; + action.registrarAccessor = registrarAccessor; action.rdapMetrics = rdapMetrics; return action; } @@ -362,21 +363,21 @@ public class RdapNameserverActionTest { @Test public void testDeletedNameserver_notFound_notLoggedIn() { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenThrow(new ForbiddenException("blah")); + when(registrarAccessor.guessClientId()).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.guessClientIdForUser(AUTH_RESULT)).thenReturn("otherregistrar"); + when(registrarAccessor.guessClientId()).thenReturn("otherregistrar"); generateActualJson("nsdeleted.cat.lol", Optional.empty(), Optional.of(true)); assertThat(response.getStatus()).isEqualTo(404); } @Test public void testDeletedNameserver_found_loggedInAsCorrectRegistrar() { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn("TheRegistrar"); + when(registrarAccessor.guessClientId()).thenReturn("TheRegistrar"); assertThat( generateActualJson("nsdeleted.cat.lol", Optional.empty(), Optional.of(true))) .isEqualTo( @@ -393,7 +394,7 @@ public class RdapNameserverActionTest { @Test public void testDeletedNameserver_found_loggedInAsAdmin() { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT_ADMIN)).thenReturn("irrelevant"); + when(registrarAccessor.guessClientId()).thenReturn("irrelevant"); newRdapNameserverAction( "nsdeleted.cat.lol", Optional.empty(), @@ -415,7 +416,7 @@ public class RdapNameserverActionTest { @Test public void testDeletedNameserver_found_sameRegistrarRequested() { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn("TheRegistrar"); + when(registrarAccessor.guessClientId()).thenReturn("TheRegistrar"); assertThat( generateActualJson("nsdeleted.cat.lol", Optional.of("TheRegistrar"), Optional.of(true))) .isEqualTo( @@ -432,7 +433,7 @@ public class RdapNameserverActionTest { @Test public void testDeletedNameserver_notFound_differentRegistrarRequested() { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn("TheRegistrar"); + when(registrarAccessor.guessClientId()).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 9499fc8f4..804864d28 100644 --- a/javatests/google/registry/rdap/RdapNameserverSearchActionTest.java +++ b/javatests/google/registry/rdap/RdapNameserverSearchActionTest.java @@ -54,7 +54,7 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.InjectRule; -import google.registry.ui.server.registrar.SessionUtils; +import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor; import java.net.URLDecoder; import java.util.Optional; import javax.annotation.Nullable; @@ -78,7 +78,8 @@ public class RdapNameserverSearchActionTest extends RdapSearchActionTestCase { 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 AuthenticatedRegistrarAccessor registrarAccessor = + mock(AuthenticatedRegistrarAccessor.class); private final RdapNameserverSearchAction action = new RdapNameserverSearchAction(); private static final AuthResult AUTH_RESULT = @@ -200,17 +201,17 @@ public class RdapNameserverSearchActionTest extends RdapSearchActionTestCase { action.includeDeletedParam = Optional.empty(); action.formatOutputParam = Optional.empty(); action.authResult = AUTH_RESULT; - action.sessionUtils = sessionUtils; + action.registrarAccessor = registrarAccessor; action.rdapMetrics = rdapMetrics; action.cursorTokenParam = Optional.empty(); } private void login(String clientId) { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT)).thenReturn(clientId); + when(registrarAccessor.guessClientId()).thenReturn(clientId); metricRole = REGISTRAR; } private void loginAsAdmin() { - when(sessionUtils.guessClientIdForUser(AUTH_RESULT_ADMIN)).thenReturn("irrelevant"); + when(registrarAccessor.guessClientId()).thenReturn("irrelevant"); action.authResult = AUTH_RESULT_ADMIN; metricRole = ADMINISTRATOR; } diff --git a/javatests/google/registry/ui/js/registrar/console_test_util.js b/javatests/google/registry/ui/js/registrar/console_test_util.js index 2eac0449c..158f95277 100644 --- a/javatests/google/registry/ui/js/registrar/console_test_util.js +++ b/javatests/google/registry/ui/js/registrar/console_test_util.js @@ -62,6 +62,8 @@ registry.registrar.ConsoleTestUtil.renderConsoleMain = function( logoutUrl: args.logoutUrl || 'https://logout.url.com', isAdmin: goog.isDefAndNotNull(args.isAdmin) ? args.isAdmin : true, clientId: args.clientId || 'ignore', + readWriteClientIds: args.readWriteClientIds || ['readWrite'], + readOnlyClientIds: args.readOnlyClientIds || ['readOnly'], logoFilename: args.logoFilename || 'logo.png', productName: args.productName || 'Nomulus', integrationEmail: args.integrationEmail || 'integration@example.com', diff --git a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java b/javatests/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessorTest.java similarity index 58% rename from javatests/google/registry/ui/server/registrar/SessionUtilsTest.java rename to javatests/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessorTest.java index 86dce17ad..a97b6523c 100644 --- a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java +++ b/javatests/google/registry/ui/server/registrar/AuthenticatedRegistrarAccessorTest.java @@ -1,4 +1,4 @@ -// Copyright 2017 The Nomulus Authors. All Rights Reserved. +// Copyright 2018 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. @@ -20,8 +20,8 @@ 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 google.registry.ui.server.registrar.SessionUtils.AccessType.READ_ONLY; -import static google.registry.ui.server.registrar.SessionUtils.AccessType.READ_WRITE; +import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.READ; +import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.UPDATE; import static org.mockito.Mockito.mock; import com.google.appengine.api.users.User; @@ -34,7 +34,7 @@ import google.registry.request.auth.AuthResult; import google.registry.request.auth.UserAuthInfo; import google.registry.testing.AppEngineRule; import google.registry.testing.InjectRule; -import google.registry.ui.server.registrar.SessionUtils.AccessType; +import google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType; import java.util.logging.Level; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -45,9 +45,9 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** Unit tests for {@link SessionUtils}. */ +/** Unit tests for {@link AuthenticatedRegistrarAccessor}. */ @RunWith(JUnit4.class) -public class SessionUtilsTest { +public class AuthenticatedRegistrarAccessorTest { @Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().build(); @Rule public final InjectRule inject = new InjectRule(); @@ -56,8 +56,6 @@ public class SessionUtilsTest { private final HttpServletResponse rsp = mock(HttpServletResponse.class); private final TestLogHandler testLogHandler = new TestLogHandler(); - private SessionUtils sessionUtils; - 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); @@ -80,15 +78,13 @@ public class SessionUtilsTest { @Before public void before() { - LoggerConfig.getConfig(SessionUtils.class).addHandler(testLogHandler); - sessionUtils = new SessionUtils(); - sessionUtils.registryAdminClientId = ADMIN_CLIENT_ID; + LoggerConfig.getConfig(AuthenticatedRegistrarAccessor.class).addHandler(testLogHandler); persistResource(loadRegistrar(ADMIN_CLIENT_ID)); } @After public void after() { - LoggerConfig.getConfig(SessionUtils.class).removeHandler(testLogHandler); + LoggerConfig.getConfig(AuthenticatedRegistrarAccessor.class).removeHandler(testLogHandler); } private String formatMessage(String message, AuthResult authResult, String clientId) { @@ -97,14 +93,78 @@ public class SessionUtilsTest { .replace("{clientId}", String.valueOf(clientId)); } + /** Users only have access to the registrars they are a contact for. */ + @Test + public void getAllClientIdWithAccess_authorizedUser() { + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(AUTHORIZED_USER, ADMIN_CLIENT_ID); + + assertThat(registrarAccessor.getAllClientIdWithAccess()) + .containsExactly( + UPDATE, DEFAULT_CLIENT_ID, + READ, DEFAULT_CLIENT_ID) + .inOrder(); + } + + /** Logged out users don't have access to anything. */ + @Test + public void getAllClientIdWithAccess_loggedOutUser() { + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(NO_USER, ADMIN_CLIENT_ID); + + ForbiddenException exception = + assertThrows(ForbiddenException.class, () -> registrarAccessor.getAllClientIdWithAccess()); + assertThat(exception).hasMessageThat().contains("Not logged in"); + } + + /** Unauthorized users don't have access to anything. */ + @Test + public void getAllClientIdWithAccess_unauthorizedUser() { + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(UNAUTHORIZED_USER, ADMIN_CLIENT_ID); + + assertThat(registrarAccessor.getAllClientIdWithAccess()).isEmpty(); + } + + /** Admins have read/write access to the authorized registrars, AND the admin registrar. */ + @Test + public void getAllClientIdWithAccess_authorizedAdmin() { + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(AUTHORIZED_ADMIN, ADMIN_CLIENT_ID); + + assertThat(registrarAccessor.getAllClientIdWithAccess()) + .containsExactly( + UPDATE, DEFAULT_CLIENT_ID, + READ, DEFAULT_CLIENT_ID, + UPDATE, ADMIN_CLIENT_ID, + READ, ADMIN_CLIENT_ID) + .inOrder(); + } + + /** + * Unauthorized admins only have full access to the admin registrar, and read-only to the rest. + */ + @Test + public void getAllClientIdWithAccess_unauthorizedAdmin() { + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(UNAUTHORIZED_ADMIN, ADMIN_CLIENT_ID); + + assertThat(registrarAccessor.getAllClientIdWithAccess()) + .containsExactly( + UPDATE, ADMIN_CLIENT_ID, + READ, ADMIN_CLIENT_ID, + READ, DEFAULT_CLIENT_ID) + .inOrder(); + } + /** Fail loading registrar if user doesn't have access to it. */ @Test public void testGetRegistrarForUser_readOnly_noAccess_isNotAdmin() { expectGetRegistrarFailure( DEFAULT_CLIENT_ID, - READ_ONLY, + READ, UNAUTHORIZED_USER, - "User {user} doesn't have READ_ONLY access to registrar {clientId}"); + "User {user} doesn't have READ access to registrar {clientId}"); } /** Fail loading registrar if user doesn't have access to it. */ @@ -112,60 +172,56 @@ public class SessionUtilsTest { public void testGetRegistrarForUser_readWrite_noAccess_isNotAdmin() { expectGetRegistrarFailure( DEFAULT_CLIENT_ID, - READ_WRITE, + UPDATE, UNAUTHORIZED_USER, - "User {user} doesn't have READ_WRITE access to registrar {clientId}"); + "User {user} doesn't have UPDATE access to registrar {clientId}"); } /** Fail loading registrar if there's no user associated with the request. */ @Test public void testGetRegistrarForUser_readOnly_noUser() { - expectGetRegistrarFailure(DEFAULT_CLIENT_ID, READ_ONLY, NO_USER, "Not logged in"); + expectGetRegistrarFailure(DEFAULT_CLIENT_ID, READ, NO_USER, "Not logged in"); } /** Fail loading registrar if there's no user associated with the request. */ @Test public void testGetRegistrarForUser_readWrite_noUser() { - expectGetRegistrarFailure(DEFAULT_CLIENT_ID, READ_WRITE, NO_USER, "Not logged in"); - - assertAboutLogs().that(testLogHandler).hasNoLogsAtLevel(Level.INFO); + expectGetRegistrarFailure(DEFAULT_CLIENT_ID, UPDATE, NO_USER, "Not logged in"); } /** Succeed loading registrar in read-only mode if user has access to it. */ @Test public void testGetRegistrarForUser_readOnly_hasAccess_isNotAdmin() { expectGetRegistrarSuccess( - AUTHORIZED_USER, READ_ONLY, "User {user} has access to registrar {clientId}"); + AUTHORIZED_USER, READ, "User {user} has READ access to registrar {clientId}"); } /** Succeed loading registrar in read-write mode if user has access to it. */ @Test public void testGetRegistrarForUser_readWrite_hasAccess_isNotAdmin() { expectGetRegistrarSuccess( - AUTHORIZED_USER, READ_WRITE, "User {user} has access to registrar {clientId}"); + AUTHORIZED_USER, UPDATE, "User {user} has UPDATE access to registrar {clientId}"); } /** Succeed loading registrar in read-only mode if admin with access. */ @Test public void testGetRegistrarForUser_readOnly_hasAccess_isAdmin() { expectGetRegistrarSuccess( - AUTHORIZED_ADMIN, READ_ONLY, "User {user} has access to registrar {clientId}"); + AUTHORIZED_ADMIN, READ, "User {user} has READ access to registrar {clientId}"); } /** Succeed loading registrar in read-write mode if admin with access. */ @Test public void testGetRegistrarForUser_readWrite_hasAccess_isAdmin() { expectGetRegistrarSuccess( - AUTHORIZED_ADMIN, READ_WRITE, "User {user} has access to registrar {clientId}"); + AUTHORIZED_ADMIN, UPDATE, "User {user} has UPDATE access to registrar {clientId}"); } /** Succeed loading registrar for read-only when admin isn't on the approved contacts list. */ @Test public void testGetRegistrarForUser_readOnly_noAccess_isAdmin() { expectGetRegistrarSuccess( - UNAUTHORIZED_ADMIN, - READ_ONLY, - "Allowing admin {user} read-only access to registrar {clientId}."); + UNAUTHORIZED_ADMIN, READ, "User {user} has READ access to registrar {clientId}."); } /** Fail loading registrar for read-write when admin isn't on the approved contacts list. */ @@ -173,9 +229,9 @@ public class SessionUtilsTest { public void testGetRegistrarForUser_readWrite_noAccess_isAdmin() { expectGetRegistrarFailure( DEFAULT_CLIENT_ID, - READ_WRITE, + UPDATE, UNAUTHORIZED_ADMIN, - "User {user} doesn't have READ_WRITE access to registrar {clientId}"); + "User {user} doesn't have UPDATE access to registrar {clientId}"); } /** Fail loading registrar even if admin, if registrar doesn't exist. */ @@ -183,9 +239,9 @@ public class SessionUtilsTest { public void testGetRegistrarForUser_readOnly_doesntExist_isAdmin() { expectGetRegistrarFailure( "BadClientId", - READ_ONLY, + READ, AUTHORIZED_ADMIN, - "Registrar {clientId} not found"); + "User {user} doesn't have READ access to registrar {clientId}"); } /** Fail loading registrar even if admin, if registrar doesn't exist. */ @@ -193,15 +249,17 @@ public class SessionUtilsTest { public void testGetRegistrarForUser_readWrite_doesntExist_isAdmin() { expectGetRegistrarFailure( "BadClientId", - READ_WRITE, + UPDATE, AUTHORIZED_ADMIN, - "Registrar {clientId} not found"); + "User {user} doesn't have UPDATE access to registrar {clientId}"); } private void expectGetRegistrarSuccess( AuthResult authResult, AccessType accessType, String message) { - assertThat(sessionUtils.getRegistrarForUser(DEFAULT_CLIENT_ID, accessType, authResult)) - .isNotNull(); + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(authResult, ADMIN_CLIENT_ID); + + assertThat(registrarAccessor.getRegistrar(DEFAULT_CLIENT_ID, accessType)).isNotNull(); assertAboutLogs() .that(testLogHandler) .hasLogAtLevelWithMessage( @@ -210,22 +268,23 @@ public class SessionUtilsTest { private void expectGetRegistrarFailure( String clientId, AccessType accessType, AuthResult authResult, String message) { + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(authResult, ADMIN_CLIENT_ID); + ForbiddenException exception = assertThrows( - ForbiddenException.class, - () -> sessionUtils.getRegistrarForUser(clientId, accessType, authResult)); + ForbiddenException.class, () -> registrarAccessor.getRegistrar(clientId, accessType)); 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}."); + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(AUTHORIZED_USER, ADMIN_CLIENT_ID); + + assertThat(registrarAccessor.guessClientId()).isEqualTo(DEFAULT_CLIENT_ID); } /** If a user doesn't have access to any registrars, guess returns nothing. */ @@ -241,40 +300,32 @@ public class SessionUtilsTest { */ @Test public void testGuessClientIdForUser_hasAccess_isAdmin() { - expectGuessRegistrarSuccess( - AUTHORIZED_ADMIN, - DEFAULT_CLIENT_ID, - "Associating user {user} with found registrar {clientId}."); + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(AUTHORIZED_ADMIN, ADMIN_CLIENT_ID); + + assertThat(registrarAccessor.guessClientId()).isEqualTo(DEFAULT_CLIENT_ID); } /** 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}."); + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(UNAUTHORIZED_ADMIN, ADMIN_CLIENT_ID); + + assertThat(registrarAccessor.guessClientId()).isEqualTo(ADMIN_CLIENT_ID); } /** - * If an admin is not associated with a registrar and there is no configured adminClientId, we - * can't guess the clientId. + * If an admin is not associated with a registrar and there is no configured adminClientId, but + * since it's an admin - we have read-only access to everything - return one of the existing + * registrars. */ @Test public void testGuessClientIdForUser_noAccess_isAdmin_adminClientIdEmpty() { - sessionUtils.registryAdminClientId = ""; - expectGuessRegistrarFailure( - UNAUTHORIZED_ADMIN, "User {user} isn't associated with any registrar"); - assertAboutLogs() - .that(testLogHandler) - .hasLogAtLevelWithMessage( - Level.INFO, - formatMessage( - "Cannot associate admin user {user} with configured client Id." - + " ClientId is null or empty.", - UNAUTHORIZED_ADMIN, - null)); + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(UNAUTHORIZED_ADMIN, ""); + + assertThat(registrarAccessor.guessClientId()).isAnyOf(ADMIN_CLIENT_ID, DEFAULT_CLIENT_ID); } /** @@ -283,24 +334,18 @@ public class SessionUtilsTest { */ @Test 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}."); - } + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(UNAUTHORIZED_ADMIN, "NonexistentRegistrar"); - private void expectGuessRegistrarSuccess(AuthResult authResult, String clientId, String message) { - assertThat(sessionUtils.guessClientIdForUser(authResult)).isEqualTo(clientId); - assertAboutLogs() - .that(testLogHandler) - .hasLogAtLevelWithMessage(Level.INFO, formatMessage(message, authResult, clientId)); + assertThat(registrarAccessor.guessClientId()).isEqualTo("NonexistentRegistrar"); } private void expectGuessRegistrarFailure(AuthResult authResult, String message) { + AuthenticatedRegistrarAccessor registrarAccessor = + new AuthenticatedRegistrarAccessor(authResult, ADMIN_CLIENT_ID); + ForbiddenException exception = - assertThrows(ForbiddenException.class, () -> sessionUtils.guessClientIdForUser(authResult)); + assertThrows(ForbiddenException.class, () -> registrarAccessor.guessClientId()); assertThat(exception) .hasMessageThat() .contains(formatMessage(message, authResult, null)); @@ -311,6 +356,6 @@ public class SessionUtilsTest { new NullPointerTester() .setDefault(HttpServletRequest.class, req) .setDefault(HttpServletResponse.class, rsp) - .testAllPublicStaticMethods(SessionUtils.class); + .testAllPublicStaticMethods(AuthenticatedRegistrarAccessor.class); } } diff --git a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java index 9e329e7ad..3e5f80112 100644 --- a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java +++ b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java @@ -17,14 +17,15 @@ 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 google.registry.ui.server.registrar.SessionUtils.AccessType.READ_ONLY; +import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.READ; +import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.AccessType.UPDATE; import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; -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.collect.ImmutableSetMultimap; import com.google.common.net.MediaType; import google.registry.request.HttpException.ForbiddenException; import google.registry.request.auth.AuthLevel; @@ -53,7 +54,8 @@ public class ConsoleUiActionTest { .withUserService(UserInfo.create("marla.singer@example.com", "12345")) .build(); - private final SessionUtils sessionUtils = mock(SessionUtils.class); + private final AuthenticatedRegistrarAccessor registrarAccessor = + mock(AuthenticatedRegistrarAccessor.class); private final HttpServletRequest request = mock(HttpServletRequest.class); private final FakeResponse response = new FakeResponse(); private final ConsoleUiAction action = new ConsoleUiAction(); @@ -71,15 +73,25 @@ public class ConsoleUiActionTest { action.technicalDocsUrl = "http://example.com/technical-docs"; action.req = request; action.response = response; - action.sessionUtils = sessionUtils; + action.registrarAccessor = registrarAccessor; action.userService = UserServiceFactory.getUserService(); action.xsrfTokenManager = new XsrfTokenManager(new FakeClock(), action.userService); action.paramClientId = Optional.empty(); AuthResult authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false)); action.authResult = authResult; - when(sessionUtils.guessClientIdForUser(authResult)).thenReturn("TheRegistrar"); - when(sessionUtils.getRegistrarForUser("TheRegistrar", READ_ONLY, authResult)) + when(registrarAccessor.getRegistrar("TheRegistrar", READ)) .thenReturn(loadRegistrar("TheRegistrar")); + when(registrarAccessor.getAllClientIdWithAccess()) + .thenReturn( + ImmutableSetMultimap.of( + UPDATE, "TheRegistrar", + READ, "TheRegistrar", + UPDATE, "ReadWriteRegistrar", + READ, "ReadWriteRegistrar", + READ, "ReadOnlyRegistrar")); + when(registrarAccessor.guessClientId()).thenCallRealMethod(); + // Used for error message in guessClientId + registrarAccessor.authResult = authResult; } @Test @@ -116,8 +128,7 @@ public class ConsoleUiActionTest { @Test public void testUserDoesntHaveAccessToAnyRegistrar_showsWhoAreYouPage() { - when(sessionUtils.guessClientIdForUser(any(AuthResult.class))) - .thenThrow(new ForbiddenException("forbidden")); + when(registrarAccessor.getAllClientIdWithAccess()).thenReturn(ImmutableSetMultimap.of()); action.run(); assertThat(response.getPayload()).contains("

You need permission

"); assertThat(response.getPayload()).contains("not associated with Nomulus."); @@ -144,7 +155,7 @@ public class ConsoleUiActionTest { @Test public void testSettingClientId_notAllowed_showsNeedPermissionPage() { action.paramClientId = Optional.of("OtherClientId"); - when(sessionUtils.getRegistrarForUser("OtherClientId", READ_ONLY, action.authResult)) + when(registrarAccessor.getRegistrar("OtherClientId", READ)) .thenThrow(new ForbiddenException("forbidden")); action.run(); assertThat(response.getPayload()).contains("

You need permission

"); @@ -154,10 +165,18 @@ public class ConsoleUiActionTest { @Test public void testSettingClientId_allowed_showsRegistrarConsole() { action.paramClientId = Optional.of("OtherClientId"); - when(sessionUtils.getRegistrarForUser("OtherClientId", READ_ONLY, action.authResult)) + when(registrarAccessor.getRegistrar("OtherClientId", READ)) .thenReturn(loadRegistrar("TheRegistrar")); action.run(); assertThat(response.getPayload()).contains("Registrar Console"); assertThat(response.getPayload()).contains("reg-content-and-footer"); } + + @Test + public void testUserHasAccessAsTheRegistrar_showsClientIdChooser() { + action.run(); + assertThat(response.getPayload()).contains("