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

Support JWT bearer client authentication for quarkus-oidc #40013

Closed
sberyozkin opened this issue Apr 11, 2024 · 12 comments · Fixed by #45131
Closed

Support JWT bearer client authentication for quarkus-oidc #40013

sberyozkin opened this issue Apr 11, 2024 · 12 comments · Fixed by #45131
Assignees
Milestone

Comments

@sberyozkin
Copy link
Member

Description

Sometimes, OIDC clients are expected to authenticate using an out of band, so called JWT bearer token, provided, for example, in Kubernetes or some shared secured file folder. Furthermore these tokens can be dynamic, so they can't configured in properties.
#38541 made it possible to easily pick up such tokens from custom OIDC client filters.

However, quarkus-oidc does not support this case yet.

Implementation ideas

May be OidcRequestFilter can be used to augment the request by setting a property on OidcRequestContextProperties, exactly as it can be done on RoutingContext.

Copy link

quarkus-bot bot commented Apr 11, 2024

/cc @geoand (kubernetes), @iocanel (kubernetes), @pedroigor (bearer-token,oidc)

@martaisty
Copy link

Perhaps OidcRequestFilter can do the job. However, I am afraid of this affecting Oidc clients as well, which might be unintended. If I understand it correctly OidcRequestFilter is a global one, it is triggered for quarkus-oidc requests as well as for quarkus-oidc-client requests, then it will add some complexity for filtering if that's right oidc server to modify a request

@martaisty
Copy link

For me as for the end-user, the ideal option would be setting configuration properties like:

  • quarkus.oidc.credentials.jwt.source=bearer and quarkus.oidc.credentials.jwt.file-path=<path-to-the-token-on-filesystem> for quarkus-oidc
  • quarkus.oidc-client.credentials.jwt.source=bearer and quarkus.oidc-client.credentials.jwt.file-path=<path-to-the-token-on-filesystem> for quarkus-oidc-client

And then have Quarkus read the token and set it as a client_assertion. As I see it most of the code for this is ready because it's already possible to use JWT generated by Quarkus for quarkus-oidc as well as for quarkus-oidc-client. What is different in this case is that the JWT token is pre-created and ready to use, we just need to read it from FS and set it.
What are your thoughts on this, @sberyozkin ?

@martaisty
Copy link

For me as for the end-user, the ideal option would be setting configuration properties like:

* `quarkus.oidc.credentials.jwt.source=bearer` and `quarkus.oidc.credentials.jwt.file-path=<path-to-the-token-on-filesystem>` for `quarkus-oidc`

* `quarkus.oidc-client.credentials.jwt.source=bearer` and `quarkus.oidc-client.credentials.jwt.file-path=<path-to-the-token-on-filesystem>` for `quarkus-oidc-client`

Then it would make it possible to use @OidcClientFilter from quarkus-rest-client-oidc-filter as is without a need to create a custom one. Another example of a good fit I can think of is the smooth usage of @AccessToken from quarkus-rest-client-oidc-token-propagation in case it is needed to exchange the token before propagating it downstream.

What I'm trying to say is that if such a feature is added to Quarkus it fits much better with other OIDC-related extensions than implementing custom request filters. However, I might not be aware of the whole picture, so I am looking forward to hearing from you

@sberyozkin
Copy link
Member Author

@martaisty Well, then it would require Quarkus scan that location and that location can be anywhere, in the file system, in Kubernetes, correctly pick up the token which Azure may be refreshing, it does look like it is out of scope.

As far as the same filter impacting both extensions is concerned, it can only cause side-effects if you have an endpoint protected by OIDC (the filter runs) and in scope of the current request, OIDC client also runs (and again the same filter runs), but it can only cause side-effects if both extensions want to use the filter for different purposes. We've thought with @pedroigor about it, and as far as I recall, the plan in this case is to add a Scope annotation, thus restricting a given filter to the client or server scope only.

@martaisty
Copy link

that location can be anywhere, in the file system, in Kubernetes
@sberyozkin That's why I suggest adding quarkus.oidc.credentials.jwt.file-path as a configuration property to specify the exact path

@sberyozkin
Copy link
Member Author

@martaisty #38458 also mentions:

The injected token is only valid for a short period of time (default 1h)

