From 5149f150bdaddf0c10e1165da0d04fb4aecd239b Mon Sep 17 00:00:00 2001
From: Paul-Christian Volkmer <paul-christian.volkmer@mni.thm.de>
Date: Wed, 21 May 2014 11:25:13 +0200
Subject: [PATCH] Reworked permission evaluator for use with all types of
authentication
---
.../ApplicationPermissionEvaluator.java | 36 +++++++++++--------
.../thm/arsnova/services/QuestionService.java | 9 +----
.../thm/arsnova/services/SessionService.java | 3 +-
.../arsnova/services/QuestionServiceTest.java | 24 ++++++++++---
.../arsnova/services/SessionServiceTest.java | 4 +--
5 files changed, 46 insertions(+), 30 deletions(-)
diff --git a/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java b/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java
index 052b3cc5a..10cd0885d 100644
--- a/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java
+++ b/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java
@@ -4,46 +4,54 @@ import java.io.Serializable;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.access.PermissionEvaluator;
+import org.springframework.security.authentication.AnonymousAuthenticationToken;
import org.springframework.security.core.Authentication;
-import org.springframework.security.core.userdetails.UserDetails;
import de.thm.arsnova.dao.IDatabaseDao;
+import de.thm.arsnova.entities.Session;
+import de.thm.arsnova.exceptions.ForbiddenException;
import de.thm.arsnova.exceptions.UnauthorizedException;
public class ApplicationPermissionEvaluator implements PermissionEvaluator {
@Autowired
- IDatabaseDao dao;
+ private IDatabaseDao dao;
@Override
public boolean hasPermission(Authentication authentication, Object targetDomainObject, Object permission) {
- UserDetails user = getUserDetails(authentication);
- return false;
+ String username = getUsername(authentication);
+
+ if (
+ targetDomainObject instanceof Session
+ && ! checkSessionPermission(username, ((Session)targetDomainObject).getKeyword(), permission)
+ ) {
+ throw new ForbiddenException();
+ }
+ return true;
}
@Override
public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permission) {
- UserDetails user = getUserDetails(authentication);
+ String username = getUsername(authentication);
- if ("session".equals(targetType)) {
- return checkSessionPermission(user, targetId, permission);
+ if ("session".equals(targetType) && ! checkSessionPermission(username, targetId, permission)) {
+ throw new ForbiddenException();
}
- return false;
+ return true;
}
- private boolean checkSessionPermission(UserDetails user, Serializable targetId, Object permission) {
+ private boolean checkSessionPermission(String username, Serializable targetId, Object permission) {
if (permission instanceof String && permission.equals("owner")) {
- return dao.getSession(targetId.toString()).getCreator().equals(user.getUsername());
+ return dao.getSession(targetId.toString()).getCreator().equals(username);
}
return false;
}
- private UserDetails getUserDetails(Authentication authentication)
- throws UnauthorizedException {
- if (authentication.getPrincipal() == null || authentication.getPrincipal() instanceof String) {
+ private String getUsername(Authentication authentication) {
+ if (authentication == null || authentication instanceof AnonymousAuthenticationToken) {
throw new UnauthorizedException();
}
- return (UserDetails)authentication.getPrincipal();
+ return authentication.getName();
}
}
diff --git a/src/main/java/de/thm/arsnova/services/QuestionService.java b/src/main/java/de/thm/arsnova/services/QuestionService.java
index b47fc1d47..b68df9615 100644
--- a/src/main/java/de/thm/arsnova/services/QuestionService.java
+++ b/src/main/java/de/thm/arsnova/services/QuestionService.java
@@ -40,7 +40,6 @@ import de.thm.arsnova.entities.Question;
import de.thm.arsnova.entities.Session;
import de.thm.arsnova.entities.User;
import de.thm.arsnova.exceptions.BadRequestException;
-import de.thm.arsnova.exceptions.ForbiddenException;
import de.thm.arsnova.exceptions.NotFoundException;
import de.thm.arsnova.exceptions.UnauthorizedException;
import de.thm.arsnova.socket.ARSnovaSocketIOServer;
@@ -80,17 +79,11 @@ public class QuestionService implements IQuestionService {
}
@Override
- @PreAuthorize("isAuthenticated()")
+ @PreAuthorize("isAuthenticated() and hasPermission(#question.getSessionKeyword(), 'session', 'owner')")
public Question saveQuestion(Question question) {
Session session = this.databaseDao.getSessionFromKeyword(question.getSessionKeyword());
question.setSessionId(session.get_id());
- User user = userService.getCurrentUser();
-
- if (! session.isCreator(user)) {
- throw new ForbiddenException();
- }
-
if ("freetext".equals(question.getQuestionType())) {
question.setPiRound(0);
} else if (question.getPiRound() < 1 || question.getPiRound() > 2) {
diff --git a/src/main/java/de/thm/arsnova/services/SessionService.java b/src/main/java/de/thm/arsnova/services/SessionService.java
index 79f98a6eb..816ea4607 100644
--- a/src/main/java/de/thm/arsnova/services/SessionService.java
+++ b/src/main/java/de/thm/arsnova/services/SessionService.java
@@ -234,7 +234,8 @@ public class SessionService implements ISessionService {
}
@Override
- @PreAuthorize("isAuthenticated() and hasPermission(#sessionkey, 'session', 'owner')")
+ @PreAuthorize("isAuthenticated() and hasPermission(#session, 'owner')")
+ //@PreAuthorize("isAuthenticated() and hasPermission(#sessionkey, 'session', 'owner')")
public Session updateSession(String sessionkey, Session session) {
return databaseDao.updateSession(session);
}
diff --git a/src/test/java/de/thm/arsnova/services/QuestionServiceTest.java b/src/test/java/de/thm/arsnova/services/QuestionServiceTest.java
index 64e251fc3..4fb11fe63 100644
--- a/src/test/java/de/thm/arsnova/services/QuestionServiceTest.java
+++ b/src/test/java/de/thm/arsnova/services/QuestionServiceTest.java
@@ -26,6 +26,7 @@ import java.util.ArrayList;
import java.util.List;
import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
@@ -38,6 +39,8 @@ 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)
@@ -65,17 +68,19 @@ public class QuestionServiceTest {
SecurityContextHolder.getContext().setAuthentication(token);
userService.setUserAuthenticated(isAuthenticated, username);
} else {
- SecurityContextHolder.setContext(
- SecurityContextHolder.createEmptyContext()
- );
userService.setUserAuthenticated(isAuthenticated);
}
}
+ @Before
+ public final void startup() {
+ SecurityContextHolder.clearContext();
+ }
+
@After
public final void cleanup() {
- databaseDao.cleanupTestData();
- setAuthenticated(false, "ptsr00");
+ //databaseDao.cleanupTestData();
+ //setAuthenticated(false, "ptsr00");
}
@Test(expected = AuthenticationCredentialsNotFoundException.class)
@@ -123,4 +128,13 @@ public class QuestionServiceTest {
assertFalse(theQ.isRead());
}
+
+ @Test(expected = ForbiddenException.class)
+ public void testShouldSaveQuestion() throws Exception{
+ setAuthenticated(true, "regular user");
+ Question question = new Question();
+ question.setSessionKeyword("12345678");
+ question.setQuestionVariant("freetext");
+ questionService.saveQuestion(question);
+ }
}
diff --git a/src/test/java/de/thm/arsnova/services/SessionServiceTest.java b/src/test/java/de/thm/arsnova/services/SessionServiceTest.java
index a294544b9..c8ee1deca 100644
--- a/src/test/java/de/thm/arsnova/services/SessionServiceTest.java
+++ b/src/test/java/de/thm/arsnova/services/SessionServiceTest.java
@@ -48,8 +48,8 @@ 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;
-import de.thm.arsnova.exceptions.UnauthorizedException;
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(locations = {
@@ -135,7 +135,7 @@ public class SessionServiceTest {
assertNotNull(sessionService.joinSession("11111111"));
}
- @Test(expected = UnauthorizedException.class)
+ @Test(expected = ForbiddenException.class)
public void testShouldUpdateSession() {
setAuthenticated(true, "ptsr00");
--
GitLab