Explorar o código

SEC-2758: Make ROLE_ consistent

Rob Winch %!s(int64=10) %!d(string=hai) anos
pai
achega
6627f76df7
Modificáronse 17 ficheiros con 479 adicións e 47 borrados
  1. 18 4
      config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java
  2. 16 4
      config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecuritySelector.java
  3. 14 0
      config/src/main/java/org/springframework/security/config/annotation/method/configuration/Jsr250MetadataSourceConfiguration.java
  4. 143 0
      config/src/test/groovy/org/springframework/security/config/annotation/sec2758/Sec2758Tests.groovy
  5. 10 0
      config/src/test/java/org/springframework/security/config/method/Jsr250AnnotationDrivenBeanDefinitionParserTests.java
  6. 33 1
      core/src/main/java/org/springframework/security/access/annotation/Jsr250MethodSecurityMetadataSource.java
  7. 48 4
      core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java
  8. 19 0
      core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java
  9. 3 0
      core/src/test/java/org/springframework/security/access/annotation/BusinessService.java
  10. 3 0
      core/src/test/java/org/springframework/security/access/annotation/BusinessServiceImpl.java
  11. 4 0
      core/src/test/java/org/springframework/security/access/annotation/ExpressionProtectedBusinessServiceImpl.java
  12. 4 0
      core/src/test/java/org/springframework/security/access/annotation/Jsr250BusinessServiceImpl.java
  13. 55 10
      core/src/test/java/org/springframework/security/access/annotation/Jsr250MethodSecurityMetadataSourceTests.java
  14. 74 8
      core/src/test/java/org/springframework/security/access/expression/SecurityExpressionRootTests.java
  15. 1 1
      core/src/test/java/org/springframework/security/access/expression/method/MethodExpressionVoterTests.java
  16. 14 14
      docs/manual/src/docs/asciidoc/index.adoc
  17. 20 1
      web/src/main/java/org/springframework/security/web/access/expression/DefaultWebSecurityExpressionHandler.java

+ 18 - 4
config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java

