Skip to content

Commit

Permalink
Improve authorization/selector code
Browse files Browse the repository at this point in the history
This commit provides various code improvements:

- improved configuration error reporting for authorizations;
- added some javadocs;
- migrated RepositoryService#selectorMatches to new SelectorMatcher;
- and a couple of other ones.
  • Loading branch information
mederly committed Jun 13, 2023
1 parent c69e96a commit 1efdb52
Show file tree
Hide file tree
Showing 30 changed files with 356 additions and 155 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
* or "tested"; the runtime code could report these errors in a simpler, less user-friendly way. This would reduce the overhead
* of carrying the whole operation context. On the other hand, configuration objects can be modified unexpectedly, e.g. by
* executing arbitrary deltas at any given time. This would need to be limited somehow.
*
* TODO consider using getPath method to provide exact path of the element in the object
*/
@Experimental
public class ConfigErrorReporter {
Expand All @@ -48,9 +50,10 @@ public String toString() {
};
}

//region Mappings and attribute definitions
public static String describe(@NotNull InboundMappingType inboundMapping) {
PrismContainerValue<?> pcv = inboundMapping.asPrismContainerValue();
var itemDef = PrismValueUtil.getNearestValueOfType(pcv, ResourceItemDefinitionType.class);
var itemDef = pcv.getNearestValueOfType(ResourceItemDefinitionType.class);
if (itemDef != null) {
return describeLocally(inboundMapping) + " in " + describe(itemDef);
} else {
Expand All @@ -65,7 +68,7 @@ private static String describeLocally(@NotNull InboundMappingType mapping) {

public static String describe(@NotNull ItemRefinedDefinitionType itemRefinedDefinition) {
PrismContainerValue<?> pcv = itemRefinedDefinition.asPrismContainerValue();
Objectable top = PrismValueUtil.getRootObject(pcv);
Objectable top = pcv.getRootObjectable();
if (top instanceof ResourceType) {
return String.format(
"%s definition with ID %d%s in %s",
Expand All @@ -89,8 +92,8 @@ private static String getItemDefinitionDescription(@NotNull ItemRefinedDefinitio
}
}

private static String getClassOrTypeDescription(PrismValue value) {
var typeDef = PrismValueUtil.getNearestValueOfType(value, ResourceObjectTypeDefinitionType.class);
private static String getClassOrTypeDescription(@NotNull PrismValue value) {
var typeDef = value.getNearestValueOfType(ResourceObjectTypeDefinitionType.class);
if (typeDef == null) {
return "";
}
Expand All @@ -107,4 +110,54 @@ private static String getClassOrTypeDescription(PrismValue value) {
return "";
}
}
//endregion

//region Authorizations and selectors
public static String describe(@NotNull AuthorizationType authorization) {
StringBuilder sb = new StringBuilder();
sb.append(describeLocally(authorization));
var top = authorization.asPrismContainerValue().getRootObjectable();
if (top != null) {
sb.append(" in ").append(top);
}
return sb.toString();
}

private static String describeLocally(@NotNull AuthorizationType authorization) {
return genericLocalDescription(
"authorization", authorization.getName(), authorization.getId());
}

public static String describe(@NotNull ObjectSelectorType selector) {
StringBuilder sb = new StringBuilder();
sb.append(describeLocally(selector));
var autz = selector.asPrismContainerValue().getNearestValueOfType(AuthorizationType.class);
if (autz != null) {
// This is a typical situation
sb.append(" in ").append(describe(autz));
} else {
var top = selector.asPrismContainerValue().getRootObjectable();
if (top != null) {
sb.append(" in ").append(top);
}
}
return sb.toString();
}

private static String describeLocally(@NotNull ObjectSelectorType selector) {
return genericLocalDescription("selector", selector.getName(), selector.getId());
}
//endregion

private static String genericLocalDescription(String item, String name, Long id) {
StringBuilder sb = new StringBuilder();
sb.append(item);
if (name != null) {
sb.append(" '").append(name).append('\'');
}
if (id != null) {
sb.append(" (#").append(id).append(')');
}
return sb.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,21 @@

import com.evolveum.midpoint.schema.selector.eval.SubjectedEvaluationContext.DelegatorSelection;

import com.evolveum.midpoint.util.annotation.Experimental;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import com.evolveum.midpoint.prism.query.ObjectFilter;
import com.evolveum.midpoint.schema.selector.spec.SelectorClause;
import com.evolveum.midpoint.schema.selector.spec.ValueSelector;

/**
* Keeps everything needed to produce a filter from given clause.
*
* Most probably will be simplified in the future.
*/
@Experimental
public class ClauseFilteringContext extends ClauseMatchingContext {

/** The object/value type we are searching for. */
Expand All @@ -42,7 +50,7 @@ public ClauseFilteringContext(
@Nullable ClauseApplicabilityPredicate clauseApplicabilityPredicate,
@NotNull FilterCollector filterCollector,
@Nullable ObjectFilterExpressionEvaluator filterEvaluator,
@NotNull MatchingTracer tracer,
@NotNull SelectorProcessingTracer tracer,
@NotNull OrgTreeEvaluator orgTreeEvaluator,
@Nullable SubjectedEvaluationContext subjectedEvaluationContext,
@Nullable OwnerResolver ownerResolver,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.evolveum.midpoint.schema.selector.eval.SubjectedEvaluationContext.DelegatorSelection;
import com.evolveum.midpoint.schema.selector.spec.SelectorClause;
import com.evolveum.midpoint.schema.selector.spec.ValueSelector;
import com.evolveum.midpoint.util.annotation.Experimental;
import com.evolveum.midpoint.xml.ns._public.common.common_3.FocusType;

import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectReferenceType;
Expand All @@ -24,11 +25,16 @@
import java.util.List;
import java.util.Set;

/** Keeps everything needed to evaluate whether a clause matches given value. */
/**
* Keeps everything needed to evaluate whether a clause matches given value.
*
* Most probably will be simplified in the future.
*/
@Experimental
public class ClauseMatchingContext {

@Nullable public final ObjectFilterExpressionEvaluator filterEvaluator;
@NotNull public final MatchingTracer tracer;
@NotNull public final SelectorProcessingTracer tracer;
@NotNull public final OrgTreeEvaluator orgTreeEvaluator;
@Nullable final SubjectedEvaluationContext subjectedEvaluationContext;
@Nullable public final OwnerResolver ownerResolver;
Expand All @@ -38,7 +44,7 @@ public class ClauseMatchingContext {

public ClauseMatchingContext(
@Nullable ObjectFilterExpressionEvaluator filterEvaluator,
@NotNull MatchingTracer tracer,
@NotNull SelectorProcessingTracer tracer,
@NotNull OrgTreeEvaluator orgTreeEvaluator,
@Nullable SubjectedEvaluationContext subjectedEvaluationContext,
@Nullable OwnerResolver ownerResolver,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,39 @@

import com.evolveum.midpoint.util.logging.Trace;

public interface MatchingTracer {
/**
* Facilitates troubleshooting of selectors and their clauses.
* Crucial for authorizations: https://docs.evolveum.com/midpoint/reference/diag/troubleshooting/authorizations/.
*/
public interface SelectorProcessingTracer {

boolean isEnabled();

/** Called only if {@link #isEnabled()} is `true`. */
void trace(@NotNull TraceEvent event);

class LoggerBased implements MatchingTracer {
/**
* Provides the default logger-based tracer.
*
* @param logger Logger to use. Necessary to e.g. group all authorization-related logging messages under common logger.
* @param logPrefix Text to prepend to each first line of a log record.
*/
static SelectorProcessingTracer loggerBased(
@NotNull Trace logger, @NotNull String logPrefix) {
return new LoggerBased(logger, logPrefix);
}

static SelectorProcessingTracer loggerBased(@NotNull Trace logger) {
return loggerBased(logger, "");
}

class LoggerBased implements SelectorProcessingTracer {

@NotNull final Trace logger;
final String logPrefix;
private final boolean enabled;

public LoggerBased(@NotNull Trace logger, String logPrefix) {
LoggerBased(@NotNull Trace logger, String logPrefix) {
this.logger = logger;
this.logPrefix = logPrefix;
this.enabled = logger.isTraceEnabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

public class OrgRelationClause extends SelectorClause {

/** Immutable. */
@NotNull private final OrgRelationObjectSpecificationType bean;

/** Immutable. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

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

import java.io.Serializable;
import java.util.Collection;
import java.util.stream.Collectors;

Expand All @@ -24,8 +25,10 @@

/**
* A clause of a {@link ValueSelector}.
*
* Immutable.
*/
public abstract class SelectorClause implements DebugDumpable {
public abstract class SelectorClause implements DebugDumpable, Serializable {

public abstract boolean matches(
@NotNull PrismValue value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;

/** TEMPORARY */
/**
* Originally, this was used for evaluation of `subtype` on objects.
* This property is deprecated and will be eventually removed.
*
* However, there are still assignment subtypes that are alive and well, at least for now.
*/
public class SubtypeClause extends SelectorClause {

@NotNull private final String subtype;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ private TypeClause(@NotNull QName typeName) {
return "type";
}

private TypeDefinition getTypeDefinition() {
private synchronized TypeDefinition getTypeDefinition() {
if (typeDefinition == null) {
typeDefinition = PrismContext.get().getSchemaRegistry().findTypeDefinitionByType(typeName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import static com.evolveum.midpoint.util.MiscUtil.configNonNull;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
Expand All @@ -21,6 +22,8 @@
import com.evolveum.midpoint.prism.query.FilterCreationUtil;
import com.evolveum.midpoint.prism.query.ObjectFilter;

import com.evolveum.midpoint.schema.error.ConfigErrorReporter;

import com.google.common.base.Preconditions;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -39,12 +42,15 @@
* Parsed form of {@link ObjectSelectorType} and its subtypes.
*
* It was created to allow easy manipulation and (eventually) better performance due to optimized data structures.
*
* Immutable.
*/
public class ValueSelector implements DebugDumpable {
public class ValueSelector implements DebugDumpable, Serializable {

/** This one is a prominent one, so we'll pull it from among other {@link #clauses}. */
@Nullable private final TypeClause typeClause;

/** Again, a prominent one. */
@Nullable private final ParentClause parentClause;

/**
Expand Down Expand Up @@ -105,8 +111,6 @@ public static ValueSelector of(SelectorClause... clauses) {
typeClause = null;
}

// Temporarily allowed, just to make tests pass
//configCheck(subtype == null, "Subtype specification is not allowed");
String subtype = bean.getSubtype();
if (subtype != null) {
clauses.add(SubtypeClause.of(subtype));
Expand Down Expand Up @@ -148,14 +152,16 @@ public static ValueSelector of(SelectorClause... clauses) {
|| orgRef != null
|| orgRelation != null
|| roleRelation != null
|| (bean instanceof OwnedObjectSelectorType && ((OwnedObjectSelectorType) bean).getTenant() != null)
|| (bean instanceof OwnedObjectSelectorType oBean && oBean.getTenant() != null)
|| !archetypeRefList.isEmpty()) {
throw new ConfigurationException(String.format( // TODO error location
"Both filter/org/role/archetype/tenant and special clause specified in %s", bean));
throw new ConfigurationException(String.format(
"Both filter/org/role/archetype/tenant and special clause specified in %s",
ConfigErrorReporter.describe(bean)));
}

} else {
throw new ConfigurationException("Unsupported special clause: " + special);
throw new ConfigurationException(
"Unsupported special clause: " + special + " in " + ConfigErrorReporter.describe(sBean));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.evolveum.midpoint.prism.delta.ObjectDelta;
import com.evolveum.midpoint.schema.ObjectDeltaOperation;
import com.evolveum.midpoint.util.DebugDumpable;
import com.evolveum.midpoint.util.MiscUtil;
import com.evolveum.midpoint.util.exception.SchemaException;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ArchetypeType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
Expand Down Expand Up @@ -65,13 +66,19 @@ public interface ModelElementContext<O extends ObjectType> extends Serializable,
PrismObject<O> getObjectNew();

/**
* @return "Any" value of the object (new, current, old). It is used when we are not interested in the details
* Returns "any" value of the object (new, current, old). It is used when we are not interested in the details
* but we want just "any" value, e.g. for reporting purposes.
*/
default PrismObject<O> getObjectAny() {
return getObjectNewOrCurrentOrOld();
}

default @NotNull PrismObject<O> getObjectAnyRequired() {
return MiscUtil.stateNonNull(
getObjectAny(),
"No object in %s", this);
}

/**
* @return The first non-null object of these: new, current, old. (Or null of all of them all null.)
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,19 @@

import org.jetbrains.annotations.NotNull;

/** TEMPORARY */
public class GlobalRuleWithId {
@NotNull final GlobalPolicyRuleType ruleBean;
@NotNull final String ruleId;
import java.io.Serializable;

private GlobalRuleWithId(@NotNull GlobalPolicyRuleType ruleBean, @NotNull String containingObjectOid) {
this.ruleBean = ruleBean;
this.ruleId = PolicyRuleTypeUtil.createId(
containingObjectOid,
MiscUtil.stateNonNull(ruleBean.getId(), () -> "Policy rule ID null: " + ruleBean));
}
/** TEMPORARY */
public record GlobalRuleWithId(
@NotNull GlobalPolicyRuleType ruleBean,
@NotNull String ruleId) implements Serializable {

public static GlobalRuleWithId of(@NotNull GlobalPolicyRuleType ruleBean, @NotNull String containingObjectOid) {
return new GlobalRuleWithId(ruleBean, containingObjectOid);
}

public @NotNull GlobalPolicyRuleType getRuleBean() {
return ruleBean;
}

public @NotNull String getRuleId() {
return ruleId;
return new GlobalRuleWithId(
ruleBean,
PolicyRuleTypeUtil.createId(
containingObjectOid,
MiscUtil.stateNonNull(ruleBean.getId(), () -> "Policy rule ID null: " + ruleBean)));
}

@Override
Expand Down

0 comments on commit 1efdb52

Please sign in to comment.