Changing OidcTenantConfig returned from TenantConfigResolver::resolve has no effect

[sberyozkin]

Discussed in #48199

^{Originally posted by maag03 June 3, 2025}
We're using Quarkus 3.20.0 and a multi-tenant dynamic Oidc setup. As the title says, when returning OidcTenantConfig in TenantConfigResolver::resolve for a tenant with a changed property (e.g. clientId), the changed property seems to be ignored and the initial value is still being used by Quarkus.

Is this the intented behavior or is it a bug?
Any workaround available?

Br
Magnus

[quarkus-bot]

/cc @pedroigor (oidc), @radcortez (config)

[sberyozkin]

The proposed solution is to swap the already resolved instance if the resolver returns a new instance.
@michalvavrik FYI, I'll ask you to review.

[michalvavrik]

The proposed solution is to swap the already resolved instance if the resolver returns a new instance.

quarkus/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/TenantConfigResolver.java

Line 24 in 55d17c1

Uni<OidcTenantConfig> resolve(RoutingContext routingContext, OidcRequestContext<OidcTenantConfig> requestContext);

signature is Uni<OidcTenantConfig> resolve(RoutingContext routingContext, OidcRequestContext<OidcTenantConfig> requestContext); so method parameters are RoutingContext and OidcRequestContext<OidcTenantConfig> (the request is in the name). And javadoc says Returns a {@link OidcTenantConfig} given a {@code RoutingContext}.. It feels like whatever the resolver returns should be what is used as a primary source of information. So swapping sounds better than keeping the old one.

The config is stored in a concurrent hash map and if users keep creating new instance for every call, we have to make equals between instances on the every call, we can only update the concurrent hash map if absolutely necessary (avoid syncing). We shouldn't do it just because the resolver did not return null.

[sberyozkin]

@michalvavrik, thanks for the feedback,

The config is stored in a concurrent hash map and if users keep creating new instance for every call, we have to make equals between instances on the every call, we can only update the concurrent hash map if absolutely necessary (avoid syncing). We shouldn't do it just because the resolver did not return null.

To implement equals, we'd have to do a real lot of work for 150 or so properties (equals, hashCode) which will also penalize 80% applications who never update the resolved OIDC tenant configuration.

This use case is only relevant for the advanced cases, where, after the resolution has happened, something needs to be tuned to sync with the relevant OIDC provider updates, like client secret updates or something else.

So what I'm starting with, is, if the instances are not the same, just swap them and reconnect. I'm also doc-ing that modifying existing instances in the resolvers is not currently thread safe - it would cause too much complexity to handle it as well - which was the only reason I was hesitating before. And will advise to avoid creating a tenant instance on every call but copy construct a new one and modify as needed instead. I think it is very reasonable to expect most real world resolvers don't create instances on every call - users write resolvers to fetch data from DB, Kubernetes, they will cache resolved configurations.

Once this is done, I can continue with the equals work, otherwise it will take a long time.

Hope that sounds reasonable.

[michalvavrik]

Whatever works for you.

[sberyozkin]

@michalvavrik It is already proving very complex as it is - the problem is, if we have a session cookie, and the resolved config is swapped, the session remains without the user required to re-authenticate - if it is only a client secret update, fine, if it is something that requires a new user login, we have no clue it may be needed... I think of introducing an option to trigger a user re-authentication in such cases, t will be false by default.

What you did with encapsulating remove actually helps, because with explicit remove, we had no way of controlling such updates with some unclear security consequences...

[michalvavrik]

I think proposed solution the way you present it is fine. I think you must consider there most likely are users like me that didn't know this config shouldn't change except for selected properties. It wasn't doc-ed or at least not at places I'd mentioned. I think you need to update all-related doc like https://github.com/quarkusio/quarkus/blob/main/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/TenantConfigResolver.java javadoc and https://quarkus.io/guides/security-openid-connect-multitenancy#tenant-config-resolver. I still can't get my head around the fact there is RoutingContext but you don't expect it to be request-specific instance.

So what I'm starting with, is, if the instances are not the same, just swap them and reconnect.

Because this is a breaking change. Docs show creating new instance every time and I think that many users will be affected by this (performance if you reconnect on every request). Some users don't mind creating new instance every single time, especially as docs present it as a solution https://quarkus.io/guides/security-openid-connect-multitenancy#tenant-config-resolver.

Yes, once all this is done, there is little point of immutable OIDC tenant config instances, I think they will come OOTB once we drop public constructor on OidcTenantConfig class (given we reduce fields).

[sberyozkin]

Hi Michal, @michalvavrik, after reading your comments, I'm coming to the conclusion that the only entity which knows what should happen when the resolved OidcTenantConfig instance is not the same as the one which was resolved before, is the resolver itself.

There are 3 cases:

1. User returns the same instance, or as you rightly point out, a new instance to represent the same configuration, they may not want to cache it if creating it on every call is cheap
2. User returns a new instance which requires a reconnect
3. User returns a new instance which requires a user re-authentication in case of the already existing session

So my plan now is, when an instance is not the same, just update the configuration context only - that preserves the backward compatibility, this is probably similar to what you were trying to do as well.

Next, if the resolver intention is to reconnect - the resolver should set a routing context property, for example, quarkus.oidc.replace-tenant-configuration-context to true.

That may look like a breaking change too for the user in the linked discussion, since, when remove was available, doing tenantConfigBean.remove and then returning a new instance was causing a re-connect. But, first of all, it may not have always been required to re-connect, for example, a client secret update does not require a reconnect, and second, TenantConfigBean#getDynamicTenantContext()#remove in the runtime package was never doc-ed and can't really be considered part of the API. We can nearly consider it a bug fix, encapsulating remove anyway.
So I think, now, instead of users having to go through the internal API, manually remove the tenant to cause the reconnect, they just set a property to give Quarkus a hint.

Finally, if it is the existing OIDC web-app session, if the user must relogin, which is probably a very advanced case, the resolver must indicate it too, for example, with quarkus.oidc.remove-session-cookie to true - and Quarkus will trigger re-authentication, kind of custom step-up reauthentication for the code flow.

[sberyozkin]

Though may be I'd even leave the session refresh out of it completely, I'll think about it

[sberyozkin]

@michalvavrik No, it can't be avoided, we will also need to be able to optionally clear the session cookie and request re-authentication, I hit the problem today while working on the test, where, after the reconnect, the session cookie interfered with the reconnected content requirements.

The fact it worked for some users, after they explicitly removed the existing tenant context was random, assuming it was done at the current session time. So I believe the 3 point proposed plan above should stand.

The question is though how to help @maag03 to make it work in 3.20 LTS. What I'm going to do for this issue will unlikely be backported.

I'm thinking of creating a dedicated PR to 3.20 to temp reintroduce remove for the dynamic case, only, but already @Deprecated with a clear message it is not part of the OIDC API and that it will be removed; I'd like to avoid to re-introduce remove on main, even in the deprecated form, the management of the dynamic tenant must remain encapsulated...
What do you think ?

Unless you can think of some workaround for 3.20, how users can remove the dynamic tenant ?