From f1347f3bf61aa0b8cf50f0679541b10f9b0567d9 Mon Sep 17 00:00:00 2001 From: Andrew Hind Date: Mon, 4 Jun 2007 16:06:54 +0000 Subject: [PATCH] Build fix for permission model tests Fix for "allowAll" + deny something for the same auth on the same node -> now denies git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@5843 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../impl/PermissionServiceImpl.java | 38 +++++++++++------- .../impl/model/PermissionModel.java | 23 ++++++----- .../impl/model/PermissionModelTest.java | 39 ++++++++++++++++++- 3 files changed, 74 insertions(+), 26 deletions(-) diff --git a/source/java/org/alfresco/repo/security/permissions/impl/PermissionServiceImpl.java b/source/java/org/alfresco/repo/security/permissions/impl/PermissionServiceImpl.java index aece0dfdf7..0df47c9ea3 100644 --- a/source/java/org/alfresco/repo/security/permissions/impl/PermissionServiceImpl.java +++ b/source/java/org/alfresco/repo/security/permissions/impl/PermissionServiceImpl.java @@ -44,6 +44,7 @@ import org.alfresco.repo.security.permissions.NodePermissionEntry; import org.alfresco.repo.security.permissions.PermissionEntry; import org.alfresco.repo.security.permissions.PermissionReference; import org.alfresco.repo.security.permissions.PermissionServiceSPI; +import org.alfresco.repo.security.permissions.impl.model.PermissionModel; import org.alfresco.service.cmr.dictionary.DictionaryService; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; @@ -61,7 +62,8 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.InitializingBean; /** - * The Alfresco implementation of a permissions service against our APIs for the permissions model and permissions persistence. + * The Alfresco implementation of a permissions service against our APIs for the permissions model and permissions + * persistence. * * @author andyh */ @@ -459,8 +461,8 @@ public class PermissionServiceImpl implements PermissionServiceSPI, Initializing } /** - * Key for a cache object is built from all the known Authorities (which can change dynamically so they must all be used) the NodeRef ID and the permission reference itself. - * This gives a unique key for each permission test. + * Key for a cache object is built from all the known Authorities (which can change dynamically so they must all be + * used) the NodeRef ID and the permission reference itself. This gives a unique key for each permission test. */ static Serializable generateKey(Set auths, NodeRef nodeRef, PermissionReference perm, CacheType type) { @@ -694,8 +696,16 @@ public class PermissionServiceImpl implements PermissionServiceSPI, Initializing this.aspectQNames = aspectQNames; // Set the required node permissions - nodeRequirements = modelDAO.getRequiredPermissions(required, typeQName, aspectQNames, - RequiredPermission.On.NODE); + if (required.equals(getPermissionReference(ALL_PERMISSIONS))) + { + nodeRequirements = modelDAO.getRequiredPermissions(getPermissionReference(PermissionService.FULL_CONTROL), typeQName, aspectQNames, + RequiredPermission.On.NODE); + } + else + { + nodeRequirements = modelDAO.getRequiredPermissions(required, typeQName, aspectQNames, + RequiredPermission.On.NODE); + } parentRequirements = modelDAO.getRequiredPermissions(required, typeQName, aspectQNames, RequiredPermission.On.PARENT); @@ -705,7 +715,8 @@ public class PermissionServiceImpl implements PermissionServiceSPI, Initializing // Find all the permissions that grant the allowed permission // All permissions are treated specially. - granters = modelDAO.getGrantingPermissions(required); + granters = new LinkedHashSet(128, 1.0f); + granters.addAll(modelDAO.getGrantingPermissions(required)); granters.add(getAllPermissionReference()); granters.add(OLD_ALL_PERMISSIONS_REFERENCE); } @@ -747,7 +758,7 @@ public class PermissionServiceImpl implements PermissionServiceSPI, Initializing // Check the required permissions but not for sets they rely on // their underlying permissions - if (required.equals(getPermissionReference(ALL_PERMISSIONS)) || modelDAO.checkPermission(required)) + if (modelDAO.checkPermission(required)) { if (parentRequirements.contains(required)) { @@ -861,17 +872,14 @@ public class PermissionServiceImpl implements PermissionServiceSPI, Initializing public boolean hasSinglePermission(Set authorisations, NodeRef nodeRef) { - Serializable key = generateKey( - authorisations, - nodeRef, - this.required, CacheType.SINGLE_PERMISSION_GLOBAL); - + Serializable key = generateKey(authorisations, nodeRef, this.required, CacheType.SINGLE_PERMISSION_GLOBAL); + AccessStatus status = accessCache.get(key); if (status != null) { return status == AccessStatus.ALLOWED; } - + // Check global permission if (checkGlobalPermissions(authorisations)) @@ -883,7 +891,7 @@ public class PermissionServiceImpl implements PermissionServiceSPI, Initializing Set> denied = new HashSet>(); return hasSinglePermission(authorisations, nodeRef, denied); - + } public boolean hasSinglePermission(Set authorisations, NodeRef nodeRef, @@ -935,7 +943,7 @@ public class PermissionServiceImpl implements PermissionServiceSPI, Initializing NodePermissionEntry nodePermissions = permissionsDaoComponent.getPermissions(nodeRef); if ((nodePermissions == null) || (nodePermissions.inheritPermissions())) { - if(hasSinglePermission(authorisations, car.getParentRef(), denied)) + if (hasSinglePermission(authorisations, car.getParentRef(), denied)) { if (key != null) { diff --git a/source/java/org/alfresco/repo/security/permissions/impl/model/PermissionModel.java b/source/java/org/alfresco/repo/security/permissions/impl/model/PermissionModel.java index ecdc30d0bf..867647bc09 100644 --- a/source/java/org/alfresco/repo/security/permissions/impl/model/PermissionModel.java +++ b/source/java/org/alfresco/repo/security/permissions/impl/model/PermissionModel.java @@ -122,10 +122,10 @@ public class PermissionModel implements ModelDAO, InitializingBean private HashMap permissionReferenceMap; - private Map> cachedTypePermissionsExposed = new HashMap>( + private Map> cachedTypePermissionsExposed = new HashMap>( 128, 1.0f); - private Map> cachedTypePermissionsUnexposed = new HashMap>( + private Map> cachedTypePermissionsUnexposed = new HashMap>( 128, 1.0f); public PermissionModel() @@ -288,10 +288,10 @@ public class PermissionModel implements ModelDAO, InitializingBean return getAllPermissionsImpl(type, true); } - @SuppressWarnings("unchecked") + private Set getAllPermissionsImpl(QName type, boolean exposedOnly) { - Map> cache; + Map> cache; if (exposedOnly) { cache = this.cachedTypePermissionsExposed; @@ -300,7 +300,7 @@ public class PermissionModel implements ModelDAO, InitializingBean { cache = this.cachedTypePermissionsUnexposed; } - LinkedHashSet permissions = cache.get(type); + Set permissions = cache.get(type); if (permissions == null) { permissions = new LinkedHashSet(128, 1.0f); @@ -317,9 +317,10 @@ public class PermissionModel implements ModelDAO, InitializingBean addTypePermissions(type, permissions, exposedOnly); } } + permissions = Collections.unmodifiableSet(permissions); cache.put(type, permissions); } - return (Set) permissions.clone(); + return permissions; } /** @@ -446,7 +447,8 @@ public class PermissionModel implements ModelDAO, InitializingBean // QName typeName = nodeService.getType(nodeRef); - Set permissions = getAllPermissionsImpl(typeName, exposedOnly); + Set permissions = new LinkedHashSet(128, 1.0f); + permissions.addAll(getAllPermissionsImpl(typeName, exposedOnly)); mergeGeneralAspectPermissions(permissions, exposedOnly); // Add non mandatory aspects... Set defaultAspects = new HashSet(); @@ -471,6 +473,7 @@ public class PermissionModel implements ModelDAO, InitializingBean if (granters == null) { granters = getGrantingPermissionsImpl(permissionReference); + granters = Collections.unmodifiableSet(granters); grantingPermissions.put(permissionReference, granters); } return granters; @@ -543,6 +546,7 @@ public class PermissionModel implements ModelDAO, InitializingBean if (grantees == null) { grantees = getGranteePermissionsImpl(permissionReference); + grantees = Collections.unmodifiableSet(grantees); granteePermissions.put(permissionReference, grantees); } return grantees; @@ -763,6 +767,7 @@ public class PermissionModel implements ModelDAO, InitializingBean { answer = getRequirementsForPermissionGroup(pg, on, qName, aspectQNames); } + answer = Collections.unmodifiableSet(answer); requiredPermissionsCache.put(key, answer); } return answer; @@ -814,7 +819,7 @@ public class PermissionModel implements ModelDAO, InitializingBean for (PermissionGroup pg : ps.getPermissionGroups()) { PermissionGroup base = getBasePermissionGroupOrNull(pg); - if (target.equals(base) + if ((target.equals(base) || target.isAllowFullControl()) && (!base.isTypeRequired() || isPartOfDynamicPermissionGroup(pg, qName, aspectQNames))) { // Add includes @@ -830,7 +835,7 @@ public class PermissionModel implements ModelDAO, InitializingBean for (PermissionReference grantedTo : p.getGrantedToGroups()) { PermissionGroup base = getBasePermissionGroupOrNull(getPermissionGroupOrNull(grantedTo)); - if (target.equals(base) + if ((target.equals(base) || target.isAllowFullControl()) && (!base.isTypeRequired() || isPartOfDynamicPermissionGroup(grantedTo, qName, aspectQNames))) { if (on == RequiredPermission.On.NODE) diff --git a/source/java/org/alfresco/repo/security/permissions/impl/model/PermissionModelTest.java b/source/java/org/alfresco/repo/security/permissions/impl/model/PermissionModelTest.java index d69a6b13a5..2d9fb7d24b 100644 --- a/source/java/org/alfresco/repo/security/permissions/impl/model/PermissionModelTest.java +++ b/source/java/org/alfresco/repo/security/permissions/impl/model/PermissionModelTest.java @@ -24,6 +24,7 @@ */ package org.alfresco.repo.security.permissions.impl.model; +import java.util.Collections; import java.util.Set; import org.alfresco.repo.security.permissions.PermissionEntry; @@ -104,12 +105,12 @@ public class PermissionModelTest extends AbstractPermissionTest Set granters = permissionModelDAO.getGrantingPermissions(new SimplePermissionReference(QName.createQName("sys", "base", namespacePrefixResolver), "ReadProperties")); // NB This has gone from 10 to 14 because of the new WCM roles, I believe. - assertEquals(16, granters.size()); + assertEquals(14, granters.size()); granters = permissionModelDAO.getGrantingPermissions(new SimplePermissionReference(QName.createQName("sys", "base", namespacePrefixResolver), "_ReadProperties")); // NB 11 to 15 as above. - assertEquals(17, granters.size()); + assertEquals(15, granters.size()); } public void testGlobalPermissions() @@ -118,4 +119,38 @@ public class PermissionModelTest extends AbstractPermissionTest assertEquals(5, globalPermissions.size()); } + public void testRequiredPermissions() + { + Set required = permissionModelDAO.getRequiredPermissions(new SimplePermissionReference(QName.createQName("sys", "base", + namespacePrefixResolver), "Read"), QName.createQName("sys", "base", + namespacePrefixResolver), Collections.emptySet(), On.NODE); + assertEquals(3, required.size()); + + required = permissionModelDAO.getRequiredPermissions(new SimplePermissionReference(QName.createQName("sys", "base", + namespacePrefixResolver), "ReadContent"), QName.createQName("sys", "base", + namespacePrefixResolver), Collections.emptySet(), On.NODE); + assertEquals(1, required.size()); + + required = permissionModelDAO.getRequiredPermissions(new SimplePermissionReference(QName.createQName("sys", "base", + namespacePrefixResolver), "_ReadContent"), QName.createQName("sys", "base", + namespacePrefixResolver), Collections.emptySet(), On.NODE); + assertEquals(0, required.size()); + + + + required = permissionModelDAO.getRequiredPermissions(new SimplePermissionReference(QName.createQName("cm", "cmobject", + namespacePrefixResolver), "Coordinator"), QName.createQName("cm", "cmobject", + namespacePrefixResolver), Collections.emptySet(), On.NODE); + assertEquals(17, required.size()); + + + required = permissionModelDAO.getRequiredPermissions(new SimplePermissionReference(QName.createQName("sys", "base", + namespacePrefixResolver), "FullControl"), QName.createQName("sys", "base", + namespacePrefixResolver), Collections.emptySet(), On.NODE); + assertEquals(17, required.size()); + + + + + } }