Describe the bug
Setting up spring security, I have a security config that uses http basic auth. I have two users; rob, who is not an admin, and admin who is an admin. Here are the following 3 cases when trying to hit an endpoint secured by @\PreAuthorize("hasRole('ADMIN')"):
✅ Attempt to hit the endpoint with no or incorrect (i.e. user / pass does not map to a user) http basic auth: 403 forbidden. ✅ Attempt to hit the endpoint with user / pass "admin", "admin": the request completes successfully! ❌ Attempt to hit an endpoint with a user that does not have admin role (rob, in this case): 500 error. Stacktrace
Ideally, case 3 should be a 403 error, not a 500. Note that this does not happen if public SecurityWebFilterChain securityWebFilterChain is not included; the routes work correctly in this case.
Running on spring-security 6.0.0 - I did reproduce it on 4.7.x though. Did not run a bisect.
To Reproduce Security Config:
@Configuration
@EnableWebFluxSecurity
@EnableReactiveMethodSecurity(useAuthorizationManager = true)
public class SecurityConfig {
@Bean
public MapReactiveUserDetailsService userDetailsService() {
User.UserBuilder userBuilder = User.withDefaultPasswordEncoder();
UserDetails rob = userBuilder.username("rob")
.password("rob")
.roles("USER")
.build();
UserDetails admin = userBuilder.username("admin")
.password("admin")
.roles("USER","ADMIN")
.build();
return new MapReactiveUserDetailsService(rob, admin);
}
@Bean
public SecurityWebFilterChain securityWebFilterChain(ServerHttpSecurity http) {
return http
.httpBasic()
.and()
.csrf()
.disable()
.build();
}
}
Rest controller:
@RestController
public class StandardController {
@PreAuthorize("hasRole('ADMIN')")
@GetMapping("/hello")
public Mono<Map<String, String>> hello() {
final Map<String, String> map = new HashMap<>();
map.put("hi", "hi");
return Mono.just(map);
}
}
Expected behavior You should get a 403 when getting rejected by @/PreAuthorize, not a 500.
Sample
A link to a GitHub repository with a minimal, reproducible sample
Let me know if anything else is needed.
Comment From: marcusdacoregio
Hi @antholeole, thanks for the report.
Is there any particular reason why you are not using authorizeExchange()? At first, it should not interfere with method security, but I'm curious if that's a use case since when adding it the problem does not happen.
Comment From: antholeole
@marcusdacoregio I do use authorizeExchange for routes targetable by globs. I have some routes though that require specific permissions; I like to put them directly over the route for aesthetic purposes. I find it @PreAuthorization(hasRole(...)) to be easier to read then having to go to the security config and mentally match the glob.
In short, I think it looks better :)
Comment From: marcusdacoregio
Keep in mind that combining filter chain security and method security is recommended since you will have a more in-depth security posture. Anyway, I don't think it should interfere with the response status code. I'll reach out to the team if they have more context on this since that can be reproducible in older versions.
Comment From: antholeole
Thanks for letting me know - do you have any documentation on the security posture you mentioned? I’d like to take a look. Unfortunately I don’t fully understand that comment but I’d like to so I can be sure I’m maximizing my security. Thanks!On Dec 15, 2022, at 8:04 AM, Marcus Hert Da Coregio @.***> wrote: Keep in mind that combining filter chain security and method security is recommended since you will have a more in-depth security posture. Anyway, I don't think it should interfere with the response status code. I'll reach out to the team if they have more context on this since that can be reproducible in older versions.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>
Comment From: marcusdacoregio
I remember reading a discussion somewhere but I cannot find it now. But, as an example, if some malicious user is able to bypass the security rules in the filter chain, they will still have to deal with the method security.
Comment From: antholeole
To add some context, been investigating this.
This throws a 500 exception instead of 401:
@Configuration
@EnableWebFluxSecurity
@EnableWebFlux
@EnableReactiveMethodSecurity(useAuthorizationManager = true)
static class ReactiveMethodSecurityConfig {
@Order(Ordered.HIGHEST_PRECEDENCE)
@Bean
SecurityWebFilterChain apiHttpSecurity(ServerHttpSecurity http) {
return http.build();
}
@RestController
static class DenyAllResolver {
@RequestMapping("/**")
@PreAuthorize("denyAll")
Mono<String> resolver() {
return Mono.just("How did this happen???");
}
}
}
while this completes normally, with the 401:
@Configuration
@EnableWebFluxSecurity
@EnableWebFlux
@EnableReactiveMethodSecurity(useAuthorizationManager = true)
static class ReactiveMethodSecurityConfig {
@Order(Ordered.HIGHEST_PRECEDENCE)
@Bean
SecurityWebFilterChain apiHttpSecurity(ServerHttpSecurity http) {
return http
.authorizeExchange()
.anyExchange()
.permitAll()
.and()
.build();
}
@RestController
static class DenyAllResolver {
@RequestMapping("/**")
@PreAuthorize("denyAll")
Mono<String> resolver() {
return Mono.just("How did this happen???");
}
}
}
Just wanting to verify that this is still unintended behavior? is
@Order(Ordered.HIGHEST_PRECEDENCE)
@Bean
SecurityWebFilterChain apiHttpSecurity(ServerHttpSecurity http) {
return http.build();
}
and abuse of SecurityWebFilterChain?
Comment From: marcusdacoregio
Hi @antholeole, I contacted the team to get some context on this.
The ExceptionTranslationWebFilter is only added if you are using authorizeExchange(), without that, Spring Security will just let the exception go all the way to the web handler, which interprets it as 500. So, it would be preferable to at least use authorizeExchange().authenticated() as a catch-all. If you feel like annotations have a better look, this still affords the option to have a global policy at the filter level and a fine-grained policy at the method level. Alternatively, you can add the ExceptionTranslationWebFilter using http.addFilterAt.