Repository navigation
feat(auth): support signed JWT client assertions - #1678
Conversation
18a850b to
c309f67
Compare
Signed-off-by: Sebastian Łaskawiec <sebastian.laskawiec@defenseunicorns.com>
c309f67 to
c82ef33
Compare
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved logout client_id issues affect production authentication, and the README example is incomplete.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds file-backed signed JWT client assertion support for Keycloak, including rotating Kubernetes service-account tokens, request filtering, redaction, tests, documentation, and CI coverage.
Changes:
- Adds configurable assertion providers and request integration.
- Adds sensitive-data redaction and token/logout test coverage.
- Documents Kubernetes usage and adds Keycloak 26.6 CI coverage.
File summaries
| File | Reviewed change |
|---|---|
src/test/resources/client-assertion-token |
Provides assertion test data. |
src/test/java/de/adorsys/keycloak/config/util/resteasy/FileClientAssertionProviderTest.java |
Tests file-backed assertion loading and validation. |
src/test/java/de/adorsys/keycloak/config/util/resteasy/ClientAssertionFilterTest.java |
Tests assertion request mutation. |
src/test/java/de/adorsys/keycloak/config/util/logging/SensitiveDataSanitizingFilterTest.java |
Tests assertion redaction. |
src/test/java/de/adorsys/keycloak/config/provider/KeycloakProviderTest.java |
Tests provider construction. |
src/test/java/de/adorsys/keycloak/config/properties/KeycloakConfigPropertiesTest.java |
Tests configuration binding. |
src/test/java/de/adorsys/keycloak/config/mock/ClientAssertionMockIT.java |
Tests token and logout flows; logout should verify client_id. |
src/test/java/de/adorsys/keycloak/config/AbstractImportIT.java |
Enables required Keycloak features. |
src/main/java/de/adorsys/keycloak/config/util/resteasy/FileClientAssertionProvider.java |
Reads assertions from files. |
src/main/java/de/adorsys/keycloak/config/util/resteasy/ClientAssertionProvider.java |
Defines the assertion provider interface. |
src/main/java/de/adorsys/keycloak/config/util/resteasy/ClientAssertionFilter.java |
Injects assertions; must preserve client_id and remove only client_secret. |
src/main/java/de/adorsys/keycloak/config/util/logging/SensitiveDataSanitizingFilter.java |
Redacts sensitive assertion data from logs. |
src/main/java/de/adorsys/keycloak/config/provider/KeycloakProvider.java |
Integrates assertion authentication; logout must include the configured client ID. |
src/main/java/de/adorsys/keycloak/config/properties/KeycloakConfigProperties.java |
Adds assertion-file configuration. |
README.md |
Documents configuration and Kubernetes usage; the non-master example needs KEYCLOAK_SKIPSERVERINFO=true. |
CHANGELOG.md |
Records the new feature. |
.github/workflows/ci.yaml |
Adds Keycloak 26.6 CI coverage. |
Review details
Suppressed comments (2)
README.md:337
- This example logs into the non-master
kubernetesrealm but does not setKEYCLOAK_SKIPSERVERINFO=true. The option table above documents that this is required for non-master realm authentication, and the provider otherwise fetches server info during initialization, so following this example can fail before the import starts. Add the missing environment variable.
- name: KEYCLOAK_LOGINREALM
value: kubernetes
- name: KEYCLOAK_GRANTTYPE
value: client_credentials
- name: KEYCLOAK_CLIENTID
src/test/java/de/adorsys/keycloak/config/mock/ClientAssertionMockIT.java:73
- These assertions encode the opposite of the logout endpoint's contract. Real Keycloak requires
client_idto identify the client before authenticating the assertion, but this mock accepts the request without validating it, so the test currently passes for a request that production rejects. Assert that the configured client ID is present instead.
assertThat(body, not(containsString("client_id=")));
- Files reviewed: 17/17 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
chance-coleman
left a comment
There was a problem hiding this comment.
Reviewed. No issues found in testing locally. CI is green. Approving.
|
Hey @Nkwenti-Severian-Ndongtsop 👋🏻 Could I kindly ask you to have a look at this PR? Thank you 🙏🏻 |
|
Hey @AssahBismarkabah 👋🏻 Could I kindly ask you to have a look at this PR? Thank you 🙏🏻 |
|
@slaskawi Apologies for the delayed response. I'll look into this |
@chance-coleman Based on your testing, did you encounter any edge cases or potential issues, or does everything look good? |
|
Motouom
left a comment
There was a problem hiding this comment.
Reviewed the resolved conflicts and final diff. Local mvnw clean install passed with 349 tests, Checkstyle, SpotBugs, and the dedicated client-assertion test. The Keycloak 26.6.2 CLI JSON import/idempotence test passed, and GitHub CI run 3612 passed across the required matrix. Approving.



feat(auth): support signed JWT client assertions
What this PR does / why we need it:
"Signed JWT - Federated" Client Authentication is one of the recent add-ons in Keycloak. The implementation uses OAuth 2.0 Client Assertions which an authentication mechanism suitable for service accounts, which assumes short-lived workload identity tokens, so the CLI does not need to store static credentials or client secrets.
This implementation focuses on supporting Kubernetes Service Account Tokens and can be easily extended for supporting SPIFFE JWT SVIDs. However the latter is a preview feature in Keycloak as the spec is still in draft. We'll probably need to wait a few more releases until it's fully supported.
Highlights of this Pull Request:
ClientAssertionProviderso sources such as SPIFFE JWT-SVIDs can be added independently.Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): N/A — no issue supplied.How this PR was tested:
Automatically tested in CI across supported Keycloak versions, Java 17 and Java 21, including dedicated Client Assertion tests for token rotation, token requests, logout, configuration, and log redaction.
Screenshots / logs (if relevant): N/A.
Special notes for your reviewer:
This addition would provide a lot of benefits for platforms that do not store credentials, such as UDS Core. We've been experimenting with this approach recently: defenseunicorns/uds-identity-config#853
PR Readiness Checklist:
Complete these before marking the PR as
ready to review:mvn testor relevant subset)CHANGELOG.mdrelease notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR