Make sure admin events are not referencing sensitive data from their representation

Closes #21562

Signed-off-by: Joerg Matysiak <joerg.matysiak@bosch.com>
This commit is contained in:
Pedro Igor 2023-07-10 11:40:34 -03:00
parent 0be34d64e7
commit 7483bae130
13 changed files with 63 additions and 38 deletions

View file

@ -62,6 +62,7 @@ import java.util.stream.Collectors;
import java.util.stream.Stream;
import static org.keycloak.models.utils.ModelToRepresentation.toRepresentation;
import static org.keycloak.models.utils.StripSecretsUtils.stripSecrets;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@ -255,7 +256,7 @@ public class ExportUtils {
// Message Bundle
rep.setLocalizationTexts(realm.getRealmLocalizationTexts());
return rep;
return stripSecrets(session, rep);
}
public static MultivaluedHashMap<String, ComponentExportRepresentation> exportComponents(RealmModel realm, String parentId) {

View file

@ -123,7 +123,6 @@ import static org.keycloak.models.utils.RepresentationToModel.createGroups;
import static org.keycloak.models.utils.RepresentationToModel.createRoleMappings;
import static org.keycloak.models.utils.RepresentationToModel.importGroup;
import static org.keycloak.models.utils.RepresentationToModel.importRoles;
import static org.keycloak.models.utils.StripSecretsUtils.stripForExport;
/**
* This wraps the functionality about export/import for the storage.
@ -143,7 +142,6 @@ public class DefaultExportImportManager implements ExportImportManager {
callback.setType(MediaType.APPLICATION_JSON);
callback.writeToOutputStream(outputStream -> {
RealmRepresentation rep = ExportUtils.exportRealm(session, realm, options, false);
stripForExport(session, rep);
JsonSerialization.writeValueToStream(outputStream, rep);
outputStream.close();
});

View file

@ -17,6 +17,8 @@
package org.keycloak.models.utils;
import static org.keycloak.models.utils.StripSecretsUtils.stripSecrets;
import org.jboss.logging.Logger;
import org.keycloak.authentication.otp.OTPApplicationProvider;
import org.keycloak.authorization.AuthorizationProvider;
@ -496,7 +498,7 @@ public class ModelToRepresentation {
}
List<IdentityProviderRepresentation> identityProviders = realm.getIdentityProvidersStream()
.map(provider -> toRepresentation(realm, provider)).collect(Collectors.toList());
.map(provider -> toRepresentation(session, realm, provider)).collect(Collectors.toList());
rep.setIdentityProviders(identityProviders);
List<IdentityProviderMapperRepresentation> identityProviderMappers = realm.getIdentityProviderMappersStream()
@ -518,7 +520,7 @@ public class ModelToRepresentation {
rep.getAttributes().putAll(stripRealmAttributesIncludedAsFields(realm.getAttributes()));
if (!internal) {
rep = StripSecretsUtils.strip(rep);
rep = stripSecrets(session, rep);
}
return rep;
@ -782,7 +784,7 @@ public class ModelToRepresentation {
return providerRep;
}
public static IdentityProviderRepresentation toRepresentation(RealmModel realm, IdentityProviderModel identityProviderModel) {
public static IdentityProviderRepresentation toRepresentation(KeycloakSession session, RealmModel realm, IdentityProviderModel identityProviderModel) {
IdentityProviderRepresentation providerRep = toBriefRepresentation(realm, identityProviderModel);
providerRep.setLinkOnly(identityProviderModel.isLinkOnly());
@ -816,7 +818,7 @@ public class ModelToRepresentation {
providerRep.setPostBrokerLoginFlowAlias(flow.getAlias());
}
return providerRep;
return stripSecrets(session, providerRep);
}
public static ProtocolMapperRepresentation toRepresentation(ProtocolMapperModel model) {
@ -936,7 +938,7 @@ public class ModelToRepresentation {
public static ComponentRepresentation toRepresentation(KeycloakSession session, ComponentModel component, boolean internal) {
ComponentRepresentation rep = toRepresentationWithoutConfig(component);
if (!internal) {
rep = StripSecretsUtils.strip(session, rep);
return stripSecrets(session, rep);
}
return rep;
}

View file

@ -29,9 +29,11 @@ import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
@ -42,6 +44,28 @@ public class StripSecretsUtils {
private static final Pattern VAULT_VALUE = Pattern.compile("^\\$\\{vault\\.(.+?)}$");
private static final Map<Class<?>, BiConsumer<KeycloakSession, Object>> REPRESENTATION_FORMATTER = new HashMap<>();
static {
REPRESENTATION_FORMATTER.put(RealmRepresentation.class, (session, o) -> StripSecretsUtils.stripRealm(session, (RealmRepresentation) o));
REPRESENTATION_FORMATTER.put(UserRepresentation.class, (session, o) -> StripSecretsUtils.stripUser((UserRepresentation) o));
REPRESENTATION_FORMATTER.put(ClientRepresentation.class, (session, o) -> StripSecretsUtils.stripClient((ClientRepresentation) o));
REPRESENTATION_FORMATTER.put(IdentityProviderRepresentation.class, (session, o) -> StripSecretsUtils.stripBroker((IdentityProviderRepresentation) o));
REPRESENTATION_FORMATTER.put(ComponentRepresentation.class, (session, o) -> StripSecretsUtils.stripComponent(session, (ComponentRepresentation) o));
}
public static <T> T stripSecrets(KeycloakSession session, T representation) {
BiConsumer<KeycloakSession, Object> formatter = REPRESENTATION_FORMATTER.get(representation.getClass());
if (formatter == null) {
return representation;
}
formatter.accept(session, representation);
return representation;
}
private static String maskNonVaultValue(String value) {
return value == null
? null
@ -51,7 +75,7 @@ public class StripSecretsUtils {
);
}
public static ComponentRepresentation strip(KeycloakSession session, ComponentRepresentation rep) {
private static ComponentRepresentation stripComponent(KeycloakSession session, ComponentRepresentation rep) {
Map<String, ProviderConfigProperty> configProperties = ComponentUtil.getComponentConfigProperties(session, rep);
if (rep.getConfig() == null) {
return rep;
@ -76,33 +100,33 @@ public class StripSecretsUtils {
return rep;
}
public static RealmRepresentation strip(RealmRepresentation rep) {
private static RealmRepresentation stripRealm(RealmRepresentation rep) {
if (rep.getSmtpServer() != null && rep.getSmtpServer().containsKey("password")) {
rep.getSmtpServer().put("password", maskNonVaultValue(rep.getSmtpServer().get("password")));
}
return rep;
}
public static IdentityProviderRepresentation strip(IdentityProviderRepresentation rep) {
private static IdentityProviderRepresentation stripBroker(IdentityProviderRepresentation rep) {
if (rep.getConfig() != null && rep.getConfig().containsKey("clientSecret")) {
rep.getConfig().put("clientSecret", maskNonVaultValue(rep.getConfig().get("clientSecret")));
}
return rep;
}
public static void stripForExport(KeycloakSession session, RealmRepresentation rep) {
strip(rep);
private static void stripRealm(KeycloakSession session, RealmRepresentation rep) {
stripRealm(rep);
List<ClientRepresentation> clients = rep.getClients();
if (clients != null) {
for (ClientRepresentation c : clients) {
strip(c);
stripClient(c);
}
}
List<IdentityProviderRepresentation> providers = rep.getIdentityProviders();
if (providers != null) {
for (IdentityProviderRepresentation r : providers) {
strip(r);
stripBroker(r);
}
}
@ -110,7 +134,7 @@ public class StripSecretsUtils {
if (components != null) {
for (Map.Entry<String, List<ComponentExportRepresentation>> ent : components.entrySet()) {
for (ComponentExportRepresentation c : ent.getValue()) {
strip(session, ent.getKey(), c);
stripComponentExport(session, ent.getKey(), c);
}
}
}
@ -118,24 +142,24 @@ public class StripSecretsUtils {
List<UserRepresentation> users = rep.getUsers();
if (users != null) {
for (UserRepresentation u: users) {
strip(u);
stripUser(u);
}
}
users = rep.getFederatedUsers();
if (users != null) {
for (UserRepresentation u: users) {
strip(u);
stripUser(u);
}
}
}
public static UserRepresentation strip(UserRepresentation user) {
private static UserRepresentation stripUser(UserRepresentation user) {
user.setCredentials(null);
return user;
}
public static ClientRepresentation strip(ClientRepresentation rep) {
private static ClientRepresentation stripClient(ClientRepresentation rep) {
if (rep.getSecret() != null) {
rep.setSecret(maskNonVaultValue(rep.getSecret()));
}
@ -148,7 +172,7 @@ public class StripSecretsUtils {
return rep;
}
public static ComponentExportRepresentation strip(KeycloakSession session, String providerType, ComponentExportRepresentation rep) {
private static ComponentExportRepresentation stripComponentExport(KeycloakSession session, String providerType, ComponentExportRepresentation rep) {
Map<String, ProviderConfigProperty> configProperties = ComponentUtil.getComponentConfigProperties(session, providerType, rep.getProviderId());
if (rep.getConfig() == null) {
return rep;
@ -174,7 +198,7 @@ public class StripSecretsUtils {
MultivaluedHashMap<String, ComponentExportRepresentation> sub = rep.getSubComponents();
for (Map.Entry<String, List<ComponentExportRepresentation>> ent: sub.entrySet()) {
for (ComponentExportRepresentation c: ent.getValue()) {
strip(session, ent.getKey(), c);
stripComponentExport(session, ent.getKey(), c);
}
}
return rep;

View file

@ -31,14 +31,14 @@ public class StripSecretsUtilsTest {
@Test
public void checkStrippedRotatedSecret() {
ClientRepresentation stripped = StripSecretsUtils.strip(createClient("unmasked_secret"));
ClientRepresentation stripped = StripSecretsUtils.stripSecrets(null, createClient("unmasked_secret"));
assertEquals(ComponentRepresentation.SECRET_VALUE, getRotatedSecret(stripped));
}
@Test
public void checkStrippedRotatedSecretVaultUnaffected() {
String rotatedSecret = "${vault.key}";
ClientRepresentation stripped = StripSecretsUtils.strip(createClient(rotatedSecret));
ClientRepresentation stripped = StripSecretsUtils.stripSecrets(null, createClient(rotatedSecret));
assertEquals(rotatedSecret, getRotatedSecret(stripped));
}

View file

@ -16,6 +16,8 @@
*/
package org.keycloak.services.resources.admin;
import static org.keycloak.models.utils.StripSecretsUtils.stripSecrets;
import org.jboss.logging.Logger;
import org.keycloak.common.ClientConnection;
import org.keycloak.common.util.Time;
@ -49,6 +51,7 @@ public class AdminEventBuilder {
private final RealmModel realm;
private final AdminEvent adminEvent;
private final Map<String, EventListenerProvider> listeners;
private final KeycloakSession session;
private EventStoreProvider store;
@ -74,6 +77,7 @@ public class AdminEventBuilder {
authUser(auth.getUser());
authIpAddress(ipAddress);
}
this.session = session;
}
/**
@ -253,9 +257,7 @@ public class AdminEventBuilder {
return this;
}
if (value instanceof UserRepresentation) {
StripSecretsUtils.strip((UserRepresentation) value);
}
stripSecrets(session, value);
try {
adminEvent.setRepresentation(JsonSerialization.writeValueAsString(value));

View file

@ -140,7 +140,7 @@ public class ComponentResource {
model = realm.addComponentModel(model);
adminEvent.operation(OperationType.CREATE).resourcePath(session.getContext().getUri(), model.getId()).representation(StripSecretsUtils.strip(session, rep)).success();
adminEvent.operation(OperationType.CREATE).resourcePath(session.getContext().getUri(), model.getId()).representation(rep).success();
return Response.created(session.getContext().getUri().getAbsolutePathBuilder().path(model.getId()).build()).build();
} catch (ComponentValidationException e) {
return localizedErrorResponse(e);
@ -178,7 +178,7 @@ public class ComponentResource {
throw new NotFoundException("Could not find component");
}
RepresentationToModel.updateComponent(session, rep, model, false);
adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()).representation(StripSecretsUtils.strip(session, rep)).success();
adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()).representation(rep).success();
realm.updateComponent(model);
return Response.noContent().build();
} catch (ComponentValidationException e) {

View file

@ -113,8 +113,7 @@ public class IdentityProviderResource {
throw new jakarta.ws.rs.NotFoundException();
}
IdentityProviderRepresentation rep = ModelToRepresentation.toRepresentation(realm, this.identityProviderModel);
return StripSecretsUtils.strip(rep);
return ModelToRepresentation.toRepresentation(session, realm, this.identityProviderModel);
}
/**

View file

@ -191,7 +191,7 @@ public class IdentityProvidersResource {
Function<IdentityProviderModel, IdentityProviderRepresentation> toRepresentation = briefRepresentation != null && briefRepresentation
? m -> ModelToRepresentation.toBriefRepresentation(realm, m)
: m -> StripSecretsUtils.strip(ModelToRepresentation.toRepresentation(realm, m));
: m -> ModelToRepresentation.toRepresentation(session, realm, m);
Stream<IdentityProviderModel> stream = realm.getIdentityProvidersStream().sorted(new IdPComparator());
if (!StringUtil.isBlank(search)) {
@ -240,7 +240,7 @@ public class IdentityProvidersResource {
representation.setInternalId(identityProvider.getInternalId());
adminEvent.operation(OperationType.CREATE).resourcePath(session.getContext().getUri(), identityProvider.getAlias())
.representation(StripSecretsUtils.strip(representation)).success();
.representation(representation).success();
return Response.created(session.getContext().getUri().getAbsolutePathBuilder().path(representation.getAlias()).build()).build();
} catch (IllegalArgumentException e) {

View file

@ -457,7 +457,7 @@ public class RealmAdminResource {
// This populates the map in DefaultKeycloakContext to be used when treating the event
session.getContext().getUri();
adminEvent.operation(OperationType.UPDATE).representation(StripSecretsUtils.strip(rep)).success();
adminEvent.operation(OperationType.UPDATE).representation(rep).success();
if (rep.isDuplicateEmailsAllowed() != null && rep.isDuplicateEmailsAllowed() != wasDuplicateEmailsAllowed) {
session.invalidate(InvalidationHandler.ObjectType.REALM, realm.getId());

View file

@ -155,8 +155,7 @@ public class RealmsAdminResource {
AdminEventBuilder adminEvent = new AdminEventBuilder(auth.getRealm(), auth, session, clientConnection);
adminEvent.resource(ResourceType.REALM).realm(auth.getRealm().getId()).operation(OperationType.CREATE)
.resourcePath(realm.getName())
.representation(
StripSecretsUtils.strip(ModelToRepresentation.toRepresentation(session, realm, false)))
.representation(ModelToRepresentation.toRepresentation(session, realm, false))
.success();
return Response.created(location).build();

View file

@ -574,7 +574,7 @@ public class IdentityProviderTest extends AbstractAdminTest {
getCleanup().addIdentityProviderAlias(idpRep.getAlias());
String secret = idpRep.getConfig() != null ? idpRep.getConfig().get("clientSecret") : null;
idpRep = StripSecretsUtils.strip(idpRep);
idpRep = StripSecretsUtils.stripSecrets(null, idpRep);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.identityProviderPath(idpRep.getAlias()), idpRep, ResourceType.IDENTITY_PROVIDER);

View file

@ -243,7 +243,7 @@ public class UserTest extends AbstractAdminTest {
createdId = ApiUtil.getCreatedId(response);
}
StripSecretsUtils.strip(userRep);
StripSecretsUtils.stripSecrets(null, userRep);
if (assertAdminEvent) {
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.userResourcePath(createdId), userRep,