So as I said, that means Quarkus itself would need to run a timer, check that location, compare it with the original token. In general, we do try to help users as much as we can, but this kind of work does look like something that is out of scope.
The filter approach itself is very straightforward, and the application knows how to scan it.
I can nearly guarantee that if we go with file-path then the next release someone will say, I want the token injected into Vault extracted, or have this token picked up from the request parameter, which will require us introducing a customizer, while we already have filters.
That said, lets keep your idea in mind, for this issue I will not be looking at this option, but once it is resolved, I can open a follow up enhancement request to discuss it further

@martaisty
Copy link

I thought about a solution involving a cache, similar to this: read the token from FS and save it, whenever this token is needed check exp claim, if expired, read from FS again.

@sberyozkin that's a good point, I just thought k8s service account token volume projection is more or less common to worth being implemented (https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#serviceaccount-token-volume-projection)

I can nearly guarantee that if we go with file-path then the next release someone will say, I want the token injected into Vault extracted, or have this token picked up from the request parameter

Never mind, as long as filters can do the job, this file-path is more like a "syntax sugar" 😃

@sberyozkin
Copy link
Member Author

sberyozkin commented Apr 12, 2024

@martaisty This is a good idea, thanks for it, it is just that at the get go I'd like to avoid a specialized, possibly very provider specific solution and just make sure that concept of the filters supplying extra request parameters dynamically works.
But at the next phase, may be we can indeed do something more targeted, like I said, I'll open a follow up enhancement request and CC you

@Sopka
Copy link

Sopka commented Jun 21, 2024

I am testing JWT bearer client authentication with Quarkus version 3.11.2, following the instructions detailed at Quarkus Security OpenID Connect Client Reference.

To implement this, I created a custom filter that overrides the additionalParameters method as per the documentation, to provide the JWT bearer token. In my setup, the bearer token is provided by Azure Workload Identity and is located inside a Kubernetes Pod at /var/run/secrets/azure/tokens/azure-identity-token.

The issue I am encountering is that the setup only works with the initially exchanged access token. Once this token expires, it appears that the additionalParameters method is not invoked before attempting to exchange a new token. Invoking this method is crucial because the client_assertion JWT token is not static; it has its own expiration date.

@sberyozkin
Copy link
Member Author

sberyozkin commented Jul 9, 2024

@Sopka Apologies for a delay, missed this comment.

Once this token expires, it appears that the additionalParameters method is not invoked before attempting to exchange a new token

I don't see why it can be happening. This addiitonalParameters which you implemented is invoked every time the custom OidcClient filter is invoked, here is a sequence:

  1. super.getTokens() in: https://github.com/quarkusio/quarkus/blob/3.11.2/extensions/oidc-client-reactive-filter/runtime/src/main/java/io/quarkus/oidc/client/reactive/filter/runtime/AbstractOidcClientRequestReactiveFilter.java#L31
    super.getTokens() calls tokensHelper.getTokens(oidcClient, additionalParameters(), forceNewTokens);

  2. If the token is refreshed, these additional parameters are included: https://github.com/quarkusio/quarkus/blob/main/extensions/oidc-client/runtime/src/main/java/io/quarkus/oidc/client/runtime/TokensHelper.java#L73

  3. Finally in OidcClientImpl, the client assertion type is set here: https://github.com/quarkusio/quarkus/blob/main/extensions/oidc-client/runtime/src/main/java/io/quarkus/oidc/client/runtime/OidcClientImpl.java#L159 and the assertion itself is here: https://github.com/quarkusio/quarkus/blob/main/extensions/oidc-client/runtime/src/main/java/io/quarkus/oidc/client/runtime/OidcClientImpl.java#L190

But what is important is that what drives it all is the current request flowing through the filter.

Would you like to try to debug at those points ? Do debug log messages for io.quarkus.oidc.client.runtime reveal anything ?

I've also added more log messages at #41638

@sberyozkin
Copy link
Member Author

sberyozkin commented Jul 24, 2024

@martaisty FYI, I've added this issue to the proposed WG list of tasks at #42116. Also, I'm leaning toward supporting it exactly as you proposed. Unfortunately it will take a bit more time but this issue is tracked, thanks

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