diff --git a/src/main/java/sh/libre/scim/core/AbstractScimService.java b/src/main/java/sh/libre/scim/core/AbstractScimService.java
index 637283c0431a6c217d666ae5c2b36c1b6d17f61a..b72bef678d13d07ab7308901bff5a91ce018ad2b 100644
--- a/src/main/java/sh/libre/scim/core/AbstractScimService.java
+++ b/src/main/java/sh/libre/scim/core/AbstractScimService.java
@@ -82,10 +82,12 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends
entityOnRemoteScimId
.ifPresentOrElse(
externalId -> doReplace(roleMapperModel, externalId),
- () -> LOGGER.warnf("failed to replace resource %s, scim mapping not found", id)
+ () -> {
+ // TODO Exception Handling : should we throw a ScimPropagationException here ?
+ LOGGER.warnf("failed to replace resource %s, scim mapping not found", id);
+ }
);
} catch (Exception e) {
- LOGGER.error(e);
throw new ScimPropagationException("[SCIM] Error while replacing SCIM resource", e);
}
}
@@ -106,21 +108,21 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends
}
public void refreshResources(SynchronizationResult syncRes) throws ScimPropagationException {
- LOGGER.info("Refresh resources");
+ LOGGER.info("[SCIM] Refresh resources for endpoint " + this.getConfiguration().getEndPoint());
try (Stream<RMM> resourcesStream = getResourceStream()) {
Set<RMM> resources = resourcesStream.collect(Collectors.toUnmodifiableSet());
for (RMM resource : resources) {
KeycloakId id = getId(resource);
- LOGGER.infof("Reconciling local resource %s", id);
+ LOGGER.infof("[SCIM] Reconciling local resource %s", id);
if (isSkipRefresh(resource)) {
- LOGGER.infof("Skip local resource %s", id);
+ LOGGER.infof("[SCIM] Skip local resource %s", id);
continue;
}
if (findById(id).isPresent()) {
- LOGGER.info("Replacing it");
+ LOGGER.info("[SCIM] Replacing it");
replace(resource);
} else {
- LOGGER.info("Creating it");
+ LOGGER.info("[SCIM] Creating it");
create(resource);
}
syncRes.increaseUpdated();
@@ -133,11 +135,11 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends
protected abstract Stream<RMM> getResourceStream();
public void importResources(SynchronizationResult syncRes) throws ScimPropagationException {
- LOGGER.info("Import");
+ LOGGER.info("[SCIM] Import resources for scim endpoint " + this.getConfiguration().getEndPoint());
try {
for (S resource : scimClient.listResources()) {
try {
- LOGGER.infof("Reconciling remote resource %s", resource);
+ LOGGER.infof("[SCIM] Reconciling remote resource %s", resource);
EntityOnRemoteScimId externalId = resource.getId()
.map(EntityOnRemoteScimId::new)
.orElseThrow(() -> new ScimPropagationException("remote SCIM resource doesn't have an id"));
@@ -145,49 +147,50 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends
if (optionalMapping.isPresent()) {
ScimResource mapping = optionalMapping.get();
if (entityExists(mapping.getIdAsKeycloakId())) {
- LOGGER.info("Valid mapping found, skipping");
+ LOGGER.info("[SCIM] Valid mapping found, skipping");
continue;
} else {
- LOGGER.info("Delete a dangling mapping");
+ LOGGER.info("[SCIM] Delete a dangling mapping");
getScimResourceDao().delete(mapping);
}
}
Optional<KeycloakId> mapped = tryToMap(resource);
if (mapped.isPresent()) {
- LOGGER.info("Matched");
+ LOGGER.info("[SCIM] Matched");
createMapping(mapped.get(), externalId);
} else {
switch (scimProviderConfiguration.getImportAction()) {
case CREATE_LOCAL:
- LOGGER.info("Create local resource");
+ LOGGER.info("[SCIM] Create local resource");
try {
KeycloakId id = createEntity(resource);
createMapping(id, externalId);
syncRes.increaseAdded();
} catch (Exception e) {
+ // TODO ExceptionHandling should we stop and throw ScimPropagationException here ?
LOGGER.error(e);
}
break;
case DELETE_REMOTE:
- LOGGER.info("Delete remote resource");
+ LOGGER.info("[SCIM] Delete remote resource");
this.scimClient.delete(externalId);
syncRes.increaseRemoved();
break;
case NOTHING:
- LOGGER.info("Import action set to NOTHING");
+ LOGGER.info("[SCIM] Import action set to NOTHING");
break;
}
}
} catch (Exception e) {
- // TODO should we stop and throw ScimPropagationException here ?
+ // TODO ExceptionHandling should we stop and throw ScimPropagationException here ?
LOGGER.error(e);
e.printStackTrace();
syncRes.increaseFailed();
}
}
} catch (ResponseException e) {
- // TODO should we stop and throw ScimPropagationException here ?
+ // TODO ExceptionHandling should we stop and throw ScimPropagationException here ?
LOGGER.error(e);
e.printStackTrace();
syncRes.increaseFailed();
diff --git a/src/main/java/sh/libre/scim/core/GroupScimService.java b/src/main/java/sh/libre/scim/core/GroupScimService.java
index e5b23155523e64d0aa27caba48b4c895b25630dc..c448a9a00a3bc1f8fc23bb6807403dd25b72dc53 100644
--- a/src/main/java/sh/libre/scim/core/GroupScimService.java
+++ b/src/main/java/sh/libre/scim/core/GroupScimService.java
@@ -20,7 +20,7 @@ import java.util.TreeSet;
import java.util.stream.Stream;
public class GroupScimService extends AbstractScimService<GroupModel, Group> {
- private final Logger logger = Logger.getLogger(GroupScimService.class);
+ private final Logger LOGGER = Logger.getLogger(GroupScimService.class);
public GroupScimService(KeycloakSession keycloakSession, ScrimProviderConfiguration scimProviderConfiguration) {
super(keycloakSession, scimProviderConfiguration, ScimResourceType.GROUP);
@@ -95,13 +95,13 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
if (optionalGroupMemberMapping.isPresent()) {
ScimResource groupMemberMapping = optionalGroupMemberMapping.get();
EntityOnRemoteScimId externalIdAsEntityOnRemoteScimId = groupMemberMapping.getExternalIdAsEntityOnRemoteScimId();
- logger.debugf("found mapping for group member %s as %s", member, externalIdAsEntityOnRemoteScimId);
groupMember.setValue(externalIdAsEntityOnRemoteScimId.asString());
URI ref = getUri(ScimResourceType.USER, externalIdAsEntityOnRemoteScimId);
groupMember.setRef(ref.toString());
group.addMember(groupMember);
} else {
- logger.warnf("member %s not found for group %s", member, groupModel.getId());
+ // TODO Exception Handling : should we throw an exception when some group members can't be found ?
+ LOGGER.warnf("member %s not found for group %s", member, groupModel.getId());
}
}
return group;
diff --git a/src/main/java/sh/libre/scim/core/ScimClient.java b/src/main/java/sh/libre/scim/core/ScimClient.java
index ea89c60415f091fcd9b24e0445ec90b6f32f3fbc..caa318a60cbfd2907969f876fada42ed5001d895 100644
--- a/src/main/java/sh/libre/scim/core/ScimClient.java
+++ b/src/main/java/sh/libre/scim/core/ScimClient.java
@@ -85,6 +85,7 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
private void checkResponseIsSuccess(ServerResponse<S> response) {
if (!response.isSuccess()) {
+ // TODO Exception handling : we should throw a SCIM Progagation exception here
LOGGER.warn("[SCIM] Issue on SCIM Server response ");
LOGGER.warn(response.getResponseBody());
LOGGER.warn(response.getHttpStatus());
diff --git a/src/main/java/sh/libre/scim/core/ScimDispatcher.java b/src/main/java/sh/libre/scim/core/ScimDispatcher.java
index 54c484399848ab24ea50cc88310e42229381352a..2ec51aa924ddbd6b5b32fdafa6bac6a2cc00e83a 100644
--- a/src/main/java/sh/libre/scim/core/ScimDispatcher.java
+++ b/src/main/java/sh/libre/scim/core/ScimDispatcher.java
@@ -16,7 +16,7 @@ import java.util.Set;
*/
public class ScimDispatcher {
- private static final Logger logger = Logger.getLogger(ScimDispatcher.class);
+ private static final Logger LOGGER = Logger.getLogger(ScimDispatcher.class);
private final KeycloakSession session;
private final ScimExceptionHandler exceptionHandler;
@@ -34,42 +34,32 @@ public class ScimDispatcher {
* Lists all active ScimStorageProviderFactory and create new ScimClients for each of them
*/
public void refreshActiveScimEndpoints() {
- try {
- // Step 1: close existing clients
- for (GroupScimService c : groupScimServices) {
- c.close();
- }
- groupScimServices.clear();
- for (UserScimService c : userScimServices) {
- c.close();
- }
- userScimServices.clear();
-
- // Step 2: Get All SCIM endpoints defined in Admin Console (enabled ScimStorageProviderFactory)
- session.getContext().getRealm().getComponentsStream()
- .filter(m -> ScimEndpointConfigurationStorageProviderFactory.ID.equals(m.getProviderId())
- && m.get("enabled", true))
- .forEach(scimEndpointConfigurationRaw -> {
- ScrimProviderConfiguration scrimProviderConfiguration = new ScrimProviderConfiguration(scimEndpointConfigurationRaw);
- try {
- // Step 3 : create scim clients for each endpoint
- if (scimEndpointConfigurationRaw.get(ScrimProviderConfiguration.CONF_KEY_PROPAGATION_GROUP, false)) {
- GroupScimService groupScimService = new GroupScimService(session, scrimProviderConfiguration);
- groupScimServices.add(groupScimService);
- }
- if (scimEndpointConfigurationRaw.get(ScrimProviderConfiguration.CONF_KEY_PROPAGATION_USER, false)) {
- UserScimService userScimService = new UserScimService(session, scrimProviderConfiguration);
- userScimServices.add(userScimService);
- }
- } catch (Exception e) {
- logger.warnf("[SCIM] Invalid Endpoint configuration %s: %s", scimEndpointConfigurationRaw.getId(), e.getMessage());
- // TODO is it ok to log and try to create the other clients ?
+ // Step 1: close existing clients (as configuration may have changed)
+ groupScimServices.forEach(GroupScimService::close);
+ groupScimServices.clear();
+ userScimServices.forEach(UserScimService::close);
+ userScimServices.clear();
+
+ // Step 2: Get All SCIM endpoints defined in Admin Console (enabled ScimStorageProviderFactory)
+ session.getContext().getRealm().getComponentsStream()
+ .filter(m -> ScimEndpointConfigurationStorageProviderFactory.ID.equals(m.getProviderId())
+ && m.get("enabled", true))
+ .forEach(scimEndpointConfigurationRaw -> {
+ ScrimProviderConfiguration scrimProviderConfiguration = new ScrimProviderConfiguration(scimEndpointConfigurationRaw);
+ try {
+ // Step 3 : create scim clients for each endpoint
+ if (scimEndpointConfigurationRaw.get(ScrimProviderConfiguration.CONF_KEY_PROPAGATION_GROUP, false)) {
+ GroupScimService groupScimService = new GroupScimService(session, scrimProviderConfiguration);
+ groupScimServices.add(groupScimService);
}
- });
- } catch (Exception e) {
- logger.error("[SCIM] Error while refreshing scim clients ", e);
- // TODO : how to handle exception here ?
- }
+ if (scimEndpointConfigurationRaw.get(ScrimProviderConfiguration.CONF_KEY_PROPAGATION_USER, false)) {
+ UserScimService userScimService = new UserScimService(session, scrimProviderConfiguration);
+ userScimServices.add(userScimService);
+ }
+ } catch (Exception e) {
+ exceptionHandler.handleInvalidEndpointConfiguration(scimEndpointConfigurationRaw, e);
+ }
+ });
}
public void dispatchUserModificationToAll(SCIMPropagationConsumer<UserScimService> operationToDispatch) {
@@ -84,7 +74,7 @@ public class ScimDispatcher {
}
});
// TODO we could iterate on servicesCorrectlyPropagated to undo modification
- logger.infof("[SCIM] User operation dispatched to %d SCIM server", servicesCorrectlyPropagated.size());
+ LOGGER.infof("[SCIM] User operation dispatched to %d SCIM server", servicesCorrectlyPropagated.size());
}
public void dispatchGroupModificationToAll(SCIMPropagationConsumer<GroupScimService> operationToDispatch) {
@@ -99,7 +89,7 @@ public class ScimDispatcher {
}
});
// TODO we could iterate on servicesCorrectlyPropagated to undo modification
- logger.infof("[SCIM] Group operation dispatched to %d SCIM server", servicesCorrectlyPropagated.size());
+ LOGGER.infof("[SCIM] Group operation dispatched to %d SCIM server", servicesCorrectlyPropagated.size());
}
public void dispatchUserModificationToOne(ComponentModel scimServerConfiguration, SCIMPropagationConsumer<UserScimService> operationToDispatch) {
@@ -109,12 +99,12 @@ public class ScimDispatcher {
if (matchingClient.isPresent()) {
try {
operationToDispatch.acceptThrows(matchingClient.get());
- logger.infof("[SCIM] User operation dispatched to SCIM server %s", matchingClient.get().getConfiguration().getId());
+ LOGGER.infof("[SCIM] User operation dispatched to SCIM server %s", matchingClient.get().getConfiguration().getId());
} catch (ScimPropagationException e) {
exceptionHandler.handleException(matchingClient.get().getConfiguration(), e);
}
} else {
- logger.error("[SCIM] Could not find a Scim Client matching User endpoint configuration" + scimServerConfiguration.getId());
+ LOGGER.error("[SCIM] Could not find a Scim Client matching User endpoint configuration" + scimServerConfiguration.getId());
}
}
@@ -126,12 +116,12 @@ public class ScimDispatcher {
if (matchingClient.isPresent()) {
try {
operationToDispatch.acceptThrows(matchingClient.get());
- logger.infof("[SCIM] Group operation dispatched to SCIM server %s", matchingClient.get().getConfiguration().getId());
+ LOGGER.infof("[SCIM] Group operation dispatched to SCIM server %s", matchingClient.get().getConfiguration().getId());
} catch (ScimPropagationException e) {
exceptionHandler.handleException(matchingClient.get().getConfiguration(), e);
}
} else {
- logger.error("[SCIM] Could not find a Scim Client matching Group endpoint configuration" + scimServerConfiguration.getId());
+ LOGGER.error("[SCIM] Could not find a Scim Client matching Group endpoint configuration" + scimServerConfiguration.getId());
}
}
diff --git a/src/main/java/sh/libre/scim/core/ScimExceptionHandler.java b/src/main/java/sh/libre/scim/core/ScimExceptionHandler.java
index 44d65ff0e114f87af23cf2068d95224fd70d24f9..47b8c718dad5aac127aaef6ea5f6b3346d50feef 100644
--- a/src/main/java/sh/libre/scim/core/ScimExceptionHandler.java
+++ b/src/main/java/sh/libre/scim/core/ScimExceptionHandler.java
@@ -1,6 +1,7 @@
package sh.libre.scim.core;
import org.jboss.logging.Logger;
+import org.keycloak.component.ComponentModel;
import org.keycloak.models.KeycloakSession;
/**
@@ -10,8 +11,8 @@ import org.keycloak.models.KeycloakSession;
* - The thrown exception itself
*/
public class ScimExceptionHandler {
+ private static final Logger LOGGER = Logger.getLogger(ScimExceptionHandler.class);
- private static final Logger logger = Logger.getLogger(ScimDispatcher.class);
private final KeycloakSession session;
public ScimExceptionHandler(KeycloakSession session) {
@@ -25,7 +26,14 @@ public class ScimExceptionHandler {
* @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();
+ LOGGER.error("[SCIM] Error while propagating to SCIM endpoint %s", scimProviderConfiguration.getId(), e);
+ // TODO Exception Handling : rollback only for critical operations, if configuration says so
+ // session.getTransactionManager().rollback();
+ }
+
+ public void handleInvalidEndpointConfiguration(ComponentModel scimEndpointConfigurationRaw, Exception e) {
+ LOGGER.error("[SCIM] Invalid Endpoint configuration " + scimEndpointConfigurationRaw.getId(), e);
+ // TODO Exception Handling is it ok to ignore an invalid Scim endpoint Configuration ?
+ // IF not, we should propagate the exception here
}
}
diff --git a/src/main/java/sh/libre/scim/core/UserScimService.java b/src/main/java/sh/libre/scim/core/UserScimService.java
index 3c30023c6406e8664ccf9a7931d183a0616e80bc..fbae36236b4fee916133a273e8b1d35ea2dbbc13 100644
--- a/src/main/java/sh/libre/scim/core/UserScimService.java
+++ b/src/main/java/sh/libre/scim/core/UserScimService.java
@@ -20,7 +20,7 @@ import java.util.Optional;
import java.util.stream.Stream;
public class UserScimService extends AbstractScimService<UserModel, User> {
- private final Logger logger = Logger.getLogger(UserScimService.class);
+ private final Logger LOGGER = Logger.getLogger(UserScimService.class);
public UserScimService(
KeycloakSession keycloakSession,
@@ -41,17 +41,18 @@ public class UserScimService extends AbstractScimService<UserModel, User> {
@Override
protected Optional<KeycloakId> tryToMap(User resource) {
Optional<KeycloakId> matchedByUsername = resource.getUserName()
- .map(getKeycloakDao()::getUserByUsername)
- .map(this::getId);
+ .map(getKeycloakDao()::getUserByUsername)
+ .map(this::getId);
Optional<KeycloakId> matchedByEmail = resource.getEmails().stream()
.findFirst()
.flatMap(MultiComplexNode::getValue)
.map(getKeycloakDao()::getUserByEmail)
.map(this::getId);
if (matchedByUsername.isPresent()
- && matchedByEmail.isPresent()
- && !matchedByUsername.equals(matchedByEmail)) {
- logger.warnf("found 2 possible users for remote user %s %s", matchedByUsername.get(), matchedByEmail.get());
+ && matchedByEmail.isPresent()
+ && !matchedByUsername.equals(matchedByEmail)) {
+ // TODO Exception Handling : what should we do here ?
+ LOGGER.warnf("found 2 possible users for remote user %s %s", matchedByUsername.get(), matchedByEmail.get());
return Optional.empty();
}
if (matchedByUsername.isPresent()) {
diff --git a/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java b/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java
index 4195e6b6188e36c3fb0b9c12b63259a478964cf1..546522294073ec6a0729511a6848931875cea595 100644
--- a/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java
+++ b/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java
@@ -228,11 +228,7 @@ public class ScimEventListenerProvider implements EventListenerProvider {
@Override
public void close() {
- try {
- dispatcher.close();
- } catch (Exception e) {
- LOGGER.error("Error while closing dispatcher", e);
- }
+ dispatcher.close();
}
diff --git a/src/main/java/sh/libre/scim/storage/ScimEndpointConfigurationStorageProviderFactory.java b/src/main/java/sh/libre/scim/storage/ScimEndpointConfigurationStorageProviderFactory.java
index 00a527fe3ecb22e62e2ebd469dd72061b8fdfd8b..724fa925b72b3082b78d2324d22a4115f42567e8 100644
--- a/src/main/java/sh/libre/scim/storage/ScimEndpointConfigurationStorageProviderFactory.java
+++ b/src/main/java/sh/libre/scim/storage/ScimEndpointConfigurationStorageProviderFactory.java
@@ -31,7 +31,7 @@ import java.util.List;
public class ScimEndpointConfigurationStorageProviderFactory
implements UserStorageProviderFactory<ScimEndpointConfigurationStorageProviderFactory.ScimEndpointConfigurationStorageProvider>, ImportSynchronization {
public static final String ID = "scim";
- private final Logger logger = Logger.getLogger(ScimEndpointConfigurationStorageProviderFactory.class);
+ private final Logger LOGGER = Logger.getLogger(ScimEndpointConfigurationStorageProviderFactory.class);
@Override
public String getId() {
@@ -43,7 +43,7 @@ public class ScimEndpointConfigurationStorageProviderFactory
public SynchronizationResult sync(KeycloakSessionFactory sessionFactory, String realmId,
UserStorageProviderModel model) {
// TODO if this should be kept here, better document purpose & usage
- logger.infof("[SCIM] Sync from ScimStorageProvider - Realm %s - Model %s", realmId, model.getId());
+ 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);
@@ -78,7 +78,7 @@ public class ScimEndpointConfigurationStorageProviderFactory
ScimDispatcher dispatcher = new ScimDispatcher(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());
+ LOGGER.infof("[SCIM] Dirty group: %s", group.getName());
dispatcher.dispatchGroupModificationToAll(client -> client.replace(group));
group.removeAttribute("scim-dirty");
}