From fbfdb547459dfa4f82256a60aef03f6b32dba1dd Mon Sep 17 00:00:00 2001 From: Rinus Wiskerke Date: Fri, 9 Jun 2023 10:46:28 +0200 Subject: [PATCH] Strip rotated client secret from export json (#19394) Closes #19373 --- .../models/utils/StripSecretsUtils.java | 7 +++ .../models/StripSecretsUtilsTest.java | 57 +++++++++++++++++++ .../partialexport/PartialExportTest.java | 6 ++ .../export/partialexport-testrealm.json | 5 +- 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 server-spi-private/src/test/java/org/keycloak/models/StripSecretsUtilsTest.java diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/StripSecretsUtils.java b/server-spi-private/src/main/java/org/keycloak/models/utils/StripSecretsUtils.java index 401d37d950f..cee3efa066b 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/StripSecretsUtils.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/StripSecretsUtils.java @@ -18,6 +18,7 @@ package org.keycloak.models.utils; import org.keycloak.common.util.MultivaluedHashMap; +import org.keycloak.models.ClientSecretConstants; import org.keycloak.models.KeycloakSession; import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.representations.idm.ClientRepresentation; @@ -138,6 +139,12 @@ public class StripSecretsUtils { if (rep.getSecret() != null) { rep.setSecret(maskNonVaultValue(rep.getSecret())); } + if (rep.getAttributes() != null && rep.getAttributes().containsKey(ClientSecretConstants.CLIENT_ROTATED_SECRET)) { + rep.getAttributes().put( + ClientSecretConstants.CLIENT_ROTATED_SECRET, + maskNonVaultValue(rep.getAttributes().get(ClientSecretConstants.CLIENT_ROTATED_SECRET)) + ); + } return rep; } diff --git a/server-spi-private/src/test/java/org/keycloak/models/StripSecretsUtilsTest.java b/server-spi-private/src/test/java/org/keycloak/models/StripSecretsUtilsTest.java new file mode 100644 index 00000000000..fa38a9ae7b1 --- /dev/null +++ b/server-spi-private/src/test/java/org/keycloak/models/StripSecretsUtilsTest.java @@ -0,0 +1,57 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.models; + +import org.junit.Test; +import org.keycloak.models.utils.StripSecretsUtils; +import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.ComponentRepresentation; + +import java.util.HashMap; +import java.util.Map; + +import static org.junit.Assert.assertEquals; + +public class StripSecretsUtilsTest { + + @Test + public void checkStrippedRotatedSecret() { + ClientRepresentation stripped = StripSecretsUtils.strip(createClient("unmasked_secret")); + assertEquals(ComponentRepresentation.SECRET_VALUE, getRotatedSecret(stripped)); + } + + @Test + public void checkStrippedRotatedSecretVaultUnaffected() { + String rotatedSecret = "${vault.key}"; + ClientRepresentation stripped = StripSecretsUtils.strip(createClient(rotatedSecret)); + assertEquals(rotatedSecret, getRotatedSecret(stripped)); + } + + private ClientRepresentation createClient(String rotatedSecret) { + ClientRepresentation client = new ClientRepresentation(); + Map attrs = new HashMap<>(); + attrs.put(ClientSecretConstants.CLIENT_ROTATED_SECRET, rotatedSecret); + client.setAttributes(attrs); + return client; + } + + private String getRotatedSecret(ClientRepresentation clientRepresentation) { + return clientRepresentation.getAttributes().get(ClientSecretConstants.CLIENT_ROTATED_SECRET); + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialexport/PartialExportTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialexport/PartialExportTest.java index 9cbeb8f62a4..74e668a91db 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialexport/PartialExportTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialexport/PartialExportTest.java @@ -3,6 +3,7 @@ package org.keycloak.testsuite.admin.partialexport; import org.junit.Test; import org.keycloak.common.Profile; import org.keycloak.common.util.MultivaluedHashMap; +import org.keycloak.models.ClientSecretConstants; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ComponentExportRepresentation; import org.keycloak.representations.idm.ComponentRepresentation; @@ -19,6 +20,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.hamcrest.Matchers; import org.keycloak.common.constants.ServiceAccountConstants; @@ -146,6 +148,10 @@ public class PartialExportTest extends AbstractAdminTest { for (ClientRepresentation client: rep.getClients()) { if (Boolean.FALSE.equals(client.isPublicClient()) && Boolean.FALSE.equals(client.isBearerOnly())) { Assert.assertEquals("Client secret masked", ComponentRepresentation.SECRET_VALUE, client.getSecret()); + String rotatedSecret = Optional.ofNullable(client.getAttributes()) + .flatMap(attrs -> Optional.ofNullable(attrs.get(ClientSecretConstants.CLIENT_ROTATED_SECRET))) + .orElse(ComponentRepresentation.SECRET_VALUE); + Assert.assertEquals("Rotated client secret masked", ComponentRepresentation.SECRET_VALUE, rotatedSecret); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/export/partialexport-testrealm.json b/testsuite/integration-arquillian/tests/base/src/test/resources/export/partialexport-testrealm.json index 4c80bae4442..b10deb90b8c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/export/partialexport-testrealm.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/export/partialexport-testrealm.json @@ -732,7 +732,10 @@ "tls.client.certificate.bound.access.tokens": "false", "saml.authnstatement": "false", "display.on.consent.screen": "false", - "saml.onetimeuse.condition": "false" + "saml.onetimeuse.condition": "false", + "client.secret.rotated.creation.time": "1680684475", + "client.secret.rotated.expiration.time": "1680857275", + "client.secret.rotated": "oldPassword" }, "authenticationFlowBindingOverrides": {}, "fullScopeAllowed": true,