Allow to customize OIDC verification

[sberyozkin]

Fixes #32701

[sberyozkin]

Hi @pedroigor This PR supports a case of verifying Azure bearer JWT tokens such as those issued for the enterprise Azure Graph services - we have both external (see the linked issue) and internal reports about such tokens not verifiable by default, using the traditional verification techniques - specifically using public Azure tenant JWKs to verify the signatures.

The reason behind it is that Azure obfuscates the nonce token header after the signature was created, i.e. an SHA-256 digest is used before the signature is created, but the receivers of such a token will only see the actual nonce value.
So to verify such a token one needs to pre-process it and recreate the original headers sequence by replacing the nonce value with its SHA-256 digest.

This case is quite strange but Azure Graph bearer tokens can be quite widely used, and other providers might need to have similar tweaks applied.

PR introduces TokenCustomizer interface, ships an Azure customizer, and users will be able to preprocess headers if needed. I named the interface TokenCustomizer as it is not certain that over time it won't be necessary to have some payload tweaks applied as well which would require adding another method to the interface.

[sberyozkin]

Let me sort out the conflicts

[sberyozkin]

@pedroigor Let me add one more test to confirm that the random token headers update will cause the verification failure

[sberyozkin]

@pedroigor I added one more test by modifying DefaultTenantTokenCustomizer in integration-tests/oidc-wiremock to remove one of the headers and the test expecting 401 in this case.

Good point in general, do we really need a new interface given that the only real world use case so far is the Azure Graph V1 token verification. It feels right, as we can cleanly encapsulate it in a Azure specific implementation and Id not be surprised at all we'd have more such cases coming in, we can also let users support custom JWT header obfuscation techniques, so hopefully adding an interface will prove useful beyond this Azure special case :-).

We also discussed with Georgios how to link such providers with individual tenants, not even every Azure tenant will need it, so it can be via @Tenant annotation or in case of the reusable Azure customizer - via the property...

[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.

[pedroigor]

@sberyozkin I would avoid introducing an API if now we only have this use case in mind. It should be easier to just rely on the defaults and the extension being smart enough to identify if any customization should be done in order to complete the expected flows (e.g.: bearer authorization).

I mean, if the use case we are trying to solve is a corner case and not aligned with the specifications, we should deal with it as such.

Perhaps what we want here is to delegate to the application the token verification completely?

[sberyozkin]

Hi @pedroigor

I would avoid introducing an API if now we only have this use case in mind. It should be easier to just rely on the defaults and the extension being smart enough to identify if any customization should be done in order to complete the expected flows (e.g.: bearer authorization).

I honestly don't know how we'd do it without encapsulating inside an interface implementation - adding somewhere in OidcIdentityProvider some function which will do if this token is from Azure then lets customize the headers does not sound like a good idea to me, and it will not work in a multi-tenant setup, how would quarkus-oidc know a given Azure token was issued for example by Graph V1 service for a given tenant ? In general quarkus-oidc verification code is meant to be totally provider neutral, we have one or two Keycloak specific fallbacks but it is 98% neutral otherwise.

I'd not worry that it is not spec compliant, we already do quite a few things in quarkus-oidc which are not OIDC spec blessed :-) I kind of see your point, here is a single such case, why an interface ? Well, it is a 1st case, according to Paulo, other providers have their own peculiarities, quarkus-oidc is stressed a lot and this interface could bring new options...

Perhaps what we want here is to delegate to the application the token verification completely?

I'd say it is not an option :-), i.e, users can just skip quarkus-oidc if they want to, but otherwise quarkus-oidc should retain a control of the verification process if uses work with it. With this PR we try to let them only influence the verification outcome...

Thanks

[sberyozkin]

Hey @pedroigor

As far as TokenCustomizer is concerned, it is certainly true it addresses a single use case only, but while Azure token customization kind of takes a central stage in this PR, the broader goal is to plug in a gap in Quarkus OIDC capabilities. Specifically, we can let users customize the already verified security identity with SecurityIdentityAugmentor, but the feedback I've had - is that it is impossible to preprocess the tokens, something for example, Vert.x OIDC users can with Vert.x handlers...

In any case, one way or another, quarkus-oidc will need to be able to handle such tokens. If we don't do it with a feature like the one proposed here, then I'd need to get them verifiable by supporting an indirect JWT token verification by requesting UserInfo (something we do for GitHub binary access tokens for example), this is something we might need to do anyway for some providers as the chance some other provider does something similar to Azure is non-zero, but for now offering a generic mechanism for doing the local customization is not too bad an idea IMHO :-)

[sberyozkin]

I've been considering today, if this interface should also accept a blocking request context, but I honestly can't imagine a blocking call situation during the JsonObject headers customization :-)

[pedroigor]

@sberyozkin LGTM. I don't have a better alternative to solve this corner case.

[sberyozkin]

Hey @pedroigor Thanks for the review.

Right, no ideal solution exists. I like your idea of users being totally unaware of such a possible customization with Quarkus doing in dynamically, but I'm also concerned it would need to become strongly aware of such a requirement as a result with Azure specific code branch and in reality only a user will know 100% if a given Azure JWT nonce needs some preprocessing. This PR is not ideal either, true, we add a new interface, users may need to refer to the shipped Azure customizer with the property so it can become a bit technical which is balanced a bit by the fact that the Azure code is encapsulated.

So we two have quite similar alternatives, but what I feel just tips it over to this PR is that a generic token customization feature is introduced and along the way Georgios have come up with @Tenant idea to link features to tenants.

Thanks for your feedback, I'll keep it open till Friday.

@stuartwdouglas would you like to add something to the review ? Summary: as discussed earlier, this PR offers a way to preprocess tokens before they are verified with the Azure customizer shipped out of the box - it can be referred to as a named bean, while users are encouraged to use @Tenant to link custom customizers to tenants.

Thanks

[sberyozkin]

Merging now, as always, we can tune things going forward