Merge branch 'master' of github.com:Evolveum/midpoint

Evolveum · Apr 4, 2023 · 07430f8 · 07430f8
2 parents 0e1f002 + 131cb46
commit 07430f8
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 26 deletions.
diff --git a/...del-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/SchemaTransformer.java b/...del-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/SchemaTransformer.java
@@ -390,7 +390,9 @@ private <O extends ObjectType> ObjectSecurityConstraints compileSecurityConstrai
         }
     }
 
-    private void applySecurityConstraints(PrismContainerValue<?> pcv, ObjectSecurityConstraints securityConstraints, AuthorizationPhaseType phase,
+    /** The variant of `applySecurityConstraints` for a {@link PrismContainerValue}. */
+    private void applySecurityConstraints(
+            PrismContainerValue<?> pcv, ObjectSecurityConstraints securityConstraints, AuthorizationPhaseType phase,
             AuthorizationDecisionType defaultReadDecision, AuthorizationDecisionType defaultAddDecision,
             AuthorizationDecisionType defaultModifyDecision, boolean applyToDefinitions) {
         LOGGER.trace("applySecurityConstraints(items): items={}, phase={}, defaults R={}, A={}, M={}",
@@ -402,10 +404,6 @@ private void applySecurityConstraints(PrismContainerValue<?> pcv, ObjectSecurity
         for (Item<?, ?> item : pcv.getItems()) {
             ItemPath itemPath = item.getPath();
             ItemDefinition<?> itemDef = item.getDefinition();
-            if (itemDef != null && itemDef.isElaborate()) {
-                LOGGER.trace("applySecurityConstraints(item): {}: skip (elaborate)", itemPath);
-                continue;
-            }
             ItemPath nameOnlyItemPath = itemPath.namedSegmentsOnly();
             AuthorizationDecisionType itemReadDecision = computeItemDecision(securityConstraints, nameOnlyItemPath, ModelAuthorizationAction.AUTZ_ACTIONS_URLS_GET, defaultReadDecision, phase);
             AuthorizationDecisionType itemAddDecision = computeItemDecision(securityConstraints, nameOnlyItemPath, ModelAuthorizationAction.AUTZ_ACTIONS_URLS_ADD, defaultReadDecision, phase);
@@ -428,6 +426,15 @@ private void applySecurityConstraints(PrismContainerValue<?> pcv, ObjectSecurity
                 if (itemReadDecision == AuthorizationDecisionType.DENY) {
                     // Explicitly denied access to the entire container
                     itemsToRemove.add(item);
+                } else if (itemDef != null && itemDef.isElaborate()) {
+                    LOGGER.trace("applySecurityConstraints(item): {}: skipping deeper dive, applying decision '{}' (elaborate)",
+                            itemPath, itemReadDecision);
+                    if (itemReadDecision == null) {
+                        // "default" (null) means we do not analyze things further, and simply deny the access completely
+                        itemsToRemove.add(item);
+                    } else {
+                        // ALLOW means we do not analyze things further, and simply allow the access completely
+                    }
                 } else {
                     // No explicit decision (even ALLOW is not final here as something may be denied deeper inside)
                     AuthorizationDecisionType subDefaultReadDecision = defaultReadDecision;
@@ -477,8 +484,11 @@ private MutableItemDefinition<?> mutable(ItemDefinition<?> itemDef) {
         return itemDef.toMutable();
     }
 
-    private <O extends ObjectType> void applySecurityConstraints(ObjectDelta<O> objectDelta, ObjectSecurityConstraints securityConstraints, AuthorizationPhaseType phase,
-            AuthorizationDecisionType defaultReadDecision, AuthorizationDecisionType defaultAddDecision, AuthorizationDecisionType defaultModifyDecision) {
+    /** The variant of `applySecurityConstraints` for an {@link ObjectDelta}. */
+    private <O extends ObjectType> void applySecurityConstraints(
+            ObjectDelta<O> objectDelta, ObjectSecurityConstraints securityConstraints, AuthorizationPhaseType phase,
+            AuthorizationDecisionType defaultReadDecision, AuthorizationDecisionType defaultAddDecision,
+            AuthorizationDecisionType defaultModifyDecision) {
         LOGGER.trace("applySecurityConstraints(objectDelta): items={}, phase={}, defaults R={}, A={}, M={}",
                 objectDelta, phase, defaultReadDecision, defaultAddDecision, defaultModifyDecision);
         if (objectDelta == null) {
@@ -503,10 +513,6 @@ private <O extends ObjectType> void applySecurityConstraints(ObjectDelta<O> obje
         for (ItemDelta<?, ?> modification : modifications) {
             ItemPath itemPath = modification.getPath();
             ItemDefinition<?> itemDef = modification.getDefinition();
-            if (itemDef != null && itemDef.isElaborate()) {
-                LOGGER.trace("applySecurityConstraints(item): {}: skip (elaborate)", itemPath);
-                continue;
-            }
             ItemPath nameOnlyItemPath = itemPath.namedSegmentsOnly();
             AuthorizationDecisionType itemReadDecision = computeItemDecision(securityConstraints, nameOnlyItemPath, ModelAuthorizationAction.AUTZ_ACTIONS_URLS_GET, defaultReadDecision, phase);
             AuthorizationDecisionType itemAddDecision = computeItemDecision(securityConstraints, nameOnlyItemPath, ModelAuthorizationAction.AUTZ_ACTIONS_URLS_ADD, defaultReadDecision, phase);
@@ -517,6 +523,13 @@ private <O extends ObjectType> void applySecurityConstraints(ObjectDelta<O> obje
                 if (itemReadDecision == AuthorizationDecisionType.DENY) {
                     // Explicitly denied access to the entire container
                     itemsToRemove.add(modification);
+                } else if (itemDef != null && itemDef.isElaborate()) {
+                    LOGGER.trace("applySecurityConstraints(item): {}: skipping deeper dive, applying decision '{}' (elaborate)",
+                            itemPath, itemReadDecision);
+                    // See comments in the "PCV" version of this method (above)
+                    if (itemReadDecision == null) {
+                        itemsToRemove.add(modification);
+                    }
                 } else {
                     // No explicit decision (even ALLOW is not final here as something may be denied deeper inside)
                     AuthorizationDecisionType subDefaultReadDecision = defaultReadDecision;
@@ -574,8 +587,9 @@ private boolean reduceContainerValues(List<? extends PrismContainerValue<?>> val
         return removedSomething;
     }
 
-    public <D extends ItemDefinition> void applySecurityConstraints(D itemDefinition, ObjectSecurityConstraints securityConstraints,
-            AuthorizationPhaseType phase) {
+    /** The variant of `applySecurityConstraints` for a {@link ItemDefinition}. */
+    public <D extends ItemDefinition> void applySecurityConstraints(
+            D itemDefinition, ObjectSecurityConstraints securityConstraints, AuthorizationPhaseType phase) {
         if (phase == null) {
             applySecurityConstraintsPhase(itemDefinition, securityConstraints, AuthorizationPhaseType.REQUEST);
             applySecurityConstraintsPhase(itemDefinition, securityConstraints, AuthorizationPhaseType.EXECUTION);
@@ -611,7 +625,7 @@ private <D extends ItemDefinition> void applySecurityConstraintsItemDef(D itemDe
         boolean anySubElementModify = false;
         if (itemDefinition instanceof PrismContainerDefinition<?> && !thisWasSeen) {
             PrismContainerDefinition<?> containerDefinition = (PrismContainerDefinition<?>)itemDefinition;
-            List<? extends ItemDefinition> subDefinitions = ((PrismContainerDefinition<?>)containerDefinition).getDefinitions();
+            List<? extends ItemDefinition> subDefinitions = containerDefinition.getDefinitions();
             for (ItemDefinition subDef: subDefinitions) {
                 ItemPath subPath = ItemPath.create(nameOnlyItemPath, subDef.getItemName());
                 if (subDef.isElaborate()) {

diff --git a/...n/java/com/evolveum/midpoint/model/impl/schema/transform/TransformableItemDefinition.java b/...n/java/com/evolveum/midpoint/model/impl/schema/transform/TransformableItemDefinition.java
@@ -324,6 +324,7 @@ public ItemDefinition<I> deepClone(@NotNull DeepCloneOperation operation) {
 
     protected abstract TransformableItemDefinition<I,D> copy();
 
+    // FIXME we should display overridden values here (e.g. RAM access flags)
     @Override
     public String toString() {
         return "Transformable:" + delegate().toString();

diff --git a/...l-intest/src/test/java/com/evolveum/midpoint/model/intest/security/TestSecurityBasic.java b/...l-intest/src/test/java/com/evolveum/midpoint/model/intest/security/TestSecurityBasic.java
@@ -17,6 +17,8 @@
 
 import com.evolveum.midpoint.model.api.ModelInteractionService;
 import com.evolveum.midpoint.schema.processor.ResourceObjectDefinition;
+
+import org.assertj.core.api.Assertions;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.annotation.DirtiesContext.ClassMode;
 import org.springframework.test.context.ContextConfiguration;
@@ -91,8 +93,9 @@ public void test201AutzJackSuperuserRole() throws Exception {
         then();
         assertSuperuserAccess(NUMBER_OF_ALL_USERS);
 
-        Collection<SelectorOptions<GetOperationOptions>> withCases = SchemaService.get().getOperationOptionsBuilder().
-                item(AccessCertificationCampaignType.F_CASE).retrieve().build();
+        Collection<SelectorOptions<GetOperationOptions>> withCases =
+                SchemaService.get().getOperationOptionsBuilder()
+                        .item(AccessCertificationCampaignType.F_CASE).retrieve().build();
         assertSearch(AccessCertificationCampaignType.class, null, withCases, new SearchAssertion<>() {
 
             public void assertObjects(String message, List<PrismObject<AccessCertificationCampaignType>> objects) {
@@ -3166,7 +3169,11 @@ public void test380AutzJackSelfTaskOwner() throws Exception {
         assertGetDeny(UserType.class, USER_GUYBRUSH_OID);
 
         assertGetDeny(TaskType.class, TASK_USELESS_ADMINISTRATOR_OID);
-        assertGetAllow(TaskType.class, TASK_USELESS_JACK_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();
+
+        assertOwnTaskEditSchema(task.asPrismObject());
 
         assertSearch(UserType.class, null, 0);
         assertSearch(ObjectType.class, null, 0);
@@ -3188,6 +3195,18 @@ public void test380AutzJackSelfTaskOwner() throws Exception {
         assertGlobalStateUntouched();
     }
 
+    private void assertOwnTaskEditSchema(PrismObject<TaskType> task) throws Exception {
+        PrismObjectDefinition<TaskType> def = getEditObjectDefinition(task);
+        displayDumpable("task edit def", def);
+        // All items are readable, except for `activityState` (see `read-self-task-owner` autz)
+        // All items are "addable" (see `add-self-task-owner` autz)
+        // No items are modifiable, as there's no autz to modify the task.
+        assertItemFlags(def, TaskType.F_NAME, true, true, false);
+        assertItemFlags(def, TaskType.F_DESCRIPTION, true, true, false);
+        assertItemFlags(def, TaskType.F_ACTIVITY, true, true, false);
+        assertItemFlags(def, TaskType.F_ACTIVITY_STATE, false, true, false);
+    }
+
     /**
      * Searches for users with given assignment/targetRef (both directly and using EXISTS clause). See MID-7931.
      */

diff --git a/model/model-intest/src/test/resources/security/role-self-task-owner.xml b/model/model-intest/src/test/resources/security/role-self-task-owner.xml
@@ -6,9 +6,7 @@
   -->
 <role oid="455edc40-30c6-11e7-937f-df84f38dd402"
         xmlns="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:org='http://midpoint.evolveum.com/xml/ns/public/common/org-3'>
+        xmlns:q="http://prism.evolveum.com/xml/ns/public/query-3">
     <name>Role Self Task Owner</name>
     <authorization>
         <name>read-self-task-owner</name>
@@ -19,6 +17,7 @@
                 <special>self</special>
             </owner>
         </object>
+        <exceptItem>activityState</exceptItem> <!-- MID-8635 -->
     </authorization>
     <authorization>
         <name>add-self-task-owner</name>

diff --git a/model/model-intest/src/test/resources/security/task-useless-administrator.xml b/model/model-intest/src/test/resources/security/task-useless-administrator.xml
@@ -7,9 +7,7 @@
   -->
 
 <task oid="daa36dba-30c7-11e7-bd7d-6311953a3ecd"
-    xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
-    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-    xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+    xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3">
 
     <name>Useless task: administrator</name>
 
@@ -21,4 +19,15 @@
     <executionState>closed</executionState>
 
     <handlerUri>http://midpoint.evolveum.com/xml/ns/public/model/synchronization/task/useless/handler-3</handlerUri>
+
+    <!-- MID-8635 -->
+    <activity>
+        <work>
+            <focusValidityScan/>
+        </work>
+    </activity>
+    <activityState>
+        <activity/>
+        <tree/>
+    </activityState>
 </task>
diff --git a/model/model-intest/src/test/resources/security/task-useless-jack.xml b/model/model-intest/src/test/resources/security/task-useless-jack.xml
@@ -7,9 +7,7 @@
   -->
 
 <task oid="642d8174-30c8-11e7-b338-c3cf3a6c548a"
-    xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
-    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-    xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+    xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3">
 
     <name>Useless task: jack</name>
 
@@ -21,4 +19,15 @@
     <executionState>closed</executionState>
 
     <handlerUri>http://midpoint.evolveum.com/xml/ns/public/model/synchronization/task/useless/handler-3</handlerUri>
+
+    <!-- MID-8635 -->
+    <activity>
+        <work>
+            <focusValidityScan/>
+        </work>
+    </activity>
+    <activityState>
+        <activity/>
+        <tree/>
+    </activityState>
 </task>