From 2ded3f723698e6c3c3f4771cd233abd8599a91a4 Mon Sep 17 00:00:00 2001 From: Alex Morel Date: Thu, 20 Jun 2024 16:00:14 +0200 Subject: [PATCH] Update SCIM clients when configuration changes and improve ScimDispatcher lifecycle --- .../sh/libre/scim/core/GroupScimClient.java | 2 +- .../sh/libre/scim/core/ScimDispatcher.java | 74 ++++++++++++++----- .../scim/core/ScrimProviderConfiguration.java | 42 ++++++----- .../scim/event/ScimEventListenerProvider.java | 48 ++++++++++-- .../scim/jpa/ScimResourceProviderFactory.java | 13 +++- .../storage/ScimStorageProviderFactory.java | 11 ++- 6 files changed, 137 insertions(+), 53 deletions(-) diff --git a/src/main/java/sh/libre/scim/core/GroupScimClient.java b/src/main/java/sh/libre/scim/core/GroupScimClient.java index b4c911d864..c1f09e1c28 100644 --- a/src/main/java/sh/libre/scim/core/GroupScimClient.java +++ b/src/main/java/sh/libre/scim/core/GroupScimClient.java @@ -37,6 +37,6 @@ public class GroupScimClient implements ScimClientInterface { @Override public void close() throws Exception { - throw new UnsupportedOperationException(); + } } diff --git a/src/main/java/sh/libre/scim/core/ScimDispatcher.java b/src/main/java/sh/libre/scim/core/ScimDispatcher.java index 9501586d3e..b9070fbfe1 100644 --- a/src/main/java/sh/libre/scim/core/ScimDispatcher.java +++ b/src/main/java/sh/libre/scim/core/ScimDispatcher.java @@ -6,7 +6,9 @@ import org.keycloak.models.KeycloakSession; import sh.libre.scim.storage.ScimStorageProviderFactory; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.function.Consumer; @@ -17,13 +19,21 @@ public class ScimDispatcher { private static final Logger logger = Logger.getLogger(ScimDispatcher.class); + private static final Map sessionToScimDispatcher = new LinkedHashMap<>(); private final KeycloakSession session; + private boolean clientsInitialized = false; private final List userScimClients = new ArrayList<>(); private final List groupScimClients = new ArrayList<>(); - public ScimDispatcher(KeycloakSession session) { + + public static ScimDispatcher createForSession(KeycloakSession session) { + // Only create a scim dispatcher if there is none already created for session + sessionToScimDispatcher.computeIfAbsent(session, ScimDispatcher::new); + return sessionToScimDispatcher.get(session); + } + + private ScimDispatcher(KeycloakSession session) { this.session = session; - refreshActiveScimEndpoints(); } /** @@ -35,23 +45,30 @@ public class ScimDispatcher { for (GroupScimClient c : groupScimClients) { c.close(); } + groupScimClients.clear(); for (UserScimClient c : userScimClients) { c.close(); } + userScimClients.clear(); // Step 2: Get All SCIM endpoints defined in Admin Console (enabled ScimStorageProviderFactory) session.getContext().getRealm().getComponentsStream() .filter(m -> ScimStorageProviderFactory.ID.equals(m.getProviderId()) && m.get("enabled", true)) .forEach(scimEndpoint -> { - // Step 3 : create scim clients for each endpoint - if (scimEndpoint.get(ScrimProviderConfiguration.CONF_KEY_PROPAGATION_GROUP, false)) { - GroupScimClient groupScimClient = GroupScimClient.newGroupScimClient(scimEndpoint, session); - groupScimClients.add(groupScimClient); - } - if (scimEndpoint.get(ScrimProviderConfiguration.CONF_KEY_PROPAGATION_USER, false)) { - UserScimClient userScimClient = UserScimClient.newUserScimClient(scimEndpoint, session); - userScimClients.add(userScimClient); + try { + // Step 3 : create scim clients for each endpoint + if (scimEndpoint.get(ScrimProviderConfiguration.CONF_KEY_PROPAGATION_GROUP, false)) { + GroupScimClient groupScimClient = GroupScimClient.newGroupScimClient(scimEndpoint, session); + groupScimClients.add(groupScimClient); + } + if (scimEndpoint.get(ScrimProviderConfiguration.CONF_KEY_PROPAGATION_USER, false)) { + UserScimClient userScimClient = UserScimClient.newUserScimClient(scimEndpoint, session); + userScimClients.add(userScimClient); + } + } catch (Exception e) { + logger.warnf("[SCIM] Invalid Endpoint configuration %s : %s", scimEndpoint.getId(), e.getMessage()); + // TODO is it ok to log and try to create the other clients ? } }); } catch (Exception e) { @@ -61,25 +78,24 @@ public class ScimDispatcher { } public void dispatchUserModificationToAll(Consumer operationToDispatch) { - // TODO should not be required to launch a refresh here : we should refresh clients only if an endpoint configuration changes - refreshActiveScimEndpoints(); + initializeClientsIfNeeded(); userScimClients.forEach(operationToDispatch); + logger.infof("[SCIM] User operation dispatched to %d SCIM clients", userScimClients.size()); } public void dispatchGroupModificationToAll(Consumer operationToDispatch) { - // TODO should not be required to launch a refresh here : we should refresh clients only if an endpoint configuration changes - refreshActiveScimEndpoints(); + initializeClientsIfNeeded(); groupScimClients.forEach(operationToDispatch); + logger.infof("[SCIM] Group operation dispatched to %d SCIM clients", groupScimClients.size()); } public void dispatchUserModificationToOne(ComponentModel scimServerConfiguration, Consumer operationToDispatch) { - // TODO should not be required to launch a refresh here : we should refresh clients only if an endpoint configuration changes - refreshActiveScimEndpoints(); - + initializeClientsIfNeeded(); // Scim client should already have been created Optional matchingClient = userScimClients.stream().filter(u -> u.getConfiguration().getId().equals(scimServerConfiguration.getId())).findFirst(); if (matchingClient.isPresent()) { operationToDispatch.accept(matchingClient.get()); + logger.infof("[SCIM] User operation dispatched to SCIM client %s", matchingClient.get().getConfiguration().getId()); } else { logger.error("[SCIM] Could not find a Scim Client matching endpoint configuration" + scimServerConfiguration.getId()); } @@ -87,15 +103,33 @@ public class ScimDispatcher { public void dispatchGroupModificationToOne(ComponentModel scimServerConfiguration, Consumer operationToDispatch) { - // TODO should not be required to launch a refresh here : we should refresh clients only if an endpoint configuration changes - refreshActiveScimEndpoints(); - + initializeClientsIfNeeded(); // Scim client should already have been created Optional matchingClient = groupScimClients.stream().filter(u -> u.getConfiguration().getId().equals(scimServerConfiguration.getId())).findFirst(); if (matchingClient.isPresent()) { operationToDispatch.accept(matchingClient.get()); + logger.infof("[SCIM] Group operation dispatched to SCIM client %s", matchingClient.get().getConfiguration().getId()); } else { logger.error("[SCIM] Could not find a Scim Client matching endpoint configuration" + scimServerConfiguration.getId()); } } + + public void close() throws Exception { + sessionToScimDispatcher.remove(session); + for (GroupScimClient c : groupScimClients) { + c.close(); + } + for (UserScimClient c : userScimClients) { + c.close(); + } + groupScimClients.clear(); + userScimClients.clear(); + } + + private void initializeClientsIfNeeded() { + if (!clientsInitialized) { + clientsInitialized = true; + refreshActiveScimEndpoints(); + } + } } diff --git a/src/main/java/sh/libre/scim/core/ScrimProviderConfiguration.java b/src/main/java/sh/libre/scim/core/ScrimProviderConfiguration.java index cec792def0..6cd5431dc4 100644 --- a/src/main/java/sh/libre/scim/core/ScrimProviderConfiguration.java +++ b/src/main/java/sh/libre/scim/core/ScrimProviderConfiguration.java @@ -25,25 +25,29 @@ public class ScrimProviderConfiguration { private final boolean syncRefresh; public ScrimProviderConfiguration(ComponentModel scimProviderConfiguration) { - AuthMode authMode = AuthMode.valueOf(scimProviderConfiguration.get(CONF_KEY_AUTH_MODE)); - authorizationHeaderValue = switch (authMode) { - case BEARER -> "Bearer " + scimProviderConfiguration.get(CONF_KEY_AUTH_PASSWORD); - case BASIC_AUTH -> { - BasicAuth basicAuth = BasicAuth.builder() - .username(scimProviderConfiguration.get(CONF_KEY_AUTH_USER)) - .password(scimProviderConfiguration.get(CONF_KEY_AUTH_PASSWORD)) - .build(); - yield basicAuth.getAuthorizationHeaderValue(); - } - default -> - throw new IllegalArgumentException("authMode " + scimProviderConfiguration + " is not supported"); - }; - contentType = scimProviderConfiguration.get(CONF_KEY_CONTENT_TYPE); - endPoint = scimProviderConfiguration.get(CONF_KEY_ENDPOINT); - id = scimProviderConfiguration.getId(); - importAction = ImportAction.valueOf(scimProviderConfiguration.get(CONF_KEY_SYNC_IMPORT_ACTION)); - syncImport = scimProviderConfiguration.get(CONF_KEY_SYNC_IMPORT, false); - syncRefresh = scimProviderConfiguration.get(CONF_KEY_SYNC_REFRESH, false); + try { + AuthMode authMode = AuthMode.valueOf(scimProviderConfiguration.get(CONF_KEY_AUTH_MODE)); + + authorizationHeaderValue = switch (authMode) { + case BEARER -> "Bearer " + scimProviderConfiguration.get(CONF_KEY_AUTH_PASSWORD); + case BASIC_AUTH -> { + BasicAuth basicAuth = BasicAuth.builder() + .username(scimProviderConfiguration.get(CONF_KEY_AUTH_USER)) + .password(scimProviderConfiguration.get(CONF_KEY_AUTH_PASSWORD)) + .build(); + yield basicAuth.getAuthorizationHeaderValue(); + } + case NONE -> ""; + }; + contentType = scimProviderConfiguration.get(CONF_KEY_CONTENT_TYPE, ""); + endPoint = scimProviderConfiguration.get(CONF_KEY_ENDPOINT, ""); + id = scimProviderConfiguration.getId(); + importAction = ImportAction.valueOf(scimProviderConfiguration.get(CONF_KEY_SYNC_IMPORT_ACTION)); + syncImport = scimProviderConfiguration.get(CONF_KEY_SYNC_IMPORT, false); + syncRefresh = scimProviderConfiguration.get(CONF_KEY_SYNC_REFRESH, false); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("authMode '" + scimProviderConfiguration.get(CONF_KEY_AUTH_MODE) + "' is not supported"); + } } public boolean isSyncRefresh() { diff --git a/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java b/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java index abfc3da89c..6463853843 100644 --- a/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java +++ b/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java @@ -6,6 +6,7 @@ import org.keycloak.events.Event; import org.keycloak.events.EventListenerProvider; import org.keycloak.events.EventType; import org.keycloak.events.admin.AdminEvent; +import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; import org.keycloak.models.GroupModel; import org.keycloak.models.KeycloakSession; @@ -33,12 +34,13 @@ public class ScimEventListenerProvider implements EventListenerProvider { ResourceType.USER, Pattern.compile("users/(.+)"), ResourceType.GROUP, Pattern.compile("groups/([\\w-]+)(/children)?"), ResourceType.GROUP_MEMBERSHIP, Pattern.compile("users/(.+)/groups/(.+)"), - ResourceType.REALM_ROLE_MAPPING, Pattern.compile("^(.+)/(.+)/role-mappings") + ResourceType.REALM_ROLE_MAPPING, Pattern.compile("^(.+)/(.+)/role-mappings"), + ResourceType.COMPONENT, Pattern.compile("components/(.+)") ); public ScimEventListenerProvider(KeycloakSession session) { this.session = session; - dispatcher = new ScimDispatcher(session); + this.dispatcher = ScimDispatcher.createForSession(session); } @@ -47,23 +49,28 @@ public class ScimEventListenerProvider implements EventListenerProvider { // React to User-related event : creation, deletion, update EventType eventType = event.getType(); String eventUserId = event.getUserId(); - LOGGER.infof("[SCIM] Propagate User Event %s - %s", eventType, eventUserId); switch (eventType) { case REGISTER -> { + LOGGER.infof("[SCIM] Propagate User Registration - %s", eventUserId); UserModel user = getUser(eventUserId); dispatcher.dispatchUserModificationToAll(client -> client.create(user)); } case UPDATE_EMAIL, UPDATE_PROFILE -> { + LOGGER.infof("[SCIM] Propagate User %s - %s", eventType, eventUserId); UserModel user = getUser(eventUserId); dispatcher.dispatchUserModificationToAll(client -> client.replace(user)); } - case DELETE_ACCOUNT -> dispatcher.dispatchUserModificationToAll(client -> client.delete(eventUserId)); + case DELETE_ACCOUNT -> { + LOGGER.infof("[SCIM] Propagate User deletion - %s", eventUserId); + dispatcher.dispatchUserModificationToAll(client -> client.delete(eventUserId)); + } default -> { // No other event has to be propagated to Scim endpoints } } } + @Override public void onEvent(AdminEvent event, boolean includeRepresentation) { // Step 1: check if event is relevant for propagation through SCIM @@ -74,6 +81,7 @@ public class ScimEventListenerProvider implements EventListenerProvider { if (!matcher.find()) return; + // Step 2: propagate event (if needed) according to its resource type switch (event.getResourceType()) { case USER -> { @@ -94,12 +102,18 @@ public class ScimEventListenerProvider implements EventListenerProvider { String id = matcher.group(2); handleRoleMappingEvent(event, type, id); } + case COMPONENT -> { + String id = matcher.group(1); + handleScimEndpointConfigurationEvent(event, id); + + } default -> { // No other resource modification has to be propagated to Scim endpoints } } } + private void handleUserEvent(AdminEvent userEvent, String userId) { LOGGER.infof("[SCIM] Propagate User %s - %s", userEvent.getOperationType(), userId); switch (userEvent.getOperationType()) { @@ -174,6 +188,25 @@ public class ScimEventListenerProvider implements EventListenerProvider { } } + private void handleScimEndpointConfigurationEvent(AdminEvent event, String id) { + LOGGER.infof("[SCIM] SCIM Endpoint configuration %s - %s ", event.getOperationType(), id); + + // In case of a component deletion + if (event.getOperationType() == OperationType.DELETE) { + // Check if it was a Scim endpoint configuration, and forward deletion if so + // TODO : determine if deleted element is of ScimStorageProvider class and only delete in that case + dispatcher.refreshActiveScimEndpoints(); + } else { + // In case of CREATE or UPDATE, we can directly use the string representation + // to check if it defines a SCIM endpoint (faster) + if (event.getRepresentation() != null + && event.getRepresentation().contains("\"providerId\":\"scim\"")) { + dispatcher.refreshActiveScimEndpoints(); + } + } + + } + private UserModel getUser(String id) { return session.users().getUserById(session.getContext().getRealm(), id); @@ -185,7 +218,12 @@ public class ScimEventListenerProvider implements EventListenerProvider { @Override public void close() { - // Nothing to close here + try { + dispatcher.close(); + } catch (Exception e) { + LOGGER.error("Error while closing dispatcher", e); + } } + } diff --git a/src/main/java/sh/libre/scim/jpa/ScimResourceProviderFactory.java b/src/main/java/sh/libre/scim/jpa/ScimResourceProviderFactory.java index 6f4162ed4b..7f3cc323dd 100644 --- a/src/main/java/sh/libre/scim/jpa/ScimResourceProviderFactory.java +++ b/src/main/java/sh/libre/scim/jpa/ScimResourceProviderFactory.java @@ -10,10 +10,6 @@ public class ScimResourceProviderFactory implements JpaEntityProviderFactory { static final String ID = "scim-resource"; - @Override - public void close() { - } - @Override public JpaEntityProvider create(KeycloakSession session) { return new ScimResourceProvider(); @@ -26,9 +22,18 @@ public class ScimResourceProviderFactory implements JpaEntityProviderFactory { @Override public void init(Scope scope) { + // Nothing to initialise } @Override public void postInit(KeycloakSessionFactory sessionFactory) { + // Nothing to do } + + + @Override + public void close() { + // Nothing to close + } + } diff --git a/src/main/java/sh/libre/scim/storage/ScimStorageProviderFactory.java b/src/main/java/sh/libre/scim/storage/ScimStorageProviderFactory.java index 62b931bb1a..920b830f1c 100644 --- a/src/main/java/sh/libre/scim/storage/ScimStorageProviderFactory.java +++ b/src/main/java/sh/libre/scim/storage/ScimStorageProviderFactory.java @@ -25,6 +25,9 @@ import java.time.Duration; import java.util.Date; import java.util.List; +/** + * Allows to register Scim endpoints through Admin console, using the provided config properties. + */ public class ScimStorageProviderFactory implements UserStorageProviderFactory, ImportSynchronization { public static final String ID = "scim"; @@ -39,13 +42,13 @@ public class ScimStorageProviderFactory @Override public SynchronizationResult sync(KeycloakSessionFactory sessionFactory, String realmId, UserStorageProviderModel model) { - // TODO if this should be kept here, better document prupose & usage - logger.infof("[SCIM] Sync from ScimStorageProvider - Realm %s", realmId); + // TODO if this should be kept here, better document purpose & usage + logger.infof("[SCIM] Sync from ScimStorageProvider - Realm %s - Model %s", realmId, model.getId()); SynchronizationResult result = new SynchronizationResult(); KeycloakModelUtils.runJobInTransaction(sessionFactory, session -> { RealmModel realm = session.realms().getRealm(realmId); session.getContext().setRealm(realm); - ScimDispatcher dispatcher = new ScimDispatcher(session); + ScimDispatcher dispatcher = ScimDispatcher.createForSession(session); if (BooleanUtils.TRUE.equals(model.get("propagation-user"))) { dispatcher.dispatchUserModificationToOne(model, client -> client.sync(result)); } @@ -71,7 +74,7 @@ public class ScimStorageProviderFactory for (RealmModel realm : taskSession.realms().getRealmsStream().toList()) { KeycloakModelUtils.runJobInTransaction(factory, session -> { session.getContext().setRealm(realm); - ScimDispatcher dispatcher = new ScimDispatcher(session); + ScimDispatcher dispatcher = ScimDispatcher.createForSession(session); for (GroupModel group : session.groups().getGroupsStream(realm) .filter(x -> BooleanUtils.TRUE.equals(x.getFirstAttribute("scim-dirty"))).toList()) { logger.infof("[SCIM] Dirty group : %s", group.getName());