Update composite roles on child role removal

Closes #11769
This commit is contained in:
Michal Hajas 2022-04-29 13:20:26 +02:00 committed by Hynek Mlnařík
parent 491b3262de
commit fc974fc019
9 changed files with 92 additions and 2 deletions

View file

@ -143,8 +143,7 @@ jobs:
fetch-depth: 2 fetch-depth: 2
- name: Check whether HEAD^ contains HotRod storage relevant changes - name: Check whether HEAD^ contains HotRod storage relevant changes
run: echo "GIT_HOTROD_RELEVANT_DIFF=$( git diff --name-only HEAD^ | egrep -ic -e 'non-existent-folder' )" >> $GITHUB_ENV run: echo "GIT_HOTROD_RELEVANT_DIFF=$( git diff --name-only HEAD^ | egrep -ic -e '^model/hot-rod|^model/map|^model/build-processor|^testsuite/model' )" >> $GITHUB_ENV
# run: echo "GIT_HOTROD_RELEVANT_DIFF=$( git diff --name-only HEAD^ | egrep -ic -e '^model/hot-rod|^model/map|^model/build-processor|^testsuite/model' )" >> $GITHUB_ENV
- name: Cache Maven packages - name: Cache Maven packages
if: ${{ github.event_name != 'pull_request' || matrix.server != 'undertow-map-hot-rod' || env.GIT_HOTROD_RELEVANT_DIFF != 0 }} if: ${{ github.event_name != 'pull_request' || matrix.server != 'undertow-map-hot-rod' || env.GIT_HOTROD_RELEVANT_DIFF != 0 }}

View file

@ -93,6 +93,7 @@ public class HotRodRoleEntity extends AbstractHotRodEntity {
@ProtoField(number = 8) @ProtoField(number = 8)
public String clientId; public String clientId;
@ProtoDoc("@Field(index = Index.YES, store = Store.YES)")
@ProtoField(number = 9) @ProtoField(number = 9)
public Set<String> compositeRoles; public Set<String> compositeRoles;

View file

@ -35,6 +35,7 @@ import org.keycloak.models.RoleModel.SearchableFields;
import org.keycloak.models.map.common.StringKeyConverter.UUIDKey; import org.keycloak.models.map.common.StringKeyConverter.UUIDKey;
import org.keycloak.models.map.storage.CriterionNotSupportedException; import org.keycloak.models.map.storage.CriterionNotSupportedException;
import org.keycloak.models.map.storage.jpa.JpaModelCriteriaBuilder; import org.keycloak.models.map.storage.jpa.JpaModelCriteriaBuilder;
import org.keycloak.models.map.storage.jpa.hibernate.jsonb.JsonbType;
import org.keycloak.models.map.storage.jpa.role.entity.JpaRoleEntity; import org.keycloak.models.map.storage.jpa.role.entity.JpaRoleEntity;
import org.keycloak.storage.SearchableModelField; import org.keycloak.storage.SearchableModelField;
@ -61,6 +62,15 @@ public class JpaRoleModelCriteriaBuilder extends JpaModelCriteriaBuilder<JpaRole
return new JpaRoleModelCriteriaBuilder((cb, root) -> return new JpaRoleModelCriteriaBuilder((cb, root) ->
cb.equal(root.get(modelField.getName()), value[0]) cb.equal(root.get(modelField.getName()), value[0])
); );
} else if (modelField == SearchableFields.COMPOSITE_ROLE){
validateValue(value, modelField, op, String.class);
return new JpaRoleModelCriteriaBuilder((cb, root) ->
cb.isTrue(cb.function("@>",
Boolean.TYPE,
cb.function("->", JsonbType.class, root.get("metadata"), cb.literal("fCompositeRoles")),
cb.literal(convertToJson(value[0]))))
);
} else { } else {
throw new CriterionNotSupportedException(modelField, op); throw new CriterionNotSupportedException(modelField, op);
} }

View file

@ -140,6 +140,9 @@ public class LdapRoleModelCriteriaBuilder extends LdapModelCriteriaBuilder<LdapR
String field = modelFieldNameToLdap(roleMapperConfig, modelField); String field = modelFieldNameToLdap(roleMapperConfig, modelField);
return new LdapRoleModelCriteriaBuilder(roleMapperConfig, return new LdapRoleModelCriteriaBuilder(roleMapperConfig,
() -> equal(field, value[0], LdapMapEscapeStrategy.DEFAULT, false)); () -> equal(field, value[0], LdapMapEscapeStrategy.DEFAULT, false));
} else if (modelField == RoleModel.SearchableFields.COMPOSITE_ROLE) {
// Not supported at the moment
return new LdapRoleModelCriteriaBuilder(roleMapperConfig, StringBuilder::new);
} else { } else {
throw new CriterionNotSupportedException(modelField, op); throw new CriterionNotSupportedException(modelField, op);
} }

View file

