spring-security
spring-security copied to clipboard
Make default expression handler in PrePostMethodSecurityConfiguration to use existing permission evaluator
Expected Behavior
The expression handler that gets created per default in PrePostMethodSecurityConfiguration
at https://github.com/spring-projects/spring-security/blob/74d646f569ad5df4bd1055ab016444cea9af6fbd/config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java#L63 doesn't use the existing permission evaluator from the Spring context but keeps the default DenyAllPermissionEvaluator
.
Current Behavior
The expression handler should be setup in the PrePostMethodSecurityConfiguration
with the existing permission evaluator like
@Autowired(required = false)
void setPermissionEvaluator(PermissionEvaluator permissionEvaluator) {
this.expressionHandler.setPermissionEvaluator(permissionEvaluator);
}
Context
I've switched from the @EnableGlobalMethodSecurity
annotation to the @EnableMethodSecurity
annotation and this caused the existing permission evaluator not to be applied to @PreAuthorize
annotations for methods.
There is of course a way to fix that easily by defining a custom expression handler that utilizes the permission evaluator, but I would've expected that the old way of the @EnableGlobalMethodSecurity
using the existing permission evaluator should also work with the new annotation without defining additional beans.
But maybe this was a conscious decision or I'm simply missing something. Thanks in advance for taking a look.
@GFriedrich, glad to hear you are using the new @EnableMethodSecurity
.
It is intentional to try and keep things as simple as possible for the initial releases. Since this feature's absence doesn't create a security issue in the worst case (all evaluations are denied by default) and because it's quite simple to make the adjustment, it's a lower priority feature to add. Also, I don't want to add a feature just for the sake of making it easier to migrate.
That said, I think that it's reasonable to leave this ticket open and see if people vote for it as a desired feature.
In the meantime, I agree that the simplest way to use your PermissionEvaluator
is to publish a @Bean
like so:
@Bean
MethodSecurityExpressionHandler methodSecurityExpressionHandler() {
DefaultMethodSecurityExpressionHandler expressionHandler = new DefaultMethodSecurityExpressionHandler();
expressionHandler.setPermissionEvaluator(...);
return expressionHandler;
}
Or, if you don't want to affect the expression handler, you can always exercise your PermissionEvaluator
bean directly in the annotations like this:
@PreAuthorize("@permissionEvaluator.hasPermission(...)")
adding the @permissionEvaluator.
string to each expression.
@jzheaux: Thanks for checking and good to hear that I didn't miss anything. Maybe it is worth to mention it somewhere that there is at least this kind of difference when switching the annotations. Simply to make a migration path clear to everybody and not falling for the same issue over and over again. Thanks for your time nevertheless. 🤝
Agreed, @GFriedrich. I've linked this ticket to the migration guide ticket so that it gets included.