From a3bf0c32cda5648677a6d04dbca2ab467093a748 Mon Sep 17 00:00:00 2001
From: Paul-Christian Volkmer <paul-christian.volkmer@mni.thm.de>
Date: Wed, 17 Oct 2012 13:41:55 +0200
Subject: [PATCH] Check if user is authorized in intercept method

This checks if there is a user in current security context. If not, an
UnauthorizedException is thrown which will result in HTTP - 401.
---
 .../thm/arsnova/aop/AuthorizationAdviser.java | 21 +++++++-----
 .../controller/AbstractController.java        |  7 ++++
 .../arsnova/controller/SessionController.java |  4 ---
 .../exceptions/UnauthorizedException.java     |  5 +++
 .../thm/arsnova/services/SessionService.java  |  3 ++
 src/main/webapp/WEB-INF/arsnova-servlet.xml   |  1 +
 .../webapp/WEB-INF/spring/spring-main.xml     |  4 +--
 .../controller/LoginControllerTest.java       |  9 +++++-
 .../controller/SessionControllerTest.java     | 32 +++++++++++++++++++
 .../arsnova/services/SessionServiceTest.java  |  4 +++
 .../thm/arsnova/services/StubUserService.java | 25 +++++++++++++++
 src/test/resources/test-config.xml            |  7 +++-
 12 files changed, 106 insertions(+), 16 deletions(-)
 create mode 100644 src/main/java/de/thm/arsnova/exceptions/UnauthorizedException.java
 create mode 100644 src/test/java/de/thm/arsnova/services/StubUserService.java

diff --git a/src/main/java/de/thm/arsnova/aop/AuthorizationAdviser.java b/src/main/java/de/thm/arsnova/aop/AuthorizationAdviser.java
index dc7ef4a25..2d889b790 100644
--- a/src/main/java/de/thm/arsnova/aop/AuthorizationAdviser.java
+++ b/src/main/java/de/thm/arsnova/aop/AuthorizationAdviser.java
@@ -2,20 +2,25 @@ package de.thm.arsnova.aop;
 
 import org.aspectj.lang.annotation.Aspect;
 import org.aspectj.lang.annotation.Before;
-import org.aspectj.lang.annotation.Pointcut;
-import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.security.core.context.SecurityContextHolder;
 
 import de.thm.arsnova.annotation.Authenticated;
