From 46b4d0de8e343abd0cf89b417d783678f39f5863 Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Wed, 15 Mar 2017 12:11:14 +0100 Subject: [PATCH] Certification: using owner/approver defined by ownerRef/approverRef fields, as well as by owner/approver relations. --- .../main/resources/xml/ns/public/query-3.xsd | 1 + .../impl/AccCertReviewersHelper.java | 50 +++++++++++++------ .../test/AbstractCertificationTest.java | 17 +++++-- .../test/CriticalRolesCertificationTest.java | 2 +- .../test/RoleInducementCertificationTest.java | 2 +- .../certification-of-critical-roles.xml | 4 +- ...ertification-of-eroot-user-assignments.xml | 4 +- .../certification-of-role-inducements.xml | 10 +++- .../test/resources/common/orgs-and-users.xml | 3 ++ .../src/test/resources/common/role-ceo.xml | 2 +- .../test/AbstractModelIntegrationTest.java | 4 ++ 11 files changed, 74 insertions(+), 25 deletions(-) diff --git a/infra/prism/src/main/resources/xml/ns/public/query-3.xsd b/infra/prism/src/main/resources/xml/ns/public/query-3.xsd index 77a64b4ea4a..360e1bc369f 100644 --- a/infra/prism/src/main/resources/xml/ns/public/query-3.xsd +++ b/infra/prism/src/main/resources/xml/ns/public/query-3.xsd @@ -403,6 +403,7 @@ + diff --git a/model/certification-impl/src/main/java/com/evolveum/midpoint/certification/impl/AccCertReviewersHelper.java b/model/certification-impl/src/main/java/com/evolveum/midpoint/certification/impl/AccCertReviewersHelper.java index 92db0563a88..920794abd44 100644 --- a/model/certification-impl/src/main/java/com/evolveum/midpoint/certification/impl/AccCertReviewersHelper.java +++ b/model/certification-impl/src/main/java/com/evolveum/midpoint/certification/impl/AccCertReviewersHelper.java @@ -22,10 +22,12 @@ import com.evolveum.midpoint.model.impl.expr.ModelExpressionThreadLocalHolder; import com.evolveum.midpoint.prism.PrismContext; import com.evolveum.midpoint.prism.PrismObject; +import com.evolveum.midpoint.prism.PrismReferenceValue; import com.evolveum.midpoint.prism.query.ObjectQuery; import com.evolveum.midpoint.prism.query.builder.QueryBuilder; import com.evolveum.midpoint.repo.api.RepositoryService; import com.evolveum.midpoint.schema.constants.ExpressionConstants; +import com.evolveum.midpoint.schema.constants.SchemaConstants; import com.evolveum.midpoint.schema.result.OperationResult; import com.evolveum.midpoint.schema.util.CertCampaignTypeUtil; import com.evolveum.midpoint.schema.util.ObjectTypeUtil; @@ -33,11 +35,15 @@ import com.evolveum.midpoint.task.api.Task; import com.evolveum.midpoint.util.QNameUtil; import com.evolveum.midpoint.util.exception.*; +import com.evolveum.midpoint.util.logging.Trace; +import com.evolveum.midpoint.util.logging.TraceManager; import com.evolveum.midpoint.xml.ns._public.common.common_3.*; +import org.apache.commons.collections4.CollectionUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Component; +import javax.xml.namespace.QName; import java.util.*; import java.util.stream.Collectors; @@ -47,6 +53,8 @@ @Component public class AccCertReviewersHelper { + private static final transient Trace LOGGER = TraceManager.getTrace(AccCertReviewersHelper.class); + @Autowired @Qualifier("cacheRepositoryService") private RepositoryService repositoryService; @@ -192,12 +200,7 @@ protected List getTargetObjectOwners(AccessCertificationCas } ObjectType target = resolveReference(_case.getTargetRef(), ObjectType.class, result); if (target instanceof AbstractRoleType) { - ObjectReferenceType ownerRef = ((AbstractRoleType) target).getOwnerRef(); - if (ownerRef != null) { - return Arrays.asList(ownerRef); - } else { - return null; - } + return getAssignees((AbstractRoleType) target, SchemaConstants.ORG_OWNER, task, result); } else if (target instanceof ResourceType) { return ResourceTypeUtil.getOwnerRef((ResourceType) target); } else { @@ -205,18 +208,35 @@ protected List getTargetObjectOwners(AccessCertificationCas } } - protected List getObjectOwners(AccessCertificationCaseType _case, Task task, OperationResult result) throws SchemaException, ObjectNotFoundException { + private List getAssignees(AbstractRoleType role, QName relation, Task task, OperationResult result) + throws SchemaException { + List rv = new ArrayList<>(); + if (SchemaConstants.ORG_OWNER.equals(relation)) { + CollectionUtils.addIgnoreNull(rv, role.getOwnerRef()); + } else if (SchemaConstants.ORG_APPROVER.equals(relation)) { + rv.addAll(role.getApproverRef()); + } else { + throw new AssertionError(relation); + } + // TODO in theory, we could look for approvers/owners of UserType, right? + PrismReferenceValue ref = new PrismReferenceValue(role.getOid()); + ref.setRelation(relation); + ObjectQuery query = QueryBuilder.queryFor(FocusType.class, prismContext) + .item(FocusType.F_ROLE_MEMBERSHIP_REF).ref(ref) + .build(); + List> assignees = repositoryService.searchObjects(FocusType.class, query, null, result); + LOGGER.trace("Looking for '{}' of {} using {}: found: {}", relation.getLocalPart(), role, query, assignees); + assignees.forEach(o -> rv.add(ObjectTypeUtil.createObjectRef(o))); + return rv; + } + + protected List getObjectOwners(AccessCertificationCaseType _case, Task task, OperationResult result) throws SchemaException, ObjectNotFoundException { if (_case.getObjectRef() == null) { return null; } ObjectType object = resolveReference(_case.getObjectRef(), ObjectType.class, result); if (object instanceof AbstractRoleType) { - ObjectReferenceType ownerRef = ((AbstractRoleType) object).getOwnerRef(); - if (ownerRef != null) { - return Arrays.asList(ownerRef); - } else { - return null; - } + return getAssignees((AbstractRoleType) object, SchemaConstants.ORG_OWNER, task, result); } else { return null; } @@ -228,7 +248,7 @@ private Collection getTargetObjectApprovers(AccessCertifica } ObjectType target = resolveReference(_case.getTargetRef(), ObjectType.class, result); if (target instanceof AbstractRoleType) { - return ((AbstractRoleType) target).getApproverRef(); + return getAssignees((AbstractRoleType) target, SchemaConstants.ORG_APPROVER, task, result); } else if (target instanceof ResourceType) { return ResourceTypeUtil.getApproverRef((ResourceType) target); } else { @@ -242,7 +262,7 @@ private Collection getObjectApprovers(AccessCertificationCa } ObjectType object = resolveReference(_case.getObjectRef(), ObjectType.class, result); if (object instanceof AbstractRoleType) { - return ((AbstractRoleType) object).getApproverRef(); + return getAssignees((AbstractRoleType) object, SchemaConstants.ORG_APPROVER, task, result); } else { return null; } diff --git a/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/AbstractCertificationTest.java b/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/AbstractCertificationTest.java index 4ac88b4e6e6..50718a69e12 100644 --- a/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/AbstractCertificationTest.java +++ b/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/AbstractCertificationTest.java @@ -117,6 +117,7 @@ public class AbstractCertificationTest extends AbstractModelIntegrationTest { protected static final String ORG_SECURITY_TEAM_OID = "e015eb10-1426-4104-86c0-eb0cf9dc423f"; public static final File ROLE_EROOT_USER_ASSIGNMENT_CAMPAIGN_OWNER_FILE = new File(COMMON_DIR, "role-eroot-user-assignment-campaign-owner.xml"); + protected static final String ROLE_EROOT_USER_ASSIGNMENT_CAMPAIGN_OWNER_OID = "00000000-d34d-b33f-f00d-ffffffff0001"; public static final File ROLE_SUPERUSER_FILE = new File(COMMON_DIR, "role-superuser.xml"); protected static final String ROLE_SUPERUSER_OID = "00000000-0000-0000-0000-000000000004"; @@ -234,6 +235,16 @@ public void initSystem(Task initTask, OperationResult initResult) throws Excepti resourceDummyBlackType = resourceDummyBlack.asObjectable(); dummyResourceCtlBlack.setResource(resourceDummyBlack); + // Recompute relevant objects + recomputeUser(USER_JACK_OID, initTask, initResult); + recomputeUser(USER_ELAINE_OID, initTask, initResult); + recomputeUser(USER_GUYBRUSH_OID, initTask, initResult); + recomputeFocus(RoleType.class, ROLE_CEO_OID, initTask, initResult); + recomputeFocus(RoleType.class, ROLE_COO_OID, initTask, initResult); + recomputeFocus(RoleType.class, ROLE_CTO_OID, initTask, initResult); + recomputeFocus(RoleType.class, ROLE_REVIEWER_OID, initTask, initResult); + recomputeFocus(RoleType.class, ROLE_EROOT_USER_ASSIGNMENT_CAMPAIGN_OWNER_OID, initTask, initResult); + recomputeFocus(OrgType.class, ORG_SECURITY_TEAM_OID, initTask, initResult); } protected AccessCertificationCaseType checkCase(Collection caseList, String subjectOid, String targetOid, FocusType focus, String campaignOid) { @@ -360,13 +371,13 @@ protected void assertDefinitionAndOwner(AccessCertificationCampaignType campaign protected void assertCaseReviewers(AccessCertificationCaseType _case, AccessCertificationResponseType currentStageOutcome, int currentStage, List reviewerOidList) { - assertEquals("wrong current stage outcome", currentStageOutcome, _case.getCurrentStageOutcome()); - assertEquals("wrong current stage number", currentStage, _case.getCurrentStageNumber()); + assertEquals("wrong current stage outcome for "+_case, currentStageOutcome, _case.getCurrentStageOutcome()); + assertEquals("wrong current stage number for "+_case, currentStage, _case.getCurrentStageNumber()); Set realReviewerOids = new HashSet<>(); for (ObjectReferenceType ref : _case.getCurrentReviewerRef()) { realReviewerOids.add(ref.getOid()); } - assertEquals("wrong reviewer oids", new HashSet<>(reviewerOidList), realReviewerOids); + assertEquals("wrong reviewer oids for "+_case, new HashSet<>(reviewerOidList), realReviewerOids); } protected void recordDecision(String campaignOid, AccessCertificationCaseType _case, AccessCertificationResponseType response, String comment, diff --git a/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/CriticalRolesCertificationTest.java b/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/CriticalRolesCertificationTest.java index 6af264c665d..ceeb6ad80a6 100644 --- a/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/CriticalRolesCertificationTest.java +++ b/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/CriticalRolesCertificationTest.java @@ -964,7 +964,7 @@ public void test900StartRemediation() throws Exception { userElaine = getUser(USER_ELAINE_OID).asObjectable(); display("userElaine", userElaine); - assertEquals("wrong # of userElaine's assignments", 4, userElaine.getAssignment().size()); + assertEquals("wrong # of userElaine's assignments", 5, userElaine.getAssignment().size()); userGuybrush = getUser(USER_GUYBRUSH_OID).asObjectable(); display("userGuybrush", userGuybrush); diff --git a/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/RoleInducementCertificationTest.java b/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/RoleInducementCertificationTest.java index d8e446ffcb1..64e101f2531 100644 --- a/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/RoleInducementCertificationTest.java +++ b/model/certification-impl/src/test/java/com/evolveum/midpoint/certification/test/RoleInducementCertificationTest.java @@ -168,7 +168,7 @@ public void test020OpenFirstStage() throws Exception { assertCaseOutcome(caseList, ROLE_COO_OID, RESOURCE_DUMMY_BLACK_OID, ACCEPT, ACCEPT, null); assertCaseOutcome(caseList, ROLE_COO_OID, ROLE_SUPERUSER_OID, ACCEPT, ACCEPT, null); assertCaseOutcome(caseList, ROLE_SUPERUSER_OID, RESOURCE_DUMMY_OID, ACCEPT, ACCEPT, null); - assertPercentComplete(campaign, 20, 100, 0); // preliminary outcomes for all aases are "ACCEPT" + assertPercentComplete(campaign, 20, 100, 0); // preliminary outcomes for all cases are "ACCEPT" } protected void checkAllCases(Collection caseList, String campaignOid) { diff --git a/model/certification-impl/src/test/resources/common/certification-of-critical-roles.xml b/model/certification-impl/src/test/resources/common/certification-of-critical-roles.xml index e99729a6408..542424cf4d6 100644 --- a/model/certification-impl/src/test/resources/common/certification-of-critical-roles.xml +++ b/model/certification-impl/src/test/resources/common/certification-of-critical-roles.xml @@ -55,7 +55,9 @@ jack->CTO none (A) -> A none (A) -> A | A diff --git a/model/certification-impl/src/test/resources/common/certification-of-eroot-user-assignments.xml b/model/certification-impl/src/test/resources/common/certification-of-eroot-user-assignments.xml index 8ef4f9d9a6e..a239692cfbd 100644 --- a/model/certification-impl/src/test/resources/common/certification-of-eroot-user-assignments.xml +++ b/model/certification-impl/src/test/resources/common/certification-of-eroot-user-assignments.xml @@ -27,8 +27,8 @@ UserType - parentOrgRef - + + 00000000-8888-6666-0000-300000000000 SUBTREE diff --git a/model/certification-impl/src/test/resources/common/certification-of-role-inducements.xml b/model/certification-impl/src/test/resources/common/certification-of-role-inducements.xml index 6bbab98f2a3..1aaa56d7276 100644 --- a/model/certification-impl/src/test/resources/common/certification-of-role-inducements.xml +++ b/model/certification-impl/src/test/resources/common/certification-of-role-inducements.xml @@ -81,8 +81,16 @@ Superuser-Dummy: - -> A jack:A,administrator:nul diff --git a/model/certification-impl/src/test/resources/common/orgs-and-users.xml b/model/certification-impl/src/test/resources/common/orgs-and-users.xml index 708b507836f..af04fd896eb 100644 --- a/model/certification-impl/src/test/resources/common/orgs-and-users.xml +++ b/model/certification-impl/src/test/resources/common/orgs-and-users.xml @@ -130,6 +130,9 @@ + + + diff --git a/model/certification-impl/src/test/resources/common/role-ceo.xml b/model/certification-impl/src/test/resources/common/role-ceo.xml index 07605a7b1f2..130c07ab865 100644 --- a/model/certification-impl/src/test/resources/common/role-ceo.xml +++ b/model/certification-impl/src/test/resources/common/role-ceo.xml @@ -25,7 +25,7 @@ critical - + 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 7d6ac6cfba6..c97e9d2a890 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 @@ -823,6 +823,10 @@ protected void recomputeUser(String userOid, Task task, OperationResult result) modelService.recompute(UserType.class, userOid, null, task, result); } + protected void recomputeFocus(Class clazz, String userOid, Task task, OperationResult result) throws SchemaException, PolicyViolationException, ExpressionEvaluationException, ObjectNotFoundException, ObjectAlreadyExistsException, CommunicationException, ConfigurationException, SecurityViolationException { + modelService.recompute(clazz, userOid, null, task, result); + } + protected void recomputeUser(String userOid, ModelExecuteOptions options, Task task, OperationResult result) throws SchemaException, PolicyViolationException, ExpressionEvaluationException, ObjectNotFoundException, ObjectAlreadyExistsException, CommunicationException, ConfigurationException, SecurityViolationException { modelService.recompute(UserType.class, userOid, options, task, result); }