vogievetsky opened a new pull request, #20645: URL: https://github.com/apache/druid/pull/20645
### Description Since the pac4j 5 upgrade (#18259), web console sign-in through `druid-pac4j` fails on every login with `TechnicalException: privateKeyJwtConfig cannot be null` for OIDC providers that list `private_key_jwt` first in their discovery document (e.g. Keycloak). #19020 added `clientAuthenticationMethod` as a workaround, but the documented values `client_secret_jwt` and `private_key_jwt` can never work in Druid, `none` couldn't work either, and all of these only fail at login time, as a Jetty 500 page with a `WARN`. This PR fixes the default, rejects unusable values at startup, makes `none` usable, logs sign-in failures at `ERROR`, and rewrites the extension docs. Full investigation (setup with Keycloak in Docker, scripts, evidence, and further ToDos for the console and extension): https://github.com/vogievetsky/druid-oidc-investigation #### Auto-detection only picks methods Druid can configure When `clientAuthenticationMethod` is unset, pac4j takes the first method from the provider's `token_endpoint_auth_methods_supported` that it supports. pac4j 4.5.7 supported `{post, basic, none}`; 5.x added `private_key_jwt`, which needs a private key Druid has no properties for. ```diff createPac4jConfig if clientSecret is set setSecret(...) if clientAuthenticationMethod is set setClientAuthenticationMethod(...) + else + setSupportedClientAuthenticationMethods({client_secret_basic, client_secret_post}) + // no secret (method = none): pac4j sends the token request as a public client ``` | Provider | `token_endpoint_auth_methods_supported` (in order) | auto picks before | auto picks after | |---|---|---|---| | Keycloak 26 | `private_key_jwt, client_secret_basic, ...` | `private_key_jwt` ❌ | `client_secret_basic` ✅ | | Google | `client_secret_post, client_secret_basic` | `client_secret_post` | `client_secret_post` | | Microsoft Entra ID | `client_secret_post, private_key_jwt, ...` | `client_secret_post` | `client_secret_post` | #### `OIDCConfig` validation at startup ```text clientAuthenticationMethod not in {unset, client_secret_basic, client_secret_post, none} -> fail clientAuthenticationMethod = none and clientSecret set -> fail clientAuthenticationMethod != none and clientSecret missing -> fail (as before) ``` `client_secret_jwt` isn't implemented by pac4j, and `private_key_jwt` needs key configuration Druid doesn't expose. With `none`, pac4j 5.8 only works when there is no secret: with a secret it either rejects `none` against the provider metadata or throws "not supported". So no configuration that works today stops working. #### `Pac4jFilter` logs sign-in failures at `ERROR` Any exception from pac4j before the request is handed to the rest of the filter chain is logged as `ERROR ... OIDC authentication failed for request[...]` and answered with a plain 500 ("See the server log for details"), instead of a Jetty `WARN` and an error page containing the exception. Exceptions from the rest of the chain are rethrown unchanged. This also covers pac4j's `NullPointerException`s in the few seconds after a failed client initialization (pac4j delays re-initialization). #### Docs Rewrote `druid-pac4j.md`: - Replaced "should only be used at the router node" with an explanation of how the authenticator works with the Router and console, and why it belongs on every service, after an authenticator for internal traffic. - Added: provider registration (the callback URI `/druid-ext/druid-pac4j/callback` was not documented), a working example, `authorizerName`, and a section on sessions (lifetime, no sign-out, HTTPS). - Fixed: the authenticator snippet set `authenticatorChain` twice. `oidcClaim` only applies to `jwt`; `pac4j` always uses `sub`. Also added a pointer from `web-console.md`. #### Release note Fixed web console sign-in with the `druid-pac4j` extension for OIDC providers that advertise `private_key_jwt` first (such as Keycloak) when `druid.auth.pac4j.oidc.clientAuthenticationMethod` is not set. `clientAuthenticationMethod` now only accepts `client_secret_basic`, `client_secret_post`, and `none`, and Druid fails to start with any other value. With `none`, `druid.auth.pac4j.oidc.clientSecret` must not be set, which allows public OIDC clients. <hr> ##### Key changed/added classes in this PR * `OIDCConfig` * `Pac4jAuthenticator` * `Pac4jFilter` * `JwtAuthenticator` <hr> ##### Evidence Each value tested against Keycloak 26.0.7 with a separate Router, using a scripted browser login followed by a proxied SQL query ([script](https://github.com/vogievetsky/druid-oidc-investigation/blob/main/scripts/client_auth_methods.sh)): | `clientAuthenticationMethod` | Before | After | |---|---|---| | *(unset)* | ❌ 500 at login: `privateKeyJwtConfig cannot be null` | ✅ login + query | | `client_secret_basic` | ✅ | ✅ | | `client_secret_post` | ✅ | ✅ | | `client_secret_jwt` | ❌ 500 at login | Router fails to start with a clear message | | `private_key_jwt` | ❌ 500 at login | Router fails to start with a clear message | | `none`, public client, no secret | ❌ secret required | ✅ login + query | | `none`, with secret | ❌ 500 at login | Router fails to start with a clear message | Startup message: ``` druid.auth.pac4j.oidc.clientAuthenticationMethod[private_key_jwt] is not supported. Supported values are [client_secret_basic, client_secret_post, none], or leave it unset to pick one from the provider's discovery document. ``` Sign-in with an unreachable discovery document (3 requests in a row; `basic` auth on the same Router still works): ``` ERROR org.apache.druid.security.pac4j.Pac4jFilter - OIDC authentication failed for request[/unified-console.html] org.pac4j.core.exception.TechnicalException: java.io.FileNotFoundException: http://localhost:18080/realms/does-not-exist/.well-known/openid-configuration ERROR org.apache.druid.security.pac4j.Pac4jFilter - OIDC authentication failed for request[/unified-console.html] java.lang.NullPointerException: Cannot invoke "org.pac4j.core.redirect.RedirectionActionBuilder.getRedirectionAction(...)" because "this.redirectionActionBuilder" is null ``` ##### Merge danger **Two-way door.** No persisted state or API changes. **Blast radius:** clusters using `druid-pac4j` (or `jwt`, which shares `OIDCConfig`) with a `clientAuthenticationMethod` other than the three supported values, or `none` with a secret, now fail at startup instead of failing every login. <hr> This PR has: - [x] been self-reviewed. - [x] added documentation for new or modified features or behaviors. - [x] a release note entry in the PR description. - [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. - [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met. - [x] been tested in a test Druid cluster. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