@@ -88,6 +88,7 @@ public class GlobalMethodSecurityConfiguration implements ImportAware {
     private AnnotationAttributes enableMethodSecurity;
     private ApplicationContext context;
     private MethodSecurityExpressionHandler expressionHandler;
+    private Jsr250MethodSecurityMetadataSource jsr250MethodSecurityMetadataSource;
 
     /**
      * Creates the default MethodInterceptor which is a MethodSecurityInterceptor using the following methods to
@@ -172,7 +173,6 @@ public class GlobalMethodSecurityConfiguration implements ImportAware {
      *
      * @return
      */
-    @SuppressWarnings("rawtypes")
     protected AccessDecisionManager accessDecisionManager() {
         List<AccessDecisionVoter<? extends Object>> decisionVoters = new ArrayList<AccessDecisionVoter<? extends Object>>();
         ExpressionBasedPreInvocationAdvice expressionAdvice = new ExpressionBasedPreInvocationAdvice();
@@ -282,14 +282,13 @@ public class GlobalMethodSecurityConfiguration implements ImportAware {
             sources.add(customMethodSecurityMetadataSource);
         }
         if (prePostEnabled()) {
-            sources.add(new PrePostAnnotationSecurityMetadataSource(
-                    attributeFactory));
+            sources.add(new PrePostAnnotationSecurityMetadataSource(attributeFactory));
         }
         if (securedEnabled()) {
             sources.add(new SecuredAnnotationSecurityMetadataSource());
         }
         if (jsr250Enabled()) {
-            sources.add(new Jsr250MethodSecurityMetadataSource());
+            sources.add(jsr250MethodSecurityMetadataSource);
         }
         return new DelegatingMethodSecurityMetadataSource(sources);
     }
@@ -344,6 +343,12 @@ public class GlobalMethodSecurityConfiguration implements ImportAware {
         this.defaultMethodExpressionHandler = objectPostProcessor.postProcess(defaultMethodExpressionHandler);
     }
 
+    @Autowired(required = false)
+    public void setJsr250MethodSecurityMetadataSource(
+            Jsr250MethodSecurityMetadataSource jsr250MethodSecurityMetadataSource) {
+        this.jsr250MethodSecurityMetadataSource = jsr250MethodSecurityMetadataSource;
+    }
+
     @Autowired(required = false)
     public void setPermissionEvaluator(List<PermissionEvaluator> permissionEvaluators) {
         if(permissionEvaluators.size() != 1) {
@@ -352,6 +357,15 @@ public class GlobalMethodSecurityConfiguration implements ImportAware {
         this.defaultMethodExpressionHandler.setPermissionEvaluator(permissionEvaluators.get(0));
     }
 
+    @Autowired(required = false)
+    public void setMethodSecurityExpressionHandler(List<MethodSecurityExpressionHandler> handlers) {
+        if(handlers.size() != 1) {
+            logger.debug("Not autwiring PermissionEvaluator since size != 1. Got " + handlers);
+            return;
+        }
+        this.expressionHandler = handlers.get(0);
+    }
+
     @Autowired
     public void setApplicationContext(ApplicationContext context) {
         this.context = context;

+ 16 - 4
config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecuritySelector.java

@@ -15,6 +15,8 @@
  */
 package org.springframework.security.config.annotation.method.configuration;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 
 import org.springframework.context.annotation.AdviceMode;
@@ -49,10 +51,20 @@ final class GlobalMethodSecuritySelector implements ImportSelector {
         AdviceMode mode = attributes.getEnum("mode");
         String autoProxyClassName = AdviceMode.PROXY == mode ? AutoProxyRegistrar.class.getName()
                 : GlobalMethodSecurityAspectJAutoProxyRegistrar.class.getName();
-        if(skipMethodSecurityConfiguration) {
-            return new String[] { autoProxyClassName };
+
+        boolean jsr250Enabled = attributes.getBoolean("jsr250Enabled");
+
+        List<String> classNames = new ArrayList<String>(4);
+        classNames.add(autoProxyClassName);
+
+        if(!skipMethodSecurityConfiguration) {
+            classNames.add(GlobalMethodSecurityConfiguration.class.getName());
         }
-        return new String[] { autoProxyClassName,
-                GlobalMethodSecurityConfiguration.class.getName()};
+
+        if(jsr250Enabled) {
+            classNames.add(Jsr250MetadataSourceConfiguration.class.getName());
+        }
+
+        return classNames.toArray(new String[0]);
     }
 }

+ 14 - 0
config/src/main/java/org/springframework/security/config/annotation/method/configuration/Jsr250MetadataSourceConfiguration.java

@@ -0,0 +1,14 @@
+package org.springframework.security.config.annotation.method.configuration;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.security.access.annotation.Jsr250MethodSecurityMetadataSource;
+
+@Configuration
+class Jsr250MetadataSourceConfiguration {
+
+    @Bean
+    public Jsr250MethodSecurityMetadataSource jsr250MethodSecurityMetadataSource() {
+        return new Jsr250MethodSecurityMetadataSource();
+    }
+}

+ 143 - 0
config/src/test/groovy/org/springframework/security/config/annotation/sec2758/Sec2758Tests.groovy

@@ -0,0 +1,143 @@
+/*
+ * Copyright 2002-2013 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.config.annotation.sec2758;
+
+import javax.annotation.security.RolesAllowed;
+
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.config.BeanPostProcessor;
+import org.springframework.context.annotation.Bean;
+import org.springframework.core.PriorityOrdered;
+import org.springframework.mock.web.MockFilterChain;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.access.annotation.Jsr250MethodSecurityMetadataSource;
+import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
+import org.springframework.security.access.prepost.PreAuthorize;
+import org.springframework.security.authentication.TestingAuthenticationToken
+import org.springframework.security.config.annotation.BaseSpringSpec
+import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
+import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
+import org.springframework.security.config.annotation.web.builders.HttpSecurity;
+import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
+import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
+import org.springframework.security.config.annotation.web.configuration.sec2377.a.*
+import org.springframework.security.config.annotation.web.configuration.sec2377.b.*
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler;
+import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
+import org.springframework.web.context.support.AnnotationConfigWebApplicationContext
+
+public class Sec2758Tests extends BaseSpringSpec {
+
+    def cleanup() {
+        SecurityContextHolder.clearContext()
+    }
+
+    def "SEC-2758: Verify Passivity Restored with Advice from JIRA"() {
+        setup:
+        SecurityContextHolder.context.authentication = new TestingAuthenticationToken("user", "pass", "USER")
+        loadConfig(SecurityConfig)
+        Service service = context.getBean(Service)
+
+        when:
+        findFilter(FilterSecurityInterceptor).doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), new MockFilterChain())
+        then:
+        noExceptionThrown()
+
+        when:
+        service.doPreAuthorize()
+        then:
+        noExceptionThrown()
+
+        when:
+        service.doJsr250()
+        then:
+        noExceptionThrown()
+    }
+
+    @EnableWebSecurity
+    @EnableGlobalMethodSecurity(prePostEnabled=true)
+    static class SecurityConfig extends WebSecurityConfigurerAdapter {
+
+        @Override
+        protected void configure(HttpSecurity http) throws Exception {
+            http
+                .authorizeRequests()
+                    .anyRequest().hasAnyAuthority("USER");
+        }
+
+        @Autowired
+        public void configureGlobal(AuthenticationManagerBuilder auth) {
+            auth
+                .inMemoryAuthentication()
+                    .withUser("user").password("password").authorities("USER")
+        }
+
+        @Bean
+        Service service() {
+            return new ServiceImpl()
+        }
+
+        @Bean
+        static DefaultRolesPrefixPostProcessor defaultRolesPrefixPostProcessor() {
+            new DefaultRolesPrefixPostProcessor()
+        }
+    }
+
+    interface Service {
+        void doPreAuthorize()
+        void doJsr250()
+    }
+
+    static class ServiceImpl implements Service {
+        @PreAuthorize("hasRole('USER')")
+        void doPreAuthorize() {}
+
+        @RolesAllowed("USER")
+        void doJsr250() {}
+    }
+
+    static class DefaultRolesPrefixPostProcessor implements BeanPostProcessor, PriorityOrdered {
+
+    @Override
+    public Object postProcessAfterInitialization(Object bean, String beanName)
+            throws BeansException {
+        if(bean instanceof Jsr250MethodSecurityMetadataSource) {
+            ((Jsr250MethodSecurityMetadataSource) bean).setDefaultRolePrefix(null);
+        }
+        if(bean instanceof DefaultMethodSecurityExpressionHandler) {
+            ((DefaultMethodSecurityExpressionHandler) bean).setDefaultRolePrefix(null);
+        }
+        if(bean instanceof DefaultWebSecurityExpressionHandler) {
+            ((DefaultWebSecurityExpressionHandler) bean).setDefaultRolePrefix(null);
+        }
+        return bean;
+    }
+
+    @Override
+    public Object postProcessBeforeInitialization(Object bean, String beanName)
+            throws BeansException {
+        return bean;
+    }
+
+    @Override
+    public int getOrder() {
+        return PriorityOrdered.HIGHEST_PRECEDENCE;
+    }
+}
+}

+ 10 - 0
config/src/test/java/org/springframework/security/config/method/Jsr250AnnotationDrivenBeanDefinitionParserTests.java

@@ -68,4 +68,14 @@ public class Jsr250AnnotationDrivenBeanDefinitionParserTests {
 
         target.someAdminMethod();
     }
+
+
+    @Test
+    public void hasAnyRoleAddsDefaultPrefix() {
+        UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",
+                AuthorityUtils.createAuthorityList("ROLE_USER"));
+        SecurityContextHolder.getContext().setAuthentication(token);
+
+        target.rolesAllowedUser();
+    }
 }

+ 33 - 1
core/src/main/java/org/springframework/security/access/annotation/Jsr250MethodSecurityMetadataSource.java

@@ -38,6 +38,24 @@ import org.springframework.security.access.method.AbstractFallbackMethodSecurity
  */
 public class Jsr250MethodSecurityMetadataSource extends AbstractFallbackMethodSecurityMetadataSource {
 
+    private String defaultRolePrefix = "ROLE_";
+
+    /**
+     * <p>
+     * Sets the default prefix to be added to {@link RolesAllowed}. For example, if {@code @RolesAllowed("ADMIN")} or {@code @RolesAllowed("ADMIN")} is used,
+     * then the role ROLE_ADMIN will be used when the defaultRolePrefix is "ROLE_" (default).
+     * </p>
+     *
+     * <p>
+     * If null or empty, then no default role prefix is used.
+     * </p>
+     *
+     * @param defaultRolePrefix the default prefix to add to roles. Default "ROLE_".
+     */
+    public void setDefaultRolePrefix(String defaultRolePrefix) {
+        this.defaultRolePrefix = defaultRolePrefix;
+    }
+
     protected Collection<ConfigAttribute> findAttributes(Class<?> clazz) {
         return processAnnotations(clazz.getAnnotations());
     }
@@ -69,11 +87,25 @@ public class Jsr250MethodSecurityMetadataSource extends AbstractFallbackMethodSe
                 RolesAllowed ra = (RolesAllowed) a;
 
                 for (String allowed : ra.value()) {
-                    attributes.add(new Jsr250SecurityConfig(allowed));
+                    String defaultedAllowed = getRoleWithDefaultPrefix(allowed);
+                    attributes.add(new Jsr250SecurityConfig(defaultedAllowed));
                 }
                 return attributes;
             }
         }
         return null;
     }
+
+    private String getRoleWithDefaultPrefix(String role) {
+        if(role == null) {
+            return role;
+        }
+        if(defaultRolePrefix == null || defaultRolePrefix.length() == 0) {
+            return role;
+        }
+        if(role.startsWith(defaultRolePrefix)) {
+            return role;
+        }
+        return defaultRolePrefix + role;
+    }
 }

+ 48 - 4
core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java

@@ -24,6 +24,7 @@ public abstract class SecurityExpressionRoot implements SecurityExpressionOperat
     private AuthenticationTrustResolver trustResolver;
     private RoleHierarchy roleHierarchy;
     private Set<String> roles;
+    private String defaultRolePrefix = "ROLE_";
 
     /** Allows "permitAll" expression */
     public final boolean permitAll = true;
@@ -49,22 +50,27 @@ public abstract class SecurityExpressionRoot implements SecurityExpressionOperat
     }
 
     public final boolean hasAuthority(String authority) {
-        return hasRole(authority);
+        return hasAnyAuthority(authority);
     }
 
     public final boolean hasAnyAuthority(String... authorities) {
-        return hasAnyRole(authorities);
+        return hasAnyAuthorityName(null, authorities);
     }
 
     public final boolean hasRole(String role) {
-        return getAuthoritySet().contains(role);
+        return hasAnyRole(role);
     }
 
     public final boolean hasAnyRole(String... roles) {
+        return hasAnyAuthorityName(defaultRolePrefix, roles);
+    }
+
+    private boolean hasAnyAuthorityName(String prefix, String... roles) {
         Set<String> roleSet = getAuthoritySet();
 
         for (String role : roles) {
-            if (roleSet.contains(role)) {
+            String defaultedRole = getRoleWithDefaultPrefix(prefix, role);
+            if (roleSet.contains(defaultedRole)) {
                 return true;
             }
         }
@@ -116,6 +122,23 @@ public abstract class SecurityExpressionRoot implements SecurityExpressionOperat
         this.roleHierarchy = roleHierarchy;
     }
 
+    /**
+     * <p>
+     * Sets the default prefix to be added to {@link #hasAnyRole(String...)} or
+     * {@link #hasRole(String)}. For example, if hasRole("ADMIN") or hasRole("ROLE_ADMIN") is passed in,
+     * then the role ROLE_ADMIN will be used when the defaultRolePrefix is "ROLE_" (default).
+     * </p>
+     *
+     * <p>
+     * If null or empty, then no default role prefix is used.
+     * </p>
+     *
+     * @param defaultRolePrefix the default prefix to add to roles. Default "ROLE_".
+     */
+    public void setDefaultRolePrefix(String defaultRolePrefix) {
+        this.defaultRolePrefix = defaultRolePrefix;
+    }
+
     private Set<String> getAuthoritySet() {
         if (roles == null) {
             roles = new HashSet<String>();
@@ -143,4 +166,25 @@ public abstract class SecurityExpressionRoot implements SecurityExpressionOperat
     public void setPermissionEvaluator(PermissionEvaluator permissionEvaluator) {
         this.permissionEvaluator = permissionEvaluator;
     }
+
+    /**
+     * Prefixes role with defaultRolePrefix if defaultRolePrefix is non-null and
+     * if role does not already start with defaultRolePrefix.
+     *
+     * @param defaultRolePrefix
+     * @param role
+     * @return
+     */
+    private static String getRoleWithDefaultPrefix(String defaultRolePrefix, String role) {
+        if(role == null) {
+            return role;
+        }
+        if(defaultRolePrefix == null || defaultRolePrefix.length() == 0) {
+            return role;
+        }
+        if(role.startsWith(defaultRolePrefix)) {
+            return role;
+        }
+        return defaultRolePrefix + role;
+    }
 }

+ 19 - 0
core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java

@@ -37,6 +37,7 @@ public class DefaultMethodSecurityExpressionHandler extends AbstractSecurityExpr
     private AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
     private ParameterNameDiscoverer parameterNameDiscoverer = new DefaultSecurityParameterNameDiscoverer();
     private PermissionCacheOptimizer permissionCacheOptimizer = null;
+    private String defaultRolePrefix = "ROLE_";
 
     public DefaultMethodSecurityExpressionHandler() {
     }
@@ -57,6 +58,7 @@ public class DefaultMethodSecurityExpressionHandler extends AbstractSecurityExpr
         root.setPermissionEvaluator(getPermissionEvaluator());
         root.setTrustResolver(trustResolver);
         root.setRoleHierarchy(getRoleHierarchy());
+        root.setDefaultRolePrefix(defaultRolePrefix);
 
         return root;
     }
@@ -172,4 +174,21 @@ public class DefaultMethodSecurityExpressionHandler extends AbstractSecurityExpr
     public void setReturnObject(Object returnObject, EvaluationContext ctx) {
         ((MethodSecurityExpressionOperations)ctx.getRootObject().getValue()).setReturnObject(returnObject);
     }
+
+    /**
+     * <p>
+     * Sets the default prefix to be added to {@link #hasAnyRole(String...)} or
+     * {@link #hasRole(String)}. For example, if hasRole("ADMIN") or hasRole("ROLE_ADMIN") is passed in,
+     * then the role ROLE_ADMIN will be used when the defaultRolePrefix is "ROLE_" (default).
+     * </p>
+     *
+     * <p>
+     * If null or empty, then no default role prefix is used.
+     * </p>
+     *
+     * @param defaultRolePrefix the default prefix to add to roles. Default "ROLE_".
+     */
+    public void setDefaultRolePrefix(String defaultRolePrefix) {
+        this.defaultRolePrefix = defaultRolePrefix;
+    }
 }

