spring-security icon indicating copy to clipboard operation
spring-security copied to clipboard

ReactiveAuthorizationManager + Reactive Method Security

Open evgeniycheban opened this issue 3 years ago • 14 comments

Closes gh-9401

evgeniycheban avatar Jun 04 '21 00:06 evgeniycheban

Hi, @evgeniycheban, are you able to apply the requested changes?

jzheaux avatar Oct 01 '21 16:10 jzheaux

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.

evgeniycheban avatar Oct 01 '21 17:10 evgeniycheban

@jzheaux I updated the PR according to your comments.

evgeniycheban avatar Oct 11 '21 23:10 evgeniycheban

@jzheaux I've updated the PR, please take a look.

evgeniycheban avatar Oct 17 '21 20:10 evgeniycheban

@jzheaux @eleftherias I updated the PR, please take a look.

evgeniycheban avatar Feb 20 '22 21:02 evgeniycheban

@rwinch @jzheaux I updated the PR, please take a look.

evgeniycheban avatar Mar 18 '22 19:03 evgeniycheban

@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 avatar Mar 25 '22 07:03 code-uri

@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?

evgeniycheban avatar Mar 26 '22 00:03 evgeniycheban

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 avatar Mar 30 '22 23:03 jzheaux

@jzheaux @rwinch Any updates on this?

evgeniycheban avatar May 16 '22 20:05 evgeniycheban

@rwinch @jzheaux I've rebased it on top of 5.8.x and updated @since to 5.8.

evgeniycheban avatar Jun 10 '22 00:06 evgeniycheban

Any update on this?

koenpunt avatar Aug 08 '22 10:08 koenpunt

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?

jens-meiss avatar Aug 08 '22 14:08 jens-meiss

Hello @koenpunt @jens-meiss, I'm going to fix it this week.

evgeniycheban avatar Aug 08 '22 17:08 evgeniycheban

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.

jzheaux avatar Aug 17 '22 15:08 jzheaux

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!

jzheaux avatar Aug 25 '22 21:08 jzheaux