Allow restricting OidcRequestFilters to specific OIDC endpoints

[sberyozkin]

Fixes #37256

[pedroigor]

A few comments ...

[sberyozkin]

@pedroigor I'll need to update the docs

[sberyozkin]

@pedroigor Let me drop scope property for now as it addresses a rare case which has not yet occurred, and we can review it when users ask for it

[sberyozkin]

Let me also investigate the option of injecting OidcConfigurationMetadata

[pedroigor]

@sberyozkin The point about injecting metadata is that we still allow users to perform decision checks based on the client request URL and the URL from the metadata. For instance, to filter only requests to the JWKS endpoint, we would have something like that in a filter implementation:

if (metadata.getJwksUrl().equals(request.url()) { // do filter request }

The main reason for this is to avoid additional complexity for Oidc Client and reduce maintenance burden when supporting additional endpoints.

[sberyozkin]

Hi @pedroigor, I've done some updates to this PR, and @michalvavrik also helped out analyzing the metadata injection options. So here is is the summary:

• I've got rid of the Scope enum for now as agreed, lets see if users will ever need it
• I've added a support for the filters to access OidcConfigurationMetadata - Michal and myself looked into it and injecting it via the request scope is messy and tricky, another option was to pass via the Vertx duplicated context, but eventually the simplest solution was found where we can simply pass it as one of the properties in OidcRequestContextProperties - which is available as a filter method parameter. Unfortunately, Vert.x HttpRequest.uri() returns relative path only while the discovered metadata contains absolute URI, so users would have to write the code like this one: https://github.com/quarkusio/quarkus/pull/37257/files#diff-8f3225844cc7a339b14050ce3a5d9e172a0c0d7628ac0f01236c357020c7d9c4R23 but I have to admit I'm not too excited about it, as one need to check the JWK/etc endpoints for null (will be null for OAuth2 servers, or when the discovery is disabled), but it is definitely a viable option in simple setups
• Retained an Endpoint enum option: https://github.com/quarkusio/quarkus/pull/37257/files#diff-3425fd162ebeddd5d267c2b1032832f28419cb5605b5bf74ccd0bfba34773820, for me it looks like the easiest option - it is not difficult to maintain - I can't imagine this enum changing, if it at all, during the next 6 or 12 months :-). Annotation based approach would definitely be trickier to implement - this will involve the addition of 7 or so new annotations to the API among other things which I'd probably like to avoid.

IMHO it is a reasonable compromise - if users want to filter all the endpoints or say only 2 of them, then OidcConfigurationMetadata checks can be handy, for single endpoints, Endpoint enum works very well IMHO.

How does it sound ?

[sberyozkin]

Dropped a noteworthy feature label as it is more like a necessary improvement

[sberyozkin]

/cc @pedroigor @calvernaz

Hey Pedro, I hope you'll be glad to know I've decided to follow your suggestion and I've dropped OidcRequestFilter#endpoint() which this PR started from, and users can restrict with annotations, for example: https://github.com/quarkusio/quarkus/pull/37257/files#diff-3425fd162ebeddd5d267c2b1032832f28419cb5605b5bf74ccd0bfba34773820R15

So, now users can either restrict to specific endpoints using annotations or use OidcConfigurationMetadata to filter requests to all the endpoints.

[sberyozkin]

Unfortunately I forgot again to update the docs, done now

[quarkus-bot]

✔️ 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.

[github-actions]

🙈 The PR is closed and the preview is expired.