feat(host): hard wall-clock bound + bounded concurrency on OPC UA LDAP authentication (03/S2)

archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json

@@ -4,7 +4,7 @@
     { "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": "completed", "blockedBy": [] },
     { "id": "R2-08-T3", "subject": "LdapAuthResult.IsSystemFailure seam + OtOpcUaLdapAuthService.Adapt mapping (ServiceAccountBindFailed => system-side)", "status": "completed", "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-T4", "subject": "LdapOpcUaUserAuthenticator: WaitAsync wall-clock bound + SemaphoreSlim concurrency cap with release-on-core-completion (turns T1 green)", "status": "completed", "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"] },
     { "id": "R2-08-T6", "subject": "Outage-sim integration test (real library, unroutable host, via internal HandleImpersonation) + SDK m_eventLock contract doc + docs/security.md", "status": "pending", "blockedBy": ["R2-08-T4"] },
     { "id": "R2-08-T7", "subject": "S3 sequential-serving repro test (RED): MaxObservedConcurrency >= 2 on an 8-node Raw batch — fails on current code", "status": "pending", "blockedBy": [] },

src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/LdapOpcUaUserAuthenticator.cs

@@ -1,5 +1,6 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 using ZB.MOM.WW.Auth.Abstractions.Roles;
 using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;
 using ZB.MOM.WW.OtOpcUa.Security.Ldap;
@@ -16,22 +17,108 @@ namespace ZB.MOM.WW.OtOpcUa.Host.OpcUa;
 /// session identity for the downstream data-plane ACL evaluator.
 /// </summary>
 /// <remarks>
+/// <para>
 /// This authenticator is registered as a singleton, but <see cref="IGroupRoleMapper{TRole}"/>
 /// (and its DbContext-backed mapping service) is scoped. A per-call DI scope is opened to
 /// resolve the mapper so the singleton never captures a scoped dependency.
+/// </para>
+/// <para>
+/// <b>Resilience (arch-review 03/S2).</b> The OPC UA SDK raises the impersonation callback that
+/// reaches here <em>synchronously</em>, inside a <c>SessionManager</c>-wide event lock (verified
+/// against <c>OPCFoundation.NetStandard.Opc.Ua.Server</c> 1.5.378 —
+/// <c>SessionManager.ActivateSessionAsync</c> holds <c>m_eventLock</c> while invoking the
+/// <c>void</c> impersonation delegate). A slow / hung LDAP bind inside that callback therefore does
+/// not merely stall one SDK thread — it serializes <em>every</em> session activation server-wide
+/// (Anonymous and X509 included). The callback contract is irreducibly synchronous (there is no
+/// async impersonation callback), so the fix lives entirely at this boundary: a hard whole-flow
+/// wall-clock bound (<see cref="LdapOptions.AuthTimeoutMs"/>) and a bounded in-flight concurrency cap
+/// (<see cref="LdapOptions.MaxConcurrentAuthentications"/>), both fail-closed. All new paths deny.
+/// </para>
 /// </remarks>
-public sealed class LdapOpcUaUserAuthenticator(
-    ILdapAuthService ldap,
-    IServiceScopeFactory scopeFactory,
-    ILogger<LdapOpcUaUserAuthenticator> logger)
-    : IOpcUaUserAuthenticator
+public sealed class LdapOpcUaUserAuthenticator : IOpcUaUserAuthenticator
 {
+    private readonly ILdapAuthService _ldap;
+    private readonly IServiceScopeFactory _scopeFactory;
+    private readonly ILogger<LdapOpcUaUserAuthenticator> _logger;
+    private readonly LdapOptions _options;
+
+    /// <summary>Bounds in-flight authentication flows. A boundary timeout abandons its core task, which
+    /// keeps holding its slot until it actually completes — so an outage cannot accumulate unbounded
+    /// orphan binds.</summary>
+    private readonly SemaphoreSlim _gate;
+
+    /// <summary>Constructs the authenticator from its dependencies and the bound LDAP options.</summary>
+    /// <param name="ldap">The app LDAP auth service (shared with the Admin UI login flow).</param>
+    /// <param name="scopeFactory">Opens a per-call DI scope to resolve the scoped role mapper.</param>
+    /// <param name="options">The bound <see cref="LdapOptions"/> (timeout + concurrency knobs).</param>
+    /// <param name="logger">The logger.</param>
+    public LdapOpcUaUserAuthenticator(
+        ILdapAuthService ldap,
+        IServiceScopeFactory scopeFactory,
+        IOptions<LdapOptions> options,
+        ILogger<LdapOpcUaUserAuthenticator> logger)
+    {
+        _ldap = ldap;
+        _scopeFactory = scopeFactory;
+        _options = options.Value;
+        _logger = logger;
+        _gate = new SemaphoreSlim(Math.Max(1, _options.MaxConcurrentAuthentications));
+    }
+
     /// <inheritdoc />
     public async Task<OpcUaUserAuthResult> AuthenticateUserNameAsync(string username, string password, CancellationToken ct)
     {
+        var authTimeout = TimeSpan.FromMilliseconds(Math.Max(1, _options.AuthTimeoutMs));
+
+        // Bounded in-flight concurrency: acquire a slot within our own wall-clock budget. All slots
+        // busy is LOCAL backpressure, not directory evidence — deny "busy" (fail-closed).
+        if (!await _gate.WaitAsync(authTimeout, ct).ConfigureAwait(false))
+        {
+            _logger.LogWarning(
+                "LDAP authentication backend busy (concurrency cap {Cap}) — denying OPC UA user {User}",
+                _options.MaxConcurrentAuthentications, username);
+            return OpcUaUserAuthResult.Deny("Authentication backend busy");
+        }
+
+        // Release the slot when the CORE task completes — deliberately NOT in a finally here. A boundary
+        // timeout below abandons the core task (it runs to completion in the background; the underlying
+        // library is documented never-throw, so there are no unobserved exceptions), and it must keep
+        // counting against the cap until it finishes. The point of the boundary is to release the SDK
+        // thread (and m_eventLock), not to cancel the bind.
+        var core = AuthenticateCoreAsync(username, password, ct);
+        _ = core.ContinueWith(
+            static (_, state) => ((SemaphoreSlim)state!).Release(),
+            _gate,
+            CancellationToken.None,
+            TaskContinuationOptions.None,
+            TaskScheduler.Default);
+
         try
         {
-            var result = await ldap.AuthenticateAsync(username, password, ct).ConfigureAwait(false);
+            return await core.WaitAsync(authTimeout, ct).ConfigureAwait(false);
+        }
+        catch (TimeoutException)
+        {
+            _logger.LogWarning(
+                "LDAP authentication timed out after {Timeout} for OPC UA user {User} — denying (fail-closed)",
+                authTimeout, username);
+            return OpcUaUserAuthResult.Deny("Authentication timed out");
+        }
+    }
+
+    /// <summary>
+    /// The unbounded core authenticate flow (bind + role-map) that the outer
+    /// <see cref="AuthenticateUserNameAsync"/> wraps in a wall-clock bound + concurrency cap. Fail-closed:
+    /// any backend throw becomes a denial rather than escaping into the SDK.
+    /// </summary>
+    /// <param name="username">The username to authenticate.</param>
+    /// <param name="password">The cleartext password.</param>
+    /// <param name="ct">Cancellation token.</param>
+    private async Task<OpcUaUserAuthResult> AuthenticateCoreAsync(string username, string password, CancellationToken ct)
+    {
+        try
+        {
+            var result = await _ldap.AuthenticateAsync(username, password, ct).ConfigureAwait(false);
             if (!result.Success)
             {
                 return OpcUaUserAuthResult.Deny(result.Error ?? "Invalid credentials");
@@ -42,7 +129,7 @@ public sealed class LdapOpcUaUserAuthenticator(
         }
         catch (Exception ex) when (ex is not OperationCanceledException)
         {
-            logger.LogWarning(ex, "LDAP authentication threw for OPC UA user {User}", username);
+            _logger.LogWarning(ex, "LDAP authentication threw for OPC UA user {User}", username);
             return OpcUaUserAuthResult.Deny("Authentication backend error");
         }
     }
@@ -62,7 +149,7 @@ public sealed class LdapOpcUaUserAuthenticator(
     {
         try
         {
-            await using var scope = scopeFactory.CreateAsyncScope();
+            await using var scope = _scopeFactory.CreateAsyncScope();
             var mapper = scope.ServiceProvider.GetRequiredService<IGroupRoleMapper<string>>();
             var mapping = await mapper.MapAsync(groups, ct).ConfigureAwait(false);
 
@@ -73,7 +160,7 @@ public sealed class LdapOpcUaUserAuthenticator(
         }
         catch (Exception ex) when (ex is not OperationCanceledException)
         {
-            logger.LogWarning(ex,
+            _logger.LogWarning(ex,
                 "Role-map lookup failed for OPC UA user {User}; using pre-resolved baseline roles", username);
             return preResolved;
         }

tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs

@@ -1,6 +1,7 @@
 using System.Diagnostics;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.Extensions.Options;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.Auth.Abstractions.Roles;
@@ -32,10 +33,7 @@ public sealed class LdapAuthResilienceTests
             TimeSpan.FromSeconds(5),
             new LdapAuthResult(true, "Alice", "alice", new[] { "configeditor" }, Array.Empty<string>(), null));
         var mapper = new FakeMapper(g => g.ToArray());
-        var sut = new LdapOpcUaUserAuthenticator(
-            ldap,
-            ScopeFactoryWith(mapper),
-            NullLogger<LdapOpcUaUserAuthenticator>.Instance);
+        var sut = Build(ldap, mapper, new LdapOptions { AuthTimeoutMs = 200 });
 
         var sw = Stopwatch.StartNew();
         var result = await sut
@@ -49,6 +47,69 @@ public sealed class LdapAuthResilienceTests
         sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(1.5));
     }
 
+    /// <summary>A fast, healthy backend is unaffected by the resilience wrapping — success + roles pass
+    /// through untouched (03/S2).</summary>
+    [Fact]
+    public async Task Authenticate_fast_success_unaffected()
+    {
+        var ldap = new DelayingFakeLdap(
+            TimeSpan.Zero,
+            new LdapAuthResult(true, "Alice", "alice", new[] { "configeditor" }, Array.Empty<string>(), null));
+        var mapper = new FakeMapper(g => g.Select(x => x == "configeditor" ? "Designer" : x).ToArray());
+        var sut = Build(ldap, mapper, new LdapOptions());
+
+        var result = await sut
+            .AuthenticateUserNameAsync("alice", "secret", CancellationToken.None)
+            .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None);
+
+        result.Success.ShouldBeTrue();
+        result.DisplayName.ShouldBe("Alice");
+        result.Roles.ShouldBe(new[] { "Designer" });
+    }
+
+    /// <summary>With the concurrency cap saturated (a parked in-flight bind holding the only slot), a
+    /// second call is denied "backend busy" within its own budget — it never waits behind the parked
+    /// bind indefinitely (03/S2).</summary>
+    [Fact]
+    public async Task Authenticate_saturated_concurrency_denies_busy()
+    {
+        var ldap = new GatedFakeLdap(
+            new LdapAuthResult(true, "Alice", "alice", new[] { "configeditor" }, Array.Empty<string>(), null));
+        var mapper = new FakeMapper(g => g.ToArray());
+        // MaxConcurrentAuthentications = 1 → the second call cannot get a slot; small AuthTimeoutMs so the
+        // "busy" denial is quick and the first call's boundary also trips fast (its core stays parked,
+        // holding the slot).
+        var sut = Build(ldap, mapper, new LdapOptions { MaxConcurrentAuthentications = 1, AuthTimeoutMs = 300 });
+
+        // First call: acquires the only slot; its core parks in the gated fake.
+        var first = sut.AuthenticateUserNameAsync("alice", "secret", CancellationToken.None);
+
+        // Wait until the fake has actually been entered (slot acquired, core parked).
+        var spin = Stopwatch.StartNew();
+        while (ldap.Entered == 0 && spin.Elapsed < TimeSpan.FromSeconds(5))
+            await Task.Delay(10, TestContext.Current.CancellationToken);
+        ldap.Entered.ShouldBe(1);
+
+        // Second concurrent call: the slot is held by the parked core, so acquisition times out → busy.
+        var second = await sut
+            .AuthenticateUserNameAsync("bob", "secret", CancellationToken.None)
+            .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None);
+
+        second.Success.ShouldBeFalse();
+        second.Error.ShouldNotBeNull();
+        second.Error!.ShouldContain("busy");
+        ldap.Entered.ShouldBe(1, "the second call must be denied without ever reaching the backend");
+
+        // Release the parked bind so the first call's core completes cleanly (no orphaned resources).
+        ldap.Release();
+        await first.WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None);
+    }
+
+    private static LdapOpcUaUserAuthenticator Build(
+        ILdapAuthService ldap, IGroupRoleMapper<string> mapper, LdapOptions options) =>
+        new(ldap, ScopeFactoryWith(mapper), Options.Create(options),
+            NullLogger<LdapOpcUaUserAuthenticator>.Instance);
+
     /// <summary>Builds an IServiceScopeFactory whose scopes resolve the supplied mapper.</summary>
     private static IServiceScopeFactory ScopeFactoryWith(IGroupRoleMapper<string> mapper)
     {
@@ -92,6 +153,40 @@ public sealed class LdapAuthResilienceTests
         }
     }
 
