From b37d2ed9787546da877a8f9d4bd773d54de2949f Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Tue, 16 Feb 2010 23:43:42 +0000 Subject: [PATCH] SEC-593: Added PermissionCacheOptimizer strategy interface and implementation in Acl module. This is used by DefaultMethodSecurityExpressionHandler to allow permissions to be cached before repeatedly evaluating an expression for a collection of domain objects. --- .../acls/AclPermissionCacheOptimizer.java | 62 +++++++++++++++++++ .../security/acls/AclPermissionEvaluator.java | 12 +++- .../access/PermissionCacheOptimizer.java | 23 +++++++ ...efaultMethodSecurityExpressionHandler.java | 36 +++++++++-- .../WEB-INF/applicationContext-security.xml | 6 ++ .../webapp/WEB-INF/classes/log4j.properties | 2 +- 6 files changed, 131 insertions(+), 10 deletions(-) create mode 100644 acl/src/main/java/org/springframework/security/acls/AclPermissionCacheOptimizer.java create mode 100644 core/src/main/java/org/springframework/security/access/PermissionCacheOptimizer.java diff --git a/acl/src/main/java/org/springframework/security/acls/AclPermissionCacheOptimizer.java b/acl/src/main/java/org/springframework/security/acls/AclPermissionCacheOptimizer.java new file mode 100644 index 0000000000..5e5d32d580 --- /dev/null +++ b/acl/src/main/java/org/springframework/security/acls/AclPermissionCacheOptimizer.java @@ -0,0 +1,62 @@ +package org.springframework.security.acls; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.security.access.PermissionCacheOptimizer; +import org.springframework.security.acls.domain.ObjectIdentityRetrievalStrategyImpl; +import org.springframework.security.acls.domain.SidRetrievalStrategyImpl; +import org.springframework.security.acls.model.AclService; +import org.springframework.security.acls.model.ObjectIdentity; +import org.springframework.security.acls.model.ObjectIdentityRetrievalStrategy; +import org.springframework.security.acls.model.Sid; +import org.springframework.security.acls.model.SidRetrievalStrategy; +import org.springframework.security.core.Authentication; + +/** + * Batch loads ACLs for collections of objects to allow optimised filtering. + * + * @author Luke Taylor + * @since 3.1 + */ +public class AclPermissionCacheOptimizer implements PermissionCacheOptimizer { + private final Log logger = LogFactory.getLog(getClass()); + private final AclService aclService; + private SidRetrievalStrategy sidRetrievalStrategy = new SidRetrievalStrategyImpl(); + private ObjectIdentityRetrievalStrategy oidRetrievalStrategy = new ObjectIdentityRetrievalStrategyImpl(); + + public AclPermissionCacheOptimizer(AclService aclService) { + this.aclService = aclService; + } + + public void cachePermissionsFor(Authentication authentication, Collection objects) { + List oidsToCache = new ArrayList(objects.size()); + + for (Object domainObject : objects) { + if (domainObject == null) { + continue; + } + ObjectIdentity oid = oidRetrievalStrategy.getObjectIdentity(domainObject); + oidsToCache.add(oid); + } + + List sids = sidRetrievalStrategy.getSids(authentication); + + if (logger.isDebugEnabled()) { + logger.debug("Eagerly loading Acls for " + oidsToCache.size() + " objects"); + } + + aclService.readAclsById(oidsToCache, sids); + } + + public void setObjectIdentityRetrievalStrategy(ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy) { + this.oidRetrievalStrategy = objectIdentityRetrievalStrategy; + } + + public void setSidRetrievalStrategy(SidRetrievalStrategy sidRetrievalStrategy) { + this.sidRetrievalStrategy = sidRetrievalStrategy; + } +} diff --git a/acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java b/acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java index f9f4014a63..40af57e0fe 100644 --- a/acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java +++ b/acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java @@ -70,24 +70,30 @@ public class AclPermissionEvaluator implements PermissionEvaluator { List sids = sidRetrievalStrategy.getSids(authentication); List requiredPermission = resolvePermission(permission); + final boolean debug = logger.isDebugEnabled(); + + if (debug) { + logger.debug("Checking permission '" + permission + "' for object '" + oid + "'"); + } + try { // Lookup only ACLs for SIDs we're interested in Acl acl = aclService.readAclById(oid, sids); if (acl.isGranted(requiredPermission, sids, false)) { - if (logger.isDebugEnabled()) { + if (debug) { logger.debug("Access is granted"); } return true; } - if (logger.isDebugEnabled()) { + if (debug) { logger.debug("Returning false - ACLs returned, but insufficient permissions for this principal"); } } catch (NotFoundException nfe) { - if (logger.isDebugEnabled()) { + if (debug) { logger.debug("Returning false - no ACLs apply for this principal"); } } diff --git a/core/src/main/java/org/springframework/security/access/PermissionCacheOptimizer.java b/core/src/main/java/org/springframework/security/access/PermissionCacheOptimizer.java new file mode 100644 index 0000000000..c022982970 --- /dev/null +++ b/core/src/main/java/org/springframework/security/access/PermissionCacheOptimizer.java @@ -0,0 +1,23 @@ +package org.springframework.security.access; + +import java.util.Collection; + +import org.springframework.aop.framework.AopInfrastructureBean; +import org.springframework.security.core.Authentication; + +/** + * Allows permissions to be pre-cached when using pre or post filtering with expressions + * + * @author Luke Taylor + * @since 3.1 + */ +public interface PermissionCacheOptimizer extends AopInfrastructureBean { + /** + * Optimises the permission cache for anticipated operation on the supplied collection of objects. + * Usually this will entail batch loading of permissions for the objects in the collection. + * + * @param a the user for whom permissions should be obtained. + * @param objects the (non-null) collection of domain objects for which permissions should be retrieved. + */ + void cachePermissionsFor(Authentication a, Collection objects); +} diff --git a/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java b/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java index b12b0d6ab8..99cb2099d1 100644 --- a/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java +++ b/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java @@ -2,6 +2,7 @@ package org.springframework.security.access.expression.method; import java.lang.reflect.Array; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -14,6 +15,7 @@ import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.security.access.PermissionCacheOptimizer; import org.springframework.security.access.PermissionEvaluator; import org.springframework.security.access.expression.ExpressionUtils; import org.springframework.security.access.hierarchicalroles.RoleHierarchy; @@ -35,6 +37,7 @@ public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExp private ParameterNameDiscoverer parameterNameDiscoverer = new LocalVariableTableParameterNameDiscoverer(); private PermissionEvaluator permissionEvaluator = new DenyAllPermissionEvaluator(); + private PermissionCacheOptimizer permissionCacheOptimizer = null; private AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl(); private ExpressionParser expressionParser = new SpelExpressionParser(); private RoleHierarchy roleHierarchy; @@ -57,12 +60,20 @@ public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExp return ctx; } + /** + * Filters the {@code filterTarget} object (which must be either a collection or an array), by evaluating the + * supplied expression. + *

+ * If a {@Collection} is used, the original instance will be modified to contain the elements for which + * the permission expression evaluates to {@code true}. For an array, a new array instance will be returned. + */ @SuppressWarnings("unchecked") public Object filter(Object filterTarget, Expression filterExpression, EvaluationContext ctx) { MethodSecurityExpressionRoot rootObject = (MethodSecurityExpressionRoot) ctx.getRootObject().getValue(); + final boolean debug = logger.isDebugEnabled(); List retainList; - if (logger.isDebugEnabled()) { + if (debug) { logger.debug("Filtering with expression: " + filterExpression.getExpressionString()); } @@ -70,9 +81,14 @@ public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExp Collection collection = (Collection)filterTarget; retainList = new ArrayList(collection.size()); - if (logger.isDebugEnabled()) { + if (debug) { logger.debug("Filtering collection with " + collection.size() + " elements"); } + + if (permissionCacheOptimizer != null) { + permissionCacheOptimizer.cachePermissionsFor(rootObject.getAuthentication(), collection); + } + for (Object filterObject : (Collection)filterTarget) { rootObject.setFilterObject(filterObject); @@ -81,7 +97,7 @@ public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExp } } - if (logger.isDebugEnabled()) { + if (debug) { logger.debug("Retaining elements: " + retainList); } @@ -95,8 +111,12 @@ public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExp Object[] array = (Object[])filterTarget; retainList = new ArrayList(array.length); - if (logger.isDebugEnabled()) { - logger.debug("Filtering collection with " + array.length + " elements"); + if (debug) { + logger.debug("Filtering array with " + array.length + " elements"); + } + + if (permissionCacheOptimizer != null) { + permissionCacheOptimizer.cachePermissionsFor(rootObject.getAuthentication(), Arrays.asList(array)); } for (int i = 0; i < array.length; i++) { @@ -107,7 +127,7 @@ public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExp } } - if (logger.isDebugEnabled()) { + if (debug) { logger.debug("Retaining elements: " + retainList); } @@ -135,6 +155,10 @@ public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExp this.permissionEvaluator = permissionEvaluator; } + public void setPermissionCacheOptimizer(PermissionCacheOptimizer permissionCacheOptimizer) { + this.permissionCacheOptimizer = permissionCacheOptimizer; + } + public void setTrustResolver(AuthenticationTrustResolver trustResolver) { this.trustResolver = trustResolver; } diff --git a/samples/contacts/src/main/webapp/WEB-INF/applicationContext-security.xml b/samples/contacts/src/main/webapp/WEB-INF/applicationContext-security.xml index 014afc84ce..957670168a 100644 --- a/samples/contacts/src/main/webapp/WEB-INF/applicationContext-security.xml +++ b/samples/contacts/src/main/webapp/WEB-INF/applicationContext-security.xml @@ -52,9 +52,15 @@ + + + + + + diff --git a/samples/contacts/src/main/webapp/WEB-INF/classes/log4j.properties b/samples/contacts/src/main/webapp/WEB-INF/classes/log4j.properties index e7e926d3e7..22cdec739e 100644 --- a/samples/contacts/src/main/webapp/WEB-INF/classes/log4j.properties +++ b/samples/contacts/src/main/webapp/WEB-INF/classes/log4j.properties @@ -1,5 +1,5 @@ # Global logging configuration -log4j.rootLogger=DEBUG, stdout, fileout +log4j.rootLogger=INFO, stdout, fileout log4j.logger.sample.contact=DEBUG log4j.logger.org.springframework.web.*=DEBUG