Summary
@PreAuthorize is executed after @Valid validation
Actual Behavior
@PreAuthorize is executed after @Valid validation
Using @PreAuthorize to check user permissions/authorities result in user first getting information of not valid request and then about not having permissions. Workaround for this is custom annotation or using http.authorizeRequests() in configuration classes.
Expected Behavior
I believe we should pursue to: @PreAuthorize is executed before @Valid validation
As described in ticket, which was said to be defined in wrong project (I was not able to find relevant ticket/discussion in spring-security): https://github.com/spring-projects/spring-boot/issues/10157
Maybe it is possible to add additional @PreAuthorize validation as HandlerInterceptorAdapter (in case handler is in type of HandlerMethod) - it seems to be executed before @Valid annotations.
Version
5.1.3
Comment From: dmitriychernov7
I agree. I expected that security checks go first, and only then validation. Consider next scenario: there is an endpoint to create a Book. Book has a field "authorId". There is a custom validation annotation that will check that author exists with id "authorId". Now if security goes after validation, User that does not have access to this endpoint can deduct ids of existing/nonexisting authors. This should not be possible
Comment From: onkobu
This even allows enumerating endpoints by simple binary distinction: HTTP.400 vs. HTTP.403. This was already tried with databases to enumerate tables based on two different error codes and fuzzying table names. An attacker without permission could use the distinction between HTTP.400 and HTTP.404/ 403 to permutate endpoint-URLs. On first sight it might be a counter measure to block enumeration based on number of connections but a sophisticated attacker has plenty of time and files maybe 10-100 requests a day over a 2 year span (to a high frequency set of endpoints).
(It'd still be a distinction between HTTP.404 for a URL not bound to an endpoint and HTTP.403 if unauthorized, but it is a bit better to filter and set up counter measures than HTTP.400.)
Comment From: Illapikov
We experience the same issue with using both @PreAuthorize and @RequestParam. Spring complains about missing parameter (or invalid parameter if property editor was used) before the security checks.
Comment From: rwinch
I don't think there is a lot we can do here from the Spring Security side. Spring Security's method support requires all of the arguments to the method to be resolved. In order for the arguments to be resolved type conversion and validation needs to pass. If you don't have the need for that arguments, then you can leverage Spring Security's URL based security to ensure security is executed first.
As far as validation goes, if you accept the BindingResult as an argument, then Spring Security will be invoked even though there are errors.
If anyone has suggestions, I'd be glad to discuss. However, at this time I don't think there is much we can do.
Comment From: spring-projects-issues
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.
Comment From: pbialonc
It is general issue. I've created custom annotation in my solution that uses HandlerMethod as I didn't need all features that @PreAuthorize has. So this is not an issue for me anymore but in general there is such.
Comment From: rwinch
I'm closing this. There isn't anything we can do to solve this generically. Additionally, users can easily use the suggestion above https://github.com/spring-projects/spring-security/issues/6545#issuecomment-514409908 to solve the problem
Comment From: onkobu
If I had to implement this, I'd split the handler chain into a request- and an error handler chain. While request handling determines the method and triggers the error the error handler is a separate concern. This way request handling can stay as it is and Security's error handler sits before Validation's (which is before default). This also makes second pass unnecessary.
In addition Security's error handler can be configured to either show historic behavior or turn any error code other than 200 into something different – like always returning 404. (During development or testing it can be switched off and in production mitigate endpoint mapping breach.)
Comment From: UltimateFighter
I agree. I expected that security checks go first, and only then validation. Consider next scenario: there is an endpoint to create a Book. Book has a field "authorId". There is a custom validation annotation that will check that author exists with id "authorId". Now if security goes after validation, User that does not have access to this endpoint can deduct ids of existing/nonexisting authors. This should not be possible
I also agree that any structure of the request/endpoint should be hidden behind 403. However, the example is not correct, since a Bean validation should not access the DB. This should be done in the code.
Comment From: onkobu
I also agree that any structure of the request/endpoint should be hidden behind 403. However, the example is not correct, since a Bean validation should not access the DB. This should be done in the code.
At least there ought to be a configuration for this like Order-annotation for beans, a handler to be overwritten or a property to set.
Regarding the existence check I could imagine some Spring Expression Language annotated to the RestController doing exactly this: fancy 404 without lots of coding. I agree that this isn't appropriate to the novice but Tao is even with such a »Look Ma, no service for validation«. (Didn't actually try this but from my experience there ought to be a sophisticated eval-like runtime-way into this.)
Comment From: UltimateFighter
It is simply part of the "information hiding". I think it is mandatory (at least it should be default) that the Spring auth handling never lets a user on the endpoint who has no authorization. Otherwise he could spy on the endpoint based on the validation messages.
Comment From: UltimateFighter
Are there still attempts to close the security gap here?
In all of our security audit findings, these errors come up again and again.
I can't understand the explanation „worked by design“ here.
Why doesn't @PreAuthorize work the same way as executed Spring Security's URL based security ?
It could trigger the same configuration and that would be what a developer expects.
The advantage of a better overview (directly as the endpoint annotation) would remain.
Comment From: nithril
I don't think there is a lot we can do here from the Spring Security side. Spring Security's method support requires all of the arguments to the method to be resolved. In order for the arguments to be resolved type conversion and validation needs to pass. If you don't have the need for that arguments, then you can leverage Spring Security's URL based security to ensure security is executed first.
For example @Secured is an annotation that theoretically does not need the method arguments to be resolved.
Can be an argument on @PreAuthorize to execute after / before the resolution of the argument.
Comment From: abccbaandy
It's 2024 now. Still not fix this issue?
We all know it's not bug, but If the design is wrong, then we should fix it. Even it may need a lot of work and cause breaking change, but spring security is not like JDK, we all know you guys don't really care about back compatible.
Comment From: abdellahi-brahim
Please re-open this!