Skip to content

Commit

Permalink
Make archetypeRef selector consider assignments
Browse files Browse the repository at this point in the history
The 'archetypeRef' selector traditionally matched the (effective)
'archetypeRef' object property. However, this does not work well
e.g. with the current GUI implementation for adding new objects,
where only the assignments are present. So this commit changes that.
See also the reasoning in the ArchetypeRefClause code.

(Note that this change involves only matching existing values,
not creating the filters.)

Related to MID-9268.
  • Loading branch information
mederly committed Nov 30, 2023
1 parent ea591fa commit d3a84ac
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@

package com.evolveum.midpoint.schema.selector.spec;

import static com.evolveum.midpoint.schema.util.ObjectTypeUtil.getOidsFromRefs;
import static com.evolveum.midpoint.util.MiscUtil.configNonNull;

import java.util.*;

import com.evolveum.midpoint.schema.selector.eval.FilteringContext;
import com.evolveum.midpoint.schema.selector.eval.MatchingContext;
import com.evolveum.midpoint.schema.util.ObjectTypeUtil;
import com.evolveum.midpoint.util.DebugUtil;

import com.google.common.base.Preconditions;
Expand All @@ -26,6 +26,10 @@
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentHolderType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectReferenceType;

/**
* Processes the `archetypeRef` clause. For checking assignments vs effective archetype references, please see
* javadoc for {@link #matches(PrismValue, MatchingContext)} method.
*/
public class ArchetypeRefClause extends SelectorClause {

/** Not empty, no null values. Immutable. */
Expand All @@ -52,13 +56,26 @@ static ArchetypeRefClause of(@NotNull List<ObjectReferenceType> archetypeRefList
return "archetypeRef";
}

/**
* Determines whether the archetype(s) do match the selector.
*
* We intentionally check assignments instead of "effective" archetype OIDs here. The reasons are:
*
* 1. When GUI creates an object, it sets the assignment, not the archetypeRef.
* 2. Deltas that change the archetype refer to the assignment, not the archetypeRef.
* (This is important when determining whether we are leaving the zone of control.)
*
* The #1 could be worked around by setting the archetypeRef before autz are checked. However, the #2 is more serious.
*
* To be seen if this is the correct way forward. An alternative would be to compute the effective object content
* (archetypeRef, but also e.g. parentOrgRef) before the actual matching. However, this could be quite complex and
* expensive. So, currently we have no option other than matching the assignments values.
*/
@Override
public boolean matches(@NotNull PrismValue value, @NotNull MatchingContext ctx) {
Object realValue = value.getRealValueIfExists();
if (realValue instanceof AssignmentHolderType) {
Collection<String> actualArchetypeOids =
getOidsFromRefs(
((AssignmentHolderType) realValue).getArchetypeRef());
if (realValue instanceof AssignmentHolderType assignmentHolder) {
Collection<String> actualArchetypeOids = ObjectTypeUtil.getAssignedArchetypeOids(assignmentHolder);
for (String actualArchetypeOid : actualArchetypeOids) {
if (archetypeOids.contains(actualArchetypeOid)) {
return true;
Expand All @@ -75,6 +92,13 @@ public boolean matches(@NotNull PrismValue value, @NotNull MatchingContext ctx)
return false;
}

/**
* Currently, we act upon the effective archetypeRef value, not the value in assignments. It should be far more efficient
* when running against the repository. Moreover, in the repository, it should be equivalent, as the effective value
* should precisely reflect the assignments' values there.
*
* @see #matches(PrismValue, MatchingContext)
*/
@Override
public boolean toFilter(@NotNull FilteringContext ctx) throws SchemaException {
addConjunct(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -522,10 +522,8 @@ public <H extends AssignmentHolderType, R extends AbstractRoleType> RoleSelectio
result.recordFatalError(e);
throw e;
}
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Security constrains for getAssignableRoleSpecification on {}:\n{}",
focus, securityConstraints.debugDump(1));
}
LOGGER.trace("Security constrains for getAssignableRoleSpecification on {}:\n{}",
focus, securityConstraints.debugDumpLazily(1));

// Global decisions: processing #modify authorizations: allow/deny for all items or allow/deny for assignment/inducement item.
ItemPath assignmentPath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ public abstract class AbstractInitializedSecurityTest extends AbstractInitialize

public static final File TEST_DIR = new File("src/test/resources/security");

protected static final TestObject<RoleType> ARCHETYPE_BUSINESS_ROLE = TestObject.file(TEST_DIR, "archetype-business-role.xml", "00000000-0000-0000-0000-000000000321");
protected static final TestObject<RoleType> ARCHETYPE_APPLICATION_ROLE = TestObject.file(TEST_DIR, "archetype-application-role.xml", "32073084-65d0-11e9-baff-bbb479bb05b7");
protected static final TestObject<ArchetypeType> ARCHETYPE_BUSINESS_ROLE = TestObject.file(TEST_DIR, "archetype-business-role.xml", "00000000-0000-0000-0000-000000000321");
protected static final TestObject<ArchetypeType> ARCHETYPE_APPLICATION_ROLE = TestObject.file(TEST_DIR, "archetype-application-role.xml", "32073084-65d0-11e9-baff-bbb479bb05b7");
protected static final TestObject<UserType> USER_LECHUCK = TestObject.file(TEST_DIR, "user-lechuck.xml", "c0c010c0-d34d-b33f-f00d-1c1c11cc11c2");

// Persona of LeChuck
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import static com.evolveum.midpoint.schema.constants.SchemaConstants.RI_ACCOUNT_OBJECT_CLASS;

import static org.assertj.core.api.Assertions.assertThat;
import static org.testng.AssertJUnit.*;

import java.util.ArrayList;
Expand All @@ -20,7 +21,8 @@
import com.evolveum.midpoint.schema.*;
import com.evolveum.midpoint.schema.processor.ResourceObjectDefinition;

import org.assertj.core.api.Assertions;
import com.evolveum.midpoint.test.TestObject;

import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.annotation.DirtiesContext.ClassMode;
import org.springframework.test.context.ContextConfiguration;
Expand Down Expand Up @@ -609,6 +611,10 @@ public void test207rAutzJackObjectFilterCaribbeanRole() throws Exception {
@Test
public void test208AutzJackReadSomeRoles() throws Exception {
testAutzJackReadSomeRoles(ROLE_READ_SOME_ROLES.oid);

assertNewRoleGetAllow(ARCHETYPE_BUSINESS_ROLE);
assertNewRoleGetAllow(ARCHETYPE_APPLICATION_ROLE);
assertNewRoleGetDeny(ARCHETYPE_PERSONA_ROLE);
}

/**
Expand Down Expand Up @@ -653,6 +659,24 @@ private void testAutzJackReadSomeRoles(String roleOid) throws Exception {
assertGlobalStateUntouched();
}

private void assertNewRoleGetAllow(TestObject<ArchetypeType> archetype) throws CommonException {
assertNewRoleGet(archetype, true);
}

@SuppressWarnings("SameParameterValue")
private void assertNewRoleGetDeny(TestObject<ArchetypeType> archetype) throws CommonException {
assertNewRoleGet(archetype, false);
}

private void assertNewRoleGet(TestObject<ArchetypeType> archetype, boolean expected) throws CommonException {
var role = new RoleType()
.assignment(archetype.assignmentTo())
.asPrismObject();
PrismObjectDefinition<RoleType> def = getEditObjectDefinition(role);
var canRead = def.findItemDefinition(RoleType.F_NAME).canRead();
assertThat(canRead).as("canRead role name for " + archetype).isEqualTo(expected);
}

/**
* MID-5002
*/
Expand Down Expand Up @@ -3151,8 +3175,8 @@ public void test380AutzJackSelfTaskOwner() throws Exception {

assertGetDeny(TaskType.class, TASK_USELESS_ADMINISTRATOR.oid);
var task = assertGetAllow(TaskType.class, TASK_USELESS_JACK.oid).asObjectable();
Assertions.assertThat(task.getActivity()).as("activity in the task").isNotNull();
Assertions.assertThat(task.getActivityState()).as("activity state in the task, denied by the autz").isNull();
assertThat(task.getActivity()).as("activity in the task").isNotNull();
assertThat(task.getActivityState()).as("activity state in the task, denied by the autz").isNull();

assertOwnTaskEditSchema(task.asPrismObject());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
-->
<!-- MID-5581 -->
<role oid="5549cb8e-d573-11e9-a61e-7f2eff22715a"
xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
xmlns:c="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
xmlns:t="http://prism.evolveum.com/xml/ns/public/types-3"
xmlns:q="http://prism.evolveum.com/xml/ns/public/query-3">
xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3">
<name>Employee manager</name>
<authorization>
<name>read-some-roles</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
-->
<!-- MID-3647 -->
<role oid="7b4a3880-e167-11e6-b38b-2b6a550a03e7"
xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
xmlns:c="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
xmlns:t="http://prism.evolveum.com/xml/ns/public/types-3"
xmlns:q="http://prism.evolveum.com/xml/ns/public/query-3">
xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3">
<name>Read some roles</name>
<description>Archetype-based method</description>
<authorization>
Expand Down

0 comments on commit d3a84ac

Please sign in to comment.