Skip to content

Add AuthorizationDecision to Authorization events #9286

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

Closed
jzheaux opened this issue Dec 16, 2020 · 3 comments
Closed

Add AuthorizationDecision to Authorization events #9286

jzheaux opened this issue Dec 16, 2020 · 3 comments
Assignees
Labels
in: core An issue in spring-security-core type: enhancement A general enhancement
Milestone

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Dec 16, 2020

It would be nice if AuthorizationFailureEvent and AuthorizedEvent each held a reference to the AuthorizationDecision that was made.

The new constructor should not take a Collection of ConfigAttributes.

@jzheaux jzheaux added in: core An issue in spring-security-core type: enhancement A general enhancement status: ideal-for-contribution An issue that we actively are looking for someone to help us with labels Dec 16, 2020
@jzheaux jzheaux changed the title Add AuthorizationDecision to AuthorizationFailureEvent Add AuthorizationDecision to Authorization events Dec 17, 2020
@parikshitdutta
Copy link
Contributor

parikshitdutta commented Mar 29, 2021

Thanks @jzheaux for assigning #9288 and pointing me to this one.

I was thinking to keep reference of Authentication in event classes. To my understanding, AuthorizationSuccessEvent should be raised if AuthorizationDecision.isGranted(), otherwise AuthorizationFailureEvent should be raised.

I mean, either it is granted or not, there is nothing more in AuthorizationDecision.

While wrapping Authentication in events can help in passing GrantedAuthority, Principal, for which either AuthorizationDecision is granted or not, determined with success or failure event respectively.

Please share your thought.

@jzheaux
Copy link
Contributor Author

jzheaux commented Mar 29, 2021

I mean, either it is granted or not, there is nothing more in AuthorizationDecision.

Since AuthorizationDecision can be subclassed, we can't say that there is nothing more in it.

For example, if you take a look at #9287, there's value in including the reasons that an authorization decision was made.

@parikshitdutta
Copy link
Contributor

Since AuthorizationDecision can be subclassed, we can't say that there is nothing more in it.

Totally! as an after-thought I knew it coming :), thanks for sharing your thought.
Please assign this to me as well.

@jzheaux jzheaux removed the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Mar 29, 2021
@jzheaux jzheaux added this to the 5.7.0-RC1 milestone Jun 6, 2022
@jzheaux jzheaux closed this as completed Jun 6, 2022
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 type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants