Simplify tenant resolution for the OIDC/Hibernate Multitenancy combination

[sberyozkin]

Description
Tenant id has to be resolved to support both OIDC and Hibernate multi-tenancy flows.
In both cases a tenant id can be extracted from RoutingContext, for example, see OIDC and Hibernate resolver examples.

Effectively a user needs to write nearly the same code twice to get a tenant id out of RoutingContext.

Implementation ideas
OIDC security always runs first, hence a simple proposal is:

1. OIDC sets a RoutingContext tenant-id attribute
2. Hibernate layer checks its io.quarkus.hibernate.orm.runtime.tenant.TenantResolver. If it is not available then it picks up this current tenant-id RoutingContext attribute if an OIDC-neutral property like quarkus.hibernate.use-context-tenant-id=true

The end result is that Hibernate users will benefit, they won't have to write their own resolver id when the OIDC and Hibernate tenant resolution logic is the same :-)

See also the related discussion at #11861.

Hi @gsmet @Sanne @FroMage @gavinking - if you are also OK with it then I'll open a PR making an OIDC related update (save the OIDC tenant id as a RoutingContext attribute). And a related Hibernate (etc) enhancement can follow ?

[quarkusbot]

/cc @gsmet, @Sanne

[michael-schnell]

We should avoid creating a direct dependency to the vertx extension by using the io.vertx.ext.web.RoutingContext for the multitenancy feature. That's why we avoided this for the io.quarkus.hibernate.orm.runtime.tenant.TenantResolver. In contrast to the ODIC resolver, it has not a direct dependency to vertx.

[sberyozkin]

@michael-schnell Right, in the OIDC code we do need RoutingContext but I see that it is not needed at the data level...
Hmm... I think what I can do is to create this small OIDC PR and do a small update to the Hibernate Multitenancy docs pointing out that a tenant-id RoutingContext attribute will be available, so the custom Hibernate resolvers which also need to parse something from RoutingContext can just pick up that attribute if needed...
thanks

[gavinking]

This shouldn't be something specific to either OIDC or Hibernate. It should be a neutral aspect of the Quarkus environment, by which I mean that there should be an agreed place where tenant ids are made available by some extensions, and consumed by others. It shouldn't be a matter of OIDC and Hibernate agreeing on some sort of protocol, but rather the Quarkus environment providing a standard place where the tenant id set.

[sberyozkin]

@gavinking I just switched off my laptop and then on again to close this issue :-), because I realized a custom user OIDC resolver can pass it as a routing context attribute over a custom user Hibernate resolver, so it is entirely up to the users if they would want it or not and Quarkus does not need to do anything itself (at least on the OIDC + Hibernate multi-tenancy path).
I can only propose a small update to the Hibernate Multitenancy docs to suggest this user-driven 'optimization'.
A more generic solution as you suggest can be interesting indeed.

[michael-schnell]

I think it is still worth thinking about harmonizing the TenantResolver for ODIC and DataSource (currently Hibernate ORM). This would allow other extensions to use the same information. But the TenantResolver interface itself should be independent of other extensions. I guess it shoudl be possible to adjust the current ODIC TenantResolver in a way that is backward compatible, but still uses the new interface.

[sberyozkin]

Hi @michael-schnell I'm not sure there is much scope in it, we'd need to introduce some tenant-common extension, etc, but if the users can easily pass the tenant id via a RoutingContext medium then there is no much code saving by supporting a single interface.
The mechanics of the tenant resolution are also somewhat different, in OIDC the dynamic tenant configurations are possible.

But what would concern me more with respect to focusing on a single interface is that the tenant concept may have a different granularity in OIDC and Data cases.
For example, we may have the same OIDC tenant (login via keycloak vs google, etc), while the data tenant id would be a path segment bound, /users/{id}, etc... I see that in a number of cases the tenants may be the same, different sets of data are returned for different Keycloak realm users etc but I don't think we can assume it will always be the case

[michael-schnell]

I guess the 90% use case is exactly what you described:

1. Extract the realm (tenant) from the URL path or request header
2. Authenticate/authorize with ODIC (keycloak) using the realm
3. Use the realm as tenant id for accessing protected tenant resources like the database

You must ensure that any user is authorized for a realm/tenant to access the database and possibly other resources.

Another scenario is for example a batch process that executes some kind of updates for all tenants:

1. Read a list of all tenants from the database (including the tenant id)
2. Iterate over the list entries and use the tenant id for accessing protected tenant resources like the database

For that use case you will not have a RoutingContext and can implicitly assume that the batch process is authorized to access all tenants. You will need to set the tenant in in the context directly, simply by using the tenant id from the list query.

So you have two parts:

1. Determine the tenant id
2. Provide the tenant id in the context to be used by any extension (like DataSource)

[sberyozkin]

@michael-schnell great, thanks for this overview. I can keep this open if you'd like but I do feel that just doing

OIDC resolver: requestContext.put("tenant-id", tenantId);
Hibernate resolver: return injectedRequestContext.get("tenant-id");

is enough for such cases when both OIDC and Hibernate resolvers need to get the id from the HTTP request.
So yes, we can keep this issue open in order to get more feedback though I'm trying to close as many OIDC related issues as possible to keep controlling the open issues :-).