merge(r2): PLAN-R2-07 UI/Management/Security

This commit is contained in:
Joseph Doherty
2026-07-13 11:08:10 -04:00
26 changed files with 852 additions and 92 deletions
@@ -35,7 +35,12 @@
"JwtExpiryMinutes": 15,
"IdleTimeoutMinutes": 30,
"RequireHttpsCookie": false,
"CookieName": "ZB.MOM.WW.ScadaBridge.Auth.env2"
"CookieName": "ZB.MOM.WW.ScadaBridge.Auth.env2",
"ForwardedHeaders": {
"_comment": "Traefik fronts this node on the external scadabridge-net docker network. Trust X-Forwarded-For from the docker bridge address pool so LoginThrottle keys on the real client IP (arch-review R2 N2). Narrow to the Traefik container IP if the network is pinned.",
"Enabled": true,
"KnownNetworks": [ "172.16.0.0/12" ]
}
},
"Communication": {
"DeploymentTimeout": "00:02:00",
@@ -35,7 +35,12 @@
"JwtExpiryMinutes": 15,
"IdleTimeoutMinutes": 30,
"RequireHttpsCookie": false,
"CookieName": "ZB.MOM.WW.ScadaBridge.Auth.env2"
"CookieName": "ZB.MOM.WW.ScadaBridge.Auth.env2",
"ForwardedHeaders": {
"_comment": "Traefik fronts this node on the external scadabridge-net docker network. Trust X-Forwarded-For from the docker bridge address pool so LoginThrottle keys on the real client IP (arch-review R2 N2). Narrow to the Traefik container IP if the network is pinned.",
"Enabled": true,
"KnownNetworks": [ "172.16.0.0/12" ]
}
},
"Communication": {
"DeploymentTimeout": "00:02:00",
@@ -38,7 +38,12 @@
"JwtSigningKey": "scadabridge-dev-jwt-signing-key-must-be-at-least-32-characters-long",
"JwtExpiryMinutes": 15,
"IdleTimeoutMinutes": 30,
"RequireHttpsCookie": false
"RequireHttpsCookie": false,
"ForwardedHeaders": {
"_comment": "Traefik fronts this node on the external scadabridge-net docker network. Trust X-Forwarded-For from the docker bridge address pool so LoginThrottle keys on the real client IP (arch-review R2 N2). Narrow to the Traefik container IP if the network is pinned.",
"Enabled": true,
"KnownNetworks": [ "172.16.0.0/12" ]
}
},
"Communication": {
"DeploymentTimeout": "00:02:00",
@@ -38,7 +38,12 @@
"JwtSigningKey": "scadabridge-dev-jwt-signing-key-must-be-at-least-32-characters-long",
"JwtExpiryMinutes": 15,
"IdleTimeoutMinutes": 30,
"RequireHttpsCookie": false
"RequireHttpsCookie": false,
"ForwardedHeaders": {
"_comment": "Traefik fronts this node on the external scadabridge-net docker network. Trust X-Forwarded-For from the docker bridge address pool so LoginThrottle keys on the real client IP (arch-review R2 N2). Narrow to the Traefik container IP if the network is pinned.",
"Enabled": true,
"KnownNetworks": [ "172.16.0.0/12" ]
}
},
"Communication": {
"DeploymentTimeout": "00:02:00",
+1 -1
View File
@@ -191,7 +191,7 @@ Per-leaf alarm rendering (leaf nodes are individual conditions for native alarms
- **Live updates** — the page is driven by a **transient, per-site central live alarm cache** (`ISiteAlarmLiveCache`, owned by the Communication component; see [Component-Communication](Component-Communication.md)). On site select the page subscribes to the cache; the cache runs one shared, reference-counted per-site aggregator that **seeds** from the snapshot fan-out and then stays warm on a single **site-wide, alarm-only** `SubscribeSite` gRPC stream (seed-then-stream, dedup by `(InstanceUniqueName, AlarmName, SourceReference)`). Applied deltas raise an in-process change event (mirroring `IDeploymentStatusNotifier`) that the Blazor circuit pushes to the browser via `StateHasChanged()` — no new SignalR hub. `AlarmSummaryService.BuildFromLiveAlarms` rebuilds the roll-up + rows from the cache's current alarm set. The cache is **purely in-memory on the active central node** — there is still **no persisted central alarm store**; on a NodeA↔NodeB failover the new active node re-seeds from scratch.
- **View** — roll-up tiles (total active, worst severity, unacked count, per-`AlarmKind` counts) plus a flat, sortable, filterable table. Filters cover instance, `AlarmKind` (Computed / NativeOpcUa / NativeMxAccess), state, acked/unacked, severity threshold, and name search.
- **Read-only** — there are no ack / shelve / suppress controls (native alarms remain read-only by design).
- **Refresh** — manual refresh button plus the 15s poll timer (mirroring the Health dashboard), now retained as a **fallback + `NotReporting` authority** behind the live cache: when the cache reports `IsLive`, the page renders live-cache state; when a stream is unhealthy or a site has not yet seeded, the poll keeps the page fresh so a stream failure never blanks it. (Aggregated live stream **delivered 2026-07-10** — see `docs/plans/2026-07-10-aggregated-live-alarm-stream-plan.md`.)
- **Refresh** — manual refresh button plus the 15s poll timer (mirroring the Health dashboard), now retained as a **fallback + `NotReporting` authority** behind the live cache: when the cache reports `IsLive`, the page renders live-cache state; when a stream is unhealthy, **the aggregator has died (deathwatch resets `IsLive`)**, or a site has not yet seeded, the poll keeps the page fresh so a stream failure never blanks it. When live, the poll updates only the `NotReporting` list and leaves the row set to the delta path, so a slow fan-out can never momentarily revert a fresher live delta (R2 N5). (Aggregated live stream **delivered 2026-07-10** — see `docs/plans/2026-07-10-aggregated-live-alarm-stream-plan.md`.)
- **Reuse** — the alarm badge/formatter markup is factored out of Debug View into a shared `AlarmStateBadges` component consumed by both Debug View and this page.
### Parked Message Management (Deployment Role)
+4 -3
View File
@@ -138,7 +138,7 @@ Set in a local or docker-dev environment via the environment variable `ScadaBrid
- Approve or reject a pending secured write from the Secured Writes page — the *approving* half of the two-person write workflow.
- **Purpose**: The approving counterpart to **Operator**. Separation of duties is enforced **server-side**: the ManagementActor rejects any approval where the approving user equals the submitting Operator (no self-approval), so the two roles must be held by distinct principals for a write to execute. (See Component-ManagementService.md and Component-CentralUI.md.)
> **Two-person secured-write workflow.** `Operator` and `Verifier` are deliberately separate global roles so a single principal cannot both initiate and approve a write through the MxAccess Gateway. Both are coarse global roles like the others; any site scoping is layered on at the LDAP-mapping level.
> **Two-person secured-write workflow.** `Operator` and `Verifier` are deliberately separate global roles so a single principal cannot both initiate and approve a write through the MxAccess Gateway. Both are coarse global roles like the others. **Site scoping from the LDAP mapping is enforced server-side on every secured-write command** (arch-review R2 N3): submit checks the target site, approve/reject check the row's site (before the TTL/self-approval/CAS chain), and the list constrains a scoped caller to their permitted sites — mirroring the deployment commands' `EnforceSiteScope` rule. Administrators and system-wide principals (empty permitted-site set) are unrestricted.
>
> **Deployment-configuration hazard — never grant one principal both roles.** The whole control collapses if the same identity (or LDAP group) maps to *both* `Operator` and `Verifier`: server-side no-self-approval blocks approving the *exact* write you submitted, but a dual-role principal can still trivially pair-approve with a second submission, so a single compromised or careless account regains unilateral write. Keep `SCADA-Operators` and `SCADA-Verifiers` group membership disjoint. The dev `DisableLogin` caveat is the extreme of this: with `DisableLogin` on, the auto-login principal holds **all** roles, so the two-person flow cannot be exercised end-to-end by a single identity — which is exactly why `DisableLogin=true` is refused outside Development (see the `DisableLoginGuard` note above). No-self-approval is covered by handler tests; real two-person use requires two real, distinct-role identities.
>
@@ -205,7 +205,8 @@ 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.
- **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.
- **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; the IP half is the **forwarded-header-resolved client IP** when `ScadaBridge:Security:ForwardedHeaders` names a trusted proxy, otherwise the raw connection peer). 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.
- **Proxy topologies and the username-lockout DoS.** The key is deliberately `{username}|{IP}`, not `{username}` alone: keying on username alone would let any network-adjacent actor lock any operator out of a SCADA control surface with five wrong passwords, repeatable indefinitely. That isolation only exists when the throttle sees real client IPs — behind a proxy **without** ForwardedHeaders trust, all clients collapse onto the proxy's IP and the DoS returns. Mitigations: (a) the shipped Traefik topologies enable trusted-proxy ForwardedHeaders (so per-IP isolation is real in the documented deployment); (b) the lockout window is short by default (`LoginLockoutMinutes` = 5) and never touches the directory account itself; (c) residual risk — an attacker spraying from their *own* IP locking out a victim username *at that IP only* — is the throttle working as designed. A success-path bypass ("a correct password unlocks") was considered and rejected: it would let an attacker who has the password bypass the lockout entirely, and it converts the throttle into a password oracle during the lockout window.
- **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.
@@ -291,9 +291,27 @@
try
{
var result = await AlarmSummaryService.GetSiteAlarmsAsync(siteId);
_rows = result.Alarms;
// Stale-site guard (arch-review R2 N4): the operator may have switched sites
// while this fan-out was in flight — drop the result rather than labeling
// site A's alarms under site B's picker. Mirrors OnLiveAlarmsChanged (:374).
if (_selectedSiteId != siteId)
{
return;
}
// _notReporting is the poll's unique authority — the alarm-only live cache
// cannot compute it — so it is always refreshed.
_notReporting = result.NotReportingInstances;
// While the cache is live, the live deltas own the row set: a poll whose fan-out
// started BEFORE a delta must not land after it and momentarily revert the alarm
// state (arch-review R2 N5). When not live (pre-seed / degraded stream / dead
// aggregator — see R2 N6), the poll remains the full-rebuild safety net.
if (!LiveAlarmCache.IsLive(siteId))
{
_rows = result.Alarms;
_rollup = AlarmSummaryService.ComputeRollup(_rows);
}
RecomputeVisibleRows();
}
catch
@@ -341,10 +359,11 @@
// aggregated alarm set changes. We rebuild _rows/_rollup/_visibleRows from the
// immutable live snapshot — but deliberately DO NOT touch _notReporting, since
// the alarm-only live cache can't compute it.
// • The 15s poll (RefreshAsync) still runs untouched: it is the authority for
// _notReporting and the safety net when the cache is not live (pre-seed or a
// degraded/failed stream — IsLive == false). When live, the poll simply
// re-affirms the same snapshot; the live path just makes updates arrive sooner.
// • The 15s poll (RefreshAsync) is the authority for _notReporting and the
// full-rebuild safety net ONLY while the cache is not live (pre-seed or a
// degraded/failed stream — IsLive == false); when live it deliberately leaves
// _rows to the delta path, so a slow fan-out can never revert a fresher live
// delta (arch-review R2 N5).
// • Both paths mutate shared state only via the Blazor dispatcher (the poll via
// its InvokeAsync callback, the live delta via OnLiveChanged's InvokeAsync), so
// they are serialized and never race. Each rebuild is an idempotent snapshot, so
@@ -363,15 +382,20 @@
_liveSubscription = null;
}
// N7: set BEFORE teardown so a live callback racing Dispose is dropped both
// before the InvokeAsync marshal and inside it (mirrors DebugView.razor).
private volatile bool _disposed;
// Raised on the aggregator's thread — marshal onto the circuit before touching state.
private void OnLiveAlarmsChanged(int siteId)
{
if (_disposed) return;
_ = InvokeAsync(() =>
{
// Drop stale callbacks for a site we've since navigated away from, and
// let the poll drive until the aggregator has actually seeded (so we never
// clobber a good poll snapshot with an empty pre-seed list).
if (_selectedSiteId != siteId || !LiveAlarmCache.IsLive(siteId))
if (_disposed || _selectedSiteId != siteId || !LiveAlarmCache.IsLive(siteId))
{
return;
}
@@ -506,6 +530,7 @@
public void Dispose()
{
_disposed = true;
StopTimer();
DisposeLiveSubscription();
}
@@ -46,6 +46,11 @@ public interface ISecuredWriteRepository
/// <param name="siteId">Site id filter; <c>null</c> matches every site.</param>
/// <param name="skip">Number of rows to skip (offset paging).</param>
/// <param name="take">Maximum number of rows to return.</param>
/// <param name="permittedSiteIds">When non-null, only rows whose <c>SiteId</c> (site
/// identifier) is in the set match; <c>null</c> matches every site. Applied IN ADDITION
/// to <paramref name="siteId"/>. Used by the ManagementActor to constrain an unfiltered
/// list to a site-scoped caller's permitted sites at the query level (arch-review R2 N3),
/// so paging and totals stay correct.</param>
/// <param name="ct">Cancellation token.</param>
/// <returns>A task that resolves to a page of matching rows, newest submission first.</returns>
Task<IReadOnlyList<PendingSecuredWrite>> QueryAsync(
@@ -53,6 +58,7 @@ public interface ISecuredWriteRepository
string? siteId,
int skip,
int take,
IReadOnlyCollection<string>? permittedSiteIds = null,
CancellationToken ct = default);
/// <summary>
@@ -106,7 +112,16 @@ public interface ISecuredWriteRepository
/// </summary>
/// <param name="status">Status filter; <c>null</c> matches every status.</param>
/// <param name="siteId">Site id filter; <c>null</c> matches every site.</param>
/// <param name="permittedSiteIds">When non-null, only rows whose <c>SiteId</c> (site
/// identifier) is in the set match; <c>null</c> matches every site. Applied IN ADDITION
/// to <paramref name="siteId"/>. Used by the ManagementActor to constrain an unfiltered
/// list to a site-scoped caller's permitted sites at the query level (arch-review R2 N3),
/// so paging and totals stay correct.</param>
/// <param name="ct">Cancellation token.</param>
/// <returns>A task that resolves to the count of matching rows.</returns>
Task<int> CountAsync(string? status, string? siteId, CancellationToken ct = default);
Task<int> CountAsync(
string? status,
string? siteId,
IReadOnlyCollection<string>? permittedSiteIds = null,
CancellationToken ct = default);
}
@@ -60,6 +60,6 @@ public interface ISiteAlarmLiveCache
/// cache is authoritative.
/// </summary>
/// <param name="siteId">The numeric site id.</param>
/// <returns><c>true</c> once the site's aggregator has seeded and published at least once.</returns>
/// <returns><c>true</c> while a <b>living</b> aggregator has seeded and published; reverts to false if the aggregator terminates (R2 N6) — a frozen snapshot is never reported as live.</returns>
bool IsLive(int siteId);
}
@@ -239,6 +239,14 @@ public sealed class SiteAlarmLiveCacheService : ISiteAlarmLiveCache
TimeSpan.FromSeconds(60))); // stability window — former StabilityWindow static default
entry.Actor = system.ActorOf(props, $"site-alarm-aggregator-{entry.SiteId}-{Guid.NewGuid():N}");
var aggregator = entry.Actor;
// Deathwatch (arch-review R2 N6): if the aggregator terminates for any reason,
// liveness must reset — a dead feed must never keep IsLive == true, or the page
// grafts a frozen snapshot over fresh poll data for up to 15 s.
system.ActorOf(Props.Create(() => new AggregatorTerminationWatcher(
aggregator!, () => OnAggregatorTerminated(entry.SiteId, aggregator!))));
entry.StartRetryTimer?.Dispose();
entry.StartRetryTimer = null;
_logger.LogInformation("Started live alarm aggregator for site {SiteId} ({SiteIdentifier})",
@@ -421,6 +429,57 @@ public sealed class SiteAlarmLiveCacheService : ISiteAlarmLiveCache
}
}
// ── Deathwatch (R2 N6) ──────────────────────────────────────────────────────
/// <summary>Watches one aggregator and fires a callback exactly once on Terminated.</summary>
private sealed class AggregatorTerminationWatcher : ReceiveActor
{
public AggregatorTerminationWatcher(IActorRef watched, Action onTerminated)
{
Context.Watch(watched);
Receive<Terminated>(_ =>
{
onTerminated();
Context.Stop(Self);
});
}
}
/// <summary>
/// Terminated hook (R2 N6). A DELIBERATE stop (linger fired, entry already removed
/// from _sites) is a no-op — the ReferenceEquals guard also ignores a stale watcher
/// firing after a newer aggregator replaced the dead one. A crash-death with live
/// viewers resets liveness (IsLive → false, snapshot cleared, so the page's poll is
/// authoritative again) and arms the existing bounded start-retry so the feature
/// self-heals, mirroring StartAggregatorAsync's transient-failure path.
/// </summary>
private void OnAggregatorTerminated(int siteId, IActorRef deadActor)
{
lock (_lock)
{
if (!_sites.TryGetValue(siteId, out var entry) || !ReferenceEquals(entry.Actor, deadActor))
return;
entry.Actor = null;
entry.HasPublished = false;
entry.Current = Empty;
if (entry.Subscribers.Count > 0 && !entry.Starting)
{
entry.Starting = true;
entry.StartRetryTimer?.Dispose();
entry.StartRetryTimer = new Timer(
_ => { _ = StartAggregatorAsync(entry); },
state: null,
dueTime: _options.LiveAlarmCacheReconcileInterval,
period: Timeout.InfiniteTimeSpan);
}
}
_logger.LogWarning(
"Live alarm aggregator for site {SiteId} terminated unexpectedly; liveness reset " +
"(page falls back to polling) and a restart is armed while viewers remain", siteId);
}
// ── Nested types ────────────────────────────────────────────────────────────
/// <summary>Per-site bookkeeping. All mutable fields are guarded by the service's <c>_lock</c>.</summary>
@@ -57,6 +57,7 @@ public class SecuredWriteRepository : ISecuredWriteRepository
string? siteId,
int skip,
int take,
IReadOnlyCollection<string>? permittedSiteIds = null,
CancellationToken ct = default)
{
IQueryable<PendingSecuredWrite> query = _context.Set<PendingSecuredWrite>()
@@ -72,6 +73,11 @@ public class SecuredWriteRepository : ISecuredWriteRepository
query = query.Where(p => p.SiteId == siteId);
}
if (permittedSiteIds is not null)
{
query = query.Where(p => permittedSiteIds.Contains(p.SiteId));
}
return await query
.OrderByDescending(p => p.SubmittedAtUtc)
.ThenByDescending(p => p.Id)
@@ -128,7 +134,11 @@ WHERE Id = {id}
}
/// <inheritdoc />
public async Task<int> CountAsync(string? status, string? siteId, CancellationToken ct = default)
public async Task<int> CountAsync(
string? status,
string? siteId,
IReadOnlyCollection<string>? permittedSiteIds = null,
CancellationToken ct = default)
{
IQueryable<PendingSecuredWrite> query = _context.Set<PendingSecuredWrite>()
.AsNoTracking();
@@ -143,6 +153,11 @@ WHERE Id = {id}
query = query.Where(p => p.SiteId == siteId);
}
if (permittedSiteIds is not null)
{
query = query.Where(p => permittedSiteIds.Contains(p.SiteId));
}
return await query.CountAsync(ct);
}
}
+12
View File
@@ -337,6 +337,18 @@ try
}
// Middleware pipeline
// Trusted-proxy forwarded headers (arch-review R2 N2): behind Traefik every client
// shares the proxy's IP, collapsing the LoginThrottle's {username}|{ip} keys onto
// one address (per-IP isolation gone + cheap username-lockout DoS). When enabled,
// X-Forwarded-For from the CONFIGURED proxies only is honored. MUST run first —
// everything downstream keys on Connection.RemoteIpAddress.
var forwardedOptions = builder.Configuration
.GetSection(ZB.MOM.WW.ScadaBridge.Security.ForwardedHeadersSecurityOptions.SectionName)
.Get<ZB.MOM.WW.ScadaBridge.Security.ForwardedHeadersSecurityOptions>() ?? new();
if (ZB.MOM.WW.ScadaBridge.Security.ForwardedHeadersSetup.Build(forwardedOptions) is { } fho)
app.UseForwardedHeaders(fho);
app.UseWebSockets();
app.UseRouting();
app.UseAuthentication();
@@ -18,7 +18,7 @@ internal static class ConfigSecretScrubber
private static readonly string[] SecretNameFragments =
["password", "secret", "token", "apikey", "credential", "passphrase"];
private static bool IsSecretName(string propertyName)
internal static bool IsSecretName(string propertyName)
{
foreach (var fragment in SecretNameFragments)
{
@@ -127,6 +127,15 @@ internal static class ConfigSecretScrubber
/// <param name="incoming">The client-submitted config JSON, possibly containing sentinel values.</param>
/// <param name="stored">The previously stored config JSON to source real secret values from.</param>
/// <returns>The incoming JSON with sentinel values replaced by the corresponding stored values.</returns>
/// <remarks>
/// <b>Array limitation (arch-review R2 N9):</b> sentinel restoration inside arrays
/// pairs incoming[i] with stored[i] BY INDEX. A client that reorders an array of
/// credentialed objects while round-tripping sentinels grafts the wrong stored
/// secret into each slot — a silent misconfiguration (never a disclosure). Clients
/// must not reorder arrays that carry sentinels (the CLI round-trip does not);
/// key-based matching is a possible future refinement if a config type ever nests
/// credentialed arrays.
/// </remarks>
public static string? MergeSentinels(string? incoming, string? stored)
{
if (string.IsNullOrEmpty(incoming))
@@ -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
// dev user (ALL roles incl. Deployer, system-wide) — mirroring the cookie/management
// bypass so the CLI debug stream is usable in a login-disabled (e.g. no-LDAP) deployment.
// See ManagementAuthenticator.
var bypassUser = ManagementAuthenticator.TryDisableLoginUser(httpContext);
if (bypassUser is not null)
// Authenticate via the SAME shared Basic→LDAP flow as POST /management and the
// audit REST (arch-review R2 N1): ManagementAuthenticator applies the DisableLogin
// bypass, the LoginThrottle lockout check BEFORE the bind, and RecordFailure/
// RecordSuccess bookkeeping — so hub connection attempts can no longer be used as
// an unthrottled LDAP-bind oracle, and hub failures feed the shared lockout.
var outcome = await ManagementAuthenticator.AuthenticateAsync(httpContext);
if (outcome.User is null)
{
Context.Items[RolesKey] = bypassUser.Roles;
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");
_logger.LogWarning("DebugStreamHub connection rejected: authentication failed or throttled");
Context.Abort();
return;
}
var user = outcome.User;
string username, password;
try
// Role check — Deployer role required (unchanged policy).
if (!user.Roles.Contains(Roles.Deployer))
{
var decoded = Encoding.UTF8.GetString(Convert.FromBase64String(authHeader["Basic ".Length..]));
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);
_logger.LogWarning("DebugStreamHub connection rejected: {Username} lacks Deployer role", user.Username);
Context.Abort();
return;
}
// Persist the resolved identity on the connection so per-instance site-scope
// enforcement can be applied to SubscribeInstance calls.
Context.Items[RolesKey] = mappingResult.Roles.ToArray();
Context.Items[PermittedSiteIdsKey] = mappingResult.PermittedSiteIds.ToArray();
// enforcement can be applied to SubscribeInstance calls. PermittedSiteIds is the
// authenticator's normalized form (empty == system-wide), same as every other surface.
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();
}
@@ -467,7 +467,7 @@ public class ManagementActor : ReceiveActor
SubmitSecuredWriteCommand cmd => await HandleSubmitSecuredWrite(sp, cmd, user),
ApproveSecuredWriteCommand cmd => await HandleApproveSecuredWrite(sp, cmd, user),
RejectSecuredWriteCommand cmd => await HandleRejectSecuredWrite(sp, cmd, user),
ListSecuredWritesCommand cmd => await HandleListSecuredWrites(sp, cmd),
ListSecuredWritesCommand cmd => await HandleListSecuredWrites(sp, cmd, user),
// Transport bundle operations
ExportBundleCommand cmd => await HandleExportBundle(sp, cmd, user.Username),
@@ -1247,6 +1247,11 @@ public class ManagementActor : ReceiveActor
var site = await siteRepo.GetSiteByIdentifierAsync(cmd.SiteId)
?? throw new ManagementCommandException($"Site '{cmd.SiteId}' not found.");
// Site scope (arch-review R2 N3): an LDAP-mapping-scoped Operator may only
// submit device writes to their permitted sites — same rule C2 applies to
// DeployArtifacts, on a higher-consequence path. Checked before the row exists.
EnforceSiteScope(user, site.Id);
var connections = await siteRepo.GetDataConnectionsBySiteIdAsync(site.Id);
var conn = connections.FirstOrDefault(c =>
string.Equals(c.Name, cmd.ConnectionName, StringComparison.Ordinal))
@@ -1293,6 +1298,11 @@ public class ManagementActor : ReceiveActor
throw new ManagementCommandException(
$"Secured write {cmd.Id} is '{row.Status}', not Pending; it cannot be approved.");
// Site scope (arch-review R2 N3): the approving Verifier must be permitted on
// the row's target site — checked BEFORE the TTL/self-approval/CAS chain so an
// out-of-scope verifier can neither consume the Pending transition nor relay.
await EnforceSiteScopeForIdentifier(sp, user, row.SiteId);
// Server-side TTL: a Pending write older than the configured age is transitioned
// to Expired and can never be approved — the stale setpoint is never relayed
// (arch-review S2). Checked BEFORE the self-approval/CAS so an overdue row never
@@ -1450,6 +1460,10 @@ public class ManagementActor : ReceiveActor
throw new ManagementCommandException(
$"Secured write {cmd.Id} is '{entity.Status}', not Pending; it cannot be rejected.");
// Site scope (arch-review R2 N3): the rejecting Verifier must be permitted on
// the row's target site — checked BEFORE the TTL/self-approval chain.
await EnforceSiteScopeForIdentifier(sp, user, entity.SiteId);
// Server-side TTL: an overdue Pending write expires here too, so a reject on a
// stale row deterministically reports the expiry rather than recording a
// human decision (arch-review S2).
@@ -1479,16 +1493,36 @@ public class ManagementActor : ReceiveActor
}
private static async Task<object?> HandleListSecuredWrites(
IServiceProvider sp, ListSecuredWritesCommand cmd)
IServiceProvider sp, ListSecuredWritesCommand cmd, AuthenticatedUser user)
{
// Site scoping (arch-review R2 N3): an explicit SiteId filter is scope-checked
// like every other identifier-keyed command; an UNFILTERED list from a
// site-scoped non-admin caller is constrained to their permitted sites at the
// query level (rows AND totalCount both scoped — see ISecuredWriteRepository).
IReadOnlyCollection<string>? permittedIdentifiers = null;
if (cmd.SiteId is not null)
{
await EnforceSiteScopeForIdentifier(sp, user, cmd.SiteId);
}
else if (user.PermittedSiteIds.Length > 0
&& !user.Roles.Contains(Roles.Administrator, StringComparer.OrdinalIgnoreCase))
{
var siteRepo = sp.GetRequiredService<ISiteRepository>();
var permitted = new HashSet<string>(user.PermittedSiteIds);
permittedIdentifiers = (await siteRepo.GetAllSitesAsync())
.Where(s => permitted.Contains(s.Id.ToString()))
.Select(s => s.SiteIdentifier)
.ToList();
}
var repo = sp.GetRequiredService<ISecuredWriteRepository>();
// Offset paging (arch-review P3): clamp untrusted paging inputs so a crafted
// command can neither request a negative offset nor an unbounded page.
var take = Math.Clamp(cmd.Take, 1, 500);
var skip = Math.Max(0, cmd.Skip);
var rows = await repo.QueryAsync(cmd.Status, cmd.SiteId, skip, take);
var totalCount = await repo.CountAsync(cmd.Status, cmd.SiteId);
var rows = await repo.QueryAsync(cmd.Status, cmd.SiteId, skip, take, permittedIdentifiers);
var totalCount = await repo.CountAsync(cmd.Status, cmd.SiteId, permittedIdentifiers);
// Opportunistic expiry sweep (arch-review S2): every time the list is read, walk
// the page and expire any overdue Pending row. TryExpireIfStaleAsync self-filters
@@ -0,0 +1,64 @@
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.HttpOverrides;
using System.Net;
namespace ZB.MOM.WW.ScadaBridge.Security;
/// <summary>
/// Config shape for trusted-proxy forwarded-header handling
/// (<c>ScadaBridge:Security:ForwardedHeaders</c>). Disabled by default: a deployment
/// with no reverse proxy must NOT honor X-Forwarded-For (a client could spoof its
/// throttle key). The documented Traefik topologies enable it with the docker
/// network range so LoginThrottle keys on the REAL client IP (arch-review R2 N2).
/// </summary>
public sealed class ForwardedHeadersSecurityOptions
{
/// <summary>Configuration section binding this options class.</summary>
public const string SectionName = "ScadaBridge:Security:ForwardedHeaders";
/// <summary>Whether X-Forwarded-For from trusted proxies is honored.</summary>
public bool Enabled { get; set; }
/// <summary>Exact proxy IPs allowed to supply X-Forwarded-For.</summary>
public string[] KnownProxies { get; set; } = [];
/// <summary>CIDR networks allowed to supply X-Forwarded-For (e.g. "172.16.0.0/12").</summary>
public string[] KnownNetworks { get; set; } = [];
}
/// <summary>
/// Builds the <see cref="ForwardedHeadersOptions"/> for <c>app.UseForwardedHeaders</c>.
/// Pure and fail-fast: malformed entries throw at startup rather than silently
/// trusting nothing/everything. Defaults (loopback) are CLEARED — only the
/// explicitly configured proxies/networks are trusted, and ForwardLimit=1 means
/// only the entry the trusted proxy itself appended is honored.
/// </summary>
public static class ForwardedHeadersSetup
{
/// <summary>
/// Builds the forwarded-headers options, or <see langword="null"/> when disabled.
/// </summary>
/// <param name="options">The bound <see cref="ForwardedHeadersSecurityOptions"/>.</param>
/// <returns>The configured <see cref="ForwardedHeadersOptions"/>, or <see langword="null"/> when disabled.</returns>
public static ForwardedHeadersOptions? Build(ForwardedHeadersSecurityOptions options)
{
if (!options.Enabled) return null;
var built = new ForwardedHeadersOptions
{
ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto,
ForwardLimit = 1,
};
built.KnownProxies.Clear();
// KnownIPNetworks (System.Net.IPNetwork) is the net10 successor to the now-deprecated
// KnownNetworks; UseForwardedHeaders reads it identically.
built.KnownIPNetworks.Clear();
foreach (var proxy in options.KnownProxies)
built.KnownProxies.Add(IPAddress.Parse(proxy));
foreach (var network in options.KnownNetworks)
{
var parts = network.Split('/');
built.KnownIPNetworks.Add(new(IPAddress.Parse(parts[0]), int.Parse(parts[1])));
}
return built;
}
}
@@ -41,6 +41,18 @@ public sealed class LoginThrottle
/// </summary>
public const int MaxTrackedKeys = 10_000;
/// <summary>Amortization cadence: a full prune sweep runs every Nth failure write
/// (plus immediately whenever the map exceeds <see cref="MaxTrackedKeys"/>), so a
/// spray does not pay an O(n) scan — and a possible full sort — on every failed
/// request (arch-review R2 N8). Lockout SEMANTICS are unaffected: IsLockedOut reads
/// window/lockout expiry directly and never depends on pruning.</summary>
internal const int PruneEveryNFailures = 64;
/// <summary>Diagnostics/test accessor: number of currently tracked keys.</summary>
internal int TrackedKeyCount => _entries.Count;
private int _failureWrites;
private readonly TimeProvider _clock;
private readonly IOptions<SecurityOptions> _options;
@@ -118,8 +130,14 @@ public sealed class LoginThrottle
return Arm(existing with { Count = existing.Count + 1 }, opts, now, lockout);
});
// Amortized prune (R2 N8): every Nth failure, or immediately when over the hard
// cap — the cap is the memory-safety invariant and must never wait for the cadence.
if (Interlocked.Increment(ref _failureWrites) % PruneEveryNFailures == 0
|| _entries.Count > MaxTrackedKeys)
{
Prune(now);
}
}
/// <summary>
/// Records a successful bind for the given <paramref name="username"/> +
@@ -208,6 +208,100 @@ public class AlarmSummaryRenderTests : BunitContext
Assert.True(_liveCache.DisposeCount >= 1);
}
// ── arch-review R2 N4: stale-site poll guard ───────────────────────────────
[Fact]
public void InFlightPollForPreviousSite_DoesNotOverwriteNewSitesRows()
{
// Site 1's fan-out hangs on a TCS; site 2 answers immediately (arch-review R2 N4).
var site1Tcs = new TaskCompletionSource<AlarmSummaryResult>();
_summary.GetSiteAlarmsAsync(1, Arg.Any<CancellationToken>()).Returns(site1Tcs.Task);
_summary.GetSiteAlarmsAsync(2, Arg.Any<CancellationToken>()).Returns(Task.FromResult(
new AlarmSummaryResult(
new List<AlarmSummaryRow>
{
new("Site2Instance", new AlarmStateChanged("Site2Instance", "S2-alarm", AlarmState.Active, 100, DateTimeOffset.UtcNow)),
},
Array.Empty<string>())));
var cut = Render<AlarmSummaryPage>();
cut.Find("[data-test='alarm-summary-site']").Change("1"); // poll in flight, hung
cut.Find("[data-test='alarm-summary-site']").Change("2"); // site 2 renders
cut.WaitForAssertion(() => Assert.Equal("Site2Instance", FirstRowInstance(cut)));
// The stale site-1 fan-out now completes — it must be dropped, not rendered.
site1Tcs.SetResult(new AlarmSummaryResult(
new List<AlarmSummaryRow>
{
new("StaleSite1", new AlarmStateChanged("StaleSite1", "S1-alarm", AlarmState.Active, 999, DateTimeOffset.UtcNow)),
},
new[] { "StaleNotReporting" }));
cut.WaitForAssertion(() =>
{
Assert.Equal("Site2Instance", FirstRowInstance(cut));
Assert.DoesNotContain("StaleNotReporting", cut.Markup);
});
}
// ── arch-review R2 N5: while live, the poll updates only _notReporting ──────
[Fact]
public void PollWhileLive_UpdatesNotReportingOnly_DoesNotRegressLiveRows()
{
// Poll snapshot = Zeta/Alpha (constructor stub) + a not-reporting instance.
_summary.GetSiteAlarmsAsync(Arg.Any<int>(), Arg.Any<CancellationToken>())
.Returns(ci => Task.FromResult(new AlarmSummaryResult(
new List<AlarmSummaryRow>
{
new("Zeta", new AlarmStateChanged("Zeta", "Z-alarm", AlarmState.Active, 900, DateTimeOffset.UtcNow)),
},
new[] { "OfflineInstance" })));
var cut = RenderWithSiteSelected();
// Go live with a fresher delta: Gamma only (arch-review R2 N5).
_liveCache.PushAlarms(new List<AlarmStateChanged>
{
new("Gamma", "G-alarm", AlarmState.Active, 300, DateTimeOffset.UtcNow),
});
cut.WaitForAssertion(() => Assert.Equal("Gamma", FirstRowInstance(cut)));
// A poll now completes: it owns _notReporting but must NOT rebuild the rows.
cut.Find("[data-test='alarm-summary-refresh']").Click();
cut.WaitForAssertion(() =>
{
Assert.Contains("OfflineInstance", cut.Markup); // notReporting refreshed
Assert.Equal("Gamma", FirstRowInstance(cut)); // live rows NOT regressed
Assert.Single(cut.FindAll("tr[data-test='alarm-summary-row']"));
});
}
// ── arch-review R2 N7: disposal guard on the live callback ─────────────────
[Fact]
public void LiveCallbackRacingDispose_IsDropped_NoRebuildNoThrow()
{
var cut = RenderWithSiteSelected();
_liveCache.PushAlarms(new List<AlarmStateChanged>
{
new("Gamma", "G-alarm", AlarmState.Active, 300, DateTimeOffset.UtcNow),
});
cut.WaitForAssertion(() => Assert.Equal("Gamma", FirstRowInstance(cut)));
// Snapshot the callback BEFORE disposal — this models a delta thread that
// already read the delegate when Dispose ran (arch-review R2 N7).
var inFlight = _liveCache.CapturedCallback!;
cut.Instance.Dispose();
_liveCache.PushAlarms(new List<AlarmStateChanged>()); // mutate the fake's snapshot
var ex = Record.Exception(inFlight);
Assert.Null(ex);
// The disposed page must not have re-applied the (now empty) live snapshot.
Assert.Equal("Gamma", FirstRowInstance(cut));
}
/// <summary>
/// Controllable in-memory fake of <see cref="ISiteAlarmLiveCache"/>: records
/// subscribe/dispose counts and lets a test push a live snapshot (which flips
@@ -223,6 +317,7 @@ public class AlarmSummaryRenderTests : BunitContext
public int SubscribeCount { get; private set; }
public int DisposeCount { get; private set; }
public int? LastSubscribedSite { get; private set; }
public Action? CapturedCallback => _onChanged;
public IDisposable Subscribe(int siteId, Action onChanged)
{
@@ -302,4 +302,40 @@ public class SiteAlarmLiveCacheServiceTests : TestKit
AwaitCondition(() => service.IsLive(3), TimeSpan.FromSeconds(5));
Assert.True(factory.GetOrCreateCount >= 1); // a client was created for the single endpoint
}
// ── R2 T10: deathwatch un-sticks IsLive on aggregator death (N6) ──
[Fact]
public async Task Aggregator_Death_Resets_IsLive_And_Clears_The_Snapshot()
{
var service = CreateService(TimeSpan.FromMinutes(5), out _);
using var sub = service.Subscribe(SiteId, () => { });
AwaitAssert(() => Assert.True(service.IsLive(SiteId)), TimeSpan.FromSeconds(10));
// Kill the aggregator out from under the service (crash-death, NOT a linger stop).
var aggregator = await Sys.ActorSelection($"/user/site-alarm-aggregator-{SiteId}-*")
.ResolveOne(TimeSpan.FromSeconds(5));
Sys.Stop(aggregator);
AwaitAssert(() =>
{
Assert.False(service.IsLive(SiteId)); // sticky no more (R2 N6)
Assert.Empty(service.GetCurrentAlarms(SiteId)); // frozen snapshot never grafted
}, TimeSpan.FromSeconds(10));
}
[Fact]
public void Linger_Stop_Does_Not_Resurrect_The_Aggregator()
{
// Last viewer leaves, linger fires, entry removed: the deathwatch on the
// deliberately-stopped actor must be a no-op (entry gone), not a restart.
var service = CreateService(TimeSpan.FromMilliseconds(50), out var factory);
var sub = service.Subscribe(SiteId, () => { });
AwaitAssert(() => Assert.True(service.IsLive(SiteId)), TimeSpan.FromSeconds(10));
sub.Dispose();
AwaitAssert(() => Assert.False(service.IsLive(SiteId)), TimeSpan.FromSeconds(10));
var startsAfterStop = factory.GetOrCreateCount;
Thread.Sleep(300);
Assert.Equal(startsAfterStop, factory.GetOrCreateCount); // no self-heal restart fired
}
}
@@ -164,6 +164,72 @@ public class SecuredWriteRepositoryTests : IClassFixture<MsSqlMigrationFixture>
Assert.Equal(0, await repo.CountAsync(status: "Executed", siteId: siteId));
}
[SkippableFact]
public async Task QueryAsync_PermittedSiteIds_ReturnsOnlyPermittedRows()
{
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
// Distinct per-run site identifiers stand in for "SITE1"/"SITE2"/"SITE3" so the
// permitted-set filter is asserted deterministically against a shared MSSQL.
var site1 = NewSiteId();
var site2 = NewSiteId();
var site3 = NewSiteId();
await using var context = CreateContext();
var repo = new SecuredWriteRepository(context);
await repo.AddAsync(NewRow(site1, status: "Pending"));
await repo.AddAsync(NewRow(site1, status: "Pending"));
await repo.AddAsync(NewRow(site2, status: "Pending"));
await repo.AddAsync(NewRow(site3, status: "Pending"));
var rows = await repo.QueryAsync(status: null, siteId: null, skip: 0, take: 100,
permittedSiteIds: new[] { site1, site3 });
Assert.Equal(3, rows.Count);
Assert.DoesNotContain(rows, r => r.SiteId == site2);
}
[SkippableFact]
public async Task CountAsync_PermittedSiteIds_CountsOnlyPermittedRows()
{
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
var site1 = NewSiteId();
var site2 = NewSiteId();
var site3 = NewSiteId();
await using var context = CreateContext();
var repo = new SecuredWriteRepository(context);
await repo.AddAsync(NewRow(site1, status: "Pending"));
await repo.AddAsync(NewRow(site1, status: "Pending"));
await repo.AddAsync(NewRow(site2, status: "Pending"));
await repo.AddAsync(NewRow(site3, status: "Pending"));
var count = await repo.CountAsync(status: null, siteId: null,
permittedSiteIds: new[] { site1, site3 });
Assert.Equal(3, count);
}
[SkippableFact]
public async Task QueryAsync_NullPermittedSiteIds_IsUnfiltered_BackCompat()
{
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
// 4 rows on a single unique site; null permittedSiteIds must not filter them
// out — exactly today's behavior.
var siteId = NewSiteId();
await using var context = CreateContext();
var repo = new SecuredWriteRepository(context);
await repo.AddAsync(NewRow(siteId, status: "Pending"));
await repo.AddAsync(NewRow(siteId, status: "Pending"));
await repo.AddAsync(NewRow(siteId, status: "Executed"));
await repo.AddAsync(NewRow(siteId, status: "Rejected"));
var rows = await repo.QueryAsync(status: null, siteId: siteId, skip: 0, take: 100,
permittedSiteIds: null);
Assert.Equal(4, rows.Count);
}
// --- helpers ------------------------------------------------------------
private ScadaBridgeDbContext CreateContext()
@@ -42,4 +42,47 @@ public class ConfigSecretScrubberTests
Assert.Contains("\"real\"", merged);
Assert.DoesNotContain(ConfigSecretScrubber.Sentinel, merged);
}
// ── arch-review R2 N9: frozen scrubber fragment coverage ───────────────────
// Frozen non-secret allowlist (arch-review R2 N9), mirroring RequiredRoleMatrixTests:
// every string-typed property on the DataConnections config types must EITHER match
// the scrubber's secret fragments (it gets elided) OR be listed here after a
// deliberate "this is not a secret" decision. A new string property with neither
// fails EveryConfigStringProperty_IsScrubbedOrDeliberatelyNonSecret, so a future
// "SigningKey"/"Pin" cannot silently leak through List/Get/audit.
private static readonly Dictionary<Type, string[]> KnownNonSecretStringProperties = new()
{
[typeof(ZB.MOM.WW.ScadaBridge.Commons.Types.DataConnections.OpcUaEndpointConfig)] = ["EndpointUrl", "SubscriptionDisplayName"],
[typeof(ZB.MOM.WW.ScadaBridge.Commons.Types.DataConnections.OpcUaUserIdentityConfig)] = ["Username", "CertificatePath"],
[typeof(ZB.MOM.WW.ScadaBridge.Commons.Types.DataConnections.MxGatewayEndpointConfig)] = ["Endpoint", "ClientName", "CaFile", "ServerName"],
[typeof(ZB.MOM.WW.ScadaBridge.Commons.Types.DataConnections.OpcUaHeartbeatConfig)] = ["TagPath"],
[typeof(ZB.MOM.WW.ScadaBridge.Commons.Types.DataConnections.OpcUaDeadbandConfig)] = [],
};
[Fact]
public void EveryConfigStringProperty_IsScrubbedOrDeliberatelyNonSecret()
{
foreach (var (type, allowlist) in KnownNonSecretStringProperties)
{
var stringProps = type.GetProperties(System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance)
.Where(p => p.PropertyType == typeof(string))
.Select(p => p.Name);
foreach (var name in stringProps)
{
var scrubbed = ConfigSecretScrubber.IsSecretName(name);
var allowlisted = allowlist.Contains(name);
Assert.True(scrubbed ^ allowlisted,
$"{type.Name}.{name}: must be EITHER fragment-scrubbed OR explicitly " +
"allowlisted as non-secret (and never both) — see arch-review R2 N9.");
}
}
}
[Theory]
[InlineData("Password")]
[InlineData("CertificatePassword")]
[InlineData("ApiKey")]
public void KnownSecretProperties_MatchTheFragmentList(string name)
=> Assert.True(ConfigSecretScrubber.IsSecretName(name));
}
@@ -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; } }
}
}
@@ -145,7 +145,7 @@ public class ManagementActorTests : TestKit, IDisposable
var securedWriteRepo = Substitute.For<ISecuredWriteRepository>();
securedWriteRepo.QueryAsync(
Arg.Any<string?>(), Arg.Any<string?>(), Arg.Any<int>(), Arg.Any<int>(),
Arg.Any<CancellationToken>())
Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
.Returns(Task.FromResult<IReadOnlyList<PendingSecuredWrite>>(
Array.Empty<PendingSecuredWrite>()));
_services.AddScoped(_ => securedWriteRepo);
@@ -151,6 +151,9 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
new(new AuthenticatedUser(username, username, roles, Array.Empty<string>()),
command, Guid.NewGuid().ToString("N"));
private static ManagementEnvelope ScopedEnvelope(object command, string username, string[] permittedSiteIds, params string[] roles) =>
new(new AuthenticatedUser(username, username, roles, permittedSiteIds), command, Guid.NewGuid().ToString("N"));
void IDisposable.Dispose() => Shutdown();
/// <summary>Wires a site whose single connection uses the given protocol.</summary>
@@ -346,7 +349,7 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
[Fact]
public void List_FiltersByStatusAndSite()
{
_securedWriteRepo.QueryAsync("Pending", "SITE1", 0, 200, Arg.Any<CancellationToken>())
_securedWriteRepo.QueryAsync("Pending", "SITE1", 0, 200, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
.Returns(new List<PendingSecuredWrite>
{
new()
@@ -369,13 +372,13 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
Assert.Contains("\"id\":3", response.JsonData);
Assert.Contains("alice", response.JsonData);
_securedWriteRepo.Received(1).QueryAsync("Pending", "SITE1", 0, 200, Arg.Any<CancellationToken>());
_securedWriteRepo.Received(1).QueryAsync("Pending", "SITE1", 0, 200, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>());
}
[Fact]
public void List_NoFilters_PassesNullsToRepository()
{
_securedWriteRepo.QueryAsync(null, null, 0, 200, Arg.Any<CancellationToken>())
_securedWriteRepo.QueryAsync(null, null, 0, 200, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
.Returns(new List<PendingSecuredWrite>());
var actor = CreateActor();
@@ -385,7 +388,49 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
actor.Tell(envelope);
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
_securedWriteRepo.Received(1).QueryAsync(null, null, 0, 200, Arg.Any<CancellationToken>());
_securedWriteRepo.Received(1).QueryAsync(null, null, 0, 200, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>());
}
[Fact]
public void List_SiteScopedReader_ExplicitOutOfScopeSiteFilter_Unauthorized()
{
SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway");
var actor = CreateActor();
actor.Tell(ScopedEnvelope(new ListSecuredWritesCommand(null, "SITE1"), "bob", ["3"], "Verifier"));
ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
}
[Fact]
public void List_SiteScopedReader_Unfiltered_QueriesOnlyPermittedIdentifiers()
{
_siteRepo.GetAllSitesAsync(Arg.Any<CancellationToken>()).Returns(new List<Site>
{
new("Site1", "SITE1") { Id = 1 },
new("Site3", "SITE3") { Id = 3 },
});
_securedWriteRepo.QueryAsync(null, null, 0, 200,
Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
.Returns(new List<PendingSecuredWrite>());
var actor = CreateActor();
actor.Tell(ScopedEnvelope(new ListSecuredWritesCommand(null, null), "bob", ["3"], "Verifier"));
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
_securedWriteRepo.Received(1).QueryAsync(null, null, 0, 200,
Arg.Is<IReadOnlyCollection<string>?>(s => s != null && s.Single() == "SITE3"),
Arg.Any<CancellationToken>());
}
[Fact]
public void List_SystemWideReader_PassesNullPermittedFilter()
{
_securedWriteRepo.QueryAsync(null, null, 0, 200,
Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
.Returns(new List<PendingSecuredWrite>());
var actor = CreateActor();
actor.Tell(Envelope(new ListSecuredWritesCommand(null, null), "carol", "Operator"));
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
_securedWriteRepo.Received(1).QueryAsync(null, null, 0, 200,
Arg.Is<IReadOnlyCollection<string>?>(s => s == null),
Arg.Any<CancellationToken>());
}
// ------------------------------------------------------------------------
@@ -620,16 +665,16 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
[Fact]
public void List_WithSkipTake_ForwardsPagingAndReturnsTotalCount()
{
_securedWriteRepo.QueryAsync(null, null, 10, 25, Arg.Any<CancellationToken>())
_securedWriteRepo.QueryAsync(null, null, 10, 25, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
.Returns(new List<PendingSecuredWrite>());
_securedWriteRepo.CountAsync(null, null, Arg.Any<CancellationToken>()).Returns(87);
_securedWriteRepo.CountAsync(null, null, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>()).Returns(87);
var actor = CreateActor();
actor.Tell(Envelope(new ListSecuredWritesCommand(null, null, Skip: 10, Take: 25), "carol", "Operator"));
var resp = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
_securedWriteRepo.Received(1).QueryAsync(null, null, 10, 25, Arg.Any<CancellationToken>());
_securedWriteRepo.Received(1).CountAsync(null, null, Arg.Any<CancellationToken>());
_securedWriteRepo.Received(1).QueryAsync(null, null, 10, 25, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>());
_securedWriteRepo.Received(1).CountAsync(null, null, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>());
Assert.Contains("\"totalCount\":87", resp.JsonData);
}
@@ -652,7 +697,7 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
ValueJson = "false", ValueType = "Boolean", Status = "Pending",
OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow.AddHours(-25) // TTL default 24h
};
_securedWriteRepo.QueryAsync(null, null, 0, 200, Arg.Any<CancellationToken>())
_securedWriteRepo.QueryAsync(null, null, 0, 200, Arg.Any<IReadOnlyCollection<string>?>(), Arg.Any<CancellationToken>())
.Returns(new List<PendingSecuredWrite> { fresh, stale });
_securedWriteRepo.TryMarkExpiredAsync(2, Arg.Any<DateTime>(), Arg.Any<CancellationToken>())
.Returns(true);
@@ -899,4 +944,86 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
var execute = SingleOfKind(captured, AuditKind.SecuredWriteExecute);
Assert.Equal(ZB.MOM.WW.Audit.AuditOutcome.Failure, execute.Outcome);
}
// ------------------------------------------------------------------------
// Site scope on submit / approve / reject (arch-review R2 N3)
// ------------------------------------------------------------------------
[Fact]
public void Submit_SiteScopedOperator_OutOfScopeSite_Unauthorized_NoRowInserted()
{
SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway");
var actor = CreateActor();
actor.Tell(ScopedEnvelope(
new SubmitSecuredWriteCommand("SITE1", "Gw1", "Tag.A", "true", "Boolean", "go"),
"alice", ["3"], "Operator"));
ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
_securedWriteRepo.DidNotReceiveWithAnyArgs().AddAsync(default!, default);
}
[Fact]
public void Submit_SiteScopedOperator_InScopeSite_Succeeds()
{
SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway");
var actor = CreateActor();
actor.Tell(ScopedEnvelope(
new SubmitSecuredWriteCommand("SITE1", "Gw1", "Tag.A", "true", "Boolean", "go"),
"alice", ["1"], "Operator"));
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
}
[Fact]
public void Approve_SiteScopedVerifier_OutOfScopeSite_Unauthorized_NeverRelays()
{
SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway");
_securedWriteRepo.GetAsync(42, Arg.Any<CancellationToken>()).Returns(new PendingSecuredWrite
{
Id = 42, SiteId = "SITE1", ConnectionName = "Gw1", TagPath = "Tag.A",
ValueJson = "true", ValueType = "Boolean", Status = "Pending",
OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow,
});
var actor = CreateActor();
actor.Tell(ScopedEnvelope(new ApproveSecuredWriteCommand(42, "ok"), "bob", ["3"], "Verifier"));
ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
_securedWriteRepo.DidNotReceiveWithAnyArgs().TryMarkApprovedAsync(default, default!, default, default, default);
Assert.Equal(0, _comms.CallCount); // the device write is NEVER relayed
}
[Fact]
public void Reject_SiteScopedVerifier_OutOfScopeSite_Unauthorized()
{
SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway");
_securedWriteRepo.GetAsync(42, Arg.Any<CancellationToken>()).Returns(new PendingSecuredWrite
{
Id = 42, SiteId = "SITE1", ConnectionName = "Gw1", TagPath = "Tag.A",
ValueJson = "true", ValueType = "Boolean", Status = "Pending",
OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow,
});
var actor = CreateActor();
actor.Tell(ScopedEnvelope(new RejectSecuredWriteCommand(42, "no"), "bob", ["3"], "Verifier"));
ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
_securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default);
}
[Fact]
public void Approve_ScopedAdministrator_BypassesSiteScope()
{
// A scoped Verifier who is ALSO an Administrator bypasses site scope
// (EnforceSiteScope admin bypass, ManagementActor.cs:499) — the approve
// proceeds past the scope gate rather than returning ManagementUnauthorized.
SeedSiteWithConnection(1, "SITE1", "Gw1", "MxGateway");
_securedWriteRepo.GetAsync(42, Arg.Any<CancellationToken>()).Returns(new PendingSecuredWrite
{
Id = 42, SiteId = "SITE1", ConnectionName = "Gw1", TagPath = "Tag.A",
ValueJson = "true", ValueType = "Boolean", Status = "Pending",
OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow,
});
_securedWriteRepo.TryMarkApprovedAsync(
42, Arg.Any<string>(), Arg.Any<string?>(), Arg.Any<DateTime>(), Arg.Any<CancellationToken>())
.Returns(true);
var actor = CreateActor();
actor.Tell(ScopedEnvelope(new ApproveSecuredWriteCommand(42, "ok"), "carol", ["3"], "Verifier", "Administrator"));
var response = ExpectMsg<object>(TimeSpan.FromSeconds(5));
Assert.IsNotType<ManagementUnauthorized>(response);
}
}
@@ -0,0 +1,36 @@
using Microsoft.AspNetCore.HttpOverrides;
using ZB.MOM.WW.ScadaBridge.Security;
using Xunit;
namespace ZB.MOM.WW.ScadaBridge.Security.Tests;
public class ForwardedHeadersSetupTests
{
[Fact]
public void Build_Disabled_ReturnsNull()
=> Assert.Null(ForwardedHeadersSetup.Build(new ForwardedHeadersSecurityOptions { Enabled = false }));
[Fact]
public void Build_Enabled_ParsesProxiesAndNetworks_AndClearsDefaults()
{
var built = ForwardedHeadersSetup.Build(new ForwardedHeadersSecurityOptions
{
Enabled = true,
KnownProxies = ["10.5.0.9"],
KnownNetworks = ["172.16.0.0/12"],
})!;
Assert.Equal(ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto, built.ForwardedHeaders);
Assert.Equal(1, built.ForwardLimit);
Assert.Single(built.KnownProxies); // ONLY the configured proxy —
Assert.DoesNotContain(System.Net.IPAddress.IPv6Loopback, // the loopback default is cleared
built.KnownProxies);
Assert.Single(built.KnownIPNetworks); // net10 successor to the deprecated KnownNetworks
}
[Theory]
[InlineData("not-an-ip", "172.16.0.0/12")]
[InlineData("10.5.0.9", "not-a-cidr")]
public void Build_MalformedEntry_ThrowsAtStartup(string proxy, string network)
=> Assert.ThrowsAny<Exception>(() => ForwardedHeadersSetup.Build(
new ForwardedHeadersSecurityOptions { Enabled = true, KnownProxies = [proxy], KnownNetworks = [network] }));
}
@@ -25,6 +25,12 @@ public class LoginThrottleTests
private static LoginThrottle Create(TimeProvider clock, SecurityOptions? options = null) =>
new(clock, Options.Create(options ?? new SecurityOptions()));
private static (LoginThrottle throttle, TestTimeProvider clock) Create()
{
var clock = new TestTimeProvider(Start);
return (Create(clock), clock);
}
[Fact]
public void FiveFailuresInWindow_LocksOut_ThenWindowExpiryUnlocks()
{
@@ -34,7 +40,7 @@ public class LoginThrottleTests
for (var i = 0; i < 5; i++) throttle.RecordFailure("alice", "10.0.0.1");
Assert.True(throttle.IsLockedOut("alice", "10.0.0.1"));
Assert.False(throttle.IsLockedOut("alice", "10.0.0.2")); // per username+IP key
Assert.False(throttle.IsLockedOut("alice", "10.0.0.2")); // per username+IP key — real isolation in production requires the trusted-proxy ForwardedHeaders config (R2 N2); without it all clients share the proxy IP
Assert.False(throttle.IsLockedOut("bob", "10.0.0.1"));
clock.Advance(TimeSpan.FromMinutes(6)); // lockout window passed
@@ -110,4 +116,29 @@ public class LoginThrottleTests
Assert.False(throttle.IsLockedOut("alice", "10.0.0.1"));
}
// ── arch-review R2 N8: amortized prune ─────────────────────────────────────
[Fact]
public void KeySpray_MapStaysBounded_AtTheHardCap()
{
var (throttle, _) = Create(); // existing helper: throttle over a fake clock
// Spray far past the cap: the over-cap eviction must run on EVERY write once
// the cap is exceeded, regardless of the amortized cadence (arch-review R2 N8).
for (var i = 0; i < LoginThrottle.MaxTrackedKeys + 500; i++)
throttle.RecordFailure($"user{i}", "10.0.0.1");
Assert.True(throttle.TrackedKeyCount <= LoginThrottle.MaxTrackedKeys);
}
[Fact]
public void ExpiredEntries_AreEventuallySwept_OnTheAmortizedCadence()
{
var (throttle, clock) = Create();
throttle.RecordFailure("stale", "10.0.0.1");
clock.Advance(TimeSpan.FromMinutes(10)); // window fully expired
for (var i = 0; i < LoginThrottle.PruneEveryNFailures; i++)
throttle.RecordFailure($"fresh{i}", "10.0.0.1"); // cadence tick fires within these
Assert.False(throttle.IsLockedOut("stale", "10.0.0.1"));
Assert.True(throttle.TrackedKeyCount <= LoginThrottle.PruneEveryNFailures); // "stale" swept
}
}