diff --git a/src/main/java/sh/libre/scim/core/AbstractScimService.java b/src/main/java/sh/libre/scim/core/AbstractScimService.java
index b72bef678d13d07ab7308901bff5a91ce018ad2b..9fc896bbe0166471b17fab56109a1a9aceab4b68 100644
--- a/src/main/java/sh/libre/scim/core/AbstractScimService.java
+++ b/src/main/java/sh/libre/scim/core/AbstractScimService.java
@@ -1,14 +1,17 @@
package sh.libre.scim.core;
-import de.captaingoldfish.scim.sdk.common.exceptions.ResponseException;
import de.captaingoldfish.scim.sdk.common.resources.ResourceNode;
import de.captaingoldfish.scim.sdk.common.resources.complex.Meta;
import org.jboss.logging.Logger;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RoleMapperModel;
import org.keycloak.storage.user.SynchronizationResult;
-import sh.libre.scim.jpa.ScimResource;
+import sh.libre.scim.core.exceptions.ErrorForScimEndpointException;
+import sh.libre.scim.core.exceptions.InconsistentScimDataException;
+import sh.libre.scim.core.exceptions.ScimPropagationException;
+import sh.libre.scim.core.exceptions.UnexpectedScimDataException;
import sh.libre.scim.jpa.ScimResourceDao;
+import sh.libre.scim.jpa.ScimResourceMapping;
import java.net.URI;
import java.net.URISyntaxException;
@@ -17,7 +20,8 @@ import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
-public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends ResourceNode> implements AutoCloseable {
+// TODO Document K and S
+public abstract class AbstractScimService<K extends RoleMapperModel, S extends ResourceNode> implements AutoCloseable {
private static final Logger LOGGER = Logger.getLogger(AbstractScimService.class);
@@ -36,179 +40,164 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends
this.scimClient = ScimClient.open(scimProviderConfiguration, type);
}
- public void create(RMM roleMapperModel) throws ScimPropagationException {
- boolean skip = isSkip(roleMapperModel);
- if (skip)
+ public void create(K roleMapperModel) throws InconsistentScimDataException, ErrorForScimEndpointException {
+ if (isMarkedToIgnore(roleMapperModel)) {
+ // Silently return: resource is explicitly marked as to ignore
return;
- // If mapping exist then it was created by import so skip.
+ }
+ // If mapping, then we are trying to recreate a user that was already created by import
KeycloakId id = getId(roleMapperModel);
- if (findById(id).isPresent()) {
- return;
+ if (findMappingById(id).isPresent()) {
+ throw new InconsistentScimDataException("Trying to create user with id " + id + ": id already exists in Keycloak database");
}
- S scimForCreation = toScimForCreation(roleMapperModel);
+ S scimForCreation = scimRequestBodyForCreate(roleMapperModel);
EntityOnRemoteScimId externalId = scimClient.create(id, scimForCreation);
createMapping(id, externalId);
}
- protected abstract S toScimForCreation(RMM roleMapperModel);
-
- protected abstract KeycloakId getId(RMM roleMapperModel);
-
- protected abstract boolean isSkip(RMM roleMapperModel);
-
- private void createMapping(KeycloakId keycloakId, EntityOnRemoteScimId externalId) {
- getScimResourceDao().create(keycloakId, externalId, type);
- }
-
- protected ScimResourceDao getScimResourceDao() {
- return ScimResourceDao.newInstance(getKeycloakSession(), scimProviderConfiguration.getId());
- }
-
- private Optional<ScimResource> findById(KeycloakId keycloakId) {
- return getScimResourceDao().findById(keycloakId, type);
- }
-
- private KeycloakSession getKeycloakSession() {
- return keycloakSession;
- }
-
- public void replace(RMM roleMapperModel) throws ScimPropagationException {
- try {
- if (isSkip(roleMapperModel))
- return;
- KeycloakId id = getId(roleMapperModel);
- Optional<EntityOnRemoteScimId> entityOnRemoteScimId = findById(id)
- .map(ScimResource::getExternalIdAsEntityOnRemoteScimId);
- entityOnRemoteScimId
- .ifPresentOrElse(
- externalId -> doReplace(roleMapperModel, externalId),
- () -> {
- // TODO Exception Handling : should we throw a ScimPropagationException here ?
- LOGGER.warnf("failed to replace resource %s, scim mapping not found", id);
- }
- );
- } catch (Exception e) {
- throw new ScimPropagationException("[SCIM] Error while replacing SCIM resource", e);
+ public void update(K roleMapperModel) throws InconsistentScimDataException, ErrorForScimEndpointException {
+ if (isMarkedToIgnore(roleMapperModel)) {
+ // Silently return: resource is explicitly marked as to ignore
+ return;
}
+ KeycloakId keycloakId = getId(roleMapperModel);
+ EntityOnRemoteScimId entityOnRemoteScimId = findMappingById(keycloakId)
+ .map(ScimResourceMapping::getExternalIdAsEntityOnRemoteScimId)
+ .orElseThrow(() -> new InconsistentScimDataException("Failed to find SCIM mapping for " + keycloakId));
+ S scimForReplace = scimRequestBodyForUpdate(roleMapperModel, entityOnRemoteScimId);
+ scimClient.update(entityOnRemoteScimId, scimForReplace);
}
- private void doReplace(RMM roleMapperModel, EntityOnRemoteScimId externalId) {
- S scimForReplace = toScimForReplace(roleMapperModel, externalId);
- scimClient.replace(externalId, scimForReplace);
- }
-
- protected abstract S toScimForReplace(RMM roleMapperModel, EntityOnRemoteScimId externalId);
+ protected abstract S scimRequestBodyForUpdate(K roleMapperModel, EntityOnRemoteScimId externalId);
- public void delete(KeycloakId id) throws ScimPropagationException {
- ScimResource resource = findById(id)
- .orElseThrow(() -> new ScimPropagationException("Failed to delete resource %s, scim mapping not found: ".formatted(id)));
+ public void delete(KeycloakId id) throws InconsistentScimDataException, ErrorForScimEndpointException {
+ ScimResourceMapping resource = findMappingById(id)
+ .orElseThrow(() -> new InconsistentScimDataException("Failed to delete resource %s, scim mapping not found: ".formatted(id)));
EntityOnRemoteScimId externalId = resource.getExternalIdAsEntityOnRemoteScimId();
scimClient.delete(externalId);
getScimResourceDao().delete(resource);
}
- public void refreshResources(SynchronizationResult syncRes) throws ScimPropagationException {
- 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("[SCIM] Reconciling local resource %s", id);
- if (isSkipRefresh(resource)) {
- LOGGER.infof("[SCIM] Skip local resource %s", id);
- continue;
- }
- if (findById(id).isPresent()) {
- LOGGER.info("[SCIM] Replacing it");
- replace(resource);
- } else {
- LOGGER.info("[SCIM] Creating it");
- create(resource);
+ public void pushResourcesToScim(SynchronizationResult syncRes) throws ErrorForScimEndpointException, InconsistentScimDataException {
+ LOGGER.info("[SCIM] Push resources to endpoint " + this.getConfiguration().getEndPoint());
+ try (Stream<K> resourcesStream = getResourceStream()) {
+ Set<K> resources = resourcesStream.collect(Collectors.toUnmodifiableSet());
+ for (K resource : resources) {
+ try {
+ KeycloakId id = getId(resource);
+ LOGGER.infof("[SCIM] Reconciling local resource %s", id);
+ if (shouldIgnoreForScimSynchronization(resource)) {
+ LOGGER.infof("[SCIM] Skip local resource %s", id);
+ continue;
+ }
+ if (findMappingById(id).isPresent()) {
+ LOGGER.info("[SCIM] Replacing it");
+ update(resource);
+ } else {
+ LOGGER.info("[SCIM] Creating it");
+ create(resource);
+ }
+ syncRes.increaseUpdated();
+ } catch (ErrorForScimEndpointException e) {
+ // TODO Error handling use strategy SOUPLE or STICT to determine if we re-trhoguh or log & continue
+ throw e;
}
- syncRes.increaseUpdated();
}
}
}
- protected abstract boolean isSkipRefresh(RMM resource);
- protected abstract Stream<RMM> getResourceStream();
+ public void pullResourcesFromScim(SynchronizationResult syncRes) {
+ LOGGER.info("[SCIM] Import resources from endpoint " + this.getConfiguration().getEndPoint());
+ for (S resource : scimClient.listResources()) {
+ try {
+ LOGGER.infof("[SCIM] Reconciling remote resource %s", resource);
+ EntityOnRemoteScimId externalId = resource.getId()
+ .map(EntityOnRemoteScimId::new)
+ .orElseThrow(() -> new UnexpectedScimDataException("Remote SCIM resource doesn't have an id, cannot import it in Keycloak"));
+ Optional<ScimResourceMapping> optionalMapping = getScimResourceDao().findByExternalId(externalId, type);
- public void importResources(SynchronizationResult syncRes) throws ScimPropagationException {
- LOGGER.info("[SCIM] Import resources for scim endpoint " + this.getConfiguration().getEndPoint());
- try {
- for (S resource : scimClient.listResources()) {
- try {
- 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"));
- Optional<ScimResource> optionalMapping = getScimResourceDao().findByExternalId(externalId, type);
- if (optionalMapping.isPresent()) {
- ScimResource mapping = optionalMapping.get();
- if (entityExists(mapping.getIdAsKeycloakId())) {
- LOGGER.info("[SCIM] Valid mapping found, skipping");
- continue;
- } else {
- LOGGER.info("[SCIM] Delete a dangling mapping");
- getScimResourceDao().delete(mapping);
- }
+ // If an existing mapping exists, delete potential dangling references
+ if (optionalMapping.isPresent()) {
+ ScimResourceMapping mapping = optionalMapping.get();
+ if (entityExists(mapping.getIdAsKeycloakId())) {
+ LOGGER.debug("[SCIM] Valid mapping found, skipping");
+ continue;
+ } else {
+ LOGGER.info("[SCIM] Delete a dangling mapping");
+ getScimResourceDao().delete(mapping);
}
+ }
- Optional<KeycloakId> mapped = tryToMap(resource);
- if (mapped.isPresent()) {
- LOGGER.info("[SCIM] Matched");
- createMapping(mapped.get(), externalId);
- } else {
- switch (scimProviderConfiguration.getImportAction()) {
- case CREATE_LOCAL:
- 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("[SCIM] Delete remote resource");
- this.scimClient.delete(externalId);
- syncRes.increaseRemoved();
- break;
- case NOTHING:
- LOGGER.info("[SCIM] Import action set to NOTHING");
- break;
+ // Here no keycloak user/group matching the SCIM external id exists
+ // Try to match existing keycloak resource by properties (username, email, name)
+ Optional<KeycloakId> mapped = matchKeycloakMappingByScimProperties(resource);
+ if (mapped.isPresent()) {
+ LOGGER.info("[SCIM] Matched SCIM resource " + externalId + " from properties with keycloak entity " + mapped.get());
+ createMapping(mapped.get(), externalId);
+ syncRes.increaseUpdated();
+ } else {
+ switch (scimProviderConfiguration.getImportAction()) {
+ case CREATE_LOCAL -> {
+ LOGGER.info("[SCIM] Create local resource for SCIM resource " + externalId);
+ KeycloakId id = createEntity(resource);
+ createMapping(id, externalId);
+ syncRes.increaseAdded();
}
+ case DELETE_REMOTE -> {
+ LOGGER.info("[SCIM] Delete remote resource " + externalId);
+ scimClient.delete(externalId);
+ }
+ case NOTHING -> LOGGER.info("[SCIM] Import action set to NOTHING");
}
- } catch (Exception e) {
- // TODO ExceptionHandling should we stop and throw ScimPropagationException here ?
- LOGGER.error(e);
- e.printStackTrace();
- syncRes.increaseFailed();
}
+ } catch (ScimPropagationException e) {
+ throw new RuntimeException(e);
}
- } catch (ResponseException e) {
- // TODO ExceptionHandling should we stop and throw ScimPropagationException here ?
- LOGGER.error(e);
- e.printStackTrace();
- syncRes.increaseFailed();
+
}
}
- protected abstract KeycloakId createEntity(S resource) throws ScimPropagationException;
- protected abstract Optional<KeycloakId> tryToMap(S resource) throws ScimPropagationException;
+ protected abstract S scimRequestBodyForCreate(K roleMapperModel);
+
+ protected abstract KeycloakId getId(K roleMapperModel);
+
+ protected abstract boolean isMarkedToIgnore(K roleMapperModel);
+
+ private void createMapping(KeycloakId keycloakId, EntityOnRemoteScimId externalId) {
+ getScimResourceDao().create(keycloakId, externalId, type);
+ }
+
+ protected ScimResourceDao getScimResourceDao() {
+ return ScimResourceDao.newInstance(getKeycloakSession(), scimProviderConfiguration.getId());
+ }
+
+ private Optional<ScimResourceMapping> findMappingById(KeycloakId keycloakId) {
+ return getScimResourceDao().findById(keycloakId, type);
+ }
+
+ private KeycloakSession getKeycloakSession() {
+ return keycloakSession;
+ }
+
+
+ protected abstract boolean shouldIgnoreForScimSynchronization(K resource);
+
+ protected abstract Stream<K> getResourceStream();
+
+ protected abstract KeycloakId createEntity(S resource) throws UnexpectedScimDataException;
+
+ protected abstract Optional<KeycloakId> matchKeycloakMappingByScimProperties(S resource) throws InconsistentScimDataException;
protected abstract boolean entityExists(KeycloakId keycloakId);
- public void sync(SynchronizationResult syncRes) throws ScimPropagationException {
+ public void sync(SynchronizationResult syncRes) throws InconsistentScimDataException, ErrorForScimEndpointException {
if (this.scimProviderConfiguration.isSyncImport()) {
- this.importResources(syncRes);
+ this.pullResourcesFromScim(syncRes);
}
if (this.scimProviderConfiguration.isSyncRefresh()) {
- this.refreshResources(syncRes);
+ this.pushResourcesToScim(syncRes);
}
}
diff --git a/src/main/java/sh/libre/scim/core/GroupScimService.java b/src/main/java/sh/libre/scim/core/GroupScimService.java
index c448a9a00a3bc1f8fc23bb6807403dd25b72dc53..4679725256c8a4e0c4bdd51acfee6efe02c5d8b9 100644
--- a/src/main/java/sh/libre/scim/core/GroupScimService.java
+++ b/src/main/java/sh/libre/scim/core/GroupScimService.java
@@ -10,7 +10,8 @@ import org.jboss.logging.Logger;
import org.keycloak.models.GroupModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.UserModel;
-import sh.libre.scim.jpa.ScimResource;
+import sh.libre.scim.core.exceptions.UnexpectedScimDataException;
+import sh.libre.scim.jpa.ScimResourceMapping;
import java.net.URI;
import java.util.List;
@@ -37,7 +38,7 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
}
@Override
- protected Optional<KeycloakId> tryToMap(Group resource) {
+ protected Optional<KeycloakId> matchKeycloakMappingByScimProperties(Group resource) {
Set<String> names = new TreeSet<>();
resource.getId().ifPresent(names::add);
resource.getDisplayName().ifPresent(names::add);
@@ -52,20 +53,20 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
}
@Override
- protected KeycloakId createEntity(Group resource) throws ScimPropagationException {
+ protected KeycloakId createEntity(Group resource) throws UnexpectedScimDataException {
String displayName = resource.getDisplayName()
.filter(StringUtils::isNotBlank)
- .orElseThrow(() -> new ScimPropagationException("can't create group without name: " + resource));
+ .orElseThrow(() -> new UnexpectedScimDataException("Remote Scim group has empty name, can't create. Resource id = %s".formatted(resource.getId())));
GroupModel group = getKeycloakDao().createGroup(displayName);
List<Member> groupMembers = resource.getMembers();
if (CollectionUtils.isNotEmpty(groupMembers)) {
for (Member groupMember : groupMembers) {
EntityOnRemoteScimId externalId = groupMember.getValue()
.map(EntityOnRemoteScimId::new)
- .orElseThrow(() -> new ScimPropagationException("can't create group member for group '%s' without id: ".formatted(displayName) + resource));
+ .orElseThrow(() -> new UnexpectedScimDataException("can't create group member for group '%s' without id: ".formatted(displayName) + resource));
KeycloakId userId = getScimResourceDao().findUserByExternalId(externalId)
- .map(ScimResource::getIdAsKeycloakId)
- .orElseThrow(() -> new ScimPropagationException("can't find mapping for group member %s".formatted(externalId)));
+ .map(ScimResourceMapping::getIdAsKeycloakId)
+ .orElseThrow(() -> new UnexpectedScimDataException("can't find mapping for group member %s".formatted(externalId)));
UserModel userModel = getKeycloakDao().getUserById(userId);
userModel.joinGroup(group);
}
@@ -74,7 +75,7 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
}
@Override
- protected boolean isSkip(GroupModel groupModel) {
+ protected boolean isMarkedToIgnore(GroupModel groupModel) {
return BooleanUtils.TRUE.equals(groupModel.getFirstAttribute("scim-skip"));
}
@@ -84,16 +85,16 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
}
@Override
- protected Group toScimForCreation(GroupModel groupModel) {
+ protected Group scimRequestBodyForCreate(GroupModel groupModel) {
Set<KeycloakId> members = getKeycloakDao().getGroupMembers(groupModel);
Group group = new Group();
group.setExternalId(groupModel.getId());
group.setDisplayName(groupModel.getName());
for (KeycloakId member : members) {
Member groupMember = new Member();
- Optional<ScimResource> optionalGroupMemberMapping = getScimResourceDao().findUserById(member);
+ Optional<ScimResourceMapping> optionalGroupMemberMapping = getScimResourceDao().findUserById(member);
if (optionalGroupMemberMapping.isPresent()) {
- ScimResource groupMemberMapping = optionalGroupMemberMapping.get();
+ ScimResourceMapping groupMemberMapping = optionalGroupMemberMapping.get();
EntityOnRemoteScimId externalIdAsEntityOnRemoteScimId = groupMemberMapping.getExternalIdAsEntityOnRemoteScimId();
groupMember.setValue(externalIdAsEntityOnRemoteScimId.asString());
URI ref = getUri(ScimResourceType.USER, externalIdAsEntityOnRemoteScimId);
@@ -108,8 +109,8 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
}
@Override
- protected Group toScimForReplace(GroupModel groupModel, EntityOnRemoteScimId externalId) {
- Group group = toScimForCreation(groupModel);
+ protected Group scimRequestBodyForUpdate(GroupModel groupModel, EntityOnRemoteScimId externalId) {
+ Group group = scimRequestBodyForCreate(groupModel);
group.setId(externalId.asString());
Meta meta = newMetaLocation(externalId);
group.setMeta(meta);
@@ -117,7 +118,7 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
}
@Override
- protected boolean isSkipRefresh(GroupModel resource) {
+ protected boolean shouldIgnoreForScimSynchronization(GroupModel resource) {
return false;
}
}
diff --git a/src/main/java/sh/libre/scim/core/KeycloakId.java b/src/main/java/sh/libre/scim/core/KeycloakId.java
index f35817dfbe97bb5232bca9784eabf6b7eb9d7564..432892e6f1cd4940682a0302daef12b644b75789 100644
--- a/src/main/java/sh/libre/scim/core/KeycloakId.java
+++ b/src/main/java/sh/libre/scim/core/KeycloakId.java
@@ -1,5 +1,6 @@
package sh.libre.scim.core;
+// TODO rename this
public record KeycloakId(
String asString
) {
diff --git a/src/main/java/sh/libre/scim/core/ScimClient.java b/src/main/java/sh/libre/scim/core/ScimClient.java
index caa318a60cbfd2907969f876fada42ed5001d895..9bdf3cce88077de6bd7902ecbbf69379ae0a14f8 100644
--- a/src/main/java/sh/libre/scim/core/ScimClient.java
+++ b/src/main/java/sh/libre/scim/core/ScimClient.java
@@ -12,6 +12,7 @@ import io.github.resilience4j.retry.RetryConfig;
import io.github.resilience4j.retry.RetryRegistry;
import jakarta.ws.rs.ProcessingException;
import org.jboss.logging.Logger;
+import sh.libre.scim.core.exceptions.ErrorForScimEndpointException;
import java.util.Collections;
import java.util.HashMap;
@@ -63,13 +64,14 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
return new ScimClient(scimRequestBuilder, scimResourceType);
}
- public EntityOnRemoteScimId create(KeycloakId id, S scimForCreation) throws ScimPropagationException {
+ public EntityOnRemoteScimId create(KeycloakId id, S scimForCreation) throws ErrorForScimEndpointException {
Optional<String> scimForCreationId = scimForCreation.getId();
if (scimForCreationId.isPresent()) {
- throw new ScimPropagationException(
- "%s is already created on remote with id %s".formatted(id, scimForCreationId.get())
+ throw new IllegalArgumentException(
+ "User to create should never have an existing id: %s %s".formatted(id, scimForCreationId.get())
);
}
+ // TODO Exception handling : check that all exceptions are wrapped in server response
Retry retry = retryRegistry.retry("create-%s".formatted(id.asString()));
ServerResponse<S> response = retry.executeSupplier(() -> scimRequestBuilder
.create(getResourceClass(), getScimEndpoint())
@@ -80,15 +82,12 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
S resource = response.getResource();
return resource.getId()
.map(EntityOnRemoteScimId::new)
- .orElseThrow(() -> new IllegalStateException("created resource does not have id"));
+ .orElseThrow(() -> new ErrorForScimEndpointException("Created SCIM resource does not have id"));
}
- private void checkResponseIsSuccess(ServerResponse<S> response) {
+ private void checkResponseIsSuccess(ServerResponse<S> response) throws ErrorForScimEndpointException {
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());
+ throw new ErrorForScimEndpointException("Server answered with status " + response.getResponseBody() + ": " + response.getResponseBody());
}
}
@@ -100,8 +99,9 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
return scimResourceType.getResourceClass();
}
- public void replace(EntityOnRemoteScimId externalId, S scimForReplace) {
+ public void update(EntityOnRemoteScimId externalId, S scimForReplace) throws ErrorForScimEndpointException {
Retry retry = retryRegistry.retry("replace-%s".formatted(externalId.asString()));
+ // TODO Exception handling : check that all exceptions are wrapped in server response
ServerResponse<S> response = retry.executeSupplier(() -> scimRequestBuilder
.update(getResourceClass(), getScimEndpoint(), externalId.asString())
.setResource(scimForReplace)
@@ -110,8 +110,9 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
checkResponseIsSuccess(response);
}
- public void delete(EntityOnRemoteScimId externalId) {
+ public void delete(EntityOnRemoteScimId externalId) throws ErrorForScimEndpointException {
Retry retry = retryRegistry.retry("delete-%s".formatted(externalId.asString()));
+ // TODO Exception handling : check that all exceptions are wrapped in server response
ServerResponse<S> response = retry.executeSupplier(() -> scimRequestBuilder
.delete(getResourceClass(), getScimEndpoint(), externalId.asString())
.sendRequest()
diff --git a/src/main/java/sh/libre/scim/core/ScimDispatcher.java b/src/main/java/sh/libre/scim/core/ScimDispatcher.java
index 2ec51aa924ddbd6b5b32fdafa6bac6a2cc00e83a..927d0f9e025502ebd71892ee42aba30e086e00f7 100644
--- a/src/main/java/sh/libre/scim/core/ScimDispatcher.java
+++ b/src/main/java/sh/libre/scim/core/ScimDispatcher.java
@@ -3,6 +3,8 @@ package sh.libre.scim.core;
import org.jboss.logging.Logger;
import org.keycloak.component.ComponentModel;
import org.keycloak.models.KeycloakSession;
+import sh.libre.scim.core.exceptions.ScimExceptionHandler;
+import sh.libre.scim.core.exceptions.ScimPropagationException;
import sh.libre.scim.storage.ScimEndpointConfigurationStorageProviderFactory;
import java.util.ArrayList;
diff --git a/src/main/java/sh/libre/scim/core/UserScimService.java b/src/main/java/sh/libre/scim/core/UserScimService.java
index fbae36236b4fee916133a273e8b1d35ea2dbbc13..23b20795b2d49df8844e5a0aed6a6123a02f209d 100644
--- a/src/main/java/sh/libre/scim/core/UserScimService.java
+++ b/src/main/java/sh/libre/scim/core/UserScimService.java
@@ -13,6 +13,8 @@ import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RoleMapperModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.UserModel;
+import sh.libre.scim.core.exceptions.InconsistentScimDataException;
+import sh.libre.scim.core.exceptions.UnexpectedScimDataException;
import java.util.ArrayList;
import java.util.List;
@@ -39,7 +41,7 @@ public class UserScimService extends AbstractScimService<UserModel, User> {
}
@Override
- protected Optional<KeycloakId> tryToMap(User resource) {
+ protected Optional<KeycloakId> matchKeycloakMappingByScimProperties(User resource) throws InconsistentScimDataException {
Optional<KeycloakId> matchedByUsername = resource.getUserName()
.map(getKeycloakDao()::getUserByUsername)
.map(this::getId);
@@ -51,9 +53,7 @@ public class UserScimService extends AbstractScimService<UserModel, User> {
if (matchedByUsername.isPresent()
&& 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();
+ throw new InconsistentScimDataException("Found 2 possible users for remote user " + matchedByUsername.get() + " - " + matchedByEmail.get());
}
if (matchedByUsername.isPresent()) {
return matchedByUsername;
@@ -62,21 +62,22 @@ public class UserScimService extends AbstractScimService<UserModel, User> {
}
@Override
- protected KeycloakId createEntity(User resource) throws ScimPropagationException {
+ protected KeycloakId createEntity(User resource) throws UnexpectedScimDataException {
String username = resource.getUserName()
.filter(StringUtils::isNotBlank)
- .orElseThrow(() -> new ScimPropagationException("can't create user with empty username, resource id = %s".formatted(resource.getId())));
+ .orElseThrow(() -> new UnexpectedScimDataException("Remote Scim user has empty username, can't create. Resource id = %s".formatted(resource.getId())));
UserModel user = getKeycloakDao().addUser(username);
resource.getEmails().stream()
.findFirst()
.flatMap(MultiComplexNode::getValue)
.ifPresent(user::setEmail);
- user.setEnabled(resource.isActive().orElseThrow(() -> new ScimPropagationException("can't create user with undefined 'active', resource id = %s".formatted(resource.getId()))));
+ boolean userEnabled = resource.isActive().orElse(false);
+ user.setEnabled(userEnabled);
return new KeycloakId(user.getId());
}
@Override
- protected boolean isSkip(UserModel userModel) {
+ protected boolean isMarkedToIgnore(UserModel userModel) {
return BooleanUtils.TRUE.equals(userModel.getFirstAttribute("scim-skip"));
}
@@ -86,7 +87,7 @@ public class UserScimService extends AbstractScimService<UserModel, User> {
}
@Override
- protected User toScimForCreation(UserModel roleMapperModel) {
+ protected User scimRequestBodyForCreate(UserModel roleMapperModel) {
String firstAndLastName = String.format("%s %s",
StringUtils.defaultString(roleMapperModel.getFirstName()),
StringUtils.defaultString(roleMapperModel.getLastName())).trim();
@@ -123,8 +124,8 @@ public class UserScimService extends AbstractScimService<UserModel, User> {
}
@Override
- protected User toScimForReplace(UserModel userModel, EntityOnRemoteScimId externalId) {
- User user = toScimForCreation(userModel);
+ protected User scimRequestBodyForUpdate(UserModel userModel, EntityOnRemoteScimId externalId) {
+ User user = scimRequestBodyForCreate(userModel);
user.setId(externalId.asString());
Meta meta = newMetaLocation(externalId);
user.setMeta(meta);
@@ -132,7 +133,7 @@ public class UserScimService extends AbstractScimService<UserModel, User> {
}
@Override
- protected boolean isSkipRefresh(UserModel userModel) {
+ protected boolean shouldIgnoreForScimSynchronization(UserModel userModel) {
return "admin".equals(userModel.getUsername());
}
}
diff --git a/src/main/java/sh/libre/scim/core/exceptions/ErrorForScimEndpointException.java b/src/main/java/sh/libre/scim/core/exceptions/ErrorForScimEndpointException.java
new file mode 100644
index 0000000000000000000000000000000000000000..1705e9981da8d9c73d3256b6c635e3739e858e91
--- /dev/null
+++ b/src/main/java/sh/libre/scim/core/exceptions/ErrorForScimEndpointException.java
@@ -0,0 +1,8 @@
+package sh.libre.scim.core.exceptions;
+
+public class ErrorForScimEndpointException extends ScimPropagationException {
+
+ public ErrorForScimEndpointException(String message) {
+ super(message);
+ }
+}
diff --git a/src/main/java/sh/libre/scim/core/exceptions/InconsistentScimDataException.java b/src/main/java/sh/libre/scim/core/exceptions/InconsistentScimDataException.java
new file mode 100644
index 0000000000000000000000000000000000000000..947a9f1eb37c43a60c9a06cfa5e5eb13463c7f03
--- /dev/null
+++ b/src/main/java/sh/libre/scim/core/exceptions/InconsistentScimDataException.java
@@ -0,0 +1,7 @@
+package sh.libre.scim.core.exceptions;
+
+public class InconsistentScimDataException extends ScimPropagationException {
+ public InconsistentScimDataException(String message) {
+ super(message);
+ }
+}
diff --git a/src/main/java/sh/libre/scim/core/ScimExceptionHandler.java b/src/main/java/sh/libre/scim/core/exceptions/ScimExceptionHandler.java
similarity index 94%
rename from src/main/java/sh/libre/scim/core/ScimExceptionHandler.java
rename to src/main/java/sh/libre/scim/core/exceptions/ScimExceptionHandler.java
index 47b8c718dad5aac127aaef6ea5f6b3346d50feef..81afeeade0f5ac3ca946aef8ec8e93a48fa6ab06 100644
--- a/src/main/java/sh/libre/scim/core/ScimExceptionHandler.java
+++ b/src/main/java/sh/libre/scim/core/exceptions/ScimExceptionHandler.java
@@ -1,8 +1,9 @@
-package sh.libre.scim.core;
+package sh.libre.scim.core.exceptions;
import org.jboss.logging.Logger;
import org.keycloak.component.ComponentModel;
import org.keycloak.models.KeycloakSession;
+import sh.libre.scim.core.ScrimProviderConfiguration;
/**
* In charge of dealing with SCIM exceptions by ignoring, logging or rollback transaction according to :
diff --git a/src/main/java/sh/libre/scim/core/ScimPropagationException.java b/src/main/java/sh/libre/scim/core/exceptions/ScimPropagationException.java
similarity index 64%
rename from src/main/java/sh/libre/scim/core/ScimPropagationException.java
rename to src/main/java/sh/libre/scim/core/exceptions/ScimPropagationException.java
index 135b555961234788d6e3b6ede89905bfa761dcad..7b1a747b2167bd19791965fd1e775c64328cdfe0 100644
--- a/src/main/java/sh/libre/scim/core/ScimPropagationException.java
+++ b/src/main/java/sh/libre/scim/core/exceptions/ScimPropagationException.java
@@ -1,6 +1,6 @@
-package sh.libre.scim.core;
+package sh.libre.scim.core.exceptions;
-public class ScimPropagationException extends Exception {
+public abstract class ScimPropagationException extends Exception {
public ScimPropagationException(String message) {
super(message);
diff --git a/src/main/java/sh/libre/scim/core/exceptions/UnexpectedScimDataException.java b/src/main/java/sh/libre/scim/core/exceptions/UnexpectedScimDataException.java
new file mode 100644
index 0000000000000000000000000000000000000000..918127ef0b3c2244d26ddeb41a16a1e44c40befb
--- /dev/null
+++ b/src/main/java/sh/libre/scim/core/exceptions/UnexpectedScimDataException.java
@@ -0,0 +1,7 @@
+package sh.libre.scim.core.exceptions;
+
+public class UnexpectedScimDataException extends ScimPropagationException {
+ public UnexpectedScimDataException(String message) {
+ super(message);
+ }
+}
diff --git a/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java b/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java
index 546522294073ec6a0729511a6848931875cea595..4d6f7f470f5e2d0759e75340fbbaa77609084da4 100644
--- a/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java
+++ b/src/main/java/sh/libre/scim/event/ScimEventListenerProvider.java
@@ -33,7 +33,7 @@ public class ScimEventListenerProvider implements EventListenerProvider {
private final KeycloakSession session;
- private final KeycloakDao keycloackDao;
+ private final KeycloakDao keycloakDao;
private final Map<ResourceType, Pattern> patterns = Map.of(
ResourceType.USER, Pattern.compile("users/(.+)"),
@@ -45,7 +45,7 @@ public class ScimEventListenerProvider implements EventListenerProvider {
public ScimEventListenerProvider(KeycloakSession session) {
this.session = session;
- this.keycloackDao = new KeycloakDao(session);
+ this.keycloakDao = new KeycloakDao(session);
this.dispatcher = new ScimDispatcher(session);
}
@@ -63,7 +63,7 @@ public class ScimEventListenerProvider implements EventListenerProvider {
case UPDATE_EMAIL, UPDATE_PROFILE -> {
LOGGER.infof("[SCIM] Propagate User %s - %s", eventType, eventUserId);
UserModel user = getUser(eventUserId);
- dispatcher.dispatchUserModificationToAll(client -> client.replace(user));
+ dispatcher.dispatchUserModificationToAll(client -> client.update(user));
}
case DELETE_ACCOUNT -> {
LOGGER.infof("[SCIM] Propagate User deletion - %s", eventUserId);
@@ -131,12 +131,12 @@ public class ScimEventListenerProvider implements EventListenerProvider {
UserModel user = getUser(userId);
dispatcher.dispatchUserModificationToAll(client -> client.create(user));
user.getGroupsStream().forEach(group ->
- dispatcher.dispatchGroupModificationToAll(client -> client.replace(group)
+ dispatcher.dispatchGroupModificationToAll(client -> client.update(group)
));
}
case UPDATE -> {
UserModel user = getUser(userId);
- dispatcher.dispatchUserModificationToAll(client -> client.replace(user));
+ dispatcher.dispatchUserModificationToAll(client -> client.update(user));
}
case DELETE -> dispatcher.dispatchUserModificationToAll(client -> client.delete(userId));
default -> {
@@ -160,7 +160,7 @@ public class ScimEventListenerProvider implements EventListenerProvider {
}
case UPDATE -> {
GroupModel group = getGroup(groupId);
- dispatcher.dispatchGroupModificationToAll(client -> client.replace(group));
+ dispatcher.dispatchGroupModificationToAll(client -> client.update(group));
}
case DELETE -> dispatcher.dispatchGroupModificationToAll(client -> client.delete(groupId));
default -> {
@@ -174,7 +174,7 @@ public class ScimEventListenerProvider implements EventListenerProvider {
GroupModel group = getGroup(groupId);
group.setSingleAttribute("scim-dirty", BooleanUtils.TRUE);
UserModel user = getUser(userId);
- dispatcher.dispatchUserModificationToAll(client -> client.replace(user));
+ dispatcher.dispatchUserModificationToAll(client -> client.update(user));
}
private void handleRoleMappingEvent(AdminEvent roleMappingEvent, ScimResourceType type, KeycloakId id) {
@@ -182,14 +182,14 @@ public class ScimEventListenerProvider implements EventListenerProvider {
switch (type) {
case USER -> {
UserModel user = getUser(id);
- dispatcher.dispatchUserModificationToAll(client -> client.replace(user));
+ dispatcher.dispatchUserModificationToAll(client -> client.update(user));
}
case GROUP -> {
GroupModel group = getGroup(id);
session.users()
.getGroupMembersStream(session.getContext().getRealm(), group)
.forEach(user ->
- dispatcher.dispatchUserModificationToAll(client -> client.replace(user)
+ dispatcher.dispatchUserModificationToAll(client -> client.update(user)
));
}
default -> {
@@ -204,7 +204,7 @@ public class ScimEventListenerProvider implements EventListenerProvider {
// 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
+ // TODO : determine if deleted element is of ScimStorageProvider class and only refresh in that case
dispatcher.refreshActiveScimEndpoints();
} else {
// In case of CREATE or UPDATE, we can directly use the string representation
@@ -219,11 +219,11 @@ public class ScimEventListenerProvider implements EventListenerProvider {
private UserModel getUser(KeycloakId id) {
- return keycloackDao.getUserById(id);
+ return keycloakDao.getUserById(id);
}
private GroupModel getGroup(KeycloakId id) {
- return keycloackDao.getGroupById(id);
+ return keycloakDao.getGroupById(id);
}
@Override
diff --git a/src/main/java/sh/libre/scim/jpa/ScimResourceDao.java b/src/main/java/sh/libre/scim/jpa/ScimResourceDao.java
index 7d96b2819d42b320a6f026b47b1b4d5046170d0b..97db33b9ea3caf8dad20e7112420481c495c775e 100644
--- a/src/main/java/sh/libre/scim/jpa/ScimResourceDao.java
+++ b/src/main/java/sh/libre/scim/jpa/ScimResourceDao.java
@@ -44,7 +44,7 @@ public class ScimResourceDao {
}
public void create(KeycloakId id, EntityOnRemoteScimId externalId, ScimResourceType type) {
- ScimResource entity = new ScimResource();
+ ScimResourceMapping entity = new ScimResourceMapping();
entity.setType(type.name());
entity.setExternalId(externalId.asString());
entity.setComponentId(componentId);
@@ -53,16 +53,16 @@ public class ScimResourceDao {
entityManager.persist(entity);
}
- private TypedQuery<ScimResource> getScimResourceTypedQuery(String queryName, String id, ScimResourceType type) {
+ private TypedQuery<ScimResourceMapping> getScimResourceTypedQuery(String queryName, String id, ScimResourceType type) {
return getEntityManager()
- .createNamedQuery(queryName, ScimResource.class)
+ .createNamedQuery(queryName, ScimResourceMapping.class)
.setParameter("type", type.name())
.setParameter("realmId", getRealmId())
.setParameter("componentId", getComponentId())
.setParameter("id", id);
}
- public Optional<ScimResource> findByExternalId(EntityOnRemoteScimId externalId, ScimResourceType type) {
+ public Optional<ScimResourceMapping> findByExternalId(EntityOnRemoteScimId externalId, ScimResourceType type) {
try {
return Optional.of(
getScimResourceTypedQuery("findByExternalId", externalId.asString(), type).getSingleResult()
@@ -72,7 +72,7 @@ public class ScimResourceDao {
}
}
- public Optional<ScimResource> findById(KeycloakId keycloakId, ScimResourceType type) {
+ public Optional<ScimResourceMapping> findById(KeycloakId keycloakId, ScimResourceType type) {
try {
return Optional.of(
getScimResourceTypedQuery("findById", keycloakId.asString(), type).getSingleResult()
@@ -82,15 +82,15 @@ public class ScimResourceDao {
}
}
- public Optional<ScimResource> findUserById(KeycloakId id) {
+ public Optional<ScimResourceMapping> findUserById(KeycloakId id) {
return findById(id, ScimResourceType.USER);
}
- public Optional<ScimResource> findUserByExternalId(EntityOnRemoteScimId externalId) {
+ public Optional<ScimResourceMapping> findUserByExternalId(EntityOnRemoteScimId externalId) {
return findByExternalId(externalId, ScimResourceType.USER);
}
- public void delete(ScimResource resource) {
+ public void delete(ScimResourceMapping resource) {
EntityManager entityManager = getEntityManager();
entityManager.remove(resource);
}
diff --git a/src/main/java/sh/libre/scim/jpa/ScimResource.java b/src/main/java/sh/libre/scim/jpa/ScimResourceMapping.java
similarity index 96%
rename from src/main/java/sh/libre/scim/jpa/ScimResource.java
rename to src/main/java/sh/libre/scim/jpa/ScimResourceMapping.java
index 5536a6dd965cc447057738dab647a8813a600332..26eafa04c2315b5bef0b60309d3b04ca71821f50 100644
--- a/src/main/java/sh/libre/scim/jpa/ScimResource.java
+++ b/src/main/java/sh/libre/scim/jpa/ScimResourceMapping.java
@@ -12,12 +12,12 @@ import sh.libre.scim.core.KeycloakId;
@Entity
@IdClass(ScimResourceId.class)
-@Table(name = "SCIM_RESOURCE")
+@Table(name = "SCIM_RESOURCE_MAPPING")
@NamedQueries({
@NamedQuery(name = "findById", query = "from ScimResource where realmId = :realmId and componentId = :componentId and type = :type and id = :id"),
@NamedQuery(name = "findByExternalId", query = "from ScimResource where realmId = :realmId and componentId = :componentId and type = :type and externalId = :id")
})
-public class ScimResource {
+public class ScimResourceMapping {
@Id
@Column(name = "ID", nullable = false)
diff --git a/src/main/java/sh/libre/scim/jpa/ScimResourceProvider.java b/src/main/java/sh/libre/scim/jpa/ScimResourceProvider.java
index ab12d4cc43506931e2af59337558a0e69f4ee8ac..3dc284e22fffaf86fb1d63ab560521998fef62f2 100644
--- a/src/main/java/sh/libre/scim/jpa/ScimResourceProvider.java
+++ b/src/main/java/sh/libre/scim/jpa/ScimResourceProvider.java
@@ -9,7 +9,7 @@ public class ScimResourceProvider implements JpaEntityProvider {
@Override
public List<Class<?>> getEntities() {
- return Collections.singletonList(ScimResource.class);
+ return Collections.singletonList(ScimResourceMapping.class);
}
@Override
diff --git a/src/main/java/sh/libre/scim/storage/ScimEndpointConfigurationStorageProviderFactory.java b/src/main/java/sh/libre/scim/storage/ScimEndpointConfigurationStorageProviderFactory.java
index 724fa925b72b3082b78d2324d22a4115f42567e8..8bc99d6211ea57b40abbb6c4052f6e7964649bd8 100644
--- a/src/main/java/sh/libre/scim/storage/ScimEndpointConfigurationStorageProviderFactory.java
+++ b/src/main/java/sh/libre/scim/storage/ScimEndpointConfigurationStorageProviderFactory.java
@@ -79,7 +79,7 @@ public class ScimEndpointConfigurationStorageProviderFactory
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());
- dispatcher.dispatchGroupModificationToAll(client -> client.replace(group));
+ dispatcher.dispatchGroupModificationToAll(client -> client.update(group));
group.removeAttribute("scim-dirty");
}
dispatcher.close();