+    /// <summary>
+    /// A fake <see cref="ILdapAuthService"/> that parks ASYNCHRONOUSLY (awaits a
+    /// <see cref="TaskCompletionSource"/>) until the test calls <see cref="Release"/> — used to hold an
+    /// in-flight bind so the concurrency cap is deterministically saturated WITHOUT blocking a thread.
+    /// Records how many calls have entered (past the slot acquire).
+    /// </summary>
+    private sealed class GatedFakeLdap : ILdapAuthService
+    {
+        private readonly TaskCompletionSource _park = new(TaskCreationOptions.RunContinuationsAsynchronously);
+        private readonly LdapAuthResult _then;
+        private int _entered;
+
+        /// <summary>Initializes the fake with the result to return once released.</summary>
+        /// <param name="then">The result returned once <see cref="Release"/> is called.</param>
+        public GatedFakeLdap(LdapAuthResult then) => _then = then;
+
+        /// <summary>Number of authenticate calls that reached the backend body.</summary>
+        public int Entered => Volatile.Read(ref _entered);
+
+        /// <summary>Releases the parked bind so its awaiting call can complete.</summary>
+        public void Release() => _park.TrySetResult();
+
+        /// <summary>Marks entry, awaits the parking gate, then returns the canned result.</summary>
+        /// <param name="username">The username (ignored).</param>
+        /// <param name="password">The password (ignored).</param>
+        /// <param name="ct">Cancellation token, observed while parked.</param>
+        public async Task<LdapAuthResult> AuthenticateAsync(string username, string password, CancellationToken ct = default)
+        {
+            Interlocked.Increment(ref _entered);
+            await _park.Task.WaitAsync(ct).ConfigureAwait(false);
+            return _then;
+        }
+    }
+
     /// <summary>Test group→role mapper driven by a delegate over the supplied groups.</summary>
     private sealed class FakeMapper(Func<IReadOnlyList<string>, IReadOnlyList<string>> map) : IGroupRoleMapper<string>
     {

tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOpcUaUserAuthenticatorTests.cs

@@ -1,5 +1,6 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.Extensions.Options;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.Auth.Abstractions.Roles;
@@ -26,7 +27,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests
         // group "configeditor" -> "Designer" (canonical, Task 1.7).
         var ldap = new FakeLdap(new LdapAuthResult(true, "Alice", "alice", new[] { "configeditor" }, Array.Empty<string>(), null));
         var mapper = new FakeMapper(g => g.Select(x => x == "configeditor" ? "Designer" : x).ToArray());
-        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
+        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
 
         var result = await sut.AuthenticateUserNameAsync("alice", "secret", CancellationToken.None);
 
@@ -44,7 +45,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests
         // nothing, so the union is exactly {Administrator}.
         var ldap = new FakeLdap(new LdapAuthResult(true, "dev", "dev", new[] { "dev" }, new[] { "Administrator" }, null));
         var mapper = new FakeMapper(_ => Array.Empty<string>());
-        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
+        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
 
         var result = await sut.AuthenticateUserNameAsync("dev", "anything", CancellationToken.None);
 
@@ -59,7 +60,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests
     {
         var ldap = new FakeLdap(new LdapAuthResult(true, "dev", "dev", new[] { "dev" }, new[] { "Administrator" }, null));
         var mapper = new FakeMapper(_ => throw new InvalidOperationException("DB down"));
-        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
+        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
 
         var result = await sut.AuthenticateUserNameAsync("dev", "anything", CancellationToken.None);
 
@@ -73,7 +74,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests
     {
         var ldap = new FakeLdap(new LdapAuthResult(false, null, "mallory", Array.Empty<string>(), Array.Empty<string>(), "Invalid username or password"));
         var mapper = new FakeMapper(g => g.ToArray());
-        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
+        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
 
         var result = await sut.AuthenticateUserNameAsync("mallory", "wrong", CancellationToken.None);
 
@@ -87,7 +88,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests
     {
         var ldap = new FakeLdap(_ => throw new InvalidOperationException("LDAP unreachable"));
         var mapper = new FakeMapper(g => g.ToArray());
-        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
+        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
 
         var result = await sut.AuthenticateUserNameAsync("anyone", "x", CancellationToken.None);
 
@@ -102,7 +103,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests
     {
         var ldap = new FakeLdap(new LdapAuthResult(true, null, "alice", new[] { "ReadOnly" }, Array.Empty<string>(), null));
         var mapper = new FakeMapper(g => g.ToArray());
-        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
+        var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger<LdapOpcUaUserAuthenticator>.Instance);
 
         var result = await sut.AuthenticateUserNameAsync("alice", "x", CancellationToken.None);