quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.56k stars 2.62k forks source link

Introduce AuthorizationPolicy annotation #42710

Closed sberyozkin closed 2 weeks ago

sberyozkin commented 3 weeks ago

Description

We have briefly discussed the idea with Michal @michalvavrik.

So we have @RolesAllowed and @PermissionsAllowed annotations. We also have HttpSecurityPolicy beans.

In some cases it could be useful to have general authorization checks registered as annotations, to bind policy checks to specific JAX-RS methods, but without having to introduce some kind of expression language, the way it is done for example, in Spring's @PreAuthorize.

Implementation ideas

One idea is to do @AuthorizationPolicy(policy=CustomHttpSecurityPolicy.class) where CustomHttpSecurityPolicy is a custom HttpSecurityPolicy class without CDI annotatioons, which can also take extra request parameters in the constructor, similarly to how a custom permission class can.

Or, @AuthorizationPolicy(name="custom-policy") which points to a CDI named custom HttpSecurityPolicy class (CDI @Name would be the only CDI annotation used by the custom policy class).

In this case, it is really about making it easy to bind a specific HttpSecurityPolicy to a specific JAX-RS method, without having to resort to the HTTP policy configuration.

quarkus-bot[bot] commented 3 weeks ago

/cc @pedroigor (bearer-token)

sberyozkin commented 3 weeks ago

I'm not sure we need to get to the EL at all, it can only offer some limited options anyway but at the cost of the extra complexity

michalvavrik commented 3 weeks ago

I'm not sure we need to get to the EL at all, it can only offer some limited options anyway but at the cost of the extra complexity

I agree. Also I happen to remember @geoand closed one issue that suggested to implement Jakarta Expression language for security annotations.

Or, @AuthorizationPolicy(name="custom-policy") which points to a CDI named custom HttpSecurityPolicy class (CDI @Name would be the only CDI annotation used by the custom policy class).

We already have named policies, see io.quarkus.vertx.http.runtime.security.HttpSecurityPolicy#name, so I think it would be better to use that rather than named CDI beans.

In this case, it is really about making it easy to bind a specific HttpSecurityPolicy to a specific JAX-RS method, without having to resort to the HTTP policy configuration.

We have JAX-RS specific policies so the annotation would be relying on the implementation in place. That is the same concept and it is performing JAX-RS policies at the moment when endpoint annotations are already available. So this cannot be implemented as additional security check because it has difference interface, but IMHO it is doable without extra complexity.

@sberyozkin maybe we can wait few weeks so that people can express their opinions and suggestions? I like your implementation idea though and I can implement it. UPDATE: Actually, I have idea how to implement it without too much of a code, I will try to create POC next week at the latest.

michalvavrik commented 3 weeks ago

It's a minor detail, but I am not quite sure about @AuthorizationPolicy#name as I think value attribute would be less verbose. Maybe you have in mind extensibility of this for future?

I am against @AuthorizationPolicy(policy=CustomHttpSecurityPolicy.class) as I think we should rely on https://github.com/quarkusio/quarkus/blob/main/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityPolicy.java#L30 because I don't see difference between named policies in config and in annotations.

which can also take extra request parameters in the constructor, similarly to how a custom permission class can.

This is not possible because policies / security checks are applied before these extra request parameters are available (meaning method parameters of actual invoked endpoint). How it works with custom permissions is that we require eagerly authentication and postpone authorization to CDI (as Stuart suggested back then). I don't know how Spring Security handles this, but one cannot have both: authorization before serialization and serialized params.

sberyozkin commented 3 weeks ago

Thanks @michalvavrik, in general, I think we are missing a way to bind policies to JAX-RS methods with annotations, so let's experiment

geoand commented 3 weeks ago

I agree. Also I happen to remember @geoand closed one issue that suggested to implement Jakarta Expression language for security annotations

Right, we have generally been against ELs in Quarkus