From e38778b4d0bce9edd11a758258c3809f2b75b475 Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Thu, 9 Oct 2014 08:31:34 -0400 Subject: [PATCH 1/8] Added tests to UserREST.login --- .../frontend/resource/UserRestTest.java | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 src/test/java/com/commafeed/frontend/resource/UserRestTest.java diff --git a/src/test/java/com/commafeed/frontend/resource/UserRestTest.java b/src/test/java/com/commafeed/frontend/resource/UserRestTest.java new file mode 100644 index 00000000..337b6398 --- /dev/null +++ b/src/test/java/com/commafeed/frontend/resource/UserRestTest.java @@ -0,0 +1,64 @@ +package com.commafeed.frontend.resource; + +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +import javax.servlet.http.HttpSession; + +import org.junit.Test; + +import com.commafeed.backend.model.User; +import com.commafeed.backend.service.UserService; +import com.commafeed.frontend.model.request.LoginRequest; +import com.google.common.base.Optional; + +public class UserRestTest { + + @Test public void + login_should_not_populate_http_session_if_unsuccessfull() { + // Absent user + Optional absentUser = Optional.absent(); + + // Create UserService partial mock + UserService service = mock(UserService.class); + when(service.login("user", "password")).thenReturn(absentUser); + when(service.login(any(String.class), any(String.class), any(HttpSession.class))).thenCallRealMethod(); + + HttpSession session = mock(HttpSession.class); + UserREST userREST = new UserREST(null, null, null, service, null, null, null); + + LoginRequest req = new LoginRequest(); + req.setName("user"); + req.setPassword("password"); + + userREST.login(req, session); + + verifyZeroInteractions(session); + } + + @Test public void + login_should_populate_http_session_if_successfull() { + // Create a user + User user = new User(); + + // Create UserService partial mock + UserService service = mock(UserService.class); + when(service.login("user", "password")).thenReturn(Optional.of(user)); + when(service.login(any(String.class), any(String.class), any(HttpSession.class))).thenCallRealMethod(); + + HttpSession session = mock(HttpSession.class); + UserREST userREST = new UserREST(null, null, null, service, null, null, null); + + LoginRequest req = new LoginRequest(); + req.setName("user"); + req.setPassword("password"); + + userREST.login(req, session); + + verify(session).setAttribute("user", user); + } + +} From 54cc265ee62978f707c40d7bf15d263c3c2a4ee4 Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Thu, 9 Oct 2014 08:38:50 -0400 Subject: [PATCH 2/8] Refactored UserREST login to populate session itself --- .../java/com/commafeed/backend/service/UserService.java | 7 +++---- .../java/com/commafeed/frontend/resource/UserREST.java | 4 +++- .../java/com/commafeed/frontend/resource/UserRestTest.java | 3 --- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/commafeed/backend/service/UserService.java b/src/main/java/com/commafeed/backend/service/UserService.java index 93b79eff..892320b6 100644 --- a/src/main/java/com/commafeed/backend/service/UserService.java +++ b/src/main/java/com/commafeed/backend/service/UserService.java @@ -21,6 +21,7 @@ import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserRole; import com.commafeed.backend.model.UserRole.Role; import com.commafeed.backend.service.internal.PostLoginActivities; +import com.commafeed.frontend.resource.UserREST; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -28,8 +29,6 @@ import com.google.common.base.Preconditions; @Singleton public class UserService { - private static final String SESSION_KEY_USER = "user"; - private final FeedCategoryDAO feedCategoryDAO; private final UserDAO userDAO; private final UserSettingsDAO userSettingsDAO; @@ -67,7 +66,7 @@ public class UserService { public Optional login(String nameOrEmail, String password, HttpSession sessionToFill) { Optional user = login(nameOrEmail, password); if (user.isPresent()) { - sessionToFill.setAttribute(SESSION_KEY_USER, user.get()); + sessionToFill.setAttribute(UserREST.SESSION_KEY_USER, user.get()); } return user; } @@ -77,7 +76,7 @@ public class UserService { */ public Optional login(HttpSession session) { if (session != null) { - User user = (User) session.getAttribute(SESSION_KEY_USER); + User user = (User) session.getAttribute(UserREST.SESSION_KEY_USER); if (user != null) { afterLogin(user); return Optional.of(user); diff --git a/src/main/java/com/commafeed/frontend/resource/UserREST.java b/src/main/java/com/commafeed/frontend/resource/UserREST.java index 2dc0c807..19f9ad59 100644 --- a/src/main/java/com/commafeed/frontend/resource/UserREST.java +++ b/src/main/java/com/commafeed/frontend/resource/UserREST.java @@ -79,6 +79,7 @@ public class UserREST { private final PasswordEncryptionService encryptionService; private final MailService mailService; private final CommaFeedConfiguration config; + public static final String SESSION_KEY_USER = "user"; @Path("/settings") @GET @@ -243,8 +244,9 @@ public class UserREST { @UnitOfWork @ApiOperation(value = "Login and create a session") public Response login(@ApiParam(required = true) LoginRequest req, @Session HttpSession session) { - Optional user = userService.login(req.getName(), req.getPassword(), session); + Optional user = userService.login(req.getName(), req.getPassword()); if (user.isPresent()) { + session.setAttribute(SESSION_KEY_USER, user.get()); return Response.ok().build(); } else { return Response.status(Response.Status.UNAUTHORIZED).entity("wrong username or password").build(); diff --git a/src/test/java/com/commafeed/frontend/resource/UserRestTest.java b/src/test/java/com/commafeed/frontend/resource/UserRestTest.java index 337b6398..1289b92a 100644 --- a/src/test/java/com/commafeed/frontend/resource/UserRestTest.java +++ b/src/test/java/com/commafeed/frontend/resource/UserRestTest.java @@ -1,6 +1,5 @@ package com.commafeed.frontend.resource; -import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; @@ -25,7 +24,6 @@ public class UserRestTest { // Create UserService partial mock UserService service = mock(UserService.class); when(service.login("user", "password")).thenReturn(absentUser); - when(service.login(any(String.class), any(String.class), any(HttpSession.class))).thenCallRealMethod(); HttpSession session = mock(HttpSession.class); UserREST userREST = new UserREST(null, null, null, service, null, null, null); @@ -47,7 +45,6 @@ public class UserRestTest { // Create UserService partial mock UserService service = mock(UserService.class); when(service.login("user", "password")).thenReturn(Optional.of(user)); - when(service.login(any(String.class), any(String.class), any(HttpSession.class))).thenCallRealMethod(); HttpSession session = mock(HttpSession.class); UserREST userREST = new UserREST(null, null, null, service, null, null, null); From 326ee79c8cef6dc1cbba30367c9f96d9e2edebbd Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Thu, 9 Oct 2014 20:53:38 -0400 Subject: [PATCH 3/8] Remove HttpSession dependency in UserService phase 1 complete --- .../backend/service/UserService.java | 11 ---- .../commafeed/frontend/resource/UserREST.java | 5 +- .../frontend/resource/UserRestTest.java | 56 ++++++++++++++++++- 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/commafeed/backend/service/UserService.java b/src/main/java/com/commafeed/backend/service/UserService.java index 892320b6..4acaae20 100644 --- a/src/main/java/com/commafeed/backend/service/UserService.java +++ b/src/main/java/com/commafeed/backend/service/UserService.java @@ -60,17 +60,6 @@ public class UserService { return Optional.absent(); } - /** - * try to log in with given credentials and create a session for the user - */ - public Optional login(String nameOrEmail, String password, HttpSession sessionToFill) { - Optional user = login(nameOrEmail, password); - if (user.isPresent()) { - sessionToFill.setAttribute(UserREST.SESSION_KEY_USER, user.get()); - } - return user; - } - /** * try to log in by checking if the user has an active session */ diff --git a/src/main/java/com/commafeed/frontend/resource/UserREST.java b/src/main/java/com/commafeed/frontend/resource/UserREST.java index 19f9ad59..f572626f 100644 --- a/src/main/java/com/commafeed/frontend/resource/UserREST.java +++ b/src/main/java/com/commafeed/frontend/resource/UserREST.java @@ -226,8 +226,9 @@ public class UserREST { @ApiOperation(value = "Register a new account") public Response register(@Valid @ApiParam(required = true) RegistrationRequest req, @Session HttpSession session) { try { - userService.register(req.getName(), req.getPassword(), req.getEmail(), Arrays.asList(Role.USER)); - userService.login(req.getName(), req.getPassword(), session); + User registeredUser = userService.register(req.getName(), req.getPassword(), req.getEmail(), Arrays.asList(Role.USER)); + userService.login(req.getName(), req.getPassword()); + session.setAttribute(SESSION_KEY_USER, registeredUser); return Response.ok().build(); } catch (final IllegalArgumentException e) { return Response.status(422).entity(new ValidationErrorMessage(Collections.> emptySet()) { diff --git a/src/test/java/com/commafeed/frontend/resource/UserRestTest.java b/src/test/java/com/commafeed/frontend/resource/UserRestTest.java index 1289b92a..f43c6a43 100644 --- a/src/test/java/com/commafeed/frontend/resource/UserRestTest.java +++ b/src/test/java/com/commafeed/frontend/resource/UserRestTest.java @@ -1,17 +1,25 @@ package com.commafeed.frontend.resource; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; +import java.util.Arrays; + import javax.servlet.http.HttpSession; import org.junit.Test; +import org.mockito.InOrder; +import org.mockito.Matchers; import com.commafeed.backend.model.User; +import com.commafeed.backend.model.UserRole.Role; import com.commafeed.backend.service.UserService; import com.commafeed.frontend.model.request.LoginRequest; +import com.commafeed.frontend.model.request.RegistrationRequest; import com.google.common.base.Optional; public class UserRestTest { @@ -42,7 +50,7 @@ public class UserRestTest { // Create a user User user = new User(); - // Create UserService partial mock + // Create UserService mock UserService service = mock(UserService.class); when(service.login("user", "password")).thenReturn(Optional.of(user)); @@ -55,7 +63,51 @@ public class UserRestTest { userREST.login(req, session); - verify(session).setAttribute("user", user); + verify(session).setAttribute(UserREST.SESSION_KEY_USER, user); + } + + @Test public void + register_should_register_and_then_login() { + // Create UserService mock + UserService service = mock(UserService.class); + + RegistrationRequest req = new RegistrationRequest(); + req.setName("user"); + req.setPassword("password"); + req.setEmail("test@test.com"); + + HttpSession session = mock(HttpSession.class); + + InOrder inOrder = inOrder(service); + + UserREST userREST = new UserREST(null, null, null, service, null, null, null); + userREST.register(req, session); + + inOrder.verify(service).register("user", "password", "test@test.com", Arrays.asList(Role.USER)); + inOrder.verify(service).login("user", "password"); + } + + @Test public void + register_should_populate_http_session() { + // Create a user + User user = new User(); + + // Create UserService partial mock + UserService service = mock(UserService.class); + when(service.register(any(String.class), any(String.class), any(String.class), Matchers.anyListOf(Role.class))).thenReturn(user); + when(service.login(any(String.class), any(String.class))).thenReturn(Optional.of(user)); + + RegistrationRequest req = new RegistrationRequest(); + req.setName("user"); + req.setPassword("password"); + req.setEmail("test@test.com"); + + HttpSession session = mock(HttpSession.class); + + UserREST userREST = new UserREST(null, null, null, service, null, null, null); + userREST.register(req, session); + + verify(session).setAttribute(UserREST.SESSION_KEY_USER, user); } } From 0059cabebe78d97dfd0dcf377a2cf36a7544cb33 Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Sat, 11 Oct 2014 13:18:09 -0400 Subject: [PATCH 4/8] Cover SecurityCheckProvider.SecurityCheckInjectable.cookieLogin with tests --- .../backend/service/UserService.java | 2 +- .../frontend/auth/SecurityCheckProvider.java | 4 +- .../auth/SecurityCheckInjectableTest.java | 103 ++++++++++++++++++ 3 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java diff --git a/src/main/java/com/commafeed/backend/service/UserService.java b/src/main/java/com/commafeed/backend/service/UserService.java index 4acaae20..40a309d8 100644 --- a/src/main/java/com/commafeed/backend/service/UserService.java +++ b/src/main/java/com/commafeed/backend/service/UserService.java @@ -54,7 +54,7 @@ public class UserService { boolean authenticated = encryptionService.authenticate(password, user.getPassword(), user.getSalt()); if (authenticated) { afterLogin(user); - return Optional.fromNullable(user); + return Optional.of(user); } } return Optional.absent(); diff --git a/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java b/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java index 3c9c055e..a7d95185 100644 --- a/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java +++ b/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java @@ -35,7 +35,7 @@ public class SecurityCheckProvider implements InjectableProvider extends AbstractHttpContextInjectable { + static class SecurityCheckInjectable extends AbstractHttpContextInjectable { private static final String PREFIX = "Basic"; private final HttpServletRequest request; @@ -66,7 +66,7 @@ public class SecurityCheckProvider implements InjectableProvider cookieSessionLogin() { + Optional cookieSessionLogin() { return userService.login(request.getSession(false)); } diff --git a/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java b/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java new file mode 100644 index 00000000..4113632e --- /dev/null +++ b/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java @@ -0,0 +1,103 @@ +package com.commafeed.frontend.auth; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; + +import org.junit.Assert; +import org.junit.Test; + +import com.commafeed.backend.model.User; +import com.commafeed.backend.service.UserService; +import com.commafeed.backend.service.internal.PostLoginActivities; +import com.commafeed.frontend.auth.SecurityCheckProvider.SecurityCheckInjectable; +import com.commafeed.frontend.resource.UserREST; +import com.google.common.base.Optional; + +public class SecurityCheckInjectableTest { + + @Test public void + cookie_login_does_not_create_a_session_if_not_present() { + HttpServletRequest request = mock(HttpServletRequest.class); + UserService service = mock(UserService.class); + + SecurityCheckInjectable injectable = new SecurityCheckInjectable(request, service, null, false); + injectable.cookieSessionLogin(); + + verify(request).getSession(false); + } + + @Test public void + cookie_login_should_not_return_user_if_there_is_no_preexisting_http_session() { + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getSession(false)).thenReturn(null); + + UserService service = new UserService(null, null, null, null, null, null); + + SecurityCheckInjectable injectable = new SecurityCheckInjectable(request, service, null, false); + Optional user = injectable.cookieSessionLogin(); + + Assert.assertFalse(user.isPresent()); + } + + @Test public void + cookie_login_should_not_return_user_if_user_not_present_in_http_session() { + HttpSession session = mock(HttpSession.class); + when(session.getAttribute(UserREST.SESSION_KEY_USER)).thenReturn(null); + + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getSession(false)).thenReturn(session); + + UserService service = new UserService(null, null, null, null, null, null); + + SecurityCheckInjectable injectable = new SecurityCheckInjectable(request, service, null, false); + Optional user = injectable.cookieSessionLogin(); + + Assert.assertFalse(user.isPresent()); + } + + @Test public void + cookie_login_should_perform_post_login_activities_if_user_present_in_http_session() { + User userInSession = new User(); + + HttpSession session = mock(HttpSession.class); + when(session.getAttribute(UserREST.SESSION_KEY_USER)).thenReturn(userInSession); + + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getSession(false)).thenReturn(session); + + PostLoginActivities postLoginActivities = mock(PostLoginActivities.class); + + UserService service = new UserService(null, null, null, null, null, postLoginActivities); + + SecurityCheckInjectable injectable = new SecurityCheckInjectable(request, service, null, false); + Optional user = injectable.cookieSessionLogin(); + + verify(postLoginActivities).executeFor(userInSession); + } + + @Test public void + calling_login_should_return_user_if_user_present_in_http_session() { + User userInSession = new User(); + + HttpSession session = mock(HttpSession.class); + when(session.getAttribute(UserREST.SESSION_KEY_USER)).thenReturn(userInSession); + + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getSession(false)).thenReturn(session); + + PostLoginActivities postLoginActivities = mock(PostLoginActivities.class); + + UserService service = new UserService(null, null, null, null, null, postLoginActivities); + + SecurityCheckInjectable injectable = new SecurityCheckInjectable(request, service, null, false); + Optional user = injectable.cookieSessionLogin(); + + Assert.assertTrue(user.isPresent()); + Assert.assertEquals(userInSession, user.get()); + } + +} From b9f27b2b00666c2f8ee635b3d46132342e69af3d Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Sat, 11 Oct 2014 13:24:12 -0400 Subject: [PATCH 5/8] Make cookieLogin handle HttpSession by itself --- .../com/commafeed/backend/service/UserService.java | 2 +- .../frontend/auth/SecurityCheckProvider.java | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/commafeed/backend/service/UserService.java b/src/main/java/com/commafeed/backend/service/UserService.java index 40a309d8..b146dfd1 100644 --- a/src/main/java/com/commafeed/backend/service/UserService.java +++ b/src/main/java/com/commafeed/backend/service/UserService.java @@ -93,7 +93,7 @@ public class UserService { /** * should triggers after successful login */ - private void afterLogin(User user) { + public void afterLogin(User user) { postLoginActivities.executeFor(user); } diff --git a/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java b/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java index a7d95185..df81142c 100644 --- a/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java +++ b/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java @@ -1,6 +1,7 @@ package com.commafeed.frontend.auth; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Context; import javax.ws.rs.core.HttpHeaders; @@ -15,6 +16,7 @@ import org.eclipse.jetty.util.StringUtil; import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserRole.Role; import com.commafeed.backend.service.UserService; +import com.commafeed.frontend.resource.UserREST; import com.google.common.base.Optional; import com.sun.jersey.api.core.HttpContext; import com.sun.jersey.api.model.Parameter; @@ -67,7 +69,15 @@ public class SecurityCheckProvider implements InjectableProvider cookieSessionLogin() { - return userService.login(request.getSession(false)); + HttpSession session = request.getSession(false); + if (session != null) { + User user = (User) session.getAttribute(UserREST.SESSION_KEY_USER); + if (user != null) { + userService.afterLogin(user); + return Optional.of(user); + } + } + return Optional.absent(); } private Optional basicAuthenticationLogin(HttpContext c) { From ce95772afa4e5e405dc9689303abee2d9265e20b Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Sat, 11 Oct 2014 13:29:29 -0400 Subject: [PATCH 6/8] Delete method UserService.login(HttpSession) and copy body to callers --- .../commafeed/backend/service/UserService.java | 18 +----------------- .../frontend/servlet/CustomCssServlet.java | 12 +++++++++++- .../frontend/servlet/NextUnreadServlet.java | 12 +++++++++++- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/commafeed/backend/service/UserService.java b/src/main/java/com/commafeed/backend/service/UserService.java index b146dfd1..3bab6522 100644 --- a/src/main/java/com/commafeed/backend/service/UserService.java +++ b/src/main/java/com/commafeed/backend/service/UserService.java @@ -6,7 +6,6 @@ import java.util.UUID; import javax.inject.Inject; import javax.inject.Singleton; -import javax.servlet.http.HttpSession; import lombok.RequiredArgsConstructor; @@ -21,7 +20,6 @@ import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserRole; import com.commafeed.backend.model.UserRole.Role; import com.commafeed.backend.service.internal.PostLoginActivities; -import com.commafeed.frontend.resource.UserREST; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -58,21 +56,7 @@ public class UserService { } } return Optional.absent(); - } - - /** - * try to log in by checking if the user has an active session - */ - public Optional login(HttpSession session) { - if (session != null) { - User user = (User) session.getAttribute(UserREST.SESSION_KEY_USER); - if (user != null) { - afterLogin(user); - return Optional.of(user); - } - } - return Optional.absent(); - } + } /** * try to log in with given api key diff --git a/src/main/java/com/commafeed/frontend/servlet/CustomCssServlet.java b/src/main/java/com/commafeed/frontend/servlet/CustomCssServlet.java index f65e17cc..9c0f55b2 100644 --- a/src/main/java/com/commafeed/frontend/servlet/CustomCssServlet.java +++ b/src/main/java/com/commafeed/frontend/servlet/CustomCssServlet.java @@ -8,6 +8,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; import lombok.RequiredArgsConstructor; @@ -18,6 +19,7 @@ import com.commafeed.backend.dao.UserSettingsDAO; import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserSettings; import com.commafeed.backend.service.UserService; +import com.commafeed.frontend.resource.UserREST; import com.google.common.base.Optional; @SuppressWarnings("serial") @@ -36,7 +38,15 @@ public class CustomCssServlet extends HttpServlet { final Optional user = new UnitOfWork>(sessionFactory) { @Override protected Optional runInSession() throws Exception { - return userService.login(req.getSession(false)); + HttpSession session = req.getSession(false); + if (session != null) { + User user = (User) session.getAttribute(UserREST.SESSION_KEY_USER); + if (user != null) { + userService.afterLogin(user); + return Optional.of(user); + } + } + return Optional.absent(); } }.run(); if (!user.isPresent()) { diff --git a/src/main/java/com/commafeed/frontend/servlet/NextUnreadServlet.java b/src/main/java/com/commafeed/frontend/servlet/NextUnreadServlet.java index affebcce..e8fe6131 100644 --- a/src/main/java/com/commafeed/frontend/servlet/NextUnreadServlet.java +++ b/src/main/java/com/commafeed/frontend/servlet/NextUnreadServlet.java @@ -9,6 +9,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; import lombok.RequiredArgsConstructor; @@ -27,6 +28,7 @@ import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserSettings.ReadingOrder; import com.commafeed.backend.service.UserService; import com.commafeed.frontend.resource.CategoryREST; +import com.commafeed.frontend.resource.UserREST; import com.google.common.base.Optional; import com.google.common.collect.Iterables; @@ -53,7 +55,15 @@ public class NextUnreadServlet extends HttpServlet { final Optional user = new UnitOfWork>(sessionFactory) { @Override protected Optional runInSession() throws Exception { - return userService.login(req.getSession(false)); + HttpSession session = req.getSession(false); + if (session != null) { + User user = (User) session.getAttribute(UserREST.SESSION_KEY_USER); + if (user != null) { + userService.afterLogin(user); + return Optional.of(user); + } + } + return Optional.absent(); } }.run(); if (!user.isPresent()) { From 8d5c3bdec8e630ae72efb75d6bc45f50553d1753 Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Sat, 11 Oct 2014 13:37:11 -0400 Subject: [PATCH 7/8] Rename method --- .../commafeed/frontend/auth/SecurityCheckInjectableTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java b/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java index 4113632e..f56add79 100644 --- a/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java +++ b/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java @@ -80,7 +80,7 @@ public class SecurityCheckInjectableTest { } @Test public void - calling_login_should_return_user_if_user_present_in_http_session() { + cookie_login_should_return_user_if_user_present_in_http_session() { User userInSession = new User(); HttpSession session = mock(HttpSession.class); From 12030f6ce96b2a3d08cf2908414abe0e9a56fa30 Mon Sep 17 00:00:00 2001 From: Sankaranarayanan Viswanathan Date: Wed, 22 Oct 2014 01:17:33 -0400 Subject: [PATCH 8/8] Provide a SessionHelper to manage the session --- .../com/commafeed/CommaFeedApplication.java | 4 +- .../com/commafeed/backend/model/User.java | 9 ++++ .../backend/service/UserService.java | 8 ++-- .../service/internal/PostLoginActivities.java | 7 ++- .../com/commafeed/frontend/SessionHelper.java | 39 ++++++++++++++++ .../frontend/SessionHelperProvider.java | 44 +++++++++++++++++++ .../frontend/auth/SecurityCheckProvider.java | 16 +++---- .../commafeed/frontend/resource/UserREST.java | 13 +++--- .../frontend/servlet/CustomCssServlet.java | 16 +++---- .../frontend/servlet/NextUnreadServlet.java | 16 +++---- .../auth/SecurityCheckInjectableTest.java | 9 ++-- .../frontend/resource/UserRestTest.java | 37 ++++++++-------- 12 files changed, 148 insertions(+), 70 deletions(-) create mode 100644 src/main/java/com/commafeed/frontend/SessionHelper.java create mode 100644 src/main/java/com/commafeed/frontend/SessionHelperProvider.java diff --git a/src/main/java/com/commafeed/CommaFeedApplication.java b/src/main/java/com/commafeed/CommaFeedApplication.java index 68a8cdd7..db904e89 100644 --- a/src/main/java/com/commafeed/CommaFeedApplication.java +++ b/src/main/java/com/commafeed/CommaFeedApplication.java @@ -4,7 +4,6 @@ import io.dropwizard.Application; import io.dropwizard.assets.AssetsBundle; import io.dropwizard.db.DataSourceFactory; import io.dropwizard.hibernate.HibernateBundle; -import io.dropwizard.jersey.sessions.HttpSessionProvider; import io.dropwizard.migrations.MigrationsBundle; import io.dropwizard.servlets.CacheBustingFilter; import io.dropwizard.setup.Bootstrap; @@ -36,6 +35,7 @@ import com.commafeed.backend.service.UserService; import com.commafeed.backend.task.OldStatusesCleanupTask; import com.commafeed.backend.task.OrphansCleanupTask; import com.commafeed.backend.task.SchedulingService; +import com.commafeed.frontend.SessionHelperProvider; import com.commafeed.frontend.auth.SecurityCheckProvider; import com.commafeed.frontend.auth.SecurityCheckProvider.SecurityCheckUserServiceProvider; import com.commafeed.frontend.resource.AdminREST; @@ -108,7 +108,7 @@ public class CommaFeedApplication extends Application { environment.servlets().setSessionHandler(new SessionHandler(config.getSessionManagerFactory().build())); environment.jersey().register(new SecurityCheckUserServiceProvider(injector.getInstance(UserService.class))); environment.jersey().register(SecurityCheckProvider.class); - environment.jersey().register(HttpSessionProvider.class); + environment.jersey().register(SessionHelperProvider.class); // REST resources environment.jersey().setUrlPattern("/rest/*"); diff --git a/src/main/java/com/commafeed/backend/model/User.java b/src/main/java/com/commafeed/backend/model/User.java index e1fe1394..ddd3fab5 100644 --- a/src/main/java/com/commafeed/backend/model/User.java +++ b/src/main/java/com/commafeed/backend/model/User.java @@ -15,6 +15,7 @@ import javax.persistence.TemporalType; import lombok.Getter; import lombok.Setter; +import org.apache.commons.lang.time.DateUtils; import org.hibernate.annotations.Cascade; import com.commafeed.backend.model.UserRole.Role; @@ -77,5 +78,13 @@ public class User extends AbstractModel { } return false; } + + public boolean shouldRefreshFeedsAt(Date when) { + return (lastFullRefresh == null || lastFullRefreshMoreThan30MinutesBefore(when)); + } + + private boolean lastFullRefreshMoreThan30MinutesBefore(Date when) { + return lastFullRefresh.before(DateUtils.addMinutes(when, -30)); + } } diff --git a/src/main/java/com/commafeed/backend/service/UserService.java b/src/main/java/com/commafeed/backend/service/UserService.java index 3bab6522..747de4fb 100644 --- a/src/main/java/com/commafeed/backend/service/UserService.java +++ b/src/main/java/com/commafeed/backend/service/UserService.java @@ -51,7 +51,7 @@ public class UserService { if (user != null && !user.isDisabled()) { boolean authenticated = encryptionService.authenticate(password, user.getPassword(), user.getSalt()); if (authenticated) { - afterLogin(user); + performPostLoginActivities(user); return Optional.of(user); } } @@ -68,8 +68,8 @@ public class UserService { User user = userDAO.findByApiKey(apiKey); if (user != null && !user.isDisabled()) { - afterLogin(user); - return Optional.fromNullable(user); + performPostLoginActivities(user); + return Optional.of(user); } return Optional.absent(); } @@ -77,7 +77,7 @@ public class UserService { /** * should triggers after successful login */ - public void afterLogin(User user) { + public void performPostLoginActivities(User user) { postLoginActivities.executeFor(user); } diff --git a/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java b/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java index 92f0d3e7..0466cb99 100644 --- a/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java +++ b/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java @@ -28,16 +28,15 @@ public class PostLoginActivities { boolean saveUser = false; // only update lastLogin field every hour in order to not - // invalidate the cache everytime someone logs in + // invalidate the cache every time someone logs in if (lastLogin == null || lastLogin.before(DateUtils.addHours(now, -1))) { user.setLastLogin(now); saveUser = true; } - if (config.getApplicationSettings().isHeavyLoad() - && (user.getLastFullRefresh() == null || user.getLastFullRefresh().before(DateUtils.addMinutes(now, -30)))) { + if (config.getApplicationSettings().isHeavyLoad() && user.shouldRefreshFeedsAt(now)) { + feedSubscriptionService.refreshAll(user); user.setLastFullRefresh(now); saveUser = true; - feedSubscriptionService.refreshAll(user); } if (saveUser) { userDAO.merge(user); diff --git a/src/main/java/com/commafeed/frontend/SessionHelper.java b/src/main/java/com/commafeed/frontend/SessionHelper.java new file mode 100644 index 00000000..021d0e44 --- /dev/null +++ b/src/main/java/com/commafeed/frontend/SessionHelper.java @@ -0,0 +1,39 @@ +package com.commafeed.frontend; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; + +import lombok.RequiredArgsConstructor; + +import com.commafeed.backend.model.User; +import com.google.common.base.Optional; + +@RequiredArgsConstructor() +public class SessionHelper { + + private static final String SESSION_KEY_USER = "user"; + + private final HttpServletRequest request; + + public Optional getLoggedInUser() { + Optional session = getSession(false); + + if (session.isPresent()) { + User user = (User) session.get().getAttribute(SESSION_KEY_USER); + return Optional.fromNullable(user); + } + + return Optional.absent(); + } + + public void setLoggedInUser(User user) { + Optional session = getSession(true); + session.get().setAttribute(SESSION_KEY_USER, user); + } + + private Optional getSession(boolean force) { + HttpSession session = request.getSession(force); + return Optional.fromNullable(session); + } + +} diff --git a/src/main/java/com/commafeed/frontend/SessionHelperProvider.java b/src/main/java/com/commafeed/frontend/SessionHelperProvider.java new file mode 100644 index 00000000..b77a6d5d --- /dev/null +++ b/src/main/java/com/commafeed/frontend/SessionHelperProvider.java @@ -0,0 +1,44 @@ +package com.commafeed.frontend; + +import java.lang.reflect.Type; + +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.core.Context; +import javax.ws.rs.ext.Provider; + +import com.sun.jersey.core.spi.component.ComponentContext; +import com.sun.jersey.core.spi.component.ComponentScope; +import com.sun.jersey.spi.inject.Injectable; +import com.sun.jersey.spi.inject.InjectableProvider; + +@Provider +public class SessionHelperProvider implements InjectableProvider { + + private final ThreadLocal request; + + public SessionHelperProvider(@Context ThreadLocal request) { + this.request = request; + } + + @Override + public ComponentScope getScope() { + return ComponentScope.PerRequest; + } + + @Override + public Injectable getInjectable(ComponentContext ic, final Context session, Type type) { + if (type.equals(SessionHelper.class)) { + return new Injectable() { + @Override + public SessionHelper getValue() { + final HttpServletRequest req = request.get(); + if (req != null) { + return new SessionHelper(req); + } + return null; + } + }; + } + return null; + } +} \ No newline at end of file diff --git a/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java b/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java index df81142c..ab1ba238 100644 --- a/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java +++ b/src/main/java/com/commafeed/frontend/auth/SecurityCheckProvider.java @@ -1,7 +1,6 @@ package com.commafeed.frontend.auth; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpSession; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Context; import javax.ws.rs.core.HttpHeaders; @@ -16,7 +15,7 @@ import org.eclipse.jetty.util.StringUtil; import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserRole.Role; import com.commafeed.backend.service.UserService; -import com.commafeed.frontend.resource.UserREST; +import com.commafeed.frontend.SessionHelper; import com.google.common.base.Optional; import com.sun.jersey.api.core.HttpContext; import com.sun.jersey.api.model.Parameter; @@ -69,15 +68,12 @@ public class SecurityCheckProvider implements InjectableProvider cookieSessionLogin() { - HttpSession session = request.getSession(false); - if (session != null) { - User user = (User) session.getAttribute(UserREST.SESSION_KEY_USER); - if (user != null) { - userService.afterLogin(user); - return Optional.of(user); - } + SessionHelper sessionHelper = new SessionHelper(request); + Optional loggedInUser = sessionHelper.getLoggedInUser(); + if (loggedInUser.isPresent()) { + userService.performPostLoginActivities(loggedInUser.get()); } - return Optional.absent(); + return loggedInUser; } private Optional basicAuthenticationLogin(HttpContext c) { diff --git a/src/main/java/com/commafeed/frontend/resource/UserREST.java b/src/main/java/com/commafeed/frontend/resource/UserREST.java index f572626f..4865e7de 100644 --- a/src/main/java/com/commafeed/frontend/resource/UserREST.java +++ b/src/main/java/com/commafeed/frontend/resource/UserREST.java @@ -1,7 +1,6 @@ package com.commafeed.frontend.resource; import io.dropwizard.hibernate.UnitOfWork; -import io.dropwizard.jersey.sessions.Session; import io.dropwizard.jersey.validation.ValidationErrorMessage; import java.util.Arrays; @@ -11,7 +10,6 @@ import java.util.UUID; import javax.inject.Inject; import javax.inject.Singleton; -import javax.servlet.http.HttpSession; import javax.validation.ConstraintViolation; import javax.validation.Valid; import javax.ws.rs.Consumes; @@ -20,6 +18,7 @@ import javax.ws.rs.POST; import javax.ws.rs.Path; import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; +import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; @@ -49,6 +48,7 @@ import com.commafeed.backend.model.UserSettings.ViewMode; import com.commafeed.backend.service.MailService; import com.commafeed.backend.service.PasswordEncryptionService; import com.commafeed.backend.service.UserService; +import com.commafeed.frontend.SessionHelper; import com.commafeed.frontend.auth.SecurityCheck; import com.commafeed.frontend.model.Settings; import com.commafeed.frontend.model.UserModel; @@ -79,7 +79,6 @@ public class UserREST { private final PasswordEncryptionService encryptionService; private final MailService mailService; private final CommaFeedConfiguration config; - public static final String SESSION_KEY_USER = "user"; @Path("/settings") @GET @@ -224,11 +223,11 @@ public class UserREST { @POST @UnitOfWork @ApiOperation(value = "Register a new account") - public Response register(@Valid @ApiParam(required = true) RegistrationRequest req, @Session HttpSession session) { + public Response register(@Valid @ApiParam(required = true) RegistrationRequest req, @Context SessionHelper sessionHelper) { try { User registeredUser = userService.register(req.getName(), req.getPassword(), req.getEmail(), Arrays.asList(Role.USER)); userService.login(req.getName(), req.getPassword()); - session.setAttribute(SESSION_KEY_USER, registeredUser); + sessionHelper.setLoggedInUser(registeredUser); return Response.ok().build(); } catch (final IllegalArgumentException e) { return Response.status(422).entity(new ValidationErrorMessage(Collections.> emptySet()) { @@ -244,10 +243,10 @@ public class UserREST { @POST @UnitOfWork @ApiOperation(value = "Login and create a session") - public Response login(@ApiParam(required = true) LoginRequest req, @Session HttpSession session) { + public Response login(@ApiParam(required = true) LoginRequest req, @Context SessionHelper sessionHelper) { Optional user = userService.login(req.getName(), req.getPassword()); if (user.isPresent()) { - session.setAttribute(SESSION_KEY_USER, user.get()); + sessionHelper.setLoggedInUser(user.get()); return Response.ok().build(); } else { return Response.status(Response.Status.UNAUTHORIZED).entity("wrong username or password").build(); diff --git a/src/main/java/com/commafeed/frontend/servlet/CustomCssServlet.java b/src/main/java/com/commafeed/frontend/servlet/CustomCssServlet.java index 9c0f55b2..2f5f4b44 100644 --- a/src/main/java/com/commafeed/frontend/servlet/CustomCssServlet.java +++ b/src/main/java/com/commafeed/frontend/servlet/CustomCssServlet.java @@ -8,7 +8,6 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; import lombok.RequiredArgsConstructor; @@ -19,7 +18,7 @@ import com.commafeed.backend.dao.UserSettingsDAO; import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserSettings; import com.commafeed.backend.service.UserService; -import com.commafeed.frontend.resource.UserREST; +import com.commafeed.frontend.SessionHelper; import com.google.common.base.Optional; @SuppressWarnings("serial") @@ -38,15 +37,12 @@ public class CustomCssServlet extends HttpServlet { final Optional user = new UnitOfWork>(sessionFactory) { @Override protected Optional runInSession() throws Exception { - HttpSession session = req.getSession(false); - if (session != null) { - User user = (User) session.getAttribute(UserREST.SESSION_KEY_USER); - if (user != null) { - userService.afterLogin(user); - return Optional.of(user); - } + SessionHelper sessionHelper = new SessionHelper(req); + Optional loggedInUser = sessionHelper.getLoggedInUser(); + if (loggedInUser.isPresent()) { + userService.performPostLoginActivities(loggedInUser.get()); } - return Optional.absent(); + return loggedInUser; } }.run(); if (!user.isPresent()) { diff --git a/src/main/java/com/commafeed/frontend/servlet/NextUnreadServlet.java b/src/main/java/com/commafeed/frontend/servlet/NextUnreadServlet.java index e8fe6131..0e4fed98 100644 --- a/src/main/java/com/commafeed/frontend/servlet/NextUnreadServlet.java +++ b/src/main/java/com/commafeed/frontend/servlet/NextUnreadServlet.java @@ -9,7 +9,6 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; import lombok.RequiredArgsConstructor; @@ -27,8 +26,8 @@ import com.commafeed.backend.model.FeedSubscription; import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserSettings.ReadingOrder; import com.commafeed.backend.service.UserService; +import com.commafeed.frontend.SessionHelper; import com.commafeed.frontend.resource.CategoryREST; -import com.commafeed.frontend.resource.UserREST; import com.google.common.base.Optional; import com.google.common.collect.Iterables; @@ -55,15 +54,12 @@ public class NextUnreadServlet extends HttpServlet { final Optional user = new UnitOfWork>(sessionFactory) { @Override protected Optional runInSession() throws Exception { - HttpSession session = req.getSession(false); - if (session != null) { - User user = (User) session.getAttribute(UserREST.SESSION_KEY_USER); - if (user != null) { - userService.afterLogin(user); - return Optional.of(user); - } + SessionHelper sessionHelper = new SessionHelper(req); + Optional loggedInUser = sessionHelper.getLoggedInUser(); + if (loggedInUser.isPresent()) { + userService.performPostLoginActivities(loggedInUser.get()); } - return Optional.absent(); + return loggedInUser; } }.run(); if (!user.isPresent()) { diff --git a/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java b/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java index f56add79..417d7183 100644 --- a/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java +++ b/src/test/java/com/commafeed/frontend/auth/SecurityCheckInjectableTest.java @@ -14,11 +14,12 @@ import com.commafeed.backend.model.User; import com.commafeed.backend.service.UserService; import com.commafeed.backend.service.internal.PostLoginActivities; import com.commafeed.frontend.auth.SecurityCheckProvider.SecurityCheckInjectable; -import com.commafeed.frontend.resource.UserREST; import com.google.common.base.Optional; public class SecurityCheckInjectableTest { + private static String SESSION_KEY_USER = "user"; + @Test public void cookie_login_does_not_create_a_session_if_not_present() { HttpServletRequest request = mock(HttpServletRequest.class); @@ -46,7 +47,7 @@ public class SecurityCheckInjectableTest { @Test public void cookie_login_should_not_return_user_if_user_not_present_in_http_session() { HttpSession session = mock(HttpSession.class); - when(session.getAttribute(UserREST.SESSION_KEY_USER)).thenReturn(null); + when(session.getAttribute(SESSION_KEY_USER)).thenReturn(null); HttpServletRequest request = mock(HttpServletRequest.class); when(request.getSession(false)).thenReturn(session); @@ -64,7 +65,7 @@ public class SecurityCheckInjectableTest { User userInSession = new User(); HttpSession session = mock(HttpSession.class); - when(session.getAttribute(UserREST.SESSION_KEY_USER)).thenReturn(userInSession); + when(session.getAttribute(SESSION_KEY_USER)).thenReturn(userInSession); HttpServletRequest request = mock(HttpServletRequest.class); when(request.getSession(false)).thenReturn(session); @@ -84,7 +85,7 @@ public class SecurityCheckInjectableTest { User userInSession = new User(); HttpSession session = mock(HttpSession.class); - when(session.getAttribute(UserREST.SESSION_KEY_USER)).thenReturn(userInSession); + when(session.getAttribute(SESSION_KEY_USER)).thenReturn(userInSession); HttpServletRequest request = mock(HttpServletRequest.class); when(request.getSession(false)).thenReturn(session); diff --git a/src/test/java/com/commafeed/frontend/resource/UserRestTest.java b/src/test/java/com/commafeed/frontend/resource/UserRestTest.java index f43c6a43..3b6a59ba 100644 --- a/src/test/java/com/commafeed/frontend/resource/UserRestTest.java +++ b/src/test/java/com/commafeed/frontend/resource/UserRestTest.java @@ -3,14 +3,12 @@ package com.commafeed.frontend.resource; import static org.mockito.Matchers.any; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import java.util.Arrays; -import javax.servlet.http.HttpSession; - import org.junit.Test; import org.mockito.InOrder; import org.mockito.Matchers; @@ -18,6 +16,7 @@ import org.mockito.Matchers; import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserRole.Role; import com.commafeed.backend.service.UserService; +import com.commafeed.frontend.SessionHelper; import com.commafeed.frontend.model.request.LoginRequest; import com.commafeed.frontend.model.request.RegistrationRequest; import com.google.common.base.Optional; @@ -33,16 +32,16 @@ public class UserRestTest { UserService service = mock(UserService.class); when(service.login("user", "password")).thenReturn(absentUser); - HttpSession session = mock(HttpSession.class); UserREST userREST = new UserREST(null, null, null, service, null, null, null); + SessionHelper sessionHelper = mock(SessionHelper.class); LoginRequest req = new LoginRequest(); req.setName("user"); req.setPassword("password"); - userREST.login(req, session); + userREST.login(req, sessionHelper); - verifyZeroInteractions(session); + verify(sessionHelper, never()).setLoggedInUser(any(User.class)); } @Test public void @@ -54,16 +53,16 @@ public class UserRestTest { UserService service = mock(UserService.class); when(service.login("user", "password")).thenReturn(Optional.of(user)); - HttpSession session = mock(HttpSession.class); - UserREST userREST = new UserREST(null, null, null, service, null, null, null); - LoginRequest req = new LoginRequest(); req.setName("user"); req.setPassword("password"); - userREST.login(req, session); + UserREST userREST = new UserREST(null, null, null, service, null, null, null); + SessionHelper sessionHelper = mock(SessionHelper.class); - verify(session).setAttribute(UserREST.SESSION_KEY_USER, user); + userREST.login(req, sessionHelper); + + verify(sessionHelper).setLoggedInUser(user); } @Test public void @@ -76,12 +75,12 @@ public class UserRestTest { req.setPassword("password"); req.setEmail("test@test.com"); - HttpSession session = mock(HttpSession.class); - InOrder inOrder = inOrder(service); + SessionHelper sessionHelper = mock(SessionHelper.class); UserREST userREST = new UserREST(null, null, null, service, null, null, null); - userREST.register(req, session); + + userREST.register(req, sessionHelper); inOrder.verify(service).register("user", "password", "test@test.com", Arrays.asList(Role.USER)); inOrder.verify(service).login("user", "password"); @@ -92,7 +91,7 @@ public class UserRestTest { // Create a user User user = new User(); - // Create UserService partial mock + // Create UserService mock UserService service = mock(UserService.class); when(service.register(any(String.class), any(String.class), any(String.class), Matchers.anyListOf(Role.class))).thenReturn(user); when(service.login(any(String.class), any(String.class))).thenReturn(Optional.of(user)); @@ -102,12 +101,12 @@ public class UserRestTest { req.setPassword("password"); req.setEmail("test@test.com"); - HttpSession session = mock(HttpSession.class); - + SessionHelper sessionHelper = mock(SessionHelper.class); UserREST userREST = new UserREST(null, null, null, service, null, null, null); - userREST.register(req, session); - verify(session).setAttribute(UserREST.SESSION_KEY_USER, user); + userREST.register(req, sessionHelper); + + verify(sessionHelper).setLoggedInUser(user); } }