Remove all unsafe Optional.get calls

This commit is contained in:
Brendan Le Ny 2024-06-24 11:39:32 +02:00 committed by Alex Morel
parent 8e97335de4
commit ad60363cbd
3 changed files with 94 additions and 101 deletions

View file

@ -12,8 +12,9 @@ import sh.libre.scim.jpa.ScimResourceDao;
import java.net.URI; import java.net.URI;
import java.net.URISyntaxException; import java.net.URISyntaxException;
import java.util.NoSuchElementException;
import java.util.Optional; import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream; import java.util.stream.Stream;
public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends ResourceNode> implements AutoCloseable { public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends ResourceNode> implements AutoCloseable {
@ -76,52 +77,55 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends
if (isSkip(roleMapperModel)) if (isSkip(roleMapperModel))
return; return;
KeycloakId id = getId(roleMapperModel); KeycloakId id = getId(roleMapperModel);
ScimResource scimResource = findById(id).get(); Optional<EntityOnRemoteScimId> entityOnRemoteScimId = findById(id)
EntityOnRemoteScimId externalId = scimResource.getExternalIdAsEntityOnRemoteScimId(); .map(ScimResource::getExternalIdAsEntityOnRemoteScimId);
S scimForReplace = toScimForReplace(roleMapperModel, externalId); entityOnRemoteScimId
scimClient.replace(externalId, scimForReplace); .ifPresentOrElse(
} catch (NoSuchElementException e) { externalId -> doReplace(roleMapperModel, externalId),
LOGGER.warnf("failed to replace resource %s, scim mapping not found", getId(roleMapperModel)); () -> LOGGER.warnf("failed to replace resource %s, scim mapping not found", id)
);
} catch (Exception e) { } catch (Exception e) {
LOGGER.error(e); LOGGER.error(e);
throw new ScimPropagationException("[SCIM] Error while replacing SCIM resource", 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); protected abstract S toScimForReplace(RMM roleMapperModel, EntityOnRemoteScimId externalId);
public void delete(KeycloakId id) throws ScimPropagationException { public void delete(KeycloakId id) throws ScimPropagationException {
try { ScimResource resource = findById(id)
ScimResource resource = findById(id).get(); .orElseThrow(() -> new ScimPropagationException("Failed to delete resource %s, scim mapping not found: ".formatted(id)));
EntityOnRemoteScimId externalId = resource.getExternalIdAsEntityOnRemoteScimId(); EntityOnRemoteScimId externalId = resource.getExternalIdAsEntityOnRemoteScimId();
scimClient.delete(externalId); scimClient.delete(externalId);
getScimResourceDao().delete(resource); getScimResourceDao().delete(resource);
} catch (NoSuchElementException e) {
throw new ScimPropagationException("Failed to delete resource %s, scim mapping not found: ".formatted(id), e);
}
} }
public void refreshResources(SynchronizationResult syncRes) throws ScimPropagationException { public void refreshResources(SynchronizationResult syncRes) throws ScimPropagationException {
LOGGER.info("Refresh resources"); LOGGER.info("Refresh resources");
getResourceStream().forEach(resource -> { try (Stream<RMM> resourcesStream = getResourceStream()) {
KeycloakId id = getId(resource); Set<RMM> resources = resourcesStream.collect(Collectors.toUnmodifiableSet());
LOGGER.infof("Reconciling local resource %s", id); for (RMM resource : resources) {
if (!isSkipRefresh(resource)) { KeycloakId id = getId(resource);
try { LOGGER.infof("Reconciling local resource %s", id);
try { if (isSkipRefresh(resource)) {
findById(id).get(); LOGGER.infof("Skip local resource %s", id);
LOGGER.info("Replacing it"); continue;
replace(resource);
} catch (NoSuchElementException e) {
LOGGER.info("Creating it");
create(resource);
}
syncRes.increaseUpdated();
} catch (ScimPropagationException e) {
// TODO handle exception
} }
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); protected abstract boolean isSkipRefresh(RMM resource);
@ -130,14 +134,16 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends
public void importResources(SynchronizationResult syncRes) { public void importResources(SynchronizationResult syncRes) {
LOGGER.info("Import"); LOGGER.info("Import");
ScimClient<S> scimClient = this.scimClient;
try { try {
for (S resource : scimClient.listResources()) { for (S resource : scimClient.listResources()) {
try { try {
LOGGER.infof("Reconciling remote resource %s", resource); LOGGER.infof("Reconciling remote resource %s", resource);
EntityOnRemoteScimId externalId = resource.getId().map(EntityOnRemoteScimId::new).get(); EntityOnRemoteScimId externalId = resource.getId()
try { .map(EntityOnRemoteScimId::new)
ScimResource mapping = getScimResourceDao().findByExternalId(externalId, type).get(); .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())) { if (entityExists(mapping.getIdAsKeycloakId())) {
LOGGER.info("Valid mapping found, skipping"); LOGGER.info("Valid mapping found, skipping");
continue; continue;
@ -145,8 +151,6 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends
LOGGER.info("Delete a dangling mapping"); LOGGER.info("Delete a dangling mapping");
getScimResourceDao().delete(mapping); getScimResourceDao().delete(mapping);
} }
} catch (NoSuchElementException e) {
// nothing to do
} }
Optional<KeycloakId> mapped = tryToMap(resource); Optional<KeycloakId> mapped = tryToMap(resource);
@ -167,7 +171,7 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends
break; break;
case DELETE_REMOTE: case DELETE_REMOTE:
LOGGER.info("Delete remote resource"); LOGGER.info("Delete remote resource");
scimClient.delete(externalId); this.scimClient.delete(externalId);
syncRes.increaseRemoved(); syncRes.increaseRemoved();
break; break;
case NOTHING: case NOTHING:
@ -186,9 +190,9 @@ public abstract class AbstractScimService<RMM extends RoleMapperModel, S extends
} }
} }
protected abstract KeycloakId createEntity(S resource); protected abstract KeycloakId createEntity(S resource) throws ScimPropagationException;
protected abstract Optional<KeycloakId> tryToMap(S resource); protected abstract Optional<KeycloakId> tryToMap(S resource) throws ScimPropagationException;
protected abstract boolean entityExists(KeycloakId keycloakId); protected abstract boolean entityExists(KeycloakId keycloakId);

View file

@ -3,9 +3,9 @@ package sh.libre.scim.core;
import de.captaingoldfish.scim.sdk.common.resources.Group; import de.captaingoldfish.scim.sdk.common.resources.Group;
import de.captaingoldfish.scim.sdk.common.resources.complex.Meta; import de.captaingoldfish.scim.sdk.common.resources.complex.Meta;
import de.captaingoldfish.scim.sdk.common.resources.multicomplex.Member; import de.captaingoldfish.scim.sdk.common.resources.multicomplex.Member;
import jakarta.persistence.NoResultException;
import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.jboss.logging.Logger; import org.jboss.logging.Logger;
import org.keycloak.models.GroupModel; import org.keycloak.models.GroupModel;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
@ -15,9 +15,9 @@ import sh.libre.scim.jpa.ScimResource;
import java.net.URI; import java.net.URI;
import java.net.URISyntaxException; import java.net.URISyntaxException;
import java.util.List; import java.util.List;
import java.util.NoSuchElementException;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
import java.util.TreeSet;
import java.util.stream.Stream; import java.util.stream.Stream;
public class GroupScimService extends AbstractScimService<GroupModel, Group> { public class GroupScimService extends AbstractScimService<GroupModel, Group> {
@ -39,9 +39,9 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
@Override @Override
protected Optional<KeycloakId> tryToMap(Group resource) { protected Optional<KeycloakId> tryToMap(Group resource) {
String externalId = resource.getId().get(); Set<String> names = new TreeSet<>();
String displayName = resource.getDisplayName().get(); resource.getId().ifPresent(names::add);
Set<String> names = Set.of(externalId, displayName); resource.getDisplayName().ifPresent(names::add);
Optional<GroupModel> group = getKeycloakDao().getGroupsStream() Optional<GroupModel> group = getKeycloakDao().getGroupsStream()
.filter(groupModel -> names.contains(groupModel.getName())) .filter(groupModel -> names.contains(groupModel.getName()))
.findFirst(); .findFirst();
@ -49,28 +49,22 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
} }
@Override @Override
protected KeycloakId createEntity(Group resource) { protected KeycloakId createEntity(Group resource) throws ScimPropagationException {
String displayName = resource.getDisplayName().get(); String displayName = resource.getDisplayName()
.filter(StringUtils::isNotBlank)
.orElseThrow(() -> new ScimPropagationException("can't create group without name: " + resource));
GroupModel group = getKeycloakDao().createGroup(displayName); GroupModel group = getKeycloakDao().createGroup(displayName);
List<Member> groupMembers = resource.getMembers(); List<Member> groupMembers = resource.getMembers();
if (CollectionUtils.isNotEmpty(groupMembers)) { if (CollectionUtils.isNotEmpty(groupMembers)) {
for (Member groupMember : groupMembers) { for (Member groupMember : groupMembers) {
try { EntityOnRemoteScimId externalId = groupMember.getValue()
EntityOnRemoteScimId externalId = new EntityOnRemoteScimId(groupMember.getValue().get()); .map(EntityOnRemoteScimId::new)
ScimResource userMapping = getScimResourceDao().findUserByExternalId(externalId).get(); .orElseThrow(() -> new ScimPropagationException("can't create group member for group '%s' without id: ".formatted(displayName) + resource));
KeycloakId userId = userMapping.getIdAsKeycloakId(); KeycloakId userId = getScimResourceDao().findUserByExternalId(externalId)
try { .map(ScimResource::getIdAsKeycloakId)
UserModel user = getKeycloakDao().getUserById(userId); .orElseThrow(() -> new ScimPropagationException("can't find mapping for group member %s".formatted(externalId)));
if (user == null) { UserModel userModel = getKeycloakDao().getUserById(userId);
throw new NoResultException(); userModel.joinGroup(group);
}
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());
}
} }
} }
return new KeycloakId(group.getId()); return new KeycloakId(group.getId());
@ -94,19 +88,22 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
group.setDisplayName(groupModel.getName()); group.setDisplayName(groupModel.getName());
for (KeycloakId member : members) { for (KeycloakId member : members) {
Member groupMember = new Member(); Member groupMember = new Member();
try { Optional<ScimResource> optionalGroupMemberMapping = getScimResourceDao().findUserById(member);
ScimResource userMapping = getScimResourceDao().findUserById(member).get(); if (optionalGroupMemberMapping.isPresent()) {
logger.debug(userMapping.getExternalIdAsEntityOnRemoteScimId()); ScimResource groupMemberMapping = optionalGroupMemberMapping.get();
logger.debug(userMapping.getIdAsKeycloakId()); EntityOnRemoteScimId externalIdAsEntityOnRemoteScimId = groupMemberMapping.getExternalIdAsEntityOnRemoteScimId();
groupMember.setValue(userMapping.getExternalIdAsEntityOnRemoteScimId().asString()); logger.debugf("found mapping for group member %s as %s", member, externalIdAsEntityOnRemoteScimId);
String refString = String.format("Users/%s", userMapping.getExternalIdAsEntityOnRemoteScimId().asString()); groupMember.setValue(externalIdAsEntityOnRemoteScimId.asString());
URI ref = new URI(refString); try {
groupMember.setRef(ref.toString()); 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); group.addMember(groupMember);
} catch (NoSuchElementException e) { } else {
logger.warnf("member %s not found for group %s", member, groupModel.getId()); 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; return group;

View file

@ -40,45 +40,37 @@ public class UserScimService extends AbstractScimService<UserModel, User> {
@Override @Override
protected Optional<KeycloakId> tryToMap(User resource) { protected Optional<KeycloakId> tryToMap(User resource) {
String username = resource.getUserName().get(); Optional<KeycloakId> matchedByUsername = resource.getUserName()
String email = resource.getEmails().stream() .map(getKeycloakDao()::getUserByUsername)
.map(this::getId);
Optional<KeycloakId> matchedByEmail = resource.getEmails().stream()
.findFirst() .findFirst()
.flatMap(MultiComplexNode::getValue) .flatMap(MultiComplexNode::getValue)
.orElse(null); .map(getKeycloakDao()::getUserByEmail)
UserModel sameUsernameUser = null; .map(this::getId);
UserModel sameEmailUser = null; if (matchedByUsername.isPresent()
if (username != null) { && matchedByEmail.isPresent()
sameUsernameUser = getKeycloakDao().getUserByUsername(username); && !matchedByUsername.equals(matchedByEmail)) {
} logger.warnf("found 2 possible users for remote user %s %s", matchedByUsername.get(), matchedByEmail.get());
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);
return Optional.empty(); return Optional.empty();
} }
if (sameUsernameUser != null) { if (matchedByUsername.isPresent()) {
return Optional.of(getId(sameUsernameUser)); return matchedByUsername;
} }
if (sameEmailUser != null) { return matchedByEmail;
return Optional.of(getId(sameEmailUser));
}
return Optional.empty();
} }
@Override @Override
protected KeycloakId createEntity(User resource) { protected KeycloakId createEntity(User resource) throws ScimPropagationException {
String username = resource.getUserName().get(); String username = resource.getUserName()
if (StringUtils.isEmpty(username)) { .filter(StringUtils::isNotBlank)
throw new IllegalArgumentException("can't create user with empty username"); .orElseThrow(() -> new ScimPropagationException("can't create user with empty username, resource id = %s".formatted(resource.getId())));
}
UserModel user = getKeycloakDao().addUser(username); UserModel user = getKeycloakDao().addUser(username);
resource.getEmails().stream() resource.getEmails().stream()
.findFirst() .findFirst()
.flatMap(MultiComplexNode::getValue) .flatMap(MultiComplexNode::getValue)
.ifPresent(user::setEmail); .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()); return new KeycloakId(user.getId());
} }