From 642b9a63d314338cb7b729dfa7ff03376c2a988c Mon Sep 17 00:00:00 2001 From: jendib Date: Sun, 8 May 2016 12:14:06 +0200 Subject: [PATCH] Cleanup ACL checks --- .../docs/rest/resource/CommentResource.java | 14 +++++++------- .../docs/rest/resource/DocumentResource.java | 8 ++++---- .../sismics/docs/rest/resource/FileResource.java | 13 ++++++------- .../sismics/docs/rest/resource/ShareResource.java | 8 +++----- 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/docs-web/src/main/java/com/sismics/docs/rest/resource/CommentResource.java b/docs-web/src/main/java/com/sismics/docs/rest/resource/CommentResource.java index bd032dcd..ce745054 100644 --- a/docs-web/src/main/java/com/sismics/docs/rest/resource/CommentResource.java +++ b/docs-web/src/main/java/com/sismics/docs/rest/resource/CommentResource.java @@ -1,8 +1,8 @@ package com.sismics.docs.rest.resource; import com.sismics.docs.core.constant.PermType; +import com.sismics.docs.core.dao.jpa.AclDao; import com.sismics.docs.core.dao.jpa.CommentDao; -import com.sismics.docs.core.dao.jpa.DocumentDao; import com.sismics.docs.core.dao.jpa.dto.CommentDto; import com.sismics.docs.core.model.jpa.Comment; import com.sismics.rest.exception.ForbiddenClientException; @@ -42,8 +42,8 @@ public class CommentResource extends BaseResource { content = ValidationUtil.validateLength(content, "content", 1, 4000, false); // Read access on doc gives access to write comments - DocumentDao documentDao = new DocumentDao(); - if (documentDao.getDocument(documentId, PermType.READ, getTargetIdList(null)) == null) { + AclDao aclDao = new AclDao(); + if (!aclDao.checkPermission(documentId, PermType.READ, getTargetIdList(null))) { throw new NotFoundException(); } @@ -88,8 +88,8 @@ public class CommentResource extends BaseResource { // If the current user owns the comment, skip ACL check if (!comment.getUserId().equals(principal.getId())) { // Get the associated document - DocumentDao documentDao = new DocumentDao(); - if (documentDao.getDocument(comment.getDocumentId(), PermType.WRITE, getTargetIdList(null)) == null) { + AclDao aclDao = new AclDao(); + if (!aclDao.checkPermission(comment.getDocumentId(), PermType.WRITE, getTargetIdList(null))) { throw new NotFoundException(); } } @@ -116,8 +116,8 @@ public class CommentResource extends BaseResource { authenticate(); // Read access on doc gives access to read comments - DocumentDao documentDao = new DocumentDao(); - if (documentDao.getDocument(documentId, PermType.READ, getTargetIdList(shareId)) == null) { + AclDao aclDao = new AclDao(); + if (!aclDao.checkPermission(documentId, PermType.READ, getTargetIdList(shareId))) { throw new NotFoundException(); } diff --git a/docs-web/src/main/java/com/sismics/docs/rest/resource/DocumentResource.java b/docs-web/src/main/java/com/sismics/docs/rest/resource/DocumentResource.java index 4633e34b..ceb61274 100644 --- a/docs-web/src/main/java/com/sismics/docs/rest/resource/DocumentResource.java +++ b/docs-web/src/main/java/com/sismics/docs/rest/resource/DocumentResource.java @@ -668,14 +668,14 @@ public class DocumentResource extends BaseResource { // Get the document DocumentDao documentDao = new DocumentDao(); FileDao fileDao = new FileDao(); - DocumentDto documentDto = documentDao.getDocument(id, PermType.WRITE, getTargetIdList(null)); - if (documentDto == null) { + AclDao aclDao = new AclDao(); + if (!aclDao.checkPermission(id, PermType.WRITE, getTargetIdList(null))) { throw new NotFoundException(); } List fileList = fileDao.getByDocumentId(principal.getId(), id); // Delete the document - documentDao.delete(documentDto.getId(), principal.getId()); + documentDao.delete(id, principal.getId()); // Raise file deleted events (don't bother sending document updated event) for (File file : fileList) { @@ -688,7 +688,7 @@ public class DocumentResource extends BaseResource { // Raise a document deleted event DocumentDeletedAsyncEvent documentDeletedAsyncEvent = new DocumentDeletedAsyncEvent(); documentDeletedAsyncEvent.setUserId(principal.getId()); - documentDeletedAsyncEvent.setDocumentId(documentDto.getId()); + documentDeletedAsyncEvent.setDocumentId(id); AppContext.getInstance().getAsyncEventBus().post(documentDeletedAsyncEvent); // Always return OK diff --git a/docs-web/src/main/java/com/sismics/docs/rest/resource/FileResource.java b/docs-web/src/main/java/com/sismics/docs/rest/resource/FileResource.java index 31a3af3a..b692ed4e 100644 --- a/docs-web/src/main/java/com/sismics/docs/rest/resource/FileResource.java +++ b/docs-web/src/main/java/com/sismics/docs/rest/resource/FileResource.java @@ -265,8 +265,8 @@ public class FileResource extends BaseResource { ValidationUtil.validateRequired(idList, "order"); // Get the document - DocumentDao documentDao = new DocumentDao(); - if (documentDao.getDocument(documentId, PermType.WRITE, getTargetIdList(null)) == null) { + AclDao aclDao = new AclDao(); + if (!aclDao.checkPermission(documentId, PermType.WRITE, getTargetIdList(null))) { throw new NotFoundException(); } @@ -347,20 +347,19 @@ public class FileResource extends BaseResource { // Get the file FileDao fileDao = new FileDao(); - DocumentDao documentDao = new DocumentDao(); + AclDao aclDao = new AclDao(); File file = fileDao.getFile(id); if (file == null) { throw new NotFoundException(); } - DocumentDto documentDto = null; if (file.getDocumentId() == null) { // It's an orphan file if (!file.getUserId().equals(principal.getId())) { // But not ours throw new ForbiddenClientException(); } - } else if ((documentDto = documentDao.getDocument(file.getDocumentId(), PermType.WRITE, getTargetIdList(null))) == null) { + } else if (!aclDao.checkPermission(file.getDocumentId(), PermType.WRITE, getTargetIdList(null))) { throw new NotFoundException(); } @@ -384,11 +383,11 @@ public class FileResource extends BaseResource { fileDeletedAsyncEvent.setFile(file); AppContext.getInstance().getAsyncEventBus().post(fileDeletedAsyncEvent); - if (documentDto != null) { + if (file.getDocumentId() != null) { // Raise a new document updated DocumentUpdatedAsyncEvent documentUpdatedAsyncEvent = new DocumentUpdatedAsyncEvent(); documentUpdatedAsyncEvent.setUserId(principal.getId()); - documentUpdatedAsyncEvent.setDocumentId(documentDto.getId()); + documentUpdatedAsyncEvent.setDocumentId(file.getDocumentId()); AppContext.getInstance().getAsyncEventBus().post(documentUpdatedAsyncEvent); } diff --git a/docs-web/src/main/java/com/sismics/docs/rest/resource/ShareResource.java b/docs-web/src/main/java/com/sismics/docs/rest/resource/ShareResource.java index f51d2214..9e4d49e5 100644 --- a/docs-web/src/main/java/com/sismics/docs/rest/resource/ShareResource.java +++ b/docs-web/src/main/java/com/sismics/docs/rest/resource/ShareResource.java @@ -4,7 +4,6 @@ package com.sismics.docs.rest.resource; import com.sismics.docs.core.constant.AclTargetType; import com.sismics.docs.core.constant.PermType; import com.sismics.docs.core.dao.jpa.AclDao; -import com.sismics.docs.core.dao.jpa.DocumentDao; import com.sismics.docs.core.dao.jpa.ShareDao; import com.sismics.docs.core.model.jpa.Acl; import com.sismics.docs.core.model.jpa.Share; @@ -46,9 +45,9 @@ public class ShareResource extends BaseResource { ValidationUtil.validateRequired(documentId, "id"); name = ValidationUtil.validateLength(name, "name", 1, 36, true); - // Get the document - DocumentDao documentDao = new DocumentDao(); - if (documentDao.getDocument(documentId, PermType.WRITE, getTargetIdList(null)) == null) { + // Check write permission on the document + AclDao aclDao = new AclDao(); + if (!aclDao.checkPermission(documentId, PermType.WRITE, getTargetIdList(null))) { throw new NotFoundException(); } @@ -59,7 +58,6 @@ public class ShareResource extends BaseResource { shareDao.create(share); // Create the ACL - AclDao aclDao = new AclDao(); Acl acl = new Acl(); acl.setSourceId(documentId); acl.setPerm(PermType.READ);