quarkusio / quarkus

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

OIDC Dynamic TenantConfigResolver issue in dev mode #42713

Open bpasson opened 3 weeks ago

bpasson commented 3 weeks ago

Describe the bug

As requested by @sberyozkin, a bug report for a few issues which are going wrong when using OIDC Config Resolver, also known as dynamic clients.

Dev Services won't start when default tenant disabled

The dev-services won't start when the default tenant is disabled. But they should start as normal as it could be the case you want only dynamic configurations and no default configuration. A realm import for dev services could supply all needed dynamic clients.

I worked around this by disabling the default tenant on boot in this reproducer, see the DummyTenantIdHeaderFilter

Resolved Tenant ID not backed by actual OIDC client

The OidcUtils.TENANT_ID_ATTRIBUTE offered through the RoutingContext in the public Uni<OidcTenantConfig> resolve(RoutingContext context, OidcRequestContext<OidcTenantConfig> requestContext) method of the TenantConfigResolver is used by Quarkus to notify us of an earlier resolved tenant-id. It fails however to verify if the OidcTenantConfig for that specific tenant-id is still present. Use the following steps to reproduce the issue:

  1. Start the application in dev-mode
  2. Clear all cookies for http://localhost:8080
  3. Open http://localhost:8080
  4. Log in with bob/bob
  5. Restart the application in dev-mode
  6. Reload http://localhost:8080
  7. You now get a status 401, where you would expect to get a login screen.

The only way to fix it is to fully restart Quarkus in dev-mode.

Hot Code Reload in Dev Mode breaks dynamic configuration

If you alter code e.g. change the string in GreetingResource and reload the page dev-mode will perform a hot code reload and you end up with a ID token verification has failed: Client is closed log message and a status 401. Use the following steps to reproduce the issue:

  1. Start, (restart if still running) the application in dev-mode.
  2. Clear all cookies for http://localhost:8080
  3. Go to http://localhost:8080
  4. Login with bob/bob
  5. Alter the test in GreetingResource
  6. Reload http://localhost:8080
  7. You now get a status 401 and the ID token verification has failed: Client is closed message in the logs.

The only way to fix it is to fully restart Quarkus in dev-mode.

Expected behavior

The expected behavior in this case should be the reproducer steps should not give failures.

Actual behavior

See the reproducer project here https://github.com/bpasson/quarkus-issue-42713

How to Reproduce?

See the reproducer project here https://github.com/bpasson/quarkus-issue-42713

Output of uname -a or ver

Darwin Kernel Version 23.6.0: Mon Jul 29 21:14:30 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T6000 arm64

Output of java -version

openjdk version "21.0.4" 2024-07-16 LTS

Quarkus version or git rev

3.13.3

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

Apache Maven 3.9.8 (36645f6c9b5079805ea5009217e36f2cffd34256)

Additional information

No response

quarkus-bot[bot] commented 3 weeks ago

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

sberyozkin commented 3 weeks ago

@bpasson I've renamed it to avoid the confusion as there is another effort to get OIDC dynamic client registration working.

sberyozkin commented 1 week ago

@bpasson I've had a look at your reproducer, you have a custom TenantConfigResolver creating a custom tenant config using the default tenant properties which you expect to be populated by Keycloak Dev Service, as indeed you mentioning above.

Can you please rework it and get it closer to how you actually plan to use it, do not even declare the default tenant, register TenantConfigResolver only.

FYI, newly added quarkus-oidc-client-registration integration tests run exactly in the same mode, with the custom TenantConfigResolver only and the default tenant automatically disabled. Though I added a property as part of that work, quarkus.keycloak.devservices.start-with-disabled-tenant=true, but I'm considering to have it set to false by default.

So, let's update TenantConfigResolver first, please update and let me know, right now it is a workaround which may have side-effects related to 2 and 3.

bpasson commented 1 week ago

@sberyozkin in the actual setup we use now in live, the disabled default tenant acts as a template configuration for all client configurations we resolve using the TenantConfigResolver. All clients are already configured in Keycloak the TenantConfigResolver is just used to avoid having to maintain a static list of client configurations. In this way we can have the application running and dynamically add a client in Keycloak and have it work without having to update anything on the application side.

The newly created quarkus-oidc-client-registration might be usable for the configuration templating part, but as far as I can see it tries to actually register the client with the OIDC provider, which is something we explicitly do not want to happen.

How would that work with what you suggest?