Closes #245: admin group undeletable + admin can see all

This commit is contained in:
Benjamin Gamard 2018-10-18 18:57:06 +02:00
parent 58af2b00cb
commit 0c257b763d
10 changed files with 67 additions and 28 deletions

View File

@ -7,6 +7,7 @@ import com.sismics.docs.core.constant.PermType;
import com.sismics.docs.core.dao.dto.AclDto;
import com.sismics.docs.core.model.jpa.Acl;
import com.sismics.docs.core.util.AuditLogUtil;
import com.sismics.docs.core.util.SecurityUtil;
import com.sismics.util.context.ThreadLocalContext;
import javax.persistence.EntityManager;
@ -124,6 +125,10 @@ public class AclDao {
* @return True if the document is accessible
*/
public boolean checkPermission(String sourceId, PermType perm, List<String> targetIdList) {
if (SecurityUtil.skipAclCheck(targetIdList)) {
return true;
}
EntityManager em = ThreadLocalContext.get().getEntityManager();
StringBuilder sb = new StringBuilder("select a.ACL_ID_C from T_ACL a ");
sb.append(" where a.ACL_TARGETID_C in (:targetIdList) and a.ACL_SOURCEID_C = :sourceId and a.ACL_PERM_C = :perm and a.ACL_DELETEDATE_D is null ");

View File

@ -7,6 +7,7 @@ import com.sismics.docs.core.dao.dto.TagDto;
import com.sismics.docs.core.model.jpa.DocumentTag;
import com.sismics.docs.core.model.jpa.Tag;
import com.sismics.docs.core.util.AuditLogUtil;
import com.sismics.docs.core.util.SecurityUtil;
import com.sismics.docs.core.util.jpa.QueryParam;
import com.sismics.docs.core.util.jpa.QueryUtil;
import com.sismics.docs.core.util.jpa.SortCriteria;
@ -185,7 +186,7 @@ public class TagDao {
criteriaList.add("t.TAG_ID_C = :id");
parameterMap.put("id", criteria.getId());
}
if (criteria.getTargetIdList() != null) {
if (criteria.getTargetIdList() != null && !SecurityUtil.skipAclCheck(criteria.getTargetIdList())) {
sb.append(" left join T_ACL a on a.ACL_TARGETID_C in (:targetIdList) and a.ACL_SOURCEID_C = t.TAG_ID_C and a.ACL_PERM_C = 'READ' and a.ACL_DELETEDATE_D is null ");
criteriaList.add("a.ACL_ID_C is not null");
parameterMap.put("targetIdList", criteria.getTargetIdList());

View File

@ -6,6 +6,8 @@ import com.sismics.docs.core.dao.UserDao;
import com.sismics.docs.core.model.jpa.Group;
import com.sismics.docs.core.model.jpa.User;
import java.util.List;
/**
* Security utilities.
*
@ -37,4 +39,14 @@ public class SecurityUtil {
return null;
}
/**
* Return true if the ACL targets provided don't need security checks (administrator users).
*
* @param targetIdList Target ID list
* @return True if skip ACL checks
*/
public static boolean skipAclCheck(List<String> targetIdList) {
return targetIdList.contains("admin") || targetIdList.contains("administrators");
}
}

View File

@ -12,6 +12,7 @@ import com.sismics.docs.core.model.jpa.Config;
import com.sismics.docs.core.model.jpa.Document;
import com.sismics.docs.core.model.jpa.File;
import com.sismics.docs.core.util.DirectoryUtil;
import com.sismics.docs.core.util.SecurityUtil;
import com.sismics.docs.core.util.jpa.PaginatedList;
import com.sismics.docs.core.util.jpa.PaginatedLists;
import com.sismics.docs.core.util.jpa.QueryParam;
@ -229,11 +230,13 @@ public class LuceneIndexingHandler implements IndexingHandler {
// Add search criterias
if (criteria.getTargetIdList() != null) {
if (!SecurityUtil.skipAclCheck(criteria.getTargetIdList())) {
// Read permission is enough for searching
sb.append(" left join T_ACL a on a.ACL_TARGETID_C in (:targetIdList) and a.ACL_SOURCEID_C = d.DOC_ID_C and a.ACL_PERM_C = 'READ' and a.ACL_DELETEDATE_D is null ");
sb.append(" left join T_DOCUMENT_TAG dta on dta.DOT_IDDOCUMENT_C = d.DOC_ID_C and dta.DOT_DELETEDATE_D is null ");
sb.append(" left join T_ACL a2 on a2.ACL_TARGETID_C in (:targetIdList) and a2.ACL_SOURCEID_C = dta.DOT_IDTAG_C and a2.ACL_PERM_C = 'READ' and a2.ACL_DELETEDATE_D is null ");
criteriaList.add("(a.ACL_ID_C is not null or a2.ACL_ID_C is not null)");
}
parameterMap.put("targetIdList", criteria.getTargetIdList());
}
if (!Strings.isNullOrEmpty(criteria.getSearch()) || !Strings.isNullOrEmpty(criteria.getFullSearch())) {

View File

@ -377,7 +377,7 @@ public class DocumentResource extends BaseResource {
}
for (DocumentDto documentDto : paginatedList.getResultList()) {
// Get tags added by the current user on this document
// Get tags accessible by the current user on this document
List<TagDto> tagDtoList = tagDao.findByCriteria(new TagCriteria()
.setTargetIdList(getTargetIdList(null))
.setDocumentId(documentDto.getId()), new SortCriteria(1, true));

View File

@ -1,7 +1,9 @@
package com.sismics.docs.rest.resource;
import com.google.common.base.Strings;
import com.google.common.collect.Sets;
import com.sismics.docs.core.dao.GroupDao;
import com.sismics.docs.core.dao.RoleBaseFunctionDao;
import com.sismics.docs.core.dao.UserDao;
import com.sismics.docs.core.dao.criteria.GroupCriteria;
import com.sismics.docs.core.dao.criteria.UserCriteria;
@ -24,6 +26,7 @@ import javax.ws.rs.*;
import javax.ws.rs.core.Response;
import java.text.MessageFormat;
import java.util.List;
import java.util.Set;
/**
* Group REST resources.
@ -186,6 +189,15 @@ public class GroupResource extends BaseResource {
throw new NotFoundException();
}
// Ensure that the admin group is not deleted
if (group.getRoleId() != null) {
RoleBaseFunctionDao roleBaseFunctionDao = new RoleBaseFunctionDao();
Set<String> baseFunctionSet = roleBaseFunctionDao.findByRoleId(Sets.newHashSet(group.getRoleId()));
if (baseFunctionSet.contains(BaseFunction.ADMIN.name())) {
throw new ClientException("ForbiddenError", "The administrators group cannot be deleted");
}
}
// Delete the group
groupDao.delete(group.getId(), principal.getId());

View File

@ -445,7 +445,7 @@ public class UserResource extends BaseResource {
throw new ForbiddenClientException();
}
// Ensure that the admin user is not deleted
// Ensure that the admin or guest users are not deleted
if (hasBaseFunction(BaseFunction.ADMIN) || principal.isGuest()) {
throw new ClientException("ForbiddenError", "This user cannot be deleted");
}
@ -519,8 +519,8 @@ public class UserResource extends BaseResource {
}
// Ensure that the admin user is not deleted
RoleBaseFunctionDao userBaseFuction = new RoleBaseFunctionDao();
Set<String> baseFunctionSet = userBaseFuction.findByRoleId(Sets.newHashSet(user.getRoleId()));
RoleBaseFunctionDao roleBaseFunctionDao = new RoleBaseFunctionDao();
Set<String> baseFunctionSet = roleBaseFunctionDao.findByRoleId(Sets.newHashSet(user.getRoleId()));
if (baseFunctionSet.contains(BaseFunction.ADMIN.name())) {
throw new ClientException("ForbiddenError", "The admin user cannot be deleted");
}

View File

@ -1,17 +1,16 @@
package com.sismics.docs.rest;
import java.util.ArrayList;
import java.util.List;
import com.sismics.util.filter.TokenBasedSecurityFilter;
import org.junit.Assert;
import org.junit.Test;
import javax.json.JsonArray;
import javax.json.JsonObject;
import javax.ws.rs.client.Entity;
import javax.ws.rs.core.Form;
import org.junit.Assert;
import org.junit.Test;
import com.sismics.util.filter.TokenBasedSecurityFilter;
import javax.ws.rs.core.Response;
import java.util.ArrayList;
import java.util.List;
/**
@ -168,6 +167,15 @@ public class TestGroupResource extends BaseJerseyTest {
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, adminToken)
.delete(JsonObject.class);
// Delete group administrators
Response response = target().path("/group/administrators").request()
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, adminToken)
.delete();
Assert.assertEquals(Response.Status.BAD_REQUEST, Response.Status.fromStatusCode(response.getStatus()));
json = response.readEntity(JsonObject.class);
Assert.assertEquals("ForbiddenError", json.getString("type"));
Assert.assertEquals("The administrators group cannot be deleted", json.getString("message"));
// Check group1 groups (all computed groups)
json = target().path("/user").request()
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, group1Token)

View File

@ -216,7 +216,7 @@ public class TestRouteResource extends BaseJerseyTest {
.param("documentId", document1Id)
.param("transition", "APPROVED")), JsonObject.class);
Assert.assertFalse(json.containsKey("route_step"));
Assert.assertFalse(json.getBoolean("readable"));
Assert.assertTrue(json.getBoolean("readable")); // Admin can read everything
Assert.assertTrue(popEmail().contains("workflow step"));
// Get the route on document 1
@ -239,10 +239,9 @@ public class TestRouteResource extends BaseJerseyTest {
Assert.assertEquals("APPROVED", step.getString("transition"));
// Get document 1 as admin
Response response = target().path("/document/" + document1Id).request()
target().path("/document/" + document1Id).request()
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, adminToken)
.get();
Assert.assertEquals(Response.Status.NOT_FOUND, Response.Status.fromStatusCode(response.getStatus()));
.get(JsonObject.class);
// Get document 1 as route1
json = target().path("/document/" + document1Id).request()
@ -265,7 +264,7 @@ public class TestRouteResource extends BaseJerseyTest {
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, adminToken)
.get(JsonObject.class);
documents = json.getJsonArray("documents");
Assert.assertEquals(0, documents.size());
Assert.assertEquals(1, documents.size()); // Admin can read all documents
// Start the default route on document 1
target().path("/route/start").request()
@ -282,7 +281,7 @@ public class TestRouteResource extends BaseJerseyTest {
Assert.assertTrue(json.containsKey("route_step"));
// Get document 1 as admin
response = target().path("/document/" + document1Id).request()
Response response = target().path("/document/" + document1Id).request()
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, adminToken)
.get();
Assert.assertEquals(Response.Status.OK, Response.Status.fromStatusCode(response.getStatus()));
@ -328,10 +327,9 @@ public class TestRouteResource extends BaseJerseyTest {
Assert.assertFalse(json.containsKey("route_step"));
// Get document 1 as admin
response = target().path("/document/" + document1Id).request()
target().path("/document/" + document1Id).request()
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, adminToken)
.get();
Assert.assertEquals(Response.Status.NOT_FOUND, Response.Status.fromStatusCode(response.getStatus()));
.get(JsonObject.class); // Admin can see all documents
// List all documents with route1
json = target().path("/document/list")
@ -348,7 +346,7 @@ public class TestRouteResource extends BaseJerseyTest {
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, adminToken)
.get(JsonObject.class);
documents = json.getJsonArray("documents");
Assert.assertEquals(0, documents.size());
Assert.assertEquals(1, documents.size());
}
/**

View File

@ -33,7 +33,7 @@
<org.mindrot.jbcrypt>0.3m</org.mindrot.jbcrypt>
<org.apache.lucene.version>5.5.0</org.apache.lucene.version>
<org.imgscalr.imgscalr-lib.version>4.2</org.imgscalr.imgscalr-lib.version>
<org.apache.pdfbox.pdfbox.version>2.0.8</org.apache.pdfbox.pdfbox.version>
<org.apache.pdfbox.pdfbox.version>2.0.12</org.apache.pdfbox.pdfbox.version>
<org.bouncycastle.bcprov-jdk15on.version>1.54</org.bouncycastle.bcprov-jdk15on.version>
<joda-time.joda-time.version>2.9.2</joda-time.joda-time.version>
<org.hibernate.hibernate.version>5.1.0.Final</org.hibernate.hibernate.version>