diff --git a/java/google/registry/module/frontend/FrontendRequestComponent.java b/java/google/registry/module/frontend/FrontendRequestComponent.java index 316da2b51..19c22964b 100644 --- a/java/google/registry/module/frontend/FrontendRequestComponent.java +++ b/java/google/registry/module/frontend/FrontendRequestComponent.java @@ -26,12 +26,14 @@ import google.registry.request.RequestComponentBuilder; import google.registry.request.RequestModule; import google.registry.request.RequestScope; import google.registry.ui.server.registrar.ConsoleUiAction; +import google.registry.ui.server.registrar.RegistrarConsoleModule; import google.registry.ui.server.registrar.RegistrarSettingsAction; /** Dagger component with per-request lifetime for "default" App Engine module. */ @RequestScope @Subcomponent( modules = { + RegistrarConsoleModule.class, DnsModule.class, EppTlsModule.class, RequestModule.class, diff --git a/java/google/registry/rdap/RdapActionBase.java b/java/google/registry/rdap/RdapActionBase.java index c08a71f78..6fe80ef49 100644 --- a/java/google/registry/rdap/RdapActionBase.java +++ b/java/google/registry/rdap/RdapActionBase.java @@ -18,6 +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.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.DomainNameUtils.canonicalizeDomainName; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; @@ -181,7 +182,7 @@ public abstract class RdapActionBase implements Runnable { 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); + sessionUtils.getRegistrarForUserCached(clientId, READ_ONLY, authResult); return Optional.of(clientId); } catch (Exception e) { logger.atWarning().withCause(e).log( diff --git a/java/google/registry/ui/js/registrar/console.js b/java/google/registry/ui/js/registrar/console.js index 2fc49b3f9..9c8d639da 100644 --- a/java/google/registry/ui/js/registrar/console.js +++ b/java/google/registry/ui/js/registrar/console.js @@ -160,7 +160,7 @@ registry.registrar.Console.prototype.changeNavStyle = function() { slashNdx = slashNdx == -1 ? hashToken.length : slashNdx; var regNavlist = goog.dom.getRequiredElement('reg-navlist'); var path = hashToken.substring(0, slashNdx); - var active = regNavlist.querySelector('a[href="/registrar#' + path + '"]'); + var active = regNavlist.querySelector('a[href="#' + path + '"]'); if (goog.isNull(active)) { registry.util.log('Unknown path or path form in changeNavStyle.'); return; diff --git a/java/google/registry/ui/server/registrar/ConsoleUiAction.java b/java/google/registry/ui/server/registrar/ConsoleUiAction.java index 11dbf510c..c3d682a3f 100644 --- a/java/google/registry/ui/server/registrar/ConsoleUiAction.java +++ b/java/google/registry/ui/server/registrar/ConsoleUiAction.java @@ -16,6 +16,8 @@ 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.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; @@ -34,12 +36,14 @@ 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.Parameter; import google.registry.request.Response; import google.registry.request.auth.Auth; import google.registry.request.auth.AuthResult; import google.registry.security.XsrfTokenManager; import google.registry.ui.server.SoyTemplateUtils; import google.registry.ui.soy.registrar.ConsoleSoyInfo; +import java.util.Optional; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; @@ -79,6 +83,7 @@ public final class ConsoleUiAction implements Runnable { @Inject @Config("supportPhoneNumber") String supportPhoneNumber; @Inject @Config("technicalDocsUrl") String technicalDocsUrl; @Inject @Config("registrarConsoleEnabled") boolean enabled; + @Inject @Parameter(PARAM_CLIENT_ID) Optional paramClientId; @Inject ConsoleUiAction() {} @Override @@ -126,7 +131,8 @@ public final class ConsoleUiAction implements Runnable { data.put("logoutUrl", userService.createLogoutURL(PATH)); data.put("xsrfToken", xsrfTokenManager.generateToken(user.getEmail())); try { - String clientId = sessionUtils.guessClientIdForUser(authResult); + String clientId = + paramClientId.orElseGet(() -> 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. @@ -135,7 +141,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, authResult); + Registrar registrar = sessionUtils.getRegistrarForUser(clientId, READ_ONLY, authResult); data.put("requireFeeExtension", registrar.getPremiumPriceAckRequired()); } catch (ForbiddenException e) { logger.atWarning().withCause(e).log( diff --git a/java/google/registry/ui/server/registrar/RegistrarConsoleModule.java b/java/google/registry/ui/server/registrar/RegistrarConsoleModule.java new file mode 100644 index 000000000..9026c5ce5 --- /dev/null +++ b/java/google/registry/ui/server/registrar/RegistrarConsoleModule.java @@ -0,0 +1,37 @@ +// 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.request.RequestParameters.extractOptionalParameter; + +import dagger.Module; +import dagger.Provides; +import google.registry.request.Parameter; +import java.util.Optional; +import javax.servlet.http.HttpServletRequest; + +/** Dagger module for the Registrar Console parameters. */ +@Module +public final class RegistrarConsoleModule { + + static final String PARAM_CLIENT_ID = "clientId"; + + @Provides + @Parameter(PARAM_CLIENT_ID) + static Optional provideClientId(HttpServletRequest req) { + return extractOptionalParameter(req, PARAM_CLIENT_ID); + } +} diff --git a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index 39c90aa31..cbd054243 100644 --- a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -20,6 +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 com.google.common.base.Strings; import com.google.common.collect.HashMultimap; @@ -132,7 +134,9 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA Map read(String clientId) { return JsonResponseHelper.create( - SUCCESS, "Success", sessionUtils.getRegistrarForUser(clientId, authResult).toJsonMap()); + SUCCESS, + "Success", + sessionUtils.getRegistrarForUser(clientId, READ_ONLY, authResult).toJsonMap()); } Map update(final Map args, String clientId) { @@ -142,7 +146,8 @@ 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, authResult); + Registrar registrar = + sessionUtils.getRegistrarForUser(clientId, READ_WRITE, 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 4b8c13907..57a63048f 100644 --- a/java/google/registry/ui/server/registrar/SessionUtils.java +++ b/java/google/registry/ui/server/registrar/SessionUtils.java @@ -30,7 +30,7 @@ import java.util.function.Function; import javax.annotation.concurrent.Immutable; import javax.inject.Inject; -/** Authenticated Registrar access helper class. */ +/** HTTP session management helper class. */ @Immutable public class SessionUtils { @@ -43,6 +43,9 @@ public class SessionUtils { @Inject public SessionUtils() {} + /** Type of access we're requesting. */ + public enum AccessType {READ_ONLY, READ_WRITE} + /** * Loads Registrar on behalf of an authorised user. * @@ -50,10 +53,13 @@ public class SessionUtils { * 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, AuthResult authResult) { - return getAndAuthorize(Registrar::loadByClientId, clientId, authResult); + public Registrar getRegistrarForUser( + String clientId, AccessType accessType, AuthResult authResult) { + return getAndAuthorize(Registrar::loadByClientId, clientId, accessType, authResult); } /** @@ -63,15 +69,19 @@ public class SessionUtils { * 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, AuthResult authResult) { - return getAndAuthorize(Registrar::loadByClientIdCached, clientId, authResult); + public Registrar getRegistrarForUserCached( + String clientId, AccessType accessType, AuthResult authResult) { + return getAndAuthorize(Registrar::loadByClientIdCached, clientId, accessType, authResult); } - Registrar getAndAuthorize( + private Registrar getAndAuthorize( Function> registrarLoader, String clientId, + AccessType accessType, AuthResult authResult) { UserAuthInfo userAuthInfo = authResult.userAuthInfo().orElseThrow(() -> new ForbiddenException("Not logged in")); @@ -97,8 +107,17 @@ public class SessionUtils { 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 access to registrar %s", userIdForLogging, clientId)); + String.format( + "User %s doesn't have %s access to registrar %s", + userIdForLogging, accessType, clientId)); } /** diff --git a/java/google/registry/ui/soy/registrar/Console.soy b/java/google/registry/ui/soy/registrar/Console.soy index 860aba56a..7beb32b87 100644 --- a/java/google/registry/ui/soy/registrar/Console.soy +++ b/java/google/registry/ui/soy/registrar/Console.soy @@ -78,21 +78,21 @@
{/template} @@ -130,19 +130,29 @@ {@param logoutUrl: string} /** Generated URL for logging out of Google. */ {@param logoFilename: string} {@param productName: string} + {@param? clientId: string} {call registry.soy.console.header} {param app: 'registrar' /} - {param subtitle: 'Please Login' /} + {param subtitle: 'Not Authorized' /} {/call}
{$productName}

You need permission

-

- The account you are logged in as is not associated with {$productName}. - Please contact your customer service representative or - switch to an account associated with {$productName}. + {if isNonnull($clientId)} // A clientId was given - but we don't have access to it +

+ The account you are logged in as is not associated with the registrar + {sp}{$clientId}. Please contact your customer service representative or + switch to an account associated with {$clientId}. Alternatively, click + {sp}here to find a registrar associated with your + account. + {else} +

+ The account you are logged in as is not associated with {$productName}. + Please contact your customer service representative or + switch to an account associated with {$productName}. + {/if}

You are signed in as {$username}.

diff --git a/javatests/google/registry/ui/js/registrar/console_test.js b/javatests/google/registry/ui/js/registrar/console_test.js index fd3a346e4..1ba16afa8 100644 --- a/javatests/google/registry/ui/js/registrar/console_test.js +++ b/javatests/google/registry/ui/js/registrar/console_test.js @@ -61,7 +61,7 @@ function setUp() { }); registry.registrar.ConsoleTestUtil.setup(test); var regNavlist = $('reg-navlist'); - var active = regNavlist.querySelector('a[href="/registrar#contact-us"]'); + var active = regNavlist.querySelector('a[href="#contact-us"]'); assertTrue(active != null); } diff --git a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java index c259af5df..9e329e7ad 100644 --- a/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java +++ b/javatests/google/registry/ui/server/registrar/ConsoleUiActionTest.java @@ -17,6 +17,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 google.registry.ui.server.registrar.SessionUtils.AccessType.READ_ONLY; import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; @@ -34,6 +35,7 @@ import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.UserInfo; +import java.util.Optional; import javax.servlet.http.HttpServletRequest; import org.junit.Before; import org.junit.Rule; @@ -72,10 +74,11 @@ public class ConsoleUiActionTest { action.sessionUtils = sessionUtils; 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", authResult)) + when(sessionUtils.getRegistrarForUser("TheRegistrar", READ_ONLY, authResult)) .thenReturn(loadRegistrar("TheRegistrar")); } @@ -117,6 +120,7 @@ public class ConsoleUiActionTest { .thenThrow(new ForbiddenException("forbidden")); action.run(); assertThat(response.getPayload()).contains("

You need permission

"); + assertThat(response.getPayload()).contains("not associated with Nomulus."); } @Test @@ -136,4 +140,24 @@ public class ConsoleUiActionTest { assertThat(response.getStatus()).isEqualTo(SC_MOVED_TEMPORARILY); assertThat(response.getHeaders().get(LOCATION)).isEqualTo("/"); } + + @Test + public void testSettingClientId_notAllowed_showsNeedPermissionPage() { + action.paramClientId = Optional.of("OtherClientId"); + when(sessionUtils.getRegistrarForUser("OtherClientId", READ_ONLY, action.authResult)) + .thenThrow(new ForbiddenException("forbidden")); + action.run(); + assertThat(response.getPayload()).contains("

You need permission

"); + assertThat(response.getPayload()).contains("not associated with the registrar OtherClientId."); + } + + @Test + public void testSettingClientId_allowed_showsRegistrarConsole() { + action.paramClientId = Optional.of("OtherClientId"); + when(sessionUtils.getRegistrarForUser("OtherClientId", READ_ONLY, action.authResult)) + .thenReturn(loadRegistrar("TheRegistrar")); + action.run(); + assertThat(response.getPayload()).contains("Registrar Console"); + assertThat(response.getPayload()).contains("reg-content-and-footer"); + } } diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java index 4ffd12324..596a551c1 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java @@ -92,7 +92,19 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase /** This is the default read test for the registrar settings actions. */ @Test - public void testSuccess_readRegistrarInfo_authorized() { + public void testSuccess_readRegistrarInfo_authorizedReadWrite() { + Map response = action.handleJsonRequest(ImmutableMap.of("id", CLIENT_ID)); + assertThat(response) + .containsExactly( + "status", "SUCCESS", + "message", "Success", + "results", asList(loadRegistrar(CLIENT_ID).toJsonMap())); + } + + /** This is the default read test for the registrar settings actions. */ + @Test + public void testSuccess_readRegistrarInfo_authorizedReadOnly() { + action.authResult = USER_READ_ONLY; Map response = action.handleJsonRequest(ImmutableMap.of("id", CLIENT_ID)); assertThat(response) .containsExactly( @@ -144,7 +156,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase } @Test - public void testFailute_updateRegistrarInfo_notAuthorized() { + public void testFailure_updateRegistrarInfo_notAuthorized() { action.authResult = USER_UNAUTHORIZED; Map response = action.handleJsonRequest(ImmutableMap.of( "op", "update", @@ -157,6 +169,20 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase assertNoTasksEnqueued("sheet"); } + @Test + public void testFailure_updateRegistrarInfo_readOnlyAccess() { + action.authResult = USER_READ_ONLY; + Map response = action.handleJsonRequest(ImmutableMap.of( + "op", "update", + "id", CLIENT_ID, + "args", ImmutableMap.of("lastUpdateTime", getLastUpdateTime()))); + assertThat(response).containsExactly( + "status", "ERROR", + "results", ImmutableList.of(), + "message", "forbidden test error"); + assertNoTasksEnqueued("sheet"); + } + @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 627c884e8..8f5c0b7b9 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -18,6 +18,8 @@ import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailAddres import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailDisplayName; import static google.registry.security.JsonHttpTestUtils.createJsonPayload; 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.SessionUtils.AccessType.READ_WRITE; import static org.mockito.Mockito.when; import com.google.appengine.api.users.User; @@ -58,12 +60,14 @@ 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_READ_WRITE = + AuthResult.create(AuthLevel.USER, UserAuthInfo.create(new User("rw", "gmail.com"), false)); + + static final AuthResult USER_READ_ONLY = + AuthResult.create(AuthLevel.USER, UserAuthInfo.create(new User("ro", "gmail.com"), false)); static final AuthResult USER_UNAUTHORIZED = - AuthResult.create( - AuthLevel.USER, UserAuthInfo.create(new User("unauthorized", "gmail.com"), false)); + AuthResult.create(AuthLevel.USER, UserAuthInfo.create(new User("evil", "gmail.com"), false)); @Rule public final AppEngineRule appEngine = @@ -90,7 +94,9 @@ public class RegistrarSettingsActionTestCase { action.sessionUtils = sessionUtils; action.appEngineServiceUtils = appEngineServiceUtils; when(appEngineServiceUtils.getCurrentVersionHostname("backend")).thenReturn("backend.hostname"); - action.authResult = USER_AUTHORIZED; + // We set the default user to one with read-write access, as that's the most common test case. + // When we want to specifically check read-only or unauthorized, we can switch the user here. + action.authResult = USER_READ_WRITE; action.jsonActionRunner = new JsonActionRunner( ImmutableMap.of(), new JsonResponse(new ResponseImpl(rsp))); action.registrarChangesNotificationEmailAddresses = ImmutableList.of( @@ -109,9 +115,19 @@ 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.getRegistrarForUser(CLIENT_ID, USER_AUTHORIZED)) + when(sessionUtils.getRegistrarForUser(CLIENT_ID, READ_ONLY, USER_READ_WRITE)) .thenAnswer(x -> loadRegistrar(CLIENT_ID)); - when(sessionUtils.getRegistrarForUser(CLIENT_ID, USER_UNAUTHORIZED)) + when(sessionUtils.getRegistrarForUser(CLIENT_ID, READ_WRITE, USER_READ_WRITE)) + .thenAnswer(x -> loadRegistrar(CLIENT_ID)); + + when(sessionUtils.getRegistrarForUser(CLIENT_ID, READ_ONLY, USER_READ_ONLY)) + .thenAnswer(x -> loadRegistrar(CLIENT_ID)); + when(sessionUtils.getRegistrarForUser(CLIENT_ID, READ_WRITE, USER_READ_ONLY)) + .thenThrow(new ForbiddenException("forbidden test error")); + + when(sessionUtils.getRegistrarForUser(CLIENT_ID, READ_ONLY, USER_UNAUTHORIZED)) + .thenThrow(new ForbiddenException("forbidden test error")); + when(sessionUtils.getRegistrarForUser(CLIENT_ID, READ_WRITE, USER_UNAUTHORIZED)) .thenThrow(new ForbiddenException("forbidden test error")); } } diff --git a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java b/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java index 22af3a5eb..86dce17ad 100644 --- a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java +++ b/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java @@ -20,6 +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 org.mockito.Mockito.mock; import com.google.appengine.api.users.User; @@ -32,6 +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 java.util.logging.Level; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -96,66 +99,121 @@ public class SessionUtilsTest { /** Fail loading registrar if user doesn't have access to it. */ @Test - public void testGetRegistrarForUser_noAccess_isNotAdmin() { + public void testGetRegistrarForUser_readOnly_noAccess_isNotAdmin() { expectGetRegistrarFailure( DEFAULT_CLIENT_ID, + READ_ONLY, UNAUTHORIZED_USER, - "User {user} doesn't have access to registrar {clientId}"); + "User {user} doesn't have READ_ONLY access to registrar {clientId}"); + } + + /** Fail loading registrar if user doesn't have access to it. */ + @Test + public void testGetRegistrarForUser_readWrite_noAccess_isNotAdmin() { + expectGetRegistrarFailure( + DEFAULT_CLIENT_ID, + READ_WRITE, + UNAUTHORIZED_USER, + "User {user} doesn't have READ_WRITE access to registrar {clientId}"); } /** Fail loading registrar if there's no user associated with the request. */ @Test - public void testGetRegistrarForUser_noUser() { - expectGetRegistrarFailure(DEFAULT_CLIENT_ID, NO_USER, "Not logged in"); + public void testGetRegistrarForUser_readOnly_noUser() { + expectGetRegistrarFailure(DEFAULT_CLIENT_ID, READ_ONLY, NO_USER, "Not logged in"); } - /** Succeed loading registrar if user has access to it. */ + /** Fail loading registrar if there's no user associated with the request. */ @Test - public void testGetRegistrarForUser_hasAccess_isNotAdmin() { - expectGetRegistrarSuccess(AUTHORIZED_USER, "User {user} has access to registrar {clientId}"); + public void testGetRegistrarForUser_readWrite_noUser() { + expectGetRegistrarFailure(DEFAULT_CLIENT_ID, READ_WRITE, NO_USER, "Not logged in"); + + assertAboutLogs().that(testLogHandler).hasNoLogsAtLevel(Level.INFO); } - /** Succeed loading registrar if admin. */ + /** Succeed loading registrar in read-only mode if user has access to it. */ @Test - public void testGetRegistrarForUser_hasAccess_isAdmin() { - expectGetRegistrarSuccess(AUTHORIZED_ADMIN, "User {user} has access to registrar {clientId}"); + public void testGetRegistrarForUser_readOnly_hasAccess_isNotAdmin() { + expectGetRegistrarSuccess( + AUTHORIZED_USER, READ_ONLY, "User {user} has access to registrar {clientId}"); } - /** Fail loading registrar if admin isn't on the approved contacts list. */ + /** Succeed loading registrar in read-write mode if user has access to it. */ @Test - public void testGetRegistrarForUser_noAccess_isAdmin() { + public void testGetRegistrarForUser_readWrite_hasAccess_isNotAdmin() { + expectGetRegistrarSuccess( + AUTHORIZED_USER, READ_WRITE, "User {user} has 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}"); + } + + /** 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}"); + } + + /** 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}."); + } + + /** Fail loading registrar for read-write when admin isn't on the approved contacts list. */ + @Test + public void testGetRegistrarForUser_readWrite_noAccess_isAdmin() { expectGetRegistrarFailure( DEFAULT_CLIENT_ID, + READ_WRITE, 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}."); + "User {user} doesn't have READ_WRITE 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"); + public void testGetRegistrarForUser_readOnly_doesntExist_isAdmin() { + expectGetRegistrarFailure( + "BadClientId", + READ_ONLY, + AUTHORIZED_ADMIN, + "Registrar {clientId} not found"); } - private void expectGetRegistrarSuccess(AuthResult authResult, String message) { - assertThat(sessionUtils.getRegistrarForUser(DEFAULT_CLIENT_ID, authResult)).isNotNull(); + /** Fail loading registrar even if admin, if registrar doesn't exist. */ + @Test + public void testGetRegistrarForUser_readWrite_doesntExist_isAdmin() { + expectGetRegistrarFailure( + "BadClientId", + READ_WRITE, + AUTHORIZED_ADMIN, + "Registrar {clientId} not found"); + } + + private void expectGetRegistrarSuccess( + AuthResult authResult, AccessType accessType, String message) { + assertThat(sessionUtils.getRegistrarForUser(DEFAULT_CLIENT_ID, accessType, authResult)) + .isNotNull(); assertAboutLogs() .that(testLogHandler) .hasLogAtLevelWithMessage( Level.INFO, formatMessage(message, authResult, DEFAULT_CLIENT_ID)); } - private void expectGetRegistrarFailure(String clientId, AuthResult authResult, String message) { + private void expectGetRegistrarFailure( + String clientId, AccessType accessType, AuthResult authResult, String message) { ForbiddenException exception = assertThrows( - ForbiddenException.class, () -> sessionUtils.getRegistrarForUser(clientId, authResult)); + ForbiddenException.class, + () -> sessionUtils.getRegistrarForUser(clientId, accessType, authResult)); assertThat(exception).hasMessageThat().contains(formatMessage(message, authResult, clientId)); assertAboutLogs().that(testLogHandler).hasNoLogsAtLevel(Level.INFO); @@ -221,7 +279,7 @@ public class SessionUtilsTest { /** * 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). + * non-existent registrar, we still guess it (we will later fail loading the registrar). */ @Test public void testGuessClientIdForUser_noAccess_isAdmin_adminClientIdInvalid() {