Skip to content

Commit

Permalink
Fixed MID-3942: Workflow process is skipped on disabled roles
Browse files Browse the repository at this point in the history
  • Loading branch information
mederly committed Jun 7, 2017
1 parent 506ffe1 commit 5425657
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 36 deletions.
Expand Up @@ -19,6 +19,7 @@
import com.evolveum.midpoint.util.DebugDumpable;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.FocusType;
import org.jetbrains.annotations.NotNull;

/**
* @author semancik
Expand Down Expand Up @@ -46,6 +47,7 @@ public interface EvaluatedAssignmentTarget extends DebugDumpable {
*/
AssignmentType getAssignment();

@NotNull
AssignmentPath getAssignmentPath();

boolean isValid();
Expand Down
Expand Up @@ -660,14 +660,19 @@ private void evaluateSegmentTarget(AssignmentPathSegmentImpl segment, PlusMinusZ

checkRelationWithTarget(segment, targetType, relation);

if (!LensUtil.isFocusValid(targetType, now, activationComputer)) {
LOGGER.trace("Skipping evaluation of {} because it is not valid", targetType);
return;
boolean isTargetValid = LensUtil.isFocusValid(targetType, now, activationComputer);
if (!isTargetValid) {
if (!segment.isValidityOverride()) {
LOGGER.trace("Skipping evaluation of {} because it is not valid and validityOverride is not set", targetType);
return;
} else {
isValid = false;
}
}

InternalMonitor.recordRoleEvaluation(targetType, true);

if (targetType instanceof AbstractRoleType) {
if (isTargetValid && targetType instanceof AbstractRoleType) {
MappingType roleCondition = ((AbstractRoleType)targetType).getCondition();
if (roleCondition != null) {
AssignmentPathVariables assignmentPathVariables = LensUtil.computeAssignmentPathVariables(ctx.assignmentPath);
Expand Down Expand Up @@ -695,9 +700,21 @@ private void evaluateSegmentTarget(AssignmentPathSegmentImpl segment, PlusMinusZ
segment.getAssignment(),
isValid);
ctx.evalAssignment.addRole(evalAssignmentTarget, relativeMode);


// we need to evaluate assignments also for disabled targets, because of target policy rules
for (AssignmentType roleAssignment : targetType.getAssignment()) {
evaluateAssignment(segment, relativeMode, isValid, ctx, targetType, relation, roleAssignment);
}

if ((isNonNegative(relativeMode)) && segment.isProcessMembership()) {
collectMembership(targetType, relation, ctx);
if (isTargetValid || !ObjectTypeUtil.isMembershipRelation(relation)) {
// we want to collect approver/owner/whatever-non-membership relations also for disabled targets (MID-3942)
collectMembership(targetType, relation, ctx);
}
}

if (!isTargetValid) {
return;
}

// We continue evaluation even if the relation is non-membership and non-delegation.
Expand All @@ -708,9 +725,6 @@ private void evaluateSegmentTarget(AssignmentPathSegmentImpl segment, PlusMinusZ
evaluateInducement(segment, relativeMode, isValid, ctx, targetType, roleInducement);
}
}
for (AssignmentType roleAssignment : targetType.getAssignment()) {
evaluateAssignment(segment, relativeMode, isValid, ctx, targetType, relation, roleAssignment);
}

//boolean matchesOrder = AssignmentPathSegmentImpl.computeMatchingOrder(segment.getEvaluationOrder(), 1, Collections.emptyList());
if (segment.isMatchingOrder() && targetType instanceof AbstractRoleType && isNonNegative(relativeMode)) {
Expand Down
Expand Up @@ -19,6 +19,7 @@
import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.util.DebugUtil;
import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
import org.jetbrains.annotations.NotNull;

import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -31,14 +32,14 @@ public class EvaluatedAssignmentTargetImpl implements EvaluatedAssignmentTarget

final PrismObject<? extends FocusType> target;
private final boolean evaluateConstructions;
private final AssignmentPathImpl assignmentPath; // TODO reconsider (maybe we should store only some lightweight information here)
@NotNull private final AssignmentPathImpl assignmentPath; // TODO reconsider (maybe we should store only some lightweight information here)
private final AssignmentType assignment;
private Collection<ExclusionPolicyConstraintType> exclusions = null;
private final boolean isValid;

EvaluatedAssignmentTargetImpl(
PrismObject<? extends FocusType> target, boolean evaluateConstructions,
AssignmentPathImpl assignmentPath, AssignmentType assignment,
@NotNull AssignmentPathImpl assignmentPath, AssignmentType assignment,
boolean isValid) {
this.target = target;
this.evaluateConstructions = evaluateConstructions;
Expand Down Expand Up @@ -73,6 +74,7 @@ public AssignmentType getAssignment() {
}

@Override
@NotNull
public AssignmentPathImpl getAssignmentPath() {
return assignmentPath;
}
Expand Down
Expand Up @@ -623,6 +623,9 @@ public <F extends FocusType> void addGlobalPoliciesToAssignments(LensContext<F>
}
for (EvaluatedAssignmentImpl<F> evaluatedAssignment : evaluatedAssignmentTriple.getAllValues()) {
for (EvaluatedAssignmentTargetImpl target : evaluatedAssignment.getRoles().getNonNegativeValues()) {
if (!target.getAssignmentPath().last().isMatchingOrder()) {
continue;
}
if (!repositoryService.selectorMatches(globalPolicyRule.getTargetSelector(),
target.getTarget(), LOGGER, "Global policy rule "+globalPolicyRule.getName()+" target selector: ")) {
continue;
Expand All @@ -631,9 +634,9 @@ public <F extends FocusType> void addGlobalPoliciesToAssignments(LensContext<F>
LOGGER.trace("Skipping global policy rule because the condition evaluated to false: {}", globalPolicyRule);
continue;
}
EvaluatedPolicyRule evaluatedRule = new EvaluatedPolicyRuleImpl(globalPolicyRule,
target.getAssignmentPath() != null ? target.getAssignmentPath().clone() : null);
if (target.getAssignmentPath() != null && target.getAssignmentPath().size() == 1) {
EvaluatedPolicyRule evaluatedRule = new EvaluatedPolicyRuleImpl(globalPolicyRule, target.getAssignmentPath().clone());
boolean direct = target.getAssignmentPath().getFirstOrderChain().size() == 1;
if (direct) {
evaluatedAssignment.addThisTargetPolicyRule(evaluatedRule);
} else {
evaluatedAssignment.addOtherTargetPolicyRule(evaluatedRule);
Expand Down
Expand Up @@ -876,23 +876,29 @@ public void test310DisableRoleEngineer() throws Exception {
TestUtil.assertSuccess(result);

assertNotNull(evaluatedAssignment);
display("Evaluated assignment",evaluatedAssignment.debugDump());
assertEquals(0, evaluatedAssignment.getConstructionTriple().size());
display("Evaluated assignment", evaluatedAssignment.debugDump());
assertEquals(2, evaluatedAssignment.getConstructionTriple().size());
PrismAsserts.assertParentConsistency(userTypeJack.asPrismObject());

assertNoConstruction(evaluatedAssignment, ZERO, "title");
for (Construction<UserType> construction : evaluatedAssignment.getConstructionSet(ZERO)) {
assertEquals("Wrong validity for " + construction, false, construction.isValid());
}

assertConstruction(evaluatedAssignment, ZERO, "title", ZERO, "Engineer");
assertConstruction(evaluatedAssignment, ZERO, "title", PLUS);
assertConstruction(evaluatedAssignment, ZERO, "title", MINUS);
assertNoConstruction(evaluatedAssignment, PLUS, "title");
assertNoConstruction(evaluatedAssignment, MINUS, "title");

assertNoConstruction(evaluatedAssignment, ZERO, "location");
assertConstruction(evaluatedAssignment, ZERO, "location", ZERO, "Caribbean");
assertConstruction(evaluatedAssignment, ZERO, "location", PLUS);
assertConstruction(evaluatedAssignment, ZERO, "location", MINUS);
assertNoConstruction(evaluatedAssignment, PLUS, "location");
assertNoConstruction(evaluatedAssignment, MINUS, "location");

assertEquals("Wrong number of admin GUI configs", 0, evaluatedAssignment.getAdminGuiConfigurations().size());
}



protected void assertNoConstruction(EvaluatedAssignmentImpl<UserType> evaluatedAssignment, PlusMinusZero constructionSet, String attributeName) {
Collection<Construction<UserType>> constructions = evaluatedAssignment.getConstructionSet(constructionSet);
for (Construction construction : constructions) {
Expand Down
Expand Up @@ -1077,42 +1077,42 @@ public void test400AssignJackPirate() throws Exception {
.assertAssignmentPath(2);

// swap indices if internals of evaluator change and "Pirate" branch is executed first
getRunInfo("MetaroleCrewMember!", 1)
getRunInfo("MetaroleCrewMember!", 0)
.assertThisAssignment("->MetaroleCrewMember")
.assertImmediateAssignment("->Pirate")
.assertSource("Pirate")
.assertImmediateRole(null)
.assertAssignmentPath(2);

getRunInfo("MetaroleCrewMember!", 0)
getRunInfo("MetaroleCrewMember!", 1)
.assertThisAssignment("->MetaroleCrewMember")
.assertImmediateAssignment("->Sailor")
.assertSource("Sailor")
.assertImmediateRole("Pirate")
.assertAssignmentPath(3);

getRunInfo("MetarolePerson!", 1)
getRunInfo("MetarolePerson!", 0)
.assertThisAssignment("->MetarolePerson")
.assertImmediateAssignment("->MetaroleCrewMember")
.assertSource("MetaroleCrewMember")
.assertImmediateRole("Pirate")
.assertAssignmentPath(3);

getRunInfo("MetarolePerson!", 0)
getRunInfo("MetarolePerson!", 1)
.assertThisAssignment("->MetarolePerson")
.assertImmediateAssignment("->MetaroleCrewMember")
.assertSource("MetaroleCrewMember")
.assertImmediateRole("Sailor")
.assertAssignmentPath(4);

getRunInfo("Human!", 1)
getRunInfo("Human!", 0)
.assertThisAssignment("->Human")
.assertImmediateAssignment("->MetarolePerson")
.assertSource("MetarolePerson")
.assertImmediateRole("MetaroleCrewMember")
.assertAssignmentPath(4);

getRunInfo("Human!", 0)
getRunInfo("Human!", 1)
.assertThisAssignment("->Human")
.assertImmediateAssignment("->MetarolePerson")
.assertSource("MetarolePerson")
Expand Down Expand Up @@ -1142,28 +1142,28 @@ public void test400AssignJackPirate() throws Exception {
.assertImmediateRole("Pirate")
.assertAssignmentPath(3);

getRunInfo("MetaroleCrewMember-MetarolePerson!", 1)
getRunInfo("MetaroleCrewMember-MetarolePerson!", 0)
.assertThisAssignment("->MetarolePerson")
.assertImmediateAssignment("->MetaroleCrewMember")
.assertSource("MetaroleCrewMember")
.assertImmediateRole("Pirate")
.assertAssignmentPath(3);

getRunInfo("MetaroleCrewMember-MetarolePerson!", 0)
getRunInfo("MetaroleCrewMember-MetarolePerson!", 1)
.assertThisAssignment("->MetarolePerson")
.assertImmediateAssignment("->MetaroleCrewMember")
.assertSource("MetaroleCrewMember")
.assertImmediateRole("Sailor")
.assertAssignmentPath(4);

getRunInfo("MetarolePerson-Human!", 1)
getRunInfo("MetarolePerson-Human!", 0)
.assertThisAssignment("->Human")
.assertImmediateAssignment("->MetarolePerson")
.assertSource("MetarolePerson")
.assertImmediateRole("MetaroleCrewMember")
.assertAssignmentPath(4);

getRunInfo("MetarolePerson-Human!", 0)
getRunInfo("MetarolePerson-Human!", 1)
.assertThisAssignment("->Human")
.assertImmediateAssignment("->MetarolePerson")
.assertSource("MetarolePerson")
Expand All @@ -1184,42 +1184,42 @@ public void test400AssignJackPirate() throws Exception {
.assertImmediateRole("Pirate")
.assertAssignmentPath(3);

getRunInfo("C3", 1)
getRunInfo("C3", 0)
.assertImmediateAssignment("->MetaroleCrewMember")
.assertSource("MetaroleCrewMember")
.assertThisObject("Pirate")
.assertImmediateRole("Pirate")
.assertAssignmentPath(3);

getRunInfo("C3", 0)
getRunInfo("C3", 1)
.assertImmediateAssignment("->MetaroleCrewMember")
.assertSource("MetaroleCrewMember")
.assertThisObject("Sailor")
.assertImmediateRole("Sailor")
.assertAssignmentPath(4);

getRunInfo("C4", 1)
getRunInfo("C4", 0)
.assertImmediateAssignment("->MetarolePerson")
.assertSource("MetarolePerson")
.assertThisObject("MetaroleCrewMember") // TODO
.assertImmediateRole("MetaroleCrewMember")
.assertAssignmentPath(4);

getRunInfo("C4", 0)
getRunInfo("C4", 1)
.assertImmediateAssignment("->MetarolePerson")
.assertSource("MetarolePerson")
.assertThisObject("MetaroleCrewMember") // TODO
.assertImmediateRole("MetaroleCrewMember")
.assertAssignmentPath(5);

getRunInfo("C5", 1)
getRunInfo("C5", 0)
.assertImmediateAssignment("->Human")
.assertSource("Human")
.assertThisObject("Human")
.assertImmediateRole("MetarolePerson")
.assertAssignmentPath(5);

getRunInfo("C5", 0)
getRunInfo("C5", 1)
.assertImmediateAssignment("->Human")
.assertSource("Human")
.assertThisObject("Human")
Expand Down
Expand Up @@ -66,6 +66,9 @@
<role oid="00000001-d34d-b33f-f00d-000000000001">
<name>Role1</name>
<approverRef oid="00000000-d34d-b33f-f00d-111111111111" type="UserType"/>
<activation>
<administrativeStatus>disabled</administrativeStatus>
</activation>
</role>

<role oid="00000001-d34d-b33f-f00d-000000000002" xsi:type="RoleType">
Expand Down
3 changes: 3 additions & 0 deletions model/workflow-impl/src/test/resources/policy/role-role1.xml
Expand Up @@ -17,6 +17,9 @@
<role xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
oid="00000001-d34d-b33f-f00d-000000000001">
<name>Role1</name>
<activation>
<administrativeStatus>disabled</administrativeStatus>
</activation>
<inducement>
<targetRef oid="00000001-d34d-b33f-f00d-000000000010" type="RoleType" />
</inducement>
Expand Down
3 changes: 3 additions & 0 deletions model/workflow-impl/src/test/resources/policy/role-role1a.xml
Expand Up @@ -17,6 +17,9 @@
<role xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
oid="00000001-d34d-b33f-f00d-00000000001a">
<name>Role1a</name>
<activation>
<administrativeStatus>disabled</administrativeStatus>
</activation>
<inducement>
<targetRef oid="00000001-d34d-b33f-f00d-00000000010a" type="RoleType" />
</inducement>
Expand Down
3 changes: 3 additions & 0 deletions model/workflow-impl/src/test/resources/policy/role-role1b.xml
Expand Up @@ -20,6 +20,9 @@
<assignment> <!-- metarole 1 'default' -->
<targetRef oid="00000001-d34d-b33f-f00d-M00000000001" type="RoleType"/>
</assignment>
<activation>
<administrativeStatus>disabled</administrativeStatus>
</activation>
<inducement>
<targetRef oid="00000001-d34d-b33f-f00d-00000000010b" type="RoleType" />
</inducement>
Expand Down

0 comments on commit 5425657

Please sign in to comment.