Skip to content

Commit

Permalink
Remove ModelInteractionService#canSearch method
Browse files Browse the repository at this point in the history 
This method is currently not used. In the past, it was used to check
whether one is allowed to search role members (to decide if some parts
of GUI should be shown). But we decided to drop this method. So here it
is. Consequently, some other parts of now-unused code could be removed
from SecurityEnforcer.

Other minor simplifications of the enforcer code were done as well.
  • Loading branch information
@mederly
mederly committed Jul 3, 2023
1 parent 65e786c commit 066abb2
Show file tree
Hide file tree
Showing 18 changed files with 63 additions and 284 deletions.
14 changes: 0 additions & 14 deletions infra/schema/src/main/java/com/evolveum/midpoint/schema/selector/eval/FilteringContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,13 @@ public class FilteringContext extends SelectorProcessingContext {
/** TODO explain and revise the use of this (not sure about its exact form) */
private final boolean maySkipOnSearch;

/**
* Externally-imposed constraint on the applicability of selected clauses.
* (Used to e.g. turn off evaluation of `self` clauses in some contexts.)
*/
@Nullable private final ClauseApplicabilityPredicate clauseApplicabilityPredicate;

@NotNull final FilterCollector filterCollector;

public FilteringContext(
@NotNull Class<?> filterType,
@NotNull Class<?> restrictedType,
@Nullable ObjectFilter originalFilter,
boolean maySkipOnSearch,
@Nullable ClauseApplicabilityPredicate clauseApplicabilityPredicate,
@NotNull FilterCollector filterCollector,
@Nullable ObjectFilterExpressionEvaluator filterEvaluator,
@NotNull ProcessingTracer<? super SelectorTraceEvent> tracer,
Expand All @@ -80,7 +73,6 @@ public FilteringContext(
this.restrictedType = restrictedType;
this.originalFilter = originalFilter;
this.maySkipOnSearch = maySkipOnSearch;
this.clauseApplicabilityPredicate = clauseApplicabilityPredicate;
this.filterCollector = filterCollector;
}

Expand Down Expand Up @@ -153,7 +145,6 @@ public boolean maySkipOnSearch() {
filterType,
originalFilter,
maySkipOnSearch,
clauseApplicabilityPredicate,
filterCollector,
filterEvaluator,
tracer,
Expand All @@ -164,9 +155,4 @@ public boolean maySkipOnSearch() {
description.child(idDelta, textDelta),
delegatorSelection);
}

public boolean isClauseApplicable(SelectorClause clause) {
return clauseApplicabilityPredicate == null
|| clauseApplicabilityPredicate.test(clause, this);
}
}
2 changes: 1 addition & 1 deletion infra/schema/src/main/java/com/evolveum/midpoint/schema/selector/spec/ValueSelector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ public boolean toFilter(@NotNull FilteringContext ctx)
ConfigurationException, ObjectNotFoundException {
ctx.traceFilterProcessingStart(this);
for (SelectorClause clause : clauses) {
if (!ctx.isClauseApplicable(clause) || !clause.toFilter(ctx)) {
if (!clause.toFilter(ctx)) {
ctx.traceFilterProcessingEnd(this, false);
return false;
}
Expand Down
28 changes: 12 additions & 16 deletions ...dpoint/authentication/impl/authorization/evaluator/MidPointGuiAuthorizationEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;

import com.evolveum.midpoint.authentication.impl.authorization.AuthorizationActionValue;
import com.evolveum.midpoint.authentication.impl.authorization.DescriptorLoaderImpl;
Expand Down Expand Up @@ -135,13 +134,12 @@ public void failAuthorization(String operationUrl,
@Nullable AuthorizationPhaseType phase,
@NotNull AbstractAuthorizationParameters params,
@NotNull Options options,
@Nullable Consumer<Authorization> applicableAutzConsumer,
@NotNull Task task,
@NotNull OperationResult result)
throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException, CommunicationException,
ConfigurationException, SecurityViolationException {
return securityEnforcer.decideAccess(
principal, operationUrl, phase, params, options, applicableAutzConsumer, task, result);
principal, operationUrl, phase, params, options, task, result);
}

@Override
Expand Down Expand Up @@ -334,25 +332,23 @@ private void addSecurityConfig(FilterInvocation filterInvocation, List<String> r

@Override
public @Nullable <T> ObjectFilter preProcessObjectFilter(
@Nullable MidPointPrincipal principal, String[] operationUrls, AuthorizationPhaseType phase, Class<T> searchResultType,
@Nullable ObjectFilter origFilter, String limitAuthorizationAction, List<OrderConstraintsType> paramOrderConstraints,
@NotNull Options options, Task task, OperationResult result)
@Nullable MidPointPrincipal principal,
@NotNull String @NotNull [] operationUrls,
@Nullable AuthorizationPhaseType phase,
@NotNull Class<T> filterType,
@Nullable ObjectFilter origFilter,
@Nullable String limitAuthorizationAction,
@NotNull List<OrderConstraintsType> paramOrderConstraints,
@NotNull Options options,
@NotNull Task task,
@NotNull OperationResult result)
throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException, CommunicationException,
ConfigurationException, SecurityViolationException {
return securityEnforcer.preProcessObjectFilter(
principal, operationUrls, phase, searchResultType,
principal, operationUrls, phase, filterType,
origFilter, limitAuthorizationAction, paramOrderConstraints, options, task, result);
}

@Override
public <T extends ObjectType> boolean canSearch(String[] operationUrls,
AuthorizationPhaseType phase, Class<T> objectType, boolean includeSpecial, ObjectFilter filter,
Task task, OperationResult result)
throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException, CommunicationException,
ConfigurationException, SecurityViolationException {
return securityEnforcer.canSearch(operationUrls, phase, objectType, includeSpecial, filter, task, result);
}

@Override
public <T extends ObjectType, O extends ObjectType, F> F computeTargetSecurityFilter(
MidPointPrincipal principal, String[] operationUrls, AuthorizationPhaseType phase, Class<T> searchResultType,
Expand Down
12 changes: 0 additions & 12 deletions model/model-api/src/main/java/com/evolveum/midpoint/model/api/ModelInteractionService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,18 +227,6 @@ <T extends ObjectType> ObjectFilter getDonorFilter(
throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException,
CommunicationException, ConfigurationException, SecurityViolationException;

/**
*
* @param includeSpecial include special authorizations, such as "self". If set to false those authorizations
* will be ignored. This is a good way to avoid interference of "self" when checking for
* authorizations such as ability to display role members.
*/
<T extends ObjectType, O extends ObjectType> boolean canSearch(
Class<T> resultType, Class<O> objectType, boolean includeSpecial,
ObjectQuery query, Task task, OperationResult result)
throws ObjectNotFoundException, CommunicationException, SchemaException,
ConfigurationException, SecurityViolationException, ExpressionEvaluationException;

/**
* Returns decisions for individual items for "assign" authorization. This is usually applicable to assignment parameters.
* The decisions are evaluated using the security context of a currently logged-in user.
Expand Down
2 changes: 1 addition & 1 deletion ...model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/ModelController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1653,7 +1653,7 @@ private <T> ObjectQuery preProcessQuerySecurity(
GetOperationOptions.isExecutionPhase(rootOptions) ? AuthorizationPhaseType.EXECUTION : null;
ObjectFilter secFilter = securityEnforcer.preProcessObjectFilter(
securityEnforcer.getMidPointPrincipal(), ModelAuthorizationAction.AUTZ_ACTIONS_URLS_SEARCH, phase, objectType,
origFilter, null, null, SecurityEnforcer.Options.create(), task, result);
origFilter, null, List.of(), SecurityEnforcer.Options.create(), task, result);
return updateObjectQuery(origQuery, secFilter);
}

Expand Down
17 changes: 2 additions & 15 deletions ...rc/main/java/com/evolveum/midpoint/model/impl/controller/ModelInteractionServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import javax.xml.namespace.QName;

import com.evolveum.midpoint.model.impl.controller.tasks.ActivityExecutor;
import com.evolveum.midpoint.prism.query.*;
import com.evolveum.midpoint.security.api.OtherPrivilegesLimitations;

import org.apache.commons.lang3.BooleanUtils;
Expand Down Expand Up @@ -73,10 +74,6 @@
import com.evolveum.midpoint.prism.path.ItemName;
import com.evolveum.midpoint.prism.path.ItemPath;
import com.evolveum.midpoint.prism.polystring.PolyString;
import com.evolveum.midpoint.prism.query.ObjectFilter;
import com.evolveum.midpoint.prism.query.ObjectPaging;
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.prism.query.OrderDirection;
import com.evolveum.midpoint.prism.util.CloneUtil;
import com.evolveum.midpoint.prism.util.ItemDeltaItem;
import com.evolveum.midpoint.prism.util.ItemPathTypeUtil;
Expand Down Expand Up @@ -600,20 +597,10 @@ public <T extends ObjectType> ObjectFilter getDonorFilter(
ConfigurationException, SecurityViolationException {
return securityEnforcer.preProcessObjectFilter(
securityEnforcer.getMidPointPrincipal(), ModelAuthorizationAction.AUTZ_ACTIONS_URLS_ATTORNEY, null,
searchResultType, origFilter, targetAuthorizationAction, null,
searchResultType, origFilter, targetAuthorizationAction, List.of(),
SecurityEnforcer.Options.create(), task, parentResult);
}

@Override
public <T extends ObjectType, O extends ObjectType> boolean canSearch(Class<T> resultType,
Class<O> objectType, boolean includeSpecial, ObjectQuery query, Task task, OperationResult result)
throws ObjectNotFoundException, CommunicationException, SchemaException, ConfigurationException,
SecurityViolationException, ExpressionEvaluationException {
return securityEnforcer.canSearch(
ModelAuthorizationAction.AUTZ_ACTIONS_URLS_SEARCH, null, resultType, includeSpecial,
query.getFilter(), task, result);
}

@Override
public AuthenticationsPolicyType getAuthenticationPolicy(PrismObject<UserType> user, Task task,
OperationResult parentResult) throws SchemaException, CommunicationException, ConfigurationException, SecurityViolationException, ExpressionEvaluationException {
Expand Down
37 changes: 0 additions & 37 deletions ...est/java/com/evolveum/midpoint/model/intest/security/AbstractInitializedSecurityTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -887,43 +887,6 @@ protected void assertAuditReadAllow() throws Exception {
});
}

protected void assertCanSearchRoleMemberUsers(String roleOid, boolean expectedResult) throws SchemaException, ObjectNotFoundException, CommunicationException, ConfigurationException, SecurityViolationException, ExpressionEvaluationException {
assertCanSearch("Search user members of role " + roleOid, UserType.class,
createMembersQuery(UserType.class, roleOid), expectedResult);
}

protected void assertCanSearchRoleMembers(String roleOid, boolean expectedResult) throws SchemaException, ObjectNotFoundException, CommunicationException, ConfigurationException, SecurityViolationException, ExpressionEvaluationException {
assertCanSearch("Search all members of role " + roleOid, FocusType.class,
createMembersQuery(FocusType.class, roleOid), expectedResult);
}

protected <T extends ObjectType, O extends ObjectType> void assertCanSearch(
String message, Class<T> resultType, ObjectQuery query, boolean expectedResult)
throws SchemaException, ObjectNotFoundException, CommunicationException, ConfigurationException,
SecurityViolationException, ExpressionEvaluationException {
Task task = createPlainTask("assertCanSearch");
OperationResult result = task.getResult();
String opName = "canSearch(" + message + ")";
logAttempt(opName);

boolean decision = modelInteractionService.canSearch(resultType, (Class<O>) null, false, query, task, result);

assertSuccess(result);
if (expectedResult) {
if (decision) {
logAllow(opName);
} else {
failAllow(opName, null);
}
} else {
if (decision) {
failDeny(opName);
} else {
logDeny(opName);
}
}
}

protected <O extends ObjectType> ObjectQuery createMembersQuery(Class<O> resultType, String roleOid) {
return prismContext.queryFor(resultType).item(UserType.F_ROLE_MEMBERSHIP_REF).ref(roleOid).build();
}
Expand Down
42 changes: 0 additions & 42 deletions ...ntest/src/test/java/com/evolveum/midpoint/model/intest/security/TestSecurityAdvanced.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -873,13 +873,6 @@ public void test154AutzJackApproverRead() throws Exception {
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_ORDINARY.oid), 0);
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_APPROVER_UNASSIGN_ROLES.oid), 0);

assertCanSearchRoleMemberUsers(ROLE_ORDINARY.oid, false);
assertCanSearchRoleMembers(ROLE_ORDINARY.oid, false);
assertCanSearchRoleMemberUsers(ROLE_UNINTERESTING.oid, false);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, false);
assertCanSearchRoleMemberUsers(ROLE_APPROVER_UNASSIGN_ROLES.oid, false);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, false);

assertDeny("unassign ordinary role from cobb",
(task, result) -> unassignRole(userCobbOid, ROLE_ORDINARY.oid, task, result));

Expand Down Expand Up @@ -935,13 +928,6 @@ public void test155AutzJackApproverSelf() throws Exception {
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_ORDINARY.oid), 0);
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_APPROVER_UNASSIGN_ROLES.oid), 0);

assertCanSearchRoleMemberUsers(ROLE_ORDINARY.oid, false);
assertCanSearchRoleMembers(ROLE_ORDINARY.oid, false);
assertCanSearchRoleMemberUsers(ROLE_UNINTERESTING.oid, false);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, false);
assertCanSearchRoleMemberUsers(ROLE_APPROVER_UNASSIGN_ROLES.oid, false);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, false);

assertDeny("unassign ordinary role from cobb",
(task, result) -> unassignRole(userCobbOid, ROLE_ORDINARY.oid, task, result));

Expand Down Expand Up @@ -992,13 +978,6 @@ public void test157AutzJackReadRoleMembers() throws Exception {
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_ORDINARY.oid), 2);
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_APPROVER_UNASSIGN_ROLES.oid), 0);

assertCanSearchRoleMemberUsers(ROLE_ORDINARY.oid, true);
assertCanSearchRoleMembers(ROLE_ORDINARY.oid, true);
assertCanSearchRoleMemberUsers(ROLE_UNINTERESTING.oid, true);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, true);
assertCanSearchRoleMemberUsers(ROLE_APPROVER_UNASSIGN_ROLES.oid, true);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, true);

assertDeny("unassign ordinary role from cobb",
(task, result) -> unassignRole(userCobbOid, ROLE_ORDINARY.oid, task, result));
assertDeny("unassign uninteresting role from rum",
Expand Down Expand Up @@ -1043,13 +1022,6 @@ public void test158AutzJackReadRoleMembersWrong() throws Exception {
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_ORDINARY.oid), 0);
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_APPROVER_UNASSIGN_ROLES.oid), 0);

assertCanSearchRoleMemberUsers(ROLE_ORDINARY.oid, false);
assertCanSearchRoleMembers(ROLE_ORDINARY.oid, true); // We can read roleMembershipRef from roles that are members of those roles
assertCanSearchRoleMemberUsers(ROLE_UNINTERESTING.oid, false);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, true);
assertCanSearchRoleMemberUsers(ROLE_APPROVER_UNASSIGN_ROLES.oid, false);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, true);

assertDeny("unassign ordinary role from cobb",
(task, result) -> unassignRole(userCobbOid, ROLE_ORDINARY.oid, task, result));
assertDeny("unassign uninteresting role from rum",
Expand Down Expand Up @@ -1094,13 +1066,6 @@ public void test159AutzJackReadRoleMembersNone() throws Exception {
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_ORDINARY.oid), 0);
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_APPROVER_UNASSIGN_ROLES.oid), 0);

assertCanSearchRoleMemberUsers(ROLE_ORDINARY.oid, false);
assertCanSearchRoleMembers(ROLE_ORDINARY.oid, false);
assertCanSearchRoleMemberUsers(ROLE_UNINTERESTING.oid, false);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, false);
assertCanSearchRoleMemberUsers(ROLE_APPROVER_UNASSIGN_ROLES.oid, false);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, false);

assertDeny("unassign ordinary role from cobb",
(task, result) -> unassignRole(userCobbOid, ROLE_ORDINARY.oid, task, result));
assertDeny("unassign uninteresting role from rum",
Expand All @@ -1124,13 +1089,6 @@ private void assert15xCommon() throws Exception {
assertSearch(UserType.class, createMembersQuery(UserType.class, ROLE_APPROVER_UNASSIGN_ROLES.oid), 0);
assertSearch(FocusType.class, createMembersQuery(FocusType.class, ROLE_APPROVER_UNASSIGN_ROLES.oid), 0);

assertCanSearchRoleMemberUsers(ROLE_ORDINARY.oid, true);
assertCanSearchRoleMembers(ROLE_ORDINARY.oid, true);
assertCanSearchRoleMemberUsers(ROLE_UNINTERESTING.oid, false);
assertCanSearchRoleMembers(ROLE_UNINTERESTING.oid, false);
assertCanSearchRoleMemberUsers(ROLE_APPROVER_UNASSIGN_ROLES.oid, false);
assertCanSearchRoleMembers(ROLE_APPROVER_UNASSIGN_ROLES.oid, false);

assertAllow("unassign ordinary role from cobb",
(task, result) -> unassignRole(userCobbOid, ROLE_ORDINARY.oid, task, result));

Expand Down

0 comments on commit 066abb2

Please sign in to comment.