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); } }