[#1173] Check the references of every managed attribute type of an add and ...

Valery Kharseko

yesterday 85b28b3d0fc05bc49d350a98c07459f4bcb21f43

[#1173] Check the references of every managed attribute type of an add and every modification of a modify, not only the first (#1175)

Fixes #1173

## Problem

With `check-references: true`, the Referential Integrity plugin checks
references in its two `doPreOperation` hooks, and both returned after
their first check:

```java
if (result.getResultCode() != ResultCode.SUCCESS)
{
  return result;
}
```

A reference that passes yields
`PluginResult.PreOperation.continueOperationProcessing()`, whose result
code is `null`, not `SUCCESS`, so the condition also held for a check
that passed. As a result:

- **add**: only the first managed attribute type was checked. An entry
that did not hold that attribute had none of its references checked,
because an empty attribute list passes too.
- **modify**: only the first ADD or REPLACE modification of a managed
attribute was checked, and the ones after it were not.

## Fix

Both hooks now return early only when processing must stop
(`!result.continueProcessing()`).
`isIntegrityMaintained(List<Attribute>, …)` already worked that way: it
compares with `continueOperationProcessing()`.

## Tests

`ReferentialIntegrityPluginTestCase`, with `check-references` on
`manager` and `seeAlso` below `dc=example,dc=com`, and the data provider
`missingReferenceNextToAnother`: a missing reference in one attribute,
next to a valid reference in the other attribute or alone. Both
attributes take both places, because the order in which the plugin
checks attribute types is an implementation detail.

- `testEnforceIntegrityAddChecksEveryAttributeType`: adding an entry
with a missing reference in either attribute is refused with
`CONSTRAINT_VIOLATION`.
- `testEnforceIntegrityModifyChecksEveryModification`: a modify request
whose second modification adds a missing reference is refused. Its first
modification replaces the other managed attribute with a valid
reference, or, in the rows without one, replaces `description`, which
the plugin does not manage.

The plugin configuration these tests share moved into a helper,
`enableCheckReferences`.

Without the fix, 4 of the 69 tests fail with `expected [Constraint
Violation] but found [Success]`:
- add `[seeAlso, manager]` and `[seeAlso, null]`, because the plugin
checks `manager` first;
- modify `[manager, seeAlso]` and `[seeAlso, manager]`.

The two modify rows without a valid reference passed even before the
fix, since an unmanaged first modification was already skipped; they
guard that case. With the fix the class passes 69/69.

Found while working on #1172 (PR #1174), whose test has to check
`continueProcessing()` for the same reason.

 opendj-server-legacy/src/main/java/org/opends/server/plugins/ReferentialIntegrityPlugin.java

@@ -1104,7 +1104,8 @@
        }
        PluginResult.PreOperation result =
            isIntegrityMaintained(modifiedAttribute, entryDN, entryBaseDN);
        if (result.getResultCode() != ResultCode.SUCCESS)
        // A reference that passes yields continueOperationProcessing(), whose result code is null, not SUCCESS.
        if (!result.continueProcessing())
        {
          return result;
        }
@@ -1138,7 +1139,7 @@
    {
      final List<Attribute> attrs = entry.getAllAttributes(attrType, false);
      PluginResult.PreOperation result = isIntegrityMaintained(attrs, entryDN, entryBaseDN);
      if (result.getResultCode() != ResultCode.SUCCESS)
      if (!result.continueProcessing())
      {
        return result;
      }

 opendj-server-legacy/src/test/java/org/opends/server/plugins/ReferentialIntegrityPluginTestCase.java

@@ -2302,4 +2302,98 @@
    final ModifyOperation multiModOperation = connection.processModify(modifyRequest);
    assertEquals(multiModOperation.getResultCode(), ResultCode.CONSTRAINT_VIOLATION);
  }

  /**
   * Issue #1173: two managed attributes, the first of which holds a reference to a missing entry, with the other one
   * holding a valid reference or none. Each attribute takes both places, so that one row puts the missing reference
   * after the attribute type the plugin checks first, whichever that is.
   */
  @DataProvider
  public Object[][] missingReferenceNextToAnother()
  {
    return new Object[][] {
      { "manager", "seeAlso" },
      { "seeAlso", "manager" },
      { "manager", null },
      { "seeAlso", null },
    };
  }

  /**
   * Issue #1173: an added entry is refused when any managed attribute type holds a reference to a missing entry, not
   * only when the first one the plugin checks does.
   */
  @Test(dataProvider = "missingReferenceNextToAnother")
  public void testEnforceIntegrityAddChecksEveryAttributeType(String missingRefAttr, String validRefAttr)
      throws Exception
  {
    enableCheckReferences("manager", "seeAlso");

    List<String> ldif = newArrayList(
        "dn: uid=employee,ou=people,ou=dept,dc=example,dc=com",
        "objectclass: top",
        "objectclass: person",
        "objectclass: organizationalperson",
        "objectclass: inetorgperson",
        "uid: employee",
        "cn: employee",
        "sn: employee",
        missingRefAttr + ": uid=bad,ou=people,ou=dept,dc=example,dc=com");
    if (validRefAttr != null)
    {
      ldif.add(validRefAttr + ": " + user1);
    }

    AddOperation addOperation = getRootConnection().processAdd(TestCaseUtils.makeEntry(ldif.toArray(new String[0])));
    assertEquals(addOperation.getResultCode(), ResultCode.CONSTRAINT_VIOLATION);
  }

  /**
   * Issue #1173: a modify request is refused when any of its modifications adds a reference to a missing entry, not
   * only when its first modification of a managed attribute does.
   */
  @Test(dataProvider = "missingReferenceNextToAnother")
  public void testEnforceIntegrityModifyChecksEveryModification(String missingRefAttr, String validRefAttr)
      throws Exception
  {
    enableCheckReferences("manager", "seeAlso");
    String employee = "uid=employee,ou=people,ou=dept,dc=example,dc=com";
    TestCaseUtils.addEntry(
        "dn: " + employee,
        "objectclass: top",
        "objectclass: person",
        "objectclass: organizationalperson",
        "objectclass: inetorgperson",
        "uid: employee",
        "cn: employee",
        "sn: employee");

    ModifyRequest modifyRequest = Requests.newModifyRequest(DN.valueOf(employee));
    // Without a valid managed modification first, a modification of an attribute the plugin does not manage.
    modifyRequest.addModification(REPLACE, validRefAttr != null ? validRefAttr : "description", user1);
    modifyRequest.addModification(ADD, missingRefAttr, "uid=bad,ou=people,ou=dept,dc=example,dc=com");

    ModifyOperation modOperation = getRootConnection().processModify(modifyRequest);
    assertEquals(modOperation.getResultCode(), ResultCode.CONSTRAINT_VIOLATION);
  }

  /**
   * Enables the plugin with {@code check-references} for the given attribute types below {@code dc=example,dc=com}.
   */
  private void enableCheckReferences(String... attributeTypes)
  {
    assertEquals(replaceAttrEntry(configDN, "ds-cfg-enabled", "false").getResultCode(), ResultCode.SUCCESS);
    assertEquals(replaceAttrEntry(configDN, dsConfigPluginType,
                               "postoperationdelete",
                               "postoperationmodifydn",
                               "subordinatemodifydn",
                               "subordinatedelete",
                               "preoperationadd",
                               "preoperationmodify").getResultCode(), ResultCode.SUCCESS);
    assertEquals(addAttrEntry(configDN, dsConfigBaseDN, "dc=example,dc=com").getResultCode(), ResultCode.SUCCESS);
    assertEquals(replaceAttrEntry(configDN, dsConfigEnforceIntegrity, "true").getResultCode(), ResultCode.SUCCESS);
    assertEquals(replaceAttrEntry(configDN, dsConfigAttrType, (Object[]) attributeTypes).getResultCode(),
        ResultCode.SUCCESS);
    assertEquals(replaceAttrEntry(configDN, "ds-cfg-enabled", "true").getResultCode(), ResultCode.SUCCESS);
  }
}