From 7baf5e44fd79c2678cea207ac0e5f9d10f943f3a Mon Sep 17 00:00:00 2001 From: Benjamin Gamard Date: Fri, 19 Oct 2018 17:04:49 +0200 Subject: [PATCH] Closes #237: prevent tag circular reference --- .../docs/rest/resource/TagResource.java | 16 +++++++++++++--- .../src/app/docs/controller/tag/TagEdit.js | 10 +++++++++- docs-web/src/main/webapp/src/locale/en.json | 4 +++- .../sismics/docs/rest/TestTagResource.java | 19 ++++++++++++++----- 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/docs-web/src/main/java/com/sismics/docs/rest/resource/TagResource.java b/docs-web/src/main/java/com/sismics/docs/rest/resource/TagResource.java index 27efc454..731e6307 100644 --- a/docs-web/src/main/java/com/sismics/docs/rest/resource/TagResource.java +++ b/docs-web/src/main/java/com/sismics/docs/rest/resource/TagResource.java @@ -192,7 +192,7 @@ public class TagResource extends BaseResource { throw new ClientException("ParentNotFound", MessageFormat.format("Parent not found: {0}", parentId)); } } - + // Create the tag TagDao tagDao = new TagDao(); Tag tag = new Tag(); @@ -239,6 +239,7 @@ public class TagResource extends BaseResource { * @apiError (client) ValidationError Validation error * @apiError (client) SpacesNotAllowed Spaces are not allowed in tag name * @apiError (client) ParentNotFound Parent not found + * @apiError (client) CircularReference Circular reference in parent tag * @apiError (client) NotFound Tag not found * @apiPermission user * @apiVersion 1.5.0 @@ -275,16 +276,25 @@ public class TagResource extends BaseResource { } // Check the parent + TagDao tagDao = new TagDao(); if (StringUtils.isEmpty(parentId)) { parentId = null; } else { if (!aclDao.checkPermission(parentId, PermType.READ, getTargetIdList(null))) { 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 - TagDao tagDao = new TagDao(); Tag tag = tagDao.getById(id); if (!StringUtils.isEmpty(name)) { tag.setName(name); diff --git a/docs-web/src/main/webapp/src/app/docs/controller/tag/TagEdit.js b/docs-web/src/main/webapp/src/app/docs/controller/tag/TagEdit.js index 1df1c2d2..35f2ded4 100644 --- a/docs-web/src/main/webapp/src/app/docs/controller/tag/TagEdit.js +++ b/docs-web/src/main/webapp/src/app/docs/controller/tag/TagEdit.js @@ -21,7 +21,15 @@ angular.module('docs').controller('TagEdit', function($scope, $stateParams, Rest */ $scope.edit = function() { // 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); + } + }); }; /** diff --git a/docs-web/src/main/webapp/src/locale/en.json b/docs-web/src/main/webapp/src/locale/en.json index 7d83ef1d..e20d7514 100644 --- a/docs-web/src/main/webapp/src/locale/en.json +++ b/docs-web/src/main/webapp/src/locale/en.json @@ -219,7 +219,9 @@ "name": "Name", "color": "Color", "parent": "Parent", - "info": "Permissions on this tag will also be applied to documents tagged {{ name }}" + "info": "Permissions on this tag will also be applied to documents tagged {{ name }}", + "circular_reference_title": "Circular reference", + "circular_reference_message": "The hierarchy of parent tags makes a loop, please choose another parent." } }, "group": { diff --git a/docs-web/src/test/java/com/sismics/docs/rest/TestTagResource.java b/docs-web/src/test/java/com/sismics/docs/rest/TestTagResource.java index a8f8fee1..b8f4b5fa 100644 --- a/docs-web/src/test/java/com/sismics/docs/rest/TestTagResource.java +++ b/docs-web/src/test/java/com/sismics/docs/rest/TestTagResource.java @@ -45,6 +45,15 @@ public class TestTagResource extends BaseJerseyTest { String tag4Id = json.getString("id"); 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 json = target().path("/tag/" + tag4Id).request() .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) @@ -58,7 +67,7 @@ public class TestTagResource extends BaseJerseyTest { Assert.assertEquals(2, acls.size()); // Create a tag with space (not allowed) - Response response = target().path("/tag").request() + response = target().path("/tag").request() .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .put(Entity.form(new Form() .param("name", "Tag 4"))); @@ -152,7 +161,7 @@ public class TestTagResource extends BaseJerseyTest { .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .get(JsonObject.class); tags = json.getJsonArray("tags"); - Assert.assertTrue(tags.size() == 2); + Assert.assertEquals(2, tags.size()); Assert.assertEquals("Tag4", tags.getJsonObject(1).getString("name")); Assert.assertEquals("#00ff00", tags.getJsonObject(1).getString("color")); Assert.assertEquals(tag3Id, tags.getJsonObject(1).getString("parent")); @@ -170,7 +179,7 @@ public class TestTagResource extends BaseJerseyTest { .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .get(JsonObject.class); tags = json.getJsonArray("tags"); - Assert.assertTrue(tags.size() == 2); + Assert.assertEquals(2, tags.size()); Assert.assertEquals("UpdatedName", tags.getJsonObject(1).getString("name")); Assert.assertEquals("#0000ff", tags.getJsonObject(1).getString("color")); Assert.assertNull(tags.getJsonObject(1).get("parent")); @@ -189,7 +198,7 @@ public class TestTagResource extends BaseJerseyTest { .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .get(JsonObject.class); tags = json.getJsonArray("tags"); - Assert.assertTrue(tags.size() == 2); + Assert.assertEquals(2, tags.size()); Assert.assertEquals(tag3Id, tags.getJsonObject(1).getString("parent")); // Deletes a tag @@ -202,7 +211,7 @@ public class TestTagResource extends BaseJerseyTest { .cookie(TokenBasedSecurityFilter.COOKIE_NAME, tag1Token) .get(JsonObject.class); tags = json.getJsonArray("tags"); - Assert.assertTrue(tags.size() == 1); + Assert.assertEquals(1, tags.size()); Assert.assertEquals("UpdatedName", tags.getJsonObject(0).getString("name")); Assert.assertNull(tags.getJsonObject(0).get("parent")); }