fix(security): throttle DebugStreamHub LDAP bind via ManagementAuthenticator + correct coverage doc (plan R2-07 T1)

docs/requirements/Component-Security.md

@@ -205,7 +205,7 @@ Role checks are expressed as named ASP.NET Core authorization policies (in `Auth
 
 To blunt online password-spray and brute-force against the directory, every LDAP-bind surface consults a shared in-memory throttle (`LoginThrottle`, a `ZB.MOM.WW.ScadaBridge.Security` singleton) before attempting a bind, and records the bind outcome afterwards.
 
-- **Scope — every LDAP-bind surface**: the Central UI interactive login (`POST /auth/login`) and CLI token issue (`POST /auth/token`), plus the HTTP-Basic management/CLI surfaces fronted by `ManagementAuthenticator` — `POST /management`, the audit REST endpoints, and the debug-stream hub. No bind happens outside these paths, so throttling coverage is complete. The dev/test `DisableLogin` bypass sits **above** the throttle: a login-disabled deployment never binds, so it is never throttled.
+- **Scope — every LDAP-bind surface**: the Central UI interactive login (`POST /auth/login`) and CLI token issue (`POST /auth/token`), plus the HTTP-Basic management/CLI surfaces fronted by `ManagementAuthenticator` — `POST /management`, the audit REST endpoints, **and the debug-stream hub (`DebugStreamHub.OnConnectedAsync` delegates its whole credential path to `ManagementAuthenticator.AuthenticateAsync`, closing the round-2 N1 gap where the hub bound LDAP directly and bypassed the throttle)**. No bind happens outside these paths. The dev/test `DisableLogin` bypass sits **above** the throttle: a login-disabled deployment never binds, so it is never throttled.
 - **Key and window**: counters are keyed per `{username}|{IP}` pair (lower-cased username; remote IP from the connection). A fixed window of `LoginFailureWindowMinutes` (default 5) opens on the first failure; reaching `MaxLoginFailuresPerWindow` failures (default 5) within it locks that key out for `LoginLockoutMinutes` (default 5). A successful bind clears the key; an expired window resets the count. Setting `MaxLoginFailuresPerWindow` to `0` disables throttling entirely.
 - **Behaviour when locked**: the surface refuses the bind without contacting LDAP. The Basic-Auth surfaces (`ManagementAuthenticator`) and `POST /auth/token` return HTTP `429` with `code = "AUTH_THROTTLED"`; `POST /auth/login` redirects to `/login` with a "Too many failed attempts. Try again later." message.
 - **Best-effort, per-node**: the throttle is in-memory with no external state. The two central nodes maintain independent counters (a spray hitting both simply burns its budget on each). Memory is bounded — writes prune expired keys and cap the tracked-key count. This is a rate-limit guard, not an account-lockout policy; it never disables the directory account itself.

src/ZB.MOM.WW.ScadaBridge.ManagementService/DebugStreamHub.cs

@@ -1,4 +1,3 @@
-using System.Text;
 using Microsoft.AspNetCore.SignalR;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
@@ -6,7 +5,6 @@ using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
 using ZB.MOM.WW.ScadaBridge.Commons.Messages.DebugView;
 using ZB.MOM.WW.ScadaBridge.Commons.Messages.Streaming;
 using ZB.MOM.WW.ScadaBridge.Communication;
-using ZB.MOM.WW.Auth.Abstractions.Ldap;
 using ZB.MOM.WW.ScadaBridge.Security;
 
 namespace ZB.MOM.WW.ScadaBridge.ManagementService;
@@ -75,73 +73,35 @@ public class DebugStreamHub : Hub
             return;
         }
 
-        // Dev/test: when login is disabled, auto-authenticate the debug stream as the configured
+        // Authenticate via the SAME shared Basic→LDAP flow as POST /management and the
-        // dev user (ALL roles incl. Deployer, system-wide) — mirroring the cookie/management
+        // audit REST (arch-review R2 N1): ManagementAuthenticator applies the DisableLogin
-        // bypass so the CLI debug stream is usable in a login-disabled (e.g. no-LDAP) deployment.
+        // bypass, the LoginThrottle lockout check BEFORE the bind, and RecordFailure/
-        // See ManagementAuthenticator.
+        // RecordSuccess bookkeeping — so hub connection attempts can no longer be used as
-        var bypassUser = ManagementAuthenticator.TryDisableLoginUser(httpContext);
+        // an unthrottled LDAP-bind oracle, and hub failures feed the shared lockout.
-        if (bypassUser is not null)
+        var outcome = await ManagementAuthenticator.AuthenticateAsync(httpContext);
+        if (outcome.User is null)
         {
-            Context.Items[RolesKey] = bypassUser.Roles;
+            _logger.LogWarning("DebugStreamHub connection rejected: authentication failed or throttled");
-            Context.Items[PermittedSiteIdsKey] = bypassUser.PermittedSiteIds;
-            _logger.LogInformation(
-                "DebugStreamHub connection established for {Username} (login disabled)", bypassUser.Username);
-            await base.OnConnectedAsync();
-            return;
-        }
-
-        // Extract Basic Auth credentials
-        var authHeader = httpContext.Request.Headers.Authorization.ToString();
-        if (string.IsNullOrEmpty(authHeader) || !authHeader.StartsWith("Basic ", StringComparison.OrdinalIgnoreCase))
-        {
-            _logger.LogWarning("DebugStreamHub connection rejected: missing Basic Auth header");
             Context.Abort();
             return;
         }
+        var user = outcome.User;
 
-        string username, password;
+        // Role check — Deployer role required (unchanged policy).
-        try
+        if (!user.Roles.Contains(Roles.Deployer))
         {
-            var decoded = Encoding.UTF8.GetString(Convert.FromBase64String(authHeader["Basic ".Length..]));
+            _logger.LogWarning("DebugStreamHub connection rejected: {Username} lacks Deployer role", user.Username);
-            var colon = decoded.IndexOf(':');
-            if (colon < 0) throw new FormatException();
-            username = decoded[..colon];
-            password = decoded[(colon + 1)..];
-        }
-        catch
-        {
-            _logger.LogWarning("DebugStreamHub connection rejected: malformed Basic Auth");
-            Context.Abort();
-            return;
-        }
-
-        // LDAP authentication
-        var ldapAuth = httpContext.RequestServices.GetRequiredService<ILdapAuthService>();
-        var authResult = await ldapAuth.AuthenticateAsync(username, password, Context.ConnectionAborted);
-        if (!authResult.Succeeded)
-        {
-            _logger.LogWarning("DebugStreamHub connection rejected: LDAP auth failed for {Username}", username);
-            Context.Abort();
-            return;
-        }
-
-        // Role check — Deployer role required
-        var roleMapper = httpContext.RequestServices.GetRequiredService<RoleMapper>();
-        var mappingResult = await roleMapper.MapGroupsToRolesAsync(authResult.Groups, Context.ConnectionAborted);
-
-        if (!mappingResult.Roles.Contains(Roles.Deployer))
-        {
-            _logger.LogWarning("DebugStreamHub connection rejected: {Username} lacks Deployer role", username);
             Context.Abort();
             return;
         }
 
         // Persist the resolved identity on the connection so per-instance site-scope
-        // enforcement can be applied to SubscribeInstance calls.
+        // enforcement can be applied to SubscribeInstance calls. PermittedSiteIds is the
-        Context.Items[RolesKey] = mappingResult.Roles.ToArray();
+        // authenticator's normalized form (empty == system-wide), same as every other surface.
-        Context.Items[PermittedSiteIdsKey] = mappingResult.PermittedSiteIds.ToArray();
+        Context.Items[RolesKey] = user.Roles;
+        Context.Items[PermittedSiteIdsKey] = user.PermittedSiteIds;
 
-        _logger.LogInformation("DebugStreamHub connection established for {Username}", username);
+        _logger.LogInformation("DebugStreamHub connection established for {Username}", user.Username);
         await base.OnConnectedAsync();
     }
 

tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/DebugStreamHubTests.cs

@@ -1,4 +1,19 @@
+using System.Net;
+using System.Text;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Connections.Features;
+using Microsoft.AspNetCore.Http.Features;
+using Microsoft.AspNetCore.SignalR;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.Extensions.Options;
+using NSubstitute;
+using ZB.MOM.WW.Auth.Abstractions.Ldap;
+using ZB.MOM.WW.ScadaBridge.Communication;
+using ZB.MOM.WW.ScadaBridge.Communication.Grpc;
 using ZB.MOM.WW.ScadaBridge.ManagementService;
+using ZB.MOM.WW.ScadaBridge.Security;
+using ZB.MOM.WW.ScadaBridge.Security.Auth;
 
 namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests;
 
@@ -63,4 +78,83 @@ public class DebugStreamHubTests
 
         Assert.True(allowed);
     }
+
+    // --- OnConnectedAsync delegates to ManagementAuthenticator (arch-review R2 N1) ------------
+
+    private readonly ILdapAuthService _ldap = Substitute.For<ILdapAuthService>();
+    private readonly ServiceProvider _provider;
+    private readonly LoginThrottle _throttle;
+
+    public DebugStreamHubTests()
+    {
+        var services = new ServiceCollection();
+        services.AddSingleton<IOptions<AuthDisableLoginOptions>>(
+            Options.Create(new AuthDisableLoginOptions { DisableLogin = false }));
+        services.AddSingleton<IOptions<SecurityOptions>>(Options.Create(new SecurityOptions()));
+        services.AddSingleton(TimeProvider.System);
+        services.AddSingleton<LoginThrottle>();
+        services.AddSingleton(_ldap);
+        services.AddLogging();
+        _provider = services.BuildServiceProvider();
+        _throttle = _provider.GetRequiredService<LoginThrottle>();
+    }
+
+    private async Task<TestHubCallerContext> ConnectAsync(string username, string password)
+    {
+        var http = new DefaultHttpContext { RequestServices = _provider };
+        var creds = Convert.ToBase64String(Encoding.UTF8.GetBytes($"{username}:{password}"));
+        http.Request.Headers.Authorization = $"Basic {creds}";
+        http.Connection.RemoteIpAddress = IPAddress.Parse("10.0.0.1");
+
+        var debugStreamService = new DebugStreamService(
+            new CommunicationService(Options.Create(new CommunicationOptions()), NullLogger<CommunicationService>.Instance),
+            new ServiceCollection().BuildServiceProvider(),
+            new SiteStreamGrpcClientFactory(NullLoggerFactory.Instance),
+            NullLogger<DebugStreamService>.Instance);
+        var hubContext = Substitute.For<IHubContext<DebugStreamHub>>();
+        var hub = new DebugStreamHub(debugStreamService, hubContext, NullLogger<DebugStreamHub>.Instance);
+
+        var ctx = new TestHubCallerContext(http);
+        hub.Context = ctx;
+        await hub.OnConnectedAsync();
+        return ctx;
+    }
+
+    [Fact]
+    public async Task OnConnectedAsync_FailedBinds_FeedTheSharedLoginThrottle()
+    {
+        // 5 failed hub connects for alice@10.0.0.1 must arm the same lockout the
+        // /management and /auth surfaces consult (arch-review R2 N1).
+        _ldap.AuthenticateAsync(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
+            .Returns(LdapAuthResult.Fail(LdapAuthFailure.BadCredentials));
+        for (var i = 0; i < 5; i++) await ConnectAsync("alice", "wrong"); // each Aborted
+        Assert.True(_throttle.IsLockedOut("alice", "10.0.0.1"));
+    }
+
+    [Fact]
+    public async Task OnConnectedAsync_LockedOutKey_AbortsWithoutContactingLdap()
+    {
+        for (var i = 0; i < 5; i++) _throttle.RecordFailure("alice", "10.0.0.1");
+        _ldap.ClearReceivedCalls();
+        var ctx = await ConnectAsync("alice", "whatever");
+        Assert.True(ctx.Aborted);
+        await _ldap.DidNotReceiveWithAnyArgs().AuthenticateAsync(default!, default!, default);
+    }
+
+    private sealed class TestHubCallerContext : HubCallerContext
+    {
+        private readonly FeatureCollection _features = new();
+        public TestHubCallerContext(HttpContext http) =>
+            _features.Set<IHttpContextFeature>(new TestHttpContextFeature { HttpContext = http });
+        public bool Aborted { get; private set; }
+        public override string ConnectionId => "test-conn";
+        public override string? UserIdentifier => null;
+        public override System.Security.Claims.ClaimsPrincipal? User => null;
+        public override IDictionary<object, object?> Items { get; } = new Dictionary<object, object?>();
+        public override IFeatureCollection Features => _features;
+        public override CancellationToken ConnectionAborted => CancellationToken.None;
+        public override void Abort() => Aborted = true;
+
+        private sealed class TestHttpContextFeature : IHttpContextFeature { public HttpContext? HttpContext { get; set; } }
+    }
 }