Browse Source

SEC-2332: GlobalMethodSecurityConfiguration includes proper voters

Previously GlobalMethodSecurityConfiguration did not include the correct
voters. This updates the code and the tests to ensure that the proper
voters are added. Note this got past testing previously due to all the
voters abstaining, so tests were added for ensuring that methods could also
be invoked sucessfully using the configured annotation.
Rob Winch 12 years ago
parent
commit
c5c1419521

+ 7 - 2
config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java

@@ -40,6 +40,7 @@ import org.springframework.security.access.AccessDecisionManager;
 import org.springframework.security.access.AccessDecisionVoter;
 import org.springframework.security.access.AccessDecisionVoter;
 import org.springframework.security.access.AfterInvocationProvider;
 import org.springframework.security.access.AfterInvocationProvider;
 import org.springframework.security.access.annotation.Jsr250MethodSecurityMetadataSource;
 import org.springframework.security.access.annotation.Jsr250MethodSecurityMetadataSource;
+import org.springframework.security.access.annotation.Jsr250Voter;
 import org.springframework.security.access.annotation.SecuredAnnotationSecurityMetadataSource;
 import org.springframework.security.access.annotation.SecuredAnnotationSecurityMetadataSource;
 import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
 import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
 import org.springframework.security.access.expression.method.ExpressionBasedAnnotationAttributeFactory;
 import org.springframework.security.access.expression.method.ExpressionBasedAnnotationAttributeFactory;
@@ -178,9 +179,13 @@ public class GlobalMethodSecurityConfiguration implements ImportAware {
         List<AccessDecisionVoter> decisionVoters = new ArrayList<AccessDecisionVoter>();
         List<AccessDecisionVoter> decisionVoters = new ArrayList<AccessDecisionVoter>();
         ExpressionBasedPreInvocationAdvice expressionAdvice = new ExpressionBasedPreInvocationAdvice();
         ExpressionBasedPreInvocationAdvice expressionAdvice = new ExpressionBasedPreInvocationAdvice();
         expressionAdvice.setExpressionHandler(getExpressionHandler());
         expressionAdvice.setExpressionHandler(getExpressionHandler());
-
-        decisionVoters.add(new PreInvocationAuthorizationAdviceVoter(
+        if(prePostEnabled()) {
+            decisionVoters.add(new PreInvocationAuthorizationAdviceVoter(
                 expressionAdvice));
                 expressionAdvice));
+        }
+        if(jsr250Enabled()) {
+            decisionVoters.add(new Jsr250Voter());
+        }
         decisionVoters.add(new RoleVoter());
         decisionVoters.add(new RoleVoter());
         decisionVoters.add(new AuthenticatedVoter());
         decisionVoters.add(new AuthenticatedVoter());
         return new AffirmativeBased(decisionVoters);
         return new AffirmativeBased(decisionVoters);

+ 7 - 0
config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/MethodSecurityService.groovy

@@ -16,6 +16,7 @@
 package org.springframework.security.config.annotation.method.configuration;
 package org.springframework.security.config.annotation.method.configuration;
 
 
 import javax.annotation.security.DenyAll
 import javax.annotation.security.DenyAll
+import javax.annotation.security.PermitAll;
 
 
 import org.springframework.security.access.annotation.Secured
 import org.springframework.security.access.annotation.Secured
 import org.springframework.security.access.prepost.PostAuthorize;
 import org.springframework.security.access.prepost.PostAuthorize;
@@ -34,9 +35,15 @@ public interface MethodSecurityService {
     @Secured("ROLE_ADMIN")
     @Secured("ROLE_ADMIN")
     public String secured();
     public String secured();
 
 
+    @Secured("ROLE_USER")
+    public String securedUser();
+
     @DenyAll
     @DenyAll
     public String jsr250();
     public String jsr250();
 
 
+    @PermitAll
+    public String jsr250PermitAll();
+
     @Secured(["ROLE_USER","RUN_AS_SUPER"])
     @Secured(["ROLE_USER","RUN_AS_SUPER"])
     public Authentication runAs();
     public Authentication runAs();
 
 

+ 10 - 0
config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.groovy

@@ -35,11 +35,21 @@ public class MethodSecurityServiceImpl implements MethodSecurityService {
         return null;
         return null;
     }
     }
 
 
+    @Override
+    public String securedUser() {
+        return null;
+    }
+
     @Override
     @Override
     public String jsr250() {
     public String jsr250() {
         return null;
         return null;
     }
     }
 
 
+    @Override
+    public String jsr250PermitAll() {
+        return null;
+    }
+
     @Override
     @Override
     public Authentication runAs() {
     public Authentication runAs() {
         return SecurityContextHolder.getContext().getAuthentication();
         return SecurityContextHolder.getContext().getAuthentication();

+ 7 - 0
config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy

@@ -134,6 +134,10 @@ public class NamespaceGlobalMethodSecurityTests extends BaseSpringSpec {
             service.jsr250()
             service.jsr250()
         then: "access is denied"
         then: "access is denied"
             thrown(AccessDeniedException)
             thrown(AccessDeniedException)
+        when: "@PermitAll method invoked"
+            String jsr250PermitAll = service.jsr250PermitAll()
+        then: "access is allowed"
+            jsr250PermitAll == null
     }
     }
 
 
     @EnableGlobalMethodSecurity(jsr250Enabled = true)
     @EnableGlobalMethodSecurity(jsr250Enabled = true)
@@ -345,6 +349,9 @@ public class NamespaceGlobalMethodSecurityTests extends BaseSpringSpec {
             service.secured()
             service.secured()
         then:
         then:
             thrown(AccessDeniedException)
             thrown(AccessDeniedException)
+        and: "service with ROLE_USER allowed"
+            service.securedUser() == null
+        and:
             service.preAuthorize() == null
             service.preAuthorize() == null
             service.jsr250() == null
             service.jsr250() == null
     }
     }