From 6e591305f9043c946b1c460bb997054efb40172e Mon Sep 17 00:00:00 2001 From: Michal Hajas Date: Fri, 8 Oct 2021 15:28:10 +0200 Subject: [PATCH] KEYCLOAK-19481 Make Id and RealmId mutable fields --- .../MapRootAuthenticationSessionEntity.java | 14 ++++++---- .../MapPermissionTicketStore.java | 2 +- .../entity/MapPermissionTicketEntity.java | 13 ++++++--- .../authorization/entity/MapPolicyEntity.java | 13 ++++++--- .../entity/MapResourceEntity.java | 13 ++++++--- .../entity/MapResourceServerEntity.java | 13 ++++++--- .../authorization/entity/MapScopeEntity.java | 13 ++++++--- .../models/map/client/MapClientEntity.java | 13 ++++++--- .../client/MapClientEntityLazyDelegate.java | 5 ++++ .../map/clientscope/MapClientScopeEntity.java | 23 +++++++++------ .../models/map/common/AbstractEntity.java | 2 +- .../models/map/common/Serialization.java | 28 +------------------ .../models/map/group/MapGroupEntity.java | 21 +++++++++----- .../MapUserLoginFailureEntity.java | 13 +++++---- .../models/map/realm/MapRealmEntity.java | 11 ++++++-- .../models/map/role/MapRoleEntity.java | 14 ++++++---- .../ConcurrentHashMapKeycloakTransaction.java | 2 +- .../storage/chm/ConcurrentHashMapStorage.java | 2 +- .../models/map/user/MapUserEntity.java | 23 +++++++++------ .../MapAuthenticatedClientSessionEntity.java | 16 +++++------ .../map/userSession/MapUserSessionEntity.java | 14 ++++++---- 21 files changed, 156 insertions(+), 112 deletions(-) diff --git a/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionEntity.java b/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionEntity.java index 89bc2c0551..73ec32aac3 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionEntity.java @@ -38,14 +38,9 @@ public class MapRootAuthenticationSessionEntity implements AbstractEntity, Updat private int timestamp; private Map authenticationSessions = new ConcurrentHashMap<>(); - protected MapRootAuthenticationSessionEntity() { - this.id = null; - this.realmId = null; - } + protected MapRootAuthenticationSessionEntity() {} public MapRootAuthenticationSessionEntity(String id, String realmId) { - Objects.requireNonNull(realmId, "realmId"); - this.id = id; this.realmId = realmId; } @@ -55,6 +50,13 @@ public class MapRootAuthenticationSessionEntity implements AbstractEntity, Updat return this.id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + @Override public boolean isUpdated() { return this.updated; diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/MapPermissionTicketStore.java b/model/map/src/main/java/org/keycloak/models/map/authorization/MapPermissionTicketStore.java index fc5cc298db..c12ef73691 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/MapPermissionTicketStore.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/MapPermissionTicketStore.java @@ -109,7 +109,7 @@ public class MapPermissionTicketStore implements PermissionTicketStore { + ", Resource: " + resourceId + ", owner: " + owner + ", scopeId: " + scopeId + " already exists."); } - MapPermissionTicketEntity entity = new MapPermissionTicketEntity(null); + MapPermissionTicketEntity entity = new MapPermissionTicketEntity(); entity.setResourceId(resourceId); entity.setRequester(requester); entity.setCreatedTimestamp(System.currentTimeMillis()); diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPermissionTicketEntity.java b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPermissionTicketEntity.java index b33ff517d6..88bb276047 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPermissionTicketEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPermissionTicketEntity.java @@ -24,7 +24,7 @@ import java.util.Objects; public class MapPermissionTicketEntity implements AbstractEntity, UpdatableEntity { - private final String id; + private String id; private String owner; private String requester; private Long createdTimestamp; @@ -39,15 +39,20 @@ public class MapPermissionTicketEntity implements AbstractEntity, UpdatableEntit this.id = id; } - public MapPermissionTicketEntity() { - this.id = null; - } + public MapPermissionTicketEntity() {} @Override public String getId() { return id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + public String getOwner() { return owner; } diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPolicyEntity.java b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPolicyEntity.java index b2ac564845..16395b8adf 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPolicyEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPolicyEntity.java @@ -30,7 +30,7 @@ import java.util.Objects; public class MapPolicyEntity implements AbstractEntity, UpdatableEntity { - private final String id; + private String id; private String name; private String description; private String type; @@ -48,9 +48,7 @@ public class MapPolicyEntity implements AbstractEntity, UpdatableEntity { this.id = id; } - public MapPolicyEntity() { - this.id = null; - } + public MapPolicyEntity() {} public String getName() { return name; @@ -182,6 +180,13 @@ public class MapPolicyEntity implements AbstractEntity, UpdatableEntity { return id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + @Override public boolean isUpdated() { return updated; diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceEntity.java b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceEntity.java index 927bcde36c..0d20073562 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceEntity.java @@ -29,7 +29,7 @@ import java.util.Set; public class MapResourceEntity implements AbstractEntity, UpdatableEntity { - private final String id; + private String id; private String name; private String displayName; private final Set uris = new HashSet<>(); @@ -47,15 +47,20 @@ public class MapResourceEntity implements AbstractEntity, UpdatableEntity { this.id = id; } - public MapResourceEntity() { - this.id = null; - } + public MapResourceEntity() {} @Override public String getId() { return id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + public String getName() { return name; } diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceServerEntity.java b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceServerEntity.java index e5a9816624..c35ccefa13 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceServerEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceServerEntity.java @@ -26,7 +26,7 @@ import java.util.Objects; public class MapResourceServerEntity implements AbstractEntity, UpdatableEntity { - private final String id; + private String id; private boolean updated = false; private boolean allowRemoteResourceManagement; @@ -37,15 +37,20 @@ public class MapResourceServerEntity implements AbstractEntity, UpdatableEntity this.id = id; } - public MapResourceServerEntity() { - this.id = null; - } + public MapResourceServerEntity() {} @Override public String getId() { return id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + public boolean isAllowRemoteResourceManagement() { return allowRemoteResourceManagement; } diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapScopeEntity.java b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapScopeEntity.java index 6567ea2c41..fec2f5dace 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapScopeEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapScopeEntity.java @@ -24,7 +24,7 @@ import java.util.Objects; public class MapScopeEntity implements AbstractEntity, UpdatableEntity { - private final String id; + private String id; private String name; private String displayName; private String iconUri; @@ -35,15 +35,20 @@ public class MapScopeEntity implements AbstractEntity, UpdatableEntity { this.id = id; } - public MapScopeEntity() { - this.id = null; - } + public MapScopeEntity() {} @Override public String getId() { return id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + public String getName() { return name; } diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java index ae80c98baf..bc39403600 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java @@ -41,10 +41,8 @@ public interface MapClientEntity extends AbstractEntity, UpdatableEntity { */ protected boolean updated; private String id; - - protected AbstractClientEntity() { - this.id = null; - } + + protected AbstractClientEntity() {} public AbstractClientEntity(String id) { this.id = id; @@ -55,6 +53,13 @@ public interface MapClientEntity extends AbstractEntity, UpdatableEntity { return this.id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + @Override public boolean isUpdated() { return this.updated; diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntityLazyDelegate.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntityLazyDelegate.java index 95d66c72af..1e0bf81287 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntityLazyDelegate.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntityLazyDelegate.java @@ -468,6 +468,11 @@ public class MapClientEntityLazyDelegate implements MapClientEntity { return getReadDelegate().getId(); } + @Override + public void setId(String id) { + getWriteDelegate().setId(id); + } + @Override public boolean isUpdated() { return isWriteDelegateInitialized() && getWriteDelegate().isUpdated(); diff --git a/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeEntity.java b/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeEntity.java index a583aed543..ff4b594a32 100644 --- a/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeEntity.java @@ -33,8 +33,8 @@ import org.keycloak.models.map.common.UpdatableEntity; public class MapClientScopeEntity implements AbstractEntity, UpdatableEntity { - private final String id; - private final String realmId; + private String id; + private String realmId; private String name; private String protocol; @@ -49,14 +49,9 @@ public class MapClientScopeEntity implements AbstractEntity, UpdatableEntity { */ protected boolean updated; - protected MapClientScopeEntity() { - this.id = null; - this.realmId = null; - } + protected MapClientScopeEntity() {} public MapClientScopeEntity(String id, String realmId) { - Objects.requireNonNull(realmId, "realmId"); - this.id = id; this.realmId = realmId; } @@ -66,6 +61,13 @@ public class MapClientScopeEntity implements AbstractEntity, UpdatableEntity { return this.id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + @Override public boolean isUpdated() { return this.updated; @@ -148,6 +150,11 @@ public class MapClientScopeEntity implements AbstractEntity, UpdatableEntity { return this.realmId; } + public void setRealmId(String realmId) { + this.updated |= !Objects.equals(this.realmId, realmId); + this.realmId = realmId; + } + public Stream getScopeMappings() { return scopeMappings.stream(); } diff --git a/model/map/src/main/java/org/keycloak/models/map/common/AbstractEntity.java b/model/map/src/main/java/org/keycloak/models/map/common/AbstractEntity.java index b8129b4834..80dd9c2502 100644 --- a/model/map/src/main/java/org/keycloak/models/map/common/AbstractEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/common/AbstractEntity.java @@ -23,5 +23,5 @@ package org.keycloak.models.map.common; public interface AbstractEntity { String getId(); - + void setId(String id); } diff --git a/model/map/src/main/java/org/keycloak/models/map/common/Serialization.java b/model/map/src/main/java/org/keycloak/models/map/common/Serialization.java index 1db0d2f650..ae6aba86ab 100644 --- a/model/map/src/main/java/org/keycloak/models/map/common/Serialization.java +++ b/model/map/src/main/java/org/keycloak/models/map/common/Serialization.java @@ -54,7 +54,6 @@ public class Serialization { .setVisibility(PropertyAccessor.FIELD, Visibility.ANY) .activateDefaultTyping(new LaissezFaireSubTypeValidator() /* TODO - see javadoc */, ObjectMapper.DefaultTyping.JAVA_LANG_OBJECT, JsonTypeInfo.As.PROPERTY) .addMixIn(UpdatableEntity.class, IgnoreUpdatedMixIn.class) - .addMixIn(AbstractEntity.class, AbstractEntityMixIn.class) ; public static final ConcurrentHashMap, ObjectReader> READERS = new ConcurrentHashMap<>(); @@ -64,11 +63,6 @@ public class Serialization { @JsonIgnore public abstract boolean isUpdated(); } - abstract class AbstractEntityMixIn { - @JsonTypeInfo(property="id", use=Id.CLASS, include=As.WRAPPER_ARRAY) - abstract Object getId(); - } - static { JavaType type = TypeFactory.unknownType(); JavaType streamType = MAPPER.getTypeFactory().constructParametricType(Stream.class, type); @@ -78,10 +72,6 @@ public class Serialization { public static T from(T orig) { - return from(orig, null); - } - - public static T from(T orig, String newId) { if (orig == null) { return null; } @@ -94,26 +84,10 @@ public class Serialization { ObjectWriter writer = WRITERS.computeIfAbsent(origClass, MAPPER::writerFor); final T res; res = reader.readValue(writer.writeValueAsBytes(orig)); - if (newId != null) { - updateId(origClass, res, newId); - } + return res; } catch (IOException ex) { throw new IllegalStateException(ex); } } - - private static void updateId(Class origClass, AbstractEntity res, K newId) { - Field field = Reflections.findDeclaredField(origClass, "id"); - if (field == null) { - throw new IllegalArgumentException("Cannot find id for " + origClass + " class"); - } - try { - Reflections.setAccessible(field).set(res, newId); - } catch (IllegalArgumentException | IllegalAccessException ex) { - Logger.getLogger(Serialization.class.getName()).log(Level.SEVERE, null, ex); - throw new IllegalArgumentException("Cannot set id for " + origClass + " class"); - } - } - } diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java index 4b516105b6..4809cd39ad 100644 --- a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java @@ -34,7 +34,7 @@ import java.util.Set; public class MapGroupEntity implements AbstractEntity, UpdatableEntity { private String id; - private final String realmId; + private String realmId; private String name; private String parentId; @@ -46,14 +46,9 @@ public class MapGroupEntity implements AbstractEntity, UpdatableEntity { */ protected boolean updated; - protected MapGroupEntity() { - this.id = null; - this.realmId = null; - } + protected MapGroupEntity() {} public MapGroupEntity(String id, String realmId) { - Objects.requireNonNull(realmId, "realmId"); - this.id = id; this.realmId = realmId; } @@ -63,6 +58,13 @@ public class MapGroupEntity implements AbstractEntity, UpdatableEntity { return this.id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + @Override public boolean isUpdated() { return this.updated; @@ -113,6 +115,11 @@ public class MapGroupEntity implements AbstractEntity, UpdatableEntity { return this.realmId; } + public void setRealmId(String realmId) { + this.updated |= !Objects.equals(this.realmId, realmId); + this.realmId = realmId; + } + public Set getGrantedRoles() { return grantedRoles; } diff --git a/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureEntity.java b/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureEntity.java index 5cf90f64a9..ec1ffba306 100644 --- a/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureEntity.java @@ -39,11 +39,7 @@ public class MapUserLoginFailureEntity implements AbstractEntity, UpdatableEntit private long lastFailure; private String lastIPFailure; - public MapUserLoginFailureEntity() { - this.id = null; - this.realmId = null; - this.userId = null; - } + public MapUserLoginFailureEntity() {} public MapUserLoginFailureEntity(String id, String realmId, String userId) { this.id = id; @@ -56,6 +52,13 @@ public class MapUserLoginFailureEntity implements AbstractEntity, UpdatableEntit return this.id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + @Override public boolean isUpdated() { return this.updated; diff --git a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmEntity.java b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmEntity.java index 2f05b235c3..1d20189557 100644 --- a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmEntity.java @@ -130,9 +130,7 @@ public class MapRealmEntity implements AbstractEntity, UpdatableEntity { */ protected boolean updated; - protected MapRealmEntity() { - this.id = null; - } + protected MapRealmEntity() {} public MapRealmEntity(String id) { this.id = id; @@ -143,6 +141,13 @@ public class MapRealmEntity implements AbstractEntity, UpdatableEntity { return this.id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + @Override public boolean isUpdated() { return this.updated diff --git a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java index ead14b003f..fdb9dc5b1c 100644 --- a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java @@ -42,14 +42,9 @@ public class MapRoleEntity implements AbstractEntity, UpdatableEntity { */ protected boolean updated; - protected MapRoleEntity() { - this.id = null; - this.realmId = null; - } + protected MapRoleEntity() {} public MapRoleEntity(String id, String realmId) { - Objects.requireNonNull(realmId, "realmId"); - this.id = id; this.realmId = realmId; } @@ -59,6 +54,13 @@ public class MapRoleEntity implements AbstractEntity, UpdatableEntity { return this.id; } + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } + @Override public boolean isUpdated() { return this.updated; diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapKeycloakTransaction.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapKeycloakTransaction.java index e28cac4265..c0dbbaca2a 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapKeycloakTransaction.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapKeycloakTransaction.java @@ -214,7 +214,7 @@ public class ConcurrentHashMapKeycloakTransaction