From 65945e9a833b54de93d22ae6eef39b45c2c50085 Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Tue, 21 Mar 2017 10:59:57 +0100 Subject: [PATCH] Added relation parameter to min/maxAssignees policy constraints (MID-3797). --- .../midpoint/schema/util/ObjectTypeUtil.java | 2 +- .../xml/ns/public/common/common-policy-3.xsd | 2 +- .../lens/projector/PolicyRuleProcessor.java | 124 ++++++++++++------ .../midpoint/model/intest/rbac/TestRbac.java | 112 +++++++++++++++- .../model/intest/rbac/TestRbacDeprecated.java | 63 ++------- .../src/test/resources/rbac/role-cannibal.xml | 4 + .../src/test/resources/rbac/role-governor.xml | 16 ++- .../test/AbstractModelIntegrationTest.java | 17 ++- 8 files changed, 236 insertions(+), 104 deletions(-) diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectTypeUtil.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectTypeUtil.java index 81aaf5cd947..a31e59cc211 100644 --- a/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectTypeUtil.java +++ b/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectTypeUtil.java @@ -627,7 +627,7 @@ public static boolean relationsEquivalent(QName relation1, QName relation2) { if (ObjectTypeUtil.isDefaultRelation(relation1)) { return ObjectTypeUtil.isDefaultRelation(relation2); } else { - return QNameUtil.match(relation2, relation1); + return QNameUtil.match(relation1, relation2); } } } \ No newline at end of file diff --git a/infra/schema/src/main/resources/xml/ns/public/common/common-policy-3.xsd b/infra/schema/src/main/resources/xml/ns/public/common/common-policy-3.xsd index a41c796539f..8bbbd411a5a 100644 --- a/infra/schema/src/main/resources/xml/ns/public/common/common-policy-3.xsd +++ b/infra/schema/src/main/resources/xml/ns/public/common/common-policy-3.xsd @@ -192,7 +192,7 @@ - Relation(s) to which this constraint applies. All of these relation must match + Relation(s) to which this constraint applies. All of these relations must match the defined multiplicity. If no relation is present, org:default (i.e. null) is assumed. diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/PolicyRuleProcessor.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/PolicyRuleProcessor.java index f4e4344f629..a94e12e8793 100644 --- a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/PolicyRuleProcessor.java +++ b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/PolicyRuleProcessor.java @@ -308,59 +308,101 @@ private void checkAssigneeConstraints(LensContext conte private void checkAssigneeConstraints(LensContext context, EvaluatedAssignment assignment, PlusMinusZero plusMinus, OperationResult result) throws PolicyViolationException, SchemaException { PrismObject target = assignment.getTarget(); - if (target != null) { - Objectable targetType = target.asObjectable(); - if (targetType instanceof AbstractRoleType) { - Collection policyRules = assignment.getThisTargetPolicyRules(); - for (EvaluatedPolicyRule policyRule: policyRules) { - PolicyConstraintsType policyConstraints = policyRule.getPolicyConstraints(); - if (policyConstraints != null && (!policyConstraints.getMinAssignees().isEmpty() || !policyConstraints.getMaxAssignees().isEmpty())) { - String focusOid = null; - if (context.getFocusContext() != null) { - focusOid = context.getFocusContext().getOid(); - } - int numberOfAssigneesExceptMyself = countAssignees((PrismObject)target, focusOid, result); - if (plusMinus == PlusMinusZero.PLUS) { - numberOfAssigneesExceptMyself++; - } - for (MultiplicityPolicyConstraintType constraint: policyConstraints.getMinAssignees()) { - Integer multiplicity = XsdTypeMapper.multiplicityToInteger(constraint.getMultiplicity()); - // Complain only if the situation is getting worse - if (multiplicity >= 0 && numberOfAssigneesExceptMyself < multiplicity && plusMinus == PlusMinusZero.MINUS) { - EvaluatedPolicyRuleTrigger trigger = new EvaluatedPolicyRuleTrigger(PolicyConstraintKindType.MIN_ASSIGNEES, - constraint, ""+target+" requires at least "+multiplicity+ - " assignees. The operation would result in "+numberOfAssigneesExceptMyself+" assignees."); - assignment.triggerConstraint(policyRule, trigger); - - } - } - for (MultiplicityPolicyConstraintType constraint: policyConstraints.getMaxAssignees()) { - Integer multiplicity = XsdTypeMapper.multiplicityToInteger(constraint.getMultiplicity()); - // Complain only if the situation is getting worse - if (multiplicity >= 0 && numberOfAssigneesExceptMyself > multiplicity && plusMinus == PlusMinusZero.PLUS) { - EvaluatedPolicyRuleTrigger trigger = new EvaluatedPolicyRuleTrigger(PolicyConstraintKindType.MAX_ASSIGNEES, - constraint, ""+target+" requires at most "+multiplicity+ - " assignees. The operation would result in "+numberOfAssigneesExceptMyself+" assignees."); - assignment.triggerConstraint(policyRule, trigger); - - } - } - } + if (target == null || !(target.asObjectable() instanceof AbstractRoleType)) { + return; + } + AbstractRoleType targetRole = (AbstractRoleType) target.asObjectable(); + QName relation = ObjectTypeUtil.normalizeRelation(assignment.getRelation()); + Collection policyRules = assignment.getThisTargetPolicyRules(); + for (EvaluatedPolicyRule policyRule: policyRules) { + PolicyConstraintsType policyConstraints = policyRule.getPolicyConstraints(); + if (policyConstraints == null) { + continue; + } + List relevantMinAssignees = getForRelation(policyConstraints.getMinAssignees(), relation); + List relevantMaxAssignees = getForRelation(policyConstraints.getMaxAssignees(), relation); + if (relevantMinAssignees.isEmpty() && relevantMaxAssignees.isEmpty()) { + continue; + } + String focusOid = null; + if (context.getFocusContext() != null) { + focusOid = context.getFocusContext().getOid(); + } + int currentAssigneesExceptMyself = getNumberOfAssigneesExceptMyself(targetRole, focusOid, relation, result); + for (MultiplicityPolicyConstraintType constraint: relevantMinAssignees) { + Integer requiredMultiplicity = XsdTypeMapper.multiplicityToInteger(constraint.getMultiplicity()); + if (requiredMultiplicity <= 0) { + continue; // unbounded or 0 + } + // Complain only if the situation is getting worse + if (currentAssigneesExceptMyself < requiredMultiplicity && plusMinus == PlusMinusZero.MINUS) { + EvaluatedPolicyRuleTrigger trigger = + new EvaluatedPolicyRuleTrigger<>(PolicyConstraintKindType.MIN_ASSIGNEES, + constraint, target+" requires at least " + requiredMultiplicity + + " assignees with the relation of '" + relation.getLocalPart() + + "'. The operation would result in "+currentAssigneesExceptMyself+" assignees."); + assignment.triggerConstraint(policyRule, trigger); + } + } + for (MultiplicityPolicyConstraintType constraint: relevantMaxAssignees) { + Integer requiredMultiplicity = XsdTypeMapper.multiplicityToInteger(constraint.getMultiplicity()); + if (requiredMultiplicity < 0) { + continue; // unbounded + } + // Complain only if the situation is getting worse + if (currentAssigneesExceptMyself >= requiredMultiplicity && plusMinus == PlusMinusZero.PLUS) { + EvaluatedPolicyRuleTrigger trigger = + new EvaluatedPolicyRuleTrigger<>(PolicyConstraintKindType.MAX_ASSIGNEES, + constraint, target + " requires at most " + requiredMultiplicity + + " assignees with the relation of '" + relation.getLocalPart() + + "'. The operation would result in " + (currentAssigneesExceptMyself+1) + " assignees."); + assignment.triggerConstraint(policyRule, trigger); } } } } - private int countAssignees(PrismObject target, String selfOid, OperationResult result) throws SchemaException { + private List getForRelation(List all, QName relation) { + return all.stream() + .filter(c -> containsRelation(c, relation)) + .collect(Collectors.toList()); + } + + private boolean containsRelation(MultiplicityPolicyConstraintType constraint, QName relation) { + return getConstraintRelations(constraint).stream() + .anyMatch(constraintRelation -> ObjectTypeUtil.relationMatches(constraintRelation, relation)); + } + + private List getConstraintRelations(MultiplicityPolicyConstraintType constraint) { + return !constraint.getRelation().isEmpty() ? + constraint.getRelation() : + Collections.singletonList(SchemaConstants.ORG_DEFAULT); + } + + /** + * Returns numbers of assignees with the given relation name. + */ + private int getNumberOfAssigneesExceptMyself(AbstractRoleType target, String selfOid, QName relation, OperationResult result) + throws SchemaException { S_AtomicFilterExit q = QueryBuilder.queryFor(FocusType.class, prismContext) .item(FocusType.F_ASSIGNMENT, AssignmentType.F_TARGET_REF).ref(target.getOid()); if (selfOid != null) { q = q.and().not().id(selfOid); } ObjectQuery query = q.build(); - return repositoryService.countObjects(FocusType.class, query, result); + List> assignees = repositoryService.searchObjects(FocusType.class, query, null, result); + int count = 0; + assignee: for (PrismObject assignee : assignees) { + for (AssignmentType assignment : assignee.asObjectable().getAssignment()) { + if (assignment.getTargetRef() != null + && ObjectTypeUtil.relationsEquivalent(relation, assignment.getTargetRef().getRelation())) { + count++; + continue assignee; + } + } + } + return count; } - public boolean processPruning(LensContext context, DeltaSetTriple> evaluatedAssignmentTriple, diff --git a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/rbac/TestRbac.java b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/rbac/TestRbac.java index b38add2d05b..25098c990c4 100644 --- a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/rbac/TestRbac.java +++ b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/rbac/TestRbac.java @@ -29,6 +29,7 @@ import javax.xml.datatype.XMLGregorianCalendar; import javax.xml.namespace.QName; +import com.evolveum.midpoint.util.QNameUtil; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.annotation.DirtiesContext.ClassMode; import org.springframework.test.context.ContextConfiguration; @@ -41,7 +42,6 @@ import com.evolveum.midpoint.model.api.context.EvaluatedAssignmentTarget; import com.evolveum.midpoint.model.api.context.ModelContext; import com.evolveum.midpoint.model.intest.AbstractInitializedModelIntegrationTest; -import com.evolveum.midpoint.prism.Objectable; import com.evolveum.midpoint.prism.PrismContainer; import com.evolveum.midpoint.prism.PrismObject; import com.evolveum.midpoint.prism.PrismProperty; @@ -212,6 +212,8 @@ public void initSystem(Task initTask, OperationResult initResult) repoAddObjectFromFile(ROLE_META_FOOL_FILE, RoleType.class, initResult); repoAddObjectFromFile(ROLE_BLOODY_FOOL_FILE, RoleType.class, initResult); + repoAddObjectFromFile(USER_RAPP_FILE, initResult); + dummyResourceCtl.addGroup(GROUP_FOOLS_NAME); dummyResourceCtl.addGroup(GROUP_SIMPLETONS_NAME); @@ -1844,6 +1846,43 @@ public void test612JackAssignRoleGovernor() throws Exception { assertAssignees(ROLE_GOVERNOR_OID, 1); } + /** + * Governor has maxAssignees=0 for 'approver' + */ + @Test + public void test613JackAssignRoleGovernorAsApprover() throws Exception { + + if (!testMultiplicityConstraintsForNonDefaultRelations()) { + return; + } + + final String TEST_NAME = "test613JackAssignRoleGovernorAsApprover"; + TestUtil.displayTestTile(this, TEST_NAME); + assumeAssignmentPolicy(AssignmentPolicyEnforcementType.RELATIVE); + + Task task = taskManager.createTaskInstance(TestRbac.class.getName() + "." + TEST_NAME); + OperationResult result = task.getResult(); + + try { + // WHEN + assignRole(USER_JACK_OID, ROLE_GOVERNOR_OID, SchemaConstants.ORG_APPROVER, task, result); + + AssertJUnit.fail("Unexpected success"); + } catch (PolicyViolationException e) { + // this is expected + display("Expected exception", e); + } + + // THEN + TestUtil.displayThen(TEST_NAME); + result.computeStatus(); + TestUtil.assertFailure(result); + + assertNoAssignments(USER_JACK_OID); + + assertAssignees(ROLE_GOVERNOR_OID, 1); + } + /** * Role cannibal has minAssignees=2. It is assigned to nobody. Even though assigning * it to lemonhead would result in assignees=1 which violates the policy, the assignment @@ -2043,7 +2082,73 @@ public void test628RedskullUnassignRoleCanibal() throws Exception { assertAssignees(ROLE_CANNIBAL_OID, 2); assertAssignees(ROLE_GOVERNOR_OID, 1); } - + + @Test + public void test630RappAssignRoleCanibalAsOwner() throws Exception { + + if (!testMultiplicityConstraintsForNonDefaultRelations()) { + return; + } + + final String TEST_NAME = "test630RappAssignRoleCanibalAsOwner"; + TestUtil.displayTestTile(this, TEST_NAME); + assumeAssignmentPolicy(AssignmentPolicyEnforcementType.RELATIVE); + + Task task = taskManager.createTaskInstance(TestRbac.class.getName() + "." + TEST_NAME); + OperationResult result = task.getResult(); + + assertAssignees(ROLE_CANNIBAL_OID, 2); + + // WHEN + assignRole(USER_RAPP_OID, ROLE_CANNIBAL_OID, SchemaConstants.ORG_OWNER, task, result); + + // THEN + TestUtil.displayThen(TEST_NAME); + result.computeStatus(); + TestUtil.assertSuccess(result); + + assertAssignees(ROLE_CANNIBAL_OID, 2); + assertAssignees(ROLE_CANNIBAL_OID, SchemaConstants.ORG_OWNER, 1); + } + + @Test + public void test632RappUnassignRoleCanibalAsOwner() throws Exception { + + if (!testMultiplicityConstraintsForNonDefaultRelations()) { + return; + } + + final String TEST_NAME = "test632RappUnassignRoleCanibalAsOwner"; + TestUtil.displayTestTile(this, TEST_NAME); + assumeAssignmentPolicy(AssignmentPolicyEnforcementType.RELATIVE); + + Task task = taskManager.createTaskInstance(TestRbac.class.getName() + "." + TEST_NAME); + OperationResult result = task.getResult(); + + assertAssignees(ROLE_CANNIBAL_OID, 2); + assertAssignees(ROLE_CANNIBAL_OID, SchemaConstants.ORG_OWNER, 1); + + try { + // WHEN + TestUtil.displayWhen(TEST_NAME); + // null namespace to test no-namespace "approver" relation + unassignRole(USER_RAPP_OID, ROLE_CANNIBAL_OID, QNameUtil.nullNamespace(SchemaConstants.ORG_OWNER), task, result); + + AssertJUnit.fail("Unexpected success"); + } catch (PolicyViolationException e) { + // this is expected + display("Expected exception", e); + } + + // THEN + TestUtil.displayThen(TEST_NAME); + result.computeStatus(); + TestUtil.assertFailure(result); + + assertAssignees(ROLE_CANNIBAL_OID, 2); + assertAssignees(ROLE_CANNIBAL_OID, SchemaConstants.ORG_OWNER, 1); + } + @Test public void test649ElaineUnassignRoleGovernor() throws Exception { final String TEST_NAME = "test649ElaineUnassignRoleGovernor"; @@ -4110,4 +4215,7 @@ public void test857JackReconcile() throws Exception { } + protected boolean testMultiplicityConstraintsForNonDefaultRelations() { + return true; + } } diff --git a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/rbac/TestRbacDeprecated.java b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/rbac/TestRbacDeprecated.java index cc962aa144d..84ed9d97a3f 100644 --- a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/rbac/TestRbacDeprecated.java +++ b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/rbac/TestRbacDeprecated.java @@ -15,64 +15,11 @@ */ package com.evolveum.midpoint.model.intest.rbac; -import static org.testng.AssertJUnit.assertNotNull; -import static org.testng.AssertJUnit.assertNull; -import static com.evolveum.midpoint.test.IntegrationTestTools.display; -import static org.testng.AssertJUnit.assertEquals; -import static org.testng.AssertJUnit.assertTrue; - -import java.io.File; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; - -import javax.xml.namespace.QName; - -import com.evolveum.midpoint.prism.query.builder.QueryBuilder; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.annotation.DirtiesContext.ClassMode; import org.springframework.test.context.ContextConfiguration; -import org.testng.AssertJUnit; -import org.testng.annotations.Test; -import com.evolveum.icf.dummy.resource.DummyAccount; -import com.evolveum.midpoint.model.api.context.EvaluatedAssignmentTarget; -import com.evolveum.midpoint.model.api.context.EvaluatedAssignment; -import com.evolveum.midpoint.model.api.context.ModelContext; -import com.evolveum.midpoint.model.intest.AbstractInitializedModelIntegrationTest; -import com.evolveum.midpoint.prism.PrismContainer; -import com.evolveum.midpoint.prism.PrismObject; -import com.evolveum.midpoint.prism.PrismProperty; -import com.evolveum.midpoint.prism.PrismPropertyDefinition; -import com.evolveum.midpoint.prism.delta.DeltaSetTriple; -import com.evolveum.midpoint.prism.delta.ItemDelta; -import com.evolveum.midpoint.prism.delta.ObjectDelta; -import com.evolveum.midpoint.prism.path.IdItemPathSegment; -import com.evolveum.midpoint.prism.path.ItemPath; -import com.evolveum.midpoint.prism.path.NameItemPathSegment; -import com.evolveum.midpoint.prism.query.EqualFilter; -import com.evolveum.midpoint.prism.query.ObjectFilter; -import com.evolveum.midpoint.prism.query.ObjectQuery; -import com.evolveum.midpoint.prism.schema.PrismSchema; -import com.evolveum.midpoint.prism.util.PrismAsserts; -import com.evolveum.midpoint.prism.util.PrismTestUtil; -import com.evolveum.midpoint.schema.constants.SchemaConstants; -import com.evolveum.midpoint.schema.result.OperationResult; -import com.evolveum.midpoint.schema.util.MiscSchemaUtil; -import com.evolveum.midpoint.task.api.Task; -import com.evolveum.midpoint.test.DummyResourceContoller; -import com.evolveum.midpoint.test.IntegrationTestTools; -import com.evolveum.midpoint.test.util.TestUtil; -import com.evolveum.midpoint.util.DOMUtil; -import com.evolveum.midpoint.util.exception.PolicyViolationException; -import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentPolicyEnforcementType; -import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentType; -import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectReferenceType; -import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType; -import com.evolveum.midpoint.xml.ns._public.common.common_3.RoleType; -import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowType; -import com.evolveum.midpoint.xml.ns._public.common.common_3.UserType; -import com.evolveum.prism.xml.ns._public.types_3.EvaluationTimeType; +import java.io.File; /** * @author semancik @@ -89,8 +36,14 @@ public class TestRbacDeprecated extends TestRbac { protected File getRoleGovernorFile() { return ROLE_GOVERNOR_DEPRECATED_FILE; } - + + @Override protected File getRoleCannibalFile() { return ROLE_CANNIBAL_DEPRECATED_FILE; } + + @Override + protected boolean testMultiplicityConstraintsForNonDefaultRelations() { + return false; + } } diff --git a/model/model-intest/src/test/resources/rbac/role-cannibal.xml b/model/model-intest/src/test/resources/rbac/role-cannibal.xml index 6007852976e..e4a8ef3ade1 100644 --- a/model/model-intest/src/test/resources/rbac/role-cannibal.xml +++ b/model/model-intest/src/test/resources/rbac/role-cannibal.xml @@ -26,6 +26,10 @@ 2 + + 1 + owner + 3 diff --git a/model/model-intest/src/test/resources/rbac/role-governor.xml b/model/model-intest/src/test/resources/rbac/role-governor.xml index c8ebdbac29e..bf65f9ee761 100644 --- a/model/model-intest/src/test/resources/rbac/role-governor.xml +++ b/model/model-intest/src/test/resources/rbac/role-governor.xml @@ -21,7 +21,7 @@ Governor - min1 + max1 1 @@ -31,6 +31,20 @@ + + + + no-approvers + + + 0 + approver + + + + + + diff --git a/model/model-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java b/model/model-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java index e34e0397d10..b52d421c128 100644 --- a/model/model-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java +++ b/model/model-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java @@ -1201,18 +1201,29 @@ protected void unassignPrametricRole(String userOid, String roleOid, String orgO } protected void assertAssignees(String targetOid, int expectedAssignees) throws SchemaException { + assertAssignees(targetOid, SchemaConstants.ORG_DEFAULT, expectedAssignees); + } + + protected void assertAssignees(String targetOid, QName relation, int expectedAssignees) throws SchemaException { OperationResult result = new OperationResult(AbstractModelIntegrationTest.class.getName()+".assertAssignees"); - int count = countAssignees(targetOid, result); + int count = countAssignees(targetOid, relation, result); if (count != expectedAssignees) { SearchResultList> assignees = listAssignees(targetOid, result); - AssertJUnit.fail("Unexpected number of assignees of "+targetOid+", expected "+expectedAssignees+", but was " + count+ ": "+assignees); + AssertJUnit.fail("Unexpected number of assignees of "+targetOid+" as '"+relation+"', expected "+expectedAssignees+", but was " + count+ ": "+assignees); } } protected int countAssignees(String targetOid, OperationResult result) throws SchemaException { + return countAssignees(targetOid, SchemaConstants.ORG_DEFAULT, result); + } + + protected int countAssignees(String targetOid, QName relation, OperationResult result) throws SchemaException { + PrismReferenceValue refVal = new PrismReferenceValue(); + refVal.setOid(targetOid); + refVal.setRelation(relation); ObjectQuery query = QueryBuilder.queryFor(FocusType.class, prismContext) - .item(FocusType.F_ASSIGNMENT, AssignmentType.F_TARGET_REF).ref(targetOid) + .item(FocusType.F_ASSIGNMENT, AssignmentType.F_TARGET_REF).ref(refVal) .build(); return repositoryService.countObjects(FocusType.class, query, result); }