From ce19b470e2647e9de5948759a2a5649a420eb5a4 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Tue, 17 May 2011 22:55:20 +0100 Subject: [PATCH] SEC-1560: Change AccessControlListTag to use PermissionEvaluator rather than explicit ACL classes. --- .../taglibs/authz/AccessControlListTag.java | 114 +++--------------- .../authz/AccessControlListTagTests.java | 51 +++----- 2 files changed, 40 insertions(+), 125 deletions(-) diff --git a/taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java b/taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java index f19491aca4..adeebe8ca1 100644 --- a/taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java +++ b/taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java @@ -14,57 +14,33 @@ */ package org.springframework.security.taglibs.authz; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.StringTokenizer; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.context.ApplicationContext; +import org.springframework.security.access.PermissionEvaluator; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.taglibs.TagLibConfig; +import org.springframework.web.context.support.WebApplicationContextUtils; +import org.springframework.web.util.ExpressionEvaluationUtils; import javax.servlet.ServletContext; import javax.servlet.jsp.JspException; import javax.servlet.jsp.PageContext; import javax.servlet.jsp.tagext.Tag; import javax.servlet.jsp.tagext.TagSupport; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.springframework.context.ApplicationContext; -import org.springframework.security.acls.domain.DefaultPermissionFactory; -import org.springframework.security.acls.domain.ObjectIdentityRetrievalStrategyImpl; -import org.springframework.security.acls.domain.PermissionFactory; -import org.springframework.security.acls.domain.SidRetrievalStrategyImpl; -import org.springframework.security.acls.model.Acl; -import org.springframework.security.acls.model.AclService; -import org.springframework.security.acls.model.NotFoundException; -import org.springframework.security.acls.model.ObjectIdentity; -import org.springframework.security.acls.model.ObjectIdentityRetrievalStrategy; -import org.springframework.security.acls.model.Permission; -import org.springframework.security.acls.model.Sid; -import org.springframework.security.acls.model.SidRetrievalStrategy; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.taglibs.TagLibConfig; -import org.springframework.web.context.support.WebApplicationContextUtils; -import org.springframework.web.util.ExpressionEvaluationUtils; +import java.util.*; /** * An implementation of {@link Tag} that allows its body through if some authorizations are granted to the request's * principal. *

- * One or more comma separate numeric are specified via the hasPermission attribute. - * These permissions are then converted into {@link Permission} instances using the {@link PermissionFactory} - * instance obtained from the application context, or a {@link DefaultPermissionFactory} if one isn't found. - * The Permission instances are then presented as a list - * array to the {@link Acl#isGranted(List, List, boolean)} method. - * The {@link Sid} presented is determined by the {@link SidRetrievalStrategy}. + * One or more comma separate numeric are specified via the {@code hasPermission} attribute. The tag delegates + * to the configured {@link PermissionEvaluator} which it obtains from the {@code ApplicationContext}. *

* For this class to operate it must be able to access the application context via the - * WebApplicationContextUtils and attempt to locate an {@link AclService}, - * {@link ObjectIdentityRetrievalStrategy} and {@link SidRetrievalStrategy}. - * There cannot be more than one of these present. The AclService must be provided, but a - * {@link SidRetrievalStrategyImpl} and/or an {@link ObjectIdentityRetrievalStrategyImpl} will be created if no - * implementations are found in the application context. + * {@code WebApplicationContextUtils} and attempt to locate the {@code PermissionEvaluator} instance. + * There cannot be more than one of these present for the tag to function. * * @author Ben Alex * @author Luke Taylor @@ -76,12 +52,9 @@ public class AccessControlListTag extends TagSupport { //~ Instance fields ================================================================================================ - private AclService aclService; private ApplicationContext applicationContext; private Object domainObject; - private ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy; - private SidRetrievalStrategy sidRetrievalStrategy; - private PermissionFactory permissionFactory; + private PermissionEvaluator permissionEvaluator; private String hasPermission = ""; private String var; @@ -97,8 +70,6 @@ public class AccessControlListTag extends TagSupport { final String evaledPermissionsString = ExpressionEvaluationUtils.evaluateString("hasPermission", hasPermission, pageContext); - List requiredPermissions = parsePermissionsString(evaledPermissionsString); - Object resolvedDomainObject; if (domainObject instanceof String) { @@ -126,21 +97,12 @@ public class AccessControlListTag extends TagSupport { return skipBody(); } - List sids = sidRetrievalStrategy.getSids(SecurityContextHolder.getContext().getAuthentication()); - ObjectIdentity oid = objectIdentityRetrievalStrategy.getObjectIdentity(resolvedDomainObject); - - // Obtain aclEntrys applying to the current Authentication object - try { - Acl acl = aclService.readAclById(oid, sids); - - if (acl.isGranted(requiredPermissions, sids, false)) { - return evalBody(); - } else { - return skipBody(); - } - } catch (NotFoundException nfe) { - return skipBody(); + if (permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), + resolvedDomainObject, evaledPermissionsString)) { + return evalBody(); } + + return skipBody(); } private int skipBody() { @@ -187,25 +149,7 @@ public class AccessControlListTag extends TagSupport { this.applicationContext = getContext(pageContext); - aclService = getBeanOfType(AclService.class); - - sidRetrievalStrategy = getBeanOfType(SidRetrievalStrategy.class); - - if (sidRetrievalStrategy == null) { - sidRetrievalStrategy = new SidRetrievalStrategyImpl(); - } - - objectIdentityRetrievalStrategy = getBeanOfType(ObjectIdentityRetrievalStrategy.class); - - if (objectIdentityRetrievalStrategy == null) { - objectIdentityRetrievalStrategy = new ObjectIdentityRetrievalStrategyImpl(); - } - - permissionFactory = getBeanOfType(PermissionFactory.class); - - if (permissionFactory == null) { - permissionFactory = new DefaultPermissionFactory(); - } + permissionEvaluator = getBeanOfType(PermissionEvaluator.class); } private T getBeanOfType(Class type) throws JspException { @@ -226,24 +170,6 @@ public class AccessControlListTag extends TagSupport { + "application context - you must have only have one!"); } - private List parsePermissionsString(String permissionsString) throws NumberFormatException { - final Set permissions = new HashSet(); - final StringTokenizer tokenizer; - tokenizer = new StringTokenizer(permissionsString, ",", false); - - while (tokenizer.hasMoreTokens()) { - String permission = tokenizer.nextToken(); - try { - permissions.add(permissionFactory.buildFromMask(Integer.valueOf(permission))); - } catch (NumberFormatException nfe) { - // Not an integer mask. Try using a name - permissions.add(permissionFactory.buildFromName(permission)); - } - } - - return new ArrayList(permissions); - } - public void setDomainObject(Object domainObject) { this.domainObject = domainObject; } diff --git a/taglibs/src/test/java/org/springframework/security/taglibs/authz/AccessControlListTagTests.java b/taglibs/src/test/java/org/springframework/security/taglibs/authz/AccessControlListTagTests.java index ac53cdd208..9c209cdf48 100644 --- a/taglibs/src/test/java/org/springframework/security/taglibs/authz/AccessControlListTagTests.java +++ b/taglibs/src/test/java/org/springframework/security/taglibs/authz/AccessControlListTagTests.java @@ -1,29 +1,22 @@ package org.springframework.security.taglibs.authz; import static org.junit.Assert.*; -import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; -import java.util.HashMap; -import java.util.Map; - -import javax.servlet.jsp.tagext.Tag; - -import org.junit.After; -import org.junit.Before; -import org.junit.Test; +import org.junit.*; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockPageContext; import org.springframework.mock.web.MockServletContext; -import org.springframework.security.acls.model.Acl; -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.access.PermissionEvaluator; import org.springframework.security.authentication.TestingAuthenticationToken; +import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.web.context.WebApplicationContext; +import javax.servlet.jsp.tagext.Tag; +import java.util.*; + /** * * @author Luke Taylor @@ -32,28 +25,21 @@ import org.springframework.web.context.WebApplicationContext; @SuppressWarnings("unchecked") public class AccessControlListTagTests { AccessControlListTag tag; - Acl acl; + PermissionEvaluator pe; MockPageContext pageContext; + Authentication bob = new TestingAuthenticationToken("bob","bobspass","A"); @Before public void setup() { - SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("bob","bobspass","A")); + SecurityContextHolder.getContext().setAuthentication(bob); tag = new AccessControlListTag(); WebApplicationContext ctx = mock(WebApplicationContext.class); - AclService service = mock(AclService.class); - ObjectIdentity oid = mock(ObjectIdentity.class); - ObjectIdentityRetrievalStrategy oidStrategy = mock(ObjectIdentityRetrievalStrategy.class); - when(oidStrategy.getObjectIdentity(anyObject())).thenReturn(oid); - acl = mock(Acl.class); + pe = mock(PermissionEvaluator.class); - when(service.readAclById(any(ObjectIdentity.class), anyList())).thenReturn(acl); Map beanMap = new HashMap(); - beanMap.put("service", service); - when(ctx.getBeansOfType(AclService.class)).thenReturn(beanMap); - beanMap = new HashMap(); - beanMap.put("oidStrategy", oidStrategy); - when(ctx.getBeansOfType(ObjectIdentityRetrievalStrategy.class)).thenReturn(beanMap); + beanMap.put("pe", pe); + when(ctx.getBeansOfType(PermissionEvaluator.class)).thenReturn(beanMap); MockServletContext servletCtx = new MockServletContext(); servletCtx.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, ctx); @@ -68,11 +54,14 @@ public class AccessControlListTagTests { @Test public void bodyIsEvaluatedIfAclGrantsAccess() throws Exception { - when(acl.isGranted(anyList(), anyList(), eq(false))).thenReturn(true); + Object domainObject = new Object(); + when(pe.hasPermission(bob, domainObject, "READ")).thenReturn(true); - tag.setDomainObject(new Object()); + tag.setDomainObject(domainObject); tag.setHasPermission("READ"); tag.setVar("allowed"); + assertSame(domainObject, tag.getDomainObject()); + assertEquals("READ", tag.getHasPermission()); assertEquals(Tag.EVAL_BODY_INCLUDE, tag.doStartTag()); assertTrue((Boolean)pageContext.getAttribute("allowed")); @@ -80,14 +69,14 @@ public class AccessControlListTagTests { @Test public void bodyIsSkippedIfAclDeniesAccess() throws Exception { - when(acl.isGranted(anyList(), anyList(), eq(false))).thenReturn(false); + Object domainObject = new Object(); + when(pe.hasPermission(bob, domainObject, "READ")).thenReturn(false); - tag.setDomainObject(new Object()); + tag.setDomainObject(domainObject); tag.setHasPermission("READ"); tag.setVar("allowed"); assertEquals(Tag.SKIP_BODY, tag.doStartTag()); assertFalse((Boolean)pageContext.getAttribute("allowed")); } - }