diff --git a/src/main/java/sh/libre/scim/core/AbstractScimService.java b/src/main/java/sh/libre/scim/core/AbstractScimService.java index a0939adb46..9586a23a3f 100644 --- a/src/main/java/sh/libre/scim/core/AbstractScimService.java +++ b/src/main/java/sh/libre/scim/core/AbstractScimService.java @@ -12,8 +12,9 @@ import sh.libre.scim.jpa.ScimResourceDao; import java.net.URI; import java.net.URISyntaxException; -import java.util.NoSuchElementException; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; public abstract class AbstractScimService implements AutoCloseable { @@ -76,52 +77,55 @@ public abstract class AbstractScimService entityOnRemoteScimId = findById(id) + .map(ScimResource::getExternalIdAsEntityOnRemoteScimId); + entityOnRemoteScimId + .ifPresentOrElse( + externalId -> doReplace(roleMapperModel, externalId), + () -> 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); } } + private void doReplace(RMM roleMapperModel, EntityOnRemoteScimId externalId) { + S scimForReplace = toScimForReplace(roleMapperModel, externalId); + scimClient.replace(externalId, scimForReplace); + } + protected abstract S toScimForReplace(RMM roleMapperModel, EntityOnRemoteScimId externalId); public void delete(KeycloakId id) throws ScimPropagationException { - try { - ScimResource resource = findById(id).get(); - EntityOnRemoteScimId externalId = resource.getExternalIdAsEntityOnRemoteScimId(); - scimClient.delete(externalId); - getScimResourceDao().delete(resource); - } catch (NoSuchElementException e) { - throw new ScimPropagationException("Failed to delete resource %s, scim mapping not found: ".formatted(id), e); - } + ScimResource resource = findById(id) + .orElseThrow(() -> new ScimPropagationException("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("Refresh resources"); - getResourceStream().forEach(resource -> { - KeycloakId id = getId(resource); - LOGGER.infof("Reconciling local resource %s", id); - if (!isSkipRefresh(resource)) { - try { - try { - findById(id).get(); - LOGGER.info("Replacing it"); - replace(resource); - } catch (NoSuchElementException e) { - LOGGER.info("Creating it"); - create(resource); - } - syncRes.increaseUpdated(); - } catch (ScimPropagationException e) { - // TODO handle exception + try (Stream resourcesStream = getResourceStream()) { + Set resources = resourcesStream.collect(Collectors.toUnmodifiableSet()); + for (RMM resource : resources) { + KeycloakId id = getId(resource); + LOGGER.infof("Reconciling local resource %s", id); + if (isSkipRefresh(resource)) { + LOGGER.infof("Skip local resource %s", id); + continue; } + if (findById(id).isPresent()) { + LOGGER.info("Replacing it"); + replace(resource); + } else { + LOGGER.info("Creating it"); + create(resource); + } + syncRes.increaseUpdated(); } - }); + } } protected abstract boolean isSkipRefresh(RMM resource); @@ -130,14 +134,16 @@ public abstract class AbstractScimService scimClient = this.scimClient; try { for (S resource : scimClient.listResources()) { try { LOGGER.infof("Reconciling remote resource %s", resource); - EntityOnRemoteScimId externalId = resource.getId().map(EntityOnRemoteScimId::new).get(); - try { - ScimResource mapping = getScimResourceDao().findByExternalId(externalId, type).get(); + EntityOnRemoteScimId externalId = resource.getId() + .map(EntityOnRemoteScimId::new) + .orElseThrow(() -> new ScimPropagationException("remote SCIM resource doesn't have an id")); + Optional optionalMapping = getScimResourceDao().findByExternalId(externalId, type); + if (optionalMapping.isPresent()) { + ScimResource mapping = optionalMapping.get(); if (entityExists(mapping.getIdAsKeycloakId())) { LOGGER.info("Valid mapping found, skipping"); continue; @@ -145,8 +151,6 @@ public abstract class AbstractScimService mapped = tryToMap(resource); @@ -167,7 +171,7 @@ public abstract class AbstractScimService tryToMap(S resource); + protected abstract Optional tryToMap(S resource) throws ScimPropagationException; protected abstract boolean entityExists(KeycloakId keycloakId); diff --git a/src/main/java/sh/libre/scim/core/GroupScimService.java b/src/main/java/sh/libre/scim/core/GroupScimService.java index 99c8c5cb52..687d52a292 100644 --- a/src/main/java/sh/libre/scim/core/GroupScimService.java +++ b/src/main/java/sh/libre/scim/core/GroupScimService.java @@ -3,9 +3,9 @@ package sh.libre.scim.core; import de.captaingoldfish.scim.sdk.common.resources.Group; import de.captaingoldfish.scim.sdk.common.resources.complex.Meta; import de.captaingoldfish.scim.sdk.common.resources.multicomplex.Member; -import jakarta.persistence.NoResultException; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.StringUtils; import org.jboss.logging.Logger; import org.keycloak.models.GroupModel; import org.keycloak.models.KeycloakSession; @@ -15,9 +15,9 @@ import sh.libre.scim.jpa.ScimResource; import java.net.URI; import java.net.URISyntaxException; import java.util.List; -import java.util.NoSuchElementException; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import java.util.stream.Stream; public class GroupScimService extends AbstractScimService { @@ -39,9 +39,9 @@ public class GroupScimService extends AbstractScimService { @Override protected Optional tryToMap(Group resource) { - String externalId = resource.getId().get(); - String displayName = resource.getDisplayName().get(); - Set names = Set.of(externalId, displayName); + Set names = new TreeSet<>(); + resource.getId().ifPresent(names::add); + resource.getDisplayName().ifPresent(names::add); Optional group = getKeycloakDao().getGroupsStream() .filter(groupModel -> names.contains(groupModel.getName())) .findFirst(); @@ -49,28 +49,22 @@ public class GroupScimService extends AbstractScimService { } @Override - protected KeycloakId createEntity(Group resource) { - String displayName = resource.getDisplayName().get(); + protected KeycloakId createEntity(Group resource) throws ScimPropagationException { + String displayName = resource.getDisplayName() + .filter(StringUtils::isNotBlank) + .orElseThrow(() -> new ScimPropagationException("can't create group without name: " + resource)); GroupModel group = getKeycloakDao().createGroup(displayName); List groupMembers = resource.getMembers(); if (CollectionUtils.isNotEmpty(groupMembers)) { for (Member groupMember : groupMembers) { - try { - EntityOnRemoteScimId externalId = new EntityOnRemoteScimId(groupMember.getValue().get()); - ScimResource userMapping = getScimResourceDao().findUserByExternalId(externalId).get(); - KeycloakId userId = userMapping.getIdAsKeycloakId(); - try { - UserModel user = getKeycloakDao().getUserById(userId); - if (user == null) { - throw new NoResultException(); - } - user.joinGroup(group); - } catch (Exception e) { - logger.warn(e); - } - } catch (NoSuchElementException e) { - logger.warnf("member %s not found for scim group %s", groupMember.getValue().get(), resource.getId().get()); - } + EntityOnRemoteScimId externalId = groupMember.getValue() + .map(EntityOnRemoteScimId::new) + .orElseThrow(() -> new ScimPropagationException("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))); + UserModel userModel = getKeycloakDao().getUserById(userId); + userModel.joinGroup(group); } } return new KeycloakId(group.getId()); @@ -94,19 +88,22 @@ public class GroupScimService extends AbstractScimService { group.setDisplayName(groupModel.getName()); for (KeycloakId member : members) { Member groupMember = new Member(); - try { - ScimResource userMapping = getScimResourceDao().findUserById(member).get(); - logger.debug(userMapping.getExternalIdAsEntityOnRemoteScimId()); - logger.debug(userMapping.getIdAsKeycloakId()); - groupMember.setValue(userMapping.getExternalIdAsEntityOnRemoteScimId().asString()); - String refString = String.format("Users/%s", userMapping.getExternalIdAsEntityOnRemoteScimId().asString()); - URI ref = new URI(refString); - groupMember.setRef(ref.toString()); + Optional optionalGroupMemberMapping = getScimResourceDao().findUserById(member); + 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()); + try { + String refString = String.format("Users/%s", externalIdAsEntityOnRemoteScimId.asString()); + URI ref = new URI(refString); + groupMember.setRef(ref.toString()); + } catch (URISyntaxException e) { + logger.warnf("bad ref uri for member " + member); + } group.addMember(groupMember); - } catch (NoSuchElementException e) { + } else { logger.warnf("member %s not found for group %s", member, groupModel.getId()); - } catch (URISyntaxException e) { - logger.warnf("bad ref uri for member " + member); } } return group; diff --git a/src/main/java/sh/libre/scim/core/UserScimService.java b/src/main/java/sh/libre/scim/core/UserScimService.java index 9a65849cc3..3c30023c64 100644 --- a/src/main/java/sh/libre/scim/core/UserScimService.java +++ b/src/main/java/sh/libre/scim/core/UserScimService.java @@ -40,45 +40,37 @@ public class UserScimService extends AbstractScimService { @Override protected Optional tryToMap(User resource) { - String username = resource.getUserName().get(); - String email = resource.getEmails().stream() + Optional matchedByUsername = resource.getUserName() + .map(getKeycloakDao()::getUserByUsername) + .map(this::getId); + Optional matchedByEmail = resource.getEmails().stream() .findFirst() .flatMap(MultiComplexNode::getValue) - .orElse(null); - UserModel sameUsernameUser = null; - UserModel sameEmailUser = null; - if (username != null) { - sameUsernameUser = getKeycloakDao().getUserByUsername(username); - } - if (email != null) { - sameEmailUser = getKeycloakDao().getUserByEmail(email); - } - if ((sameUsernameUser != null && sameEmailUser != null) - && (!StringUtils.equals(sameUsernameUser.getId(), sameEmailUser.getId()))) { - logger.warnf("found 2 possible users for remote user %s %s", username, email); + .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()); return Optional.empty(); } - if (sameUsernameUser != null) { - return Optional.of(getId(sameUsernameUser)); + if (matchedByUsername.isPresent()) { + return matchedByUsername; } - if (sameEmailUser != null) { - return Optional.of(getId(sameEmailUser)); - } - return Optional.empty(); + return matchedByEmail; } @Override - protected KeycloakId createEntity(User resource) { - String username = resource.getUserName().get(); - if (StringUtils.isEmpty(username)) { - throw new IllegalArgumentException("can't create user with empty username"); - } + protected KeycloakId createEntity(User resource) throws ScimPropagationException { + String username = resource.getUserName() + .filter(StringUtils::isNotBlank) + .orElseThrow(() -> new ScimPropagationException("can't create user with empty username, 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().get()); + user.setEnabled(resource.isActive().orElseThrow(() -> new ScimPropagationException("can't create user with undefined 'active', resource id = %s".formatted(resource.getId())))); return new KeycloakId(user.getId()); }