diff --git a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryMapModelCriteriaBuilder.java b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryMapModelCriteriaBuilder.java index 577ae2be6e..be4ea420b3 100644 --- a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryMapModelCriteriaBuilder.java +++ b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryMapModelCriteriaBuilder.java @@ -64,8 +64,6 @@ public class IckleQueryMapModelCriteriaBuilder parameters) { + if (op != ModelCriteriaBuilder.Operator.EQ && op != ModelCriteriaBuilder.Operator.NE) { + throw new CriterionNotSupportedException(RoleModel.SearchableFields.IS_CLIENT_ROLE, op); + } + if (values == null || values.length != 1) { + throw new CriterionNotSupportedException(RoleModel.SearchableFields.IS_CLIENT_ROLE, op, "Invalid arguments, expected (boolean), got: " + Arrays.toString(values)); + } + + Boolean b = (Boolean) values[0]; + if (op == Operator.EQ) { + if (b == null || b == Boolean.FALSE) { + return IckleQueryWhereClauses.produceWhereClause(RoleModel.SearchableFields.CLIENT_ID, Operator.NOT_EXISTS, new Object[] {}, parameters); + } else { + return IckleQueryWhereClauses.produceWhereClause(RoleModel.SearchableFields.CLIENT_ID, Operator.EXISTS, new Object[] {}, parameters); + } + } else /* if (op == Operator.NE) */ { + if (b == null || b == Boolean.TRUE) { + return IckleQueryWhereClauses.produceWhereClause(RoleModel.SearchableFields.CLIENT_ID, Operator.NOT_EXISTS, new Object[] {}, parameters); + } else { + return IckleQueryWhereClauses.produceWhereClause(RoleModel.SearchableFields.CLIENT_ID, Operator.EXISTS, new Object[] {}, parameters); + } + } + } + private static String whereClauseForCorrespondingSessionId(String modelFieldName, ModelCriteriaBuilder.Operator op, Object[] values, Map parameters) { if (op != ModelCriteriaBuilder.Operator.EQ) { throw new CriterionNotSupportedException(UserSessionModel.SearchableFields.CORRESPONDING_SESSION_ID, op); diff --git a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/role/HotRodRoleEntity.java b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/role/HotRodRoleEntity.java index 652f41af51..d29cc0ef4c 100644 --- a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/role/HotRodRoleEntity.java +++ b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/role/HotRodRoleEntity.java @@ -75,6 +75,16 @@ public class HotRodRoleEntity extends AbstractHotRodEntity { entity.updated |= id != null; } + @Override + public void setClientId(String clientId) { + HotRodRoleEntity entity = getHotRodEntity(); + entity.updated |= ! Objects.equals(entity.clientId, clientId); + entity.clientId = clientId; + + // Migration from previous version + entity.clientRole = clientId != null; + } + @Override public void setName(String name) { HotRodRoleEntity entity = getHotRodEntity(); diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/delegate/JpaRoleDelegateProvider.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/delegate/JpaRoleDelegateProvider.java index f8a88696e5..52edaff4d2 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/delegate/JpaRoleDelegateProvider.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/delegate/JpaRoleDelegateProvider.java @@ -47,7 +47,6 @@ public class JpaRoleDelegateProvider extends JpaDelegateProvider case ID: case REALM_ID: case CLIENT_ID: - case CLIENT_ROLE: case NAME: case DESCRIPTION: return getDelegate(); diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java index bd5e3f39d8..6f5f1d8079 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java @@ -177,11 +177,6 @@ public class JpaRoleEntity extends AbstractRoleEntity implements JpaRootVersione return description; } - @Override - public void setClientRole(Boolean clientRole) { - // intentionally do nothing, assuming the role is client-role when this.getClientId() != null; - } - @Override public void setRealmId(String realmId) { metadata.setRealmId(realmId); diff --git a/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/LdapRoleMapKeycloakTransaction.java b/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/LdapRoleMapKeycloakTransaction.java index 68a0752b35..02cf6d6f42 100644 --- a/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/LdapRoleMapKeycloakTransaction.java +++ b/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/LdapRoleMapKeycloakTransaction.java @@ -53,6 +53,7 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.keycloak.models.map.storage.ldap.role.config.LdapMapRoleMapperConfig.COMMON_ROLES_DN; public class LdapRoleMapKeycloakTransaction extends LdapMapKeycloakTransaction implements Provider { @@ -153,12 +154,12 @@ public class LdapRoleMapKeycloakTransaction extends LdapMapKeycloakTransaction roleObjectClasses = ldapMapConfig.getRoleObjectClasses(); diff --git a/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/config/LdapMapRoleMapperConfig.java b/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/config/LdapMapRoleMapperConfig.java index bb07aa35ed..5a70940e7b 100644 --- a/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/config/LdapMapRoleMapperConfig.java +++ b/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/config/LdapMapRoleMapperConfig.java @@ -76,23 +76,20 @@ public class LdapMapRoleMapperConfig extends LdapMapCommonGroupMapperConfig { return rolesDn; } - public String getRolesDn(Boolean isClientRole, String clientId) { + public String getRolesDn(String clientId) { String rolesDn; - if (isClientRole == null && clientId == null) { - rolesDn = mapperModel.getConfig().getFirst(COMMON_ROLES_DN); + boolean isClientRole = clientId != null; + if (! isClientRole) { + rolesDn = mapperModel.getConfig().getFirst(REALM_ROLES_DN); } else { - if (isClientRole != null && !isClientRole) { - rolesDn = config.get(REALM_ROLES_DN); - } else { - rolesDn = config.get(CLIENT_ROLES_DN); - if (rolesDn != null) { - LdapMapDn dn = LdapMapDn.fromString(rolesDn); - LdapMapDn.RDN firstRdn = dn.getFirstRdn(); - for (String key : firstRdn.getAllKeys()) { - firstRdn.setAttrValue(key, firstRdn.getAttrValue(key).replaceAll("\\{0}", Matcher.quoteReplacement(clientId))); - } - rolesDn = dn.toString(); + rolesDn = config.get(CLIENT_ROLES_DN); + if (rolesDn != null) { + LdapMapDn dn = LdapMapDn.fromString(rolesDn); + LdapMapDn.RDN firstRdn = dn.getFirstRdn(); + for (String key : firstRdn.getAllKeys()) { + firstRdn.setAttrValue(key, firstRdn.getAttrValue(key).replaceAll("\\{0}", Matcher.quoteReplacement(clientId))); } + rolesDn = dn.toString(); } } if (rolesDn == null) { diff --git a/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/entity/LdapRoleEntity.java b/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/entity/LdapRoleEntity.java index 86cd3a35cc..c35c2053ac 100644 --- a/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/entity/LdapRoleEntity.java +++ b/model/map-ldap/src/main/java/org/keycloak/models/map/storage/ldap/role/entity/LdapRoleEntity.java @@ -55,7 +55,6 @@ public class LdapRoleEntity extends UpdatableEntity.Impl implements EntityFieldD SETTERS.put(MapRoleEntityFields.ID, (e, v) -> e.setId((String) v)); SETTERS.put(MapRoleEntityFields.REALM_ID, (e, v) -> e.setRealmId((String) v)); SETTERS.put(MapRoleEntityFields.CLIENT_ID, (e, v) -> e.setClientId((String) v)); - SETTERS.put(MapRoleEntityFields.CLIENT_ROLE, (e, v) -> e.setClientRole((Boolean) v)); //noinspection unchecked SETTERS.put(MapRoleEntityFields.ATTRIBUTES, (e, v) -> e.setAttributes((Map>) v)); //noinspection unchecked @@ -69,7 +68,6 @@ public class LdapRoleEntity extends UpdatableEntity.Impl implements EntityFieldD GETTERS.put(MapRoleEntityFields.ID, LdapRoleEntity::getId); GETTERS.put(MapRoleEntityFields.REALM_ID, LdapRoleEntity::getRealmId); GETTERS.put(MapRoleEntityFields.CLIENT_ID, LdapRoleEntity::getClientId); - GETTERS.put(MapRoleEntityFields.CLIENT_ROLE, LdapRoleEntity::isClientRole); GETTERS.put(MapRoleEntityFields.ATTRIBUTES, LdapRoleEntity::getAttributes); GETTERS.put(MapRoleEntityFields.COMPOSITE_ROLES, LdapRoleEntity::getCompositeRoles); GETTERS.put(MapRoleEntityFields.NAME, LdapRoleEntity::getName); @@ -184,16 +182,6 @@ public class LdapRoleEntity extends UpdatableEntity.Impl implements EntityFieldD return ldapMapObject.getAttributeAsString("description"); } - public void setClientRole(Boolean clientRole) { - if (!Objects.equals(this.isClientRole(), clientRole)) { - throw new NotImplementedException(); - } - } - - public boolean isClientRole() { - return clientId != null; - } - public void setRealmId(String realmId) { // we'll not store this information, as LDAP store might be used from different realms } @@ -207,7 +195,7 @@ public class LdapRoleEntity extends UpdatableEntity.Impl implements EntityFieldD public void setName(String name) { this.updated |= !Objects.equals(getName(), name); ldapMapObject.setSingleAttribute(roleMapperConfig.getRoleNameLdapAttribute(), name); - LdapMapDn dn = LdapMapDn.fromString(roleMapperConfig.getRolesDn(clientId != null, clientId)); + LdapMapDn dn = LdapMapDn.fromString(roleMapperConfig.getRolesDn(clientId)); dn.addFirst(roleMapperConfig.getRoleNameLdapAttribute(), name); ldapMapObject.setDn(dn); } diff --git a/model/map-ldap/src/test/java/org/keycloak/models/map/storage/ldap/role/config/LdapMapRoleMapperConfigTest.java b/model/map-ldap/src/test/java/org/keycloak/models/map/storage/ldap/role/config/LdapMapRoleMapperConfigTest.java index 8ef228b0c1..edb7040955 100644 --- a/model/map-ldap/src/test/java/org/keycloak/models/map/storage/ldap/role/config/LdapMapRoleMapperConfigTest.java +++ b/model/map-ldap/src/test/java/org/keycloak/models/map/storage/ldap/role/config/LdapMapRoleMapperConfigTest.java @@ -30,9 +30,9 @@ public class LdapMapRoleMapperConfigTest { LdapMapRoleMapperConfig sut = new LdapMapRoleMapperConfig(config); Assert.assertEquals("ou=myclient,dc=keycloak,dc=org", - sut.getRolesDn(true, "myclient")); + sut.getRolesDn("myclient")); Assert.assertEquals("ou=\\ me\\=co\\\\ol\\, val\\=V\u00E9ronique,dc=keycloak,dc=org", - sut.getRolesDn(true, " me=co\\ol, val=V\u00E9ronique")); + sut.getRolesDn(" me=co\\ol, val=V\u00E9ronique")); } } \ No newline at end of file diff --git a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleAdapter.java b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleAdapter.java index 157680f3d2..2e28488ebd 100644 --- a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleAdapter.java @@ -99,8 +99,7 @@ public class MapRoleAdapter extends AbstractRoleModel implements @Override public boolean isClientRole() { - final Boolean clientRole = entity.isClientRole(); - return clientRole == null ? false : clientRole; + return entity.getClientId() != null; } @Override 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 5f03030a6d..9843da687d 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 @@ -44,15 +44,8 @@ public interface MapRoleEntity extends AbstractEntity, UpdatableEntity, EntityWi this.id = id; this.updated |= id != null; } - - @Override - public Boolean isClientRole() { - return getClientId() != null; - } } - Boolean isClientRole(); - String getRealmId(); String getClientId(); @@ -61,8 +54,6 @@ public interface MapRoleEntity extends AbstractEntity, UpdatableEntity, EntityWi String getDescription(); - void setClientRole(Boolean clientRole); - void setRealmId(String realmId); void setClientId(String clientId); diff --git a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java index fb71c71a8d..8fb4c0f116 100644 --- a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java @@ -80,7 +80,6 @@ public class MapRoleProvider implements RoleProvider { entity.setId(id); entity.setRealmId(realm.getId()); entity.setName(name); - entity.setClientRole(false); if (entity.getId() != null && txInRealm(realm).exists(entity.getId())) { throw new ModelDuplicateException("Role exists: " + id); } @@ -138,7 +137,6 @@ public class MapRoleProvider implements RoleProvider { entity.setId(id); entity.setRealmId(realm.getId()); entity.setName(name); - entity.setClientRole(true); entity.setClientId(client.getId()); if (entity.getId() != null && txInRealm(realm).exists(entity.getId())) { throw new ModelDuplicateException("Role exists: " + id); @@ -170,7 +168,7 @@ public class MapRoleProvider implements RoleProvider { } @Override public boolean removeRole(RoleModel role) { - LOG.tracef("removeRole(%s(%s))%s", role.getName(), role.getId(), getShortStackTrace()); + LOG.tracef("removeRole(%s)%s", role, getShortStackTrace()); RealmModel realm = role.isClientRole() ? ((ClientModel)role.getContainer()).getRealm() : (RealmModel)role.getContainer(); diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java index 405cf1d57f..2eba137e49 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java @@ -129,7 +129,7 @@ public class MapFieldPredicates { put(ROLE_PREDICATES, RoleModel.SearchableFields.CLIENT_ID, MapRoleEntity::getClientId); put(ROLE_PREDICATES, RoleModel.SearchableFields.DESCRIPTION, MapRoleEntity::getDescription); put(ROLE_PREDICATES, RoleModel.SearchableFields.NAME, MapRoleEntity::getName); - put(ROLE_PREDICATES, RoleModel.SearchableFields.IS_CLIENT_ROLE, MapRoleEntity::isClientRole); + put(ROLE_PREDICATES, RoleModel.SearchableFields.IS_CLIENT_ROLE, MapFieldPredicates::isClientRole); put(ROLE_PREDICATES, RoleModel.SearchableFields.COMPOSITE_ROLE, MapFieldPredicates::checkCompositeRoles); put(USER_PREDICATES, UserModel.SearchableFields.REALM_ID, MapUserEntity::getRealmId); @@ -390,6 +390,17 @@ public class MapFieldPredicates { } + private static MapModelCriteriaBuilder isClientRole(MapModelCriteriaBuilder mcb, Operator op, Object[] values) { + if (values == null || values.length != 1 || ! (op == Operator.EQ || op == Operator.NE) || ! (values[0] instanceof Boolean)) { + throw new CriterionNotSupportedException(RoleModel.SearchableFields.IS_CLIENT_ROLE, op, "Invalid arguments, got: " + Arrays.toString(values)); + } + Function getter; + Predicate valueComparator = CriteriaOperator.predicateFor(op, values); + getter = re -> re.getClientId() != null; + + return mcb.fieldCompare(valueComparator, getter); + } + private static MapModelCriteriaBuilder checkCompositeRoles(MapModelCriteriaBuilder mcb, Operator op, Object[] values) { String roleIdS = ensureEqSingleValue(RoleModel.SearchableFields.COMPOSITE_ROLE, "composite_role_id", op, values); Function getter;