SEC-922: Spring Security should respect Spring XML boolean operators for AJ pointcut

http://jira.springframework.org/browse/SEC-922. Added method to substitute boolean operators "and, not, or" with aspectj versions "&&, !, ||".

core-tiger/src/test/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParserTests.java

@@ -1,6 +1,6 @@
 package org.springframework.security.config;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 import static org.springframework.security.config.ConfigTestUtils.*;
 
 import org.junit.After;
@@ -42,8 +42,10 @@ public class GlobalMethodSecurityBeanDefinitionParserTests {
     public void closeAppContext() {
         if (appContext != null) {
             appContext.close();
+            appContext = null;
         }
         SecurityContextHolder.clearContext();
+        target = null;
     }
 
     @Test(expected=AuthenticationCredentialsNotFoundException.class)
@@ -55,8 +57,7 @@ public class GlobalMethodSecurityBeanDefinitionParserTests {
     @Test
     public void targetShouldAllowProtectedMethodInvocationWithCorrectRole() {
         loadContext();
-        UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",
+        UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("user", "password");
-                new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_USER")});
         SecurityContextHolder.getContext().setAuthentication(token);
 
         target.someUserMethod1();
@@ -111,13 +112,46 @@ public class GlobalMethodSecurityBeanDefinitionParserTests {
         setContext(
                 "<b:bean id='target' class='org.springframework.security.annotation.BusinessServiceImpl'/>" +
                 "<global-method-security>" +
-                "   <protect-pointcut expression='execution(* *.someOther(String))' access='ROLE_ADMIN'/>" +
+                "   <protect-pointcut expression='execution(* org.springframework.security.annotation.BusinessService.someOther(String))' access='ROLE_ADMIN'/>" +
-                "   <protect-pointcut expression='execution(* *.BusinessService*(..))' access='ROLE_USER'/>" +
+                "   <protect-pointcut expression='execution(* org.springframework.security.annotation.BusinessService.*(..))' access='ROLE_USER'/>" +
                 "</global-method-security>" + ConfigTestUtils.AUTH_PROVIDER_XML
         );
         SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken("user", "password"));
         target = (BusinessService) appContext.getBean("target");
-        // someOther(int) should not be matched by someOther(String)
+        // someOther(int) should not be matched by someOther(String), but should require ROLE_USER
+        target.someOther(0);
+
+        try {
+            // String version should required admin role
+            target.someOther("somestring");
+            fail("Expected AccessDeniedException");
+        } catch (AccessDeniedException expected) {
+        }
+    }
+
+    @Test
+    public void supportsBooleanPointcutExpressions() {
+        setContext(
+                "<b:bean id='target' class='org.springframework.security.annotation.BusinessServiceImpl'/>" +
+                "<global-method-security>" +
+                "   <protect-pointcut expression=" +
+                "     'execution(* org.springframework.security.annotation.BusinessService.*(..)) " +
+                "       and not execution(* org.springframework.security.annotation.BusinessService.someOther(String)))' " +
+                "               access='ROLE_USER'/>" +
+                "</global-method-security>" + ConfigTestUtils.AUTH_PROVIDER_XML
+        );
+        target = (BusinessService) appContext.getBean("target");
+        // String method should not be protected
+        target.someOther("somestring");
+
+        // All others should require ROLE_USER
+        try {
+            target.someOther(0);
+            fail("Expected AuthenticationCredentialsNotFoundException");
+        } catch (AuthenticationCredentialsNotFoundException expected) {
+        }
+
+        SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken("user", "password"));
         target.someOther(0);
     }
 

core/src/main/java/org/springframework/security/intercept/method/ProtectPointcutPostProcessor.java

@@ -17,6 +17,7 @@ import org.springframework.beans.factory.config.BeanPostProcessor;
 import org.springframework.security.ConfigAttributeDefinition;
 import org.springframework.security.intercept.method.aopalliance.MethodDefinitionSourceAdvisor;
 import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
 
 /**
  * Parses AspectJ pointcut expressions, registering methods that match the pointcut with a
@@ -141,9 +142,10 @@ public final class ProtectPointcutPostProcessor implements BeanPostProcessor {
         }
     }
 
-    public void addPointcut(String pointcutExpression, ConfigAttributeDefinition definition) {
+    private void addPointcut(String pointcutExpression, ConfigAttributeDefinition definition) {
         Assert.hasText(pointcutExpression, "An AspectJ pointcut expression is required");
         Assert.notNull(definition, "ConfigAttributeDefinition required");
+        pointcutExpression = replaceBooleanOperators(pointcutExpression);
         pointcutMap.put(pointcutExpression, definition);
 
         if (logger.isDebugEnabled()) {
@@ -151,4 +153,14 @@ public final class ProtectPointcutPostProcessor implements BeanPostProcessor {
         }
     }
 
+    /**
+     * @see org.springframework.aop.aspectj.AspectJExpressionPointcut#replaceBooleanOperators
+     */
+    private String replaceBooleanOperators(String pcExpr) {
+        pcExpr = StringUtils.replace(pcExpr," and "," && ");
+        pcExpr = StringUtils.replace(pcExpr, " or ", " || ");
+        pcExpr = StringUtils.replace(pcExpr, " not ", " ! ");
+        return pcExpr;
+    }
+
 }