Improve SecurityEnforcer (and related) code

Some documentation was added and the worst naming faults were corrected.
Still not finished, but other work is waiting.

infra/schema/src/main/java/com/evolveum/midpoint/schema/expression/VariablesMap.java

@@ -432,6 +432,20 @@ public void setRootNode(ObjectReferenceType objectRef, PrismReferenceDefinition
         put(null, objectRef, def);
     }
 
+    /** Tries to determine the definition of the variable value. */
+    public void addVariableWithDeterminedDefinition(@NotNull String name, @NotNull ObjectType value) {
+        PrismObjectDefinition<?> def;
+        PrismObject<? extends ObjectType> prismObject = value.asPrismObject();
+        PrismObjectDefinition<?> explicitDef = prismObject.getDefinition();
+        if (explicitDef != null) {
+            def = explicitDef;
+        } else {
+            def = PrismContext.get().getSchemaRegistry().findObjectDefinitionByCompileTimeClass(value.getClass());
+        }
+        // When storing Objectable, the use of the variable in path expressions fails
+        addVariableDefinition(name, prismObject, def);
+    }
+
     // TODO: maybe replace by put?
     public <D extends ItemDefinition> void addVariableDefinition(String name, Object value, D definition) {
         if (containsKey(name)) {

infra/schema/src/main/java/com/evolveum/midpoint/schema/selector/spec/ParentClause.java

@@ -25,13 +25,24 @@
 
 import javax.xml.namespace.QName;
 
+/**
+ * A clause that:
+ *
+ * . Puts the (presumably) sub-object selector into the context of either a prism object, or upper-level container.
+ * (Using a type in the {@link #parentSelector} and a {@link #path}.)
+ *
+ * . Optionally restricts the set of candidate parent values (of object or container).
+ */
 public class ParentClause extends SelectorClause {
 
-    @NotNull private final ValueSelector parent;
+    /** Selector that should be applied onto the parent value. */
+    @NotNull private final ValueSelector parentSelector;
+
+    /** Path from the parent value to the current value. */
     @NotNull private final ItemPath path;
 
-    private ParentClause(@NotNull ValueSelector parent, @NotNull ItemPath path) {
-        this.parent = parent;
+    private ParentClause(@NotNull ValueSelector parentSelector, @NotNull ItemPath path) {
+        this.parentSelector = parentSelector;
         this.path = path;
     }
 
@@ -44,8 +55,8 @@ public static ParentClause of(@NotNull ValueSelector parent, @NotNull ItemPath p
         return "parent";
     }
 
-    public @NotNull ValueSelector getParent() {
-        return parent;
+    public @NotNull ValueSelector getParentSelector() {
+        return parentSelector;
     }
 
     public @NotNull ItemPath getPath() {
@@ -63,7 +74,7 @@ public boolean matches(@NotNull PrismValue value, @NotNull MatchingContext ctx)
             traceNotApplicable(ctx, "value has no parent");
             return false;
         }
-        boolean matches = parent.matches(parent2, ctx.child("p", "parent"));
+        boolean matches = parentSelector.matches(parent2, ctx.child("p", "parent"));
         traceApplicability(ctx, matches, "parent specification matches: %s", matches);
         return matches;
     }
@@ -73,7 +84,7 @@ public boolean toFilter(@NotNull FilteringContext ctx)
             throws SchemaException, ExpressionEvaluationException, CommunicationException, SecurityViolationException,
             ConfigurationException, ObjectNotFoundException {
         PrismContext prismContext = PrismContext.get();
-        QName parentTypeName = parent.getTypeName();
+        QName parentTypeName = parentSelector.getTypeName();
         if (parentTypeName == null) {
             throw new ConfigurationException("Parent specification must have type name");
         }
@@ -94,7 +105,7 @@ public boolean toFilter(@NotNull FilteringContext ctx)
                 null, // the original filter is not interesting (or, should we look for parent there?)
                 "p", "parent");
 
-        var applicable = parent.toFilter(childCtx);
+        var applicable = parentSelector.toFilter(childCtx);
         if (!applicable) {
             traceNotApplicable(ctx, "parent selector not applicable");
             return false;
@@ -114,6 +125,6 @@ public boolean toFilter(@NotNull FilteringContext ctx)
     void addDebugDumpContent(StringBuilder sb, int indent) {
         sb.append(" path=").append(path);
         sb.append("\n");
-        DebugUtil.debugDumpWithLabel(sb, "parent selector", parent, indent + 1);
+        DebugUtil.debugDumpWithLabel(sb, "parent selector", parentSelector, indent + 1);
     }
 }

infra/schema/src/main/java/com/evolveum/midpoint/schema/selector/spec/TypeClause.java

@@ -122,6 +122,8 @@ public boolean toFilter(@NotNull FilteringContext ctx) throws ConfigurationExcep
                     filterType.getSimpleName(), restrictedType.getSimpleName());
             return false;
         }
+        // FIXME what if restrictedType is assignable from filterType?
+        //  e.g. we are looking for RoleType but we have a authorization/selector for type=AbstractRoleType
     }
 
     synchronized @NotNull Class<?> getTypeClass() {

infra/schema/src/main/java/com/evolveum/midpoint/schema/selector/spec/ValueSelector.java

@@ -17,21 +17,17 @@
 import java.util.stream.Collectors;
 import javax.xml.namespace.QName;
 
-import com.evolveum.midpoint.prism.Objectable;
-
-import com.evolveum.midpoint.prism.query.FilterCreationUtil;
-import com.evolveum.midpoint.prism.query.ObjectFilter;
-
-import com.evolveum.midpoint.schema.error.ConfigErrorReporter;
-
-import com.evolveum.midpoint.schema.selector.eval.MatchingContext;
-
 import com.google.common.base.Preconditions;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import com.evolveum.midpoint.prism.PrismValue;
+import com.evolveum.midpoint.prism.query.FilterCreationUtil;
+import com.evolveum.midpoint.prism.query.ObjectFilter;
+import com.evolveum.midpoint.schema.error.ConfigErrorReporter;
 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.DebugDumpable;
 import com.evolveum.midpoint.util.DebugUtil;
 import com.evolveum.midpoint.util.QNameUtil;
@@ -60,7 +56,7 @@ public class ValueSelector implements DebugDumpable, Serializable {
      */
     @NotNull private final List<SelectorClause> clauses;
 
-    /** For additional information, like name, description, and so on. Immutable. */
+    /** For additional information, like name, description, and so on. May be missing for generated selectors. Immutable. */
     @Nullable private final ObjectSelectorType bean;
 
     private ValueSelector(
@@ -77,6 +73,7 @@ private ValueSelector(
         }
     }
 
+    /** Covers all values. */
     public static @NotNull ValueSelector empty() {
         return new ValueSelector(null, null, List.of(), null);
     }
@@ -317,7 +314,7 @@ public boolean toFilter(@NotNull FilteringContext ctx)
         return parentClause;
     }
 
-    public @NotNull Class<?> getTypeOrDefault() {
+    public @NotNull Class<?> getEffectiveType() {
         if (typeClause != null) {
             return typeClause.getTypeClass();
         } else {
@@ -434,11 +431,15 @@ public ValueSelector withParentRemoved() {
     }
 
     public boolean isSubObject() {
-        return !Objectable.class.isAssignableFrom(getTypeOrDefault());
+        return !ObjectTypeUtil.isObjectable(getEffectiveType());
     }
 
     @SuppressWarnings({ "WeakerAccess", "BooleanMethodIsAlwaysInverted" })
     public boolean isPureSelf() {
         return clauses.size() == 1 && clauses.get(0) instanceof SelfClause;
     }
+
+    public boolean isParentLess() {
+        return parentClause == null;
+    }
 }

model/model-common/src/main/java/com/evolveum/midpoint/model/common/expression/evaluator/path/PathExpressionEvaluation.java

@@ -88,6 +88,7 @@ private ResolutionContext getInitialResolveContextFromVariable() throws Expressi
         } else if (variableValue instanceof PrismValue) {
             return new ValueResolutionContext((PrismValue) variableValue, context.getContextDescription());
         } else if (variableValueAndDefinition.getTypeClass().isAssignableFrom(variableValue.getClass())) {
+            // FIXME this fails for ObjectType variable values, as getTypeClass() is null for them
             return ValueResolutionContext.fromRealValue(variableValue, context.getContextDescription(), evaluator.getPrismContext());
         } else {
             throw new ExpressionEvaluationException("Unexpected variable value "+variableValue+" ("+variableValue.getClass()+")");

repo/security-api/src/main/java/com/evolveum/midpoint/security/api/Authorization.java

@@ -6,6 +6,7 @@
  */
 package com.evolveum.midpoint.security.api;
 
+import java.io.Serial;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
@@ -24,11 +25,12 @@
 import com.evolveum.prism.xml.ns._public.types_3.ItemPathType;
 
 /**
- * @author semancik
+ * Parsed form of {@link AuthorizationType}.
  *
+ * @author semancik
  */
 public class Authorization implements GrantedAuthority, DebugDumpable {
-    private static final long serialVersionUID = 1L;
+    @Serial private static final long serialVersionUID = 1L;
 
     @NotNull private final AuthorizationType authorizationBean;
     private String sourceDescription;

repo/security-enforcer-api/src/main/java/com/evolveum/midpoint/security/enforcer/api/AbstractAuthorizationParameters.java

@@ -10,12 +10,23 @@
 import com.evolveum.midpoint.prism.PrismValue;
 import com.evolveum.midpoint.util.ShortDumpable;
 
-/** Temporary */
+/**
+ * Parameters describing the details of the situation we want to check authorization for.
+ *
+ * Currently, there are two kinds of parameters, which may be unified in the future.
+ *
+ * . Traditional, object-based ones ({@link AuthorizationParameters}) assume we talk about a prism object and optionally
+ * a delta over it.
+ *
+ * . New, experimental, value-based ones ({@link ValueAuthorizationParameters}) assume we talk about a prism value.
+ * There is now way of specifying a delta there, as delta-like operations are currently carried out on the object level only.
+ */
 public interface AbstractAuthorizationParameters extends ShortDumpable {
 
+    /** The value (for value-based params) or "any object" - for traditional params. */
+    PrismValue getValue();
+
     default boolean hasValue() {
         return getValue() != null;
     }
-
-    PrismValue getValue();
 }

repo/security-enforcer-api/src/main/java/com/evolveum/midpoint/security/enforcer/api/AuthorizationParameters.java

@@ -48,15 +48,6 @@ public ObjectDeltaObject<O> getOdo() {
         return odo;
     }
 
-    private boolean hasObject() {
-        return odo != null && odo.hasAnyObject();
-    }
-
-    @Override
-    public boolean hasValue() {
-        return hasObject();
-    }
-
     public PrismObject<O> getOldObject() {
         if (odo == null) {
             return null;
@@ -72,7 +63,7 @@ public PrismObject<O> getNewObject() {
     }
 
     public PrismObjectValue<O> getValue() {
-        PrismObject<O> object = odo.getAnyObject();
+        PrismObject<O> object = getAnyObject();
         return object != null ? object.getValue() : null;
     }
 

repo/security-enforcer-api/src/main/java/com/evolveum/midpoint/security/enforcer/api/CompileConstraintsOptions.java

@@ -7,9 +7,14 @@
 
 package com.evolveum.midpoint.security.enforcer.api;
 
+import com.evolveum.midpoint.util.annotation.Experimental;
+
+@Experimental
 public class CompileConstraintsOptions {
 
     /**
+     * FIXME this will be implemented in a different way (LensContext authorizations will be computed precisely)
+     *
      * If `true`, sub-object selectors (i.e. ones that deal with values of containers, references, and properties)
      * are skipped. This is used e.g. when dealing with objects and deltas in lens element context, where the data being
      * processed may differ from the data upon which the authorizations are derived. (Hence, we can safely deal only with

repo/security-enforcer-api/src/main/java/com/evolveum/midpoint/security/enforcer/api/ItemSecurityConstraints.java

@@ -19,6 +19,9 @@
  *
  * Basically, it can answer questions whether the operation is allowed for given item (with children) or all item paths.
  *
+ * Contained in {@link ObjectSecurityConstraints}. However, the latter contains information about multiple operations and phases.
+ * This one does not.
+ *
  * @author semancik
  */
 public interface ItemSecurityConstraints extends DebugDumpable {

repo/security-enforcer-api/src/main/java/com/evolveum/midpoint/security/enforcer/api/ObjectSecurityConstraints.java

@@ -9,7 +9,6 @@
 import com.evolveum.midpoint.prism.PrismObject;
 import com.evolveum.midpoint.prism.path.ItemPath;
 import com.evolveum.midpoint.schema.result.OperationResult;
-import com.evolveum.midpoint.schema.selector.eval.OwnerResolver;
 import com.evolveum.midpoint.task.api.Task;
 import com.evolveum.midpoint.util.DebugDumpable;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.AuthorizationDecisionType;
@@ -23,7 +22,7 @@
  * Extracted relevant security constraints related to given object.
  * Unlike {@link PrismEntityOpConstraints}, this one covers all operations (represented by action URLs).
  *
- * @see SecurityEnforcer#compileSecurityConstraints(PrismObject, OwnerResolver, Task, OperationResult)
+ * @see SecurityEnforcer#compileSecurityConstraints(PrismObject, SecurityEnforcer.Options, Task, OperationResult)
  * @see PrismEntityOpConstraints
  */
 public interface ObjectSecurityConstraints extends DebugDumpable {

repo/security-enforcer-api/src/main/java/com/evolveum/midpoint/security/enforcer/api/PositiveNegativeItemPaths.java

@@ -11,11 +11,18 @@
 import java.util.Iterator;
 import java.util.List;
 
+import com.evolveum.midpoint.prism.PrismObject;
+import com.evolveum.midpoint.prism.PrismObjectValue;
 import com.evolveum.midpoint.prism.path.ItemPath;
 import com.evolveum.midpoint.prism.path.ItemPathCollectionsUtil;
+import com.evolveum.midpoint.schema.result.OperationResult;
+import com.evolveum.midpoint.security.api.MidPointPrincipal;
+import com.evolveum.midpoint.task.api.Task;
 import com.evolveum.midpoint.util.PrettyPrinter;
 import com.evolveum.midpoint.util.ShortDumpable;
 
+import com.evolveum.midpoint.xml.ns._public.common.common_3.AuthorizationPhaseType;
+
 import org.jetbrains.annotations.NotNull;
 
 import static com.evolveum.midpoint.prism.path.ItemPath.*;
@@ -33,6 +40,15 @@
  * Then, it is queried by calling {@link #includes(ItemPath)} to determine whether given item path is _completely_
  * covered by this set.
  *
+ * An alternative approach is represented by {@link PrismEntityOpConstraints} and its implementations:
+ *
+ * . This (older) class is used e.g. in {@link ObjectSecurityConstraints} implementation, returned e.g. by
+ * {@link SecurityEnforcer#compileSecurityConstraints(PrismObject, SecurityEnforcer.Options, Task, OperationResult)} method.
+ *
+ * . The newer class ({@link PrismEntityOpConstraints}) is used e.g. as a return value of narrow-focused
+ * {@link SecurityEnforcer#compileOperationConstraints(MidPointPrincipal, PrismObjectValue, AuthorizationPhaseType, String[],
+ * SecurityEnforcer.Options, CompileConstraintsOptions, Task, OperationResult)}.
+ *
  * @author semancik
  */
 public class PositiveNegativeItemPaths implements ShortDumpable {
@@ -50,14 +66,6 @@ public boolean includesAllItems() {
         return allItemsIncluded;
     }
 
-    /**
-     * If returns `true`, then this specification provably includes no items, i.e. {@link #includes(ItemPath)}
-     * would return `false` when called with any argument.
-     */
-    public boolean includesNoItems() {
-        return includedItems.isEmpty() && excludedItems.isEmpty() && !allItemsIncluded;
-    }
-
     protected @NotNull List<? extends ItemPath> getIncludedItems() {
         return includedItems;
     }

repo/security-enforcer-api/src/main/java/com/evolveum/midpoint/security/enforcer/api/PrismEntityOpConstraints.java

@@ -17,7 +17,14 @@
 
 import org.jetbrains.annotations.NotNull;
 
-/** Compiled security constraints for a given operation applicable to a prism entity ({@link Item} or {@link PrismValue}). */
+/**
+ * Compiled security constraints for a given operation and phase applicable to a prism entity
+ * ({@link Item} or {@link PrismValue}).
+ *
+ * It is a simplified version of {@link ObjectSecurityConstraints} and (somewhat) also {@link ItemSecurityConstraints}.
+ *
+ * TODO the "prism entity" name is not the best; the idea is to cover both items and values, but something better should be found
+ */
 public interface PrismEntityOpConstraints extends DebugDumpable {
 
     /**

repo/security-enforcer-api/src/main/java/com/evolveum/midpoint/security/enforcer/api/ValueAuthorizationParameters.java

@@ -24,7 +24,7 @@ public class ValueAuthorizationParameters<V extends PrismValue> implements Abstr
 
     @Nullable private final V value;
 
-    public ValueAuthorizationParameters(@Nullable V value) {
+    private ValueAuthorizationParameters(@Nullable V value) {
         this.value = value;
     }