@ -279,6 +279,14 @@ public class MapRoleProvider implements RoleProvider {
tx.delete(withCriteria(mcb)); tx.delete(withCriteria(mcb));
} }
public void preRemove(RealmModel realm, RoleModel role) {
// Remove reference from all composite roles
DefaultModelCriteria<RoleModel> mcb = criteria();
mcb = mcb.compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId())
.compare(SearchableFields.COMPOSITE_ROLE, Operator.EQ, role.getId());
tx.read(withCriteria(mcb)).forEach(mapRoleEntity -> mapRoleEntity.removeCompositeRole(role.getId()));
}
@Override @Override
public void close() { public void close() {
} }

View file

@ -28,6 +28,7 @@ import org.keycloak.provider.InvalidationHandler;
import static org.keycloak.models.map.common.AbstractMapProviderFactory.MapProviderObjectType.CLIENT_BEFORE_REMOVE; import static org.keycloak.models.map.common.AbstractMapProviderFactory.MapProviderObjectType.CLIENT_BEFORE_REMOVE;
import static org.keycloak.models.map.common.AbstractMapProviderFactory.MapProviderObjectType.REALM_BEFORE_REMOVE; import static org.keycloak.models.map.common.AbstractMapProviderFactory.MapProviderObjectType.REALM_BEFORE_REMOVE;
import static org.keycloak.models.map.common.AbstractMapProviderFactory.MapProviderObjectType.ROLE_AFTER_REMOVE; import static org.keycloak.models.map.common.AbstractMapProviderFactory.MapProviderObjectType.ROLE_AFTER_REMOVE;
import static org.keycloak.models.map.common.AbstractMapProviderFactory.MapProviderObjectType.ROLE_BEFORE_REMOVE;
public class MapRoleProviderFactory extends AbstractMapProviderFactory<MapRoleProvider, MapRoleEntity, RoleModel> implements RoleProviderFactory<MapRoleProvider>, InvalidationHandler { public class MapRoleProviderFactory extends AbstractMapProviderFactory<MapRoleProvider, MapRoleEntity, RoleModel> implements RoleProviderFactory<MapRoleProvider>, InvalidationHandler {
@ -51,6 +52,8 @@ public class MapRoleProviderFactory extends AbstractMapProviderFactory<MapRolePr
create(session).preRemove((RealmModel) params[0]); create(session).preRemove((RealmModel) params[0]);
} else if (type == CLIENT_BEFORE_REMOVE) { } else if (type == CLIENT_BEFORE_REMOVE) {
create(session).removeRoles((ClientModel) params[1]); create(session).removeRoles((ClientModel) params[1]);
} else if (type == ROLE_BEFORE_REMOVE) {
create(session).preRemove((RealmModel) params[0], (RoleModel) params[1]);
} else if (type == ROLE_AFTER_REMOVE) { } else if (type == ROLE_AFTER_REMOVE) {
session.getKeycloakSessionFactory().publish(new RoleContainerModel.RoleRemovedEvent() { session.getKeycloakSessionFactory().publish(new RoleContainerModel.RoleRemovedEvent() {
@Override public RoleModel getRole() { return (RoleModel) params[1]; } @Override public RoleModel getRole() { return (RoleModel) params[1]; }

View file

@ -122,6 +122,7 @@ public class MapFieldPredicates {
put(ROLE_PREDICATES, RoleModel.SearchableFields.DESCRIPTION, MapRoleEntity::getDescription); put(ROLE_PREDICATES, RoleModel.SearchableFields.DESCRIPTION, MapRoleEntity::getDescription);
put(ROLE_PREDICATES, RoleModel.SearchableFields.NAME, MapRoleEntity::getName); 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, MapRoleEntity::isClientRole);
put(ROLE_PREDICATES, RoleModel.SearchableFields.COMPOSITE_ROLE, MapFieldPredicates::checkCompositeRoles);
put(USER_PREDICATES, UserModel.SearchableFields.REALM_ID, MapUserEntity::getRealmId); put(USER_PREDICATES, UserModel.SearchableFields.REALM_ID, MapUserEntity::getRealmId);
put(USER_PREDICATES, UserModel.SearchableFields.USERNAME, MapUserEntity::getUsername); put(USER_PREDICATES, UserModel.SearchableFields.USERNAME, MapUserEntity::getUsername);
@ -332,6 +333,14 @@ public class MapFieldPredicates {
return mcb.fieldCompare(Boolean.TRUE::equals, getter); return mcb.fieldCompare(Boolean.TRUE::equals, 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;
getter = re -> Optional.ofNullable(re.getCompositeRoles()).orElseGet(Collections::emptySet).contains(roleIdS);
return mcb.fieldCompare(Boolean.TRUE::equals, getter);
}
private static MapModelCriteriaBuilder<Object, MapUserEntity, UserModel> checkGrantedUserRole(MapModelCriteriaBuilder<Object, MapUserEntity, UserModel> mcb, Operator op, Object[] values) { private static MapModelCriteriaBuilder<Object, MapUserEntity, UserModel> checkGrantedUserRole(MapModelCriteriaBuilder<Object, MapUserEntity, UserModel> mcb, Operator op, Object[] values) {
String roleIdS = ensureEqSingleValue(UserModel.SearchableFields.ASSIGNED_ROLE, "role_id", op, values); String roleIdS = ensureEqSingleValue(UserModel.SearchableFields.ASSIGNED_ROLE, "role_id", op, values);
Function<MapUserEntity, ?> getter; Function<MapUserEntity, ?> getter;

View file

@ -38,6 +38,7 @@ public interface RoleModel {
public static final SearchableModelField<RoleModel> NAME = new SearchableModelField<>("name", String.class); public static final SearchableModelField<RoleModel> NAME = new SearchableModelField<>("name", String.class);
public static final SearchableModelField<RoleModel> DESCRIPTION = new SearchableModelField<>("description", String.class); public static final SearchableModelField<RoleModel> DESCRIPTION = new SearchableModelField<>("description", String.class);
public static final SearchableModelField<RoleModel> IS_CLIENT_ROLE = new SearchableModelField<>("isClientRole", Boolean.class); public static final SearchableModelField<RoleModel> IS_CLIENT_ROLE = new SearchableModelField<>("isClientRole", Boolean.class);
public static final SearchableModelField<RoleModel> COMPOSITE_ROLE = new SearchableModelField<>("compositeRoles", Boolean.class);
} }
String getName(); String getName();

View file

@ -16,6 +16,7 @@ import org.keycloak.testsuite.model.RequireProvider;
import java.util.Collection; import java.util.Collection;
import java.util.List; import java.util.List;
import java.util.UUID; import java.util.UUID;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import java.util.stream.IntStream; import java.util.stream.IntStream;
import java.util.stream.Stream; import java.util.stream.Stream;
@ -227,6 +228,61 @@ public class RoleModelTest extends KeycloakModelTest {
}); });
} }
@Test
public void testCompositeRolesUpdateOnChildRoleRemoval() {
final AtomicReference<String> parentRealmRoleId = new AtomicReference<>();
final AtomicReference<String> parentClientRoleId = new AtomicReference<>();
final AtomicReference<String> childRealmRoleId = new AtomicReference<>();
final AtomicReference<String> childClientRoleId = new AtomicReference<>();
withRealm(realmId, (session, realm) -> {
// Create realm role
RoleModel parentRealmRole = session.roles().addRealmRole(realm, "parentRealmRole");
parentRealmRoleId.set(parentRealmRole.getId());
// Create client role
ClientModel client = session.clients().addClient(realm,"clientWithRole");
RoleModel parentClientRole = session.roles().addClientRole(client, "parentClientRole");
parentClientRoleId.set(parentClientRole.getId());
// Create realm child role
RoleModel childRealmRole = session.roles().addRealmRole(realm, "childRealmRole");
childRealmRoleId.set(childRealmRole.getId());
RoleModel childClientRole = session.roles().addClientRole(client, "childClientRole");
childClientRoleId.set(childClientRole.getId());
// Add composites
parentRealmRole.addCompositeRole(childRealmRole);
parentRealmRole.addCompositeRole(childClientRole);
parentClientRole.addCompositeRole(childRealmRole);
parentClientRole.addCompositeRole(childClientRole);
return null;
});
withRealm(realmId, (session, realm) -> {
RoleModel parentRealmRole = session.roles().getRoleById(realm, parentRealmRoleId.get());
RoleModel parentClientRole = session.roles().getRoleById(realm, parentClientRoleId.get());
assertThat(parentRealmRole.getCompositesStream().collect(Collectors.toSet()), hasSize(2));
assertThat(parentClientRole.getCompositesStream().collect(Collectors.toSet()), hasSize(2));
session.roles().removeRole(session.roles().getRoleById(realm, childRealmRoleId.get()));
session.roles().removeRole(session.roles().getRoleById(realm, childClientRoleId.get()));
return null;
});
withRealm(realmId, (session, realm) -> {
RoleModel parentRealmRole = session.roles().getRoleById(realm, parentRealmRoleId.get());
RoleModel parentClientRole = session.roles().getRoleById(realm, parentClientRoleId.get());
assertThat(parentRealmRole.getCompositesStream().collect(Collectors.toSet()), empty());
assertThat(parentClientRole.getCompositesStream().collect(Collectors.toSet()), empty());
return null;
});
}
public void testRolesWithIdsPaginationSearchQueries(GetResult resultProvider) { public void testRolesWithIdsPaginationSearchQueries(GetResult resultProvider) {
// test all parameters together // test all parameters together
List<RoleModel> result = resultProvider.getResult("1", 4, 3); List<RoleModel> result = resultProvider.getResult("1", 4, 3);