+ 3 - 0
core/src/test/java/org/springframework/security/access/annotation/BusinessService.java

@@ -47,6 +47,9 @@ public interface BusinessService extends Serializable {
     @RolesAllowed({"ROLE_USER"})
     public void someUserMethod2();
 
+    @RolesAllowed({"USER"})
+    public void rolesAllowedUser();
+
     public int someOther(String s);
 
     public int someOther(int input);

+ 3 - 0
core/src/test/java/org/springframework/security/access/annotation/BusinessServiceImpl.java

@@ -49,4 +49,7 @@ public class BusinessServiceImpl<E extends Entity> implements BusinessService {
         return null;
     }
 
+    public void rolesAllowedUser() {
+
+    }
 }

+ 4 - 0
core/src/test/java/org/springframework/security/access/annotation/ExpressionProtectedBusinessServiceImpl.java

@@ -49,4 +49,8 @@ public class ExpressionProtectedBusinessServiceImpl implements BusinessService {
     public void methodWithBeanNamePropertyAccessExpression(String x) {
 
     }
+
+    public void rolesAllowedUser() {
+
+    }
 }

+ 4 - 0
core/src/test/java/org/springframework/security/access/annotation/Jsr250BusinessServiceImpl.java

@@ -50,4 +50,8 @@ public class Jsr250BusinessServiceImpl implements BusinessService {
         return null;
     }
 
+    @RolesAllowed({"USER"})
+    public void rolesAllowedUser() {
+
+    }
 }

+ 55 - 10
core/src/test/java/org/springframework/security/access/annotation/Jsr250MethodDefinitionSourceTests.java → core/src/test/java/org/springframework/security/access/annotation/Jsr250MethodSecurityMetadataSourceTests.java

@@ -24,6 +24,7 @@ import javax.annotation.security.PermitAll;
 import javax.annotation.security.RolesAllowed;
 
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.access.intercept.method.MockMethodInvocation;
@@ -32,10 +33,17 @@ import org.springframework.security.access.intercept.method.MockMethodInvocation
  * @author Luke Taylor
  * @author Ben Alex
  */
-public class Jsr250MethodDefinitionSourceTests {
-    Jsr250MethodSecurityMetadataSource mds = new Jsr250MethodSecurityMetadataSource();
-    A a = new A();
-    UserAllowedClass userAllowed = new UserAllowedClass();
+public class Jsr250MethodSecurityMetadataSourceTests {
+    Jsr250MethodSecurityMetadataSource mds;
+    A a;
+    UserAllowedClass userAllowed;
+
+    @Before
+    public void setup() {
+        mds = new Jsr250MethodSecurityMetadataSource();
+        a = new A();
+        userAllowed = new UserAllowedClass();
+    }
 
     private ConfigAttribute[] findAttributes(String methodName) throws Exception {
         return mds.findAttributes(a.getClass().getMethod(methodName), null).toArray(new ConfigAttribute[0]);
@@ -45,7 +53,7 @@ public class Jsr250MethodDefinitionSourceTests {
     public void methodWithRolesAllowedHasCorrectAttribute() throws Exception {
         ConfigAttribute[] accessAttributes = findAttributes("adminMethod");
         assertEquals(1, accessAttributes.length);
-        assertEquals("ADMIN", accessAttributes[0].toString());
+        assertEquals("ROLE_ADMIN", accessAttributes[0].toString());
     }
 
     @Test
@@ -71,7 +79,41 @@ public class Jsr250MethodDefinitionSourceTests {
     public void methodRoleOverridesClassRole() throws Exception {
         Collection<ConfigAttribute> accessAttributes = mds.findAttributes(userAllowed.getClass().getMethod("adminMethod"), null);
         assertEquals(1, accessAttributes.size());
-        assertEquals("ADMIN", accessAttributes.toArray()[0].toString());
+        assertEquals("ROLE_ADMIN", accessAttributes.toArray()[0].toString());
+    }
+
+    @Test
+    public void customDefaultRolePrefix() throws Exception {
+        mds.setDefaultRolePrefix("CUSTOMPREFIX_");
+
+        ConfigAttribute[] accessAttributes = findAttributes("adminMethod");
+        assertEquals(1, accessAttributes.length);
+        assertEquals("CUSTOMPREFIX_ADMIN", accessAttributes[0].toString());
+    }
+
+    @Test
+    public void emptyDefaultRolePrefix() throws Exception {
+        mds.setDefaultRolePrefix("");
+
+        ConfigAttribute[] accessAttributes = findAttributes("adminMethod");
+        assertEquals(1, accessAttributes.length);
+        assertEquals("ADMIN", accessAttributes[0].toString());
+    }
+
+    @Test
+    public void nullDefaultRolePrefix() throws Exception {
+        mds.setDefaultRolePrefix(null);
+
+        ConfigAttribute[] accessAttributes = findAttributes("adminMethod");
+        assertEquals(1, accessAttributes.length);
+        assertEquals("ADMIN", accessAttributes[0].toString());
+    }
+
+    @Test
+    public void alreadyHasDefaultPrefix() throws Exception {
+        ConfigAttribute[] accessAttributes = findAttributes("roleAdminMethod");
+        assertEquals(1, accessAttributes.length);
+        assertEquals("ROLE_ADMIN", accessAttributes[0].toString());
     }
 
     // JSR-250 Spec Tests
@@ -98,7 +140,7 @@ public class Jsr250MethodDefinitionSourceTests {
 
         Collection<ConfigAttribute> accessAttributes = mds.getAttributes(mi);
         assertEquals(1, accessAttributes.size());
-        assertEquals("DERIVED", accessAttributes.toArray()[0].toString());
+        assertEquals("ROLE_DERIVED", accessAttributes.toArray()[0].toString());
     }
 
     @Test
@@ -108,7 +150,7 @@ public class Jsr250MethodDefinitionSourceTests {
 
         Collection<ConfigAttribute> accessAttributes = mds.getAttributes(mi);
         assertEquals(1, accessAttributes.size());
-        assertEquals("DERIVED", accessAttributes.toArray()[0].toString());
+        assertEquals("ROLE_DERIVED", accessAttributes.toArray()[0].toString());
     }
 
     @Test
@@ -118,7 +160,7 @@ public class Jsr250MethodDefinitionSourceTests {
 
         Collection<ConfigAttribute> accessAttributes = mds.getAttributes(mi);
         assertEquals(1, accessAttributes.size());
-        assertEquals("EXPLICIT", accessAttributes.toArray()[0].toString());
+        assertEquals("ROLE_EXPLICIT", accessAttributes.toArray()[0].toString());
     }
 
     /**
@@ -151,7 +193,7 @@ public class Jsr250MethodDefinitionSourceTests {
 
         Collection<ConfigAttribute> accessAttributes = mds.getAttributes(mi);
         assertEquals(1, accessAttributes.size());
-        assertEquals("DERIVED", accessAttributes.toArray()[0].toString());
+        assertEquals("ROLE_DERIVED", accessAttributes.toArray()[0].toString());
     }
 
     //~ Inner Classes ======================================================================================================
@@ -163,6 +205,9 @@ public class Jsr250MethodDefinitionSourceTests {
         @RolesAllowed("ADMIN")
         public void adminMethod() {}
 
+        @RolesAllowed("ROLE_ADMIN")
+        public void roleAdminMethod() {}
+
         @PermitAll
         public void permitAllMethod() {}
     }

+ 74 - 8
core/src/test/java/org/springframework/security/access/expression/SecurityExpressionRootTests.java

@@ -3,9 +3,11 @@ package org.springframework.security.access.expression;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.fest.assertions.Assertions.*;
 
 import java.util.Collection;
 
+import org.junit.Before;
 import org.junit.Test;
 import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
 import org.springframework.security.authentication.AuthenticationTrustResolver;
@@ -20,11 +22,17 @@ import org.springframework.security.core.authority.AuthorityUtils;
  * @since 3.0
  */
 public class SecurityExpressionRootTests {
-    final static Authentication JOE = new TestingAuthenticationToken("joe", "pass", "A", "B");
+    final static Authentication JOE = new TestingAuthenticationToken("joe", "pass", "ROLE_A", "ROLE_B");
+
+    SecurityExpressionRoot root;
+
+    @Before
+    public void setup() {
+        root = new SecurityExpressionRoot(JOE) {};
+    }
 
     @Test
     public void denyAllIsFalsePermitAllTrue() throws Exception {
-        SecurityExpressionRoot root = new SecurityExpressionRoot(JOE) {};
         assertFalse(root.denyAll());
         assertFalse(root.denyAll);
         assertTrue(root.permitAll());
@@ -33,7 +41,6 @@ public class SecurityExpressionRootTests {
 
     @Test
     public void rememberMeIsCorrectlyDetected() throws Exception {
-        SecurityExpressionRoot root = new SecurityExpressionRoot(JOE) {};
         AuthenticationTrustResolver atr = mock(AuthenticationTrustResolver.class);
         root.setTrustResolver(atr);
         when(atr.isRememberMe(JOE)).thenReturn(true);
@@ -43,20 +50,79 @@ public class SecurityExpressionRootTests {
 
     @Test
     public void roleHierarchySupportIsCorrectlyUsedInEvaluatingRoles() throws Exception {
-        SecurityExpressionRoot root = new SecurityExpressionRoot(JOE) {};
-
         root.setRoleHierarchy(new RoleHierarchy() {
             public Collection<GrantedAuthority> getReachableGrantedAuthorities(Collection<? extends GrantedAuthority> authorities) {
-                return AuthorityUtils.createAuthorityList("C");
+                return AuthorityUtils.createAuthorityList("ROLE_C");
             }
         });
 
         assertTrue(root.hasRole("C"));
-        assertTrue(root.hasAuthority("C"));
+        assertTrue(root.hasAuthority("ROLE_C"));
         assertFalse(root.hasRole("A"));
         assertFalse(root.hasRole("B"));
         assertTrue(root.hasAnyRole("C", "A", "B"));
-        assertTrue(root.hasAnyAuthority("C", "A", "B"));
+        assertTrue(root.hasAnyAuthority("ROLE_C", "ROLE_A", "ROLE_B"));
         assertFalse(root.hasAnyRole("A", "B"));
     }
+
+    @Test
+    public void hasRoleAddsDefaultPrefix() throws Exception {
+        assertThat(root.hasRole("A")).isTrue();
+        assertThat(root.hasRole("NO")).isFalse();
+    }
+
+    @Test
+    public void hasRoleEmptyPrefixDoesNotAddsDefaultPrefix() throws Exception {
+        root.setDefaultRolePrefix("");
+        assertThat(root.hasRole("A")).isFalse();
+        assertThat(root.hasRole("ROLE_A")).isTrue();
+    }
+
+    @Test
+    public void hasRoleNullPrefixDoesNotAddsDefaultPrefix() throws Exception {
+        root.setDefaultRolePrefix(null);
+        assertThat(root.hasRole("A")).isFalse();
+        assertThat(root.hasRole("ROLE_A")).isTrue();
+    }
+
+    @Test
+    public void hasRoleDoesNotAddDefaultPrefixForAlreadyPrefixedRoles() throws Exception {
+        SecurityExpressionRoot root = new SecurityExpressionRoot(JOE) {};
+
+        assertThat(root.hasRole("ROLE_A")).isTrue();
+        assertThat(root.hasRole("ROLE_NO")).isFalse();
+    }
+
+    @Test
+    public void hasAnyRoleAddsDefaultPrefix() throws Exception {
+        assertThat(root.hasAnyRole("NO","A")).isTrue();
+        assertThat(root.hasAnyRole("NO","NOT")).isFalse();
+    }
+
+    @Test
+    public void hasAnyRoleDoesNotAddDefaultPrefixForAlreadyPrefixedRoles() throws Exception {
+        assertThat(root.hasAnyRole("ROLE_NO","ROLE_A")).isTrue();
+        assertThat(root.hasAnyRole("ROLE_NO","ROLE_NOT")).isFalse();
+    }
+
+    @Test
+    public void hasAnyRoleEmptyPrefixDoesNotAddsDefaultPrefix() throws Exception {
+        root.setDefaultRolePrefix("");
+        assertThat(root.hasRole("A")).isFalse();
+        assertThat(root.hasRole("ROLE_A")).isTrue();
+    }
+
+    @Test
+    public void hasAnyRoleNullPrefixDoesNotAddsDefaultPrefix() throws Exception {
+        root.setDefaultRolePrefix(null);
+        assertThat(root.hasAnyRole("A")).isFalse();
+        assertThat(root.hasAnyRole("ROLE_A")).isTrue();
+    }
+
+    @Test
+    public void hasAuthorityDoesNotAddDefaultPrefix() throws Exception {
+        assertThat(root.hasAuthority("A")).isFalse();
+        assertThat(root.hasAnyAuthority("NO","A")).isFalse();
+        assertThat(root.hasAnyAuthority("ROLE_A","NOT")).isTrue();
+    }
 }

+ 1 - 1
core/src/test/java/org/springframework/security/access/expression/method/MethodExpressionVoterTests.java

@@ -19,7 +19,7 @@ import org.springframework.security.util.SimpleMethodInvocation;
 
 @SuppressWarnings("unchecked")
 public class MethodExpressionVoterTests {
-    private TestingAuthenticationToken joe = new TestingAuthenticationToken("joe", "joespass", "blah");
+    private TestingAuthenticationToken joe = new TestingAuthenticationToken("joe", "joespass", "ROLE_blah");
     private PreInvocationAuthorizationAdviceVoter am =
         new PreInvocationAuthorizationAdviceVoter(new ExpressionBasedPreInvocationAdvice());
 

+ 14 - 14
docs/manual/src/docs/asciidoc/index.adoc

@@ -628,7 +628,7 @@ protected void configure(HttpSecurity http) throws Exception {
         .authorizeRequests()                                                                <1>
             .antMatchers("/resources/**", "/signup", "/about").permitAll()                  <2>
             .antMatchers("/admin/**").hasRole("ADMIN")                                      <3>
-            .antMatchers("/db/**").access("hasRole('ROLE_ADMIN') and hasRole('ROLE_DBA')")  <4>
+            .antMatchers("/db/**").access("hasRole('ADMIN') and hasRole('DBA')")            <4>
             .anyRequest().authenticated()                                                   <5>
             .and()
         // ...
@@ -639,7 +639,7 @@ protected void configure(HttpSecurity http) throws Exception {
 <1> There are multiple children to the `http.authorizeRequests()` method each matcher is considered in the order they were declared.
 <2> We specified multiple URL patterns that any user can access. Specifically, any user can access a request if the URL starts with "/resources/", equals "/signup", or equals "/about".
 <3> Any URL that starts with "/admin/" will be resticted to users who have the role "ROLE_ADMIN". You will notice that since we are invoking the `hasRole` method we do not need to specify the "ROLE_" prefix.
-<4> Any URL that starts with "/db/" requires the user to have both "ROLE_ADMIN" and "ROLE_DBA"
+<4> Any URL that starts with "/db/" requires the user to have both "ROLE_ADMIN" and "ROLE_DBA". You will notice that since we are using the `hasRole` expression we do not need to specify the "ROLE_" prefix.
 <5> Any URL that has not already been matched on only requires that the user be authenticated
 
 [[jc-authentication]]
@@ -817,7 +817,7 @@ public class MethodSecurityConfig {
 }
 ----
 
-Adding an annotation to a method (on an class or interface) would then limit the access to that method accordingly. Spring Security���s native annotation support defines a set of attributes for the method. These will be passed to the AccessDecisionManager for it to make the actual decision:
+Adding an annotation to a method (on an class or interface) would then limit the access to that method accordingly. Spring Security's native annotation support defines a set of attributes for the method. These will be passed to the AccessDecisionManager for it to make the actual decision:
 
 [source,java]
 ----
@@ -844,7 +844,7 @@ public class MethodSecurityConfig {
 }
 ----
 
-These are standards-based and allow simple role-based constraints to be applied but do not have the power Spring Security���s native annotations. To use the new expression-based syntax, you would use
+These are standards-based and allow simple role-based constraints to be applied but do not have the power Spring Security's native annotations. To use the new expression-based syntax, you would use
 
 [source,java]
 ----
@@ -1017,7 +1017,7 @@ All you need to enable web security to begin with is
 [source,xml]
 ----
 <http>
-  <intercept-url pattern="/**" access="hasRole('ROLE_USER')" />
+  <intercept-url pattern="/**" access="hasRole('USER')" />
   <form-login />
   <logout />
 </http>
@@ -2388,7 +2388,7 @@ As we saw earlier in the namespace chapter, it's possible to use multiple `http`
 ----
 <!-- Stateless RESTful service using Basic authentication -->
 <http pattern="/restful/**" create-session="stateless">
-  <intercept-url pattern='/**' access="hasRole('ROLE_REMOTE')" />
+  <intercept-url pattern='/**' access="hasRole('REMOTE')" />
   <http-basic />
 </http>
 
@@ -2397,7 +2397,7 @@ As we saw earlier in the namespace chapter, it's possible to use multiple `http`
 
 <!-- Additional filter chain for normal users, matching all other requests -->
 <http>
-  <intercept-url pattern='/**' access="hasRole('ROLE_USER')" />
+  <intercept-url pattern='/**' access="hasRole('USER')" />
   <form-login login-page='/login.htm' default-target-url="/home.htm"/>
   <logout />
 </http>
@@ -4270,16 +4270,16 @@ The base class for expression root objects is `SecurityExpressionRoot`. This pro
 | Expression | Description
 
 | `hasRole([role])`
-| Returns `true` if the current principal has the specified role. This is a synonym for `hasAuthority([authority])`
+| Returns `true` if the current principal has the specified role. By default if the supplied role does not start with 'ROLE_' it will be added. This can be customized by modifying the `defaultRolePrefix` on `DefaultWebSecurityExpressionHandler`.
 
 | `hasAnyRole([role1,role2])`
-| Returns `true` if the current principal has any of the supplied roles (given as a comma-separated list of strings) This is a synonym for `hasAnyAuthority([authority1,authority2])`
+| Returns `true` if the current principal has any of the supplied roles (given as a comma-separated list of strings). By default if the supplied role does not start with 'ROLE_' it will be added. This can be customized by modifying the `defaultRolePrefix` on `DefaultWebSecurityExpressionHandler`.
 
 | `hasAuthority([authority])`
-| Returns `true` if the current principal has the specified authority. This is a synonym for `hasRole([role])`
+| Returns `true` if the current principal has the specified authority.
 
 | `hasAnyAuthority([authority1,authority2])`
-| Returns `true` if the current principal has any of the supplied roles (given as a comma-separated list of strings) `hasAnyRole([role1,role2])``hasAnyRole([role1,role2])`
+| Returns `true` if the current principal has any of the supplied roles (given as a comma-separated list of strings)
 
 | `principal`
 | Allows direct access to the principal object representing the current user
@@ -4351,7 +4351,7 @@ The most obviously useful annotation is `@PreAuthorize` which decides whether a
 
 [source,java]
 ----
-@PreAuthorize("hasRole('ROLE_USER')")
+@PreAuthorize("hasRole('USER')")
 public void create(Contact contact);
 ----
 
@@ -4430,7 +4430,7 @@ As you may already be aware, Spring Security supports filtering of collections a
 
 [source,java]
 ----
-@PreAuthorize("hasRole('ROLE_USER')")
+@PreAuthorize("hasRole('USER')")
 @PostFilter("hasPermission(filterObject, 'read') or hasPermission(filterObject, 'admin')")
 public List<Contact> getAll();
 ----
@@ -7694,7 +7694,7 @@ Defines an authorization rule for a message.
 * **pattern** An ant based pattern that matches on the Message destination. For example, "/**" matches any Message with a destination; "/admin/**" matches any Message that has a destination that starts with "/admin/**".
 
 [[nsa-message-interceptor-access]]
-* **access** The expression used to secure the Message. For example, "denyAll" will deny access to all of the matching Messages; "permitAll" will grant access to all of the matching Messages; "hasRole('ROLE_ADMIN') requires the current user to have the role 'ROLE_ADMIN' for the matching Messages.
+* **access** The expression used to secure the Message. For example, "denyAll" will deny access to all of the matching Messages; "permitAll" will grant access to all of the matching Messages; "hasRole('ADMIN') requires the current user to have the role 'ROLE_ADMIN' for the matching Messages.
 
 [[nsa-authentication]]
 === Authentication Services

+ 20 - 1
web/src/main/java/org/springframework/security/web/access/expression/DefaultWebSecurityExpressionHandler.java

@@ -17,6 +17,7 @@ import org.springframework.util.Assert;
 public class DefaultWebSecurityExpressionHandler extends AbstractSecurityExpressionHandler<FilterInvocation> implements SecurityExpressionHandler<FilterInvocation> {
 
     private AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+    private String defaultRolePrefix = "ROLE_";
 
     @Override
     protected SecurityExpressionOperations createSecurityExpressionRoot(Authentication authentication, FilterInvocation fi) {
@@ -24,6 +25,7 @@ public class DefaultWebSecurityExpressionHandler extends AbstractSecurityExpress
         root.setPermissionEvaluator(getPermissionEvaluator());
         root.setTrustResolver(trustResolver);
         root.setRoleHierarchy(getRoleHierarchy());
+        root.setDefaultRolePrefix(defaultRolePrefix);
         return root;
     }
 
@@ -39,4 +41,21 @@ public class DefaultWebSecurityExpressionHandler extends AbstractSecurityExpress
         Assert.notNull(trustResolver, "trustResolver cannot be null");
         this.trustResolver = trustResolver;
     }
-}
+
+    /**
+     * <p>
+     * Sets the default prefix to be added to {@link #hasAnyRole(String...)} or
+     * {@link #hasRole(String)}. For example, if hasRole("ADMIN") or hasRole("ROLE_ADMIN") is passed in,
+     * then the role ROLE_ADMIN will be used when the defaultRolePrefix is "ROLE_" (default).
+     * </p>
+     *
+     * <p>
+     * If null or empty, then no default role prefix is used.
+     * </p>
+     *
+     * @param defaultRolePrefix the default prefix to add to roles. Default "ROLE_".
+     */
+    public void setDefaultRolePrefix(String defaultRolePrefix) {
+        this.defaultRolePrefix = defaultRolePrefix;
+    }
+}