Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GrantedAuthorityDefaults does not apply on ExpressionInterceptUrlRegistry#hasRole #4134

Closed
kazuki43zoo opened this issue Nov 15, 2016 · 4 comments · Fixed by #10078
Closed
Assignees
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement

Comments

@kazuki43zoo
Copy link
Contributor

kazuki43zoo commented Nov 15, 2016

Summary

Since 4.2, we can control the default role prefix(default is ROLE_) at the single point of definition (gh-3701).
However, it not apply to access control definitions using the ExpressionInterceptUrlRegistry#hasRole().

@Configuration
public class SecurityConfig extends WebSecurityConfigurerAdapter {

    @Bean
    GrantedAuthorityDefaults grantedAuthorityDefaults() {
        return new GrantedAuthorityDefaults(""); // Remove the ROLE_ prefix
    }

    @Override
    protected void configure(HttpSecurity http) throws Exception {
        // ...
        http.authorizeRequests()
                .anyRequest().hasRole("USER"); // Allow access from user granted USER role
    }

    @Bean
    public UserDetailsService userDetailsService() {
        InMemoryUserDetailsManager userDetailsManager = new InMemoryUserDetailsManager();
        userDetailsManager.createUser(
                User.withUsername("kazuki43zoo")
                        .password("password")
                        .authorities("USER")  // Grant the USER role (without ROLE_ prefix)
                        .build());
        return userDetailsManager;
    }

}

Actual Behavior

If login with the kazuki43zoo, 403 Forbidden error has been occurred.

Whitelabel Error Page

This application has no explicit mapping for /error, so you are seeing this as a fallback.

Wed Nov 16 01:06:38 JST 2016
There was an unexpected error (type=Forbidden, status=403).
Access is denied

Expected Behavior

Can access the specified resource after authentication success. (200 OK)

Version

4.2.0

Workaround

It work as following configuration instead of.

    @Override
    protected void configure(HttpSecurity http) throws Exception {
        // ...
        http.authorizeRequests()
                .anyRequest().access("hasRole('USER')"); // Use the access() instead of hasRole()
    }
@kazuki43zoo
Copy link
Contributor Author

kazuki43zoo commented Nov 15, 2016

Related with this, the UserBuilder#roles method append the ROLE_ prefix to granted authority. Is this behavior works as designed ?

    @Bean
    public UserDetailsService userDetailsService() {
        InMemoryUserDetailsManager userDetailsManager = new InMemoryUserDetailsManager();
        userDetailsManager.createUser(
                User.withUsername("kazuki43zoo")
                        .password("password")
                        .roles("USER") // has been append the ROLE_ prefix
//                        .authorities("USER")
                        .build());
        return userDetailsManager;
    }

In this case, we can use the authorities method instead of.

@rwinch rwinch added this to the 4.2.1 milestone Nov 15, 2016
@rwinch rwinch self-assigned this Nov 15, 2016
@rwinch rwinch modified the milestones: 4.2.1, 5.0.0.M1 Dec 21, 2016
@rwinch rwinch modified the milestones: 5.0.0.M1, 5.0.0.M2 May 10, 2017
@rwinch rwinch modified the milestones: 5.0.0.M2, 5.0.0.M3 Jun 15, 2017
@jgrandja jgrandja modified the milestones: 5.0.0.M3, 5.0.0.M4 Jul 24, 2017
@rwinch rwinch modified the milestones: 5.0.0.M4, 5.0.0.M5 Sep 13, 2017
@tomas-silhavy
Copy link

I can confirm that it's not working in version 4.2.3.
The problem lies here

ExpressionUrlAuthorizationConfigurer.hasRole(role)

vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv

private static String hasRole(String role) {
        Assert.notNull(role, "role cannot be null");
        if(role.startsWith("ROLE_")) {
            throw new IllegalArgumentException("role should not start with 'ROLE_' since it is automatically inserted. Got '" + role + "'");
        } else {
            return "hasRole('ROLE_" + role + "')";
        }
    }

and same constant string "ROLE_" is on several places in this class.

@rwinch rwinch modified the milestones: 5.0.0.M5, 5.0.0.RC1 Oct 3, 2017
@jeffsheets
Copy link

jeffsheets commented Oct 18, 2017

Possibly related to this: https://stackoverflow.com/a/46817507/1469525
Or maybe completely separate.

GrantedAuthorityDefaults will change the prefix for the DefaultWebSecurityExpressionHandler and the DefaultMethodSecurityExpressionHandler, but it doesn't modify the RoleVoter.rolePrefix that is setup from @EnableGlobalMethodSecurity.

It would be nice if the RoleVoter also could get the rolePrefix from GrantedAuthorityDefaults

@rwinch rwinch modified the milestones: 5.0.0.RC1, 5.0.0 Oct 30, 2017
@rwinch rwinch modified the milestones: 5.0.0, 5.1.0.M1 Nov 27, 2017
@BenDol
Copy link

BenDol commented Dec 4, 2017

protected AccessDecisionManager accessDecisionManager() { inside GlobalMethodSecurityConfiguration this is simply calling new RoleVoter() rather than using the Bean factory that has the proper prefix configuration.

@rwinch rwinch modified the milestones: 5.1.0.M1, 5.1.0.RC1 Dec 19, 2017
@rwinch rwinch modified the milestones: 5.1.0.M2, 5.1.0.RC1 Jul 26, 2018
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 7, 2019
@rwinch rwinch removed their assignment Jul 29, 2019
@jzheaux jzheaux self-assigned this Sep 2, 2021
@jzheaux jzheaux added in: config An issue in spring-security-config type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 2, 2021
quaff added a commit to quaff/spring-security that referenced this issue Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants