Some changes to security implementation

Set default evaluation result to false and throw an AccessDeniedException.
SecurityExceptionControllerAdvice checks if there is an valid login and
decides whether to send HTTP 401 or HTTP 403.

Conflicts:
	src/test/java/de/thm/arsnova/controller/AbstractControllerTest.java

src/main/java/de/thm/arsnova/controller/SecurityExceptionControllerAdvice.java

 package de.thm.arsnova.controller;
 
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.springframework.http.HttpStatus;
 import org.springframework.security.access.AccessDeniedException;
+import org.springframework.security.authentication.AnonymousAuthenticationToken;
 import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.web.bind.annotation.ControllerAdvice;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.ResponseStatus;
@@ -35,9 +39,22 @@ public class SecurityExceptionControllerAdvice {
 	public void handleAuthenticationCredentialsNotFoundException(final Exception e, final HttpServletRequest request) {
 	}
 
-	@ResponseStatus(HttpStatus.UNAUTHORIZED)
 	@ExceptionHandler(AccessDeniedException.class)
-	public void handleAccessDeniedException(final Exception e, final HttpServletRequest request) {
+	public void handleAccessDeniedException(
+			final Exception e,
+			final HttpServletRequest request,
+			final HttpServletResponse response
+			) {
+		final Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+		if (
+				authentication == null
+				|| authentication.getPrincipal() == null
+				|| authentication instanceof AnonymousAuthenticationToken
+				) {
+			response.setStatus(HttpStatus.UNAUTHORIZED.value());
+			return;
+		}
+		response.setStatus(HttpStatus.FORBIDDEN.value());
 	}
 
 	@ResponseStatus(HttpStatus.FORBIDDEN)
@@ -64,9 +81,4 @@ public class SecurityExceptionControllerAdvice {
 	@ExceptionHandler(NotImplementedException.class)
 	public void handleNotImplementedException(final Exception e, final HttpServletRequest request) {
 	}
-
-	@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
-	@ExceptionHandler(Exception.class)
-	public void handleAllOtherExceptions(final Exception e, final HttpServletRequest request) {
-	}
 }

src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java

@@ -17,7 +17,6 @@ import de.thm.arsnova.entities.InterposedQuestion;
 import de.thm.arsnova.entities.Question;
 import de.thm.arsnova.entities.Session;
 import de.thm.arsnova.entities.User;
-import de.thm.arsnova.exceptions.ForbiddenException;
 import de.thm.arsnova.exceptions.UnauthorizedException;
 
 public class ApplicationPermissionEvaluator implements PermissionEvaluator {
@@ -35,11 +34,11 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator {
 
 		if (
 				targetDomainObject instanceof Session
-				&& !checkSessionPermission(username, ((Session) targetDomainObject).getKeyword(), permission)
+				&& checkSessionPermission(username, ((Session) targetDomainObject).getKeyword(), permission)
 				) {
-			throw new ForbiddenException();
+			return true;
 		}
-		return true;
+		return false;
 	}
 
 	@Override
@@ -51,20 +50,22 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator {
 			) {
 		final String username = getUsername(authentication);
 
-		if ("session".equals(targetType) && !checkSessionPermission(username, targetId, permission)) {
-			throw new ForbiddenException();
+		if (
+				"session".equals(targetType)
+				&& checkSessionPermission(username, targetId, permission)) {
+			return true;
 		} else if (
 				"question".equals(targetType)
-				&& !checkQuestionPermission(username, targetId, permission)
+				&& checkQuestionPermission(username, targetId, permission)
 				) {
-			throw new ForbiddenException();
+			return true;
 		} else if (
 				"interposedquestion".equals(targetType)
-				&& !checkInterposedQuestionPermission(username, targetId, permission)
+				&& checkInterposedQuestionPermission(username, targetId, permission)
 				) {
-			throw new ForbiddenException();
+			return true;
 		}
-		return true;
+		return false;
 	}
 
 	private boolean checkSessionPermission(

src/test/java/de/thm/arsnova/controller/AbstractControllerTest.java

+package de.thm.arsnova.controller;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.After;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.context.SecurityContextHolder;
+
+import de.thm.arsnova.services.StubUserService;
+
+public abstract class AbstractControllerTest {
+
+	@Autowired protected StubUserService userService;
+
+	public AbstractControllerTest() {
+		super();
+	}
+
+	protected void setAuthenticated(final boolean isAuthenticated, final String username) {
+		final List<GrantedAuthority> ga = new ArrayList<GrantedAuthority>();
+		if (isAuthenticated) {
+			final UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken(username, "secret", ga);
+			SecurityContextHolder.getContext().setAuthentication(token);
+			userService.setUserAuthenticated(isAuthenticated, username);
+		} else {
+			userService.setUserAuthenticated(isAuthenticated);
+		}
+	}
+
+	@After
+	public final void cleanup() {
+		SecurityContextHolder.clearContext();
+		userService.setUserAuthenticated(false);
+	}
+
+}
\ No newline at end of file

src/test/java/de/thm/arsnova/controller/SessionControllerTest.java

 package de.thm.arsnova.controller;
 
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@@ -152,4 +154,20 @@ public class SessionControllerTest {
 		mockMvc.perform(post("/session/12345678/online").accept(MediaType.APPLICATION_JSON))
 		.andExpect(status().isUnauthorized());
 	}
+
+	@Test
+	public void testShouldEndInForbidden() throws Exception {
+		setAuthenticated(true, "ptsr00");
+
+		mockMvc.perform(
+				put("/session/12345678")
+				.content("{\"keyword\":\"12345678\", \"name\":\"Testsession\"}, \"shortName\":\"TS\", \"creator\":\"ptsr00\", \"active\":true")
+				.contentType(MediaType.APPLICATION_JSON)
+				.accept(MediaType.APPLICATION_JSON))
+				.andExpect(status().isOk());
+
+		setAuthenticated(true, "other");
+
+		mockMvc.perform(delete("/session/12345678")).andExpect(status().isForbidden());
+	}
 }

src/test/java/de/thm/arsnova/services/QuestionServiceTest.java

@@ -30,6 +30,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.GrantedAuthority;
@@ -41,7 +42,6 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 import de.thm.arsnova.dao.StubDatabaseDao;
 import de.thm.arsnova.entities.InterposedQuestion;
 import de.thm.arsnova.entities.Question;
-import de.thm.arsnova.exceptions.ForbiddenException;
 import de.thm.arsnova.exceptions.NotFoundException;
 
 @RunWith(SpringJUnit4ClassRunner.class)
@@ -129,7 +129,7 @@ public class QuestionServiceTest {
 		assertFalse(theQ.isRead());
 	}
 
-	@Test(expected = ForbiddenException.class)
+	@Test(expected = AccessDeniedException.class)
 	public void testShouldSaveQuestion() throws Exception{
 		setAuthenticated(true, "regular user");
 		final Question question = new Question();
@@ -138,13 +138,13 @@ public class QuestionServiceTest {
 		questionService.saveQuestion(question);
 	}
 
-	@Test(expected = ForbiddenException.class)
+	@Test(expected = AccessDeniedException.class)
 	public void testShouldNotDeleteQuestion() throws Exception{
 		setAuthenticated(true, "otheruser");
 		questionService.deleteQuestion("a1a2a3a4a5a6a7a8a9a");
 	}
 
-	@Test(expected = ForbiddenException.class)
+	@Test(expected = AccessDeniedException.class)
 	public void testShouldNotDeleteInterposedQuestion() throws Exception{
 		setAuthenticated(true, "otheruser");
 		questionService.deleteInterposedQuestion("a1a2a3a4a5a6a7a8a9a");

src/test/java/de/thm/arsnova/services/SessionServiceTest.java

@@ -38,6 +38,7 @@ import org.junit.runner.RunWith;
 import org.springframework.aop.framework.Advised;
 import org.springframework.aop.support.AopUtils;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.GrantedAuthority;
@@ -51,7 +52,6 @@ import de.thm.arsnova.dao.IDatabaseDao;
 import de.thm.arsnova.dao.StubDatabaseDao;
 import de.thm.arsnova.entities.Question;
 import de.thm.arsnova.entities.Session;
-import de.thm.arsnova.exceptions.ForbiddenException;
 import de.thm.arsnova.exceptions.NotFoundException;
 
 @RunWith(SpringJUnit4ClassRunner.class)
@@ -143,7 +143,7 @@ public class SessionServiceTest {
 		assertNotNull(sessionService.joinSession("11111111"));
 	}
 
-	@Test(expected = ForbiddenException.class)
+	@Test(expected = AccessDeniedException.class)
 	public void testShouldUpdateSession() {
 		setAuthenticated(true, "ptsr00");
 
@@ -192,7 +192,7 @@ public class SessionServiceTest {
 		sessionService.deleteSession("12345678");
 	}
 
-	@Test(expected = ForbiddenException.class)
+	@Test(expected = AccessDeniedException.class)
 	public void testShouldNotDeleteSessionIfNotOwner() {
 		setAuthenticated(true, "anybody");
 		sessionService.deleteSession("12345678");