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 Interceptor based @RolesAllowed #295

Open
arjantijms opened this issue Oct 17, 2023 · 5 comments
Open

Add Interceptor based @RolesAllowed #295

arjantijms opened this issue Oct 17, 2023 · 5 comments
Assignees
Milestone

Comments

@arjantijms
Copy link
Contributor

The Jakarta platform included the @RolesAllowed annotation (via commons annotation), but it's only used and defined by Enterprise Beans in Jakarta (and in JWT in MicroProfile).

We would like to introduce an interceptor based version of this, that largely does what @RolesAllowed does, but owned by Jakarta Security, being an interceptor (so can be composed and will be automatically applied by CDI), and having some additional features not directly supported by @RolesAllowed.

Specifically needed is the distinction to throw an exception when the caller of the annotated method is not the correct role, or to trigger authenticate (in a web context). See SecurityContext#authenticate for details on triggering authentication.

@OndroMih
Copy link
Contributor

Would it be OK to also support the existing @RolesAllowed annotation, which is in the Common annotations spec? Or, maybe even better, to modify the existing RolesAllowed annotation directly in the Commons annotations spec to make it an interceptor binding annotation, and add some optoinal attributes, e.g. to set the behavior when the caller doesn't have the required role.

I think that it's better to reuse the existing annotation if possible, rather than introducing a new annotation. Even if that means we need to make the change in the Common annotations spec.

@arjantijms
Copy link
Contributor Author

The new annotation could have extra attributes to set the "trigger authentication mechanism" or "throw exception" when not authenticated.

We don't necessarily need to update common annotations to support the existing RolesAllowed though. A CDI extension can pick it up, and then add an interceptor manually. With binding this just happens automatically, and without binding we need to do it ourselves.

@OndroMih
Copy link
Contributor

Yes, that's true.

Then I would be for using the existing @RolesAllowed annotation and activate an interceptor from a CDI extension.
We can support additional properties via another annotation, like:

@RolesAllowed("admin")
@ActionWhenDenied(TRIGGER_AUTH)
public class AdminCommands { ...
}

@arjantijms
Copy link
Contributor Author

That's certainly an option to be considered; an extra annotation to finetune behaviour.

@hantsy
Copy link

hantsy commented Nov 13, 2024

I think it is better to provide alternatives to the existing RolesAllowed and copy some modern concepts from Spring and other frameworks, such as Quakrus/Micronaut. I want to accept EL in the new annotations, which also support config in the future when the config spec is done and integrated with EL.

eg.

@Authencticated // valid for any authenticated users
public void someMethod(){...}

@PreAuthorized("#{securityContext.xxx && p0.xxx & p1.xxx}") // read method parameters into p0, p1
@PreAuthorized("#{roles.contains()}")
public void someMethod(){...}

@PostAuthorized("#{return.xxx}") //read the returned instance into `return`
public SomeObject someMethod(){...}

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

No branches or pull requests

3 participants