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

Different behaviour of @RolesAllowed annotation on @EnableMethodSecurity vs @EnableGlobalMethodSecurity #11701

Closed
mgr32 opened this issue Aug 12, 2022 · 1 comment
Assignees
Labels
in: core An issue in spring-security-core status: waiting-for-triage An issue we've not yet triaged type: bug A general bug

Comments

@mgr32
Copy link

mgr32 commented Aug 12, 2022

Describe the bug

Consider the following controller:

@RestController
public class SecuredController {

    @GetMapping(path = "/rolesAllowed_GUEST")
    @RolesAllowed("GUEST")
    public String rolesAllowed_GUEST() {
        return "GUEST";
    }

    @GetMapping(path = "/rolesAllowed_ROLE_GUEST")
    @RolesAllowed("ROLE_GUEST")
    public String rolesAllowed_ROLE_GUEST() {
        return "ROLE_GUEST";
    }

}
  1. When legacy @EnableGlobalMethodSecurity(jsr250Enabled = true) is used, both endpoints are accessible when request is authenticated with ROLE_GUEST:

    • this is because ROLE_ prefix is added only when not already present in @RolesAllowed value (code).
  2. After switching to the new annotation (@EnableMethodSecurity(jsr250Enabled = true), /rolesAllowed_GUEST works ok, but /rolesAllowed_ROLE_GUEST returns 403 instead of 200:

To Reproduce

  1. Enable method security with @EnableMethodSecurity(jsr250Enabled = true).
  2. Create a REST Controller with @RolesAllowed("ROLE_GUEST").
  3. Configure a simple UserDetailsService:
    @Bean
        public UserDetailsService userDetailsService() {
            return new InMemoryUserDetailsManager(User.builder().username("guest").password("{noop}guest").roles("GUEST").build());
        }
    
  4. Start the server.
  5. Send a request with basic auth:
    curl --request GET 'http://localhost:8080/rolesAllowed_ROLE_GUEST' --header 'Authorization: Basic Z3Vlc3Q6Z3Vlc3Q='
    
  6. Observe the response code - it is 403 instead of 200.
  7. Stop the server, switch the @EnableMethodSecurity annotation to @EnableGlobalMethodSecurity and retry the test. Observe the response code - it is 200.

Tested on Spring Security 5.7.2, but the code that unconditionally adds the prefix seems to be the same on main.
See the sample repository provided below for a complete example.

Expected behavior

@EnableMethodSecurity should behave in the same way as @EnableGlobalMethodSecurity regarding conditional adding of ROLE_ prefix.
Alternatively, the change should be documented in Method Security docs, as it can be breaking for some applications.

Sample

https://github.com/mgr32/spring-security-method-security-issue

Summary including other annotations (assuming HTTP request with proper Authorization header):

Enabling annotation Endpoint annotation HTTP response status code
@EnableGlobalMethodSecurity @RolesAllowed("GUEST") ✔️ 200
@EnableMethodSecurity @RolesAllowed("GUEST") ✔️ 200
@EnableGlobalMethodSecurity @RolesAllowed("ROLE_GUEST") ✔️ 200
@EnableMethodSecurity @RolesAllowed("ROLE_GUEST") ❌ ❗ 403
@EnableGlobalMethodSecurity @Secured("GUEST") ❌ 403
@EnableMethodSecurity @Secured("GUEST") ❌ 403
@EnableGlobalMethodSecurity @Secured("ROLE_GUEST") ✔️ 200
@EnableMethodSecurity @Secured("ROLE_GUEST") ✔️ 200
@EnableGlobalMethodSecurity @PreAuthorize("hasRole('GUEST')") ✔️ 200
@EnableMethodSecurity @PreAuthorize("hasRole('GUEST')") ✔️ 200
@EnableGlobalMethodSecurity @PreAuthorize("hasRole('ROLE_GUEST')") ✔️ 200
@EnableMethodSecurity @PreAuthorize("hasRole('ROLE_GUEST')") ✔️ 200
@mgr32 mgr32 added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Aug 12, 2022
@sjohnr sjohnr added the in: core An issue in spring-security-core label Aug 15, 2022
@jzheaux
Copy link
Contributor

jzheaux commented Aug 16, 2022

Thanks, @mgr32, especially for the detailed tabular comparison. It seems more consistent to not conditionally add ROLE_. As such, I'm going to close this ticket, but also ensure that an example like this is included in the AuthorizationManager guide.

@jzheaux jzheaux closed this as completed Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core An issue in spring-security-core status: waiting-for-triage An issue we've not yet triaged type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants