SEC-2022: AccessControlListTag again supports , separated list of per…

…missions Spring Security 3.0.x allowed developers to pass in a , separated list of permissions. However, this functionality was accidentally removed in SEC-1560. The AcessControlListTag now splits the permissions using , as a delimiter which fixes this passivity issue.
spring-projects · Aug 2, 2012 · b481a6c · b481a6c
1 parent e659315
commit b481a6c
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 5 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
@@ -18,6 +18,7 @@
 import org.apache.commons.logging.LogFactory;
 import org.springframework.context.ApplicationContext;
 import org.springframework.security.access.PermissionEvaluator;
+import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.taglibs.TagLibConfig;
 import org.springframework.web.context.support.WebApplicationContextUtils;
@@ -43,6 +44,7 @@
  *
  * @author Ben Alex
  * @author Luke Taylor
+ * @author Rob Winch
  */
 public class AccessControlListTag extends TagSupport {
     //~ Static fields/initializers =====================================================================================
@@ -75,7 +77,8 @@ public int doStartTag() throws JspException {
             return evalBody();
         }
 
-        if (SecurityContextHolder.getContext().getAuthentication() == null) {
+        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+        if (authentication == null) {
             if (logger.isDebugEnabled()) {
                 logger.debug(
                     "SecurityContextHolder did not return a non-null Authentication object, so skipping tag body");
@@ -84,12 +87,14 @@ public int doStartTag() throws JspException {
             return skipBody();
         }
 
-        if (permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(),
+        String[] requiredPermissions = hasPermission.split(",");
-                domainObject, hasPermission)) {
+        for(String requiredPermission : requiredPermissions) {
-            return evalBody();
+            if (!permissionEvaluator.hasPermission(authentication, domainObject, requiredPermission)) {
+                return skipBody();
+            }
         }
 
-        return skipBody();
+        return evalBody();
     }
 
     private int skipBody() {

diff --git a/...s/src/test/java/org/springframework/security/taglibs/authz/AccessControlListTagTests.java b/...s/src/test/java/org/springframework/security/taglibs/authz/AccessControlListTagTests.java
@@ -1,3 +1,15 @@
+/*
+ * Copyright 2002-2012 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
 package org.springframework.security.taglibs.authz;
 
 import static org.junit.Assert.*;
@@ -20,6 +32,7 @@
 /**
  *
  * @author Luke Taylor
+ * @author Rob Winch
  * @since 3.0
  */
 @SuppressWarnings("unchecked")
@@ -67,6 +80,26 @@ public void bodyIsEvaluatedIfAclGrantsAccess() throws Exception {
         assertTrue((Boolean)pageContext.getAttribute("allowed"));
     }
 
+    // SEC-2022
+    @Test
+    public void multiHasPermissionsAreSplit() throws Exception {
+        Object domainObject = new Object();
+        when(pe.hasPermission(bob, domainObject, "READ")).thenReturn(true);
+        when(pe.hasPermission(bob, domainObject, "WRITE")).thenReturn(true);
+
+        tag.setDomainObject(domainObject);
+        tag.setHasPermission("READ,WRITE");
+        tag.setVar("allowed");
+        assertSame(domainObject, tag.getDomainObject());
+        assertEquals("READ,WRITE", tag.getHasPermission());
+
+        assertEquals(Tag.EVAL_BODY_INCLUDE, tag.doStartTag());
+        assertTrue((Boolean)pageContext.getAttribute("allowed"));
+        verify(pe).hasPermission(bob, domainObject, "READ");
+        verify(pe).hasPermission(bob, domainObject, "WRITE");
+        verifyNoMoreInteractions(pe);
+    }
+
     @Test
     public void bodyIsSkippedIfAclDeniesAccess() throws Exception {
         Object domainObject = new Object();