diff --git a/src/main/java/sh/libre/scim/core/AbstractScimService.java b/src/main/java/sh/libre/scim/core/AbstractScimService.java index 1d32e323550c5aa6908a33935a287f3e3279a114..637283c0431a6c217d666ae5c2b36c1b6d17f61a 100644 --- a/src/main/java/sh/libre/scim/core/AbstractScimService.java +++ b/src/main/java/sh/libre/scim/core/AbstractScimService.java @@ -132,7 +132,7 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends protected abstract Stream<RMM> getResourceStream(); - public void importResources(SynchronizationResult syncRes) { + public void importResources(SynchronizationResult syncRes) throws ScimPropagationException { LOGGER.info("Import"); try { for (S resource : scimClient.listResources()) { @@ -180,13 +180,17 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends } } } catch (Exception e) { + // TODO should we stop and throw ScimPropagationException here ? LOGGER.error(e); e.printStackTrace(); syncRes.increaseFailed(); } } } catch (ResponseException e) { - throw new RuntimeException(e); + // TODO should we stop and throw ScimPropagationException here ? + LOGGER.error(e); + e.printStackTrace(); + syncRes.increaseFailed(); } } @@ -216,7 +220,7 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends try { return new URI("%s/%s".formatted(type.getEndpoint(), externalId.asString())); } catch (URISyntaxException e) { - throw new IllegalStateException("should never occur: can not format URI for type %s and id %s".formatted(type, externalId) , e); + throw new IllegalStateException("should never occur: can not format URI for type %s and id %s".formatted(type, externalId), e); } } diff --git a/src/main/java/sh/libre/scim/core/ScimClient.java b/src/main/java/sh/libre/scim/core/ScimClient.java index 9189ad308b426ee33f884b4b4b4a97e7048345fe..ea89c60415f091fcd9b24e0445ec90b6f32f3fbc 100644 --- a/src/main/java/sh/libre/scim/core/ScimClient.java +++ b/src/main/java/sh/libre/scim/core/ScimClient.java @@ -17,6 +17,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; public class ScimClient<S extends ResourceNode> implements AutoCloseable { private static final Logger LOGGER = Logger.getLogger(ScimClient.class); @@ -62,10 +63,11 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable { return new ScimClient(scimRequestBuilder, scimResourceType); } - public EntityOnRemoteScimId create(KeycloakId id, S scimForCreation) { - if (scimForCreation.getId().isPresent()) { - throw new IllegalArgumentException( - "%s is already created on remote with id %s".formatted(id, scimForCreation.getId().get()) + public EntityOnRemoteScimId create(KeycloakId id, S scimForCreation) throws ScimPropagationException { + Optional<String> scimForCreationId = scimForCreation.getId(); + if (scimForCreationId.isPresent()) { + throw new ScimPropagationException( + "%s is already created on remote with id %s".formatted(id, scimForCreationId.get()) ); } Retry retry = retryRegistry.retry("create-%s".formatted(id.asString())); diff --git a/src/main/java/sh/libre/scim/core/ScimDispatcher.java b/src/main/java/sh/libre/scim/core/ScimDispatcher.java index 62a8bfbda67c404099cddbe2ad67091fd00f9a84..54c484399848ab24ea50cc88310e42229381352a 100644 --- a/src/main/java/sh/libre/scim/core/ScimDispatcher.java +++ b/src/main/java/sh/libre/scim/core/ScimDispatcher.java @@ -19,6 +19,7 @@ public class ScimDispatcher { private static final Logger logger = Logger.getLogger(ScimDispatcher.class); private final KeycloakSession session; + private final ScimExceptionHandler exceptionHandler; private boolean clientsInitialized = false; private final List<UserScimService> userScimServices = new ArrayList<>(); private final List<GroupScimService> groupScimServices = new ArrayList<>(); @@ -26,6 +27,7 @@ public class ScimDispatcher { public ScimDispatcher(KeycloakSession session) { this.session = session; + this.exceptionHandler = new ScimExceptionHandler(session); } /** @@ -78,7 +80,7 @@ public class ScimDispatcher { operationToDispatch.acceptThrows(userScimService); servicesCorrectlyPropagated.add(userScimService); } catch (ScimPropagationException e) { - logAndRollback(userScimService.getConfiguration(), e); + exceptionHandler.handleException(userScimService.getConfiguration(), e); } }); // TODO we could iterate on servicesCorrectlyPropagated to undo modification @@ -93,7 +95,7 @@ public class ScimDispatcher { operationToDispatch.acceptThrows(groupScimService); servicesCorrectlyPropagated.add(groupScimService); } catch (ScimPropagationException e) { - logAndRollback(groupScimService.getConfiguration(), e); + exceptionHandler.handleException(groupScimService.getConfiguration(), e); } }); // TODO we could iterate on servicesCorrectlyPropagated to undo modification @@ -109,10 +111,10 @@ public class ScimDispatcher { operationToDispatch.acceptThrows(matchingClient.get()); logger.infof("[SCIM] User operation dispatched to SCIM server %s", matchingClient.get().getConfiguration().getId()); } catch (ScimPropagationException e) { - logAndRollback(matchingClient.get().getConfiguration(), e); + exceptionHandler.handleException(matchingClient.get().getConfiguration(), e); } } else { - logger.error("[SCIM] Could not find a Scim Client matching endpoint configuration" + scimServerConfiguration.getId()); + logger.error("[SCIM] Could not find a Scim Client matching User endpoint configuration" + scimServerConfiguration.getId()); } } @@ -126,10 +128,10 @@ public class ScimDispatcher { operationToDispatch.acceptThrows(matchingClient.get()); logger.infof("[SCIM] Group operation dispatched to SCIM server %s", matchingClient.get().getConfiguration().getId()); } catch (ScimPropagationException e) { - logAndRollback(matchingClient.get().getConfiguration(), e); + exceptionHandler.handleException(matchingClient.get().getConfiguration(), e); } } else { - logger.error("[SCIM] Could not find a Scim Client matching endpoint configuration" + scimServerConfiguration.getId()); + logger.error("[SCIM] Could not find a Scim Client matching Group endpoint configuration" + scimServerConfiguration.getId()); } } @@ -144,11 +146,6 @@ public class ScimDispatcher { userScimServices.clear(); } - private void logAndRollback(ScrimProviderConfiguration scimServerConfiguration, ScimPropagationException e) { - logger.error("[SCIM] Error while propagating to SCIM endpoint " + scimServerConfiguration.getId(), e); - // TODO session.getTransactionManager().rollback(); - } - private void initializeClientsIfNeeded() { if (!clientsInitialized) { clientsInitialized = true; diff --git a/src/main/java/sh/libre/scim/core/ScimExceptionHandler.java b/src/main/java/sh/libre/scim/core/ScimExceptionHandler.java new file mode 100644 index 0000000000000000000000000000000000000000..44d65ff0e114f87af23cf2068d95224fd70d24f9 --- /dev/null +++ b/src/main/java/sh/libre/scim/core/ScimExceptionHandler.java @@ -0,0 +1,31 @@ +package sh.libre.scim.core; + +import org.jboss.logging.Logger; +import org.keycloak.models.KeycloakSession; + +/** + * In charge of dealing with SCIM exceptions by ignoring, logging or rollback transaction according to : + * - The context in which it occurs (sync, user creation...) + * - The related SCIM endpoint and its configuration + * - The thrown exception itself + */ +public class ScimExceptionHandler { + + private static final Logger logger = Logger.getLogger(ScimDispatcher.class); + private final KeycloakSession session; + + public ScimExceptionHandler(KeycloakSession session) { + this.session = session; + } + + /** + * Handles the given exception by loggin and/or rollback transaction. + * + * @param scimProviderConfiguration the configuration of the endpoint for which the propagation exception occured + * @param e the occuring exception + */ + public void handleException(ScrimProviderConfiguration scimProviderConfiguration, ScimPropagationException e) { + logger.error("[SCIM] Error while propagating to SCIM endpoint " + scimProviderConfiguration.getId(), e); + // TODO session.getTransactionManager().rollback(); + } +} diff --git a/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java b/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java index 06a01db52b4ab99e4089bc7daf9d0e7b04ae8f73..4195e6b6188e36c3fb0b9c12b63259a478964cf1 100644 --- a/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java +++ b/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java @@ -107,7 +107,7 @@ public class ScimEventListenerProvider implements EventListenerProvider { ScimResourceType type = switch (rawResourceType) { case "users" -> ScimResourceType.USER; case "groups" -> ScimResourceType.GROUP; - default -> throw new IllegalArgumentException("Unsuported resource type: " + rawResourceType); + default -> throw new IllegalArgumentException("Unsupported resource type: " + rawResourceType); }; KeycloakId id = new KeycloakId(matcher.group(2)); handleRoleMappingEvent(event, type, id);