Skip to content

Rotated client secret remains valid when the feature is disabled #50910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rmartinc merged 2 commits into from
Jul 20, 2026
Merged

Rotated client secret remains valid when the feature is disabled #50910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b029fdc
Rotated client secret remains valid when the feature is disabled
mabartos Jul 15, 2026
770b8ac
Simplify tests
mabartos Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions services/src/main/java/org/keycloak/protocol/oidc/OIDCClientSecretConfigWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import java.util.Objects;
import java.util.concurrent.TimeUnit;

import org.keycloak.common.Profile;
import org.keycloak.common.Profile.Feature;
import org.keycloak.common.util.Time;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientSecretConstants;
Expand All @@ -32,9 +34,11 @@
* @author <a href="mailto:masales@redhat.com">Marcelo Sales</a>
*/
public class OIDCClientSecretConfigWrapper extends AbstractClientConfigWrapper {
private final boolean isRotationFeatureEnabled;

private OIDCClientSecretConfigWrapper(ClientModel client, ClientRepresentation clientRep) {
super(client, clientRep);
this.isRotationFeatureEnabled = Profile.isFeatureEnabled(Feature.CLIENT_SECRET_ROTATION);
}

public static OIDCClientSecretConfigWrapper fromClientModel(ClientModel client) {
Expand Down Expand Up @@ -84,7 +88,7 @@ public void removeClientSecretRotationInfo() {
}

public void removeClientSecretRotated() {
if (hasRotatedSecret()) {
if (hasRotatedSecretAttributes()) {
Comment thread
mabartos marked this conversation as resolved.
setAttribute(CLIENT_ROTATED_SECRET, null);
setAttribute(CLIENT_ROTATED_SECRET_CREATION_TIME, null);
setAttribute(CLIENT_ROTATED_SECRET_EXPIRATION_TIME, null);
Expand All @@ -101,9 +105,18 @@ public void setClientSecretCreationTime(long creationTime) {
}

public boolean hasRotatedSecret() {
return StringUtil.isNotBlank(getAttribute(CLIENT_ROTATED_SECRET)) && StringUtil.isNotBlank(getAttribute(CLIENT_ROTATED_SECRET_CREATION_TIME));
return isRotationFeatureEnabled && hasRotatedSecretAttributes();
Comment thread
mabartos marked this conversation as resolved.
}

private boolean hasRotatedSecretAttributes() {
return StringUtil.isNotBlank(getAttribute(CLIENT_ROTATED_SECRET))
&& StringUtil.isNotBlank(getAttribute(CLIENT_ROTATED_SECRET_CREATION_TIME));
}

/**
* Returns the rotated client secret value resolved through the vault.
* Use {@link #hasRotatedSecret()} to check whether a rotated secret is effectively present before calling this method.
*/
public String getClientRotatedSecret(KeycloakSession session) {
String secret = getAttribute(CLIENT_ROTATED_SECRET);
return session == null ? getAttribute(CLIENT_ROTATED_SECRET) : session.vault().getStringSecret(secret).get().orElse(secret);
Expand Down
2 changes: 2 additions & 0 deletions services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,8 @@ public Response invalidateRotatedSecret() {

logger.debug("delete rotated secret");

// Always remove rotated secret attributes even when the feature is disabled,
// so stale secrets cannot become valid again if the feature is re-enabled.
OIDCClientSecretConfigWrapper wrapper = OIDCClientSecretConfigWrapper.fromClientModel(client);

CredentialRepresentation rep = new CredentialRepresentation();
Expand Down
82 changes: 82 additions & 0 deletions tests/base/src/test/java/org/keycloak/tests/client/ClientSecretRotationDisabledTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package org.keycloak.tests.client;

import org.keycloak.common.util.Time;
import org.keycloak.models.ClientSecretConstants;
import org.keycloak.testframework.annotations.InjectUser;
import org.keycloak.testframework.annotations.KeycloakIntegrationTest;
import org.keycloak.testframework.oauth.OAuthClient;
import org.keycloak.testframework.oauth.annotations.InjectOAuthClient;
import org.keycloak.testframework.realm.ManagedUser;
import org.keycloak.testframework.remote.runonserver.InjectRunOnServer;
import org.keycloak.testframework.remote.runonserver.RunOnServerClient;
import org.keycloak.tests.common.TestRealmUserConfig;
import org.keycloak.testsuite.util.oauth.AccessTokenResponse;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

@KeycloakIntegrationTest
public class ClientSecretRotationDisabledTest {

private static final String CLIENT_ID = "test-app";

@InjectOAuthClient
OAuthClient oauth;

@InjectUser(config = TestRealmUserConfig.class)
ManagedUser user;

@InjectRunOnServer
RunOnServerClient runOnServer;

/**
* Verifies that rotated client secrets are not accepted when the CLIENT_SECRET_ROTATION
* feature is disabled, even if the rotated secret attributes remain in the database.
*
* @see <a href="https://github.com/keycloak/keycloak/issues/50855">Issue #50855</a>
*/
@Test
public void rotatedSecretNotAcceptedWhenFeatureDisabled() {
String originalSecret = oauth.clientResource().getSecret().getValue();

// Verify the original secret works before rotating
oauth.client(CLIENT_ID, originalSecret);
oauth.doLogin(user.getUsername(), "password");
String code = oauth.parseLoginResponse().getCode();
AccessTokenResponse response = oauth.doAccessTokenRequest(code);
assertEquals(200, response.getStatusCode());
assertNotNull(response.getAccessToken());

String newSecret = oauth.clientResource().generateNewSecret().getValue();

// Set rotated secret attributes directly on the server-side model to bypass the admin
// API cleanup in ClientResource.update() which always removes rotation info when no
// rotation executor is active.
String creationTime = String.valueOf(Time.currentTimeSeconds());
String expirationTime = String.valueOf(Time.currentTimeSeconds() + 3600);
runOnServer.run(session -> {
var realmModel = session.getContext().getRealm();
var client = realmModel.getClientByClientId(CLIENT_ID);
client.setAttribute(ClientSecretConstants.CLIENT_ROTATED_SECRET, originalSecret);
client.setAttribute(ClientSecretConstants.CLIENT_ROTATED_SECRET_CREATION_TIME, creationTime);
client.setAttribute(ClientSecretConstants.CLIENT_ROTATED_SECRET_EXPIRATION_TIME, expirationTime);
});

// Authentication with the rotated (original) secret must fail when feature is disabled
oauth.client(CLIENT_ID, originalSecret);
oauth.openLoginForm();
code = oauth.parseLoginResponse().getCode();
response = oauth.doAccessTokenRequest(code);
assertEquals(401, response.getStatusCode());

// Authentication with the current (new) secret must always work
oauth.client(CLIENT_ID, newSecret);
oauth.openLoginForm();
code = oauth.parseLoginResponse().getCode();
response = oauth.doAccessTokenRequest(code);
assertEquals(200, response.getStatusCode());
assertNotNull(response.getAccessToken());
}
}
8 changes: 8 additions & 0 deletions ...lian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientSecretRotationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import org.keycloak.admin.client.resource.ClientPoliciesPoliciesResource;
import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.authentication.authenticators.client.ClientIdAndSecretAuthenticator;
import org.keycloak.common.Profile;
import org.keycloak.common.Profile.Feature;
import org.keycloak.common.profile.CommaSeparatedListProfileConfigResolver;
import org.keycloak.common.util.Time;
import org.keycloak.events.Details;
import org.keycloak.events.EventType;
Expand Down Expand Up @@ -60,6 +62,7 @@
import org.jboss.logging.Logger;
import org.jetbrains.annotations.NotNull;
import org.junit.After;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;

Expand Down Expand Up @@ -108,6 +111,11 @@ public class ClientSecretRotationTest extends AbstractRestServiceTest {
private static final int DEFAULT_REMAIN_EXPIRATION_PERIOD = Long.valueOf(
TimeUnit.MINUTES.toSeconds(30)).intValue();

@BeforeClass
public static void beforeAll() {
Profile.configure(new CommaSeparatedListProfileConfigResolver(Feature.CLIENT_SECRET_ROTATION.getVersionedKey(), ""));
Comment thread
mabartos marked this conversation as resolved.
}

@Rule
public AssertEvents events = new AssertEvents(this);

Expand Down
Loading