Fix AttributeValuePasswordValidator: inverted substring logic and missing r...

Copilot

4 days ago 3fa69cb84f92e9f4ff279a0c3230853f0a69135f

Fix AttributeValuePasswordValidator: inverted substring logic and missing reversed-password substring check (#599)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: vharseko <6818498+vharseko@users.noreply.github.com>
Co-authored-by: Valera V Harseko <vharseko@3a-systems.ru>

 opendj-server-legacy/src/main/java/org/opends/server/extensions/AttributeValuePasswordValidator.java

@@ -13,12 +13,15 @@
 *
 * Copyright 2008 Sun Microsystems, Inc.
 * Portions Copyright 2012-2016 ForgeRock AS.
 * Portions Copyright 2026 3A Systems, LLC.
 */
package org.opends.server.extensions;

import org.forgerock.i18n.LocalizableMessage;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Set;

import org.forgerock.opendj.config.server.ConfigurationChangeListener;
@@ -81,17 +84,28 @@
  private boolean containsSubstring(String password, int minSubstringLength,
      Attribute a)
  {
    // Clamp to at least 1 so an empty substring never matches unconditionally.
    final int minLen = Math.max(1, minSubstringLength);
    final int passwordLength = password.length();

    // Precompute the lowercase password once to avoid repeated conversions.
    final String passwordLower = password.toLowerCase(Locale.ROOT);

    // Precompute lowercase attribute values once, outside the substring loops.
    final List<String> attrValuesLower = new ArrayList<>(a.size());
    for (ByteString val : a)
    {
      attrValuesLower.add(val.toString().toLowerCase(Locale.ROOT));
    }

    for (int i = 0; i < passwordLength; i++)
    {
      for (int j = i + minSubstringLength; j <= passwordLength; j++)
      for (int j = i + minLen; j <= passwordLength; j++)
      {
        Attribute substring = Attributes.create(a.getAttributeDescription().getAttributeType(),
            password.substring(i, j));
        for (ByteString val : a)
        final String pwdSubstring = passwordLower.substring(i, j);
        for (String attrValueLower : attrValuesLower)
        {
          if (substring.contains(val))
          if (attrValueLower.contains(pwdSubstring))
          {
            return true;
          }
@@ -141,7 +155,8 @@
        if (a.contains(vf) ||
            (config.isTestReversedPassword() && a.contains(vr)) ||
            (config.isCheckSubstrings() &&
                containsSubstring(password, minSubstringLength, a)))
                (containsSubstring(password, minSubstringLength, a) ||
                 (config.isTestReversedPassword() && containsSubstring(reversed, minSubstringLength, a)))))
        {
          invalidReason.append(ERR_ATTRVALUE_VALIDATOR_PASSWORD_IN_ENTRY.get());
          return false;

 opendj-server-legacy/src/test/java/org/opends/server/backends/ChangelogBackendTestCase.java

@@ -585,6 +585,8 @@
    // write 4 changes starting from changenumber 1, and search them
    String testName = "Multiple/1";
    CSN[] csns = generateAndPublishUpdateMsgForEachOperationType(testName, false);
    // Wait until changenumber 4 is visible before searching
    assertChangelogAttributesInRootDSE(1, 4);
    searchChangesForEachOperationTypeUsingChangeNumberMode(1, csns, testName);

    // write 4 more changes starting from changenumber 5, and search them
@@ -853,7 +855,7 @@
      final int expectedFirstChangeNumber, final int expectedLastChangeNumber) throws Exception
  {
    TestTimer timer = new TestTimer.Builder()
      .maxSleep(3, SECONDS)
      .maxSleep(30, SECONDS)
      .sleepTimes(100, MILLISECONDS)
      .toTimer();
    return timer.repeatUntilSuccess(new Callable<List<SearchResultEntry>>()

 opendj-server-legacy/src/test/java/org/opends/server/extensions/AttributeValuePasswordValidatorTestCase.java

@@ -13,6 +13,7 @@
 *
 * Copyright 2006-2008 Sun Microsystems, Inc.
 * Portions Copyright 2012-2016 ForgeRock AS.
 * Portions Copyright 2026 3A Systems, LLC.
 */
package org.opends.server.extensions;

@@ -371,6 +372,131 @@


  /**
   * Retrieves test data for substring and reversed-password substring checks
   * using a user entry with uid=USN123.
   *
   * @throws  Exception  If an unexpected problem occurs.
   */
  @DataProvider(name = "substringTestData")
  public Object[][] getSubstringTestData()
         throws Exception
  {
    Entry configEntry = TestCaseUtils.makeEntry(
         "dn: cn=Attribute Value,cn=Password Validators,cn=config",
         "objectClass: top",
         "objectClass: ds-cfg-password-validator",
         "objectClass: ds-cfg-attribute-value-password-validator",
         "cn: Attribute Value",
         "ds-cfg-java-class: org.opends.server.extensions." +
              "AttributeValuePasswordValidator",
         "ds-cfg-enabled: true",
         "ds-cfg-match-attribute: uid",
         "ds-cfg-check-substrings: true",
         "ds-cfg-min-substring-length: 3",
         "ds-cfg-test-reversed-password: true");

    return new Object[][]
    {
      // BLOCK: forward match "N12" in "USN123"
      new Object[] { configEntry, "USN123aa", false },
      // BLOCK: forward match "N12" in "USN123"
      new Object[] { configEntry, "aaUSN123", false },
      // BLOCK: forward match "123" in "USN123"
      new Object[] { configEntry, "U1sn123b", false },
      // BLOCK: reverse-password match "123" — reversed("NsU321ab")="ba123UsN" contains "123"
      new Object[] { configEntry, "NsU321ab", false },
      // BLOCK: forward match "N12" in "USN123"
      new Object[] { configEntry, "A9USN12z", false },
      // BLOCK: forward match "USN" in "USN123"
      new Object[] { configEntry, "xx123USN", false },
      // BLOCK: reverse-password match "USN" — reversed("NSU123xy")="yx321USN" contains "USN"
      new Object[] { configEntry, "NSU123xy", false },
      // BLOCK: forward match "N12" in "USN123"
      new Object[] { configEntry, "z9nUSN12", false },
      // BLOCK: reverse-password match "123" — reversed("usN321AA")="AA123Nsu" contains "123"
      new Object[] { configEntry, "usN321AA", false },
      // BLOCK: forward match "USN" in "USN123"
      new Object[] { configEntry, "1USN2abc", false },

      // PASS: no username substrings detected
      new Object[] { configEntry, "Sun3RiseA", true },
      // PASS: no username substrings detected
      new Object[] { configEntry, "Rock7fall", true },
      // PASS: no username substrings detected
      new Object[] { configEntry, "Tree9Bark", true },
      // PASS: no username substrings detected
      new Object[] { configEntry, "Wave4Deep", true },
      // PASS: no username substrings detected
      new Object[] { configEntry, "Glow5Star", true },
      // PASS: no username substrings detected
      new Object[] { configEntry, "Rain8Drop", true },
      // PASS: no username substrings detected
      new Object[] { configEntry, "Fire6Ash", true },
      // PASS: no username substrings detected
      new Object[] { configEntry, "Mist2Hill", true },
      // PASS: no username substrings detected
      new Object[] { configEntry, "Frog1Lake", true },
      // PASS: no username substrings detected
      new Object[] { configEntry, "Dust7Moon", true },
    };
  }



  /**
   * Tests substring and reversed-password substring checks against a user
   * entry with uid=USN123.
   *
   * @param  configEntry  The configuration entry to use for the password
   *                      validator.
   * @param  password     The password to test with the validator.
   * @param  acceptable   Indicates whether the provided password should be
   *                      considered acceptable.
   *
   * @throws  Exception  If an unexpected problem occurs.
   */
  @Test(dataProvider = "substringTestData")
  public void testSubstringPasswordIsAcceptable(Entry configEntry,
                                                String password,
                                                boolean acceptable)
         throws Exception
  {
    TestCaseUtils.initializeTestBackend(true);
    Entry userEntry = TestCaseUtils.makeEntry(
         "dn: uid=USN123,o=test",
         "objectClass: top",
         "objectClass: person",
         "objectClass: organizationalPerson",
         "objectClass: inetOrgPerson",
         "uid: USN123",
         "givenName: USN",
         "sn: 123",
         "cn: USN 123",
         "userPassword: doesntmatter");

    AttributeValuePasswordValidator validator = initializePasswordValidator(configEntry);

    ByteString pwOS = ByteString.valueOfUtf8(password);
    ArrayList<Modification> mods = CollectionUtils.newArrayList(
        new Modification(ModificationType.REPLACE, Attributes.create("userpassword", password)));

    ModifyOperationBasis modifyOperation =
         new ModifyOperationBasis(getRootConnection(), nextOperationID(), nextMessageID(),
                             new ArrayList<Control>(),
                             DN.valueOf("uid=USN123,o=test"), mods);

    LocalizableMessageBuilder invalidReason = new LocalizableMessageBuilder();
    assertEquals(validator.passwordIsAcceptable(pwOS,
                              new HashSet<ByteString>(0), modifyOperation,
                              userEntry, invalidReason),
                 acceptable, invalidReason.toString());

    validator.finalizePasswordValidator();
  }



  /**
   * Tests the {@code passwordIsAcceptable} method using the provided
   * information.
   *