diff --git a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperation.java index 3ddefde30a..3b06726d72 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperation.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperation.java @@ -13,6 +13,7 @@ * * Copyright 2006-2009 Sun Microsystems, Inc. * Portions Copyright 2011-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.core; @@ -262,6 +263,14 @@ public interface SearchOperation extends Operation boolean returnEntry(Entry entry, List controls, boolean evaluateAci); + /** + * Indicates that the search phase is over and that any further entry comes from a persistent + * search. State kept to dereference aliases during the search phase is released, and no further + * entry is matched against it: a persistent search must report every change it is notified of, + * whether or not the entry was returned by the search phase. + */ + void endSearchPhase(); + /** * Used as a callback for backends to indicate that the provided search * reference was encountered during processing and that additional processing diff --git a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationBasis.java b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationBasis.java index 00a8074d5b..9b3bb83f23 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationBasis.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationBasis.java @@ -13,11 +13,12 @@ * * Copyright 2006-2010 Sun Microsystems, Inc. * Portions Copyright 2011-2016 ForgeRock AS. - * Portions Copyright 2024 3A Systems, LLC. + * Portions Copyright 2024-2026 3A Systems, LLC. */ package org.opends.server.core; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import org.forgerock.i18n.LocalizedIllegalArgumentException; @@ -449,11 +450,46 @@ public final boolean returnEntry(Entry entry, List controls) { return returnEntry(entry, controls, true); } - Set dereferenced=new HashSet<>(); + + /** + * The DNs of the entries already returned by the search phase. An alias may be dereferenced onto + * an entry which is in the scope of the search as well, and that entry must only be returned once. + * It is emptied once the search phase is over, because a persistent search must report every + * change it is notified of, whether or not the entry was returned before. + */ + private final Set returnedDNs = ConcurrentHashMap.newKeySet(); + + /** Whether the search phase is over and only persistent search notifications remain. */ + private volatile boolean searchPhaseOver; + + @Override + public final void endSearchPhase() + { + searchPhaseOver = true; + returnedDNs.clear(); + } @Override public final boolean returnEntry(Entry entry, List controls, boolean evaluateAci) + { + return returnEntry(entry, controls, evaluateAci, null); + } + + /** + * Returns the provided entry to the client, dereferencing it first if it is an alias and the + * deref policy asks for it. + * + * @param entry The entry to return. + * @param controls The controls to attach to the entry. + * @param evaluateAci Whether the access control handler must be consulted. + * @param aliasChain The DNs of the aliases already dereferenced on the way to this entry, or + * {@code null} if no alias was dereferenced yet. It only spans the current + * chain, so it cannot grow beyond the length of that chain. + * @return {@code true} if the search should continue, {@code false} if it should stop. + */ + private boolean returnEntry(Entry entry, List controls, + boolean evaluateAci, Set aliasChain) { boolean typesOnly = getTypesOnly(); @@ -566,22 +602,12 @@ else if (state.isDisabled()) //DereferenceAliasesPolicy if ( DereferenceAliasesPolicy.ALWAYS.equals(getDerefPolicy()) || DereferenceAliasesPolicy.IN_SEARCHING.equals(getDerefPolicy()) ) { if (entry.isAlias() && !baseDN.equals(entry.getName())) { - try { - final DN dn = entry.getAliasedDN(); - final Entry dereference = DirectoryServer.getEntry(dn); - if (dereferenced.contains(dn)) { - return true; - } - dereferenced.add(dn); - return returnEntry(dereference, controls, true); - } catch (DirectoryException e) { - throw new RuntimeException(e); - } + return returnAliasedEntry(entry, controls, aliasChain); } - if (dereferenced.contains(entry.getName())) { + if (!searchPhaseOver && !returnedDNs.add(entry.getName())) { + // This entry was already returned by the search, through an alias or on its own. return true; } - dereferenced.add(entry.getName()); } // Make a copy of the entry and pare it down to only include the set @@ -709,6 +735,55 @@ else if (state.isDisabled()) return pluginResult.continueProcessing(); } + /** + * Returns the entry the provided alias points to, in place of the alias itself. + * + * @param alias The alias entry to dereference. + * @param controls The controls to attach to the entry. + * @param aliasChain The DNs of the aliases already dereferenced on the way to this alias, or + * {@code null} if this alias is the first one of the chain. + * @return {@code true} if the search should continue, {@code false} if it should stop. + */ + private boolean returnAliasedEntry(Entry alias, List controls, Set aliasChain) + { + final DN aliasedDN; + final Entry aliasedEntry; + try + { + // getAliasedDN() parses the aliasedObjectName value, which throws the unchecked + // LocalizedIllegalArgumentException when that value is not a valid DN. + aliasedDN = alias.getAliasedDN(); + aliasedEntry = DirectoryServer.getEntry(aliasedDN); + } + catch (DirectoryException | LocalizedIllegalArgumentException e) + { + // A single alias which cannot be read must not fail the whole search. + logger.traceException(e); + return true; + } + + if (aliasedEntry == null) + { + // The alias points to an entry which does not exist: there is nothing to return for it. + return true; + } + if (!searchPhaseOver && returnedDNs.contains(aliasedDN)) + { + // The aliased entry was already returned by the search. + return true; + } + if (aliasChain == null) + { + aliasChain = new HashSet<>(); + } + if (!aliasChain.add(aliasedDN)) + { + // The aliases point at each other: stop before looping forever. + return true; + } + return returnEntry(aliasedEntry, controls, true, aliasChain); + } + private AccessControlHandler getACIHandler() { return AccessControlConfigManager.getInstance().getAccessControlHandler(); diff --git a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationWrapper.java b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationWrapper.java index 42afbd3710..5bc8617054 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationWrapper.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationWrapper.java @@ -13,6 +13,7 @@ * * Copyright 2008-2009 Sun Microsystems, Inc. * Portions Copyright 2011-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.core; @@ -57,6 +58,12 @@ public boolean returnEntry(Entry entry, List controls, return getOperation().returnEntry(entry, controls, evaluateAci); } + @Override + public void endSearchPhase() + { + getOperation().endSearchPhase(); + } + @Override public boolean returnReference(DN dn, SearchResultReference reference) { diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java index 8d9db9bdec..7b47d981b5 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java @@ -258,6 +258,9 @@ private void processSearch(AtomicBoolean executePostOpPlugins) throws CanceledOp // Process the search in the backend and all its subordinates. backend.search(this); } + + // Any entry returned from now on comes from the persistent search, if there is one. + endSearchPhase(); } catch (DirectoryException de) { diff --git a/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java b/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java index 75c2deffa8..b3360fe99b 100644 --- a/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java +++ b/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java @@ -15,21 +15,33 @@ */ package org.openidentityplatform.opendj; +import org.forgerock.opendj.config.client.ManagementContext; import org.forgerock.opendj.ldap.*; +import org.forgerock.opendj.ldap.controls.PersistentSearchChangeType; +import org.forgerock.opendj.ldap.controls.PersistentSearchRequestControl; import org.forgerock.opendj.ldap.requests.Requests; import org.forgerock.opendj.ldap.requests.SearchRequest; import org.forgerock.opendj.ldap.responses.SearchResultEntry; +import org.forgerock.opendj.ldap.responses.SearchResultReference; import org.forgerock.opendj.ldif.ConnectionEntryReader; +import org.forgerock.opendj.server.config.client.GlobalCfgClient; +import org.forgerock.opendj.server.config.meta.GlobalCfgDefn; import org.opends.server.DirectoryServerTestCase; import org.opends.server.TestCaseUtils; +import org.opends.server.api.LocalBackend; import org.opends.server.backends.MemoryBackend; +import org.opends.server.core.DirectoryServer; +import org.opends.server.types.AcceptRejectWarn; import org.opends.server.types.Entry; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; import java.util.HashMap; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; @@ -464,6 +476,220 @@ public void test_alias_target_without_backend() throws SearchResultReferenceIOEx } } + // An alias whose target lies outside the search scope: the target is only + // reachable by dereferencing, so it must be returned by the search. + @Test + public void test_deref_target_outside_scope() throws Exception { + TestCaseUtils.addEntries( + "dn: ou=pointers,o=test", + "objectClass: top", + "objectClass: organizationalUnit", + "ou: pointers", + "", + "dn: cn=ptr,ou=pointers,o=test", + "objectClass: alias", + "objectClass: top", + "objectClass: extensibleObject", + "cn: ptr", + "aliasedObjectName: cn=John Doe,o=MyCompany,o=test", + "" + ); + HashMap res = + search("ou=pointers,o=test", SearchScope.WHOLE_SUBTREE, DereferenceAliasesPolicy.ALWAYS); + + assertThat(res.containsKey("cn=John Doe,o=MyCompany,o=test")).isTrue(); + } + + // An alias pointing at an entry which does not exist is skipped, and the entries next to it are + // still returned: aliases are not checked when they are written, so a dangling one is expected. + @Test + public void test_dangling_alias() throws Exception { + TestCaseUtils.addEntries( + "dn: ou=dangling,o=test", + "objectClass: top", + "objectClass: organizationalUnit", + "ou: dangling", + "", + "dn: cn=broken,ou=dangling,o=test", + "objectClass: alias", + "objectClass: top", + "objectClass: extensibleObject", + "cn: broken", + "aliasedObjectName: cn=does-not-exist,ou=dangling,o=test", + "", + "dn: cn=intact,ou=dangling,o=test", + "cn: intact", + "sn: intact", + "objectClass: person", + "" + ); + HashMap res = + search("ou=dangling,o=test", SearchScope.WHOLE_SUBTREE, DereferenceAliasesPolicy.ALWAYS); + + assertThat(res.containsKey("cn=does-not-exist,ou=dangling,o=test")).isFalse(); + assertThat(res.containsKey("ou=dangling,o=test")).isTrue(); + assertThat(res.containsKey("cn=intact,ou=dangling,o=test")).isTrue(); + } + + // A persistent search reports every change it is notified of. Dereferencing aliases must not + // make it drop a change because the entry was notified before. + @Test + public void test_persistent_search_reports_repeated_changes() throws Exception { + TestCaseUtils.addEntries( + "dn: ou=psearch,o=test", + "objectClass: top", + "objectClass: organizationalUnit", + "ou: psearch", + "", + "dn: cn=changing,ou=psearch,o=test", + "cn: changing", + "sn: changing", + "objectClass: person", + "" + ); + + final BlockingQueue notified = new LinkedBlockingQueue<>(); + final SearchRequest request = + Requests.newSearchRequest("ou=psearch,o=test", SearchScope.WHOLE_SUBTREE, "(objectclass=*)") + .setDereferenceAliasesPolicy(DereferenceAliasesPolicy.ALWAYS) + .addControl(PersistentSearchRequestControl.newControl( + true, true, false, PersistentSearchChangeType.MODIFY)); + + final LDAPConnectionFactory factory = + new LDAPConnectionFactory("localhost", TestCaseUtils.getServerLdapPort()); + try (Connection psearch = factory.getConnection()) { + psearch.bind("cn=Directory Manager", "password".toCharArray()); + psearch.searchAsync(request, new SearchResultHandler() { + @Override + public boolean handleEntry(SearchResultEntry entry) { + notified.add(entry.getName().toString()); + return true; + } + + @Override + public boolean handleReference(SearchResultReference reference) { + return true; + } + }); + + // searchAsync returns before the server has registered the persistent search, so wait + // until the backend reports it; otherwise the first modification below can be notified + // before the search is listening and be missed. + final LocalBackend backend = TestCaseUtils.getServerContext() + .getBackendConfigManager().getLocalBackendById(TestCaseUtils.TEST_BACKEND_ID); + for (int i = 0; backend.getPersistentSearches().isEmpty() && i < 500; i++) { + Thread.sleep(10); + } + assertThat(backend.getPersistentSearches()).isNotEmpty(); + + // The same entry is modified repeatedly: each change must reach the persistent search. + for (int i = 1; i <= 3; i++) { + connection.modify(Requests.newModifyRequest("cn=changing,ou=psearch,o=test") + .addModification(ModificationType.REPLACE, "description", "change " + i)); + assertThat(notified.poll(30, TimeUnit.SECONDS)) + .as("notification for change " + i) + .isEqualTo("cn=changing,ou=psearch,o=test"); + } + } + } + + // An alias is dereferenced before its target is reached on its own: the target must still be + // returned, and exactly once. The original regression was order-sensitive, dropping the target + // when the alias reached it first, so this pins the alias-before-target order specifically. The + // MemoryBackend serving o=test walks entries in insertion order, so listing the alias before the + // target below guarantees the alias is visited first. + @Test + public void test_deref_alias_before_target_returns_target_once() throws Exception { + TestCaseUtils.addEntries( + "dn: ou=order,o=test", + "objectClass: top", + "objectClass: organizationalUnit", + "ou: order", + "", + "dn: cn=aaa-alias,ou=order,o=test", + "objectClass: alias", + "objectClass: top", + "objectClass: extensibleObject", + "cn: aaa-alias", + "aliasedObjectName: cn=zzz-target,ou=order,o=test", + "", + "dn: cn=zzz-target,ou=order,o=test", + "cn: zzz-target", + "sn: target", + "objectClass: person", + "" + ); + // search() asserts every returned DN is unique, so a duplicated target would fail this call. + HashMap res = + search("ou=order,o=test", SearchScope.WHOLE_SUBTREE, DereferenceAliasesPolicy.ALWAYS); + + // The target is returned, reached through the alias visited before it ... + assertThat(res.containsKey("cn=zzz-target,ou=order,o=test")).isTrue(); + // ... and the alias entry itself is replaced by its target, never returned as-is. + assertThat(res.containsKey("cn=aaa-alias,ou=order,o=test")).isFalse(); + } + + // An alias whose aliasedObjectName is not a valid DN cannot be dereferenced. Reading it throws a + // DirectoryException, which must be logged and the alias skipped, not turned into a failed search. + @Test + public void test_alias_with_malformed_target_is_skipped() throws Exception { + // A malformed DN can only be stored while syntax enforcement is relaxed; the default REJECT + // policy refuses the add outright. + final AcceptRejectWarn previous = DirectoryServer.getCoreConfigManager().getSyntaxEnforcementPolicy(); + setSyntaxEnforcementPolicy(GlobalCfgDefn.InvalidAttributeSyntaxBehavior.WARN); + try { + TestCaseUtils.addEntries( + "dn: ou=malformed,o=test", + "objectClass: top", + "objectClass: organizationalUnit", + "ou: malformed", + "", + "dn: cn=badptr,ou=malformed,o=test", + "objectClass: alias", + "objectClass: top", + "objectClass: extensibleObject", + "cn: badptr", + "aliasedObjectName: not a valid dn", + "", + "dn: cn=neighbour,ou=malformed,o=test", + "cn: neighbour", + "sn: neighbour", + "objectClass: person", + "" + ); + } finally { + setSyntaxEnforcementPolicy(asSyntaxBehavior(previous)); + } + + // The whole search still succeeds: the broken alias is skipped, its neighbours returned. + HashMap res = + search("ou=malformed,o=test", SearchScope.WHOLE_SUBTREE, DereferenceAliasesPolicy.ALWAYS); + + assertThat(res.containsKey("cn=badptr,ou=malformed,o=test")).isFalse(); + assertThat(res.containsKey("ou=malformed,o=test")).isTrue(); + assertThat(res.containsKey("cn=neighbour,ou=malformed,o=test")).isTrue(); + } + + private static void setSyntaxEnforcementPolicy(GlobalCfgDefn.InvalidAttributeSyntaxBehavior value) throws Exception { + try (ManagementContext conf = TestCaseUtils.getServer().getConfiguration()) { + GlobalCfgClient globalCfg = conf.getRootConfiguration().getGlobalConfiguration(); + globalCfg.setInvalidAttributeSyntaxBehavior(value); + globalCfg.commit(); + } + } + + private static GlobalCfgDefn.InvalidAttributeSyntaxBehavior asSyntaxBehavior(AcceptRejectWarn policy) { + switch (policy) { + case ACCEPT: + return GlobalCfgDefn.InvalidAttributeSyntaxBehavior.ACCEPT; + case WARN: + return GlobalCfgDefn.InvalidAttributeSyntaxBehavior.WARN; + case REJECT: + default: + return GlobalCfgDefn.InvalidAttributeSyntaxBehavior.REJECT; + } + } + @Test(expectedExceptions = LdapException.class) public void test_stackoverflow() throws Exception {