Fix clientRole warning

Fixes: #16857
This commit is contained in:
Hynek Mlnarik 2023-02-13 15:17:35 +01:00 committed by Hynek Mlnařík
parent 7b604d6784
commit d768e75be7
13 changed files with 75 additions and 58 deletions

View file

@ -64,8 +64,6 @@ public class IckleQueryMapModelCriteriaBuilder<E extends AbstractHotRodEntity, M
INFINISPAN_NAME_OVERRIDES.put(GroupModel.SearchableFields.ASSIGNED_ROLE, "grantedRoles");
INFINISPAN_NAME_OVERRIDES.put(GroupModel.SearchableFields.ATTRIBUTE, "attributes");
INFINISPAN_NAME_OVERRIDES.put(RoleModel.SearchableFields.IS_CLIENT_ROLE, "clientRole");
INFINISPAN_NAME_OVERRIDES.put(UserModel.SearchableFields.USERNAME_CASE_INSENSITIVE, "usernameLowercase");
INFINISPAN_NAME_OVERRIDES.put(UserModel.SearchableFields.USERNAME, "username");
INFINISPAN_NAME_OVERRIDES.put(UserModel.SearchableFields.SERVICE_ACCOUNT_CLIENT, "serviceAccountClientLink");

View file

@ -22,10 +22,12 @@ import org.keycloak.events.Event;
import org.keycloak.events.admin.AdminEvent;
import org.keycloak.models.ClientModel;
import org.keycloak.models.GroupModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.UserSessionModel;
import org.keycloak.models.map.storage.CriterionNotSupportedException;
import org.keycloak.models.map.storage.ModelCriteriaBuilder;
import org.keycloak.models.map.storage.ModelCriteriaBuilder.Operator;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.storage.SearchableModelField;
import org.keycloak.storage.StorageId;
@ -69,6 +71,7 @@ public class IckleQueryWhereClauses {
WHERE_CLAUSE_PRODUCER_OVERRIDES.put(Policy.SearchableFields.CONFIG, IckleQueryWhereClauses::whereClauseForPolicyConfig);
WHERE_CLAUSE_PRODUCER_OVERRIDES.put(Event.SearchableFields.EVENT_TYPE, IckleQueryWhereClauses::whereClauseForEnumWithStableIndex);
WHERE_CLAUSE_PRODUCER_OVERRIDES.put(AdminEvent.SearchableFields.OPERATION_TYPE, IckleQueryWhereClauses::whereClauseForEnumWithStableIndex);
WHERE_CLAUSE_PRODUCER_OVERRIDES.put(RoleModel.SearchableFields.IS_CLIENT_ROLE, IckleQueryWhereClauses::whereClauseForClientRole);
}
@FunctionalInterface
@ -167,6 +170,30 @@ public class IckleQueryWhereClauses {
return IckleQueryOperators.combineExpressions(ModelCriteriaBuilder.Operator.LIKE, getFieldName(UserModel.SearchableFields.CONSENT_FOR_CLIENT), new String[] {providerId + "%"}, parameters);
}
private static String whereClauseForClientRole(String modelFieldName, ModelCriteriaBuilder.Operator op, Object[] values, Map<String, Object> 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<String, Object> parameters) {
if (op != ModelCriteriaBuilder.Operator.EQ) {
throw new CriterionNotSupportedException(UserSessionModel.SearchableFields.CORRESPONDING_SESSION_ID, op);

View file

@ -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();

View file

@ -47,7 +47,6 @@ public class JpaRoleDelegateProvider extends JpaDelegateProvider<JpaRoleEntity>
case ID:
case REALM_ID:
case CLIENT_ID:
case CLIENT_ROLE:
case NAME:
case DESCRIPTION:
return getDelegate();

View file

@ -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);

View file

@ -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<LdapMapRoleEntityFieldDelegate, MapRoleEntity, RoleModel> implements Provider {
@ -153,12 +154,12 @@ public class LdapRoleMapKeycloakTransaction extends LdapMapKeycloakTransaction<L
identityStore.add(mapped.getLdapMapObject());
// TODO: add a flag for temporary created roles until they are finally committed so that they don't show up in ready(query) in their temporary state
} catch (ModelException ex) {
if (value.isClientRole() && ex.getCause() instanceof NamingException) {
if (value.getClientId() != null && ex.getCause() instanceof NamingException) {
// the client hasn't been created, therefore adding it here
LdapMapObject client = new LdapMapObject();
client.setObjectClasses(Arrays.asList("top", "organizationalUnit"));
client.setRdnAttributeName("ou");
client.setDn(LdapMapDn.fromString(roleMapperConfig.getRolesDn(mapped.isClientRole(), mapped.getClientId())));
client.setDn(LdapMapDn.fromString(roleMapperConfig.getRolesDn(mapped.getClientId())));
client.setSingleAttribute("ou", mapped.getClientId());
identityStore.add(client);
@ -261,7 +262,7 @@ public class LdapRoleMapKeycloakTransaction extends LdapMapKeycloakTransaction<L
}
private LdapMapRoleEntityFieldDelegate lookupEntityById(String id, String clientId) {
LdapMapQuery ldapQuery = getLdapQuery(clientId != null, clientId);
LdapMapQuery ldapQuery = getLdapQuery(clientId);
LdapMapObject ldapObject = identityStore.fetchById(id, ldapQuery);
if (ldapObject != null) {
@ -278,7 +279,10 @@ public class LdapRoleMapKeycloakTransaction extends LdapMapKeycloakTransaction<L
Boolean isClientRole = mcb.isClientRole();
String clientId = mcb.getClientId();
LdapMapQuery ldapQuery = getLdapQuery(isClientRole, clientId);
LdapMapQuery ldapQuery = getLdapQuery(clientId);
if (isClientRole == null) {
ldapQuery.setSearchDn(roleMapperConfig.getCommonRolesDn());
}
mcb = mcb.withCustomFilter(roleMapperConfig.getCustomLdapFilter());
ldapQuery.setModelCriteriaBuilder(mcb);
@ -364,13 +368,13 @@ public class LdapRoleMapKeycloakTransaction extends LdapMapKeycloakTransaction<L
return null;
}
private LdapMapQuery getLdapQuery(Boolean isClientRole, String clientId) {
private LdapMapQuery getLdapQuery(String clientId) {
LdapMapQuery ldapMapQuery = new LdapMapQuery();
// For now, use same search scope, which is configured "globally" and used for user's search.
ldapMapQuery.setSearchScope(ldapMapConfig.getSearchScope());
String rolesDn = roleMapperConfig.getRolesDn(isClientRole, clientId);
String rolesDn = roleMapperConfig.getRolesDn(clientId);
ldapMapQuery.setSearchDn(rolesDn);
Collection<String> roleObjectClasses = ldapMapConfig.getRoleObjectClasses();

View file

@ -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) {

View file

@ -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<String, List<String>>) 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);
}

View file

@ -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"));
}
}

View file

@ -99,8 +99,7 @@ public class MapRoleAdapter extends AbstractRoleModel<MapRoleEntity> implements
@Override
public boolean isClientRole() {
final Boolean clientRole = entity.isClientRole();
return clientRole == null ? false : clientRole;
return entity.getClientId() != null;
}
@Override

View file

@ -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);

View file

@ -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();

View file

@ -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<Object, MapRoleEntity, RoleModel> isClientRole(MapModelCriteriaBuilder<Object, MapRoleEntity, RoleModel> 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<MapRoleEntity, Boolean> getter;
Predicate<Object> valueComparator = CriteriaOperator.predicateFor(op, values);
getter = re -> re.getClientId() != null;
return mcb.fieldCompare(valueComparator, getter);
}
private static MapModelCriteriaBuilder<Object, MapRoleEntity, RoleModel> checkCompositeRoles(MapModelCriteriaBuilder<Object, MapRoleEntity, RoleModel> mcb, Operator op, Object[] values) {
String roleIdS = ensureEqSingleValue(RoleModel.SearchableFields.COMPOSITE_ROLE, "composite_role_id", op, values);
Function<MapRoleEntity, ?> getter;