Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to extend method security expressions when working with EnableMethodSecurity annotation #12564

Closed
anschnapp opened this issue Jan 19, 2023 · 2 comments
Assignees
Labels
status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement

Comments

@anschnapp
Copy link

anschnapp commented Jan 19, 2023

Expected Behavior
With the deprecated annotation for method security support: @EnableGlobalMethodSecurity(prePostEnabled = true) there was a way to "give in" a custom MethodSecurityExpressionRoot by also defining a custom MethodSecurityExpressionHandler extended from the default one DefaultMethodSecurityExpressionHandler.

The purpose is to have an own custom security expression on methods by @PreAuthorize("hasCustomAttribute('attr')")

I have used it almost exactly like in this description from baeldung:
https://www.baeldung.com/spring-security-create-new-custom-security-expression#customExpression

I like to remove the deprecated annotation and replace it with @EnableMethodSecurity and still be able to support my CustomMethodSecurityExpressionHandler (which extends from DefaultMethodSecurityExpressionHandler) but have some additional expression methods defined (by an also overwritten variant of MethodSecurityExpressionRoot.

Current Behavior
It turned out that the configuration based on the new method security annotation @EnableMethodSecurity works slightly different then the old one.
And my old solution (see above) does not work any longer.

I tried quite some time to adjust it to the new configuration, but IIMO it is unnecessary hard if not impossible (without really dirty hacks and coupling to spring internals to do so.

I don't want to write a complete new DefaultMethodSecurityExpressionHandler, which also would imply to write an own MethodSecurityEvaluationContext.

I could extend from it and set an custom expression root (extended from MethodSecurityExpressionRoot) with new expression functions. (which is also not that easy to configure)

But in the end also the method createEvaluationContext must be overwritten, because it's directly create a new MethodSecurityExpressionRoot from the super class. But i cannot easily overwrite this method with my own because the original also created (a required) MethodSecurityEvaluationContext which is not known from my code base (package private). And i don't want to use hacks to couple my code to this internal class.

Potential Solutions

For me a perfect convenient solution could be to make the MethodSecurityExpressionRoot somehow configurable for the DefaultMethodSecurityExpressionHandler. Maybe as a setter for the generation code of a DefaultMethodSecurityExpressionHandler so basically something like:

defaultExpressionHandler.setSecurityRootCreation((authentication) -> new CustomMethodSecurityExpressionRoot).

Or same as a DefaultMethodSecurityExpressionHandlerCustomizer (or builder customizer).

Or something similar on upper level (so not limited to DefaultMethodSecurityExpressionHandler.

If there is already a way how to do it (don't find it in the documentation), this would of course also solve my issue.

Context
This issue prevents me from upgrading to the not deprecated annotation.
Therefore i might stick on older code and behaviour. And i might get in real trouble on next major release where this annotation could be deleted.

@anschnapp anschnapp added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Jan 19, 2023
@anschnapp anschnapp changed the title Allow to extend expression handlers when working with EnableMethodSecurity annotation Allow to extend method security expressions when working with EnableMethodSecurity annotation Jan 19, 2023
@marcusdacoregio
Copy link
Contributor

Hi @anschnapp, I believe that this is a duplicate of #12331 (comment).

Can you check the proposed workarounds in that issue and confirm if they work for you?

@marcusdacoregio marcusdacoregio added the status: waiting-for-feedback We need additional information before we can continue label Jan 19, 2023
@marcusdacoregio marcusdacoregio self-assigned this Jan 19, 2023
@anschnapp
Copy link
Author

Hi @marcusdacoregio thank you very much to point me on this!

This ticket is definitive a duplicate, sorry for that. (i searched for similar open tickets and not similar close tickets. next time i search for both)

My migrated solution was very close but i was stuck of creating of how to create the context without using package private classes.
Which is really nice solved by this workaround and this one also works in my application:
#12331 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants