Skip to content

Commit

Permalink
Fix entitlements handling during phantom renames
Browse files Browse the repository at this point in the history 
When a phantom rename is issued (e.g. DN is set to the same value
as it has had), the entitlements were added+removed, leading to their
loss. This is now fixed by checking if the rename is real.

This resolves MID-6770.
  • Loading branch information
@mederly
mederly committed Mar 5, 2021
1 parent 3895ba3 commit 377aba6
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 18 deletions.
56 changes: 43 additions & 13 deletions ...java/com/evolveum/midpoint/provisioning/impl/resourceobjects/ResourceObjectConverter.java
View file
Expand Up @@ -56,6 +56,10 @@
import java.util.Map.Entry;
import java.util.concurrent.atomic.AtomicInteger;

import static com.evolveum.midpoint.prism.PrismPropertyValue.getRealValue;

import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;

/**
*
* Responsibilities:
Expand Down Expand Up @@ -1125,9 +1129,7 @@ private PrismObject<ShadowType> executeEntitlementChangesModify(ProvisioningCont

Map<ResourceObjectDiscriminator, ResourceObjectOperations> roMap = new HashMap<>();

if (LOGGER.isTraceEnabled()) {
LOGGER.trace("executeEntitlementChangesModify, old shadow:\n{}", subjectShadowBefore.debugDump(1));
}
LOGGER.trace("executeEntitlementChangesModify, old shadow:\n{}", subjectShadowBefore.debugDumpLazily(1));

for (ItemDelta subjectDelta : subjectDeltas) {
ItemPath subjectItemPath = subjectDelta.getPath();
Expand All @@ -1141,13 +1143,11 @@ private PrismObject<ShadowType> executeEntitlementChangesModify(ProvisioningCont

ContainerDelta<ShadowAssociationType> associationDelta = prismContext.deltaFactory().container().createDelta(ShadowType.F_ASSOCIATION, subjectShadowBefore.getDefinition());
PrismContainer<ShadowAssociationType> associationContainer = subjectShadowBefore.findContainer(ShadowType.F_ASSOCIATION);
if (associationContainer == null || associationContainer.isEmpty()){
if (associationContainer == null || associationContainer.isEmpty()) {
LOGGER.trace("No shadow association container in old shadow. Skipping processing entitlements change for {}.", subjectItemPath);
continue;
}
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Processing association container in old shadow for {}:\n{}", subjectItemPath, associationContainer.debugDump(1));
}
LOGGER.trace("Processing association container in old shadow for {}:\n{}", subjectItemPath, associationContainer.debugDumpLazily(1));

// Delete + re-add association values that should ensure correct functioning in case of rename
// This has to be done only for associations that require explicit referential integrity.
Expand All @@ -1168,13 +1168,15 @@ private PrismObject<ShadowType> executeEntitlementChangesModify(ProvisioningCont
if (!ShadowUtil.matchesAttribute(subjectItemPath, valueAttributeName)) {
continue;
}
LOGGER.trace("Processing association {} on rename", associationName);
associationDelta.addValuesToDelete(associationValue.clone());
associationDelta.addValuesToAdd(associationValue.clone());
}
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Resulting association delta for {}:\n{}", subjectItemPath, associationDelta.debugDump(1));
if (isRenameReal(subjectShadowBefore, subjectShadowAfter, subjectItemPath)) {
LOGGER.trace("Processing association {} on rename", associationName);
associationDelta.addValuesToDelete(associationValue.clone());
associationDelta.addValuesToAdd(associationValue.clone());
} else {
LOGGER.trace("NOT processing association {} because the rename is phantom", associationName);
}
}
LOGGER.trace("Resulting association delta for {}:\n{}", subjectItemPath, associationDelta.debugDumpLazily(1));
if (!associationDelta.isEmpty()) {
entitlementConverter.collectEntitlementsAsObjectOperation(ctx, roMap, associationDelta, subjectShadowBefore, subjectShadowAfter, parentResult);
}
Expand All @@ -1187,6 +1189,34 @@ private PrismObject<ShadowType> executeEntitlementChangesModify(ProvisioningCont
return subjectShadowAfter;
}

private <T> boolean isRenameReal(PrismObject<ShadowType> objectBefore, PrismObject<ShadowType> objectAfter, ItemPath itemPath) throws SchemaException {
PrismProperty<T> propertyBefore = objectBefore.findProperty(itemPath);
PrismProperty<T> propertyAfter = objectAfter.findProperty(itemPath);
boolean beforeIsNull = propertyBefore == null || propertyBefore.isEmpty();
boolean afterIsNull = propertyAfter == null || propertyAfter.isEmpty();
if (beforeIsNull) {
return !afterIsNull;
} else if (afterIsNull) {
return true;
}
MatchingRule<T> matchingRule = getMatchingRule(propertyAfter.getDefinition());
return !MiscUtil.unorderedCollectionEquals(propertyBefore.getValues(), propertyAfter.getValues(),
(v1, v2) -> {
try {
return matchingRule.match(getRealValue(v1), getRealValue(v2));
} catch (SchemaException e) {
throw new IllegalStateException(e);
}
});
}

private <T> MatchingRule<T> getMatchingRule(PrismPropertyDefinition<T> definition) throws SchemaException {
QName matchingRuleName = defaultIfNull(
definition != null ? definition.getMatchingRuleQName() : null,
PrismConstants.DEFAULT_MATCHING_RULE_NAME);
return matchingRuleRegistry.getMatchingRule(matchingRuleName, null);
}

private void executeEntitlementChangesDelete(ProvisioningContext ctx, PrismObject<ShadowType> subjectShadow,
OperationProvisioningScriptsType scripts, ConnectorOperationOptions connOptions,
OperationResult parentResult) throws SchemaException {
Expand Down
90 changes: 85 additions & 5 deletions ...sioning-impl/src/test/java/com/evolveum/midpoint/provisioning/impl/opendj/TestOpenDj.java
View file
Expand Up @@ -128,6 +128,12 @@ public class TestOpenDj extends AbstractOpenDjTest {
"sk", "Kapitán Džek Sperou"
};

private static final QName QNAME_SN = new QName(RESOURCE_NS, "sn");
private static final ItemPath PATH_SN = ItemPath.create(ShadowType.F_ATTRIBUTES, QNAME_SN);

private static final QName QNAME_DN = new QName(RESOURCE_NS, "dn");
private static final ItemPath PATH_DN = ItemPath.create(ShadowType.F_ATTRIBUTES, QNAME_DN);

private String groupSailorOid;

@Autowired
Expand Down Expand Up @@ -928,7 +934,7 @@ public void test140AddAndModifyAccountJack() throws Exception {
display("Object after change", accountType);

String uid = ShadowUtil.getSingleStringAttributeValue(accountType, getPrimaryIdentifierQName());
List<Object> snValues = ShadowUtil.getAttributeValues(accountType, new QName(RESOURCE_NS, "sn"));
List<Object> snValues = ShadowUtil.getAttributeValues(accountType, QNAME_SN);
assertNotNull("No 'sn' attribute", snValues);
assertFalse("Surname attributes must not be empty", snValues.isEmpty());
assertEquals(1, snValues.size());
Expand Down Expand Up @@ -1853,8 +1859,7 @@ public void test233SearchObjectsPagedNoOffsetSortSn() throws Exception {
ObjectQuery query = getQueryConverter().createObjectQuery(ShadowType.class, queryType);

ObjectPaging paging = prismContext.queryFactory().createPaging(null, 4);
paging.setOrdering(prismContext.queryFactory().createOrdering(
ItemPath.create(ShadowType.F_ATTRIBUTES, new QName(RESOURCE_NS, "sn")), OrderDirection.ASCENDING));
paging.setOrdering(prismContext.queryFactory().createOrdering(PATH_SN, OrderDirection.ASCENDING));
query.setPaging(paging);

rememberCounter(InternalCounters.CONNECTOR_OPERATION_COUNT);
Expand Down Expand Up @@ -1889,8 +1894,7 @@ public void test234SearchObjectsPagedOffsetSortSn() throws Exception {
ObjectQuery query = getQueryConverter().createObjectQuery(ShadowType.class, queryType);

ObjectPaging paging = prismContext.queryFactory().createPaging(2, 4);
paging.setOrdering(prismContext.queryFactory().createOrdering(
ItemPath.create(ShadowType.F_ATTRIBUTES, new QName(RESOURCE_NS, "sn")), OrderDirection.ASCENDING));
paging.setOrdering(prismContext.queryFactory().createOrdering(PATH_SN, OrderDirection.ASCENDING));
query.setPaging(paging);

rememberCounter(InternalCounters.CONNECTOR_OPERATION_COUNT);
Expand Down Expand Up @@ -2513,6 +2517,76 @@ public void test418GetMorgan() throws Exception {
assertShadows(22);
}

/**
* MID-6770
*/
@Test
public void test419PhantomRenameMorgan() throws Exception {
Task task = getTestTask();
OperationResult result = task.getResult();

ObjectDelta<ShadowType> delta = deltaFor(ShadowType.class)
.item(PATH_DN, getAccountAttributeDefinition(QNAME_DN))
.replace("uid=morgan,ou=People,dc=example,dc=com")
.asObjectDelta(ACCOUNT_MORGAN_OID);

when();
provisioningService.modifyObject(ShadowType.class, ACCOUNT_MORGAN_OID, delta.getModifications(),
null, null, task, result);

then();
assertSuccess(result);

PrismObject<ShadowType> shadow = provisioningService.getObject(ShadowType.class, ACCOUNT_MORGAN_OID, null, task, result);
display("Shadow", shadow);

assertEntitlementGroup(shadow, GROUP_SWASHBUCKLERS_OID);
assertEntitlementGroup(shadow, groupSailorOid);
assertEntitlementGroup(shadow, GROUP_CORSAIRS_OID);

assertShadows(22);
}

/**
* MID-6770
*
* The same as before, but now the DN attribute in shadow is out of date.
* We intentionally change it via direct repo call before the provisioning action.
*/
@Test
public void test420PhantomRenameMorganRotten() throws Exception {
Task task = getTestTask();
OperationResult result = task.getResult();

ObjectDelta<ShadowType> rotDelta = deltaFor(ShadowType.class)
.item(PATH_DN, getAccountAttributeDefinition(QNAME_DN))
.replace("uid=morgan-rotten,ou=People,dc=example,dc=com")
.asObjectDelta(ACCOUNT_MORGAN_OID);
repositoryService.modifyObject(ShadowType.class, ACCOUNT_MORGAN_OID, rotDelta.getModifications(), result);

// This is no-op on resource. (The DN on resource has not changed.)
ObjectDelta<ShadowType> delta = deltaFor(ShadowType.class)
.item(PATH_DN, getAccountAttributeDefinition(QNAME_DN))
.replace("uid=morgan,ou=People,dc=example,dc=com")
.asObjectDelta(ACCOUNT_MORGAN_OID);

when();
provisioningService.modifyObject(ShadowType.class, ACCOUNT_MORGAN_OID, delta.getModifications(),
null, null, task, result);

then();
assertSuccess(result);

PrismObject<ShadowType> shadow = provisioningService.getObject(ShadowType.class, ACCOUNT_MORGAN_OID, null, task, result);
display("Shadow", shadow);

assertEntitlementGroup(shadow, GROUP_SWASHBUCKLERS_OID);
assertEntitlementGroup(shadow, groupSailorOid);
assertEntitlementGroup(shadow, GROUP_CORSAIRS_OID);

assertShadows(22);
}

/**
* Morgan has a group associations. If the account is gone the group memberships should also be gone.
*/
Expand Down Expand Up @@ -3318,4 +3392,10 @@ protected void assertConnectorOperationIncrement(int expectedIncrementSmart, int
private void assertDescription(Entry entry, String expectedOrigValue, String... params) {
OpenDJController.assertAttributeLang(entry, ATTRIBUTE_DESCRIPTION_NAME, expectedOrigValue, params);
}

private <T> ResourceAttributeDefinition<T> getAccountAttributeDefinition(QName attrName) throws SchemaException {
return RefinedResourceSchemaImpl.getResourceSchema(resourceType, prismContext)
.findObjectClassDefinition(RESOURCE_OPENDJ_ACCOUNT_OBJECTCLASS)
.findAttributeDefinition(attrName);
}
}

0 comments on commit 377aba6

Please sign in to comment.