From c5c141952181a09edf2a55816aff2272d958628e Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Wed, 18 Sep 2013 18:27:12 -0500 Subject: [PATCH] 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. --- .../GlobalMethodSecurityConfiguration.java | 9 +++++++-- .../method/configuration/MethodSecurityService.groovy | 7 +++++++ .../configuration/MethodSecurityServiceImpl.groovy | 10 ++++++++++ .../NamespaceGlobalMethodSecurityTests.groovy | 7 +++++++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java index f7507b84f6..a161676630 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java +++ b/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.AfterInvocationProvider; 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.expression.method.DefaultMethodSecurityExpressionHandler; import org.springframework.security.access.expression.method.ExpressionBasedAnnotationAttributeFactory; @@ -178,9 +179,13 @@ public class GlobalMethodSecurityConfiguration implements ImportAware { List decisionVoters = new ArrayList(); ExpressionBasedPreInvocationAdvice expressionAdvice = new ExpressionBasedPreInvocationAdvice(); expressionAdvice.setExpressionHandler(getExpressionHandler()); - - decisionVoters.add(new PreInvocationAuthorizationAdviceVoter( + if(prePostEnabled()) { + decisionVoters.add(new PreInvocationAuthorizationAdviceVoter( expressionAdvice)); + } + if(jsr250Enabled()) { + decisionVoters.add(new Jsr250Voter()); + } decisionVoters.add(new RoleVoter()); decisionVoters.add(new AuthenticatedVoter()); return new AffirmativeBased(decisionVoters); diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/MethodSecurityService.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/MethodSecurityService.groovy index 7bc1ce0795..743c466e4e 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/MethodSecurityService.groovy +++ b/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; import javax.annotation.security.DenyAll +import javax.annotation.security.PermitAll; import org.springframework.security.access.annotation.Secured import org.springframework.security.access.prepost.PostAuthorize; @@ -34,9 +35,15 @@ public interface MethodSecurityService { @Secured("ROLE_ADMIN") public String secured(); + @Secured("ROLE_USER") + public String securedUser(); + @DenyAll public String jsr250(); + @PermitAll + public String jsr250PermitAll(); + @Secured(["ROLE_USER","RUN_AS_SUPER"]) public Authentication runAs(); diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.groovy index c0af11e624..013f3ce70e 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/MethodSecurityServiceImpl.groovy @@ -35,11 +35,21 @@ public class MethodSecurityServiceImpl implements MethodSecurityService { return null; } + @Override + public String securedUser() { + return null; + } + @Override public String jsr250() { return null; } + @Override + public String jsr250PermitAll() { + return null; + } + @Override public Authentication runAs() { return SecurityContextHolder.getContext().getAuthentication(); diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy index 4e035d2246..534ad6075e 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy @@ -134,6 +134,10 @@ public class NamespaceGlobalMethodSecurityTests extends BaseSpringSpec { service.jsr250() then: "access is denied" thrown(AccessDeniedException) + when: "@PermitAll method invoked" + String jsr250PermitAll = service.jsr250PermitAll() + then: "access is allowed" + jsr250PermitAll == null } @EnableGlobalMethodSecurity(jsr250Enabled = true) @@ -345,6 +349,9 @@ public class NamespaceGlobalMethodSecurityTests extends BaseSpringSpec { service.secured() then: thrown(AccessDeniedException) + and: "service with ROLE_USER allowed" + service.securedUser() == null + and: service.preAuthorize() == null service.jsr250() == null }