diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java index 525daa03f7..926c00e5d5 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java +++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java @@ -27,6 +27,7 @@ package org.alfresco.rest.rules; import static java.util.stream.Collectors.toList; +import static org.alfresco.utility.constants.UserRole.SiteCollaborator; import static org.alfresco.utility.model.FileModel.getRandomFileModel; import static org.alfresco.utility.model.FileType.TEXT_PLAIN; import static org.alfresco.utility.report.log.Step.STEP; @@ -145,7 +146,7 @@ public class CreateRulesTests extends RestTest } /** Check that a user without permission to view the folder cannot create a rule in it. */ - public void requirePermissionToCreateRule() + public void requireReadPermissionToCreateRule() { STEP("Create a user and use them to create a private site containing a folder"); UserModel privateUser = dataUser.createRandomTestUser(); @@ -159,7 +160,27 @@ public class CreateRulesTests extends RestTest restClient.authenticateUser(user).withCoreAPI().usingNode(privateFolder).usingDefaultRuleSet().createSingleRule(ruleModel); restClient.assertStatusCodeIs(FORBIDDEN); - restClient.assertLastError().containsSummary("Cannot read from this node"); + restClient.assertLastError().containsSummary("Insufficient permissions to manage rules"); + } + + /** Check that a user without write permission for the folder cannot create a rule in it. */ + public void requireWritePermissionToCreateRule() + { + STEP("Create a user and use them to create a private site containing a folder"); + UserModel privateUser = dataUser.createRandomTestUser(); + SiteModel privateSite = dataSite.usingUser(privateUser).createPrivateRandomSite(); + FolderModel privateFolder = dataContent.usingUser(privateUser).usingSite(privateSite).createFolder(); + + STEP("Create a collaborator and check they cannot create a rule in the private folder"); + UserModel collaborator = dataUser.createRandomTestUser(); + dataUser.addUserToSite(collaborator, privateSite, SiteCollaborator); + RestRuleModel ruleModel = new RestRuleModel(); + ruleModel.setName("ruleName"); + + restClient.authenticateUser(collaborator).withCoreAPI().usingNode(privateFolder).usingDefaultRuleSet().createSingleRule(ruleModel); + + restClient.assertStatusCodeIs(FORBIDDEN); + restClient.assertLastError().containsSummary("Insufficient permissions to manage rules"); } /** Check we can't create a rule under a document node. */ diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/RulesImpl.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/RulesImpl.java index 11ccde66b4..0222790c0c 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/RulesImpl.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/RulesImpl.java @@ -26,6 +26,9 @@ package org.alfresco.rest.api.impl; +import static org.alfresco.service.cmr.security.AccessStatus.ALLOWED; +import static org.alfresco.service.cmr.security.PermissionService.CHANGE_PERMISSIONS; + import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -44,7 +47,6 @@ import org.alfresco.rest.framework.resource.parameters.Paging; import org.alfresco.service.Experimental; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.rule.RuleService; -import org.alfresco.service.cmr.security.AccessStatus; import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.namespace.QName; @@ -62,7 +64,7 @@ public class RulesImpl implements Rules @Override public CollectionWithPagingInfo getRules(final String folderNodeId, final String ruleSetId, final Paging paging) { - final NodeRef folderNodeRef = validateFolderNode(folderNodeId); + final NodeRef folderNodeRef = validateFolderNode(folderNodeId, false); validateRuleSetNode(ruleSetId, folderNodeRef); final List rules = ruleService.getRules(folderNodeRef).stream() @@ -75,7 +77,7 @@ public class RulesImpl implements Rules @Override public Rule getRuleById(final String folderNodeId, final String ruleSetId, final String ruleId) { - final NodeRef folderNodeRef = validateFolderNode(folderNodeId); + final NodeRef folderNodeRef = validateFolderNode(folderNodeId, false); final NodeRef ruleSetNodeRef = validateRuleSetNode(ruleSetId, folderNodeRef); final NodeRef ruleNodeRef = validateRuleNode(ruleId, ruleSetNodeRef); @@ -85,7 +87,7 @@ public class RulesImpl implements Rules @Override public List createRules(final String folderNodeId, final String ruleSetId, final List rules) { - final NodeRef folderNodeRef = validateFolderNode(folderNodeId); + final NodeRef folderNodeRef = validateFolderNode(folderNodeId, true); // Don't validate the ruleset node if -default- is passed since we may need to create it. if (RuleSet.isNotDefaultId(ruleSetId)) { @@ -102,7 +104,7 @@ public class RulesImpl implements Rules @Override public void deleteRuleById(String folderNodeId, String ruleSetId, String ruleId) { - final NodeRef folderNodeRef = validateFolderNode(folderNodeId); + final NodeRef folderNodeRef = validateFolderNode(folderNodeId, true); final NodeRef ruleSetNodeRef = validateRuleSetNode(ruleSetId, folderNodeRef); final NodeRef ruleNodeRef = validateRuleNode(ruleId, ruleSetNodeRef); final org.alfresco.service.cmr.rule.Rule rule = ruleService.getRule(ruleNodeRef); @@ -125,20 +127,31 @@ public class RulesImpl implements Rules } /** - * Validates if folder node exists and user have permission to read from it. + * Validates if folder node exists and the user has permission to use it. * * @param folderNodeId - folder node ID + * @param requireChangePermission - Whether to require change permission or just read permission. * @return folder node reference * @throws InvalidArgumentException if node is not of an expected type - * @throws PermissionDeniedException if user doesn't have right to read from folder + * @throws PermissionDeniedException if the user doesn't have the appropriate permission for the folder. */ - private NodeRef validateFolderNode(final String folderNodeId) + private NodeRef validateFolderNode(final String folderNodeId, boolean requireChangePermission) { final NodeRef nodeRef = nodes.validateOrLookupNode(folderNodeId, null); - if (permissionService.hasReadPermission(nodeRef) != AccessStatus.ALLOWED) { - throw new PermissionDeniedException("Cannot read from this node!"); + if (requireChangePermission) + { + if (permissionService.hasPermission(nodeRef, CHANGE_PERMISSIONS) != ALLOWED) + { + throw new PermissionDeniedException("Insufficient permissions to manage rules."); + } + } + else + { + if (permissionService.hasReadPermission(nodeRef) != ALLOWED) + { + throw new PermissionDeniedException("Cannot read from this node!"); + } } - verifyNodeType(nodeRef, ContentModel.TYPE_FOLDER, null); return nodeRef; diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/RulesImplTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/RulesImplTest.java index 90b3ae0efb..bfffd2436f 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/RulesImplTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/RulesImplTest.java @@ -29,6 +29,9 @@ package org.alfresco.rest.api.impl; import static java.util.Collections.emptyList; import static org.alfresco.rest.api.model.rules.RuleSet.DEFAULT_ID; +import static org.alfresco.service.cmr.security.AccessStatus.ALLOWED; +import static org.alfresco.service.cmr.security.AccessStatus.DENIED; +import static org.alfresco.service.cmr.security.PermissionService.CHANGE_PERMISSIONS; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.ArgumentMatchers.any; @@ -54,7 +57,6 @@ import org.alfresco.service.Experimental; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.rule.RuleService; -import org.alfresco.service.cmr.security.AccessStatus; import org.alfresco.service.cmr.security.PermissionService; import org.junit.Before; import org.junit.Test; @@ -98,7 +100,8 @@ public class RulesImplTest extends TestCase given(nodesMock.validateOrLookupNode(eq(FOLDER_NODE_ID), any())).willReturn(folderNodeRef); given(nodesMock.validateNode(RULE_SET_ID)).willReturn(ruleSetNodeRef); given(nodesMock.nodeMatches(any(), any(), any())).willReturn(true); - given(permissionServiceMock.hasReadPermission(any())).willReturn(AccessStatus.ALLOWED); + given(permissionServiceMock.hasReadPermission(any())).willReturn(ALLOWED); + given(permissionServiceMock.hasPermission(any(), any())).willReturn(ALLOWED); } @Test @@ -201,7 +204,7 @@ public class RulesImplTest extends TestCase @Test public void testGetRulesWithoutReadPermission() { - given(permissionServiceMock.hasReadPermission(any())).willReturn(AccessStatus.DENIED); + given(permissionServiceMock.hasReadPermission(any())).willReturn(DENIED); // when assertThatExceptionOfType(PermissionDeniedException.class).isThrownBy( @@ -392,6 +395,22 @@ public class RulesImplTest extends TestCase assertThat(actual).isEqualTo(expected); } + /** Try to create a rule without CHANGE permission and check an exception is thrown. */ + @Test + public void testSaveRules_noChangePermission() + { + given(permissionServiceMock.hasPermission(folderNodeRef, CHANGE_PERMISSIONS)).willReturn(DENIED); + + Rule ruleBody = mock(Rule.class); + List ruleList = List.of(ruleBody); + + // when + assertThatExceptionOfType(PermissionDeniedException.class).isThrownBy(() -> + rules.createRules(folderNodeRef.getId(), ruleSetNodeRef.getId(), ruleList)); + + then(ruleServiceMock).shouldHaveNoMoreInteractions(); + } + @Test public void testDeleteRuleById() { given(nodesMock.validateNode(RULE_SET_ID)).willReturn(ruleSetNodeRef); @@ -411,7 +430,7 @@ public class RulesImplTest extends TestCase then(nodesMock).should().nodeMatches(eq(ruleSetNodeRef), any(), isNull()); then(nodesMock).should().nodeMatches(eq(ruleNodeRef), any(), isNull()); then(nodesMock).shouldHaveNoMoreInteractions(); - then(permissionServiceMock).should().hasReadPermission(folderNodeRef); + then(permissionServiceMock).should().hasPermission(folderNodeRef, CHANGE_PERMISSIONS); then(permissionServiceMock).shouldHaveNoMoreInteractions(); then(ruleServiceMock).should().isRuleSetAssociatedWithFolder(ruleSetNodeRef, folderNodeRef); then(ruleServiceMock).should().isRuleAssociatedWithRuleSet(ruleNodeRef, ruleSetNodeRef); @@ -436,7 +455,7 @@ public class RulesImplTest extends TestCase then(nodesMock).should().nodeMatches(eq(folderNodeRef), any(), isNull()); then(nodesMock).should().nodeMatches(eq(ruleSetNodeRef), any(), isNull()); then(nodesMock).shouldHaveNoMoreInteractions(); - then(permissionServiceMock).should().hasReadPermission(folderNodeRef); + then(permissionServiceMock).should().hasPermission(folderNodeRef, CHANGE_PERMISSIONS); then(permissionServiceMock).shouldHaveNoMoreInteractions(); then(ruleServiceMock).should().isRuleSetAssociatedWithFolder(ruleSetNodeRef, folderNodeRef); then(ruleServiceMock).shouldHaveNoMoreInteractions(); @@ -460,7 +479,7 @@ public class RulesImplTest extends TestCase then(nodesMock).should().nodeMatches(eq(ruleSetNodeRef), any(), isNull()); then(nodesMock).should().nodeMatches(eq(ruleNodeRef), any(), isNull()); then(nodesMock).shouldHaveNoMoreInteractions(); - then(permissionServiceMock).should().hasReadPermission(folderNodeRef); + then(permissionServiceMock).should().hasPermission(folderNodeRef, CHANGE_PERMISSIONS); then(permissionServiceMock).shouldHaveNoMoreInteractions(); then(ruleServiceMock).should().isRuleSetAssociatedWithFolder(ruleSetNodeRef, folderNodeRef); then(ruleServiceMock).should().isRuleAssociatedWithRuleSet(ruleNodeRef, ruleSetNodeRef); @@ -479,7 +498,7 @@ public class RulesImplTest extends TestCase then(nodesMock).should().validateNode(RULE_SET_ID); then(nodesMock).should().nodeMatches(eq(folderNodeRef), any(), isNull()); then(nodesMock).shouldHaveNoMoreInteractions(); - then(permissionServiceMock).should().hasReadPermission(folderNodeRef); + then(permissionServiceMock).should().hasPermission(folderNodeRef, CHANGE_PERMISSIONS); then(permissionServiceMock).shouldHaveNoMoreInteractions(); then(ruleServiceMock).shouldHaveNoMoreInteractions(); } @@ -498,7 +517,7 @@ public class RulesImplTest extends TestCase then(nodesMock).should().nodeMatches(eq(folderNodeRef), any(), isNull()); then(nodesMock).should().nodeMatches(eq(ruleSetNodeRef), any(), isNull()); then(nodesMock).shouldHaveNoMoreInteractions(); - then(permissionServiceMock).should().hasReadPermission(folderNodeRef); + then(permissionServiceMock).should().hasPermission(folderNodeRef, CHANGE_PERMISSIONS); then(permissionServiceMock).shouldHaveNoMoreInteractions(); then(ruleServiceMock).should().isRuleSetAssociatedWithFolder(ruleSetNodeRef, folderNodeRef); then(ruleServiceMock).shouldHaveNoMoreInteractions(); @@ -518,6 +537,20 @@ public class RulesImplTest extends TestCase then(ruleServiceMock).shouldHaveNoMoreInteractions(); } + /** Try to delete a rule without CHANGE permission and check an exception is thrown. */ + @Test + public void testDeleteRuleById_noChangePermission() + { + given(permissionServiceMock.hasPermission(folderNodeRef, CHANGE_PERMISSIONS)).willReturn(DENIED); + + // when + assertThatExceptionOfType(PermissionDeniedException.class).isThrownBy(() -> + rules.deleteRuleById(folderNodeRef.getId(), ruleSetNodeRef.getId(), RULE_ID)); + + then(ruleServiceMock).shouldHaveNoMoreInteractions(); + } + + private static org.alfresco.service.cmr.rule.Rule createRule(final String id) { final org.alfresco.service.cmr.rule.Rule rule = new org.alfresco.service.cmr.rule.Rule(); rule.setNodeRef(new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, id));