From 31a06718a716373440b4147da852c1e8cf47a9db Mon Sep 17 00:00:00 2001 From: Daniel Gerhardt Date: Sun, 16 Jul 2017 18:22:35 +0200 Subject: [PATCH] Refactor permission handling * Use consistent permission for all target domain objects: * read, create, owner, update, delete * Use switch blocks to improve readability * Fetch target domain objects early to reduce code duplication --- .../ApplicationPermissionEvaluator.java | 148 ++++++++++-------- 1 file changed, 82 insertions(+), 66 deletions(-) diff --git a/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java b/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java index 52db4948..e5d35066 100644 --- a/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java +++ b/src/main/java/de/thm/arsnova/security/ApplicationPermissionEvaluator.java @@ -59,18 +59,20 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator { public boolean hasPermission( final Authentication authentication, final Object targetDomainObject, - final Object permission - ) { - final String username = getUsername(authentication); - if (checkAdminPermission(username)) { - return true; - } else if ( - targetDomainObject instanceof Session - && checkSessionPermission(username, ((Session) targetDomainObject).getKeyword(), permission) - ) { - return true; + final Object permission) { + if (authentication == null || targetDomainObject == null || !(permission instanceof String)) { + return false; } - return false; + + final String username = getUsername(authentication); + + return hasAdminRole(username) + || (targetDomainObject instanceof Session + && hasSessionPermission(username, ((Session) targetDomainObject), permission.toString())) + || (targetDomainObject instanceof Content + && hasContentPermission(username, ((Content) targetDomainObject), permission.toString())) + || (targetDomainObject instanceof Comment + && hasCommentPermission(username, ((Comment) targetDomainObject), permission.toString())); } @Override @@ -78,82 +80,96 @@ public class ApplicationPermissionEvaluator implements PermissionEvaluator { final Authentication authentication, final Serializable targetId, final String targetType, - final Object permission - ) { + final Object permission) { + if (authentication == null || targetId == null || targetType == null || !(permission instanceof String)) { + return false; + } + final String username = getUsername(authentication); - if (checkAdminPermission(username)) { - return true; - } else if ( - "session".equals(targetType) - && checkSessionPermission(username, targetId, permission)) { - return true; - } else if ( - "content".equals(targetType) - && checkQuestionPermission(username, targetId, permission) - ) { - return true; - } else if ( - "comment".equals(targetType) - && checkInterposedQuestionPermission(username, targetId, permission) - ) { + if (hasAdminRole(username)) { return true; } - return false; - } - private boolean checkAdminPermission(final String username) { - /* TODO: only allow accounts from arsnova db */ - return Arrays.asList(adminAccounts).contains(username); + switch (targetType) { + case "session": + final Session targetSession = sessionRepository.findByKeyword(targetId.toString()); + return targetSession != null && hasSessionPermission(username, targetSession, permission.toString()); + case "content": + final Content targetContent = contentRepository.findOne(targetId.toString()); + return targetContent != null && hasContentPermission(username, targetContent, permission.toString()); + case "comment": + final Comment targetComment = commentRepository.findOne(targetId.toString()); + return targetComment != null && hasCommentPermission(username, targetComment, permission.toString()); + default: + return false; + } } - private boolean checkSessionPermission( + private boolean hasSessionPermission( final String username, - final Serializable targetId, - final Object permission - ) { - if (permission instanceof String && ("owner".equals(permission) || "write".equals(permission))) { - return sessionRepository.findByKeyword(targetId.toString()).getCreator().equals(username); - } else if (permission instanceof String && "read".equals(permission)) { - return sessionRepository.findByKeyword(targetId.toString()).isActive(); + final Session targetSession, + final String permission) { + switch (permission) { + case "read": + return targetSession.isActive(); + case "create": + /* There are currently no limitations on session creation. */ + return true; + case "owner": + case "update": + case "delete": + return targetSession.getCreator().equals(username); + default: + return false; } - return false; } - private boolean checkQuestionPermission( + private boolean hasContentPermission( final String username, - final Serializable targetId, - final Object permission - ) { - if (permission instanceof String && "owner".equals(permission)) { - final Content content = contentRepository.findOne(targetId.toString()); - if (content != null) { - final Session session = sessionRepository.findOne(content.getSessionId()); - + final Content targetContent, + final String permission) { + switch (permission) { + case "read": + return sessionRepository.findOne(targetContent.getSessionId()).isActive(); + case "create": + case "owner": + case "update": + case "delete": + final Session session = sessionRepository.findOne(targetContent.getSessionId()); return session != null && session.getCreator().equals(username); - } + default: + return false; } - return false; } - private boolean checkInterposedQuestionPermission( + private boolean hasCommentPermission( final String username, - final Serializable targetId, - final Object permission - ) { - if (permission instanceof String && "owner".equals(permission)) { - final Comment comment = commentRepository.findOne(targetId.toString()); - if (comment != null) { - // Does the creator want to delete his own comment? - if (comment.getCreator() != null && comment.getCreator().equals(username)) { + final Comment targetComment, + final String permission) { + switch (permission) { + case "create": + return sessionRepository.findOne(targetComment.getSessionId()).isActive(); + case "owner": + case "update": + return targetComment.getCreator() != null && targetComment.getCreator().equals(username); + case "read": + case "delete": + if (targetComment.getCreator() != null && targetComment.getCreator().equals(username)) { return true; } - // Allow deletion if requested by session owner - final Session session = sessionRepository.findByKeyword(comment.getSessionId()); + + /* Allow reading & deletion by session owner */ + final Session session = sessionRepository.findOne(targetComment.getSessionId()); return session != null && session.getCreator().equals(username); - } + default: + return false; } - return false; + } + + private boolean hasAdminRole(final String username) { + /* TODO: only allow accounts from arsnova db */ + return Arrays.asList(adminAccounts).contains(username); } private String getUsername(final Authentication authentication) { -- GitLab