Skip to content

Commit

Permalink
Fix NPE with policy constraint on focus deletion
Browse files Browse the repository at this point in the history
When a policy constraint with custom presentation (message)
was evaluated during focus deletion operation, a NPE was thrown.

This commit fixes that, and reviews other similar places where
such exceptions could occur.

This resolves MID-7908.
  • Loading branch information
mederly committed Apr 26, 2022
1 parent 7fc7460 commit 4bd3847
Show file tree
Hide file tree
Showing 12 changed files with 146 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -420,21 +420,24 @@ public List<PolicyActionType> getEnabledActions() {
return enabledActions;
}

private <AH extends AssignmentHolderType> VariablesMap createVariablesMap(PolicyRuleEvaluationContext<AH> rctx, PrismObject<AH> object) {
private <AH extends AssignmentHolderType> VariablesMap createVariablesMap(
PolicyRuleEvaluationContext<AH> rctx, PrismObject<AH> object) {
VariablesMap var = new VariablesMap();
var.put(ExpressionConstants.VAR_USER, object, object.getDefinition());
var.put(ExpressionConstants.VAR_FOCUS, object, object.getDefinition());
var.put(ExpressionConstants.VAR_OBJECT, object, object.getDefinition());
PrismContext prismContext = PrismContext.get();
PrismObjectDefinition<?> definition = rctx != null ? rctx.getObjectDefinition() :
prismContext.getSchemaRegistry().findObjectDefinitionByCompileTimeClass(AssignmentHolderType.class);
var.put(ExpressionConstants.VAR_USER, object, definition);
var.put(ExpressionConstants.VAR_FOCUS, object, definition);
var.put(ExpressionConstants.VAR_OBJECT, object, definition);
if (rctx instanceof AssignmentPolicyRuleEvaluationContext) {
AssignmentPolicyRuleEvaluationContext<AH> actx = (AssignmentPolicyRuleEvaluationContext<AH>) rctx;
var.put(ExpressionConstants.VAR_TARGET, actx.evaluatedAssignment.getTarget(), actx.evaluatedAssignment.getTarget().getDefinition());
PrismObject<?> target = actx.evaluatedAssignment.getTarget();
var.put(ExpressionConstants.VAR_TARGET, target, target != null ? target.getDefinition() : getObjectDefinition());
var.put(ExpressionConstants.VAR_EVALUATED_ASSIGNMENT, actx.evaluatedAssignment, EvaluatedAssignment.class);
AssignmentType assignment = actx.evaluatedAssignment.getAssignment(actx.state == ObjectState.BEFORE);
var.put(ExpressionConstants.VAR_ASSIGNMENT, assignment, getAssignmentDefinition(assignment, prismContext));
} else if (rctx instanceof ObjectPolicyRuleEvaluationContext) {
PrismObjectDefinition<ObjectType> targetDef = prismContext.getSchemaRegistry().findObjectDefinitionByCompileTimeClass(ObjectType.class);
var.put(ExpressionConstants.VAR_TARGET, null, targetDef);
var.put(ExpressionConstants.VAR_TARGET, null, getObjectDefinition());
var.put(ExpressionConstants.VAR_EVALUATED_ASSIGNMENT, null, EvaluatedAssignment.class);
var.put(ExpressionConstants.VAR_ASSIGNMENT, null, getAssignmentDefinition(null, prismContext));
} else if (rctx != null) {
Expand All @@ -444,6 +447,10 @@ private <AH extends AssignmentHolderType> VariablesMap createVariablesMap(Policy
return var;
}

private PrismObjectDefinition<ObjectType> getObjectDefinition() {
return PrismContext.get().getSchemaRegistry().findObjectDefinitionByCompileTimeClass(ObjectType.class);
}

private PrismContainerDefinition<?> getAssignmentDefinition(AssignmentType assignment, PrismContext prismContext) {
if (assignment != null) {
PrismContainerDefinition<?> definition = assignment.asPrismContainerValue().getDefinition();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.evolveum.midpoint.prism.*;
import com.evolveum.midpoint.prism.delta.*;
import com.evolveum.midpoint.prism.path.ItemPath;
import com.evolveum.midpoint.util.MiscUtil;
import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
import com.evolveum.prism.xml.ns._public.types_3.ItemPathType;

Expand Down Expand Up @@ -97,15 +98,24 @@ public <F extends AssignmentHolderType> void process(LensContext<F> context, XML
}
}

private <F extends AssignmentHolderType> boolean shouldTransition(LensContext<F> context, LifecycleStateTransitionType transitionType, String targetLifecycleState, Task task, OperationResult result) throws SchemaException, ExpressionEvaluationException, ObjectNotFoundException, CommunicationException, ConfigurationException, SecurityViolationException {
private <F extends AssignmentHolderType> boolean shouldTransition(
LensContext<F> context,
LifecycleStateTransitionType transitionType,
String targetLifecycleState,
Task task,
OperationResult result) throws SchemaException, ExpressionEvaluationException, ObjectNotFoundException,
CommunicationException, ConfigurationException, SecurityViolationException {
ExpressionType conditionExpressionType = transitionType.getCondition();
if (conditionExpressionType == null) {
return false;
}
String desc = "condition for transition to state "+targetLifecycleState+" for "+context.getFocusContext().getHumanReadableName();

VariablesMap variables = new VariablesMap();
variables.put(ExpressionConstants.VAR_OBJECT, context.getFocusContext().getObjectNew(), context.getFocusContext().getObjectNew().getDefinition());
PrismObject<F> objectNew = MiscUtil.requireNonNull(
context.getFocusContext().getObjectNew(),
() -> new IllegalStateException("No focus 'after' (should have been checked by the processor invocation code)"));
variables.put(ExpressionConstants.VAR_OBJECT, objectNew, objectNew.getDefinition());
// TODO: more variables?

Expression<PrismPropertyValue<Boolean>,PrismPropertyDefinition<Boolean>> expression = expressionFactory.makeExpression(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@

import java.util.Collection;

public class AssignmentPolicyRuleEvaluationContext<AH extends AssignmentHolderType> extends PolicyRuleEvaluationContext<AH> {
public class AssignmentPolicyRuleEvaluationContext<AH extends AssignmentHolderType>
extends PolicyRuleEvaluationContext<AH>
implements Cloneable {

@NotNull public final EvaluatedAssignmentImpl<AH> evaluatedAssignment;
public final boolean isAdded;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
/**
* Evaluation context for object-based policy rule.
*/
public class ObjectPolicyRuleEvaluationContext<AH extends AssignmentHolderType> extends PolicyRuleEvaluationContext<AH> {
public class ObjectPolicyRuleEvaluationContext<AH extends AssignmentHolderType>
extends PolicyRuleEvaluationContext<AH>
implements Cloneable {

ObjectPolicyRuleEvaluationContext(@NotNull EvaluatedPolicyRule policyRule, RulesEvaluationContext globalCtx,
LensContext<AH> context, Task task) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.evolveum.midpoint.model.impl.lens.LensContext;
import com.evolveum.midpoint.model.impl.lens.LensFocusContext;
import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.prism.PrismObjectDefinition;
import com.evolveum.midpoint.task.api.Task;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentHolderType;
import org.jetbrains.annotations.NotNull;
Expand All @@ -21,7 +22,7 @@
/**
* Evaluation context for a policy rule.
*/
public abstract class PolicyRuleEvaluationContext<AH extends AssignmentHolderType> implements Cloneable {
public abstract class PolicyRuleEvaluationContext<AH extends AssignmentHolderType> {

@NotNull public final EvaluatedPolicyRule policyRule;
@NotNull public final LensContext<AH> lensContext;
Expand Down Expand Up @@ -55,6 +56,15 @@ public PrismObject<AH> getObject() {
}
}

public PrismObjectDefinition<AH> getObjectDefinition() {
PrismObject<AH> object = getObject();
if (object != null && object.getDefinition() != null) {
return object.getDefinition();
} else {
return focusContext.getObjectDefinition();
}
}

@SuppressWarnings("BooleanMethodIsAlwaysInverted")
public boolean isApplicableToState() {
return getObject() != null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,8 @@ private <AH extends AssignmentHolderType> boolean isRuleConditionTrue(GlobalPoli
MappingBuilder<PrismPropertyValue<Boolean>, PrismPropertyDefinition<Boolean>> builder = mappingFactory
.createMappingBuilder();
ObjectDeltaObject<AH> focusOdo = new ObjectDeltaObject<>(focus, null, focus, focus.getDefinition());
PrismObject<?> target = evaluatedAssignment != null ? evaluatedAssignment.getTarget() : null;

builder = builder.mappingBean(condition)
.mappingKind(MappingKindType.POLICY_RULE_CONDITION)
.contextDescription("condition in global policy rule " + globalPolicyRule.getName())
Expand All @@ -541,9 +543,11 @@ private <AH extends AssignmentHolderType> boolean isRuleConditionTrue(GlobalPoli
.addVariableDefinition(ExpressionConstants.VAR_FOCUS, focusOdo)
.addAliasRegistration(ExpressionConstants.VAR_USER, null)
.addAliasRegistration(ExpressionConstants.VAR_FOCUS, null)
.addVariableDefinition(ExpressionConstants.VAR_TARGET, evaluatedAssignment != null ? evaluatedAssignment.getTarget() : null, EvaluatedAssignment.class)
.addVariableDefinition(ExpressionConstants.VAR_TARGET,
target, target != null ? target.getDefinition() : getObjectDefinition())
.addVariableDefinition(ExpressionConstants.VAR_EVALUATED_ASSIGNMENT, evaluatedAssignment, EvaluatedAssignment.class)
.addVariableDefinition(ExpressionConstants.VAR_ASSIGNMENT, evaluatedAssignment != null ? evaluatedAssignment.getAssignment() : null, AssignmentType.class)
.addVariableDefinition(ExpressionConstants.VAR_ASSIGNMENT,
evaluatedAssignment != null ? evaluatedAssignment.getAssignment() : null, AssignmentType.class)
.rootNode(focusOdo);

MappingImpl<PrismPropertyValue<Boolean>, PrismPropertyDefinition<Boolean>> mapping = builder.build();
Expand All @@ -553,6 +557,10 @@ private <AH extends AssignmentHolderType> boolean isRuleConditionTrue(GlobalPoli
PrismValueDeltaSetTriple<PrismPropertyValue<Boolean>> conditionTriple = mapping.getOutputTriple();
return conditionTriple != null && ExpressionUtil.computeConditionResult(conditionTriple.getNonNegativeValues()); // TODO: null -> true (in the method) - ok?
}
//endregion

private PrismObjectDefinition<?> getObjectDefinition() {
return PrismContext.get().getSchemaRegistry().findObjectDefinitionByCompileTimeClass(ObjectType.class);
}

//endregion
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,24 +53,28 @@ public <AH extends AssignmentHolderType> VariablesMap createVariablesMap(PolicyR
JAXBElement<? extends AbstractPolicyConstraintType> constraintElement) {
VariablesMap var = new VariablesMap();
PrismObject<AH> object = rctx.getObject();
var.put(ExpressionConstants.VAR_USER, object, object.getDefinition());
var.put(ExpressionConstants.VAR_FOCUS, object, object.getDefinition());
var.put(ExpressionConstants.VAR_OBJECT, object, object.getDefinition());
PrismObjectDefinition<AH> objectDefinition = rctx.getObjectDefinition();
var.put(ExpressionConstants.VAR_USER, object, objectDefinition);
var.put(ExpressionConstants.VAR_FOCUS, object, objectDefinition);
var.put(ExpressionConstants.VAR_OBJECT, object, objectDefinition);
var.put(ExpressionConstants.VAR_OBJECT_DISPLAY_INFORMATION,
LocalizationUtil.createLocalizableMessageType(createDisplayInformation(object, false)), LocalizableMessageType.class);
if (rctx instanceof AssignmentPolicyRuleEvaluationContext) {
AssignmentPolicyRuleEvaluationContext actx = (AssignmentPolicyRuleEvaluationContext<AH>) rctx;
PrismObject target = actx.evaluatedAssignment.getTarget();
var.put(ExpressionConstants.VAR_TARGET, target, target.getDefinition());
AssignmentPolicyRuleEvaluationContext<AH> actx = (AssignmentPolicyRuleEvaluationContext<AH>) rctx;
PrismObject<?> target = actx.evaluatedAssignment.getTarget();
if (target != null) {
var.put(ExpressionConstants.VAR_TARGET, target, target.getDefinition());
} else {
var.put(ExpressionConstants.VAR_TARGET, null, getObjectTypeDefinition());
}
var.put(ExpressionConstants.VAR_TARGET_DISPLAY_INFORMATION,
LocalizationUtil.createLocalizableMessageType(createDisplayInformation(target, false)), LocalizableMessageType.class);
var.put(ExpressionConstants.VAR_EVALUATED_ASSIGNMENT, actx.evaluatedAssignment, EvaluatedAssignment.class);
AssignmentType assignment = actx.evaluatedAssignment.getAssignment(actx.state == ObjectState.BEFORE);
var.put(ExpressionConstants.VAR_ASSIGNMENT, assignment, AssignmentType.class);
} else {
SchemaRegistry schemaRegistry = PrismContext.get().getSchemaRegistry();
PrismObjectDefinition<ObjectType> targetDef = schemaRegistry.findObjectDefinitionByCompileTimeClass(ObjectType.class);
var.put(ExpressionConstants.VAR_TARGET, null, targetDef);
var.put(ExpressionConstants.VAR_TARGET, null, getObjectTypeDefinition());
var.put(ExpressionConstants.VAR_TARGET_DISPLAY_INFORMATION, null, LocalizableMessageType.class);
var.put(ExpressionConstants.VAR_EVALUATED_ASSIGNMENT, null, EvaluatedAssignment.class);
PrismContainerDefinition<AssignmentType> assignmentDef = schemaRegistry
Expand All @@ -85,6 +89,10 @@ public <AH extends AssignmentHolderType> VariablesMap createVariablesMap(PolicyR
return var;
}

private PrismObjectDefinition<?> getObjectTypeDefinition() {
return PrismContext.get().getSchemaRegistry().findObjectDefinitionByCompileTimeClass(ObjectType.class);
}

public boolean evaluateBoolean(ExpressionType expressionBean, VariablesMap VariablesMap,
String contextDescription, Task task, OperationResult result)
throws ObjectNotFoundException, SchemaException, ExpressionEvaluationException, CommunicationException, ConfigurationException, SecurityViolationException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,12 @@ public static VariablesMap getDefaultVariablesMap(@NotNull LensContext<?> contex
variables.put(ExpressionConstants.VAR_ITERATION_TOKEN, LensUtil.getIterationTokenVariableValue(projCtx), String.class);
}

variables.put(ExpressionConstants.VAR_CONFIGURATION, context.getSystemConfiguration(), context.getSystemConfiguration().getDefinition());
PrismObject<SystemConfigurationType> systemConfiguration = context.getSystemConfiguration();
if (systemConfiguration != null) {
variables.put(ExpressionConstants.VAR_CONFIGURATION, systemConfiguration, systemConfiguration.getDefinition());
} else {
variables.put(ExpressionConstants.VAR_CONFIGURATION, null, SystemConfigurationType.class);
}
return variables;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,11 @@ public VariablesMap getDefaultVariables(Event event, OperationResult result) {
variables.put(ExpressionConstants.VAR_TEXT_FORMATTER, textFormatter, TextFormatter.class);
variables.put(ExpressionConstants.VAR_NOTIFICATION_FUNCTIONS, notificationFunctions, NotificationFunctions.class);
PrismObject<SystemConfigurationType> systemConfiguration = getSystemConfiguration(result);
variables.put(ExpressionConstants.VAR_CONFIGURATION, systemConfiguration, systemConfiguration.getDefinition());
if (systemConfiguration != null) {
variables.put(ExpressionConstants.VAR_CONFIGURATION, systemConfiguration, systemConfiguration.getDefinition());
} else {
variables.put(ExpressionConstants.VAR_CONFIGURATION, null, SystemConfigurationType.class);
}
return variables;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ public class TestMiscellaneous extends AbstractWfTestPolicy {

private static final TestResource<TaskType> TASK_CLEANUP = new TestResource<>(TEST_RESOURCE_DIR, "task-cleanup.xml", "781a7c9a-7b37-45c6-9154-5e57f5ad077f");

private static final TestResource<RoleType> ROLE_TEST370 = new TestResource<>(TEST_RESOURCE_DIR, "role-test370.xml", "2c226eba-7279-4768-a34a-38392e3fcb19");
private static final TestResource<UserType> USER_TEST370 = new TestResource<>(TEST_RESOURCE_DIR, "user-test370.xml", "a981ea50-d069-431d-86dc-f4c7dbbc4723");

@Override
protected PrismObject<UserType> getDefaultActor() {
return userAdministrator;
Expand Down Expand Up @@ -608,6 +611,29 @@ public void test360ApproveAsAttorneyGizmoduck() throws Exception {
// @formatter:on
}

/**
* Deletes a user that has an assignment-related constraint with a custom message.
*
* This used to fail with an NPE - see MID-7908.
*/
@Test
public void test370UnassignRoleWithMessage() throws Exception {
Task task = getTestTask();
OperationResult result = task.getResult();
login(userAdministrator);

given("user and role are created (in raw mode)");
repoAdd(ROLE_TEST370, result);
repoAdd(USER_TEST370, result);

when("user is deleted");
deleteObject(UserType.class, USER_TEST370.oid, task, result);

then("user is gone");
assertSuccess(result);
assertNoObject(UserType.class, USER_TEST370.oid);
}

/**
* Cleans up closed cases.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<!--
~ Copyright (c) 2022 Evolveum and contributors
~
~ This work is dual-licensed under the Apache License 2.0
~ and European Union Public License. See LICENSE file for details.
-->

<role xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
oid="2c226eba-7279-4768-a34a-38392e3fcb19">
<name>test370</name>
<assignment>
<policyRule>
<policyConstraints>
<assignment>
<presentation>
<message>
<fallbackMessage>A custom message</fallbackMessage>
</message>
</presentation>
<operation>delete</operation>
</assignment>
</policyConstraints>
</policyRule>
</assignment>
</role>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<!--
~ Copyright (c) 2022 Evolveum and contributors
~
~ This work is dual-licensed under the Apache License 2.0
~ and European Union Public License. See LICENSE file for details.
-->

<user xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
oid="a981ea50-d069-431d-86dc-f4c7dbbc4723">
<name>test370</name>
<assignment>
<targetRef oid="2c226eba-7279-4768-a34a-38392e3fcb19" type="RoleType"/>
</assignment>
</user>

0 comments on commit 4bd3847

Please sign in to comment.