Add new AuthorizationPolicy annotation to bind named HttpSecurityPolicy to a Jakarta REST endpoints

[michalvavrik]

New annotation @AuthorizationPolicy allows to select HttpSecurityPolicy on endpoints via annotations as alternative to path-matching rules. This allows for general authorization check without augmentation of a SecurityIdentity.

• Closes: Introduce AuthorizationPolicy annotation #42710

[github-actions]

🙈 The PR is closed and the preview is expired.

[michalvavrik]

While CI looks ugly, I can see in the Develocity this io.quarkus.grpc.cli.GrpcCliTest.testCommand test is also failing elsewhere. Additionally, I cannot reproduce it locally. @sberyozkin maybe you can simply review and when I address your change requests, we will see if it gets better.

[sberyozkin]

Hi @michalvavrik I think this is exactly what we'd like to do at the moment, have a super easy way to bind custom policies to specific JAX-RS classes or methods, and we can evolve @AuthorizationPolicy with more options.
I can't even see what to ask about in this PR, it all looks really precise.

A question that I have so far is this one.

Let's say we have a named HttpSecurityPolicy bean declared as a CDI bean, not even referenced in application.properties, is this policy run at all if it has a non null name() ?
If not then all is clear, but if yes,
and the user also binds it to a method with @AuthorizationPolicy then that would mean the policy will be run twice...

Please clarify it.

[michalvavrik]

Let's say we have a named HttpSecurityPolicy bean declared as a CDI bean, not even referenced in application.properties, is this policy run at all if it has a non null name() ?

Yes. We don't care whether it is named bean or not, as long as there is CDI HTTPSecurityPolicy bean without name it is applied on every request. You can find information about that right above https://quarkus.io/guides/security-authorize-web-endpoints-reference#matching-on-paths-and-methods (scroll up with one stroke).

If not then all is clear, but if yes,
and the user also binds it to a method with @AuthorizationPolicy then that would mean the policy will be run twice...

In @AuthorizationPolicy you only can specify beans with HttpSecurityPolicy#getName, so it cannot run twice. I think I understand what is your question and let me explain it:

• whether CDI bean is named or not, we don't care
• we use this https://github.com/quarkusio/quarkus/blob/main/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityPolicy.java#L30 as we document here https://quarkus.io/guides/security-authorize-web-endpoints-reference#custom-http-security-policy and there are 2 reasons:
  • users already know this concept from quarkus.http.auth.permission.custom1.policy=custom
  • I remember @mkouba discouraged us from using named beans; though I can be misinterpreting him. Anyway IMHO it's not relevant as using HttpSecurityPolicy#getName is something we agreed for configuration properties and I see no difference here.

[sberyozkin]

Hi Michal, thanks, but I'm still not getting something. I'm totally fine with not using @Named, my question is different.

So let's say, even without this PR, I have

@ApplicationScoped
class MyPolicy implements HttpSecurityPolicy {
    String name() {return "mypolicy";}
} 

You say we don't really mind if it returns name or not, so does it mean this policy will be called on every request ?

See what I mean when I asked what happens if we refer to this policy from @AuthorizationPolicy(name="mypolicy") ? If the named policy, being a CDI bean, is run anyway, then we have a duplicate call with the annotation.

[michalvavrik]

You say we don't really mind if it returns name or not, so does it mean this policy will be called on every request ?

Let me try to put it differently:

• on current main your MyPolicy is only called for path /hi:

quarkus.http.auth.permission.permit1.paths=/hi
quarkus.http.auth.permission.permit1.policy=custom

while

@ApplicationScoped
class MyPolicy implements HttpSecurityPolicy {
    String name() {return null;}
} 

is always called. I believe it is rather well documented both in Javadoc https://github.com/quarkusio/quarkus/blob/main/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityPolicy.java#L26 and https://github.com/quarkusio/quarkus/blob/main/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityPolicy.java#L14 and in Quarkus documentation https://quarkus.io/guides/security-authorize-web-endpoints-reference#custom-http-security-policy (end of that section)

Which reminds me I should add note about @AuthorizationPolcy to HTTPSecurityPolicy javadoc. Thanks!

• with this PR additionally, you can replace

quarkus.http.auth.permission.permit1.paths=/hi
quarkus.http.auth.permission.permit1.policy=mypolicy

with @AuthorizationPolicy(name="mypolicy").

See what I mean when I asked what happens if we refer to this policy from @AuthorizationPolicy(name="mypolicy") ? If the named policy, being a CDI bean, is run anyway, then we have a duplicate call with the annotation.

I am really sorry for being slow, but I am yet to understand. There is no duplicate call because your mypolicy is only applied when selected by annotation or configuration properties. Is this maybe opportunity for documentation improvements you can suggest?

[michalvavrik]

heck, I've already run formatting twice. I'll try again.

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit fcb8abb.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit fcb8abb.

Failing Jobs

Status	Name	Step	Failures	Logs	Raw logs	Build scan
✖	Native Tests - Virtual Thread - Messaging	Build	⚠️ Check →	Logs	Raw logs	🔍

You can consult the Develocity build scans.

[michalvavrik]

The Kafka virtual thread failure doesn't seem related. @sberyozkin CI looks good now, please have a look.

[sberyozkin]

Thanks @michalvavrik