diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectQueryUtil.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectQueryUtil.java index 402f0860522..0115ccb71e7 100644 --- a/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectQueryUtil.java +++ b/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectQueryUtil.java @@ -15,6 +15,7 @@ import java.util.Collection; import java.util.List; import java.util.Objects; +import java.util.function.Function; import java.util.stream.Collectors; import javax.xml.namespace.QName; @@ -716,6 +717,33 @@ public static boolean hasFilter(ObjectQuery query) { return query != null && query.getFilter() != null; // TODO and "filter is not empty"? } + public static OwnedByFilter extractOwnedByFilterForReferenceSearch( + ObjectFilter filter, Function exceptionFactory) + throws E { + if (filter instanceof OwnedByFilter ownedByFilter) { + return ownedByFilter; + } else if (filter instanceof AndFilter andFilter) { + OwnedByFilter ownedByFilter = null; + for (ObjectFilter condition : andFilter.getConditions()) { + if (condition instanceof OwnedByFilter current) { + if (ownedByFilter != null) { + throw exceptionFactory.apply("Exactly one main OWNED-BY filter must be used" + + " for reference search, but multiple found. Filter: " + filter); + } + ownedByFilter = current; + } + } + if (ownedByFilter == null) { + throw exceptionFactory.apply("Exactly one main OWNED-BY filter must be used" + + " for reference search, but none found. Filter: " + filter); + } + return ownedByFilter; + } else { + throw exceptionFactory.apply("Invalid filter for reference search: " + filter + + "\nReference search filter should be OWNED-BY filter or an AND filter containing it."); + } + } + public static @NotNull ObjectQuery replaceFilter(ObjectQuery original, ObjectFilter newFilter) { ObjectQuery updatedQuery = original != null ? original.clone() : PrismContext.get().queryFactory().createQuery(); updatedQuery.setFilter(newFilter); diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/ModelController.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/ModelController.java index 01edb938544..c992882ebaa 100644 --- a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/ModelController.java +++ b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/ModelController.java @@ -61,6 +61,7 @@ import com.evolveum.midpoint.prism.delta.DiffUtil; import com.evolveum.midpoint.prism.delta.ItemDelta; import com.evolveum.midpoint.prism.delta.ObjectDelta; +import com.evolveum.midpoint.prism.impl.query.OwnedByFilterImpl; import com.evolveum.midpoint.prism.path.ItemPath; import com.evolveum.midpoint.prism.polystring.PolyString; import com.evolveum.midpoint.prism.query.*; @@ -84,6 +85,7 @@ import com.evolveum.midpoint.schema.util.cases.ApprovalUtils; import com.evolveum.midpoint.security.api.SecurityContextManager; import com.evolveum.midpoint.security.enforcer.api.AuthorizationParameters; +import com.evolveum.midpoint.security.enforcer.api.CompileConstraintsOptions; import com.evolveum.midpoint.security.enforcer.api.SecurityEnforcer; import com.evolveum.midpoint.task.api.Task; import com.evolveum.midpoint.task.api.TaskManager; @@ -1333,7 +1335,7 @@ public SearchResultList searchReferences( return SearchResultList.empty(); } - query = preProcessReferenceQuerySecurity(query, task, result); // TODO not implemented yet! + query = preProcessReferenceQuerySecurity(query, task, result); if (checkNoneFilterAfterAutz(query, result)) { return SearchResultList.empty(); @@ -1381,7 +1383,7 @@ public Integer countReferences(ObjectQuery query, return 0; } - query = preProcessReferenceQuerySecurity(query, task, result); // TODO not implemented yet! + query = preProcessReferenceQuerySecurity(query, task, result); if (checkNoneFilterAfterAutz(query, result)) { return 0; @@ -1404,12 +1406,112 @@ public Integer countReferences(ObjectQuery query, } } - private ObjectQuery preProcessReferenceQuerySecurity(ObjectQuery query, Task task, OperationResult options) { - // TODO: - // 1. extract owner object type from OWNED-BY query (if it's a container type, follow up to the object type) - // 2. use it for securityEnforcer.preProcessObjectFilter() - // 3. add filters to the owned-by filter as necessary - return query; + /** + * Applies model-level authorization preprocessing to reference-search queries. + * + * Reference searches are expected to use an {@link OwnedByFilter}. For object-owned + * references, such as {@code UserType/roleMembershipRef} or {@code FocusType/linkRef}, + * this method applies normal owner-object search authorization and verifies that the + * current principal can read the owned reference item path. If the owner search or the + * reference item read is denied, the query is converted to {@link NoneFilter}. + * + * Container-owned reference searches are not handled here yet. They require resolving + * the containing object type and translating the container filter into an object-level + * filter. For compatibility, unsupported non-object owners keep the previous behavior + * and are returned unchanged. + */ + private ObjectQuery preProcessReferenceQuerySecurity(ObjectQuery query, Task task, OperationResult result) + throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException, + CommunicationException, ConfigurationException, SecurityViolationException { + ObjectQuery processedQuery = query.clone(); + ObjectFilter filter = processedQuery.getFilter(); + if (filter instanceof NoneFilter) { + return processedQuery; + } + + OwnedByFilter ownedByFilter = + ObjectQueryUtil.extractOwnedByFilterForReferenceSearch(filter, SchemaException::new); + Class ownerClass = getObjectOwnerClassOrNull(ownedByFilter); + if (ownerClass == null) { + // TODO: Support authorization preprocessing for container-owned reference searches. + return processedQuery; + } + ItemPath refPath = getReferencePath(ownedByFilter); + + ObjectFilter securedOwnerFilter = securityEnforcer.preProcessObjectFilter( + securityEnforcer.getMidPointPrincipal(), + ModelAuthorizationAction.AUTZ_ACTIONS_URLS_SEARCH, + ModelAuthorizationAction.AUTZ_ACTIONS_URLS_SEARCH_BY, + null, ownerClass, ownedByFilter.getFilter(), null, List.of(), + SecurityEnforcer.Options.create(), task, result); + if (securedOwnerFilter instanceof NoneFilter || !isReferenceItemReadable(ownerClass, refPath, task, result)) { + processedQuery.setFilter(prismContext.queryFactory().createNone()); + return processedQuery; + } + + OwnedByFilter securedOwnedByFilter = OwnedByFilterImpl.create(ownedByFilter.getType(), refPath, securedOwnerFilter); + processedQuery.setFilter(replaceOwnedByFilter(filter, securedOwnedByFilter)); + return processedQuery; + } + + private Class getObjectOwnerClassOrNull(OwnedByFilter ownedByFilter) { + Class ownerClassRaw = ownedByFilter.getType().getCompileTimeClass(); + if (ownerClassRaw == null || !ObjectType.class.isAssignableFrom(ownerClassRaw)) { + return null; + } + @SuppressWarnings("unchecked") + Class ownerClass = (Class) ownerClassRaw; + return ownerClass; + } + + private ItemPath getReferencePath(OwnedByFilter ownedByFilter) throws SchemaException { + ItemPath refPath = ownedByFilter.getPath(); + if (refPath == null || refPath.isEmpty()) { + throw new SchemaException("Reference search OWNED-BY filter must specify owned reference path"); + } + return refPath; + } + + private boolean isReferenceItemReadable( + Class ownerClass, ItemPath refPath, Task task, OperationResult result) + throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException, + CommunicationException, ConfigurationException, SecurityViolationException { + PrismObject emptyOwner = prismContext.createObject(ownerClass); + var constraints = securityEnforcer.compileOperationConstraints( + securityEnforcer.getMidPointPrincipal(), + emptyOwner.getValue(), + null, + ModelAuthorizationAction.AUTZ_ACTIONS_URLS_GET, + SecurityEnforcer.Options.create(), + CompileConstraintsOptions.skipSubObjectSelectors() + .withFullInformationAvailable(false), + task, result); + + AccessDecision rootDecision = constraints.getDecision(); + AccessDecision itemDecision = constraints.getItemConstraints(refPath.firstToName()).getDecision(); + return switch (itemDecision) { + case DENY -> false; + case ALLOW -> true; + case DEFAULT -> rootDecision == AccessDecision.ALLOW; + }; + } + + private ObjectFilter replaceOwnedByFilter(ObjectFilter filter, OwnedByFilter securedOwnedByFilter) + throws SchemaException { + if (filter instanceof OwnedByFilter) { + return securedOwnedByFilter; + } + + if (filter instanceof AndFilter andFilter) { + List conditions = new ArrayList<>(andFilter.getConditions().size()); + for (ObjectFilter condition : andFilter.getConditions()) { + conditions.add(condition instanceof OwnedByFilter ? securedOwnedByFilter : condition); + } + return prismContext.queryFactory().createAnd(conditions); + } + + throw new SchemaException("Invalid filter for reference search: " + filter + + "\nReference search filter should be OWNED-BY filter or an AND filter containing it."); } @Override @@ -1434,7 +1536,7 @@ public SearchResultMetadata searchReferencesIterative( if (checkNoneFilterBeforeAutz(query)) { return null; } - ObjectQuery processedQuery = preProcessReferenceQuerySecurity(query, task, result); // TODO not implemented yet! + ObjectQuery processedQuery = preProcessReferenceQuerySecurity(query, task, result); if (checkNoneFilterAfterAutz(processedQuery, result)) { return null; } diff --git a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/security/TestSecurityBasic.java b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/security/TestSecurityBasic.java index d5ebd7ed784..54288cb4d93 100644 --- a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/security/TestSecurityBasic.java +++ b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/security/TestSecurityBasic.java @@ -16,6 +16,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import javax.xml.namespace.QName; import com.evolveum.midpoint.model.api.ModelInteractionService; @@ -3535,6 +3536,104 @@ public void test320AutzDenyReadAssignmentAndRoleMembershipRef() throws Exception assertSearch(RoleType.class, alexRolesQuery, 0); } + /** Reference search must respect denied access to `roleMembershipRef`. */ + @Test + public void test321AutzDenyReadRoleMembershipRefReferenceSearch() throws Exception { + given(); + cleanupAutzTest(USER_JACK_OID); + assignRole(USER_JACK_OID, ROLE_READONLY.oid); + login(USER_JACK_USERNAME); + + and("roleMembershipRef references are visible before the deny role is assigned"); + assertThat(searchJackRoleMembershipRefs()).isNotEmpty(); + + loginAdministrator(); + assignRole(USER_JACK_OID, ROLE_DENY_READ_ASSIGNMENT_AND_ROLE_MEMBERSHIP_REF.oid); + login(USER_JACK_USERNAME); + + when("searching roleMembershipRef references owned by jack"); + var refs = searchJackRoleMembershipRefs(); + + then("no references are returned"); + assertThat(refs).isEmpty(); + } + + /** Reference count must respect denied access to `roleMembershipRef`. */ + @Test + public void test322AutzDenyReadRoleMembershipRefReferenceCount() throws Exception { + given(); + cleanupAutzTest(USER_JACK_OID); + assignRole(USER_JACK_OID, ROLE_READONLY.oid); + login(USER_JACK_USERNAME); + + and("roleMembershipRef references are counted before the deny role is assigned"); + assertThat(countJackRoleMembershipRefs()).isPositive(); + + loginAdministrator(); + assignRole(USER_JACK_OID, ROLE_DENY_READ_ASSIGNMENT_AND_ROLE_MEMBERSHIP_REF.oid); + login(USER_JACK_USERNAME); + + when("counting roleMembershipRef references owned by jack"); + Integer count = countJackRoleMembershipRefs(); + + then("zero references are counted"); + assertThat(count).isZero(); + } + + /** Iterative reference search must respect denied access to `roleMembershipRef`. */ + @Test + public void test323AutzDenyReadRoleMembershipRefReferenceIterative() throws Exception { + given(); + cleanupAutzTest(USER_JACK_OID); + assignRole(USER_JACK_OID, ROLE_READONLY.oid); + login(USER_JACK_USERNAME); + + and("roleMembershipRef references are iterated before the deny role is assigned"); + assertThat(iterateJackRoleMembershipRefs()).isPositive(); + + loginAdministrator(); + assignRole(USER_JACK_OID, ROLE_DENY_READ_ASSIGNMENT_AND_ROLE_MEMBERSHIP_REF.oid); + login(USER_JACK_USERNAME); + + when("iteratively searching roleMembershipRef references owned by jack"); + int handled = iterateJackRoleMembershipRefs(); + + then("the handler is not invoked"); + assertThat(handled).isZero(); + } + + private ObjectQuery createJackRoleMembershipRefQuery() { + return prismContext.queryForReferenceOwnedBy(UserType.class, UserType.F_ROLE_MEMBERSHIP_REF) + .id(USER_JACK_OID) + .build(); + } + + private SearchResultList searchJackRoleMembershipRefs() throws Exception { + Task task = getTestTask(); + OperationResult result = task.getResult(); + return modelService.searchReferences(createJackRoleMembershipRefQuery(), null, task, result); + } + + private Integer countJackRoleMembershipRefs() throws Exception { + Task task = getTestTask(); + OperationResult result = task.getResult(); + return modelService.countReferences(createJackRoleMembershipRefQuery(), null, task, result); + } + + private int iterateJackRoleMembershipRefs() throws Exception { + Task task = getTestTask(); + OperationResult result = task.getResult(); + AtomicInteger handled = new AtomicInteger(); + modelService.searchReferencesIterative( + createJackRoleMembershipRefQuery(), + (ref, lResult) -> { + handled.incrementAndGet(); + return true; + }, + null, task, result); + return handled.get(); + } + private ObjectQuery createRolesOfTeammateQuery(String userOid) { return queryFor(RoleType.class) .referencedBy(UserType.class, UserType.F_ASSIGNMENT.append(AssignmentType.F_TARGET_REF)) diff --git a/release-notes.adoc b/release-notes.adoc index c606d71c93d..82b646296eb 100644 --- a/release-notes.adoc +++ b/release-notes.adoc @@ -102,6 +102,7 @@ Overall, midPoint 4.10 opens up the world of identity management and governance * Delineation suggestions: filter parsing broken after recent GUI change. See bug:MID-11175[] * Fixed work item search by name causing repository mapping error. See bug:MID-8834[] * Fixed translation of archetype display labels in assignment picker and summary panel. See bug:MID-11177[] +* Fixed leaking role membership references in the All accesses panel. See bug:MID-9639[] === Releases Of Other Components diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java index 41cd5fad781..61f23d42c80 100644 --- a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java +++ b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java @@ -1513,7 +1513,7 @@ SearchResultList executeSearchReferences( @NotNull private QReferenceMapping determineMapping(ObjectFilter filter) throws QueryException { - OwnedByFilter ownedByFilter = extractOwnedByFilterForReferenceSearch(filter); + OwnedByFilter ownedByFilter = ObjectQueryUtil.extractOwnedByFilterForReferenceSearch(filter, QueryException::new); ComplexTypeDefinition type = ownedByFilter.getType(); ItemPath path = ownedByFilter.getPath(); @@ -1527,32 +1527,6 @@ SearchResultList executeSearchReferences( return refMapping; } - private OwnedByFilter extractOwnedByFilterForReferenceSearch(ObjectFilter filter) - throws QueryException { - if (filter instanceof OwnedByFilter) { - return (OwnedByFilter) filter; - } else if (filter instanceof AndFilter) { - OwnedByFilter ownedByFilter = null; - for (ObjectFilter condition : ((AndFilter) filter).getConditions()) { - if (condition instanceof OwnedByFilter) { - if (ownedByFilter != null) { - throw new QueryException("Exactly one main OWNED-BY filter must be used" - + " for reference search, but multiple found. Filter: " + filter); - } - ownedByFilter = (OwnedByFilter) condition; - } - } - if (ownedByFilter == null) { - throw new QueryException("Exactly one main OWNED-BY filter must be used" - + " for reference search, but none found. Filter: " + filter); - } - return ownedByFilter; - } else { - throw new QueryException("Invalid filter for reference search: " + filter - + "\nReference search filter should be OWNED-BY filter or an AND filter containing it."); - } - } - @Override public SearchResultMetadata searchReferencesIterative( @Nullable ObjectQuery query,