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

Provide a way to specify HttpAuthenticationMechanism per JAX-RS resource #34664

Closed
Eng-Fouad opened this issue Jul 11, 2023 · 15 comments · Fixed by #36504
Closed

Provide a way to specify HttpAuthenticationMechanism per JAX-RS resource #34664

Eng-Fouad opened this issue Jul 11, 2023 · 15 comments · Fixed by #36504
Assignees
Milestone

Comments

@Eng-Fouad
Copy link
Contributor

Eng-Fouad commented Jul 11, 2023

Description

Currently, HttpAuthenticationMechanism can be specified based on paths:

quarkus.http.auth.permission.bearer.paths=/abc
quarkus.http.auth.permission.bearer.policy=authenticated
quarkus.http.auth.permission.bearer.auth-mechanism=bearer

quarkus.http.auth.permission.custom.paths=/def
quarkus.http.auth.permission.custom.policy=authenticated
quarkus.http.auth.permission.custom.auth-mechanism=custom

In addition to that, one can specify it per JAX-RS resource. Something like:

@AuthMechanism("bearer")
@Authenticated
@GET
@Path("/abc")
public String abc() {
    return "abc";
}

@AuthMechanism("custom")
@Authenticated
@GET
@Path("/def")
public String def() {
    return "def";
}

Implementation ideas

No response

@Eng-Fouad Eng-Fouad added the kind/enhancement New feature or request label Jul 11, 2023
@Eng-Fouad
Copy link
Contributor Author

/cc @sberyozkin

@sberyozkin
Copy link
Member

Hi @Eng-Fouad We don't have such an option supported right now

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 11, 2023

/cc @pedroigor (bearer-token)

@geoand
Copy link
Contributor

geoand commented Jul 11, 2023

Is this going to be really useful in real life?

@FroMage @michalvavrik @sberyozkin WDYT?

@michalvavrik
Copy link
Member

Resource auth mechanism will still be mechanism for set of paths, so basically, it is about being able to use annotation instead of config properties? Problem is that security path matching is different than Path#value, e.g. we only support wild card and not path params.

Personally I'd use HTTP security policy or https://quarkus.io/guides/security-customization#httpauthenticationmechanism-customization (and have delegates based on path) as it provides me with same options.

@Eng-Fouad
Copy link
Contributor Author

Eng-Fouad commented Jul 11, 2023

I have an app that has 3 HttpAuthenticationMechanisms which authenticate many (50+) REST endpoints. Maintaining list of paths for each auth mechanism in configuration is a bit annoying. Wild card matching does not work for me all the time, as I don't have common context path for each auth mechanism. Not to mention that for some endpoints, they share same path but use different HTTP methods.

Also, I use path params and I find it difficult to configure auth mechanism based on paths:

/api/abc/{someParam}/foo
/api/abc/{someParam}/bar

@michalvavrik
Copy link
Member

Ok, that makes sense, thank you for sharing it. Possibly it is not a very common case, though?

I agree we should improve path matching and unless I'm mistaken, I have seen opened issue for that in past (don't have time to look it up right now).

As annotation portion of this issue, it would be nice if users who are in similar situation wrote here.

@michalvavrik
Copy link
Member

Path matching portion is related to #14047

@michalvavrik
Copy link
Member

michalvavrik commented Jul 20, 2023

Is this going to be really useful in real life?

@geoand I thought about that and ability to match auth mechanism with endpoint have value considering Quarkus Security patch matching abilities (see #14047) are not yet where they can be of interest. I can imagine someone want to mark endpoint. And here #34833, I have opened myself way to implement this :-)

We can even take it further! (Note: this feature can only work with disabled proactive auth):

  • we can implement annotation as suggested here, sure
  • we can also allow to provide HTTP security policy that you already can configure via config properties (see https://quarkus.io/guides/security-authorize-web-endpoints-reference) but instead of quarkus.http.auth.permission.permit1.path=/hello you can use quarkus.http.auth.permission.permit1.path-group=hello and than every endpoint / resource annotated with @PathGroup(hello") is going to have applied this security policy. This way, we already have auth mechanism capability because HTTP security polic allows you to select one [please, take this lightly, I just try to explain sort of thing we can do, it all depends on what are users needs]

@sberyozkin I think enhancing patch matching (#14047 ) or this (match endpoint -> HTTP security policy -> auth mechanism) makes sense. I'm not going to do anything in this direction till there is at least elemental agreement on what it should be as it might be a lot of work. WDYT?

Truth is that I don't know how usual case it is to have more than one auth mechanism.

@michalvavrik
Copy link
Member

Interestingly enough, just today I read issue that uses multiple auth mechanisms #34897.

@sberyozkin
Copy link
Member

sberyozkin commented Jul 20, 2023

While I appreciate some applications can be very complex, a requirement to support multiple authentication requirements for example in a single class or in the overlapping URI spaces etc might indicate some refactoring is warranted.

I'd propose to keep is as simple as we can to start with, support annotations only to start with.
I was actually thinking about, I wonder if we can create a meta annotation and then have @Basic, @Form, @MTLS, @Bearer, @CodeFlow, and @WebAuthn...

@sberyozkin
Copy link
Member

I think enhancing patch matching (#14047 ) or this (match endpoint -> HTTP security policy -> auth mechanism) makes sense. I'm not going to do anything in this direction till there is at least elemental agreement on what it should be as it might be a lot of work. WDYT?

It is interesting, sure, worth making it more advanced; I'd only propose to iterate in this regard, start with the annotations, then do something interesting with policies 👍

@michalvavrik
Copy link
Member

michalvavrik commented Jul 21, 2023

I was actually thinking about, I wonder if we can create a meta annotation and then have @basic, @Form, @Mtls, @Bearer, @codeflow, and @webauthn...

This can be done and also I like it! I presume @Eng-Fouad won't object? It's very close to your origin proposal.

It is interesting, sure, worth making it more advanced; I'd only propose to iterate in this regard, start with the annotations, then do something interesting with policies +1

I'm afraid under the hood it might be some internal policy anyway, because it is just the easiest way to do it. Also, having @Bearer/@CodeFlow/@Form combined with RBAC is de facto HTTP policies. But when I look into it, maybe it will turn out I was wrong. Thank you for your feedback

@michalvavrik michalvavrik self-assigned this Jul 21, 2023
@FroMage
Copy link
Member

FroMage commented Aug 14, 2023

It seems useful, I've had the case where I wanted basic auth for one endpoint, and OIDC for another. It would have been nicer to do with annotations on endpoints than with config path matching.

However, correct me if I'm wrong, but I think CodeAuthenticationMechanism happens before RESTEasy Reactive is called, no? So it has its own path-matching, which is different from the path-matching and endpoint-matching that RR does, no?

I don't see how CodeAuthenticationMechanism can use endpoint-defined annotations before RR has the time to find the target endpoint.

I mean, I think Authentication (that's where we select which auth mechanism is in place) happens before RR, which again happens before we do Authorization (checking @RolesAllowed, @Authenticated and stuff), no?

@michalvavrik
Copy link
Member

I don't see how CodeAuthenticationMechanism can use endpoint-defined annotations before RR has the time to find the target endpoint.

I think it works same for all auth mechanisms - with proactive auth it can't ever work as this handler must run before RR, with lazy auth for RBAC annotations it can work for now we have way to run code for annotated endpoint before authorization is done. As for configuration based path matching HTTP policy, we would run io.quarkus.vertx.http.runtime.security.AbstractHttpAuthorizer#checkPermission the first thing after RESTEasy Reactive begun processing instead of having it as handler. Because this is annotation-based approach, we can do this postponement when this feature is actually required.

I mean, I think Authentication (that's where we select which auth mechanism is in place) happens before RR, which again happens before we do Authorization (checking @RolesAllowed, @Authenticated and stuff), no?

With proactive auth yes, but with lazy auth I think it happens when QuarkusHttpUser.DEFERRED_IDENTITY_KEY is requested (e.g. via CurrentIdentityAssociation). Having this feature for proactive auth would require such refactoring that is not worth it IMO.

Theoretically there could be a risk that if some extension runs before RESTEasy Reactive starts processing as Vert.x route handler with higher priority and requests identity, the wrong one would be resolved. But it is not unlike OIDC multitenancy annotation based approach @Tenant https://quarkus.io/version/main/guides/security-openid-connect-multitenancy#annotations-tenant-resolver when this could happen as well. This code should run first when RR begin processing and I think in almost all cases there is no handler run before it.

Anyway, I can only get to this by the end of next month, so only then I'll know whether above-mentioned approach is viable. If you have some suggestion, please bring it on.

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

Successfully merging a pull request may close this issue.

5 participants