Skip to content

Commit

Permalink
Fix MID-4450: Role exclusion ignores relations
Browse files Browse the repository at this point in the history
The exclusion constraint ignored relations altogether, triggering
even if one of conflicting roles was assigned e.g. as approver.
To resolve this, order constraints on the exclusion
are introduced (although only partially supported now).

!!! Fixed a bug in orderConstraint processing. Missing value of
relation was interpreted as "default", although it should be correctly
interpreted as "any". Deployments should be adapted. !!!
  • Loading branch information
mederly committed Feb 20, 2018
1 parent 1e22a1f commit 4ab23f3
Show file tree
Hide file tree
Showing 17 changed files with 543 additions and 41 deletions.
Expand Up @@ -615,6 +615,7 @@ public static <T> T getExtensionItemRealValue(@Nullable ExtensionType extension,
return item != null ? (T) item.getRealValue() : null;
}

@NotNull
public static QName normalizeRelation(QName name) {
if (name == null) {
return SchemaConstants.ORG_DEFAULT;
Expand Down
Expand Up @@ -3825,39 +3825,39 @@
</xsd:appinfo>
</xsd:annotation>
<xsd:sequence>
<xsd:element name="order" type="xsd:int" minOccurs="0" maxOccurs="1" default="1">
<xsd:element name="order" type="xsd:int" minOccurs="0" default="1">
<xsd:annotation>
<xsd:documentation>
Exact order to match. This is a short-hand for setting both
orderMin and orderMax to the same value.
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="orderMin" type="xsd:string" minOccurs="0" maxOccurs="1">
<xsd:element name="orderMin" type="xsd:string" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Minimum matching order. Applicable only if "order" element is not set.
Numeric value or string "unbounded".
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="orderMax" type="xsd:string" minOccurs="0" maxOccurs="1">
<xsd:element name="orderMax" type="xsd:string" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Maximum matching order. Applicable only if "order" element is not set.
Numeric value or string "unbounded".
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="resetOrder" type="xsd:int" minOccurs="0" maxOccurs="1">
<xsd:element name="resetOrder" type="xsd:int" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
The new value for order for this relation (or summary order), to be used when
evaluating subsequent inducements.
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="relation" type="xsd:QName" minOccurs="0" maxOccurs="1">
<xsd:element name="relation" type="xsd:QName" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Relation to which the order constraints apply. If none present, summary (i.e. non-delegation) order is considered.
Expand Down
Expand Up @@ -638,7 +638,39 @@
</xsd:appinfo>
</xsd:annotation>
</xsd:element>
<xsd:element name="policy" type="tns:ExclusionPolicyType" minOccurs="0">
<xsd:element name="orderConstraint" type="tns:OrderConstraintsType" minOccurs="0" maxOccurs="unbounded">
<xsd:annotation>
<xsd:documentation>
Specification of relation(s) when this exclusion constraint should be applied at the source side.
(I.e. this is evaluated against the object defining this exclusion constraint.)
The default is "order = 1".

EXPERIMENTAL. Currently it does not work with non-member relations because of assignment
evaluation optimizations (see TestSegregationOfDuties.test950JackSelfExclusion). So it can
be used only for default and manager relations.
</xsd:documentation>
<xsd:appinfo>
<a:since>3.7.1</a:since>
</xsd:appinfo>
</xsd:annotation>
</xsd:element>
<xsd:element name="targetOrderConstraint" type="tns:OrderConstraintsType" minOccurs="0" maxOccurs="unbounded">
<xsd:annotation>
<xsd:documentation>
Specification of relation(s) when this exclusion constraint should be applied at the target side.
(I.e. this is evaluated against the target object.)
The default is "order = 1".

EXPERIMENTAL. Currently it does not work with non-member relations because of assignment
evaluation optimizations (see TestSegregationOfDuties.test950JackSelfExclusion). So it can
be used only for default and manager relations.
</xsd:documentation>
<xsd:appinfo>
<a:since>3.7.1</a:since>
</xsd:appinfo>
</xsd:annotation>
</xsd:element>
<xsd:element name="policy" type="tns:ExclusionPolicyType" minOccurs="0">
<xsd:annotation>
<xsd:appinfo>
<a:deprecated>true</a:deprecated>
Expand Down
Expand Up @@ -26,6 +26,7 @@
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentPathType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ExtensionType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.OrderConstraintsType;
import org.jetbrains.annotations.NotNull;

import java.util.List;
Expand Down Expand Up @@ -89,6 +90,13 @@ public interface AssignmentPath extends DebugDumpable, ShortDumpable {
*/
ObjectType getProtoRole();

/**
* Shallow clone.
*/
AssignmentPath clone();

AssignmentPath cloneFirst(int n);

AssignmentPathType toAssignmentPathType(boolean includeAssignmentsContent);

ExtensionType collectExtensions(int startAt) throws SchemaException;
Expand All @@ -104,4 +112,17 @@ default AssignmentPathSegment getAt(int index) {
return getSegment(index);
}

/**
* Returns true if the path matches specified order constraints. All of them must match.
* Although there are some defaults, it is recommended to specify constraints explicitly.
* Currently not supported on empty paths.
*
* Not all parts of OrderConstraintsType are supported. Namely, resetOrder item has no meaning here.
*/
boolean matches(@NotNull List<OrderConstraintsType> orderConstraints);

/**
* Preliminary (limited) implementation. To be used to compare paths pointing to the same target object. Use with care.
*/
boolean equivalent(AssignmentPath other);
}
Expand Up @@ -23,8 +23,11 @@
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentPathSegmentType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.OrderConstraintsType;
import org.jetbrains.annotations.NotNull;

import java.util.List;

/**
* Single assignment in an assignment path. In addition to the AssignmentType, it contains resolved target:
* full object, resolved from targetRef. If targetRef resolves to multiple objects, in the path segment
Expand Down Expand Up @@ -77,4 +80,13 @@ public interface AssignmentPathSegment extends DebugDumpable, ShortDumpable {

@NotNull
AssignmentPathSegmentType toAssignmentPathSegmentType(boolean includeAssignmentsContent);

/**
* Returns true if the path segment matches specified order constraints. All of them must match.
* Although there are some defaults, it is recommended to specify constraints explicitly.
*/
boolean matches(@NotNull List<OrderConstraintsType> orderConstraints);

// Preliminary limited implementation. Use with care.
boolean equivalent(AssignmentPathSegment otherSegment);
}
Expand Up @@ -30,6 +30,7 @@
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentPathType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ExtensionType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.OrderConstraintsType;
import org.jetbrains.annotations.NotNull;

/**
Expand Down Expand Up @@ -149,12 +150,15 @@ public ObjectType getProtoRole() {
return protoRole;
}

/**
* Shallow clone.
*/
@Override
public AssignmentPathImpl clone() {
return cloneFirst(size());
}

@Override
public AssignmentPathImpl cloneFirst(int n) {
AssignmentPathImpl clone = new AssignmentPathImpl(prismContext);
clone.segments.addAll(this.segments);
clone.segments.addAll(this.segments.subList(0, n));
return clone;
}

Expand Down Expand Up @@ -246,4 +250,28 @@ public PrismContext getPrismContext() {
public ExtensionType collectExtensions(int startAt) throws SchemaException {
return AssignmentPathUtil.collectExtensions(this, startAt, prismContext);
}

@Override
public boolean matches(@NotNull List<OrderConstraintsType> orderConstraints) {
if (isEmpty()) {
throw new UnsupportedOperationException("Checking order constraints on empty assignment path is not currently supported.");
} else {
return last().matches(orderConstraints);
}
}

@Override
public boolean equivalent(AssignmentPath other) {
if (size() != other.size()) {
return false;
}
for (int i = 0; i < segments.size(); i++) {
AssignmentPathSegment segment = segments.get(i);
AssignmentPathSegment otherSegment = other.getSegments().get(i);
if (!segment.equivalent(otherSegment)) {
return false;
}
}
return true;
}
}
Expand Up @@ -631,4 +631,25 @@ public Integer getLastEqualOrderSegmentIndex() {
public void setLastEqualOrderSegmentIndex(Integer lastEqualOrderSegmentIndex) {
this.lastEqualOrderSegmentIndex = lastEqualOrderSegmentIndex;
}

@Override
public boolean matches(@NotNull List<OrderConstraintsType> orderConstraints) {
return computeMatchingOrder(evaluationOrder, null, orderConstraints);
}

// preliminary implementation; use only to compare segments in paths (pointing to the same target OID)
// that are to be checked for equivalency
@Override
public boolean equivalent(AssignmentPathSegment otherSegment) {
if (!ObjectTypeUtil.relationsEquivalent(relation, otherSegment.getRelation())) {
return false;
}
if (target == null && otherSegment.getTarget() == null) {
return true; // TODO reconsider this in general case
}
if (target == null || otherSegment.getTarget() == null) {
return false;
}
return java.util.Objects.equals(target.getOid(), otherSegment.getTarget().getOid());
}
}
Expand Up @@ -77,6 +77,8 @@ public class EvaluatedPolicyRuleImpl implements EvaluatedPolicyRule {
* This is what assignmentPath (Engineer->Employee->(maybe some metarole)->rule) and directOwner (Employee) are for.
*
* For global policy rules, assignmentPath is the path to the target object that matched global policy rule.
*
* TODO When it can be null?
*/
@Nullable private final AssignmentPath assignmentPath;
@Nullable private final ObjectType directOwner;
Expand Down
Expand Up @@ -107,13 +107,16 @@ public EvaluationOrder decrease(MultiSet<QName> relations) {

// must always be private: public interface will not allow to modify object state!
private void advanceThis(QName relation, int amount) {
relation = ObjectTypeUtil.normalizeRelation(relation);
orderMap.put(relation, getMatchingRelationOrder(relation) + amount);
@NotNull QName normalizedRelation = ObjectTypeUtil.normalizeRelation(relation);
orderMap.put(normalizedRelation, getMatchingRelationOrder(normalizedRelation) + amount);
}

@Override
public int getMatchingRelationOrder(QName relation) {
checkConsistence();
if (relation == null) {
return getSummaryOrder();
}
return orderMap.getOrDefault(ObjectTypeUtil.normalizeRelation(relation), 0);
}

Expand All @@ -133,6 +136,7 @@ public Map<QName, Integer> diff(EvaluationOrder newState) {
@SuppressWarnings({"unchecked", "raw"})
Collection<QName> relations = CollectionUtils.union(getRelations(), newState.getRelations());
Map<QName, Integer> rv = new HashMap<>();
// relation is not null below
relations.forEach(relation -> rv.put(relation, newState.getMatchingRelationOrder(relation) - getMatchingRelationOrder(relation)));
return rv;
}
Expand Down

0 comments on commit 4ab23f3

Please sign in to comment.