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

Consider adding PermissionAuthorizationManager #11361

Closed
jzheaux opened this issue Jun 10, 2022 · 3 comments
Closed

Consider adding PermissionAuthorizationManager #11361

jzheaux opened this issue Jun 10, 2022 · 3 comments
Assignees
Labels
in: core An issue in spring-security-core status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Jun 10, 2022

There may be some value in introducing PermissionAuthorizationManager to allow a programmatic equivalent to hasPermission available through SpEL and SecurityExpressionRoot.

@evgeniycheban
Copy link
Contributor

@jzheaux I'd like to work on this.

@jzheaux
Copy link
Contributor Author

jzheaux commented Jun 24, 2022

Thanks for putting together a PR, @evgeniycheban, so it was a little easier to consider what this would look like.

After reviewing it, I don't think PermissionAuthorizationManager provides enough value to add to Spring Security at this time.

Method security is still primarily reliant on SpEL, so there is little incentive there for a programmatic alternative. Further, if someone did want to use this class, they'd still certainly have to customize things since one needs to still extract the target (e.g. one of the method arguments) from the MethodInvocation.

Filter security also would have to customize things since one needs to extract the targetId (e.g. one of the path variables) from the RequestAuthorizationContext.

As such, I think we should wait until it's clearer in what circumstances an application could benefit from PermissionAuthorizationManager. I've made a note to add instructions in the migration guide for hasPermission, and we can evaluate the feedback there.

Otherwise, I'm going to close this ticket for now. We can always reopen should the manager's helpfulness become clearer.

@jzheaux jzheaux closed this as completed Jun 24, 2022
@jzheaux jzheaux self-assigned this Jun 24, 2022
@jzheaux jzheaux added in: core An issue in spring-security-core status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 24, 2022
@rwinch
Copy link
Member

rwinch commented Jun 27, 2022

Just chiming in here. PermissionEvaluator was nice because it allowed for simplified API for users to implement to customize authorization, but AuthorizationManager already provides a very simple API for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core An issue in spring-security-core status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants