From 5a00aaefa87372d19059de5fcf56c30fea073da8 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Wed, 19 Oct 2016 19:20:17 +0200 Subject: [PATCH] KEYCLOAK-2594 bind credential being leaked in admin tool JSON response KEYCLOAK-2972 Keycloak leaks configuration passwords in Admin Event logs --- .../common/util/MultivaluedHashMap.java | 7 ++ .../keycloak/models/utils/ComponentUtil.java | 29 ++++++-- .../models/utils/ModelToRepresentation.java | 29 +++----- .../models/utils/RepresentationToModel.java | 9 ++- .../models/utils/StripSecretsUtils.java | 69 +++++++++++++++++++ .../resources/admin/ComponentResource.java | 5 +- .../admin/IdentityProviderResource.java | 14 +++- .../admin/IdentityProvidersResource.java | 5 +- .../resources/admin/RealmAdminResource.java | 3 +- .../rest/TestingResourceProvider.java | 15 ++++ .../client/resources/TestingResource.java | 10 +++ .../testsuite/admin/ComponentsTest.java | 14 ++++ .../testsuite/admin/IdentityProviderTest.java | 30 ++++++-- .../testsuite/admin/realm/RealmTest.java | 32 +++++++++ 14 files changed, 231 insertions(+), 40 deletions(-) create mode 100644 server-spi/src/main/java/org/keycloak/models/utils/StripSecretsUtils.java diff --git a/common/src/main/java/org/keycloak/common/util/MultivaluedHashMap.java b/common/src/main/java/org/keycloak/common/util/MultivaluedHashMap.java index dea1d50e4b..82fede21cc 100755 --- a/common/src/main/java/org/keycloak/common/util/MultivaluedHashMap.java +++ b/common/src/main/java/org/keycloak/common/util/MultivaluedHashMap.java @@ -30,6 +30,13 @@ import java.util.Map; @SuppressWarnings("serial") public class MultivaluedHashMap extends HashMap> { + public MultivaluedHashMap() { + } + + public MultivaluedHashMap(MultivaluedHashMap config) { + addAll(config); + } + public void putSingle(K key, V value) { List list = new ArrayList(); diff --git a/server-spi/src/main/java/org/keycloak/models/utils/ComponentUtil.java b/server-spi/src/main/java/org/keycloak/models/utils/ComponentUtil.java index 507bfffb1d..9ebb9cf33d 100644 --- a/server-spi/src/main/java/org/keycloak/models/utils/ComponentUtil.java +++ b/server-spi/src/main/java/org/keycloak/models/utils/ComponentUtil.java @@ -23,6 +23,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.provider.Provider; import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.provider.ProviderFactory; +import org.keycloak.representations.idm.ComponentRepresentation; import java.util.HashMap; import java.util.List; @@ -33,9 +34,25 @@ import java.util.Map; */ public class ComponentUtil { + public static Map getComponentConfigProperties(KeycloakSession session, ComponentRepresentation component) { + return getComponentConfigProperties(session, component.getProviderType(), component.getProviderId()); + } + public static Map getComponentConfigProperties(KeycloakSession session, ComponentModel component) { + return getComponentConfigProperties(session, component.getProviderType(), component.getProviderId()); + } + + public static ComponentFactory getComponentFactory(KeycloakSession session, ComponentRepresentation component) { + return getComponentFactory(session, component.getProviderType(), component.getProviderId()); + } + + public static ComponentFactory getComponentFactory(KeycloakSession session, ComponentModel component) { + return getComponentFactory(session, component.getProviderType(), component.getProviderId()); + } + + private static Map getComponentConfigProperties(KeycloakSession session, String providerType, String providerId) { try { - List l = getComponentFactory(session, component).getConfigProperties(); + List l = getComponentFactory(session, providerType, providerId).getConfigProperties(); Map properties = new HashMap<>(); for (ProviderConfigProperty p : l) { properties.put(p.getName(), p); @@ -46,15 +63,15 @@ public class ComponentUtil { } } - public static ComponentFactory getComponentFactory(KeycloakSession session, ComponentModel component) { - Class provider = session.getProviderClass(component.getProviderType()); + private static ComponentFactory getComponentFactory(KeycloakSession session, String providerType, String providerId) { + Class provider = session.getProviderClass(providerType); if (provider == null) { - throw new RuntimeException("Invalid provider type '" + component.getProviderType() + "'"); + throw new RuntimeException("Invalid provider type '" + providerType + "'"); } - ProviderFactory f = session.getKeycloakSessionFactory().getProviderFactory(provider, component.getProviderId()); + ProviderFactory f = session.getKeycloakSessionFactory().getProviderFactory(provider, providerId); if (f == null) { - throw new RuntimeException("No such provider '" + component.getProviderId() + "'"); + throw new RuntimeException("No such provider '" + providerId + "'"); } ComponentFactory cf = (ComponentFactory) f; diff --git a/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index 23173528db..569faf6e05 100755 --- a/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -303,7 +303,7 @@ public class ModelToRepresentation { rep.setAccessCodeLifespan(realm.getAccessCodeLifespan()); rep.setAccessCodeLifespanUserAction(realm.getAccessCodeLifespanUserAction()); rep.setAccessCodeLifespanLogin(realm.getAccessCodeLifespanLogin()); - rep.setSmtpServer(realm.getSmtpConfig()); + rep.setSmtpServer(new HashMap<>(realm.getSmtpConfig())); rep.setBrowserSecurityHeaders(realm.getBrowserSecurityHeaders()); rep.setAccountTheme(realm.getAccountTheme()); rep.setLoginTheme(realm.getLoginTheme()); @@ -385,6 +385,10 @@ public class ModelToRepresentation { Map attributes = realm.getAttributes(); rep.setAttributes(attributes); + if (!internal) { + rep = StripSecretsUtils.strip(rep); + } + return rep; } @@ -622,7 +626,7 @@ public class ModelToRepresentation { providerRep.setStoreToken(identityProviderModel.isStoreToken()); providerRep.setTrustEmail(identityProviderModel.isTrustEmail()); providerRep.setAuthenticateByDefault(identityProviderModel.isAuthenticateByDefault()); - providerRep.setConfig(identityProviderModel.getConfig()); + providerRep.setConfig(new HashMap<>(identityProviderModel.getConfig())); providerRep.setAddReadTokenRoleOnCreate(identityProviderModel.isAddReadTokenRoleOnCreate()); String firstBrokerLoginFlowId = identityProviderModel.getFirstBrokerLoginFlowId(); @@ -796,24 +800,9 @@ public class ModelToRepresentation { rep.setProviderType(component.getProviderType()); rep.setSubType(component.getSubType()); rep.setParentId(component.getParentId()); - if (internal) { - rep.setConfig(component.getConfig()); - } else { - Map configProperties = ComponentUtil.getComponentConfigProperties(session, component); - MultivaluedHashMap config = new MultivaluedHashMap<>(); - - for (Map.Entry> e : component.getConfig().entrySet()) { - ProviderConfigProperty configProperty = configProperties.get(e.getKey()); - if (configProperty != null) { - if (configProperty.isSecret()) { - config.putSingle(e.getKey(), ComponentRepresentation.SECRET_VALUE); - } else { - config.put(e.getKey(), e.getValue()); - } - } - } - - rep.setConfig(config); + rep.setConfig(new MultivaluedHashMap<>(component.getConfig())); + if (!internal) { + rep = StripSecretsUtils.strip(session, rep); } return rep; } diff --git a/server-spi/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index 8ae1caa7e0..b972cda3bf 100755 --- a/server-spi/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -815,7 +815,12 @@ public class RepresentationToModel { } if (rep.getSmtpServer() != null) { - realm.setSmtpConfig(new HashMap(rep.getSmtpServer())); + Map config = new HashMap(rep.getSmtpServer()); + if (rep.getSmtpServer().containsKey("password") && ComponentRepresentation.SECRET_VALUE.equals(rep.getSmtpServer().get("password"))) { + String passwordValue = realm.getSmtpConfig() != null ? realm.getSmtpConfig().get("password") : null; + config.put("password", passwordValue); + } + realm.setSmtpConfig(config); } if (rep.getBrowserSecurityHeaders() != null) { @@ -1543,7 +1548,7 @@ public class RepresentationToModel { identityProviderModel.setAuthenticateByDefault(representation.isAuthenticateByDefault()); identityProviderModel.setStoreToken(representation.isStoreToken()); identityProviderModel.setAddReadTokenRoleOnCreate(representation.isAddReadTokenRoleOnCreate()); - identityProviderModel.setConfig(representation.getConfig()); + identityProviderModel.setConfig(new HashMap<>(representation.getConfig())); String flowAlias = representation.getFirstBrokerLoginFlowAlias(); if (flowAlias == null) { diff --git a/server-spi/src/main/java/org/keycloak/models/utils/StripSecretsUtils.java b/server-spi/src/main/java/org/keycloak/models/utils/StripSecretsUtils.java new file mode 100644 index 0000000000..06989a525f --- /dev/null +++ b/server-spi/src/main/java/org/keycloak/models/utils/StripSecretsUtils.java @@ -0,0 +1,69 @@ +/* + * 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.utils; + +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.provider.ProviderConfigProperty; +import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.IdentityProviderRepresentation; +import org.keycloak.representations.idm.RealmRepresentation; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; + +/** + * @author Stian Thorgersen + */ +public class StripSecretsUtils { + + public static ComponentRepresentation strip(KeycloakSession session, ComponentRepresentation rep) { + Map configProperties = ComponentUtil.getComponentConfigProperties(session, rep); + Iterator>> itr = rep.getConfig().entrySet().iterator(); + while (itr.hasNext()) { + Map.Entry> next = itr.next(); + ProviderConfigProperty configProperty = configProperties.get(next.getKey()); + if (configProperty != null) { + if (configProperty.isSecret()) { + next.setValue(Collections.singletonList(ComponentRepresentation.SECRET_VALUE)); + } + } else { + itr.remove(); + } + } + return rep; + } + + public static RealmRepresentation strip(RealmRepresentation rep) { + if (rep.getSmtpServer() != null && rep.getSmtpServer().containsKey("password")) { + rep.getSmtpServer().put("password", ComponentRepresentation.SECRET_VALUE); + } + return rep; + } + + public static IdentityProviderRepresentation strip(IdentityProviderRepresentation rep) { + if (rep.getConfig() != null && rep.getConfig().containsKey("clientSecret")) { + rep.getConfig().put("clientSecret", ComponentRepresentation.SECRET_VALUE); + } + return rep; + } + +} \ No newline at end of file diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java index 5a0e817af1..31341d3f44 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java @@ -26,6 +26,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.RepresentationToModel; +import org.keycloak.models.utils.StripSecretsUtils; import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.services.ErrorResponse; import org.keycloak.services.ErrorResponseException; @@ -119,7 +120,7 @@ public class ComponentResource { model = realm.addComponentModel(model); - adminEvent.operation(OperationType.CREATE).resourcePath(uriInfo, model.getId()).representation(rep).success(); + adminEvent.operation(OperationType.CREATE).resourcePath(uriInfo, model.getId()).representation(StripSecretsUtils.strip(session, rep)).success(); return Response.created(uriInfo.getAbsolutePathBuilder().path(model.getId()).build()).build(); } catch (ComponentValidationException e) { return localizedErrorResponse(e); @@ -149,7 +150,7 @@ public class ComponentResource { throw new NotFoundException("Could not find component"); } RepresentationToModel.updateComponent(session, rep, model, false); - adminEvent.operation(OperationType.UPDATE).resourcePath(uriInfo, model.getId()).representation(rep).success(); + adminEvent.operation(OperationType.UPDATE).resourcePath(uriInfo, model.getId()).representation(StripSecretsUtils.strip(session, rep)).success(); realm.updateComponent(model); return Response.noContent().build(); } catch (ComponentValidationException e) { diff --git a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java index a0e369f14a..af1a1eccaa 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java @@ -35,8 +35,10 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.RepresentationToModel; +import org.keycloak.models.utils.StripSecretsUtils; import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.provider.ProviderFactory; +import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.representations.idm.ConfigPropertyRepresentation; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; import org.keycloak.representations.idm.IdentityProviderMapperTypeRepresentation; @@ -101,7 +103,7 @@ public class IdentityProviderResource { } IdentityProviderRepresentation rep = ModelToRepresentation.toRepresentation(realm, this.identityProviderModel); - return rep; + return StripSecretsUtils.strip(rep); } /** @@ -152,12 +154,18 @@ public class IdentityProviderResource { } } - public static void updateIdpFromRep(IdentityProviderRepresentation providerRep, RealmModel realm, KeycloakSession session) { + private void updateIdpFromRep(IdentityProviderRepresentation providerRep, RealmModel realm, KeycloakSession session) { String internalId = providerRep.getInternalId(); String newProviderId = providerRep.getAlias(); String oldProviderId = getProviderIdByInternalId(realm, internalId); - realm.updateIdentityProvider(RepresentationToModel.toModel(realm, providerRep)); + IdentityProviderModel updated = RepresentationToModel.toModel(realm, providerRep); + + if (updated.getConfig() != null && ComponentRepresentation.SECRET_VALUE.equals(updated.getConfig().get("clientSecret"))) { + updated.getConfig().put("clientSecret", identityProviderModel.getConfig() != null ? identityProviderModel.getConfig().get("clientSecret") : null); + } + + realm.updateIdentityProvider(updated); if (oldProviderId != null && !oldProviderId.equals(newProviderId)) { diff --git a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java index 05f1f9e938..60e10759ed 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java @@ -33,6 +33,7 @@ import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.RepresentationToModel; +import org.keycloak.models.utils.StripSecretsUtils; import org.keycloak.provider.ProviderFactory; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.services.ErrorResponse; @@ -167,7 +168,7 @@ public class IdentityProvidersResource { List representations = new ArrayList(); for (IdentityProviderModel identityProviderModel : realm.getIdentityProviders()) { - representations.add(ModelToRepresentation.toRepresentation(realm, identityProviderModel)); + representations.add(StripSecretsUtils.strip(ModelToRepresentation.toRepresentation(realm, identityProviderModel))); } return representations; } @@ -191,7 +192,7 @@ public class IdentityProvidersResource { representation.setInternalId(identityProvider.getInternalId()); adminEvent.operation(OperationType.CREATE).resourcePath(uriInfo, identityProvider.getAlias()) - .representation(representation).success(); + .representation(StripSecretsUtils.strip(representation)).success(); return Response.created(uriInfo.getAbsolutePathBuilder().path(representation.getAlias()).build()).build(); } catch (ModelDuplicateException e) { diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java index 47db45e7bb..05b4b9b2f5 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java @@ -49,6 +49,7 @@ import org.keycloak.models.cache.UserCache; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.RepresentationToModel; +import org.keycloak.models.utils.StripSecretsUtils; import org.keycloak.partialimport.PartialImportManager; import org.keycloak.protocol.oidc.TokenManager; import org.keycloak.provider.ProviderFactory; @@ -309,7 +310,7 @@ public class RealmAdminResource { usersSyncManager.notifyToRefreshPeriodicSync(session, realm, fedProvider, false); } - adminEvent.operation(OperationType.UPDATE).representation(rep).success(); + adminEvent.operation(OperationType.UPDATE).representation(StripSecretsUtils.strip(rep)).success(); return Response.noContent().build(); } catch (PatternSyntaxException e) { return ErrorResponse.error("Specified regex pattern(s) is invalid.", Response.Status.BAD_REQUEST); diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java index 97980360d3..24e8b83e8a 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java @@ -37,6 +37,7 @@ import org.keycloak.keys.KeyProviderFactory; import org.keycloak.models.AuthenticationFlowModel; import org.keycloak.models.ClientModel; import org.keycloak.models.FederatedIdentityModel; +import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RealmProvider; @@ -638,6 +639,20 @@ public class TestingResourceProvider implements RealmResourceProvider { return reps; } + @GET + @Path("/smtp-config") + @Produces(MediaType.APPLICATION_JSON) + public Map getSmtpConfig() { + return session.getContext().getRealm().getSmtpConfig(); + } + + @GET + @Path("/identity-config") + @Produces(MediaType.APPLICATION_JSON) + public Map getIdentityProviderConfig(@QueryParam("alias") String alias) { + return session.getContext().getRealm().getIdentityProviderByAlias(alias).getConfig(); + } + private RealmModel getRealmByName(String realmName) { RealmProvider realmProvider = session.getProvider(RealmProvider.class); return realmProvider.getRealmByName(realmName); diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestingResource.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestingResource.java index 50352d43c2..e19653a08e 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestingResource.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestingResource.java @@ -242,4 +242,14 @@ public interface TestingResource { @Produces(MediaType.APPLICATION_JSON) Map getTestComponentDetails(); + @GET + @Path("/smtp-config") + @Produces(MediaType.APPLICATION_JSON) + Map getSmtpConfig(); + + @GET + @Path("/identity-config") + @Produces(MediaType.APPLICATION_JSON) + Map getIdentityProviderConfig(@QueryParam("alias") String alias); + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ComponentsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ComponentsTest.java index dba19dd538..409a3e1e6f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ComponentsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ComponentsTest.java @@ -21,6 +21,7 @@ import org.junit.Before; import org.junit.Test; import org.keycloak.admin.client.resource.ComponentsResource; import org.keycloak.common.util.MultivaluedHashMap; +import org.keycloak.representations.idm.AdminEventRepresentation; import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.representations.idm.ErrorRepresentation; import org.keycloak.testsuite.components.TestProvider; @@ -155,6 +156,11 @@ public class ComponentsTest extends AbstractAdminTest { ComponentRepresentation returned = components.component(id).toRepresentation(); assertEquals(ComponentRepresentation.SECRET_VALUE, returned.getConfig().getFirst("secret")); + // Check secret not leaked in admin events + AdminEventRepresentation event = testingClient.testing().pollAdminEvent(); + assertFalse(event.getRepresentation().contains("some secret value!!")); + assertTrue(event.getRepresentation().contains(ComponentRepresentation.SECRET_VALUE)); + Map details = testingClient.testing(REALM_NAME).getTestComponentDetails(); // Check value is set correctly @@ -166,6 +172,11 @@ public class ComponentsTest extends AbstractAdminTest { ComponentRepresentation returned2 = components.component(id).toRepresentation(); assertEquals(ComponentRepresentation.SECRET_VALUE, returned2.getConfig().getFirst("secret")); + // Check secret not leaked in admin events + event = testingClient.testing().pollAdminEvent(); + assertFalse(event.getRepresentation().contains("some secret value!!")); + assertTrue(event.getRepresentation().contains(ComponentRepresentation.SECRET_VALUE)); + // Check secret value is not set to '*********' details = testingClient.testing(REALM_NAME).getTestComponentDetails(); assertEquals("some secret value!!", details.get("mycomponent").getConfig().get("secret").get(0)); @@ -176,6 +187,9 @@ public class ComponentsTest extends AbstractAdminTest { // Check secret value is updated details = testingClient.testing(REALM_NAME).getTestComponentDetails(); assertEquals("updated secret value!!", details.get("mycomponent").getConfig().get("secret").get(0)); + + ComponentRepresentation returned3 = components.query().stream().filter(c -> c.getId().equals(returned2.getId())).findFirst().get(); + assertEquals(ComponentRepresentation.SECRET_VALUE, returned3.getConfig().getFirst("secret")); } private String createComponent(ComponentRepresentation rep) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java index c9c4191b6c..e3392b3a6e 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java @@ -27,6 +27,10 @@ import org.keycloak.dom.saml.v2.metadata.KeyTypes; import org.keycloak.dom.saml.v2.metadata.SPSSODescriptorType; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; +import org.keycloak.models.utils.StripSecretsUtils; +import org.keycloak.representations.idm.AdminEventRepresentation; +import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; import org.keycloak.representations.idm.IdentityProviderMapperTypeRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; @@ -78,7 +82,7 @@ public class IdentityProviderTest extends AbstractAdminTest { IdentityProviderRepresentation newIdentityProvider = createRep("new-identity-provider", "oidc"); newIdentityProvider.getConfig().put("clientId", "clientId"); - newIdentityProvider.getConfig().put("clientSecret", "clientSecret"); + newIdentityProvider.getConfig().put("clientSecret", "some secret value"); create(newIdentityProvider); @@ -94,10 +98,17 @@ public class IdentityProviderTest extends AbstractAdminTest { assertEquals("new-identity-provider", representation.getAlias()); assertEquals("oidc", representation.getProviderId()); assertEquals("clientId", representation.getConfig().get("clientId")); - assertEquals("clientSecret", representation.getConfig().get("clientSecret")); + assertEquals(ComponentRepresentation.SECRET_VALUE, representation.getConfig().get("clientSecret")); assertTrue(representation.isEnabled()); assertFalse(representation.isStoreToken()); assertFalse(representation.isTrustEmail()); + + testingClient.testing("admin-client-test").getSmtpConfig(); + + assertEquals("some secret value", testingClient.testing("admin-client-test").getIdentityProviderConfig("new-identity-provider").get("clientSecret")); + + IdentityProviderRepresentation rep = realm.identityProviders().findAll().stream().filter(i -> i.getAlias().equals("new-identity-provider")).findFirst().get(); + assertEquals(ComponentRepresentation.SECRET_VALUE, rep.getConfig().get("clientSecret")); } @Test @@ -105,7 +116,7 @@ public class IdentityProviderTest extends AbstractAdminTest { IdentityProviderRepresentation newIdentityProvider = createRep("update-identity-provider", "oidc"); newIdentityProvider.getConfig().put("clientId", "clientId"); - newIdentityProvider.getConfig().put("clientSecret", "clientSecret"); + newIdentityProvider.getConfig().put("clientSecret", "some secret value"); create(newIdentityProvider); @@ -125,7 +136,9 @@ public class IdentityProviderTest extends AbstractAdminTest { representation.getConfig().put("clientId", "changedClientId"); identityProviderResource.update(representation); - assertAdminEvents.assertEvent(realmId, OperationType.UPDATE, AdminEventPaths.identityProviderPath("update-identity-provider"), representation, ResourceType.IDENTITY_PROVIDER); + AdminEventRepresentation event = assertAdminEvents.assertEvent(realmId, OperationType.UPDATE, AdminEventPaths.identityProviderPath("update-identity-provider"), representation, ResourceType.IDENTITY_PROVIDER); + assertFalse(event.getRepresentation().contains("some secret value")); + assertTrue(event.getRepresentation().contains(ComponentRepresentation.SECRET_VALUE)); identityProviderResource = realm.identityProviders().get(representation.getInternalId()); @@ -136,6 +149,8 @@ public class IdentityProviderTest extends AbstractAdminTest { assertFalse(representation.isEnabled()); assertTrue(representation.isStoreToken()); assertEquals("changedClientId", representation.getConfig().get("clientId")); + + assertEquals("some secret value", testingClient.testing("admin-client-test").getIdentityProviderConfig("changed-alias").get("clientSecret")); } @Test @@ -168,7 +183,14 @@ public class IdentityProviderTest extends AbstractAdminTest { Assert.assertNotNull(ApiUtil.getCreatedId(response)); response.close(); + String secret = idpRep.getConfig() != null ? idpRep.getConfig().get("clientSecret") : null; + idpRep = StripSecretsUtils.strip(idpRep); + assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.identityProviderPath(idpRep.getAlias()), idpRep, ResourceType.IDENTITY_PROVIDER); + + if (secret != null) { + idpRep.getConfig().put("clientSecret", secret); + } } private IdentityProviderRepresentation createRep(String id, String providerId) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java index 7621f14242..d793a8c6c5 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java @@ -31,7 +31,10 @@ import org.keycloak.events.admin.ResourceType; import org.keycloak.models.Constants; import org.keycloak.representations.adapters.action.GlobalRequestResult; import org.keycloak.representations.adapters.action.PushNotBeforeAction; +import org.keycloak.representations.idm.AdminEventRepresentation; import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RoleRepresentation; @@ -45,6 +48,7 @@ import org.keycloak.testsuite.auth.page.AuthRealm; import org.keycloak.testsuite.util.AdminEventPaths; import org.keycloak.testsuite.util.CredentialBuilder; import org.keycloak.testsuite.util.OAuthClient.AccessTokenResponse; +import org.keycloak.testsuite.util.RealmBuilder; import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.util.JsonSerialization; @@ -124,6 +128,34 @@ public class RealmTest extends AbstractAdminTest { Assert.assertNames(adminClient.realms().findAll(), "master", AuthRealm.TEST, REALM_NAME); } + @Test + public void smtpPasswordSecret() { + RealmRepresentation rep = RealmBuilder.create().testEventListener().testMail().build(); + rep.setRealm("realm-with-smtp"); + rep.getSmtpServer().put("user", "user"); + rep.getSmtpServer().put("password", "secret"); + + adminClient.realms().create(rep); + + RealmRepresentation returned = adminClient.realm("realm-with-smtp").toRepresentation(); + assertEquals(ComponentRepresentation.SECRET_VALUE, returned.getSmtpServer().get("password")); + + assertEquals("secret", testingClient.testing("realm-with-smtp").getSmtpConfig().get("password")); + + adminClient.realm("realm-with-smtp").update(rep); + + AdminEventRepresentation event = testingClient.testing().pollAdminEvent(); + assertFalse(event.getRepresentation().contains("some secret value!!")); + assertTrue(event.getRepresentation().contains(ComponentRepresentation.SECRET_VALUE)); + + assertEquals("secret", testingClient.testing("realm-with-smtp").getSmtpConfig().get("password")); + + RealmRepresentation realm = adminClient.realms().findAll().stream().filter(r -> r.getRealm().equals("realm-with-smtp")).findFirst().get(); + assertEquals(ComponentRepresentation.SECRET_VALUE, realm.getSmtpServer().get("password")); + + adminClient.realm("realm-with-smtp").remove(); + } + @Test public void createRealmCheckDefaultPasswordPolicy() { RealmRepresentation rep = new RealmRepresentation();