libre.sh/keycloak-scim
4
0
Fork 0
Code Issues 15 Pull requests Releases 1 Activity Actions

Update secret rotation when the policy is disabled (#10674)

Browse source 
Closes #10667
This commit is contained in:
Marcelo Daniel Silva Sales 2022-03-10 13:03:09 +01:00 committed by GitHub
parent 1a4d7c297a
commit 0c25da542c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 103 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
services/src/main/java/org/keycloak/protocol/oidc/OIDCClientSecretConfigWrapper.java
View file

@ -18,6 +18,7 @@ import static org.keycloak.models.ClientSecretConstants.CLIENT_ROTATED_SECRET_CR
import static org.keycloak.models.ClientSecretConstants.CLIENT_ROTATED_SECRET_EXPIRATION_TIME;
import static org.keycloak.models.ClientSecretConstants.CLIENT_SECRET_CREATION_TIME;
import static org.keycloak.models.ClientSecretConstants.CLIENT_SECRET_EXPIRATION;
import static org.keycloak.models.ClientSecretConstants.CLIENT_SECRET_REMAINING_EXPIRATION_TIME;
/**
* @author <a href="mailto:masales@redhat.com">Marcelo Sales</a>
@ -61,6 +62,12 @@ public class OIDCClientSecretConfigWrapper extends AbstractClientConfigWrapper {
}
}
public void removeClientSecretRotationInfo() {
setAttribute(CLIENT_SECRET_EXPIRATION, null);
setAttribute(CLIENT_SECRET_REMAINING_EXPIRATION_TIME, null);
removeClientSecretRotated();
}
public void removeClientSecretRotated() {
if (hasRotatedSecret()) {
setAttribute(CLIENT_ROTATED_SECRET, null);

3
services/src/main/java/org/keycloak/services/clientpolicy/executor/ClientSecretRotationExecutor.java
View file

@ -6,6 +6,7 @@ import com.fasterxml.jackson.annotation.JsonProperty;
import org.jboss.logging.Logger;
import org.keycloak.common.util.Time;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientSecretConstants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.protocol.oidc.OIDCClientSecretConfigWrapper;
@ -48,7 +49,7 @@ public class ClientSecretRotationExecutor implements
public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyException {
if (!session.getContext().getClient().isPublicClient() && !session.getContext().getClient()
.isBearerOnly()) {
session.setAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED,Boolean.TRUE);
switch (context.getEvent()) {
case REGISTERED:
case UPDATED:

17
services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java
View file

@ -32,6 +32,7 @@ import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.AuthenticatedClientSessionModel;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.ClientSecretConstants;
import org.keycloak.models.Constants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelDuplicateException;
@ -141,6 +142,7 @@ public class ClientResource {
auth.clients().requireConfigure(client);
try {
session.setAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED,Boolean.FALSE);
session.clientPolicy().triggerOnEvent(new AdminClientUpdateContext(rep, client, auth.adminAuth()));
updateClientFromRep(rep, client, session);
@ -155,6 +157,12 @@ public class ClientResource {
session.clientPolicy().triggerOnEvent(new AdminClientUpdatedContext(rep, client, auth.adminAuth()));
if (!(boolean) session.getAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED)){
logger.debugv("Removing the previous rotation info for client {0}{1}, if there is",client.getClientId(),client.getName());
OIDCClientSecretConfigWrapper.fromClientModel(client).removeClientSecretRotationInfo();
}
session.removeAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED);
adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()).representation(rep).success();
return Response.noContent().build();
} catch (ModelDuplicateException e) {
@ -253,6 +261,7 @@ public class ClientResource {
auth.clients().requireConfigure(client);
logger.debug("regenerateSecret");
session.setAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED,Boolean.FALSE);
ClientRepresentation representation = ModelToRepresentation.toRepresentation(client, session);
ClientSecretRotationContext secretRotationContext = new ClientSecretRotationContext(
@ -265,10 +274,14 @@ public class ClientResource {
CredentialRepresentation rep = new CredentialRepresentation();
rep.setType(CredentialRepresentation.SECRET);
rep.setValue(secret);
rep.setCreatedDate(
(long) OIDCClientSecretConfigWrapper.fromClientModel(client).getClientSecretCreationTime());
if (!(boolean) session.getAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED)){
logger.debugv("Removing the previous rotation info for client {0}{1}, if there is",client.getClientId(),client.getName());
OIDCClientSecretConfigWrapper.fromClientModel(client).removeClientSecretRotationInfo();
}
adminEvent.operation(OperationType.ACTION).resourcePath(session.getContext().getUri()).representation(rep).success();
session.removeAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED);
return rep;
} catch (ClientPolicyException cpe) {

77
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientSecretRotationTest.java
View file

@ -623,6 +623,72 @@ public class ClientSecretRotationTest extends AbstractRestServiceTest {
assertThat(policies.getPolicies(), is(empty()));
}
/**
* When there is a client that has a secret rotated and the policy is disabled, Rotation information must be removed after updating a client
*
* @throws Exception
*/
@Test
public void removingPolicyMustClearRotationInformationFromClientOnUpdate() throws Exception {
//create and enable the profile
configureDefaultProfileAndPolicy();
//create client
String cidConfidential = createClientByAdmin(DEFAULT_CLIENT_ID);
ClientResource clientResource = adminClient.realm(REALM_NAME).clients().get(cidConfidential);
String firstSecret = clientResource.getSecret().getValue();
String newSecret = clientResource.generateNewSecret().getValue();
String rotatedSecret = clientResource.getClientRotatedSecret().getValue();
assertThat(firstSecret, not(equalTo(newSecret)));
assertThat(firstSecret, equalTo(rotatedSecret));
//disable the profile
disableProfile();
//force a update
ClientRepresentation clientRepresentation = clientResource.toRepresentation();
clientRepresentation.setDescription("New Description Updated");
clientResource.update(clientRepresentation);
//client must not have any information about rotation in it
clientRepresentation = clientResource.toRepresentation();
OIDCClientSecretConfigWrapper wrapper = OIDCClientSecretConfigWrapper.fromClientRepresentation(clientRepresentation);
assertThat(wrapper.hasRotatedSecret(), is(false));
assertThat(wrapper.getClientSecretExpirationTime(),is(0));
}
/**
* When there is a client that has a secret rotated and the policy is disabled, Rotation information must be removed after request a new secret
*
* @throws Exception
*/
@Test
public void removingPolicyMustClearRotationInformationFromClientOnRequestNewSecret() throws Exception {
//create and enable the profile
configureDefaultProfileAndPolicy();
//create client
String cidConfidential = createClientByAdmin(DEFAULT_CLIENT_ID);
ClientResource clientResource = adminClient.realm(REALM_NAME).clients().get(cidConfidential);
String firstSecret = clientResource.getSecret().getValue();
String newSecret = clientResource.generateNewSecret().getValue();
String rotatedSecret = clientResource.getClientRotatedSecret().getValue();
assertThat(firstSecret, not(equalTo(newSecret)));
assertThat(firstSecret, equalTo(rotatedSecret));
//disable the profile
disableProfile();
//Request a new secret
newSecret = clientResource.generateNewSecret().getValue();
//client must not have any information about rotation in it
ClientRepresentation clientRepresentation = clientResource.toRepresentation();
OIDCClientSecretConfigWrapper wrapper = OIDCClientSecretConfigWrapper.fromClientRepresentation(clientRepresentation);
assertThat(clientResource.getSecret().getValue(),equalTo(newSecret));
assertThat(wrapper.hasRotatedSecret(), is(false));
assertThat(wrapper.getClientSecretExpirationTime(),is(0));
}
/**
* -------------------- support methods --------------------
**/
@ -646,6 +712,17 @@ public class ClientSecretRotationTest extends AbstractRestServiceTest {
doConfigProfileAndPolicy(profileBuilder, profileConfig);
}
private void disableProfile() throws Exception {
Configuration config = new Configuration();
config.setType(Arrays.asList(ClientAccessTypeConditionFactory.TYPE_CONFIDENTIAL));
String json = (new ClientPoliciesBuilder()).addPolicy(
(new ClientPolicyBuilder()).createPolicy(POLICY_NAME,
"Policy for Client Secret Rotation",
Boolean.FALSE).addCondition(ClientAccessTypeConditionFactory.PROVIDER_ID, config)
.addProfile(PROFILE_NAME).toRepresentation()).toString();
updatePolicies(json);
}
private void doConfigProfileAndPolicy(ClientProfileBuilder profileBuilder,
ClientSecretRotationExecutor.Configuration profileConfig) throws Exception {
String json = (new ClientProfilesBuilder()).addProfile(