From 09faa05f068dfd8d1b56d9ce296d35ee9c996802 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 11:38:37 -0400 Subject: [PATCH] feat(security): LDAP auth timeout/concurrency/outage-circuit options; project ConnectionTimeoutMs into the shared library (03/S2) --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../Configuration/LdapOptionsValidator.cs | 13 +++ .../Ldap/LdapOptions.cs | 44 +++++++++ .../LdapOptionsBindingTests.cs | 50 +++++++++++ .../LdapOptionsValidatorTests.cs | 89 +++++++++++++++++++ 5 files changed, 197 insertions(+), 1 deletion(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index d780299b..d5df7283 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -2,7 +2,7 @@ "planPath": "archreview/plans/R2-08-ldap-historyread-async-plan.md", "tasks": [ { "id": "R2-08-T1", "subject": "S2 stall-repro test (RED): slow LDAP backend must deny within AuthTimeout — fails on current code", "status": "completed", "blockedBy": [] }, - { "id": "R2-08-T2", "subject": "LdapOptions: ConnectionTimeoutMs/AuthTimeoutMs/MaxConcurrentAuthentications/OutageFailureThreshold/OutageCooldownSeconds + ToLibraryOptions projection + validator rules", "status": "pending", "blockedBy": [] }, + { "id": "R2-08-T2", "subject": "LdapOptions: ConnectionTimeoutMs/AuthTimeoutMs/MaxConcurrentAuthentications/OutageFailureThreshold/OutageCooldownSeconds + ToLibraryOptions projection + validator rules", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T3", "subject": "LdapAuthResult.IsSystemFailure seam + OtOpcUaLdapAuthService.Adapt mapping (ServiceAccountBindFailed => system-side)", "status": "pending", "blockedBy": [] }, { "id": "R2-08-T4", "subject": "LdapOpcUaUserAuthenticator: WaitAsync wall-clock bound + SemaphoreSlim concurrency cap with release-on-core-completion (turns T1 green)", "status": "pending", "blockedBy": ["R2-08-T1", "R2-08-T2"] }, { "id": "R2-08-T5", "subject": "Directory-outage circuit: fail-fast deny after N consecutive system-side failures, half-open after cooldown", "status": "pending", "blockedBy": ["R2-08-T3", "R2-08-T4"] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs index 9b483a75..18309fe2 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs @@ -47,5 +47,18 @@ public sealed class LdapOptionsValidator : OptionsValidatorBase builder.RequireThat( !(options.Transport == LdapTransport.None && !options.AllowInsecure), "LDAP transport is None (plaintext) but AllowInsecure is false — set Transport to Ldaps/StartTls or set AllowInsecure for dev."); + + // S2 resilience knobs — a non-positive timeout / concurrency bound would silently disable the + // wall-clock/concurrency protection the OPC UA data-plane authenticator relies on, so fail fast. + builder.RequireThat(options.ConnectionTimeoutMs > 0, + "Ldap:ConnectionTimeoutMs must be > 0."); + builder.RequireThat(options.AuthTimeoutMs > 0, + "Ldap:AuthTimeoutMs must be > 0."); + builder.RequireThat(options.MaxConcurrentAuthentications > 0, + "Ldap:MaxConcurrentAuthentications must be > 0."); + // An open circuit with no cooldown would re-probe LDAP on every call — pointless. Only enforce + // a positive cooldown when the circuit is actually enabled (OutageFailureThreshold > 0; 0 disables). + builder.RequireThat(options.OutageFailureThreshold <= 0 || options.OutageCooldownSeconds > 0, + "Ldap:OutageCooldownSeconds must be > 0 when Ldap:OutageFailureThreshold > 0 (0 disables the circuit)."); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs index 5b5cdde1..ffb5db57 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs @@ -83,6 +83,49 @@ public sealed class LdapOptions /// public Dictionary GroupToRole { get; set; } = new(StringComparer.OrdinalIgnoreCase); + /// + /// Bounds (in milliseconds) the shared-library directory client's socket connect AND each + /// per-operation search inside a single bind flow. Projected into the library's + /// via . The + /// default (10000) matches the library default, so behaviour is unchanged unless configured. + /// This is the FIRST line of defence against a directory outage; the whole-flow + /// is the backstop (03/S2). + /// + public int ConnectionTimeoutMs { get; set; } = 10_000; + + /// + /// Hard whole-flow wall-clock bound (in milliseconds) the OPC UA data-plane authenticator + /// (LdapOpcUaUserAuthenticator) applies across the ENTIRE authenticate flow — bind plus + /// role-map — so the OPC UA SDK's synchronous impersonation callback (raised under a + /// SessionManager-wide event lock) can never be wedged by a slow / hung directory. + /// Deliberately > (default 15000) so a healthy-but-slow + /// full flow is not cut off while an outage is bounded by the library timeout first (03/S2). + /// + public int AuthTimeoutMs { get; set; } = 15_000; + + /// + /// Maximum number of in-flight OPC UA authentication flows the data-plane authenticator admits + /// at once. A caller that cannot acquire a slot within its budget is denied "backend busy" + /// (local backpressure, not counted against the outage circuit). Bounds accumulation of + /// orphaned / timed-out bind tasks during an outage (03/S2). Default 8. + /// + public int MaxConcurrentAuthentications { get; set; } = 8; + + /// + /// Number of CONSECUTIVE system-side failures (boundary timeout, directory-unreachable result, + /// or unexpected throw) after which the data-plane authenticator's outage circuit opens and + /// denies instantly for without touching LDAP. Any success + /// or user-side (credential) deny resets the counter. 0 disables the circuit (03/S2). + /// Default 3. + /// + public int OutageFailureThreshold { get; set; } = 3; + + /// + /// How long (in seconds) the outage circuit stays open before a half-open probe is allowed + /// through. Only meaningful when > 0. Default 15 (03/S2). + /// + public int OutageCooldownSeconds { get; set; } = 15; + /// /// Projects the wire fields onto the shared ZB.MOM.WW.Auth.Ldap /// the directory client consumes. App-only concerns @@ -104,5 +147,6 @@ public sealed class LdapOptions UserNameAttribute = UserNameAttribute, DisplayNameAttribute = DisplayNameAttribute, GroupAttribute = GroupAttribute, + ConnectionTimeoutMs = ConnectionTimeoutMs, }; } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs index c18b8d1e..d5f4d88c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs @@ -61,6 +61,56 @@ public sealed class LdapOptionsBindingTests options.DevStubMode.ShouldBeFalse(); } + + /// + /// The five S2 resilience keys bind from the real Security:Ldap section (03/S2). + /// + [Fact] + public void Binding_reads_resilience_keys() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Security:Ldap:ConnectionTimeoutMs"] = "7000", + ["Security:Ldap:AuthTimeoutMs"] = "12000", + ["Security:Ldap:MaxConcurrentAuthentications"] = "5", + ["Security:Ldap:OutageFailureThreshold"] = "4", + ["Security:Ldap:OutageCooldownSeconds"] = "20", + }) + .Build(); + + var options = configuration.GetSection(LdapOptions.SectionName).Get(); + + options.ShouldNotBeNull(); + options.ConnectionTimeoutMs.ShouldBe(7000); + options.AuthTimeoutMs.ShouldBe(12000); + options.MaxConcurrentAuthentications.ShouldBe(5); + options.OutageFailureThreshold.ShouldBe(4); + options.OutageCooldownSeconds.ShouldBe(20); + } + + /// The resilience keys carry the documented defaults when absent from config. + [Fact] + public void Resilience_key_defaults_hold_when_absent() + { + var options = new LdapOptions(); + + options.ConnectionTimeoutMs.ShouldBe(10000); + options.AuthTimeoutMs.ShouldBe(15000); + options.MaxConcurrentAuthentications.ShouldBe(8); + options.OutageFailureThreshold.ShouldBe(3); + options.OutageCooldownSeconds.ShouldBe(15); + } + + /// ConnectionTimeoutMs is projected onto the shared-library options so the + /// socket-connect + per-search timeout is bounded inside the directory client. + [Fact] + public void ToLibraryOptions_projects_ConnectionTimeoutMs() + { + var options = new LdapOptions { ConnectionTimeoutMs = 4321 }; + + options.ToLibraryOptions().ConnectionTimeoutMs.ShouldBe(4321); + } } /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs index 058f6ff1..9c5ff2b9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs @@ -203,6 +203,95 @@ public sealed class LdapOptionsValidatorTests result.Failures.ShouldContain("Ldap:SearchBase is required when LDAP login is enabled."); } + /// Enabled with a non-positive ConnectionTimeoutMs fails validation (03/S2). + [Fact] + public void Enabled_with_nonpositive_ConnectionTimeoutMs_fails() + { + var options = new LdapOptions + { + Enabled = true, + Server = "ldap", + SearchBase = "dc=x", + Port = 389, + Transport = LdapTransport.Ldaps, + ConnectionTimeoutMs = 0, + }; + + Sut.Validate(null, options).Failed.ShouldBeTrue(); + } + + /// Enabled with a non-positive AuthTimeoutMs fails validation (03/S2). + [Fact] + public void Enabled_with_nonpositive_AuthTimeoutMs_fails() + { + var options = new LdapOptions + { + Enabled = true, + Server = "ldap", + SearchBase = "dc=x", + Port = 389, + Transport = LdapTransport.Ldaps, + AuthTimeoutMs = 0, + }; + + Sut.Validate(null, options).Failed.ShouldBeTrue(); + } + + /// Enabled with a non-positive MaxConcurrentAuthentications fails validation (03/S2). + [Fact] + public void Enabled_with_nonpositive_MaxConcurrentAuthentications_fails() + { + var options = new LdapOptions + { + Enabled = true, + Server = "ldap", + SearchBase = "dc=x", + Port = 389, + Transport = LdapTransport.Ldaps, + MaxConcurrentAuthentications = 0, + }; + + Sut.Validate(null, options).Failed.ShouldBeTrue(); + } + + /// A positive OutageFailureThreshold with a zero OutageCooldownSeconds fails — + /// an open circuit with no cooldown would probe on every call (03/S2). + [Fact] + public void OutageFailureThreshold_positive_with_zero_cooldown_fails() + { + var options = new LdapOptions + { + Enabled = true, + Server = "ldap", + SearchBase = "dc=x", + Port = 389, + Transport = LdapTransport.Ldaps, + OutageFailureThreshold = 3, + OutageCooldownSeconds = 0, + }; + + Sut.Validate(null, options).Failed.ShouldBeTrue(); + } + + /// A zero OutageFailureThreshold (circuit disabled) passes regardless of the + /// cooldown value (03/S2). + [Fact] + public void OutageFailureThreshold_zero_disables_circuit_and_passes() + { + var options = new LdapOptions + { + Enabled = true, + Server = "ldap", + SearchBase = "dc=x", + Port = 389, + Transport = LdapTransport.Ldaps, + OutageFailureThreshold = 0, + OutageCooldownSeconds = 0, + }; + + Sut.Validate(null, options).Succeeded.ShouldBeTrue(); + } + /// Enabled with port 0 reports the port-range failure using the shared primitive wording. [Fact] public void Enabled_with_zero_port_fails()