hide session management inside UserService

src/main/java/com/commafeed/CommaFeedApplication.java

@@ -100,8 +100,6 @@ public class CommaFeedApplication extends Application<CommaFeedConfiguration> {
 	public static final String USERNAME_ADMIN = "admin";
 	public static final String USERNAME_DEMO = "demo";
 
-	public static final String SESSION_USER = "user";
-
 	public static final Date STARTUP_TIME = new Date();
 
 	private HibernateBundle<CommaFeedConfiguration> hibernateBundle;
@@ -221,9 +219,9 @@ public class CommaFeedApplication extends Application<CommaFeedConfiguration> {
 
 		// Servlets
 		NextUnreadServlet nextUnreadServlet = new NextUnreadServlet(sessionFactory, feedSubscriptionDAO, feedEntryStatusDAO,
-				feedCategoryDAO, config);
+				feedCategoryDAO, userService, config);
 		LogoutServlet logoutServlet = new LogoutServlet(config);
-		CustomCssServlet customCssServlet = new CustomCssServlet(sessionFactory, userSettingsDAO);
+		CustomCssServlet customCssServlet = new CustomCssServlet(sessionFactory, userSettingsDAO, userService);
 		AnalyticsServlet analyticsServlet = new AnalyticsServlet(config);
 		environment.servlets().addServlet("next", nextUnreadServlet).addMapping("/next");
 		environment.servlets().addServlet("logout", logoutServlet).addMapping("/logout");

src/main/java/com/commafeed/backend/service/UserService.java

@@ -4,6 +4,8 @@ import java.util.Collection;
 import java.util.Date;
 import java.util.UUID;
 
+import javax.servlet.http.HttpSession;
+
 import lombok.RequiredArgsConstructor;
 
 import org.apache.commons.codec.digest.DigestUtils;
@@ -23,6 +25,8 @@ import com.google.common.base.Preconditions;
 @RequiredArgsConstructor
 public class UserService {
 
+	private static final String SESSION_KEY_USER = "user";
+
 	private final FeedCategoryDAO feedCategoryDAO;
 	private final UserDAO userDAO;
 	private final UserSettingsDAO userSettingsDAO;
@@ -31,6 +35,9 @@ public class UserService {
 	private final PasswordEncryptionService encryptionService;
 	private final CommaFeedConfiguration config;
 
+	/**
+	 * try to log in with given credentials
+	 */
 	public Optional<User> login(String nameOrEmail, String password) {
 		if (nameOrEmail == null || password == null) {
 			return Optional.absent();
@@ -50,6 +57,32 @@ public class UserService {
 		return Optional.absent();
 	}
 
+	/**
+	 * try to log in with given credentials and create a session for the user
+	 */
+	public Optional<User> login(String nameOrEmail, String password, HttpSession sessionToFill) {
+		Optional<User> user = login(nameOrEmail, password);
+		if (user.isPresent()) {
+			sessionToFill.setAttribute(SESSION_KEY_USER, user.get());
+		}
+		return user;
+	}
+
+	/**
+	 * try to log in by checking if the user has an active session
+	 */
+	public Optional<User> login(HttpSession session) {
+		User user = (User) session.getAttribute(SESSION_KEY_USER);
+		if (user != null) {
+			afterLogin(user);
+			return Optional.of(user);
+		}
+		return Optional.absent();
+	}
+
+	/**
+	 * try to log in with given api key
+	 */
 	public Optional<User> login(String apiKey) {
 		if (apiKey == null) {
 			return Optional.absent();
@@ -63,7 +96,10 @@ public class UserService {
 		return Optional.absent();
 	}
 
-	public void afterLogin(User user) {
+	/**
+	 * should triggers after successful login
+	 */
+	private void afterLogin(User user) {
 		Date lastLogin = user.getLastLogin();
 		Date now = new Date();
 

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;
@@ -13,7 +12,6 @@ import lombok.RequiredArgsConstructor;
 import org.eclipse.jetty.util.B64Code;
 import org.eclipse.jetty.util.StringUtil;
 
-import com.commafeed.CommaFeedApplication;
 import com.commafeed.backend.model.User;
 import com.commafeed.backend.model.UserRole.Role;
 import com.commafeed.backend.service.UserService;
@@ -56,7 +54,12 @@ public class SecurityCheckProvider implements InjectableProvider<SecurityCheck,
 			}
 
 			if (user.isPresent()) {
-				return user.get();
+				if (user.get().hasRole(role)) {
+					return user.get();
+				} else {
+					throw new WebApplicationException(Response.status(Response.Status.FORBIDDEN)
+							.entity("You don't have the required role to access this resource.").type(MediaType.TEXT_PLAIN_TYPE).build());
+				}
 			} else {
 				throw new WebApplicationException(Response.status(Response.Status.UNAUTHORIZED)
 						.entity("Credentials are required to access this resource.").type(MediaType.TEXT_PLAIN_TYPE).build());
@@ -64,15 +67,7 @@ public class SecurityCheckProvider implements InjectableProvider<SecurityCheck,
 		}
 
 		private Optional<User> cookieSessionLogin() {
-			HttpSession session = request.getSession(false);
+			return userService.login(request.getSession());
-			if (session != null) {
-				User user = (User) session.getAttribute(CommaFeedApplication.SESSION_USER);
-				if (user != null) {
-					userService.afterLogin(user);
-					return Optional.of(user);
-				}
-			}
-			return Optional.absent();
 		}
 
 		private Optional<User> basicAuthenticationLogin(HttpContext c) {
@@ -87,10 +82,7 @@ public class SecurityCheckProvider implements InjectableProvider<SecurityCheck,
 						if (i > 0) {
 							String username = decoded.substring(0, i);
 							String password = decoded.substring(i + 1);
-							Optional<User> user = userService.login(username, password);
+							return userService.login(username, password);
-							if (user.isPresent() && user.get().hasRole(role)) {
-								return user;
-							}
 						}
 					}
 				}
@@ -101,10 +93,7 @@ public class SecurityCheckProvider implements InjectableProvider<SecurityCheck,
 		private Optional<User> apiKeyLogin(HttpContext c) {
 			String apiKey = c.getUriInfo().getPathParameters().getFirst("apiKey");
 			if (apiKey != null && apiKeyAllowed) {
-				Optional<User> user = userService.login(apiKey);
+				return userService.login(apiKey);
-				if (user.isPresent() && user.get().hasRole(role)) {
-					return user;
-				}
 			}
 			return Optional.absent();
 		}

src/main/java/com/commafeed/frontend/resource/UserREST.java

@@ -239,9 +239,8 @@ public class UserREST {
 	@UnitOfWork
 	@ApiOperation(value = "Login and create a session")
 	public Response login(@ApiParam(required = true) LoginRequest req, @Session HttpSession session) {
-		Optional<User> user = userService.login(req.getName(), req.getPassword());
+		Optional<User> user = userService.login(req.getName(), req.getPassword(), session);
 		if (user.isPresent()) {
-			session.setAttribute(CommaFeedApplication.SESSION_USER, user.get());
 			return Response.ok().build();
 		} else {
 			return Response.status(Response.Status.UNAUTHORIZED).entity("wrong username or password").build();

src/main/java/com/commafeed/frontend/servlet/CustomCssServlet.java

@@ -11,11 +11,12 @@ import lombok.RequiredArgsConstructor;
 
 import org.hibernate.SessionFactory;
 
-import com.commafeed.CommaFeedApplication;
 import com.commafeed.backend.dao.UnitOfWork;
 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.google.common.base.Optional;
 
 @SuppressWarnings("serial")
 @RequiredArgsConstructor
@@ -23,20 +24,21 @@ public class CustomCssServlet extends HttpServlet {
 
 	private final SessionFactory sessionFactory;
 	private final UserSettingsDAO userSettingsDAO;
+	private final UserService userService;
 
 	@Override
 	protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
 		resp.setContentType("text/css");
 
-		final User user = (User) req.getSession().getAttribute(CommaFeedApplication.SESSION_USER);
+		final Optional<User> user = userService.login(req.getSession());
-		if (user == null) {
+		if (!user.isPresent()) {
 			return;
 		}
 
 		UserSettings settings = new UnitOfWork<UserSettings>(sessionFactory) {
 			@Override
 			protected UserSettings runInSession() {
-				return userSettingsDAO.findByUser(user);
+				return userSettingsDAO.findByUser(user.get());
 			}
 		}.run();
 

src/main/java/com/commafeed/frontend/servlet/NextUnreadServlet.java

@@ -13,7 +13,6 @@ import lombok.RequiredArgsConstructor;
 import org.apache.commons.lang.StringUtils;
 import org.hibernate.SessionFactory;
 
-import com.commafeed.CommaFeedApplication;
 import com.commafeed.CommaFeedConfiguration;
 import com.commafeed.backend.dao.FeedCategoryDAO;
 import com.commafeed.backend.dao.FeedEntryStatusDAO;
@@ -24,7 +23,9 @@ import com.commafeed.backend.model.FeedEntryStatus;
 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.resource.CategoryREST;
+import com.google.common.base.Optional;
 import com.google.common.collect.Iterables;
 
 @SuppressWarnings("serial")
@@ -38,6 +39,7 @@ public class NextUnreadServlet extends HttpServlet {
 	private final FeedSubscriptionDAO feedSubscriptionDAO;
 	private final FeedEntryStatusDAO feedEntryStatusDAO;
 	private final FeedCategoryDAO feedCategoryDAO;
+	private final UserService userService;
 	private final CommaFeedConfiguration config;
 
 	@Override
@@ -45,8 +47,8 @@ public class NextUnreadServlet extends HttpServlet {
 		final String categoryId = req.getParameter(PARAM_CATEGORYID);
 		String orderParam = req.getParameter(PARAM_READINGORDER);
 
-		final User user = (User) req.getSession().getAttribute(CommaFeedApplication.SESSION_USER);
+		final Optional<User> user = userService.login(req.getSession());
-		if (user == null) {
+		if (!user.isPresent()) {
 			resp.sendRedirect(resp.encodeRedirectURL(config.getApplicationSettings().getPublicUrl()));
 			return;
 		}
@@ -58,17 +60,17 @@ public class NextUnreadServlet extends HttpServlet {
 			protected FeedEntryStatus runInSession() throws Exception {
 				FeedEntryStatus status = null;
 				if (StringUtils.isBlank(categoryId) || CategoryREST.ALL.equals(categoryId)) {
-					List<FeedSubscription> subs = feedSubscriptionDAO.findAll(user);
+					List<FeedSubscription> subs = feedSubscriptionDAO.findAll(user.get());
-					List<FeedEntryStatus> statuses = feedEntryStatusDAO.findBySubscriptions(user, subs, true, null, null, 0, 1, order,
+					List<FeedEntryStatus> statuses = feedEntryStatusDAO.findBySubscriptions(user.get(), subs, true, null, null, 0, 1,
-							true, false, null);
+							order, true, false, null);
 					status = Iterables.getFirst(statuses, null);
 				} else {
-					FeedCategory category = feedCategoryDAO.findById(user, Long.valueOf(categoryId));
+					FeedCategory category = feedCategoryDAO.findById(user.get(), Long.valueOf(categoryId));
 					if (category != null) {
-						List<FeedCategory> children = feedCategoryDAO.findAllChildrenCategories(user, category);
+						List<FeedCategory> children = feedCategoryDAO.findAllChildrenCategories(user.get(), category);
-						List<FeedSubscription> subscriptions = feedSubscriptionDAO.findByCategories(user, children);
+						List<FeedSubscription> subscriptions = feedSubscriptionDAO.findByCategories(user.get(), children);
-						List<FeedEntryStatus> statuses = feedEntryStatusDAO.findBySubscriptions(user, subscriptions, true, null, null, 0,
+						List<FeedEntryStatus> statuses = feedEntryStatusDAO.findBySubscriptions(user.get(), subscriptions, true, null,
-								1, order, true, false, null);
+								null, 0, 1, order, true, false, null);
 						status = Iterables.getFirst(statuses, null);
 					}
 				}