spring-security
spring-security copied to clipboard
ReactiveAuthorizationManager + Reactive Method Security
Closes gh-9401
Hi, @evgeniycheban, are you able to apply the requested changes?
Hi, @evgeniycheban, are you able to apply the requested changes?
Hi, sorry for the long time of inactivity on this PR. I've been busy at work these few months. I plan to continue working on this next week.
@jzheaux I updated the PR according to your comments.
@jzheaux I've updated the PR, please take a look.
@jzheaux @eleftherias I updated the PR, please take a look.
@rwinch @jzheaux I updated the PR, please take a look.
@evgeniycheban I am seeing error " EL1001E: Type conversion problem, cannot convert from reactor.core.publisher.MonoJust<java.lang.Boolean> to java.lang.Boolean" when mixing a non reactive and reactive expression in @PreAuthorize.
Any clue why is this happening?
Example:- @PreAuthorize("@authz.checkReactive() || hasRole('ADMIN')")
@evgeniycheban I am seeing error " EL1001E: Type conversion problem, cannot convert from reactor.core.publisher.MonoJust<java.lang.Boolean> to java.lang.Boolean" when mixing a non reactive and reactive expression in @PreAuthorize.
Any clue why is this happening?
Example:- @PreAuthorize("@authz.checkReactive() || hasRole('ADMIN')")
@code-uri Interesting note. Mixing reactive and non-reactive expressions is not currently supported. I think we should address this to the Spring Framework team so they can decide if this should be supported in Spring expressions.
@jzheaux @rwinch What do you think? Should the user be able to mix reactive and non-reactive expressions in @PreAuthorize
?
Should the user be able to mix reactive and non-reactive expressions in
@PreAuthorize
?
I wonder if the user could simply do @authz.checkReactive(#root, 'ADMIN')
and then invoke hasRole('ADMIN')
from within the authz
bean.
Personally, I'm not a huge fan of embedding logic inside the annotations as it's a bit harder to test; I'd prefer to use a bean or a custom ReactiveAuthorizationManager
. I'd probably want to wait for a clearer use case where mixing the two types in a SpEL expression is better than one of these two alternatives. Just my 2c, though.
@jzheaux @rwinch Any updates on this?
@rwinch @jzheaux I've rebased it on top of 5.8.x
and updated @since
to 5.8.
Any update on this?
i tried to rebase 5.8.x, but the 5.8.x branch was broken/couldn't be build because of mission classes ... @evgeniycheban are you going to fix the stuff mentionend in the review?
Hello @koenpunt @jens-meiss, I'm going to fix it this week.
Thanks, @evgeniycheban, just saw your update. Thank you for such a valuable and time-consuming contribution. I'll add any minor polish that remains and hopefully merge this week.
Nice, @evgeniycheban! This is now merged into 5.8.x
and main
. I also added a polish commit at e990174c89b36821a832f13e0b7096cc5cd37e4b and a documentation commit at 070dce1baf2f6ea801d4f2f28c16defb47442a2b.
Thanks again for all your consistent effort to this PR!