From 174ad952dcbf61979b008e702647fc2af14332d8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 16:37:14 -0400 Subject: [PATCH] feat(ldap): endpoint walk with sticky-preference failover in LdapAuthService --- .../ZB.MOM.WW.Auth.Ldap/LdapAuthService.cs | 161 ++++++++++-- .../LdapAuthServiceFailoverTests.cs | 237 ++++++++++++++++++ ...26-08-10-ldap-backup-dc-failover-design.md | 7 + 3 files changed, 385 insertions(+), 20 deletions(-) create mode 100644 ZB.MOM.WW.Auth/tests/ZB.MOM.WW.Auth.Ldap.Tests/LdapAuthServiceFailoverTests.cs diff --git a/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.Ldap/LdapAuthService.cs b/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.Ldap/LdapAuthService.cs index 892c173..0f7aa86 100644 --- a/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.Ldap/LdapAuthService.cs +++ b/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.Ldap/LdapAuthService.cs @@ -11,6 +11,7 @@ namespace ZB.MOM.WW.Auth.Ldap; /// so the logic is unit-testable without a live server. /// /// +/// /// Fully fail-closed: authentication never throws to the caller — every error, expected /// or unexpected, is mapped to a structured . /// A success is only returned for a user that resolved to exactly one entry, whose password @@ -18,11 +19,37 @@ namespace ZB.MOM.WW.Auth.Ldap; /// Service-account bind failures () are /// kept distinct from end-user bind failures () so /// a system misconfiguration is not mistaken for bad user input. +/// +/// +/// When is non-empty the flow above runs against +/// an ordered endpoint list (primary first, then fallbacks in declared order) and moves on +/// to the next endpoint ONLY on a system-side failure — connect/TLS, service-account bind, +/// or search — i.e. when the endpoint gave no authoritative answer. Any answer from a healthy +/// directory (success, or a user-side failure such as +/// / ) +/// is final and is never retried elsewhere: retrying a rejected password against a second DC +/// would turn one bad-password event into several, tripping account lockouts. Endpoint +/// preference is sticky — the last endpoint that answered is tried first next time — so a +/// primary outage costs its connect timeout once, not once per login. +/// +/// +/// The service is safe to share (it is registered as a singleton). The sticky index is the +/// only mutable state; concurrent logins may race on it, which is benign — the worst outcome +/// is one login starting at a stale endpoint and walking on. +/// /// public sealed class LdapAuthService : ILdapAuthService { private readonly LdapOptions _options; private readonly ILdapConnectionFactory _connectionFactory; + private readonly IReadOnlyList _endpoints; + + /// + /// Index into of the endpoint tried first. Written on any + /// authoritative answer from a non-preferred endpoint. Accessed via + /// only; benign races are accepted (see the type remarks). + /// + private int _preferredIndex; /// /// Production constructor: binds against a live directory via the real @@ -46,6 +73,25 @@ public sealed class LdapAuthService : ILdapAuthService { _options = options; _connectionFactory = connectionFactory; + _endpoints = BuildEndpoints(options); + } + + /// + /// Primary first, then fallbacks in declared order. The validator rejects malformed + /// entries at boot; unparseable entries are skipped defensively here so a service + /// constructed without options validation degrades to the parseable subset rather + /// than throwing from a constructor. + /// + private static IReadOnlyList BuildEndpoints(LdapOptions options) + { + var endpoints = new List { new(options.Server, options.Port) }; + foreach (var entry in options.FallbackServers) + { + if (LdapEndpointParser.TryParse(entry, options.Port, out var endpoint, out _)) + endpoints.Add(endpoint); + } + + return endpoints; } /// @@ -54,7 +100,10 @@ public sealed class LdapAuthService : ILdapAuthService /// bind-then-search-then-bind flow is wrapped so that any error — expected or unexpected — /// is mapped to a structured . A /// Succeeded == true result is only ever returned when the user resolved to exactly - /// one entry, their password verified, AND at least one group was extracted. + /// one entry, their password verified, AND at least one group was extracted. With + /// configured the flow may run against more than + /// one endpoint before answering — see the type-level remarks for the walk and its + /// system-side-only failover rule. /// public Task AuthenticateAsync(string username, string password, CancellationToken ct) { @@ -80,25 +129,87 @@ public sealed class LdapAuthService : ILdapAuthService // filter and the returned result (avoids two identities for one person). username = username.Trim(); + // 4. Walk the endpoint list, preferred one first. A single-endpoint configuration + // (no FallbackServers) collapses to exactly one attempt — unchanged behaviour. + var preferred = Volatile.Read(ref _preferredIndex); + if ((uint)preferred >= (uint)_endpoints.Count) + preferred = 0; // defensive: the list is immutable, but never index out of it + + // Seeded so an exhausted walk still returns the conservative system-side bucket + // even in the impossible case of an empty endpoint list. + var lastSystemSideFailure = LdapAuthResult.Fail(LdapAuthFailure.ServiceAccountBindFailed); + + foreach (var index in WalkOrder(_endpoints.Count, preferred)) + { + var result = AuthenticateAgainst(_endpoints[index], username, password); + + // ServiceAccountBindFailed is the (sole) system-side bucket: this endpoint gave + // no authoritative answer, so try the next one. Every other outcome — success, or + // a user-side failure like UserNotFound/BadCredentials — came from a healthy + // directory and is FINAL: re-asking a second DC about a rejected password would + // multiply one bad-password event into several against the account lockout policy. + if (result.Failure == LdapAuthFailure.ServiceAccountBindFailed) + { + lastSystemSideFailure = result; + continue; + } + + // Sticky preference: any authoritative answer proves this endpoint healthy. + // Updating only on Succeeded would re-pay the dead primary's connect timeout on + // every mistyped password for the whole outage. + if (index != preferred) + Volatile.Write(ref _preferredIndex, index); + + return result; + } + + // Every endpoint failed system-side. The preference is deliberately left where it was: + // a total outage says nothing about which endpoint to prefer once one recovers. + return lastSystemSideFailure; + } + + /// Preferred endpoint first, then every other endpoint in declared order. + private static IEnumerable WalkOrder(int count, int preferred) + { + yield return preferred; + for (var i = 0; i < count; i++) + { + if (i != preferred) + yield return i; + } + } + + /// + /// Runs one full bind-then-search-then-bind attempt against a single endpoint on a fresh + /// connection. The fail-closed contract holds per attempt: this never throws, and it + /// returns for — and only for — + /// system-side faults, which is what the caller reads as "no authoritative answer, try + /// the next endpoint". + /// + private LdapAuthResult AuthenticateAgainst(LdapEndpoint endpoint, string username, string password) + { // The whole flow runs inside an outer fail-closed guard: a StageFailure carries an // already-mapped failure out of a stage, and any OTHER unexpected exception defaults to // the most conservative system-side bucket. Either way the caller gets a structured result. try { + // A fresh connection per attempt: a Novell connection that has failed to connect + // or bind is not safely reusable against another host. using var conn = _connectionFactory.Create(); - // 4. Open the connection (transport/TLS handling lives in the adapter). The + // 5. Open the connection (transport/TLS handling lives in the adapter). The // per-operation timeout (ConnectionTimeoutMs) is applied by the adapter here. - // A failure to connect/upgrade means the directory is unreachable — a - // system-side fault, not the user's, so map it to ServiceAccountBindFailed. + // A failure to connect/upgrade means this endpoint is unreachable — a + // system-side fault, not the user's, so map it to ServiceAccountBindFailed + // and let the caller try the next endpoint. // NOTE: the LdapAuthFailure enum has no dedicated DirectoryUnavailable value; // ServiceAccountBindFailed is the closest system-side bucket. A future // Abstractions change could add DirectoryUnavailable to disambiguate. try { conn.Connect( - _options.Server, - _options.Port, + endpoint.Host, + endpoint.Port, _options.Transport, _options.AllowInsecure, _options.ConnectionTimeoutMs, @@ -106,10 +217,10 @@ public sealed class LdapAuthService : ILdapAuthService } catch (LdapException) { - throw new StageFailure(LdapAuthFailure.ServiceAccountBindFailed); + throw new StageFailure(LdapAuthFailure.ServiceAccountBindFailed, "connect"); } - // 5. Service-account bind so we can search for the user's DN. A bind failure + // 6. Service-account bind so we can search for the user's DN. A bind failure // here is a service-account misconfiguration — DISTINCT from a user-credential // failure — so it maps to ServiceAccountBindFailed. try @@ -118,10 +229,10 @@ public sealed class LdapAuthService : ILdapAuthService } catch (LdapException) { - throw new StageFailure(LdapAuthFailure.ServiceAccountBindFailed); + throw new StageFailure(LdapAuthFailure.ServiceAccountBindFailed, "service-bind"); } - // 6. Search for the user entry by the configured username attribute. A search + // 7. Search for the user entry by the configured username attribute. A search // failure is infrastructure (directory unreachable / unhealthy) — system-side. // NOTE: same enum limitation as Connect above; ServiceAccountBindFailed is the // closest system-side bucket until a DirectoryUnavailable value exists. @@ -133,10 +244,11 @@ public sealed class LdapAuthService : ILdapAuthService } catch (LdapException) { - throw new StageFailure(LdapAuthFailure.ServiceAccountBindFailed); + throw new StageFailure(LdapAuthFailure.ServiceAccountBindFailed, "search"); } - // 7. Require exactly one match. Zero -> UserNotFound; two or more -> AmbiguousUser. + // 8. Require exactly one match. Zero -> UserNotFound; two or more -> AmbiguousUser. + // Both are authoritative answers from a healthy directory, so they end the walk. // We never attempt a user bind against an ambiguous DN. if (entries.Count == 0) return LdapAuthResult.Fail(LdapAuthFailure.UserNotFound); @@ -145,19 +257,21 @@ public sealed class LdapAuthService : ILdapAuthService var entry = entries[0]; - // 8. User bind: re-bind as the resolved DN to verify the password. A bind failure - // here is the end user's bad credentials. + // 9. User bind: re-bind as the resolved DN to verify the password. A bind failure + // here is the end user's bad credentials — the directory answered, so this is + // returned directly and never retried against another endpoint. try { conn.Bind(entry.Dn, password); } catch (LdapException) { - throw new StageFailure(LdapAuthFailure.BadCredentials); + return LdapAuthResult.Fail(LdapAuthFailure.BadCredentials); } - // 9. Group extraction. Fail closed: an empty/missing group set, or any error while - // extracting groups, is a GroupLookupFailed — never a zero-group success. + // 10. Group extraction. Fail closed: an empty/missing group set, or any error while + // extracting groups, is a GroupLookupFailed — never a zero-group success. This + // too is an answer about the user, not about the endpoint, so it ends the walk. IReadOnlyList groups; try { @@ -165,13 +279,13 @@ public sealed class LdapAuthService : ILdapAuthService } catch { - throw new StageFailure(LdapAuthFailure.GroupLookupFailed); + return LdapAuthResult.Fail(LdapAuthFailure.GroupLookupFailed); } if (groups.Count == 0) return LdapAuthResult.Fail(LdapAuthFailure.GroupLookupFailed); - // 10. Success — and only here, with a verified password and >= 1 group. + // 11. Success — and only here, with a verified password and >= 1 group. var displayName = ExtractDisplayName(entry, username); return LdapAuthResult.Success(username, displayName, groups); } @@ -194,9 +308,16 @@ public sealed class LdapAuthService : ILdapAuthService /// private sealed class StageFailure : Exception { - public StageFailure(LdapAuthFailure failure) => Failure = failure; + public StageFailure(LdapAuthFailure failure, string stage) + { + Failure = failure; + Stage = stage; + } public LdapAuthFailure Failure { get; } + + /// Which stage faulted (connect / service-bind / search), for diagnostics. + public string Stage { get; } } /// diff --git a/ZB.MOM.WW.Auth/tests/ZB.MOM.WW.Auth.Ldap.Tests/LdapAuthServiceFailoverTests.cs b/ZB.MOM.WW.Auth/tests/ZB.MOM.WW.Auth.Ldap.Tests/LdapAuthServiceFailoverTests.cs new file mode 100644 index 0000000..7b01a7e --- /dev/null +++ b/ZB.MOM.WW.Auth/tests/ZB.MOM.WW.Auth.Ldap.Tests/LdapAuthServiceFailoverTests.cs @@ -0,0 +1,237 @@ +using ZB.MOM.WW.Auth.Abstractions.Ldap; + +namespace ZB.MOM.WW.Auth.Ldap.Tests; + +/// +/// Covers the multi-endpoint failover walk: which failures fail over (system-side only), +/// which are final (any authoritative answer from a healthy directory), how the sticky +/// endpoint preference moves, and that single-endpoint behaviour is unchanged. +/// +public class LdapAuthServiceFailoverTests +{ + private static LdapOptions Opts(params string[] fallbacks) => new() + { + Enabled = true, + Server = "dc1", + Port = 636, + Transport = LdapTransport.Ldaps, + SearchBase = "dc=x", + ServiceAccountDn = "cn=svc,dc=x", + ServiceAccountPassword = "svcpw", + FallbackServers = fallbacks, + }; + + private static FakeLdapConnection HealthyWithUser() => + new FakeLdapConnection().WithUserEntry("cn=alice,dc=x", new[] { "cn=ops,dc=x" }); + + // ---- failover triggers (system-side) ---- + + [Fact] + public async Task ConnectFailure_FailsOverToFallback_AndSucceeds() + { + var factory = new SequenceLdapConnectionFactory( + new FakeLdapConnection().ThrowOnConnect(), HealthyWithUser()); + var svc = new LdapAuthService(Opts("dc2:3894"), factory); + + var result = await svc.AuthenticateAsync("alice", "pw", CancellationToken.None); + + Assert.True(result.Succeeded); + Assert.Equal(2, factory.Created.Count); + Assert.Equal("dc1", factory.Created[0].ConnectArgs!.Value.Host); + Assert.Equal("dc2", factory.Created[1].ConnectArgs!.Value.Host); + Assert.Equal(3894, factory.Created[1].ConnectArgs!.Value.Port); + } + + [Fact] + public async Task ServiceBindFailure_FailsOverToFallback() + { + var factory = new SequenceLdapConnectionFactory( + new FakeLdapConnection().ThrowOnServiceBind(), HealthyWithUser()); + var svc = new LdapAuthService(Opts("dc2"), factory); + + var result = await svc.AuthenticateAsync("alice", "pw", CancellationToken.None); + + Assert.True(result.Succeeded); + Assert.Equal(636, factory.Created[1].ConnectArgs!.Value.Port); // bare host adopts Port + } + + // ---- user-side results NEVER fail over ---- + + [Fact] + public async Task BadUserCredentials_DoesNotFailOver() + { + var factory = new SequenceLdapConnectionFactory( + HealthyWithUser().ThrowOnUserBind()); + var svc = new LdapAuthService(Opts("dc2"), factory); + + var result = await svc.AuthenticateAsync("alice", "wrong", CancellationToken.None); + + Assert.Equal(LdapAuthFailure.BadCredentials, result.Failure); + Assert.Single(factory.Created); + } + + [Fact] + public async Task UserNotFound_DoesNotFailOver() + { + var factory = new SequenceLdapConnectionFactory(new FakeLdapConnection()); + var svc = new LdapAuthService(Opts("dc2"), factory); + + var result = await svc.AuthenticateAsync("ghost", "pw", CancellationToken.None); + + Assert.Equal(LdapAuthFailure.UserNotFound, result.Failure); + Assert.Single(factory.Created); + } + + [Fact] + public async Task GroupLookupFailed_DoesNotFailOver() + { + var factory = new SequenceLdapConnectionFactory( + new FakeLdapConnection().WithUserEntry("cn=alice,dc=x", Array.Empty())); + var svc = new LdapAuthService(Opts("dc2"), factory); + + var result = await svc.AuthenticateAsync("alice", "pw", CancellationToken.None); + + Assert.Equal(LdapAuthFailure.GroupLookupFailed, result.Failure); + Assert.Single(factory.Created); + } + + [Fact] + public async Task AmbiguousUser_DoesNotFailOver() + { + var factory = new SequenceLdapConnectionFactory( + new FakeLdapConnection().WithDuplicateMatch()); + var svc = new LdapAuthService(Opts("dc2"), factory); + + var result = await svc.AuthenticateAsync("alice", "pw", CancellationToken.None); + + Assert.Equal(LdapAuthFailure.AmbiguousUser, result.Failure); + Assert.Single(factory.Created); + } + + // ---- exhaustion ---- + + [Fact] + public async Task AllEndpointsDown_ReturnsServiceAccountBindFailed() + { + var factory = new SequenceLdapConnectionFactory( + new FakeLdapConnection().ThrowOnConnect(), + new FakeLdapConnection().ThrowOnConnect()); + var svc = new LdapAuthService(Opts("dc2"), factory); + + var result = await svc.AuthenticateAsync("alice", "pw", CancellationToken.None); + + Assert.Equal(LdapAuthFailure.ServiceAccountBindFailed, result.Failure); + Assert.Equal(2, factory.Created.Count); + } + + // ---- stickiness ---- + + [Fact] + public async Task StickyPreference_SecondLoginStartsAtLastGoodEndpoint() + { + var factory = new SequenceLdapConnectionFactory( + new FakeLdapConnection().ThrowOnConnect(), // login 1 @ dc1: dead + HealthyWithUser(), // login 1 @ dc2: answers + HealthyWithUser()); // login 2: should go straight to dc2 + var svc = new LdapAuthService(Opts("dc2"), factory); + + Assert.True((await svc.AuthenticateAsync("alice", "pw", CancellationToken.None)).Succeeded); + Assert.True((await svc.AuthenticateAsync("alice", "pw", CancellationToken.None)).Succeeded); + + Assert.Equal(3, factory.Created.Count); + Assert.Equal("dc2", factory.Created[2].ConnectArgs!.Value.Host); + } + + [Fact] + public async Task StickyPreference_DriftsBackToPrimary_WhenPreferredFails() + { + var factory = new SequenceLdapConnectionFactory( + new FakeLdapConnection().ThrowOnConnect(), // login 1 @ dc1: dead + HealthyWithUser(), // login 1 @ dc2: answers -> preferred + new FakeLdapConnection().ThrowOnConnect(), // login 2 @ dc2: now dead + HealthyWithUser(), // login 2 @ dc1 (declared order): recovered + HealthyWithUser()); // login 3: should start at dc1 again + var svc = new LdapAuthService(Opts("dc2"), factory); + + Assert.True((await svc.AuthenticateAsync("alice", "pw", CancellationToken.None)).Succeeded); + Assert.True((await svc.AuthenticateAsync("alice", "pw", CancellationToken.None)).Succeeded); + Assert.True((await svc.AuthenticateAsync("alice", "pw", CancellationToken.None)).Succeeded); + + Assert.Equal(5, factory.Created.Count); + Assert.Equal("dc1", factory.Created[3].ConnectArgs!.Value.Host); + Assert.Equal("dc1", factory.Created[4].ConnectArgs!.Value.Host); + } + + [Fact] + public async Task StickyPreference_UpdatesOnUserSideAnswer_NotOnlySuccess() + { + var factory = new SequenceLdapConnectionFactory( + new FakeLdapConnection().ThrowOnConnect(), // login 1 @ dc1: dead + new FakeLdapConnection(), // login 1 @ dc2: answers UserNotFound + HealthyWithUser()); // login 2: should start at dc2 + var svc = new LdapAuthService(Opts("dc2"), factory); + + var first = await svc.AuthenticateAsync("ghost", "pw", CancellationToken.None); + Assert.Equal(LdapAuthFailure.UserNotFound, first.Failure); + + Assert.True((await svc.AuthenticateAsync("alice", "pw", CancellationToken.None)).Succeeded); + Assert.Equal("dc2", factory.Created[2].ConnectArgs!.Value.Host); + } + + [Fact] + public async Task AllEndpointsDown_LeavesPreferenceUnchanged() + { + // A total outage teaches nothing about which endpoint is healthy, so the walk + // must not move the preference — the next login still starts at the primary. + var factory = new SequenceLdapConnectionFactory( + new FakeLdapConnection().ThrowOnConnect(), // login 1 @ dc1 + new FakeLdapConnection().ThrowOnConnect(), // login 1 @ dc2 + HealthyWithUser()); // login 2: must start at dc1 again + var svc = new LdapAuthService(Opts("dc2"), factory); + + Assert.Equal(LdapAuthFailure.ServiceAccountBindFailed, + (await svc.AuthenticateAsync("alice", "pw", CancellationToken.None)).Failure); + Assert.True((await svc.AuthenticateAsync("alice", "pw", CancellationToken.None)).Succeeded); + + Assert.Equal("dc1", factory.Created[2].ConnectArgs!.Value.Host); + } + + // ---- back-compat ---- + + [Fact] + public async Task NoFallbacks_SingleAttempt_UnchangedBehavior() + { + var factory = new SequenceLdapConnectionFactory( + new FakeLdapConnection().ThrowOnConnect()); + var svc = new LdapAuthService(Opts(/* no fallbacks */), factory); + + var result = await svc.AuthenticateAsync("alice", "pw", CancellationToken.None); + + Assert.Equal(LdapAuthFailure.ServiceAccountBindFailed, result.Failure); + Assert.Single(factory.Created); + } + + [Fact] + public async Task Disabled_NeverOpensAConnection_EvenWithFallbacks() + { + var factory = new SequenceLdapConnectionFactory(HealthyWithUser()); + var svc = new LdapAuthService(Opts("dc2") with { Enabled = false }, factory); + + var result = await svc.AuthenticateAsync("alice", "pw", CancellationToken.None); + + Assert.Equal(LdapAuthFailure.Disabled, result.Failure); + Assert.Empty(factory.Created); + } + + [Fact] + public async Task MalformedFallbackEntry_IsSkippedDefensively() + { + // The validator rejects this at boot; a service constructed without validation + // must degrade to the parseable subset, not throw. + var factory = new SequenceLdapConnectionFactory(HealthyWithUser()); + var svc = new LdapAuthService(Opts(":::"), factory); + + Assert.True((await svc.AuthenticateAsync("alice", "pw", CancellationToken.None)).Succeeded); + Assert.Single(factory.Created); + } +} diff --git a/docs/plans/2026-08-10-ldap-backup-dc-failover-design.md b/docs/plans/2026-08-10-ldap-backup-dc-failover-design.md index 7550ba1..04c529b 100644 --- a/docs/plans/2026-08-10-ldap-backup-dc-failover-design.md +++ b/docs/plans/2026-08-10-ldap-backup-dc-failover-design.md @@ -94,6 +94,13 @@ Inside `LdapAuthService.Authenticate`, an endpoint walk over an internal preferred fails; when it does, the walk tries declared order again, so preference drifts back to the primary naturally. +> **Refinement (Task 4):** sticky preference updates on ANY authoritative answer +> (success or user-side failure), not only on success — a DC that answers is healthy, +> and success-only stickiness would re-pay the dead primary's connect timeout on every +> mistyped password during an outage. Corollary, also implemented: an *exhausted* walk +> (every endpoint system-side-down) leaves the preference untouched, since a total +> outage says nothing about which endpoint to prefer once one recovers. + ## Stickiness state `volatile int _preferredIndex` on the singleton `LdapAuthService` (registered