From aaab4b2c9eba799f30ff315ce8b1c28f56b5b108 Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Mon, 3 Dec 2012 08:10:21 +0000 Subject: [PATCH] RM: Fallout from previous model security service changes * we don't need to evaluate the capabilites, just need to know if the user 'has' the capability * added ebable/disable * disabled for now since code refactor is complete, but we need to think some more about what (and why) some properties and aspects are protected git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@44230 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../rm-service-context.xml | 4 +- .../model/security/ModelSecurityService.java | 14 ++ .../security/ModelSecurityServiceImpl.java | 161 +++++++++++------- .../security/ProtectedModelArtifact.java | 23 ++- .../test/ServicesTestSuite.java | 4 + .../service/ModelSecurityServiceImplTest.java | 59 ++++--- 6 files changed, 181 insertions(+), 84 deletions(-) diff --git a/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml b/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml index b51d0c86e3..b891db5214 100644 --- a/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml +++ b/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml @@ -550,10 +550,12 @@ + - + + diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ModelSecurityService.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ModelSecurityService.java index f6b54ae559..5dc8db2285 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ModelSecurityService.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ModelSecurityService.java @@ -31,6 +31,20 @@ import org.alfresco.service.namespace.QName; */ public interface ModelSecurityService { + /** + * Sets whether model security is enabled or not. + * + * @param enabled + */ + void setEnabled(boolean enabled); + + /** + * Indicates whether model security is enabled or not. + * + * @return + */ + boolean isEnabled(); + /** * Registers a protected model artifact with the service. * diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ModelSecurityServiceImpl.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ModelSecurityServiceImpl.java index 7f0285bb45..efa8173611 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ModelSecurityServiceImpl.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ModelSecurityServiceImpl.java @@ -24,9 +24,11 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; -import org.alfresco.module.org_alfresco_module_rm.capability.Capability; +import org.alfresco.module.org_alfresco_module_rm.RecordsManagementService; import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; +import org.alfresco.module.org_alfresco_module_rm.security.RecordsManagementSecurityService; +import org.alfresco.module.org_alfresco_module_rm.security.Role; import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.policy.JavaBehaviour; import org.alfresco.repo.policy.PolicyComponent; @@ -34,7 +36,6 @@ import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; -import org.alfresco.service.cmr.security.AccessStatus; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.util.EqualsHelper; @@ -54,18 +55,24 @@ public class ModelSecurityServiceImpl implements ModelSecurityService, NodeServicePolicies.BeforeRemoveAspectPolicy, NodeServicePolicies.OnUpdatePropertiesPolicy { + /** Indicates whether model security is enabled or not */ + private boolean enabled = true; + /** Policy component */ private PolicyComponent policyComponent; /** Node service */ private NodeService nodeService; - /** Capability service */ - private CapabilityService capabilityService; - /** Namespace service */ private NamespaceService namespaceService; + /** Security service */ + private RecordsManagementSecurityService securityService; + + /** Records management service */ + private RecordsManagementService recordsManagementService; + /** Map of protected properties keyed by name */ private Map protectedProperties = new HashMap(21); @@ -83,6 +90,22 @@ public class ModelSecurityServiceImpl implements ModelSecurityService, "onUpdateProperties", NotificationFrequency.EVERY_EVENT); + /** + * @see org.alfresco.module.org_alfresco_module_rm.model.security.ModelSecurityService#setEnabled(boolean) + */ + public void setEnabled(boolean enabled) + { + this.enabled = enabled; + } + + /** + * @see org.alfresco.module.org_alfresco_module_rm.model.security.ModelSecurityService#isEnabled() + */ + public boolean isEnabled() + { + return enabled; + } + /** * @param policyComponent policy component */ @@ -99,14 +122,6 @@ public class ModelSecurityServiceImpl implements ModelSecurityService, this.nodeService = nodeService; } - /** - * @param capabilityService capability service - */ - public void setCapabilityService(CapabilityService capabilityService) - { - this.capabilityService = capabilityService; - } - /** * @param namespaceService namespace service */ @@ -115,6 +130,22 @@ public class ModelSecurityServiceImpl implements ModelSecurityService, this.namespaceService = namespaceService; } + /** + * @param securityService records management security service + */ + public void setSecurityService(RecordsManagementSecurityService securityService) + { + this.securityService = securityService; + } + + /** + * @param recordsManagementService records management service + */ + public void setRecordsManagementService(RecordsManagementService recordsManagementService) + { + this.recordsManagementService = recordsManagementService; + } + /** * Init method */ @@ -213,13 +244,17 @@ public class ModelSecurityServiceImpl implements ModelSecurityService, { boolean result = false; - for (Capability capability : artifact.getCapabilities()) + NodeRef filePlan = recordsManagementService.getFilePlan(nodeRef); + if (filePlan != null) { - AccessStatus accessStatus = capabilityService.getCapabilityAccessState(nodeRef, capability.getName()); - if (AccessStatus.ALLOWED.equals(accessStatus) == true) + Set roles = securityService.getRolesByUser(filePlan, AuthenticationUtil.getFullyAuthenticatedUser()); + for (Role role : roles) { - result = true; - break; + if (Collections.disjoint(role.getCapabilities(), artifact.getCapilityNames()) == false) + { + result = true; + break; + } } } @@ -280,17 +315,20 @@ public class ModelSecurityServiceImpl implements ModelSecurityService, @Override public void beforeAddAspect(NodeRef nodeRef, QName aspect) { - if (AuthenticationUtil.getFullyAuthenticatedUser() != null && - AuthenticationUtil.isRunAsUserTheSystemUser() == false && - isProtectedAspect(aspect) == true && - nodeService.exists(nodeRef) == true && - canEditProtectedAspect(nodeRef, aspect) == false) + if (enabled == true) { - // the user can't edit the protected aspect - throw new ModelAccessDeniedException( - "The user " + AuthenticationUtil.getFullyAuthenticatedUser() + - " does not have the permission to add the protected aspect " + aspect.toPrefixString(namespaceService) + - " from the node " + nodeRef.toString()); + if (AuthenticationUtil.getFullyAuthenticatedUser() != null && + AuthenticationUtil.isRunAsUserTheSystemUser() == false && + isProtectedAspect(aspect) == true && + nodeService.exists(nodeRef) == true && + canEditProtectedAspect(nodeRef, aspect) == false) + { + // the user can't edit the protected aspect + throw new ModelAccessDeniedException( + "The user " + AuthenticationUtil.getFullyAuthenticatedUser() + + " does not have the permission to add the protected aspect " + aspect.toPrefixString(namespaceService) + + " from the node " + nodeRef.toString()); + } } } @@ -300,18 +338,21 @@ public class ModelSecurityServiceImpl implements ModelSecurityService, @Override public void beforeRemoveAspect(NodeRef nodeRef, QName aspect) { - if (AuthenticationUtil.getFullyAuthenticatedUser() != null && - AuthenticationUtil.isRunAsUserTheSystemUser() == false && - isProtectedAspect(aspect) == true && - nodeService.exists(nodeRef) == true && - canEditProtectedAspect(nodeRef, aspect) == false) + if (enabled == true) { - // the user can't edit the protected aspect - throw new ModelAccessDeniedException( - "The user " + AuthenticationUtil.getFullyAuthenticatedUser() + - " does not have the permission to remove the protected aspect " + aspect.toPrefixString(namespaceService) + - " from the node " + nodeRef.toString()); - } + if (AuthenticationUtil.getFullyAuthenticatedUser() != null && + AuthenticationUtil.isRunAsUserTheSystemUser() == false && + isProtectedAspect(aspect) == true && + nodeService.exists(nodeRef) == true && + canEditProtectedAspect(nodeRef, aspect) == false) + { + // the user can't edit the protected aspect + throw new ModelAccessDeniedException( + "The user " + AuthenticationUtil.getFullyAuthenticatedUser() + + " does not have the permission to remove the protected aspect " + aspect.toPrefixString(namespaceService) + + " from the node " + nodeRef.toString()); + } + } } /** @@ -319,31 +360,33 @@ public class ModelSecurityServiceImpl implements ModelSecurityService, */ @Override public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) - { - if (AuthenticationUtil.getFullyAuthenticatedUser() != null && - AuthenticationUtil.isRunAsUserTheSystemUser() == false && - nodeService.exists(nodeRef) == true) + { + if (enabled == true) { - for (QName property : after.keySet()) + if (AuthenticationUtil.getFullyAuthenticatedUser() != null && + AuthenticationUtil.isRunAsUserTheSystemUser() == false && + nodeService.exists(nodeRef) == true) { - if (isProtectedProperty(property) == true) + for (QName property : after.keySet()) { - ProtectedProperty protectedProperty = getProtectedProperty(property); - if ((before == null || before.isEmpty() || before.get(property) == null) && - protectedProperty.isAllwaysAllowNew() == true) + if (isProtectedProperty(property) == true) { - return; + // always allow if this is the first time we are setting the protected property + if (before == null || before.isEmpty() || before.get(property) == null) + { + return; + } + + if (EqualsHelper.nullSafeEquals(before.get(property), after.get(property)) == false && + canEditProtectedProperty(nodeRef, property) == false) + { + // the user can't edit the protected property + throw new ModelAccessDeniedException( + "The user " + AuthenticationUtil.getFullyAuthenticatedUser() + + " does not have the permission to edit the protected property " + property.toPrefixString(namespaceService) + + " on the node " + nodeRef.toString()); + } } - - if (EqualsHelper.nullSafeEquals(before.get(property), after.get(property)) == false && - canEditProtectedProperty(nodeRef, property) == false) - { - // the user can't edit the protected property - throw new ModelAccessDeniedException( - "The user " + AuthenticationUtil.getFullyAuthenticatedUser() + - " does not have the permission to edit the protected property " + property.toPrefixString(namespaceService) + - " on the node " + nodeRef.toString()); - } } } } diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ProtectedModelArtifact.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ProtectedModelArtifact.java index 9961b898c0..662cbaf9ca 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ProtectedModelArtifact.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/security/ProtectedModelArtifact.java @@ -18,6 +18,7 @@ */ package org.alfresco.module.org_alfresco_module_rm.model.security; +import java.util.HashSet; import java.util.Set; import org.alfresco.module.org_alfresco_module_rm.capability.Capability; @@ -25,7 +26,7 @@ import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; /** - * + * Protected model artifact class. * * @author Roy Wetherall * @since 2.1 @@ -38,9 +39,13 @@ public abstract class ProtectedModelArtifact /** Namespace service */ private NamespaceService namespaceService; + /** Qualified name of the model artifact */ private QName name; - private Set capabilities; + /** Set of capabilities */ + private Set capabilities; + + private Set capabilityNames; public void setNamespaceService(NamespaceService namespaceService) { @@ -77,4 +82,18 @@ public abstract class ProtectedModelArtifact { return capabilities; } + + public Set getCapilityNames() + { + if (capabilityNames == null && capabilities != null) + { + capabilityNames = new HashSet(capabilities.size()); + for (Capability capability : capabilities) + { + capabilityNames.add(capability.getName()); + } + } + + return capabilityNames; + } } diff --git a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/ServicesTestSuite.java b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/ServicesTestSuite.java index f6ddad41a2..cd48d44c8a 100644 --- a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/ServicesTestSuite.java +++ b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/ServicesTestSuite.java @@ -23,7 +23,9 @@ import junit.framework.TestSuite; import org.alfresco.module.org_alfresco_module_rm.test.service.DataSetServiceImplTest; import org.alfresco.module.org_alfresco_module_rm.test.service.DispositionServiceImplTest; +import org.alfresco.module.org_alfresco_module_rm.test.service.ExtendedSecurityServiceImplTest; import org.alfresco.module.org_alfresco_module_rm.test.service.FreezeServiceImplTest; +import org.alfresco.module.org_alfresco_module_rm.test.service.ModelSecurityServiceImplTest; import org.alfresco.module.org_alfresco_module_rm.test.service.RecordServiceImplTest; import org.alfresco.module.org_alfresco_module_rm.test.service.RecordsManagementActionServiceImplTest; import org.alfresco.module.org_alfresco_module_rm.test.service.RecordsManagementAdminServiceImplTest; @@ -48,6 +50,8 @@ public class ServicesTestSuite extends TestSuite public static Test suite() { TestSuite suite = new TestSuite(); + suite.addTestSuite(ExtendedSecurityServiceImplTest.class); + suite.addTestSuite(ModelSecurityServiceImplTest.class); suite.addTestSuite(RecordsManagementServiceImplTest.class); suite.addTestSuite(DispositionServiceImplTest.class); suite.addTestSuite(RecordsManagementActionServiceImplTest.class); diff --git a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/ModelSecurityServiceImplTest.java b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/ModelSecurityServiceImplTest.java index 8214d250ee..9b0e7fd844 100644 --- a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/ModelSecurityServiceImplTest.java +++ b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/ModelSecurityServiceImplTest.java @@ -43,7 +43,9 @@ public class ModelSecurityServiceImplTest extends BaseRMTestCase /** Model security service */ - private ModelSecurityService modelSecurityService; + private ModelSecurityService modelSecurityService; + + private boolean enabled; /** * @see org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase#isUserTest() @@ -76,7 +78,20 @@ public class ModelSecurityServiceImplTest extends BaseRMTestCase @Override protected void setupTestDataImpl() { - super.setupTestDataImpl(); + super.setupTestDataImpl(); + + enabled = modelSecurityService.isEnabled(); + modelSecurityService.setEnabled(true); + } + + /** + * @see org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase#tearDownImpl() + */ + @Override + protected void tearDownImpl() + { + super.tearDownImpl(); + modelSecurityService.setEnabled(enabled); } /** @@ -194,27 +209,27 @@ public class ModelSecurityServiceImplTest extends BaseRMTestCase protectedProperty = modelSecurityService.getProtectedProperty(CUSTOM_PROTECTED_PROPERTY); assertNotNull(protectedProperty); assertNotNull(protectedProperty.getQName()); - assertNotNull(protectedProperty.getCapabilities()); - - doTestInTransaction(new VoidTest() - { - @Override - public void runImpl() throws Exception - { - assertTrue(modelSecurityService.canEditProtectedProperty(rmFolder, CUSTOM_PROTECTED_PROPERTY)); - } - }, rmAdminName); - - doTestInTransaction(new VoidTest() - { - @Override - public void runImpl() throws Exception - { - assertFalse(modelSecurityService.canEditProtectedProperty(rmFolder, CUSTOM_PROTECTED_PROPERTY)); - } - }, powerUserName); + assertNotNull(protectedProperty.getCapabilities()); } - }); + }); + + doTestInTransaction(new VoidTest() + { + @Override + public void runImpl() throws Exception + { + assertTrue(modelSecurityService.canEditProtectedProperty(rmFolder, CUSTOM_PROTECTED_PROPERTY)); + } + }, rmAdminName); + + doTestInTransaction(new VoidTest() + { + @Override + public void runImpl() throws Exception + { + assertFalse(modelSecurityService.canEditProtectedProperty(rmFolder, CUSTOM_PROTECTED_PROPERTY)); + } + }, powerUserName); doTestInTransaction(new VoidTest() {