fix(auth): skip AD continuation referrals in LDAP search (0.2.1)
Active Directory answers any search based at a domain root with continuation references (CN=Configuration, DC=ForestDnsZones, ...). With referral following disabled -- the default, and what we want, since chasing one would re-bind somewhere we never authenticated -- Novell surfaces each by throwing LdapReferralException from Next(). That escaped NovellLdapConnection.Search, and LdapAuthService maps a failed search to ServiceAccountBindFailed. So every login against SearchBase = DC=nam,DC=zimmer,DC=com failed system-side on BOTH DCs in ~546 ms while the directory, the credentials and the service account were all healthy. Identical on each endpoint, so failover could not help either. A continuation reference is a pointer to another naming context, never a statement that this search failed, so the loop now skips them and continues -- returning exactly the entries in the naming context we bound to. Extracted as ReadEntries(ILdapSearchResults) so the handling is unit-testable through the Novell seam without a live directory. A 1000-referral cap guards a client that throws without advancing, which would otherwise spin forever inside a fail-closed auth path. Non-referral faults still propagate untouched: swallowing those would turn "the directory is broken" into "no such user", which is the wrong answer to give about someone's credentials. 7 tests, 6 verified red against the pre-fix code; the seventh is the non-referral control that must stay green either way. Suite 257/0, 2 skipped. Live-proven against the ZB NAM domain root before and after.
This commit is contained in:
@@ -5,7 +5,10 @@
|
||||
<Nullable>enable</Nullable>
|
||||
<ImplicitUsings>enable</ImplicitUsings>
|
||||
<LangVersion>latest</LangVersion>
|
||||
<Version>0.2.0</Version>
|
||||
<!-- 0.2.1: NovellLdapConnection.Search now skips AD continuation referrals instead of
|
||||
letting them fail the search stage. Without it, a SearchBase at a domain root made
|
||||
every login fail as ServiceAccountBindFailed against a healthy directory. -->
|
||||
<Version>0.2.1</Version>
|
||||
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
|
||||
</PropertyGroup>
|
||||
|
||||
|
||||
@@ -99,23 +99,73 @@ internal sealed class NovellLdapConnection : ILdapConnection
|
||||
attributes.ToArray(),
|
||||
typesOnly: false);
|
||||
|
||||
return ReadEntries(results);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Drains a result set into entries, <b>skipping continuation referrals</b>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// Active Directory answers any search based at a domain root with one or more
|
||||
/// continuation references (to <c>CN=Configuration</c>, <c>DC=ForestDnsZones</c> and
|
||||
/// friends). With referral following disabled — the default, and what we want, since
|
||||
/// chasing a referral would re-bind somewhere we did not authenticate — Novell surfaces
|
||||
/// each one by throwing <see cref="LdapReferralException"/> from <see cref="ILdapSearchResults.Next"/>.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// A continuation reference is a <em>pointer to another naming context</em>, not an error:
|
||||
/// it says "there may be more results over there", never that this search failed. Letting
|
||||
/// it escape made the whole search stage fail, which <c>LdapAuthService</c> maps to
|
||||
/// <see cref="Abstractions.Ldap.LdapAuthFailure.ServiceAccountBindFailed"/> — so against a
|
||||
/// real AD domain root, every login failed with a system-side error while the directory,
|
||||
/// the credentials and both DCs were perfectly healthy. Verified against
|
||||
/// <c>DC=nam,DC=zimmer,DC=com</c> on 2026-08-12.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Skipping is the correct behaviour rather than a workaround: the entries we can see are
|
||||
/// exactly the ones in the naming context we bound to, which is the set the caller asked
|
||||
/// about. Internal so the referral handling can be unit-tested through the
|
||||
/// <see cref="ILdapSearchResults"/> seam without a live directory.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
internal static List<LdapSearchEntry> ReadEntries(ILdapSearchResults results)
|
||||
{
|
||||
var entries = new List<LdapSearchEntry>();
|
||||
while (results.HasMore())
|
||||
var referrals = 0;
|
||||
|
||||
while (true)
|
||||
{
|
||||
var entry = results.Next();
|
||||
var attrs = new Dictionary<string, IReadOnlyList<string>>(StringComparer.OrdinalIgnoreCase);
|
||||
|
||||
foreach (LdapAttribute attr in entry.GetAttributeSet())
|
||||
try
|
||||
{
|
||||
attrs[attr.Name] = attr.StringValueArray.ToList();
|
||||
}
|
||||
if (!results.HasMore()) break;
|
||||
|
||||
entries.Add(new LdapSearchEntry(entry.Dn, attrs));
|
||||
var entry = results.Next();
|
||||
var attrs = new Dictionary<string, IReadOnlyList<string>>(StringComparer.OrdinalIgnoreCase);
|
||||
|
||||
foreach (LdapAttribute attr in entry.GetAttributeSet())
|
||||
{
|
||||
attrs[attr.Name] = attr.StringValueArray.ToList();
|
||||
}
|
||||
|
||||
entries.Add(new LdapSearchEntry(entry.Dn, attrs));
|
||||
}
|
||||
catch (LdapException ex) when (ex is LdapReferralException || ex.ResultCode == LdapException.Referral)
|
||||
{
|
||||
// Novell dequeues the reference before throwing, so the enumeration advances and
|
||||
// this terminates. The cap is a belt-and-braces guard: a client that threw without
|
||||
// advancing would otherwise spin forever inside a fail-closed auth path, which is a
|
||||
// far worse failure than under-reporting an implausible number of referrals.
|
||||
if (++referrals > MaxReferrals) break;
|
||||
}
|
||||
}
|
||||
|
||||
return entries;
|
||||
}
|
||||
|
||||
/// <summary>Upper bound on continuation references tolerated in one search — see <see cref="ReadEntries"/>.</summary>
|
||||
private const int MaxReferrals = 1_000;
|
||||
|
||||
/// <inheritdoc/>
|
||||
public void Dispose()
|
||||
{
|
||||
|
||||
@@ -0,0 +1,179 @@
|
||||
using Novell.Directory.Ldap;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.Auth.Ldap.Internal;
|
||||
|
||||
namespace ZB.MOM.WW.Auth.Ldap.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Covers <see cref="NovellLdapConnection.ReadEntries"/>' handling of AD continuation
|
||||
/// referrals. Active Directory returns one for any search based at a domain root, and
|
||||
/// Novell surfaces it by throwing from <c>Next()</c>. Before this was handled, the
|
||||
/// exception escaped the search stage and every login against
|
||||
/// <c>DC=nam,DC=zimmer,DC=com</c> failed as <c>ServiceAccountBindFailed</c> while the
|
||||
/// directory and credentials were healthy.
|
||||
/// </summary>
|
||||
public sealed class NovellSearchReferralTests
|
||||
{
|
||||
[Fact]
|
||||
public void Referral_before_the_only_entry_does_not_lose_it()
|
||||
{
|
||||
var results = new ScriptedResults(
|
||||
Step.Referral(),
|
||||
Step.Entry("CN=dohertj2,OU=Office Users,OU=Warsaw,DC=nam,DC=zimmer,DC=com"));
|
||||
|
||||
var entries = NovellLdapConnection.ReadEntries(results);
|
||||
|
||||
var entry = Assert.Single(entries);
|
||||
Assert.Equal("CN=dohertj2,OU=Office Users,OU=Warsaw,DC=nam,DC=zimmer,DC=com", entry.Dn);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Referral_after_the_entry_does_not_discard_what_was_already_read()
|
||||
{
|
||||
var results = new ScriptedResults(
|
||||
Step.Entry("CN=a,DC=nam,DC=zimmer,DC=com"),
|
||||
Step.Referral());
|
||||
|
||||
var entries = NovellLdapConnection.ReadEntries(results);
|
||||
|
||||
Assert.Equal("CN=a,DC=nam,DC=zimmer,DC=com", Assert.Single(entries).Dn);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The real shape observed against the NAM domain root: several references interleaved
|
||||
/// with the one user entry. All three of AD's usual continuation points are present.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Multiple_interleaved_referrals_still_yield_every_entry()
|
||||
{
|
||||
var results = new ScriptedResults(
|
||||
Step.Referral(),
|
||||
Step.Entry("CN=a,DC=nam,DC=zimmer,DC=com"),
|
||||
Step.Referral(),
|
||||
Step.Referral(),
|
||||
Step.Entry("CN=b,DC=nam,DC=zimmer,DC=com"));
|
||||
|
||||
var entries = NovellLdapConnection.ReadEntries(results);
|
||||
|
||||
Assert.Equal(["CN=a,DC=nam,DC=zimmer,DC=com", "CN=b,DC=nam,DC=zimmer,DC=com"],
|
||||
entries.Select(e => e.Dn));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A referral-only result set is a legitimate "nothing here" answer, and must come back
|
||||
/// empty rather than throwing — <c>LdapAuthService</c> reads empty as <c>UserNotFound</c>,
|
||||
/// which is the honest verdict, where an exception would become a system-side failure.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Referrals_only_returns_empty_rather_than_throwing()
|
||||
{
|
||||
var results = new ScriptedResults(Step.Referral(), Step.Referral());
|
||||
|
||||
Assert.Empty(NovellLdapConnection.ReadEntries(results));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A base <see cref="LdapException"/> carrying the Referral result code is treated the
|
||||
/// same as the derived type — the catch filter must not depend on the exact class.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Referral_result_code_on_a_base_exception_is_also_skipped()
|
||||
{
|
||||
var results = new ScriptedResults(
|
||||
Step.Throwing(new LdapException("referral", LdapException.Referral, null)),
|
||||
Step.Entry("CN=a,DC=nam,DC=zimmer,DC=com"));
|
||||
|
||||
Assert.Single(NovellLdapConnection.ReadEntries(results));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Only referrals are swallowed. A genuine directory fault must still propagate so the
|
||||
/// caller maps it to a system-side failure — silently returning a short result set would
|
||||
/// turn "the directory is broken" into "no such user", which is the wrong answer to give
|
||||
/// about someone's credentials.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void A_non_referral_fault_still_propagates()
|
||||
{
|
||||
var results = new ScriptedResults(
|
||||
Step.Throwing(new LdapException("busy", LdapException.Busy, null)),
|
||||
Step.Entry("CN=a,DC=nam,DC=zimmer,DC=com"));
|
||||
|
||||
var ex = Assert.Throws<LdapException>(() => NovellLdapConnection.ReadEntries(results));
|
||||
Assert.Equal(LdapException.Busy, ex.ResultCode);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A pathological client that throws a referral without advancing must not spin forever
|
||||
/// inside a fail-closed authentication path.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void An_endless_referral_stream_terminates()
|
||||
{
|
||||
var results = new EndlessReferrals();
|
||||
|
||||
var entries = NovellLdapConnection.ReadEntries(results);
|
||||
|
||||
Assert.Empty(entries);
|
||||
Assert.InRange(results.Calls, 1, 2_000);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
private sealed record Step(LdapEntry? Value, Exception? Throw)
|
||||
{
|
||||
public static Step Entry(string dn) => new(new LdapEntry(dn, new LdapAttributeSet()), null);
|
||||
public static Step Referral() => new(null, new LdapReferralException("continuation reference"));
|
||||
public static Step Throwing(Exception ex) => new(null, ex);
|
||||
}
|
||||
|
||||
/// <summary>Replays a fixed script of entries and faults through the Novell result seam.</summary>
|
||||
private sealed class ScriptedResults : ILdapSearchResults
|
||||
{
|
||||
private readonly Step[] _steps;
|
||||
private int _index;
|
||||
|
||||
public ScriptedResults(params Step[] steps) => _steps = steps;
|
||||
|
||||
public int Count => _steps.Length;
|
||||
|
||||
public LdapControl[] ResponseControls => [];
|
||||
|
||||
public bool HasMore() => _index < _steps.Length;
|
||||
|
||||
public LdapEntry Next()
|
||||
{
|
||||
var step = _steps[_index++];
|
||||
if (step.Throw is not null) throw step.Throw;
|
||||
return step.Value!;
|
||||
}
|
||||
|
||||
// The production reader drives HasMore()/Next() directly; enumeration is part of the
|
||||
// interface but not part of the path under test.
|
||||
public IEnumerator<LdapEntry> GetEnumerator() => throw new NotSupportedException();
|
||||
|
||||
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator();
|
||||
}
|
||||
|
||||
/// <summary>Always claims more results and always throws a referral, never advancing.</summary>
|
||||
private sealed class EndlessReferrals : ILdapSearchResults
|
||||
{
|
||||
public int Calls { get; private set; }
|
||||
|
||||
public int Count => int.MaxValue;
|
||||
|
||||
public LdapControl[] ResponseControls => [];
|
||||
|
||||
public bool HasMore() => true;
|
||||
|
||||
public LdapEntry Next()
|
||||
{
|
||||
Calls++;
|
||||
throw new LdapReferralException("continuation reference");
|
||||
}
|
||||
|
||||
public IEnumerator<LdapEntry> GetEnumerator() => throw new NotSupportedException();
|
||||
|
||||
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user