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

Add PermissionsAllowed security annotation and default simple string comparison permission #22

Merged
merged 1 commit into from
Mar 7, 2023
Merged

Add PermissionsAllowed security annotation and default simple string comparison permission #22

merged 1 commit into from
Mar 7, 2023

Conversation

michalvavrik
Copy link
Member

@michalvavrik michalvavrik commented Jan 1, 2023

Adds PermissionAllowed security annotation as required pre-step for quarkusio/quarkus#10988 and transitively for quarkusio/quarkus#12219. "params" attribute can't be supported for RESTEasy Reactive endpoints as I believe whole point of the quarkusio/quarkus#19598 was to run security checks before serialization, but IMO it still makes sense to keep it for cases where security checks are done by CDI interceptor (e.g. beans and RESTEasy Classic endpoints)

The annotation is added here to accompany another standard security annotation - @Authenticated.

SecurityIdentity newly contains getPermissionCheckers as if you want to augment identity with SecurityIdentityAugmentor and keep previous permission checkers, currently you have no option.

@michalvavrik
Copy link
Member Author

cc @stuartwdouglas @sberyozkin

@michalvavrik
Copy link
Member Author

michalvavrik commented Jan 3, 2023

This blocks me as I can't open Quarkus PR till this get reviewed & merged & released. I can't assign reviewer in this project.

@sberyozkin
Copy link
Member

Hi @michalvavrik, Stuart is the project owner and he is away right now :-). Stuart, can you please add myself or someone else to the project admins so that we can review/merge PRs.

@stuartwdouglas
Copy link
Member

@michalvavrik is there a corresponding Quarkus draft PR ready to go? I don't really want to add this API until we have the implementation also ready to go (if the implementation is not ready in time we risk shipping an API that does not work).

@michalvavrik
Copy link
Member Author

@stuartwdouglas I'll open PR next week and request review from you.

Adds `PermissionAllowed` security annotation as required pre-step for quarkusio/quarkus#10988 and transitively for quarkusio/quarkus#12219. "params" attribute can't be supported for RESTEasy Reactive endpoints as I believe whole point of the quarkusio/quarkus#19598 was to run security checks before serialization, but IMO it still makes sense to keep it for cases where security checks are done through the interceptor (e.g. beans and RESTEasy Classic endpoints)

The annotation is added here to accompany another standard security annotation - `@Authenticated`.
@michalvavrik michalvavrik requested review from sberyozkin and stuartwdouglas and removed request for sberyozkin March 1, 2023 11:19
@michalvavrik
Copy link
Member Author

@stuartwdouglas ready to merge?

@sberyozkin
Copy link
Member

sberyozkin commented Mar 7, 2023

@stuartwdouglas Hi Stuart, can you actually merge now and release ? I can't approve or merge

@stuartwdouglas stuartwdouglas merged commit 51a75b7 into quarkusio:main Mar 7, 2023
@stuartwdouglas
Copy link
Member

I don't have release permissions

@michalvavrik michalvavrik deleted the feature/permissions-allowed-annotation branch March 8, 2023 08:16
@gsmet
Copy link
Member

gsmet commented Mar 8, 2023

Release done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants