Fix problems with dead entitlement shadows

When a dead entitlement shadow exists, both associationTargetSearch and entitlement resolution (in subject-to-object case) do not work correctly. This is now fixed: we explicitly reject dead shadows in those situations. This resolves MID-7895.
Evolveum · Apr 27, 2022 · d98e856 · d98e856
1 parent 27eb64a
commit d98e856
Show file tree

Hide file tree

Showing 11 changed files with 142 additions and 30 deletions.
diff --git a/...volveum/midpoint/model/common/expression/evaluator/AbstractSearchExpressionEvaluator.java b/...volveum/midpoint/model/common/expression/evaluator/AbstractSearchExpressionEvaluator.java
@@ -166,7 +166,8 @@ protected List<V> transformSingleValue(VariablesMap variables, PlusMinusZero val
         return resultValues;
     }
 
-    protected ObjectQuery extendQuery(ObjectQuery query, ExpressionEvaluationContext params) throws SchemaException, ExpressionEvaluationException {
+    protected ObjectQuery extendQuery(ObjectQuery query, ExpressionEvaluationContext params)
+            throws SchemaException, ExpressionEvaluationException {
         return query;
     }
 
@@ -345,13 +346,22 @@ private <O extends ObjectType> void executeSearch(List<V> valueResults, List<Pri
         // TODO: perhaps we should limit query to some reasonably high number of results?
         SearchResultList<PrismObject<O>> objects = objectResolver.searchObjects(targetTypeClass, query, options, task, opResult);
         for (PrismObject<O> object : objects) {
+            if (!isAcceptable(object)) {
+                LOGGER.trace("Object {} was rejected by additional filtering", object);
+                continue;
+            }
             if (rawResults != null) {
                 rawResults.add(object);
             }
             valueResults.add(createPrismValue(object.getOid(), targetTypeQName, additionalAttributeDeltas, params));
         }
     }
 
+    /** Provides additional filtering e.g. rejecting dead shadows as association targets. */
+    protected <O extends ObjectType> boolean isAcceptable(@NotNull PrismObject<O> object) {
+        return true;
+    }
+
     protected void extendOptions(Collection<SelectorOptions<GetOperationOptions>> options, boolean searchOnResource) {
         // Nothing to do. To be overridden by subclasses
     }

diff --git a/...idpoint/model/common/expression/evaluator/AssociationTargetSearchExpressionEvaluator.java b/...idpoint/model/common/expression/evaluator/AssociationTargetSearchExpressionEvaluator.java
@@ -12,6 +12,7 @@
 import javax.xml.namespace.QName;
 
 import com.evolveum.midpoint.common.LocalizationService;
+import com.evolveum.midpoint.prism.PrismObject;
 import com.evolveum.midpoint.schema.processor.ResourceObjectTypeDefinition;
 import com.evolveum.midpoint.model.api.ModelService;
 import com.evolveum.midpoint.model.common.expression.evaluator.caching.AbstractSearchExpressionEvaluatorCache;
@@ -34,14 +35,20 @@
 import com.evolveum.midpoint.schema.expression.TypedValue;
 import com.evolveum.midpoint.schema.internals.InternalsConfig;
 import com.evolveum.midpoint.schema.util.ObjectQueryUtil;
+import com.evolveum.midpoint.schema.util.ShadowUtil;
 import com.evolveum.midpoint.security.api.SecurityContextManager;
 import com.evolveum.midpoint.util.exception.ExpressionEvaluationException;
 import com.evolveum.midpoint.util.exception.SchemaException;
 import com.evolveum.midpoint.util.exception.SystemException;
+import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.SearchObjectExpressionEvaluatorType;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowAssociationType;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowType;
 
+import org.jetbrains.annotations.NotNull;
+
+import static com.evolveum.midpoint.util.MiscUtil.argCheck;
+
 /**
  * Creates an association (or associations) based on specified condition for the associated object.
  *
@@ -102,6 +109,13 @@ protected void extendOptions(Collection<SelectorOptions<GetOperationOptions>> op
         options.add(SelectorOptions.create(prismContext.toUniformPath(ShadowType.F_ASSOCIATION), GetOperationOptions.createDontRetrieve()));
     }
 
+    @Override
+    protected <O extends ObjectType> boolean isAcceptable(@NotNull PrismObject<O> object) {
+        O objectable = object.asObjectable();
+        argCheck(objectable instanceof ShadowType, "Not a shadow: %s", objectable);
+        return ShadowUtil.isNotDead((ShadowType) objectable);
+    }
+
     protected PrismContainerValue<ShadowAssociationType> createPrismValue(String oid, QName targetTypeQName, List<ItemDelta<PrismContainerValue<ShadowAssociationType>, PrismContainerDefinition<ShadowAssociationType>>> additionalAttributeDeltas, ExpressionEvaluationContext params) {
         ShadowAssociationType association = new ShadowAssociationType(prismContext)
                 .name(params.getMappingQName())

diff --git a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestEntitlements.java b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestEntitlements.java
@@ -21,6 +21,8 @@
 import com.evolveum.midpoint.schema.processor.ResourceAttributeDefinition;
 import com.evolveum.midpoint.schema.processor.ResourceSchemaFactory;
 
+import com.evolveum.midpoint.test.TestResource;
+
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.annotation.DirtiesContext.ClassMode;
 import org.springframework.test.context.ContextConfiguration;
@@ -121,6 +123,12 @@ public class TestEntitlements extends AbstractInitializedModelIntegrationTest {
     private static final String OU_CLUB_DIVERS = "divers";
     private static final String OU_CLUB_SCI_FI = "sci-fi";
 
+    private static final TestResource<ShadowType> SHADOW_MAPMAKERS_DEAD = new TestResource<>(
+            TEST_DIR, "group-mapmakers-dead.xml", "1ff68c92-d526-4cfb-8df5-539bc5fdd097");
+
+    private static final TestResource<ShadowType> SHADOW_GUYBRUSH_DEAD = new TestResource<>(
+            TEST_DIR, "account-guybrush-dead.xml", "2947d268-1d43-4114-bd4c-f4aa723d884a");
+
     private ActivationType jackSwashbucklerAssignmentActivation;
 
     @Override
@@ -298,6 +306,9 @@ public void test220AssignRoleLandluberToWally() throws Exception {
 
     /**
      * Create the group directly on resource. Therefore the shadow does NOT exist.
+     *
+     * We try to mislead midPoint by inserting dead `mapmakers` shadow. (See MID-7895.) This does not lead to the exception
+     * described there, but reveals a similar problem in `associationTargetSearch` evaluation.
      */
     @Test
     public void test222AssignRoleMapmakerToWally() throws Exception {
@@ -309,6 +320,9 @@ public void test222AssignRoleMapmakerToWally() throws Exception {
 
         PrismObject<UserType> user = findUserByUsername(USER_WALLY_NAME);
 
+        given("dead shadow for mapmakers is created");
+        repoAdd(SHADOW_MAPMAKERS_DEAD, result);
+
         // WHEN
         when();
         assignRole(user.getOid(), ROLE_MAPMAKER_OID, task, result);
@@ -321,7 +335,7 @@ public void test222AssignRoleMapmakerToWally() throws Exception {
         assertGroupMember(GROUP_DUMMY_MAPMAKERS_NAME, USER_WALLY_NAME, getDummyResource());
 
         PrismObject<ShadowType> groupLandlubersShadow = findShadowByName(RESOURCE_DUMMY_GROUP_OBJECTCLASS, GROUP_DUMMY_LANDLUBERS_NAME, getDummyResourceObject(), result);
-        PrismObject<ShadowType> groupMapmakersShadow = findShadowByName(RESOURCE_DUMMY_GROUP_OBJECTCLASS, GROUP_DUMMY_MAPMAKERS_NAME, getDummyResourceObject(), result);
+        PrismObject<ShadowType> groupMapmakersShadow = findLiveShadowByName(RESOURCE_DUMMY_GROUP_OBJECTCLASS, GROUP_DUMMY_MAPMAKERS_NAME, getDummyResourceObject(), result);
         assertShadow(groupMapmakersShadow, "mapmakers shadow")
                 .assertKind(ShadowKindType.ENTITLEMENT)
                 .assertIntent("unknown"); // This is due to missing synchronization section for groups in the dummy resource.
@@ -335,6 +349,9 @@ public void test222AssignRoleMapmakerToWally() throws Exception {
                 .assertSize(2)
                 .association(RESOURCE_DUMMY_ASSOCIATION_GROUP_QNAME)
                 .assertShadowOids(groupLandlubersShadow.getOid(), groupMapmakersShadow.getOid());
+
+        // Check if the dead group shadow does not cause problems when fetching the member account.
+        provisioningService.getObject(ShadowType.class, accountWallyOid, null, task, result);
     }
 
     @Test
@@ -864,12 +881,17 @@ public void test350AssignOrangeAccountToGuybrushAndRapp() throws Exception {
 
     /**
      * MID-2668
+     *
+     * Here we also check handling of dead association-target shadows - see MID-7895.
      */
     @Test
     public void test351AssignRoleCrewOfGuybrushToRapp() throws Exception {
         Task task = getTestTask();
         OperationResult result = task.getResult();
 
+        given("there is a dead association-target shadow");
+        repoAdd(SHADOW_GUYBRUSH_DEAD, result);
+
         PrismObject<UserType> userRappBefore = getUser(USER_RAPP_OID);
         display("User rapp before", userRappBefore);
 
@@ -1317,7 +1339,8 @@ public void test715AssociateGuybrushToThugs() throws Exception {
         assignAccountToUser(USER_GUYBRUSH_OID, RESOURCE_DUMMY_ORANGE_OID, "default", task, result);
         dumpUserAndAccounts(USER_GUYBRUSH_OID);
 
-        PrismObject<ShadowType> orangeAccount = findAccountShadowByUsername(USER_GUYBRUSH_USERNAME, getDummyResourceObject(RESOURCE_DUMMY_ORANGE_NAME), result);
+        PrismObject<ShadowType> orangeAccount = findAccountShadowByUsername(
+                USER_GUYBRUSH_USERNAME, getDummyResourceObject(RESOURCE_DUMMY_ORANGE_NAME), true, result);
         assertNotNull("No orange account for guybrush", orangeAccount);
 
         ObjectDelta<ShadowType> delta1 =

diff --git a/model/model-intest/src/test/resources/entitlements/account-guybrush-dead.xml b/model/model-intest/src/test/resources/entitlements/account-guybrush-dead.xml
@@ -0,0 +1,22 @@
+<!--
+  ~ Copyright (C) 2010-2022 Evolveum and contributors
+  ~
+  ~ This work is dual-licensed under the Apache License 2.0
+  ~ and European Union Public License. See LICENSE file for details.
+  -->
+
+<shadow xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
+        xmlns:icfs="http://midpoint.evolveum.com/xml/ns/public/connector/icf-1/resource-schema-3"
+        xmlns:ri="http://midpoint.evolveum.com/xml/ns/public/resource/instance-3"
+        oid="2947d268-1d43-4114-bd4c-f4aa723d884a">
+    <name>guybrush</name>
+    <resourceRef oid="10000000-0000-0000-0000-000000001104" type="ResourceType"/>
+    <dead>true</dead>
+    <objectClass>ri:AccountObjectClass</objectClass>
+    <kind>account</kind>
+    <intent>default</intent>
+    <attributes>
+        <icfs:name>guybrush</icfs:name>
+        <icfs:uid>guybrush</icfs:uid>
+    </attributes>
+</shadow>
diff --git a/model/model-intest/src/test/resources/entitlements/group-mapmakers-dead.xml b/model/model-intest/src/test/resources/entitlements/group-mapmakers-dead.xml
@@ -0,0 +1,23 @@
+<!--
+  ~ Copyright (C) 2010-2022 Evolveum and contributors
+  ~
+  ~ This work is dual-licensed under the Apache License 2.0
+  ~ and European Union Public License. See LICENSE file for details.
+  -->
+
+<shadow xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
+        xmlns:icfs="http://midpoint.evolveum.com/xml/ns/public/connector/icf-1/resource-schema-3"
+        xmlns:ri="http://midpoint.evolveum.com/xml/ns/public/resource/instance-3"
+        oid="1ff68c92-d526-4cfb-8df5-539bc5fdd097">
+    <name>mapmakers</name>
+    <resourceRef oid="10000000-0000-0000-0000-000000000004"/>
+    <dead>true</dead>
+    <objectClass>ri:GroupObjectClass</objectClass>
+    <kind>entitlement</kind>
+    <intent>unknown</intent>
+    <exists>false</exists>
+    <attributes>
+        <icfs:name>mapmakers</icfs:name>
+        <icfs:uid>mapmakers</icfs:uid>
+    </attributes>
+</shadow>
diff --git a/...del-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java b/...del-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java
@@ -1975,7 +1975,7 @@ protected PrismObject<ShadowType> findShadowByNameViaModel(
             ConfigurationException, ExpressionEvaluationException {
         ResourceSchema rSchema = ResourceSchemaFactory.getCompleteSchema(resource);
         ResourceObjectDefinition rOcDef = rSchema.findObjectDefinitionRequired(kind, intent);
-        ObjectQuery query = createShadowQuerySecondaryIdentifier(rOcDef, name, resource);
+        ObjectQuery query = createShadowQuerySecondaryIdentifier(rOcDef, name, resource, false);
         List<PrismObject<ShadowType>> shadows = modelService.searchObjects(ShadowType.class, query, options, task, result);
         if (shadows.isEmpty()) {
             return null;

diff --git a/...main/java/com/evolveum/midpoint/provisioning/impl/shadows/ShadowedObjectConstruction.java b/...main/java/com/evolveum/midpoint/provisioning/impl/shadows/ShadowedObjectConstruction.java
@@ -385,7 +385,7 @@ private PrismObject<ShadowType> acquireEntitlementRepoShadow(PrismContainerValue
 
         try {
             PrismObject<ShadowType> existingRepoShadow =
-                    beans.shadowManager.lookupShadowByAllIds(
+                    beans.shadowManager.lookupLiveShadowByAllIds(
                             ctxEntitlement, identifierContainer, result);
 
             if (existingRepoShadow != null) {

diff --git a/...l/src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowFinder.java b/...l/src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowFinder.java
@@ -130,8 +130,7 @@ private boolean checkExistsFlagForLiveShadow(PrismObject<ShadowType> liveShadow,
 
     List<PrismObject<ShadowType>> searchShadowsByPrimaryIds(ProvisioningContext ctx,
             Collection<ResourceAttribute<?>> identifiers, OperationResult result)
-            throws SchemaException, ConfigurationException, ObjectNotFoundException, CommunicationException,
-            ExpressionEvaluationException {
+            throws SchemaException, ConfigurationException {
 
         ObjectQuery query = createQueryBySelectedIds(ctx, identifiers, true);
         try {
@@ -162,17 +161,17 @@ PrismObject<ShadowType> lookupShadowByIndexedPrimaryIdValue(ProvisioningContext
                 lazy(() -> "primary identifier value " + primaryIdentifierValue + " (impossible because of DB constraint)"));
     }
 
-    public PrismObject<ShadowType> lookupShadowByAllIds(ProvisioningContext ctx,
+    PrismObject<ShadowType> lookupLiveShadowByAllIds(ProvisioningContext ctx,
             ResourceAttributeContainer identifierContainer, OperationResult result)
-            throws SchemaException, ConfigurationException, ObjectNotFoundException, CommunicationException, ExpressionEvaluationException {
+            throws SchemaException, ConfigurationException {
 
-        @SuppressWarnings("unchecked")
+        //noinspection rawtypes,unchecked
         ObjectQuery query = createQueryBySelectedIds(ctx, (Collection) identifierContainer.getValue().getItems(), false);
         LOGGER.trace("Searching for shadow using filter (repo):\n{}", query.debugDumpLazily());
 
         List<PrismObject<ShadowType>> shadows = searchRepoShadows(query, null, result);
 
-        PrismObject<ShadowType> singleShadow = ProvisioningUtil.selectSingleShadow(shadows, identifierContainer);
+        PrismObject<ShadowType> singleShadow = ProvisioningUtil.selectLiveShadow(shadows);
         checkConsistency(singleShadow);
         return singleShadow;
     }

diff --git a/.../src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowManager.java b/.../src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowManager.java
@@ -148,20 +148,18 @@ public PrismObject<ShadowType> lookupLiveShadowByPrimaryId(ProvisioningContext c
      */
     public PrismObject<ShadowType> lookupLiveOrAnyShadowByPrimaryIds(ProvisioningContext ctx,
             Collection<ResourceAttribute<?>> identifiers, OperationResult result)
-            throws SchemaException, CommunicationException, ConfigurationException,
-            ObjectNotFoundException, ExpressionEvaluationException {
+            throws SchemaException, ConfigurationException {
         return ProvisioningUtil.selectLiveOrAnyShadow(
                 shadowFinder.searchShadowsByPrimaryIds(ctx, identifiers, result));
     }
 
     /**
      * Looks up (any) shadow by all available identifiers.
      */
-    public PrismObject<ShadowType> lookupShadowByAllIds(ProvisioningContext ctx,
+    public PrismObject<ShadowType> lookupLiveShadowByAllIds(ProvisioningContext ctx,
             ResourceAttributeContainer identifierContainer, OperationResult result)
-            throws SchemaException, ConfigurationException, ObjectNotFoundException,
-            CommunicationException, ExpressionEvaluationException {
-        return shadowFinder.lookupShadowByAllIds(ctx, identifierContainer, result);
+            throws SchemaException, ConfigurationException, ObjectNotFoundException {
+        return shadowFinder.lookupLiveShadowByAllIds(ctx, identifierContainer, result);
     }
 
     /**

diff --git a/...sioning-impl/src/test/java/com/evolveum/midpoint/provisioning/impl/opendj/TestOpenDj.java b/...sioning-impl/src/test/java/com/evolveum/midpoint/provisioning/impl/opendj/TestOpenDj.java
@@ -22,8 +22,6 @@
 
 import com.evolveum.midpoint.schema.constants.MidPointConstants;
 
-import com.evolveum.midpoint.test.PredefinedTestMethodTracing;
-
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang3.mutable.MutableInt;
 import org.opends.server.types.Entry;
@@ -1695,7 +1693,7 @@ public void test203SearchObjectsByDnExists() throws Exception {
         Task task = getTestTask();
         OperationResult result = task.getResult();
 
-        ObjectQuery query = createAccountShadowQuerySecondaryIdentifier(ACCOUNT_BARBOSSA_DN, resource);
+        ObjectQuery query = createAccountShadowQuerySecondaryIdentifier(ACCOUNT_BARBOSSA_DN, resource, false);
 
         rememberCounter(InternalCounters.CONNECTOR_OPERATION_COUNT);
         rememberCounter(InternalCounters.CONNECTOR_SIMULATED_PAGING_SEARCH_COUNT);
@@ -1734,7 +1732,7 @@ public void test205SearchObjectsByDnNotExists() throws Exception {
         OperationResult result = task.getResult();
 
         ObjectQuery query = createAccountShadowQuerySecondaryIdentifier(
-                "uid=DoesNOTeXXXiSt,ou=People,dc=example,dc=com", resource);
+                "uid=DoesNOTeXXXiSt,ou=People,dc=example,dc=com", resource, false);
 
         rememberCounter(InternalCounters.CONNECTOR_OPERATION_COUNT);
         rememberCounter(InternalCounters.CONNECTOR_SIMULATED_PAGING_SEARCH_COUNT);