Optimize OIDC tenant id resolution

[sberyozkin]

Fixes #39417

This requires some tests and actually setting the extra attribute during the annotation processing :-)

[sberyozkin]

@michalvavrik I've updated tests now to verify that the tenant config resolver is called only during the first request or when the session cookie resolves the tenant with the resolver choosing to let this tenant id stay (this is what I wanted to retain, to let the resolvers block the possibly bogus tenant ids on some paths), as well as added log messages which can help tracing how exactly the tenant id has been set.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 0a58003.

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

You can consult the Develocity build scans.

[sberyozkin]

Proposing a backport to 3.8 if possible since otherwise custom TenantConfigResolver is forced to keep all OidcTenantConfig in some cache (for ex, memory) if it was loaded from the db and other external sources, if it wants to avoid reloading them on every request

[michalvavrik]

looks very good

[sberyozkin]

Thanks @michalvavrik.
Hi @gastaldi Can you please have a look and approve if it also looks fine to you ? It should be ready to go since @michalvavrik approved.

The PR is primarily about avoiding calling custom OIDC TenantConfigResolver and TenantResolver tenant resolvers if the tenant id is already known to be set by the @Tenant annotation. It also ensures that if the tenant id has already been deduced from the OIDC session cookie, all the available OIDC resolvers, including TenantConfigResolver and TenantResolver but also the path based resolvers get a chance to overwrite that tenant id if needed. And also adds logs to make it easy to trace where the tenant id was set up.

[sberyozkin]

Thanks @gastaldi

[sberyozkin]

@gsmet Hi, I proposed backports, but I've just noticed a related quickstart failure, I'll have a look and re-add the backport suggestions once I figure out what is wrong with that test

[sberyozkin]

@michalvavrik @gastaldi I'm removing the release breaking change, because using a shared session cookie path for more than one OIDC web-app application makes no practical sense at all, I don't think anyone would allow it. The failing test in the quickstarts was passing due to some luck before because currently, the session cookie split into chunks leads to the wrong tenant calculation - in fact that multi-tenancy demo does not work even with 3.9 if you access it from the browser. This PR has exposed that problem.
I have the PR sorting out that coming soon.
I'll add a backport to 3.9 back but will keep 3.8 off the list for now

[michalvavrik]

@sberyozkin you changed the order of resolvers. Annotation-based should be behind tenantResolver according to the https://quarkus.io/version/main/guides/security-openid-connect-multitenancy#static-tenant-resolution. I remember this change was discussed, but effectively documented order is different now?

I am asking because I did some tweaks to this code and need to know if this order should be kept now. I think it's fine, we just should change docs. Your call.

[sberyozkin]

@michalvavrik Sure, somewhere, as part of discussing this issue, I suggested the annotation based resolution check should be made before asking the custom resolvers to do the work, I'll tweak the docs

[michalvavrik]

Thank you, I wasn't sure.

[sberyozkin]

As otherwise it would defeat the purpose of using those annotations :-)

[gsmet]

This doesn't apply cleanly so I will skip backporting.

[sberyozkin]

@gsmet OK, I'll have a look if it can work for 3.9.3