Exception Handler - Step 5 : basic Rolback Strategy implementation

This commit is contained in:
Alex Morel 2024-06-25 16:45:35 +02:00
parent 09d0b6544b
commit 620c9e84bb
12 changed files with 115 additions and 65 deletions

View file

@ -6,8 +6,8 @@ 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.core.exceptions.ErrorForScimEndpointException;
import sh.libre.scim.core.exceptions.InconsistentScimDataException;
import sh.libre.scim.core.exceptions.InconsistentScimMappingException;
import sh.libre.scim.core.exceptions.InvalidResponseFromScimEndpointException;
import sh.libre.scim.core.exceptions.SkipOrStopStrategy;
import sh.libre.scim.core.exceptions.UnexpectedScimDataException;
import sh.libre.scim.jpa.ScimResourceDao;
@ -45,7 +45,7 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
this.skipOrStopStrategy = skipOrStopStrategy;
}
public void create(K roleMapperModel) throws InconsistentScimDataException, ErrorForScimEndpointException {
public void create(K roleMapperModel) throws InconsistentScimMappingException, InvalidResponseFromScimEndpointException {
if (isMarkedToIgnore(roleMapperModel)) {
// Silently return: resource is explicitly marked as to ignore
return;
@ -53,14 +53,14 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
// If mapping, then we are trying to recreate a user that was already created by import
KeycloakId id = getId(roleMapperModel);
if (findMappingById(id).isPresent()) {
throw new InconsistentScimDataException("Trying to create user with id " + id + ": id already exists in Keycloak database");
throw new InconsistentScimMappingException("Trying to create user with id " + id + ": id already exists in Keycloak database");
}
S scimForCreation = scimRequestBodyForCreate(roleMapperModel);
EntityOnRemoteScimId externalId = scimClient.create(id, scimForCreation);
createMapping(id, externalId);
}
public void update(K roleMapperModel) throws InconsistentScimDataException, ErrorForScimEndpointException {
public void update(K roleMapperModel) throws InconsistentScimMappingException, InvalidResponseFromScimEndpointException {
if (isMarkedToIgnore(roleMapperModel)) {
// Silently return: resource is explicitly marked as to ignore
return;
@ -68,22 +68,22 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
KeycloakId keycloakId = getId(roleMapperModel);
EntityOnRemoteScimId entityOnRemoteScimId = findMappingById(keycloakId)
.map(ScimResourceMapping::getExternalIdAsEntityOnRemoteScimId)
.orElseThrow(() -> new InconsistentScimDataException("Failed to find SCIM mapping for " + keycloakId));
.orElseThrow(() -> new InconsistentScimMappingException("Failed to find SCIM mapping for " + keycloakId));
S scimForReplace = scimRequestBodyForUpdate(roleMapperModel, entityOnRemoteScimId);
scimClient.update(entityOnRemoteScimId, scimForReplace);
}
protected abstract S scimRequestBodyForUpdate(K roleMapperModel, EntityOnRemoteScimId externalId) throws InconsistentScimDataException;
protected abstract S scimRequestBodyForUpdate(K roleMapperModel, EntityOnRemoteScimId externalId) throws InconsistentScimMappingException;
public void delete(KeycloakId id) throws InconsistentScimDataException, ErrorForScimEndpointException {
public void delete(KeycloakId id) throws InconsistentScimMappingException, InvalidResponseFromScimEndpointException {
ScimResourceMapping resource = findMappingById(id)
.orElseThrow(() -> new InconsistentScimDataException("Failed to delete resource %s, scim mapping not found: ".formatted(id)));
.orElseThrow(() -> new InconsistentScimMappingException("Failed to delete resource %s, scim mapping not found: ".formatted(id)));
EntityOnRemoteScimId externalId = resource.getExternalIdAsEntityOnRemoteScimId();
scimClient.delete(externalId);
getScimResourceDao().delete(resource);
}
public void pushAllResourcesToScim(SynchronizationResult syncRes) throws ErrorForScimEndpointException, InconsistentScimDataException {
public void pushAllResourcesToScim(SynchronizationResult syncRes) throws InvalidResponseFromScimEndpointException, InconsistentScimMappingException {
LOGGER.info("[SCIM] Push resources to endpoint " + this.getConfiguration().getEndPoint());
try (Stream<K> resourcesStream = getResourceStream()) {
Set<K> resources = resourcesStream.collect(Collectors.toUnmodifiableSet());
@ -94,14 +94,14 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
}
}
public void pullAllResourcesFromScim(SynchronizationResult syncRes) throws UnexpectedScimDataException, InconsistentScimDataException, ErrorForScimEndpointException {
public void pullAllResourcesFromScim(SynchronizationResult syncRes) throws UnexpectedScimDataException, InconsistentScimMappingException, InvalidResponseFromScimEndpointException {
LOGGER.info("[SCIM] Pull resources from endpoint " + this.getConfiguration().getEndPoint());
for (S resource : scimClient.listResources()) {
pullSingleResourceFromScim(syncRes, resource);
}
}
private void pushSingleResourceToScim(SynchronizationResult syncRes, K resource, KeycloakId id) throws ErrorForScimEndpointException, InconsistentScimDataException {
private void pushSingleResourceToScim(SynchronizationResult syncRes, K resource, KeycloakId id) throws InvalidResponseFromScimEndpointException, InconsistentScimMappingException {
try {
LOGGER.infof("[SCIM] Reconciling local resource %s", id);
if (shouldIgnoreForScimSynchronization(resource)) {
@ -116,13 +116,13 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
create(resource);
}
syncRes.increaseUpdated();
} catch (ErrorForScimEndpointException e) {
} catch (InvalidResponseFromScimEndpointException e) {
if (skipOrStopStrategy.allowPartialSynchronizationWhenPushingToScim(this.getConfiguration())) {
LOGGER.warn("Error while syncing " + id + " to endpoint " + getConfiguration().getEndPoint());
} else {
throw e;
}
} catch (InconsistentScimDataException e) {
} catch (InconsistentScimMappingException e) {
if (skipOrStopStrategy.allowPartialSynchronizationWhenPushingToScim(this.getConfiguration())) {
LOGGER.warn("Inconsistent data for element " + id + " and endpoint " + getConfiguration().getEndPoint());
} else {
@ -132,7 +132,7 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
}
private void pullSingleResourceFromScim(SynchronizationResult syncRes, S resource) throws UnexpectedScimDataException, InconsistentScimDataException, ErrorForScimEndpointException {
private void pullSingleResourceFromScim(SynchronizationResult syncRes, S resource) throws UnexpectedScimDataException, InconsistentScimMappingException, InvalidResponseFromScimEndpointException {
try {
LOGGER.infof("[SCIM] Reconciling remote resource %s", resource);
EntityOnRemoteScimId externalId = resource.getId()
@ -180,13 +180,13 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
} else {
throw e;
}
} catch (InconsistentScimDataException e) {
} catch (InconsistentScimMappingException e) {
if (skipOrStopStrategy.allowPartialSynchronizationWhenPullingFromScim(getConfiguration())) {
LOGGER.warn("[SCIM] Skipping element synchronisation because of inconsistent mapping for element " + resource.getId() + " : " + e.getMessage());
} else {
throw e;
}
} catch (ErrorForScimEndpointException e) {
} catch (InvalidResponseFromScimEndpointException e) {
// Can only occur in case of a DELETE_REMOTE conflict action
if (skipOrStopStrategy.allowPartialSynchronizationWhenPullingFromScim(getConfiguration())) {
LOGGER.warn("[SCIM] Could not delete SCIM resource " + resource.getId() + " during synchronisation");
@ -198,7 +198,7 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
}
protected abstract S scimRequestBodyForCreate(K roleMapperModel) throws InconsistentScimDataException;
protected abstract S scimRequestBodyForCreate(K roleMapperModel) throws InconsistentScimMappingException;
protected abstract KeycloakId getId(K roleMapperModel);
@ -225,13 +225,13 @@ public abstract class AbstractScimService<K extends RoleMapperModel, S extends R
protected abstract Stream<K> getResourceStream();
protected abstract KeycloakId createEntity(S resource) throws UnexpectedScimDataException;
protected abstract KeycloakId createEntity(S resource) throws UnexpectedScimDataException, InconsistentScimMappingException;
protected abstract Optional<KeycloakId> matchKeycloakMappingByScimProperties(S resource) throws InconsistentScimDataException;
protected abstract Optional<KeycloakId> matchKeycloakMappingByScimProperties(S resource) throws InconsistentScimMappingException;
protected abstract boolean entityExists(KeycloakId keycloakId);
public void sync(SynchronizationResult syncRes) throws InconsistentScimDataException, ErrorForScimEndpointException, UnexpectedScimDataException {
public void sync(SynchronizationResult syncRes) throws InconsistentScimMappingException, InvalidResponseFromScimEndpointException, UnexpectedScimDataException {
if (this.scimProviderConfiguration.isPullFromScimSynchronisationActivated()) {
this.pullAllResourcesFromScim(syncRes);
}

View file

@ -10,7 +10,7 @@ import org.jboss.logging.Logger;
import org.keycloak.models.GroupModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.UserModel;
import sh.libre.scim.core.exceptions.InconsistentScimDataException;
import sh.libre.scim.core.exceptions.InconsistentScimMappingException;
import sh.libre.scim.core.exceptions.SkipOrStopStrategy;
import sh.libre.scim.core.exceptions.UnexpectedScimDataException;
import sh.libre.scim.jpa.ScimResourceMapping;
@ -55,7 +55,7 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
}
@Override
protected KeycloakId createEntity(Group resource) throws UnexpectedScimDataException {
protected KeycloakId createEntity(Group resource) throws UnexpectedScimDataException, InconsistentScimMappingException {
String displayName = resource.getDisplayName()
.filter(StringUtils::isNotBlank)
.orElseThrow(() -> new UnexpectedScimDataException("Remote Scim group has empty name, can't create. Resource id = %s".formatted(resource.getId())));
@ -68,8 +68,7 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
.orElseThrow(() -> new UnexpectedScimDataException("can't create group member for group '%s' without id: ".formatted(displayName) + resource));
KeycloakId userId = getScimResourceDao().findUserByExternalId(externalId)
.map(ScimResourceMapping::getIdAsKeycloakId)
// TODO Exception handling : here if think this is a InconsistentScimData : Scim member is valid, but not mapped yet in our keycloak
.orElseThrow(() -> new UnexpectedScimDataException("can't find mapping for group member %s".formatted(externalId)));
.orElseThrow(() -> new InconsistentScimMappingException("can't find mapping for group member %s".formatted(externalId)));
UserModel userModel = getKeycloakDao().getUserById(userId);
userModel.joinGroup(group);
}
@ -88,7 +87,7 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
}
@Override
protected Group scimRequestBodyForCreate(GroupModel groupModel) throws InconsistentScimDataException {
protected Group scimRequestBodyForCreate(GroupModel groupModel) throws InconsistentScimMappingException {
Set<KeycloakId> members = getKeycloakDao().getGroupMembers(groupModel);
Group group = new Group();
group.setExternalId(groupModel.getId());
@ -108,7 +107,7 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
if (skipOrStopStrategy.allowMissingMembersWhenPushingGroupToScim(this.getConfiguration())) {
LOGGER.warn(message);
} else {
throw new InconsistentScimDataException(message);
throw new InconsistentScimMappingException(message);
}
}
}
@ -116,7 +115,7 @@ public class GroupScimService extends AbstractScimService<GroupModel, Group> {
}
@Override
protected Group scimRequestBodyForUpdate(GroupModel groupModel, EntityOnRemoteScimId externalId) throws InconsistentScimDataException {
protected Group scimRequestBodyForUpdate(GroupModel groupModel, EntityOnRemoteScimId externalId) throws InconsistentScimMappingException {
Group group = scimRequestBodyForCreate(groupModel);
group.setId(externalId.asString());
Meta meta = newMetaLocation(externalId);

View file

@ -12,7 +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 sh.libre.scim.core.exceptions.InvalidResponseFromScimEndpointException;
import java.util.Collections;
import java.util.HashMap;
@ -64,7 +64,7 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
return new ScimClient(scimRequestBuilder, scimResourceType);
}
public EntityOnRemoteScimId create(KeycloakId id, S scimForCreation) throws ErrorForScimEndpointException {
public EntityOnRemoteScimId create(KeycloakId id, S scimForCreation) throws InvalidResponseFromScimEndpointException {
Optional<String> scimForCreationId = scimForCreation.getId();
if (scimForCreationId.isPresent()) {
throw new IllegalArgumentException(
@ -82,12 +82,12 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
S resource = response.getResource();
return resource.getId()
.map(EntityOnRemoteScimId::new)
.orElseThrow(() -> new ErrorForScimEndpointException("Created SCIM resource does not have id"));
.orElseThrow(() -> new InvalidResponseFromScimEndpointException(response, "Created SCIM resource does not have id"));
}
private void checkResponseIsSuccess(ServerResponse<S> response) throws ErrorForScimEndpointException {
private void checkResponseIsSuccess(ServerResponse<S> response) throws InvalidResponseFromScimEndpointException {
if (!response.isSuccess()) {
throw new ErrorForScimEndpointException("Server answered with status " + response.getResponseBody() + ": " + response.getResponseBody());
throw new InvalidResponseFromScimEndpointException(response, "Server answered with status " + response.getResponseBody() + ": " + response.getResponseBody());
}
}
@ -99,7 +99,7 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
return scimResourceType.getResourceClass();
}
public void update(EntityOnRemoteScimId externalId, S scimForReplace) throws ErrorForScimEndpointException {
public void update(EntityOnRemoteScimId externalId, S scimForReplace) throws InvalidResponseFromScimEndpointException {
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
@ -110,7 +110,7 @@ public class ScimClient<S extends ResourceNode> implements AutoCloseable {
checkResponseIsSuccess(response);
}
public void delete(EntityOnRemoteScimId externalId) throws ErrorForScimEndpointException {
public void delete(EntityOnRemoteScimId externalId) throws InvalidResponseFromScimEndpointException {
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

View file

@ -13,7 +13,7 @@ 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.InconsistentScimMappingException;
import sh.libre.scim.core.exceptions.SkipOrStopStrategy;
import sh.libre.scim.core.exceptions.UnexpectedScimDataException;
@ -43,7 +43,7 @@ public class UserScimService extends AbstractScimService<UserModel, User> {
}
@Override
protected Optional<KeycloakId> matchKeycloakMappingByScimProperties(User resource) throws InconsistentScimDataException {
protected Optional<KeycloakId> matchKeycloakMappingByScimProperties(User resource) throws InconsistentScimMappingException {
Optional<KeycloakId> matchedByUsername = resource.getUserName()
.map(getKeycloakDao()::getUserByUsername)
.map(this::getId);
@ -55,7 +55,7 @@ public class UserScimService extends AbstractScimService<UserModel, User> {
if (matchedByUsername.isPresent()
&& matchedByEmail.isPresent()
&& !matchedByUsername.equals(matchedByEmail)) {
throw new InconsistentScimDataException("Found 2 possible users for remote user " + matchedByUsername.get() + " - " + matchedByEmail.get());
throw new InconsistentScimMappingException("Found 2 possible users for remote user " + matchedByUsername.get() + " - " + matchedByEmail.get());
}
if (matchedByUsername.isPresent()) {
return matchedByUsername;

View file

@ -1,8 +0,0 @@
package sh.libre.scim.core.exceptions;
public class ErrorForScimEndpointException extends ScimPropagationException {
public ErrorForScimEndpointException(String message) {
super(message);
}
}

View file

@ -1,7 +0,0 @@
package sh.libre.scim.core.exceptions;
public class InconsistentScimDataException extends ScimPropagationException {
public InconsistentScimDataException(String message) {
super(message);
}
}

View file

@ -0,0 +1,7 @@
package sh.libre.scim.core.exceptions;
public class InconsistentScimMappingException extends ScimPropagationException {
public InconsistentScimMappingException(String message) {
super(message);
}
}

View file

@ -0,0 +1,18 @@
package sh.libre.scim.core.exceptions;
import de.captaingoldfish.scim.sdk.client.response.ServerResponse;
public class InvalidResponseFromScimEndpointException extends ScimPropagationException {
private final ServerResponse response;
public InvalidResponseFromScimEndpointException(ServerResponse response, String message) {
super(message);
this.response = response;
}
public ServerResponse getResponse() {
return response;
}
}

View file

@ -0,0 +1,32 @@
package sh.libre.scim.core.exceptions;
import sh.libre.scim.core.ScrimEndPointConfiguration;
public class RollbackOnlyForCriticalErrorsStrategy implements RollbackStrategy {
private boolean shouldRollback(InvalidResponseFromScimEndpointException e) {
int httpStatus = e.getResponse().getHttpStatus();
return httpStatus == 500;
}
@Override
public boolean shouldRollback(ScrimEndPointConfiguration configuration, ScimPropagationException e) {
if (e instanceof InvalidResponseFromScimEndpointException invalidResponseFromScimEndpointException) {
return shouldRollback(invalidResponseFromScimEndpointException);
}
if (e instanceof InconsistentScimMappingException) {
// Occurs when mapping between a SCIM resource and a keycloak user failed (missing, ambiguous..)
// Log can be sufficient here, no rollback required
return false;
}
if (e instanceof UnexpectedScimDataException) {
// Occurs when a SCIM endpoint sends invalid date (e.g. group with empty name, user without ids...)
// No rollback required : we cannot recover. This needs to be fixed in the SCIM endpoint data
return false;
}
// Should not occur
throw new IllegalStateException("Unkown ScimPropagationException", e);
}
}

View file

@ -9,13 +9,15 @@ public class RollbackStrategyFactory {
return switch (approach) {
case ALWAYS_ROLLBACK -> new AlwaysRollbackStrategy();
case NEVER_ROLLBACK -> new NeverRollbackStrategy();
case CRITICAL_ONLY_ROLLBACK -> new RollbackOnlyForCriticalErrorsStrategy();
};
}
public enum RollbackApproach {
ALWAYS_ROLLBACK, NEVER_ROLLBACK
ALWAYS_ROLLBACK, NEVER_ROLLBACK, CRITICAL_ONLY_ROLLBACK
}
private static final class AlwaysRollbackStrategy implements RollbackStrategy {
@Override

View file

@ -17,7 +17,7 @@ public class ScimExceptionHandler {
private final RollbackStrategy rollbackStrategy;
public ScimExceptionHandler(KeycloakSession session) {
this(session, RollbackStrategyFactory.create(RollbackStrategyFactory.RollbackApproach.NEVER_ROLLBACK));
this(session, RollbackStrategyFactory.create(RollbackStrategyFactory.RollbackApproach.CRITICAL_ONLY_ROLLBACK));
}
public ScimExceptionHandler(KeycloakSession session, RollbackStrategy rollbackStrategy) {

View file

@ -1,8 +1,10 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.1.xsd">
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.1.xsd">
<changeSet author="contact@indiehosters.net" id="scim-resource-1.0">
<createTable tableName="SCIM_RESOURCE">
<createTable tableName="SCIM_RESOURCE_MAPPING">
<column name="ID" type="VARCHAR(36)">
<constraints nullable="false"/>
</column>
@ -20,9 +22,14 @@
</column>
</createTable>
<addPrimaryKey constraintName="PK_SCIM_RESOURCE" tableName="SCIM_RESOURCE" columnNames="ID,REALM_ID,TYPE,COMPONENT_ID,EXTERNAL_ID" />
<addForeignKeyConstraint baseTableName="SCIM_RESOURCE" baseColumnNames="REALM_ID" constraintName="FK_SCIM_RESOURCE_REALM" referencedTableName="REALM" referencedColumnNames="ID" onDelete="CASCADE" onUpdate="CASCADE" />
<addForeignKeyConstraint baseTableName="SCIM_RESOURCE" baseColumnNames="COMPONENT_ID" constraintName="FK_SCIM_RESOURCE_COMPONENT" referencedTableName="COMPONENT" referencedColumnNames="ID" onDelete="CASCADE" onUpdate="CASCADE" />
<addPrimaryKey constraintName="PK_SCIM_RESOURCE" tableName="SCIM_RESOURCE"
columnNames="ID,REALM_ID,TYPE,COMPONENT_ID,EXTERNAL_ID"/>
<addForeignKeyConstraint baseTableName="SCIM_RESOURCE" baseColumnNames="REALM_ID"
constraintName="FK_SCIM_RESOURCE_REALM" referencedTableName="REALM"
referencedColumnNames="ID" onDelete="CASCADE" onUpdate="CASCADE"/>
<addForeignKeyConstraint baseTableName="SCIM_RESOURCE" baseColumnNames="COMPONENT_ID"
constraintName="FK_SCIM_RESOURCE_COMPONENT" referencedTableName="COMPONENT"
referencedColumnNames="ID" onDelete="CASCADE" onUpdate="CASCADE"/>
</changeSet>
</databaseChangeLog>