From 855e2522f2c84087f38fbb06a561115e80a226e5 Mon Sep 17 00:00:00 2001 From: Tom Page Date: Thu, 6 Oct 2022 16:35:29 +0100 Subject: [PATCH] ACS-3657 Allow returning partial list of rule sets. If a user does not have access to a rule set applied to a node then it will be excluded from the results, but the user will be able to see the list of other rule sets. Also add E2E tests for permissions when viewing rule sets. --- .../alfresco/rest/rules/GetRuleSetsTests.java | 82 +++++++++++++++++++ .../rest/api/impl/rules/RuleSetsImpl.java | 36 +++++++- .../rest/api/impl/rules/RuleSetsImplTest.java | 29 +++++++ 3 files changed, 145 insertions(+), 2 deletions(-) diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/GetRuleSetsTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/GetRuleSetsTests.java index 4feaa7586e..7c117d643d 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/GetRuleSetsTests.java +++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/GetRuleSetsTests.java @@ -31,6 +31,7 @@ import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModelWithDefault import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModelWithModifiedValues; import static org.alfresco.utility.report.log.Step.STEP; import static org.junit.Assert.assertTrue; +import static org.springframework.http.HttpStatus.FORBIDDEN; import static org.springframework.http.HttpStatus.NOT_FOUND; import static org.springframework.http.HttpStatus.OK; @@ -62,6 +63,8 @@ public class GetRuleSetsTests extends RestTest private UserModel user; private SiteModel site; private FolderModel ruleFolder; + /** A folder with a rule in a private site owned by admin. */ + private FolderModel privateFolder; private FolderModel inheritingChildFolder; private FolderModel notInheritingChildFolder; private RestRuleModel rule; @@ -93,6 +96,11 @@ public class GetRuleSetsTests extends RestTest .getListOfRuleSets(); ruleSets.assertThat().entriesListCountIs(1); ruleSetId = ruleSets.getEntries().get(0).onModel().getId(); + + STEP("Use admin to create a private site containing a rule in a rule set that can be inherited."); + SiteModel privateSite = dataSite.usingAdmin().createPrivateRandomSite(); + privateFolder = dataContent.usingAdmin().usingSite(privateSite).createFolder(); + coreAPIForAdmin().usingNode(privateFolder).usingDefaultRuleSet().createSingleRule(createRuleModelWithModifiedValues()); } /** Check we can get an empty list of rule sets. */ @@ -135,6 +143,48 @@ public class GetRuleSetsTests extends RestTest restClient.assertStatusCodeIs(NOT_FOUND); } + /** Check that we get a 403 error when trying to get rule sets for a folder we don't have read access to. */ + @Test (groups = { TestGroup.REST_API, TestGroup.RULES }) + public void getRuleSetsWithoutPermission() + { + STEP("Check a user cannot list rule sets without read access."); + coreAPIForUser().usingNode(privateFolder).getListOfRuleSets(); + restClient.assertStatusCodeIs(FORBIDDEN); + } + + /** Check that we can still list some rule sets if we don't have permission to view them all. */ + @Test (groups = { TestGroup.REST_API, TestGroup.RULES }) + public void permissionsAreRespectedWhenListingRuleSets() + { + STEP("Create a public site containing a parent and child folder with rule inheritance enabled."); + SiteModel publicSite = dataSite.usingUser(user).createPublicRandomSite(); + FolderModel parentFolder = dataContent.usingUser(user).usingSite(publicSite).createFolder(); + FolderModel childFolder = dataContent.usingUser(user).usingResource(parentFolder).createFolder(); + RestRuleSettingsModel enabled = new RestRuleSettingsModel(); + enabled.setValue(true); + coreAPIForUser().usingNode(parentFolder).usingRuleSetting(IS_INHERITANCE_ENABLED).updateSetting(enabled); + + STEP("Link the parent folder to a private rule set."); + RestRuleSetLinkModel linkModel = new RestRuleSetLinkModel(); + linkModel.setId(privateFolder.getNodeRef()); + coreAPIForAdmin().usingNode(parentFolder).createRuleLink(linkModel); + + STEP("Create a rule on the child folder."); + coreAPIForUser().usingNode(childFolder).usingDefaultRuleSet().createSingleRule(createRuleModelWithDefaultValues()); + + STEP("Check admin can view both rule sets."); + RestRuleSetModelsCollection adminViewOfRuleSets = coreAPIForAdmin().usingNode(childFolder).getListOfRuleSets(); + restClient.assertStatusCodeIs(OK); + RestRuleSetModel parentRuleSet = adminViewOfRuleSets.getEntries().get(0).onModel(); + RestRuleSetModel childRuleSet = adminViewOfRuleSets.getEntries().get(1).onModel(); + + STEP("Check the normal user can only view the child rule set."); + RestRuleSetModelsCollection userViewOfRuleSets = coreAPIForUser().usingNode(childFolder).getListOfRuleSets(); + restClient.assertStatusCodeIs(OK); + userViewOfRuleSets.assertThat().entriesListContains("id", childRuleSet.getId()) + .and().entriesListDoesNotContain("id", parentRuleSet.getId()); + } + /** Check we can get the id of the folder that owns a list of rule sets. */ @Test (groups = { TestGroup.REST_API, TestGroup.RULES }) public void getRuleSetsAndOwningFolders() @@ -503,6 +553,38 @@ public class GetRuleSetsTests extends RestTest ruleSet.assertThat().field("isLinkedTo").is(false); } + /** Check that we can only view a rule set if have read permission. */ + @Test (groups = { TestGroup.REST_API, TestGroup.RULES }) + public void permissionsChecksForFolderWithPrivateAndPublicRuleSets() + { + STEP("Create a public site containing a parent and child folder with rule inheritance enabled."); + SiteModel publicSite = dataSite.usingUser(user).createPublicRandomSite(); + FolderModel parentFolder = dataContent.usingUser(user).usingSite(publicSite).createFolder(); + FolderModel childFolder = dataContent.usingUser(user).usingResource(parentFolder).createFolder(); + RestRuleSettingsModel enabled = new RestRuleSettingsModel(); + enabled.setValue(true); + coreAPIForUser().usingNode(parentFolder).usingRuleSetting(IS_INHERITANCE_ENABLED).updateSetting(enabled); + + STEP("Link the parent folder to a private rule set."); + RestRuleSetLinkModel linkModel = new RestRuleSetLinkModel(); + linkModel.setId(privateFolder.getNodeRef()); + coreAPIForAdmin().usingNode(parentFolder).createRuleLink(linkModel); + + STEP("Create a rule on the child folder."); + coreAPIForUser().usingNode(childFolder).usingDefaultRuleSet().createSingleRule(createRuleModelWithDefaultValues()); + + STEP("Use the admin user to get both rule sets."); + RestRuleSetModelsCollection adminViewOfRuleSets = coreAPIForAdmin().usingNode(childFolder).getListOfRuleSets(); + RestRuleSetModel parentRuleSet = adminViewOfRuleSets.getEntries().get(0).onModel(); + RestRuleSetModel childRuleSet = adminViewOfRuleSets.getEntries().get(1).onModel(); + + STEP("Check the normal user can only view the child rule set."); + coreAPIForUser().usingNode(childFolder).getRuleSet(parentRuleSet.getId()); + restClient.assertStatusCodeIs(FORBIDDEN); + coreAPIForUser().usingNode(childFolder).getRuleSet(childRuleSet.getId()); + restClient.assertStatusCodeIs(OK); + } + private RestCoreAPI coreAPIForUser() { return restClient.authenticateUser(user).withCoreAPI(); diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RuleSetsImpl.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RuleSetsImpl.java index 61f673f648..4c99e81225 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RuleSetsImpl.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RuleSetsImpl.java @@ -39,6 +39,7 @@ import java.util.stream.IntStream; import org.alfresco.repo.rule.RuleModel; import org.alfresco.repo.rule.RuntimeRuleService; +import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.rest.api.RuleSets; import org.alfresco.rest.api.model.rules.RuleSet; import org.alfresco.rest.api.model.rules.RuleSetLink; @@ -50,10 +51,14 @@ import org.alfresco.service.Experimental; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.rule.RuleService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; @Experimental public class RuleSetsImpl implements RuleSets { + private static final Logger LOGGER = LoggerFactory.getLogger(RuleSetsImpl.class); + private RuleSetLoader ruleSetLoader; private RuleService ruleService; private NodeValidator validator; @@ -67,15 +72,42 @@ public class RuleSetsImpl implements RuleSets List ruleSets = ruleService.getNodesSupplyingRuleSets(folderNode) .stream() - .map(ruleService::getRuleSetNode) + .map(supplyingNode -> loadRuleSet(supplyingNode, folderNode, includes)) .filter(Objects::nonNull) - .map(nodeRef -> ruleSetLoader.loadRuleSet(nodeRef, folderNode, includes)) .distinct() .collect(toList()); return ListPage.of(ruleSets, paging); } + /** + * Load the specified rule set if the user has permission. + * + * @param supplyingNode The folder supplying a rule set. + * @param folderNode The folder being supplied with rule sets. + * @param includes The list of optional fields to include for each rule set in the response. + * @return The rule set from the DB or null if the folder has no rule set, or the current user does not have permission to view it. + */ + private RuleSet loadRuleSet(NodeRef supplyingNode, NodeRef folderNode, List includes) + { + NodeRef ruleSetNode = ruleService.getRuleSetNode(supplyingNode); + // Check if the folder has no rule sets. + if (ruleSetNode == null) + { + return null; + } + + try + { + return ruleSetLoader.loadRuleSet(ruleSetNode, folderNode, includes); + } + catch (AccessDeniedException e) + { + LOGGER.debug("User does not have permission to view rule set with id {}.", ruleSetNode, e); + return null; + } + } + @Override public RuleSet getRuleSetById(String folderNodeId, String ruleSetId, List includes) { diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RuleSetsImplTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RuleSetsImplTest.java index 0619f59864..f0402b304c 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RuleSetsImplTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RuleSetsImplTest.java @@ -42,6 +42,7 @@ import java.util.List; import junit.framework.TestCase; import org.alfresco.repo.rule.RuleModel; import org.alfresco.repo.rule.RuntimeRuleService; +import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.rest.api.model.rules.RuleSet; import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo; @@ -146,6 +147,34 @@ public class RuleSetsImplTest extends TestCase assertEquals(PAGING, actual.getPaging()); } + @Test + public void testOnlyGetPermittedRuleSets() + { + // Simulate a private folder with a rule set that the current user can't access. + NodeRef privateFolder = new NodeRef("private://folder/"); + NodeRef privateRuleSetNode = new NodeRef("private://rule/set/node/"); + given(ruleServiceMock.getRuleSetNode(privateFolder)).willReturn(privateRuleSetNode); + given(ruleServiceMock.getNodesSupplyingRuleSets(FOLDER_NODE)).willReturn(List.of(FOLDER_NODE, privateFolder)); + given(ruleSetLoaderMock.loadRuleSet(eq(privateRuleSetNode), any(NodeRef.class), any(List.class))) + .willThrow(new AccessDeniedException("Cannot access private rule set.")); + + // Call the method under test. + CollectionWithPagingInfo actual = ruleSets.getRuleSets(FOLDER_ID, INCLUDES, PAGING); + + then(nodeValidatorMock).should().validateFolderNode(FOLDER_ID, false); + then(nodeValidatorMock).shouldHaveNoMoreInteractions(); + + then(ruleServiceMock).should().getNodesSupplyingRuleSets(FOLDER_NODE); + then(ruleServiceMock).should().getRuleSetNode(FOLDER_NODE); + then(ruleServiceMock).should().getRuleSetNode(privateFolder); + then(ruleServiceMock).shouldHaveNoMoreInteractions(); + + // Check we only get the accessible rule set back. + Collection expected = List.of(ruleSetMock); + assertEquals(expected, actual.getCollection()); + assertEquals(PAGING, actual.getPaging()); + } + /** Check that a folder with a parent and grandparent can inherit rule sets from the grandparent, even if the parent has no rules. */ @Test public void testGetInheritedRuleSets()