From b1e3fb42d45add30f3776df6d126d2a9a1c9a886 Mon Sep 17 00:00:00 2001
From: Paul-Christian Volkmer <paul-christian.volkmer@mni.thm.de>
Date: Tue, 10 Jun 2014 11:05:09 +0200
Subject: [PATCH] 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
---
.../SecurityExceptionControllerAdvice.java | 26 +++++++++----
.../ApplicationPermissionEvaluator.java | 23 +++++------
.../controller/AbstractControllerTest.java | 39 +++++++++++++++++++
.../controller/SessionControllerTest.java | 18 +++++++++
.../arsnova/services/QuestionServiceTest.java | 8 ++--
.../arsnova/services/SessionServiceTest.java | 6 +--
6 files changed, 95 insertions(+), 25 deletions(-)
create mode 100644 src/test/java/de/thm/arsnova/controller/AbstractControllerTest.java
diff --git a/src/main/java/de/thm/arsnova/controller/SecurityExceptionControllerAdvice.java b/src/main/java/de/thm/arsnova/controller/SecurityExceptionControllerAdvice.java
index 4ffc33b3e..a21ae905f 100644
--- a/src/main/java/de/thm/arsnova/controller/SecurityExceptionControllerAdvice.java
+++ b/src/main/java/de/thm/arsnova/controller/SecurityExceptionControllerAdvice.java
@@ -1,10 +1,14 @@
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) {
- }
}
diff --git a/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java b/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java
index 4e3d4965f..98a98b222 100644
--- a/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java
+++ b/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(
diff --git a/src/test/java/de/thm/arsnova/controller/AbstractControllerTest.java b/src/test/java/de/thm/arsnova/controller/AbstractControllerTest.java
new file mode 100644
index 000000000..626eaf31f
--- /dev/null
+++ b/src/test/java/de/thm/arsnova/controller/AbstractControllerTest.java
@@ -0,0 +1,39 @@
+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
diff --git a/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java b/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java
index f8185c1a8..4860c0746 100644
--- a/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java
+++ b/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java
@@ -1,7 +1,9 @@
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());
+ }
}
diff --git a/src/test/java/de/thm/arsnova/services/QuestionServiceTest.java b/src/test/java/de/thm/arsnova/services/QuestionServiceTest.java
index e26dd7eff..c8b86acdb 100644
--- a/src/test/java/de/thm/arsnova/services/QuestionServiceTest.java
+++ b/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");
diff --git a/src/test/java/de/thm/arsnova/services/SessionServiceTest.java b/src/test/java/de/thm/arsnova/services/SessionServiceTest.java
index bf365e01f..09c07f185 100644
--- a/src/test/java/de/thm/arsnova/services/SessionServiceTest.java
+++ b/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");
--
GitLab