diff --git a/java/google/registry/ui/server/registrar/SessionUtils.java b/java/google/registry/ui/server/registrar/SessionUtils.java index 2caa951ac..1be6e4ff3 100644 --- a/java/google/registry/ui/server/registrar/SessionUtils.java +++ b/java/google/registry/ui/server/registrar/SessionUtils.java @@ -17,9 +17,7 @@ package google.registry.ui.server.registrar; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Verify.verify; -import static com.google.common.net.HttpHeaders.LOCATION; import static google.registry.model.ofy.ObjectifyService.ofy; -import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; import com.google.appengine.api.users.User; import com.google.appengine.api.users.UserService; @@ -34,7 +32,6 @@ import javax.annotation.Nonnull; import javax.annotation.concurrent.Immutable; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; /** HTTP session management helper class. */ @@ -52,22 +49,6 @@ public class SessionUtils { this.userService = checkNotNull(userService); } - /** - * Redirects client to login URL if they aren't authenticated with the App Engine user service. - * - * @return {@code false} if request should abort. - */ - @CheckReturnValue - public boolean redirectIfNotLoggedIn(HttpServletRequest req, HttpServletResponse rsp) { - if (!isLoggedIn()) { - logger.info("User not logged in to App Engine UserService."); - rsp.setStatus(SC_MOVED_TEMPORARILY); - rsp.setHeader(LOCATION, userService.createLoginURL(req.getRequestURI())); - return false; - } - return true; - } - /** * Checks GAE user has access to Registrar Console. * @@ -82,7 +63,8 @@ public class SessionUtils { * wasn't revoked. This should only cost one memcache read. * * - *

Note: You should call {@link #redirectIfNotLoggedIn} before calling this method. + *

Note: You must ensure the user has logged in before calling this method, for example + * by setting {@code @Action(requireLogin = true)}. * * @return {@code false} if user does not have access, in which case the caller should write an * error response and abort the request. @@ -91,7 +73,7 @@ public class SessionUtils { public boolean checkRegistrarConsoleLogin(HttpServletRequest req) { HttpSession session = req.getSession(); User user = userService.getCurrentUser(); - checkState(user != null, "You forgot to call redirectIfNotLoggedIn()"); + checkState(user != null, "No logged in user found"); String clientId = (String) session.getAttribute(CLIENT_ID_ATTRIBUTE); if (clientId == null) { Optional registrar = guessRegistrar(user.getUserId()); diff --git a/javatests/google/registry/ui/server/registrar/RegistrarServletTestCase.java b/javatests/google/registry/ui/server/registrar/RegistrarServletTestCase.java index ed1d19f9c..69d2b919e 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarServletTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarServletTestCase.java @@ -101,7 +101,6 @@ public class RegistrarServletTestCase { when(req.getHeader(eq("X-CSRF-Token"))).thenReturn(generateToken("console")); when(req.getReader()).thenReturn(createJsonPayload(ImmutableMap.of("op", "read"))); when(sessionUtils.isLoggedIn()).thenReturn(true); - when(sessionUtils.redirectIfNotLoggedIn(req, rsp)).thenReturn(true); when(sessionUtils.checkRegistrarConsoleLogin(req)).thenReturn(true); when(sessionUtils.getRegistrarClientId(req)).thenReturn(CLIENT_ID); when(modulesService.getVersionHostname("backend", null)).thenReturn("backend.hostname"); diff --git a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java b/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java index fe7f3ba74..4ccad3392 100644 --- a/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java +++ b/javatests/google/registry/ui/server/registrar/SessionUtilsTest.java @@ -19,7 +19,6 @@ import static google.registry.testing.AppEngineRule.THE_REGISTRAR_GAE_USER_ID; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import com.google.appengine.api.users.User; @@ -77,24 +76,6 @@ public class SessionUtilsTest { when(req.getSession()).thenReturn(session); } - @Test - public void testRedirectIfNotLoggedIn_loggedIn_doesNothing() throws Exception { - when(userService.isUserLoggedIn()).thenReturn(true); - assertThat(sessionUtils.redirectIfNotLoggedIn(req, rsp)).isTrue(); - verifyZeroInteractions(req, rsp); - } - - @Test - public void testRedirectIfNotLoggedIn_notLoggedIn_sendsTemporaryRedirect() throws Exception { - when(userService.isUserLoggedIn()).thenReturn(false); - when(req.getRequestURI()).thenReturn("foo"); - when(userService.createLoginURL(eq("foo"))).thenReturn("bar"); - assertThat(sessionUtils.redirectIfNotLoggedIn(req, rsp)).isFalse(); - verify(rsp).setStatus(eq(302)); - verify(rsp).setHeader(eq("Location"), eq("bar")); - verifyNoMoreInteractions(rsp); - } - @Test public void testCheckRegistrarConsoleLogin_authedButNoSession_createsSession() throws Exception { when(userService.getCurrentUser()).thenReturn(jart);