quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.87k stars 2.71k forks source link

OIDC Documentation: Multitenancy TenantConfigResolver Setup #33122

Open lennartj opened 1 year ago

lennartj commented 1 year ago

Describe the bug

OIDC Multitenancy Client setup

When developing a multitenant-aware quarkus application with dynamic tenant resolution, developers need to implement a io.quarkus.oidc.TenantConfigResolver to pick the OIDC realm from the inbound call. It is rational/usable to supply the realmId as a segment within the service's resource path. Resolving the realmID in this way implies that the service can be fully multi-tenant with minimal adaptions to the codebase.

This is not documented on the quarkus OIDC site, so I will supply a small example to capture this pattern.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

Will supply a PR on the documentation to cover this.

quarkus-bot[bot] commented 1 year ago

/cc @pedroigor (oidc), @sberyozkin (oidc)

sberyozkin commented 1 year ago

Sorry, i thought for a sec I opened it, did not mean to be pedantic 🙂

sberyozkin commented 1 year ago

@lennartj I thought this issue was about a missing piece of the doc that a default tenant has to be explicitly disabled, when only non-default tenants are resolved dynamically - which is I why I asked you to open a bug but not an enhancement issue :-) and which is why I accompanied your issue with the task to make sure it would not be necessary, #33120.

I've reread this issue this morning, and I recall the code you showed but not quite precisely, so indeed, please try to find some time to have this pattern doc-ed - we should probably mark it as a NOTE clarifying that Keycloak users can find this pattern useful. I can then open another PR to resolve the bug aspect of this issue to mention the default tenant has to be disabled - or we can do it as part of the same PR.

Cheers

lennartj commented 1 year ago

Will do; working on it in a documentation branch of my fork. Sending a PR your way shortly

:)

lennartj commented 1 year ago

It seems that the main branch currently does not build the ./mvnw -DquicklyDocs, seemingly due to the relocation of the open telemetry jaeger deployment jar. Am I missing something here, @sberyozkin ?

Skärmavbild 2023-05-06 kl  12 27 03
sberyozkin commented 1 year ago

Hi @lennartj Thanks for looking into it, and do it right as far as the doc updates are concerned, I missed your pings.

Hmm, for now, you can just try mvn clean install -Dquickly and check the generated docs, but since it is a not going to be a major refactoring, if building it is problematic, you may as well just open a PR and a doc preview will be available, @gsmet helped to set it up for PRs. Thanks

Hi @gsmet, FYI, ./mvnw -DquicklyDocs does not work, have a look please.

sberyozkin commented 1 year ago

Sorry @gsmet works for me,

@lennartj Yeah, it works OK for me, are you on the main branch ?

lennartj commented 1 year ago

Yup. I am on the main branch, running on an M1 Mac. Will do the same run on another M1 Mac in an hour or so; getting back with the results to you by then.

sberyozkin commented 1 year ago

May be a local maven repo issue ?

lennartj commented 1 year ago

The other M1 box fixed the build, by running ./update-extension-dependencies.sh ; ./mvnw -DquicklyDocs on the main branch.

Will start fixing the PR.

😃

sberyozkin commented 1 year ago

Great stuff, thanks