Fix AttributeValuePasswordValidator: inverted substring logic and missing reversed-password substring check

[Copilot]

• Expand getSubstringTestData to include all 20 password test cases from the problem statement (10 BLOCK + 10 PASS)

Original prompt

Bug: AttributeValuePasswordValidator — substring check logic is inverted and reversed password is ignored for substrings

Issue: #566

Problem 1 — Inverted logic in containsSubstring

In AttributeValuePasswordValidator.java, the containsSubstring method has inverted logic:

Attribute substring = Attributes.create(a.getAttributeDescription().getAttributeType(),
    password.substring(i, j));
for (ByteString val : a)
{
  if (substring.contains(val))  // ← WRONG: checks if password-substring contains the attribute value
  {
    return true;
  }
}

What it currently does: checks whether a substring of the password (e.g. "USN") contains the full attribute value (e.g. "USN123") — which is almost always false.

What it should do: check whether the attribute value (e.g. "USN123") contains the substring of the password (e.g. "USN").

The fix should compare the password substring against each attribute value using case-insensitive string matching, e.g.:

for (ByteString val : a)
{
  String attrValue = val.toString().toLowerCase();
  String pwdSubstring = password.substring(i, j).toLowerCase();
  if (attrValue.contains(pwdSubstring))
  {
    return true;
  }
}

Problem 2 — test-reversed-password has no effect on substring checks

In passwordIsAcceptable, the reversed string is computed but never passed to containsSubstring:

final String reversed = new StringBuilder(password).reverse().toString();
// ...
(config.isCheckSubstrings() &&
    containsSubstring(password, minSubstringLength, a))  // reversed is not used here!

The fix should also call containsSubstring with the reversed password when isTestReversedPassword() is true:

(config.isCheckSubstrings() &&
    (containsSubstring(password, minSubstringLength, a) ||
     (config.isTestReversedPassword() && containsSubstring(reversed, minSubstringLength, a))))

Expected behaviour after fix

For uid=USN123, with check-substrings=true, min-substring-length=3, test-reversed-password=true:

• USN123aa → REJECTED (forward substring USN found in USN123)
• U1sn123b → REJECTED (forward substring 123 found in USN123)
• NsU321ab → REJECTED (reversed password substring 123 found in USN123)
• Sun3RiseA → ACCEPTED (no substrings of USN123 found)

Files to change

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

Please also update or add test cases in:

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

to cover:

1. Password containing a substring of the attribute value → should be rejected
2. Password containing a reversed substring of the attribute value → should be rejected
3. Password with no matching substrings → should be accepted

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: прокомментируй

Assistant: Баг подтверждён. Вот детальный анализ кода:

---

Баг подтверждён — и вот где именно он находится

Посмотрим на метод containsSubstring в AttributeValuePasswordValidator.java:

private boolean containsSubstring(String password, int minSubstringLength, Attribute a)
{
  final int passwordLength = password.length();

  for (int i = 0; i < passwordLength; i++)
  {
    for (int j = i + minSubstringLength; j <= passwordLength; j++)
    {
      Attribute substring = Attributes.create(..., password.substring(i, j));
      for (ByteString val : a)
      {
        if (substring.contains(val))  // ← БАГ ЗДЕСЬ
        {
          return true;
        }
      }
    }
  }
  return false;
}

В чём проблема

Логика перевёрнута:

substring.contains(val)

Это проверяет, содержит ли подстрока пароля значение атрибута — то есть спрашивает: «входит ли USN123 в подстроку USN?» — что всегда будет false, если подстрока пароля короче значения атрибута.

Должно быть наоборот:

val.toString().toLowerCase().contains(password.substring(i, j).toLowerCase())

Нужно проверять: «содержит ли значение атрибута (например, USN123) данную подстроку пароля (например, USN)?»

Также есть второй баг — test-reversed-password не работает для подстрок

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

📍 Connect Copilot coding agent with [Jira](https://gh.io/cca-jira-docs), [Azure Boards](https://gh.io/cca-azure-boards-docs) or [Linear](https://gh.io/cca-linear-docs) to delegate work to Copilot in one click without leaving your project management tool.

[Copilot]

Pull request overview

Fixes AttributeValuePasswordValidator substring matching so it correctly detects when attribute values contain password substrings (case-insensitive), and ensures the test-reversed-password option is applied to substring checks as well.

Changes:

• Correct substring comparison logic in containsSubstring (attribute value contains password substring, not the inverse).
• Apply reversed-password substring checks when ds-cfg-test-reversed-password is enabled.
• Expand unit tests to cover the full set of 20 substring/reversed-substring cases from the issue report.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
opendj-server-legacy/src/main/java/org/opends/server/extensions/AttributeValuePasswordValidator.java	Fixes inverted substring logic and adds reversed-password substring evaluation.
opendj-server-legacy/src/test/java/org/opends/server/extensions/AttributeValuePasswordValidatorTestCase.java	Adds a dedicated data provider + test covering 10 BLOCK and 10 PASS substring scenarios for uid=USN123.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

minSubstringLength can be 0 (the config property is an unconstrained integer), which makes j start at i and produces an empty pwdSubstring. Since every string contains "", this would cause containsSubstring to return true immediately and reject all passwords whenever substring checking is enabled. Consider clamping minSubstringLength to at least 1 (or treating non-positive values as “check whole password only”) before entering the loops.

[Copilot]

In containsSubstring, val.toString().toLowerCase(Locale.ROOT) is recomputed for every password substring, resulting in a lot of repeated UTF-8 decoding + lowercasing in the nested loops. This can be made significantly cheaper by precomputing lowercased attribute values once per Attribute (outside the i/j loops) and by lowercasing the password once up-front (e.g., passwordLower) and taking substrings from that.

[vharseko]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

Applied both fixes in commit 364094f:

1. Clamp minSubstringLength to ≥ 1 — Math.max(1, minSubstringLength) ensures an empty string can never match unconditionally when the config value is 0.
2. Precompute lowercase strings — the password is lowercased once upfront (passwordLower), and all attribute values are lowercased into a List<String> before entering the i/j loops, eliminating repeated UTF-8 decoding and toLowerCase calls from the innermost loop.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.