Integrate client secret rotation with client admin api v2 by rmartinc · Pull Request #51615 · keycloak/keycloak

rmartinc · 2026-08-11T11:08:35Z

Adding the client secret rotation bits for the admin API v2. I also needed to change the builders to make the test work with jackson3 (configuration for policies and profiles was not serialized OK). A simple test class added to check the rotation works OK with the admin API v2.

Copilot

Pull request overview

Integrates client-secret rotation into Admin API v2 and adds supporting tests and serialization fixes.

Changes:

Adds rotation and stale-secret cleanup to v2 updates.
Updates policy configuration builders for Jackson 3.
Adds Admin API v2 rotation tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`ClientProfileBuilder.java`	Updates executor configuration serialization.
`ClientPolicyBuilder.java`	Updates condition configuration serialization.
`ClientResource.java`	Simplifies rotation-state cleanup.
`ClientApiV2ClientSecretRotationTest.java`	Tests v2 secret rotation behavior.
`DefaultClientService.java`	Integrates rotation into v2 client updates.

Suppressed comments (1)

rest/admin-v2/services/src/main/java/org/keycloak/services/client/DefaultClientService.java:341

This cleanup runs before AdminClientUpdatedContext, which is the event that marks ordinary PUT/PATCH updates as rotation-enabled. An update under an active policy therefore deletes any rotated grace-period secret and resets its expiration metadata; trigger the final updated/rotation event before cleanup, matching ClientResource.java:172-176.

                        if (!Boolean.TRUE.equals(session.removeAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED))) {
                            OIDCClientSecretConfigWrapper.fromClientModel(model).removeClientSecretRotationInfo();
                        }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

