Merge branch 'feature/RM-4357_AllowableOperations' into 'master'

Feature/rm 4357 allowable operations

RM-4357 - fileplanComponent's allowable actions don't reflect RM security

RM's capabilities overrides core's security so it is enough to test the capabilities. Please see the table from the issue's comment.

See merge request !624
This commit is contained in:
Ana Bozianu
2016-11-17 07:33:42 +00:00
3 changed files with 122 additions and 52 deletions

View File

@@ -27,6 +27,7 @@
<property name="quickShareLinks" ref="rm.QuickShareLinks"/>
<property name="filePlanService" ref="FilePlanService"/>
<property name="recordsManagementServiceRegistry" ref="RecordsManagementServiceRegistry"/>
<property name="capabilityService" ref="CapabilityService"/>
</bean>
<bean id="rm.Nodes" class="org.springframework.aop.framework.ProxyFactoryBean">

View File

@@ -37,6 +37,7 @@ import java.util.concurrent.ConcurrentHashMap;
import org.alfresco.model.ContentModel;
import org.alfresco.module.org_alfresco_module_rm.RecordsManagementServiceRegistry;
import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService;
import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionSchedule;
import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService;
import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
@@ -64,6 +65,8 @@ import org.alfresco.service.namespace.QName;
import org.alfresco.util.Pair;
import org.alfresco.util.ParameterCheck;
import net.sf.acegisecurity.vote.AccessDecisionVoter;
/**
* Centralizes access to the repository.
*
@@ -84,6 +87,7 @@ public class RMNodesImpl extends NodesImpl implements RMNodes
private Repository repositoryHelper;
private DictionaryService dictionaryService;
private DispositionService dispositionService;
private CapabilityService capabilityService;
/**
* TODO to remove this after isSpecialNode is made protected in core implementation(REPO-1459)
@@ -113,6 +117,11 @@ public class RMNodesImpl extends NodesImpl implements RMNodes
this.filePlanService = filePlanService;
}
public void setCapabilityService(CapabilityService capabilityService)
{
this.capabilityService = capabilityService;
}
@Override
public Node getFolderOrDocument(final NodeRef nodeRef, NodeRef parentNodeRef, QName nodeTypeQName, List<String> includeParam, Map<String, UserInfo> mapUserInfo)
{
@@ -123,26 +132,6 @@ public class RMNodesImpl extends NodesImpl implements RMNodes
nodeTypeQName = nodeService.getType(nodeRef);
}
//TODO to remove this part of code after isSpecialNode will be made protected on core, will not need this anymore since the right allowed operations will be returned from core(REPO-1459).
if (includeParam.contains(PARAM_INCLUDE_ALLOWABLEOPERATIONS) && originalNode.getAllowableOperations() != null)
{
List<String> allowableOperations = originalNode.getAllowableOperations();
List<String> modifiedAllowableOperations = new ArrayList<>();
modifiedAllowableOperations.addAll(allowableOperations);
for (String op : allowableOperations)
{
if (op.equals(OP_DELETE) && (isSpecialNode(nodeRef, nodeTypeQName)))
{
// special case: do not return "delete" (as an allowable op) for specific system nodes
modifiedAllowableOperations.remove(op);
}
}
originalNode.setAllowableOperations((modifiedAllowableOperations.size() > 0 )? modifiedAllowableOperations : null);
}
RMNodeType type = getType(nodeTypeQName, nodeRef);
FileplanComponentNode node = null;
if (mapUserInfo == null)
@@ -193,9 +182,56 @@ public class RMNodesImpl extends NodesImpl implements RMNodes
}
}
if (includeParam.contains(PARAM_INCLUDE_ALLOWABLEOPERATIONS))
{
node.setAllowableOperations(getAllowableOperations(nodeRef, type));
}
return node;
}
/**
* Helper method that generates allowable operation for the provided node
* @param nodeRef the node to get the allowable operations for
* @param type the type of the provided nodeRef
* @return a sublist of [{@link Nodes.OP_DELETE}, {@link Nodes.OP_CREATE}, {@link Nodes.OP_UPDATE}] representing the allowable operations for the provided node
*/
private List<String> getAllowableOperations(NodeRef nodeRef, RMNodeType type)
{
List<String> allowableOperations = new ArrayList<>();
NodeRef filePlan = filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID);
boolean isFilePlan = nodeRef.equals(filePlan);
boolean isTransferContainer = nodeRef.equals(filePlanService.getTransferContainer(filePlan));
boolean isUnfiledContainer = nodeRef.equals(filePlanService.getUnfiledContainer(filePlan));
boolean isHoldsContainer = nodeRef.equals(filePlanService.getHoldContainer(filePlan)) ;
boolean isSpecialContainer = isFilePlan || isTransferContainer || isUnfiledContainer || isHoldsContainer;
// DELETE
if(!isSpecialContainer &&
capabilityService.getCapability("Delete").evaluate(nodeRef) == AccessDecisionVoter.ACCESS_GRANTED)
{
allowableOperations.add(OP_DELETE);
}
// CREATE
if(type != RMNodeType.FILE &&
!isFilePlan &&
!isTransferContainer &&
capabilityService.getCapability("FillingPermissionOnly").evaluate(nodeRef) == AccessDecisionVoter.ACCESS_GRANTED)
{
allowableOperations.add(OP_CREATE);
}
// UPDATE
if (capabilityService.getCapability("Update").evaluate(nodeRef) == AccessDecisionVoter.ACCESS_GRANTED)
{
allowableOperations.add(OP_UPDATE);
}
return allowableOperations;
}
@Override
public NodeRef validateNode(String nodeId)
{

View File

@@ -30,7 +30,6 @@ package org.alfresco.rm.rest.api.impl;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
@@ -45,6 +44,8 @@ import java.util.ArrayList;
import java.util.List;
import org.alfresco.model.ContentModel;
import org.alfresco.module.org_alfresco_module_rm.capability.Capability;
import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService;
import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionSchedule;
import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
import org.alfresco.module.org_alfresco_module_rm.test.util.AlfMock;
@@ -74,6 +75,8 @@ import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import net.sf.acegisecurity.vote.AccessDecisionVoter;
/**
* Unit Test class for RMNodesImpl.
*
@@ -103,10 +106,17 @@ public class RMNodesImplUnitTest extends BaseUnitTest
@Mock
private ServiceRegistry mockedServiceRegistry;
@Mock
private CapabilityService mockedCapabilityService;
@InjectMocks
private RMNodesImpl rmNodesImpl;
private Capability deleteCapability;
private Capability createCapability;
private Capability updateCapability;
@Before
public void before()
{
@@ -117,6 +127,12 @@ public class RMNodesImplUnitTest extends BaseUnitTest
when(mockedNamespaceService.getPrefixes(any(String.class))).thenReturn(prefixes);
when(mockedNamespaceService.getNamespaceURI(any(String.class))).thenReturn(RM_URI);
deleteCapability = mock(Capability.class);
when(mockedCapabilityService.getCapability("Delete")).thenReturn(deleteCapability);
createCapability = mock(Capability.class);
when(mockedCapabilityService.getCapability("FillingPermissionOnly")).thenReturn(createCapability);
updateCapability = mock(Capability.class);
when(mockedCapabilityService.getCapability("Update")).thenReturn(updateCapability);
}
@Test
@@ -160,19 +176,13 @@ public class RMNodesImplUnitTest extends BaseUnitTest
setPermissions(nodeRef, AccessStatus.ALLOWED);
when(deleteCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(createCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(updateCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(mockedFilePlanService.getFilePlanBySiteId(RM_SITE_ID)).thenReturn(nodeRef);
Node folderOrDocument = rmNodesImpl.getFolderOrDocument(nodeRef, null, null, includeParamList, null);
assertNotNull(folderOrDocument);
assertTrue(FileplanComponentNode.class.isInstance(folderOrDocument));
FileplanComponentNode resultNode = (FileplanComponentNode) folderOrDocument;
assertEquals(false, resultNode.getIsRecordFolder());
assertEquals(false, resultNode.getIsFile());
assertEquals(false, resultNode.getIsCategory());
List<String> allowableOperations = resultNode.getAllowableOperations();
assertTrue("Create operation should be available for FilePlan.", allowableOperations.contains(RMNodes.OP_CREATE));
assertTrue("Update operation should be available for FilePlan.", allowableOperations.contains(RMNodes.OP_UPDATE));
assertFalse("Delete operation should note be available for FilePlan.", allowableOperations.contains(RMNodes.OP_DELETE));
checksAllowedOperations(folderOrDocument, false, true, false);
}
@Test
@@ -203,15 +213,15 @@ public class RMNodesImplUnitTest extends BaseUnitTest
assertEquals(false, resultNode.getIsFile());
assertEquals(false, resultNode.getIsCategory());
List<String> allowableOperations = resultNode.getAllowableOperations();
assertNull(allowableOperations);
assertEquals(0, allowableOperations.size());
}
@Test
public void testGetTransferContainerAllowableOperations() throws Exception
{
NodeRef nodeRef = AlfMock.generateNodeRef(mockedNodeService);
when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_RECORD_CATEGORY);
when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_RECORD_CATEGORY, ContentModel.TYPE_FOLDER)).thenReturn(true);
when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_TRANSFER_CONTAINER);
when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_TRANSFER_CONTAINER, ContentModel.TYPE_FOLDER)).thenReturn(true);
setupCompanyHomeAndPrimaryParent(nodeRef);
@@ -224,16 +234,22 @@ public class RMNodesImplUnitTest extends BaseUnitTest
when(mockedFilePlanService.getFilePlanBySiteId(RM_SITE_ID)).thenReturn(filePlanNodeRef);
when(mockedFilePlanService.getTransferContainer(filePlanNodeRef)).thenReturn(nodeRef);
when(deleteCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(createCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(updateCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(mockedFilePlanService.isFilePlanComponent(nodeRef)).thenReturn(true);
Node folderOrDocument = rmNodesImpl.getFolderOrDocument(nodeRef, null, null, includeParamList, null);
checkSpecialContainersAllowedOperations(folderOrDocument);
checksAllowedOperations(folderOrDocument, false, true, false);
}
@Test
public void testGetHoldContainerAllowableOperations() throws Exception
{
NodeRef nodeRef = AlfMock.generateNodeRef(mockedNodeService);
when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_RECORD_CATEGORY);
when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_RECORD_CATEGORY, ContentModel.TYPE_FOLDER)).thenReturn(true);
when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_HOLD_CONTAINER);
when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_HOLD_CONTAINER, ContentModel.TYPE_FOLDER)).thenReturn(true);
setupCompanyHomeAndPrimaryParent(nodeRef);
@@ -250,16 +266,22 @@ public class RMNodesImplUnitTest extends BaseUnitTest
when(mockedFilePlanService.getHoldContainer(filePlanNodeRef)).thenReturn(nodeRef);
when(deleteCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(createCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(updateCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(mockedFilePlanService.isFilePlanComponent(nodeRef)).thenReturn(true);
Node folderOrDocument = rmNodesImpl.getFolderOrDocument(nodeRef, null, null, includeParamList, null);
checkSpecialContainersAllowedOperations(folderOrDocument);
checksAllowedOperations(folderOrDocument, true, true, false);
}
@Test
public void testGetUnfiledContainerAllowableOperations() throws Exception
{
NodeRef nodeRef = AlfMock.generateNodeRef(mockedNodeService);
when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_RECORD_CATEGORY);
when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_RECORD_CATEGORY, ContentModel.TYPE_FOLDER)).thenReturn(true);
when(mockedNodeService.getType(nodeRef)).thenReturn(RecordsManagementModel.TYPE_UNFILED_RECORD_CONTAINER);
when(mockedDictionaryService.isSubClass(RecordsManagementModel.TYPE_UNFILED_RECORD_CONTAINER, ContentModel.TYPE_FOLDER)).thenReturn(true);
setupCompanyHomeAndPrimaryParent(nodeRef);
@@ -279,8 +301,14 @@ public class RMNodesImplUnitTest extends BaseUnitTest
when(mockedFilePlanService.getUnfiledContainer(filePlanNodeRef)).thenReturn(nodeRef);
when(deleteCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(createCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(updateCapability.evaluate(nodeRef)).thenReturn(AccessDecisionVoter.ACCESS_GRANTED);
when(mockedFilePlanService.isFilePlanComponent(nodeRef)).thenReturn(true);
Node folderOrDocument = rmNodesImpl.getFolderOrDocument(nodeRef, null, null, includeParamList, null);
checkSpecialContainersAllowedOperations(folderOrDocument);
checksAllowedOperations(folderOrDocument, true, true, false);
}
@Test
@@ -741,18 +769,23 @@ public class RMNodesImplUnitTest extends BaseUnitTest
when(mockedPermissionService.hasPermission(nodeRef, PermissionService.ADD_CHILDREN)).thenReturn(permissionToSet);
}
private void checkSpecialContainersAllowedOperations(Node containerNode)
private void checksAllowedOperations(Node containerNode, boolean allowCreate, boolean allowUpdate, boolean allowDelete)
{
assertNotNull(containerNode);
assertTrue(RecordCategoryNode.class.isInstance(containerNode));
RecordCategoryNode resultNode = (RecordCategoryNode) containerNode;
assertEquals(false, resultNode.getIsRecordFolder());
assertEquals(false, resultNode.getIsFile());
assertEquals(true, resultNode.getIsCategory());
assertTrue(FileplanComponentNode.class.isInstance(containerNode));
FileplanComponentNode resultNode = (FileplanComponentNode) containerNode;
List<String> allowableOperations = resultNode.getAllowableOperations();
assertTrue("Create operation should be available for provided container.", allowableOperations.contains(RMNodes.OP_CREATE));
assertTrue("Update operation should be available for provided container.", allowableOperations.contains(RMNodes.OP_UPDATE));
assertFalse("Delete operation should note be available for provided container.", allowableOperations.contains(RMNodes.OP_DELETE));
assertEquals("Create operation should " + (allowCreate?"":"not ") + "be available for provided container.",
allowCreate,
allowableOperations.contains(RMNodes.OP_CREATE));
assertEquals("Update operation should " + (allowCreate?"":"not ") + "be available for provided container.",
allowUpdate,
allowableOperations.contains(RMNodes.OP_UPDATE));
assertEquals("Delete operation should " + (allowCreate?"":"not ") + "be available for provided container.",
allowDelete,
allowableOperations.contains(RMNodes.OP_DELETE));
}
}