dervoeti opened a new pull request, #19020:
URL: https://github.com/apache/druid/pull/19020
Fixes #19019.
### Description
This PR adds an optional `clientAuthenticationMethod` configuration
parameter to the druid-pac4j OIDC authentication extension, allowing users to
explicitly specify the client authentication method to use with their OIDC
provider.
#### Problem: Breaking change with pac4j 5.7.3 upgrade
The upgrade of pac4j to 5.7.3 in Druid 35 introduced support for the
`private_key_jwt` client authentication method (added in pac4j 5.7.0).
This created a breaking change for some OIDC deployments:
- pac4j 4.5.7 (Druid 34.0.0): When auto-detecting the authentication method
from an OIDC provider's discovery document, pac4j would not recognize
`private_key_jwt` and would fall back to the next available method like
`client_secret_basic`.
- pac4j 5.7.3 (Druid 35.0.1): pac4j now recognizes `private_key_jwt` in the
`SUPPORTED_METHODS` set. When an OIDC provider (e.g., Keycloak) advertises
`["private_key_jwt", "client_secret_basic", ...]`, pac4j selects
`private_key_jwt` as the first supported method. However, if
`privateKeyJwtConfig` is not configured in the code, authentication fails with:
`"privateKeyJwtConfig cannot be null"`
This affects users whose OIDC providers advertise `private_key_jwt` but who
want to use simpler authentication methods like `client_secret_basic` or
`client_secret_post`.
#### Solution: Explicit configuration parameter
Added a new optional `clientAuthenticationMethod` field to `OIDCConfig` that:
- Allows users to explicitly specify which client authentication method to
use
- Bypasses pac4j's auto-detection logic when set
- Maintains full backward compatibility (null/not specified = use pac4j
auto-detection)
The implementation:
1. Added `clientAuthenticationMethod` field to `OIDCConfig` with JSON
serialization
2. Modified `Pac4jAuthenticator` to call
`oidcConf.setClientAuthenticationMethod()` when the parameter is provided
3. Uses `ClientAuthenticationMethod.parse()` from nimbus library to parse
the method string
4. Added test coverage in `OIDCConfigTest`
#### Design decisions
- String-based configuration: Used a String type rather than an enum to
maintain flexibility as new authentication methods are added to the OIDC spec
and pac4j library
- Optional parameter: Made it nullable/optional to preserve backward
compatibility and allow users who are satisfied with auto-detection to continue
using it
- No default value: Explicitly choosing not to set a default value ensures
existing deployments continue their current behavior
#### Release note
Users of the druid-pac4j OIDC authentication extension can now explicitly
configure their preferred client authentication method using the new optional
`clientAuthenticationMethod` parameter. This resolves compatibility issues
introduced with pac4j 5.7.3 where OIDC providers advertising `private_key_jwt`
(such as Keycloak) would cause authentication failures when the asymmetric JWT
method was not configured.
Supported values include: `client_secret_basic`, `client_secret_post`,
`client_secret_jwt`, `private_key_jwt`, and `none`. If not specified, pac4j
will continue to use its auto-detection behavior.
Example configuration:
```json
{
"clientID": "druid-client",
"clientSecret": "secret",
"discoveryURI":
"https://auth.example.com/.well-known/openid-configuration";,
"clientAuthenticationMethod": "client_secret_basic"
}
#### Key changed/added classes in this PR
- OIDCConfig - Added clientAuthenticationMethod field with getter
- Pac4jAuthenticator - Added logic to set client authentication method when
configured
- OIDCConfigTest - Added test for new configuration parameter
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.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked
related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in
[licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] 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.
- [ ] added integration tests.
- [x] been tested in a test Druid cluster.
--
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]
