Closes #237: prevent tag circular reference

This commit is contained in:
Benjamin Gamard 2018-10-19 17:04:49 +02:00
parent 8fff672d2f
commit 7baf5e44fd
4 changed files with 39 additions and 10 deletions

View File

@ -239,6 +239,7 @@ public class TagResource extends BaseResource {
* @apiError (client) ValidationError Validation error * @apiError (client) ValidationError Validation error
* @apiError (client) SpacesNotAllowed Spaces are not allowed in tag name * @apiError (client) SpacesNotAllowed Spaces are not allowed in tag name
* @apiError (client) ParentNotFound Parent not found * @apiError (client) ParentNotFound Parent not found
* @apiError (client) CircularReference Circular reference in parent tag
* @apiError (client) NotFound Tag not found * @apiError (client) NotFound Tag not found
* @apiPermission user * @apiPermission user
* @apiVersion 1.5.0 * @apiVersion 1.5.0
@ -275,16 +276,25 @@ public class TagResource extends BaseResource {
} }
// Check the parent // Check the parent
TagDao tagDao = new TagDao();
if (StringUtils.isEmpty(parentId)) { if (StringUtils.isEmpty(parentId)) {
parentId = null; parentId = null;
} else { } else {
if (!aclDao.checkPermission(parentId, PermType.READ, getTargetIdList(null))) { if (!aclDao.checkPermission(parentId, PermType.READ, getTargetIdList(null))) {
throw new ClientException("ParentNotFound", MessageFormat.format("Parent not found: {0}", parentId)); throw new ClientException("ParentNotFound", MessageFormat.format("Parent not found: {0}", parentId));
} }
String parentTagId = parentId;
do {
Tag parentTag = tagDao.getById(parentTagId);
parentTagId = parentTag.getParentId();
if (parentTag.getId().equals(id)) {
throw new ClientException("CircularReference", "Circular reference in parent tag");
}
} while (parentTagId != null);
} }
// Update the tag // Update the tag
TagDao tagDao = new TagDao();
Tag tag = tagDao.getById(id); Tag tag = tagDao.getById(id);
if (!StringUtils.isEmpty(name)) { if (!StringUtils.isEmpty(name)) {
tag.setName(name); tag.setName(name);

View File

@ -21,7 +21,15 @@ angular.module('docs').controller('TagEdit', function($scope, $stateParams, Rest
*/ */
$scope.edit = function() { $scope.edit = function() {
// Update the server // Update the server
Restangular.one('tag', $scope.tag.id).post('', $scope.tag); Restangular.one('tag', $scope.tag.id).post('', $scope.tag).then(function () {
}, function (e) {
if (e.data.type === 'CircularReference') {
var title = $translate.instant('tag.edit.circular_reference_title');
var msg = $translate.instant('tag.edit.circular_reference_message');
var btns = [{result: 'ok', label: $translate.instant('ok'), cssClass: 'btn-primary'}];
$dialog.messageBox(title, msg, btns);
}
});
}; };
/** /**

View File

@ -219,7 +219,9 @@
"name": "Name", "name": "Name",
"color": "Color", "color": "Color",
"parent": "Parent", "parent": "Parent",
"info": "Permissions on this tag will also be applied to documents tagged <span class=\"label label-info\" ng-style=\"{ 'background': color }\">{{ name }}</span>" "info": "Permissions on this tag will also be applied to documents tagged <span class=\"label label-info\" ng-style=\"{ 'background': color }\">{{ name }}</span>",
"circular_reference_title": "Circular reference",
"circular_reference_message": "The hierarchy of parent tags makes a loop, please choose another parent."
} }
}, },
"group": { "group": {

View File

@ -45,6 +45,15 @@ public class TestTagResource extends BaseJerseyTest {
String tag4Id = json.getString("id"); String tag4Id = json.getString("id");
Assert.assertNotNull(tag4Id); Assert.assertNotNull(tag4Id);
// Create a circular reference
Response response = target().path("/tag/" + tag3Id).request()
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token)
.post(Entity.form(new Form()
.param("name", "Tag3")
.param("color", "#0000ff")
.param("parent", tag4Id)));
Assert.assertEquals(Status.BAD_REQUEST, Status.fromStatusCode(response.getStatus()));
// Get the tag // Get the tag
json = target().path("/tag/" + tag4Id).request() json = target().path("/tag/" + tag4Id).request()
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token)
@ -58,7 +67,7 @@ public class TestTagResource extends BaseJerseyTest {
Assert.assertEquals(2, acls.size()); Assert.assertEquals(2, acls.size());
// Create a tag with space (not allowed) // Create a tag with space (not allowed)
Response response = target().path("/tag").request() response = target().path("/tag").request()
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token)
.put(Entity.form(new Form() .put(Entity.form(new Form()
.param("name", "Tag 4"))); .param("name", "Tag 4")));
@ -152,7 +161,7 @@ public class TestTagResource extends BaseJerseyTest {
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token)
.get(JsonObject.class); .get(JsonObject.class);
tags = json.getJsonArray("tags"); tags = json.getJsonArray("tags");
Assert.assertTrue(tags.size() == 2); Assert.assertEquals(2, tags.size());
Assert.assertEquals("Tag4", tags.getJsonObject(1).getString("name")); Assert.assertEquals("Tag4", tags.getJsonObject(1).getString("name"));
Assert.assertEquals("#00ff00", tags.getJsonObject(1).getString("color")); Assert.assertEquals("#00ff00", tags.getJsonObject(1).getString("color"));
Assert.assertEquals(tag3Id, tags.getJsonObject(1).getString("parent")); Assert.assertEquals(tag3Id, tags.getJsonObject(1).getString("parent"));
@ -170,7 +179,7 @@ public class TestTagResource extends BaseJerseyTest {
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token)
.get(JsonObject.class); .get(JsonObject.class);
tags = json.getJsonArray("tags"); tags = json.getJsonArray("tags");
Assert.assertTrue(tags.size() == 2); Assert.assertEquals(2, tags.size());
Assert.assertEquals("UpdatedName", tags.getJsonObject(1).getString("name")); Assert.assertEquals("UpdatedName", tags.getJsonObject(1).getString("name"));
Assert.assertEquals("#0000ff", tags.getJsonObject(1).getString("color")); Assert.assertEquals("#0000ff", tags.getJsonObject(1).getString("color"));
Assert.assertNull(tags.getJsonObject(1).get("parent")); Assert.assertNull(tags.getJsonObject(1).get("parent"));
@ -189,7 +198,7 @@ public class TestTagResource extends BaseJerseyTest {
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token)
.get(JsonObject.class); .get(JsonObject.class);
tags = json.getJsonArray("tags"); tags = json.getJsonArray("tags");
Assert.assertTrue(tags.size() == 2); Assert.assertEquals(2, tags.size());
Assert.assertEquals(tag3Id, tags.getJsonObject(1).getString("parent")); Assert.assertEquals(tag3Id, tags.getJsonObject(1).getString("parent"));
// Deletes a tag // Deletes a tag
@ -202,7 +211,7 @@ public class TestTagResource extends BaseJerseyTest {
.cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token)
.get(JsonObject.class); .get(JsonObject.class);
tags = json.getJsonArray("tags"); tags = json.getJsonArray("tags");
Assert.assertTrue(tags.size() == 1); Assert.assertEquals(1, tags.size());
Assert.assertEquals("UpdatedName", tags.getJsonObject(0).getString("name")); Assert.assertEquals("UpdatedName", tags.getJsonObject(0).getString("name"));
Assert.assertNull(tags.getJsonObject(0).get("parent")); Assert.assertNull(tags.getJsonObject(0).get("parent"));
} }