+import de.thm.arsnova.entities.User;
+import de.thm.arsnova.exceptions.UnauthorizedException;
+import de.thm.arsnova.services.IUserService;
 
 @Aspect
 public class AuthorizationAdviser {
 
-	@Pointcut("execution(public * de.thm.arsnova.services.*.*(..))")
-	public void serviceMethods() {}
+	static IUserService userService;
 	
-	@Autowired
-	@Before("serviceMethods() && @annotation(authenticated) && this(object)")
-	public void check(Authenticated authenticated, Object object) {
-		System.out.println("******************************: " + object.getClass().getName());
+	public void setUserService(IUserService uService) {
+		userService = uService;
+	}
+	
+	@Before("execution(public * de.thm.arsnova.services.*.*(..)) && @annotation(authenticated) && this(object)")
+	public void checkAuthorization(Authenticated authenticated, Object object) {
+		User u = userService.getUser(SecurityContextHolder.getContext().getAuthentication());
+		if (u == null) throw new UnauthorizedException();
 	}
 }
diff --git a/src/main/java/de/thm/arsnova/controller/AbstractController.java b/src/main/java/de/thm/arsnova/controller/AbstractController.java
index 6ceeb3778..251dafbea 100644
--- a/src/main/java/de/thm/arsnova/controller/AbstractController.java
+++ b/src/main/java/de/thm/arsnova/controller/AbstractController.java
@@ -8,6 +8,7 @@ import org.springframework.web.bind.annotation.ResponseStatus;
 
 import de.thm.arsnova.exceptions.ForbiddenException;
 import de.thm.arsnova.exceptions.NotFoundException;
+import de.thm.arsnova.exceptions.UnauthorizedException;
 
 public class AbstractController {
 	@ResponseStatus(HttpStatus.NOT_FOUND)
@@ -21,4 +22,10 @@ public class AbstractController {
 	public void handleForbiddenException(Exception e, HttpServletRequest request) {
 		
 	}
+	
+	@ResponseStatus(HttpStatus.UNAUTHORIZED)
+	@ExceptionHandler(UnauthorizedException.class)
+	public void handleUnauthorizedException(Exception e, HttpServletRequest request) {
+		
+	}
 }
diff --git a/src/main/java/de/thm/arsnova/controller/SessionController.java b/src/main/java/de/thm/arsnova/controller/SessionController.java
index 56237cbe3..c268a0845 100644
--- a/src/main/java/de/thm/arsnova/controller/SessionController.java
+++ b/src/main/java/de/thm/arsnova/controller/SessionController.java
@@ -86,10 +86,6 @@ public class SessionController extends AbstractController {
 	@ResponseBody
 	public Feedback postFeedback(@PathVariable String sessionkey, @RequestBody int value, HttpServletResponse response) {
 		User user = userService.getUser(SecurityContextHolder.getContext().getAuthentication());
-		if (user == null) {
-			response.setStatus(HttpStatus.UNAUTHORIZED.value());
-			return null;
-		}
 		if (sessionService.saveFeedback(sessionkey, value, user)) {
 			Feedback feedback = sessionService.getFeedback(sessionkey);
 			if (feedback != null) {
diff --git a/src/main/java/de/thm/arsnova/exceptions/UnauthorizedException.java b/src/main/java/de/thm/arsnova/exceptions/UnauthorizedException.java
new file mode 100644
index 000000000..4b3e94984
--- /dev/null
+++ b/src/main/java/de/thm/arsnova/exceptions/UnauthorizedException.java
@@ -0,0 +1,5 @@
+package de.thm.arsnova.exceptions;
+
+public class UnauthorizedException extends RuntimeException {
+	private static final long serialVersionUID = 1L;
+}
diff --git a/src/main/java/de/thm/arsnova/services/SessionService.java b/src/main/java/de/thm/arsnova/services/SessionService.java
index 6f7246ac5..4de5febaa 100644
--- a/src/main/java/de/thm/arsnova/services/SessionService.java
+++ b/src/main/java/de/thm/arsnova/services/SessionService.java
@@ -87,6 +87,7 @@ public class SessionService implements ISessionService {
 	}
 
 	@Override
+	@Authenticated
 	public boolean saveFeedback(String keyword, int value, User user) {
 		return databaseDao.saveFeedback(keyword, value, user);
 	}
@@ -148,12 +149,14 @@ public class SessionService implements ISessionService {
 	}
 
 	@Override
+	@Authenticated
 	public boolean saveQuestion(Question question) {
 		Session session = this.databaseDao.getSessionFromKeyword(question.getSession());
 		return this.databaseDao.saveQuestion(session, question);
 	}
 	
 	@Override
+	@Authenticated
 	public Question getQuestion(String id) {
 		return databaseDao.getQuestion(id);
 	}
diff --git a/src/main/webapp/WEB-INF/arsnova-servlet.xml b/src/main/webapp/WEB-INF/arsnova-servlet.xml
index 065ede779..6a17391e4 100644
--- a/src/main/webapp/WEB-INF/arsnova-servlet.xml
+++ b/src/main/webapp/WEB-INF/arsnova-servlet.xml
@@ -4,4 +4,5 @@
 	xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.1.xsd">
 
 	<!-- ARSnova Root Context: defines shared resources visible to all OTHER web components -->
+	
 </beans>
diff --git a/src/main/webapp/WEB-INF/spring/spring-main.xml b/src/main/webapp/WEB-INF/spring/spring-main.xml
index 1f58deebd..7b46a7669 100644
--- a/src/main/webapp/WEB-INF/spring/spring-main.xml
+++ b/src/main/webapp/WEB-INF/spring/spring-main.xml
@@ -14,8 +14,6 @@
 		http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util-3.1.xsd
 		http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.1.xsd">
 
-	<aop:aspectj-autoproxy />
-
 	<bean id="propertyPlaceholderConfigurer"
 		class="org.springframework.beans.factory.config.PropertyPlaceholderConfigurer"
 		p:ignoreUnresolvablePlaceholders="false" p:ignoreResourceNotFound="true">
@@ -30,6 +28,8 @@
 	<import resource="spring-security.xml" />
 
 	<context:component-scan base-package="de.thm.arsnova" />
+	<context:annotation-config />
+	
 	<mvc:annotation-driven />
 	<mvc:resources mapping="/**" location="/" />
 
diff --git a/src/test/java/de/thm/arsnova/controller/LoginControllerTest.java b/src/test/java/de/thm/arsnova/controller/LoginControllerTest.java
index 2eb8a76af..ea2c531d1 100644
--- a/src/test/java/de/thm/arsnova/controller/LoginControllerTest.java
+++ b/src/test/java/de/thm/arsnova/controller/LoginControllerTest.java
@@ -40,6 +40,8 @@ import org.springframework.web.servlet.ModelAndView;
 import org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerAdapter;
 import org.springframework.web.servlet.view.RedirectView;
 
+import de.thm.arsnova.services.StubUserService;
+
 
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration(locations={
@@ -58,6 +60,9 @@ public class LoginControllerTest {
 	@Autowired
 	private LoginController loginController;
 	
+	@Autowired
+	private StubUserService userService;
+	
 	@Before
 	public void setUp() throws Exception {
 		this.request = new MockHttpServletRequest();
@@ -101,12 +106,14 @@ public class LoginControllerTest {
 	
 	@Test
 	public void testUser() throws Exception {
+		userService.setUserAuthenticated(true);
+		
 		request.setMethod("GET");
 		request.setRequestURI("/whoami");
 
 		handlerAdapter.handle(request, response, loginController);
 		assertNotNull(response);
-		assertEquals(response.getContentAsString(),"{\"username\":\"Guest1234567890\"}");
+		assertEquals(response.getContentAsString(),"{\"username\":\"ptsr00\"}");
 	}
 
 	@Test
diff --git a/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java b/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java
index 6e9646534..cfe9e41e1 100644
--- a/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java
+++ b/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java
@@ -21,6 +21,8 @@ import org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerAda
 
 import de.thm.arsnova.exceptions.ForbiddenException;
 import de.thm.arsnova.exceptions.NotFoundException;
+import de.thm.arsnova.exceptions.UnauthorizedException;
+import de.thm.arsnova.services.StubUserService;
 
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration(locations = {
@@ -37,6 +39,9 @@ public class SessionControllerTest {
 
 	@Autowired
 	private SessionController sessionController;
+	
+	@Autowired
+	private StubUserService userService;
 
 	@Before
 	public void setUp() {
@@ -48,6 +53,8 @@ public class SessionControllerTest {
 
 	@Test
 	public void testShouldNotGetUnknownSession() {
+		userService.setUserAuthenticated(true);
+		
 		request.setMethod("GET");
 		request.setRequestURI("/session/00000000");
 		try {
@@ -67,6 +74,8 @@ public class SessionControllerTest {
 
 	@Test
 	public void testShouldNotGetForbiddenSession() {
+		userService.setUserAuthenticated(true);
+		
 		request.setMethod("GET");
 		request.setRequestURI("/session/99999999");
 		try {
@@ -86,6 +95,8 @@ public class SessionControllerTest {
 	
 	@Test
 	public void testShouldNotGetFeedbackForUnknownSession() {
+		userService.setUserAuthenticated(true);
+		
 		request.setMethod("GET");
 		request.setRequestURI("/session/00000000/feedback");
 		try {
@@ -102,4 +113,25 @@ public class SessionControllerTest {
 
 		fail("Expected exception 'NotFoundException' did not occure");
 	}
+	
+	@Test
+	public void testShouldNotGetSessionIfUnauthorized() {
+		userService.setUserAuthenticated(false);
+		
+		request.setMethod("GET");
+		request.setRequestURI("/session/00000000");
+		try {
+			final ModelAndView mav = handlerAdapter.handle(request, response,
+					sessionController);
+			assertNull(mav);
+			assertTrue(response.getStatus() == 403);
+		} catch (UnauthorizedException e) {
+			return;
+		} catch (Exception e) {
+			e.printStackTrace();
+			fail("An exception occured");
+		}
+
+		fail("Expected exception 'UnauthorizedException' did not occure");
+	}
 }
diff --git a/src/test/java/de/thm/arsnova/services/SessionServiceTest.java b/src/test/java/de/thm/arsnova/services/SessionServiceTest.java
index a0abba0c6..3b950f226 100644
--- a/src/test/java/de/thm/arsnova/services/SessionServiceTest.java
+++ b/src/test/java/de/thm/arsnova/services/SessionServiceTest.java
@@ -39,10 +39,14 @@ public class SessionServiceTest {
 
 	@Autowired
 	ISessionService sessionService;
+	
+	@Autowired
+	StubUserService userService;
 
 	@Test
 	public void testShouldFail() {
 		try {
+			userService.setUserAuthenticated(true);
 			sessionService.getFeedback("00000000");
 		} catch (NotFoundException e) {
 			return;
diff --git a/src/test/java/de/thm/arsnova/services/StubUserService.java b/src/test/java/de/thm/arsnova/services/StubUserService.java
new file mode 100644
index 000000000..8ab9f5712
--- /dev/null
+++ b/src/test/java/de/thm/arsnova/services/StubUserService.java
@@ -0,0 +1,25 @@
+package de.thm.arsnova.services;
+
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.core.Authentication;
+
+import de.thm.arsnova.entities.User;
+import de.thm.arsnova.services.IUserService;
+
+public class StubUserService implements IUserService {
+
+	private User stubUser = null;
+	
+	public void setUserAuthenticated(boolean isAuthenticated) {
+		if (isAuthenticated) {
+			stubUser = new User(new UsernamePasswordAuthenticationToken("ptsr00","testpassword"));
+			return;
+		}
+		stubUser = null;
+	}
+	
+	@Override
+	public User getUser(Authentication authentication) {
+		return stubUser;
+	}
+}
diff --git a/src/test/resources/test-config.xml b/src/test/resources/test-config.xml
index db94937d9..1ef56bd0a 100644
--- a/src/test/resources/test-config.xml
+++ b/src/test/resources/test-config.xml
@@ -6,10 +6,15 @@
 		http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.1.xsd">
 
 	<bean id="sessionService" class="de.thm.arsnova.services.SessionService">
-		<property name="databaseDao" ref="databaseDao"></property>
+		<property name="databaseDao" ref="databaseDao" />
 	</bean>
 	
 	<bean id="databaseDao" class="de.thm.arsnova.dao.StubDatabaseDao">
 	</bean>
 
+	<bean id="authorizationAdviser" class="de.thm.arsnova.aop.AuthorizationAdviser">
+		<property name="userService" ref="userService" />
+	</bean>
+
+	<bean id="userService" class="de.thm.arsnova.services.StubUserService" />
 </beans>
-- 
GitLab