michalvavrik · 2026-08-11T17:58:08Z

        }
        try {
-            executor.setConfiguration(RawJsonValue.of(JsonSerialization.mapper.readValue(JsonSerialization.mapper.writeValueAsBytes(config), JsonNode.class)));
+            executor.setConfiguration(JsonSerialization.mapper.readValue(JsonSerialization.mapper.writeValueAsBytes(config), RawJsonValue.class));


I agree this fixes the serialization in Jackson 3 test profile, but the JsonSerialization.mapper is Jackson 2 specific. I imagine something like this would be better:

diff --git a/test-framework/builders/src/main/java/org/keycloak/testframework/realm/ClientPolicyBuilder.java b/test-framework/builders/src/main/java/org/keycloak/testframework/realm/ClientPolicyBuilder.java index 3aa10c5655..9968094389 100644 --- a/test-framework/builders/src/main/java/org/keycloak/testframework/realm/ClientPolicyBuilder.java +++ b/test-framework/builders/src/main/java/org/keycloak/testframework/realm/ClientPolicyBuilder.java @@ -13,7 +13,7 @@ import org.keycloak.services.clientpolicy.condition.ClientAccessTypeCondition; import org.keycloak.services.clientpolicy.condition.ClientScopesCondition; import org.keycloak.services.clientpolicy.condition.GrantTypeCondition; import org.keycloak.services.clientpolicy.condition.IdentityProviderCondition; -import org.keycloak.util.JsonSerialization; +import org.keycloak.json.KeycloakJsonMapperFactory; /** @@ -94,7 +94,8 @@ public class ClientPolicyBuilder extends Builder<ClientPolicyRepresentation> { config = new ClientPolicyConditionConfigurationRepresentation(); } try { - condition.setConfiguration(JsonSerialization.mapper.readValue(JsonSerialization.mapper.writeValueAsBytes(config), RawJsonValue.class)); + var mapper = KeycloakJsonMapperFactory.mapper(); + condition.setConfiguration(mapper.readValue(mapper.writeValueAsBytes(config), RawJsonValue.class)); } catch(IOException e) { throw new IllegalArgumentException("Invalid configuration", e); } diff --git a/test-framework/builders/src/main/java/org/keycloak/testframework/realm/ClientProfileBuilder.java b/test-framework/builders/src/main/java/org/keycloak/testframework/realm/ClientProfileBuilder.java index 852aaf21ae..924ffa6d35 100644 --- a/test-framework/builders/src/main/java/org/keycloak/testframework/realm/ClientProfileBuilder.java +++ b/test-framework/builders/src/main/java/org/keycloak/testframework/realm/ClientProfileBuilder.java @@ -8,7 +8,7 @@ import org.keycloak.json.RawJsonValue; import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation; import org.keycloak.representations.idm.ClientPolicyExecutorRepresentation; import org.keycloak.representations.idm.ClientProfileRepresentation; -import org.keycloak.util.JsonSerialization; +import org.keycloak.json.KeycloakJsonMapperFactory; /** @@ -46,7 +46,8 @@ public class ClientProfileBuilder extends Builder<ClientProfileRepresentation> { config = new ClientPolicyExecutorConfigurationRepresentation(); } try { - executor.setConfiguration(JsonSerialization.mapper.readValue(JsonSerialization.mapper.writeValueAsBytes(config), RawJsonValue.class)); + var mapper = KeycloakJsonMapperFactory.mapper(); + executor.setConfiguration(mapper.readValue(mapper.writeValueAsBytes(config), RawJsonValue.class)); } catch (IOException e) { throw new IllegalArgumentException("Invalid configuration", e); }

Thanks @michalvavrik! Done!

michalvavrik

LGTM

Closes keycloak#51535 Closes keycloak#51527 Signed-off-by: rmartinc <rmartinc@redhat.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

keycloak-github-bot · 2026-08-12T08:18:55Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#salesPostSigEmailTest

Keycloak CI - Adapter IT Strict Cookies

java.lang.NullPointerException: Cannot invoke "String.contains(java.lang.CharSequence)" because the return value of "org.openqa.selenium.WebDriver.getPageSource()" is null
	at org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest.assertForbiddenLogin(SAMLServletAdapterTest.java:547)
	at org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest.testSuccessfulAndUnauthorizedLogin(SAMLServletAdapterTest.java:581)
	at org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest.testSuccessfulAndUnauthorizedLogin(SAMLServletAdapterTest.java:574)
	at org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest.salesPostSigEmailTest(SAMLServletAdapterTest.java:1064)
...

Report flaky test

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#salesPostEmptyConsumerPostURL

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
...

Report flaky test

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#testReloginWithInvalidAuthSessionCookie

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: Expected page 'login-login' to be loaded, but currently on page 'null' after timeout
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:38)
	at org.junit.jupiter.api.Assertions.fail(Assertions.java:138)
	at org.keycloak.testsuite.pages.AbstractPage.waitForPage(AbstractPage.java:62)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

michalvavrik

Thanks, I think this works for the default service. However I am not much involved in SCIM integration that is happening right now and is probably going to replace the default service soon, you will need review @shawkins to get proper context

rmartinc · 2026-08-12T11:00:07Z

Thanks @michalvavrik! The AI just detected the DefaultClientService for the admin v2, @shawkins if there is another place that the client/secret can be updated please let us know.

rmartinc requested a review from a team as a code owner August 11, 2026 11:08

Copilot AI balanced review requested due to automatic review settings August 11, 2026 11:08

rmartinc requested a review from a team as a code owner August 11, 2026 11:08

keycloak-github-bot Bot added team/cloud-native team/core-authn labels Aug 11, 2026

Copilot started reviewing on behalf of rmartinc August 11, 2026 11:09 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 11, 2026 12:53

rmartinc force-pushed the issue-51535 branch from f3aecf6 to 9a313c9 Compare August 11, 2026 12:53

Copilot started reviewing on behalf of rmartinc August 11, 2026 12:56 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

michalvavrik reviewed Aug 11, 2026

View reviewed changes

Integrate client secret rotation with client admin api v2

4119b37

Closes keycloak#51535 Closes keycloak#51527 Signed-off-by: rmartinc <rmartinc@redhat.com>

rmartinc force-pushed the issue-51535 branch from 9a313c9 to 4119b37 Compare August 12, 2026 07:13

Copilot AI review requested due to automatic review settings August 12, 2026 07:13

Copilot started reviewing on behalf of rmartinc August 12, 2026 07:14 View session

Copilot AI reviewed Aug 12, 2026

View reviewed changes

keycloak-github-bot Bot added the flaky-test label Aug 12, 2026

keycloak-github-bot Bot mentioned this pull request Aug 12, 2026

Flaky test: org.keycloak.testsuite.webauthn.registration.WebAuthnOtherSettingsTest#apiInvalidStateErrorMessage #51175

Open

keycloak-github-bot Bot reviewed Aug 12, 2026

View reviewed changes

michalvavrik approved these changes Aug 12, 2026

View reviewed changes

Navigation Menu

Uh oh!

Integrate client secret rotation with client admin api v2 - #51615

Integrate client secret rotation with client admin api v2#51615
rmartinc wants to merge 1 commit into
keycloak:mainfrom
rmartinc:issue-51535

rmartinc commented Aug 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

michalvavrik Aug 11, 2026

Uh oh!

rmartinc Aug 12, 2026

Uh oh!

michalvavrik left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

keycloak-github-bot Bot commented Aug 12, 2026

Uh oh!

keycloak-github-bot Bot left a comment

Uh oh!

michalvavrik left a comment

Uh oh!

rmartinc commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

rmartinc commented Aug 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

michalvavrik Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

rmartinc Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

michalvavrik left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

keycloak-github-bot Bot commented Aug 12, 2026

Unreported flaky test detected

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#salesPostSigEmailTest

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#salesPostEmptyConsumerPostURL

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#testReloginWithInvalidAuthSessionCookie

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

michalvavrik left a comment

Choose a reason for hiding this comment

Uh oh!

rmartinc commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants