Forbid changing ID

Closes: #16881
This commit is contained in:
Hynek Mlnarik 2023-02-21 13:25:03 +01:00 committed by Michal Hajas
parent b9ab942ef8
commit 878debd2ab
4 changed files with 47 additions and 3 deletions

View file

@ -22,11 +22,13 @@ import org.keycloak.models.map.common.DeepCloner;
import org.keycloak.models.map.common.StringKeyConverter; import org.keycloak.models.map.common.StringKeyConverter;
import org.keycloak.models.map.common.StringKeyConverter.StringKey; import org.keycloak.models.map.common.StringKeyConverter.StringKey;
import org.keycloak.models.map.common.UpdatableEntity; import org.keycloak.models.map.common.UpdatableEntity;
import org.keycloak.models.map.common.delegate.EntityFieldDelegate;
import org.keycloak.models.map.storage.MapKeycloakTransaction; import org.keycloak.models.map.storage.MapKeycloakTransaction;
import org.keycloak.models.map.storage.ModelEntityUtil; import org.keycloak.models.map.storage.ModelEntityUtil;
import org.keycloak.models.map.storage.chm.ConcurrentHashMapKeycloakTransaction; import org.keycloak.models.map.storage.chm.ConcurrentHashMapKeycloakTransaction;
import org.keycloak.models.map.storage.chm.MapFieldPredicates; import org.keycloak.models.map.storage.chm.MapFieldPredicates;
import org.keycloak.models.map.storage.chm.MapModelCriteriaBuilder.UpdatePredicatesFunc; import org.keycloak.models.map.storage.chm.MapModelCriteriaBuilder.UpdatePredicatesFunc;
import org.keycloak.storage.ReadOnlyException;
import org.keycloak.storage.SearchableModelField; import org.keycloak.storage.SearchableModelField;
import java.io.IOException; import java.io.IOException;
import java.io.UncheckedIOException; import java.io.UncheckedIOException;
@ -38,6 +40,7 @@ import java.util.HashMap;
import java.util.LinkedList; import java.util.LinkedList;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Objects;
import java.util.function.Function; import java.util.function.Function;
/** /**
@ -82,11 +85,11 @@ public class FileMapKeycloakTransaction<V extends AbstractEntity & UpdatableEnti
@Override @Override
public void commit() { public void commit() {
super.commit(); super.commit();
this.renameOnCommit.forEach(FileMapKeycloakTransaction::silentMove); this.renameOnCommit.forEach(FileMapKeycloakTransaction::move);
this.pathsToDelete.forEach(FileMapKeycloakTransaction::silentDelete); this.pathsToDelete.forEach(FileMapKeycloakTransaction::silentDelete);
} }
private static void silentMove(Path from, Path to) { private static void move(Path from, Path to) {
try { try {
Files.move(from, to, StandardCopyOption.REPLACE_EXISTING); Files.move(from, to, StandardCopyOption.REPLACE_EXISTING);
} catch (IOException ex) { } catch (IOException ex) {
@ -121,6 +124,12 @@ public class FileMapKeycloakTransaction<V extends AbstractEntity & UpdatableEnti
this.pathsToDelete.add(from); this.pathsToDelete.add(from);
} }
@Override
public V registerEntityForChanges(V origEntity) {
final V watchedValue = super.registerEntityForChanges(origEntity);
return DeepCloner.DUMB_CLONER.entityFieldDelegate(watchedValue, new IdProtector(watchedValue));
}
private static class Crud<V extends AbstractEntity & UpdatableEntity, M> extends FileMapStorage.Crud<V, M> { private static class Crud<V extends AbstractEntity & UpdatableEntity, M> extends FileMapStorage.Crud<V, M> {
private FileMapKeycloakTransaction tx; private FileMapKeycloakTransaction tx;
@ -148,6 +157,26 @@ public class FileMapKeycloakTransaction<V extends AbstractEntity & UpdatableEnti
protected String getTxId() { protected String getTxId() {
return tx.txId; return tx.txId;
} }
}
private class IdProtector extends EntityFieldDelegate.WithEntity<V> {
public IdProtector(V entity) {
super(entity);
}
@Override
public <T, EF extends java.lang.Enum<? extends org.keycloak.models.map.common.EntityField<V>> & org.keycloak.models.map.common.EntityField<V>> void set(EF field, T value) {
String id = entity.getId();
super.set(field, value);
if (! Objects.equals(id, map.determineKeyFromValue(entity, false))) {
throw new ReadOnlyException("Cannot change " + field + " as that would change primary key");
}
}
@Override
public String toString() {
return super.toString() + " [protected ID]";
}
} }
} }

View file

@ -7,7 +7,7 @@ import org.keycloak.models.map.common.UpdatableEntity;
public interface EntityFieldDelegate<E> extends UpdatableEntity { public interface EntityFieldDelegate<E> extends UpdatableEntity {
public abstract class WithEntity<E extends UpdatableEntity> implements EntityFieldDelegate<E> { public abstract class WithEntity<E extends UpdatableEntity> implements EntityFieldDelegate<E> {
private final E entity; protected final E entity;
public WithEntity(E entity) { public WithEntity(E entity) {
this.entity = entity; this.entity = entity;
@ -52,6 +52,11 @@ public interface EntityFieldDelegate<E> extends UpdatableEntity {
public boolean isUpdated() { public boolean isUpdated() {
return entity.isUpdated(); return entity.isUpdated();
} }
@Override
public String toString() {
return "&" + String.valueOf(entity);
}
} }
// Non-collection values // Non-collection values

View file

@ -224,6 +224,11 @@ public class ModelEntityUtil {
super.set(field, value); super.set(field, value);
} }
} }
@Override
public String toString() {
return super.toString() + " [fixed " + entityField + "=" + value + "]";
}
}); });
} }

View file

@ -206,15 +206,20 @@ public class UserResource {
} }
return Response.noContent().build(); return Response.noContent().build();
} catch (ModelDuplicateException e) { } catch (ModelDuplicateException e) {
session.getTransactionManager().setRollbackOnly();
return ErrorResponse.exists("User exists with same username or email"); return ErrorResponse.exists("User exists with same username or email");
} catch (ReadOnlyException re) { } catch (ReadOnlyException re) {
session.getTransactionManager().setRollbackOnly();
return ErrorResponse.error("User is read only!", Status.BAD_REQUEST); return ErrorResponse.error("User is read only!", Status.BAD_REQUEST);
} catch (ModelException me) { } catch (ModelException me) {
logger.warn("Could not update user!", me); logger.warn("Could not update user!", me);
session.getTransactionManager().setRollbackOnly();
return ErrorResponse.error("Could not update user!", Status.BAD_REQUEST); return ErrorResponse.error("Could not update user!", Status.BAD_REQUEST);
} catch (ForbiddenException fe) { } catch (ForbiddenException fe) {
session.getTransactionManager().setRollbackOnly();
throw fe; throw fe;
} catch (Exception me) { // JPA } catch (Exception me) { // JPA
session.getTransactionManager().setRollbackOnly();
logger.warn("Could not update user!", me);// may be committed by JTA which can't logger.warn("Could not update user!", me);// may be committed by JTA which can't
return ErrorResponse.error("Could not update user!", Status.BAD_REQUEST); return ErrorResponse.error("Could not update user!", Status.BAD_REQUEST);
} }