diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/SiteScopeService.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/SiteScopeService.cs index 95fae501..7566251c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/SiteScopeService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/SiteScopeService.cs @@ -1,3 +1,4 @@ +using System.Security.Claims; using Microsoft.AspNetCore.Components.Authorization; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; using ZB.MOM.WW.ScadaBridge.Security; @@ -89,20 +90,49 @@ public sealed class SiteScopeService return isSystemWide || allowed.Contains(siteId); } + /// + /// The site-scope rule as a pure function of an already-resolved principal — + /// no I/O, no authentication-state fetch, no session revalidation. + /// + /// Every instance method here resolves the AUTHORITATIVE state first (a fail-closed + /// refresh that re-maps groups to roles against the database), which is correct + /// before a mutation but far too expensive for a component that merely decides what + /// to render — and ruinous for one that re-decides on a timer. Display-gating + /// callers holding a principal (the cascading , or + /// a non-forced GetAuthenticationStateAsync) call this instead and pay + /// nothing. Anything that ACTS on a site must still go through + /// so the check is made against fresh claims. + /// + /// + /// The principal to classify. An unauthenticated principal is scoped to nothing. + /// + /// Whether the user is unrestricted, and — when they are not — the set of + /// Site.Id values they may see. The set is always empty for a system-wide user. + /// + public static (bool IsSystemWide, IReadOnlySet Sites) Resolve(ClaimsPrincipal user) + { + ArgumentNullException.ThrowIfNull(user); + return ResolveCore(user); + } + private async Task<(bool IsSystemWide, IReadOnlySet Sites)> ResolveAsync() { var state = await _authStateProvider.GetAuthoritativeStateAsync(); + return ResolveCore(state.User); + } + private static (bool IsSystemWide, IReadOnlySet Sites) ResolveCore(ClaimsPrincipal user) + { // Administrator => system-wide, whatever SiteId claims are present. Read // the role CLAIM rather than IsInRole: the principal's role claim type is // only bound to JwtTokenService.RoleClaimType when the identity was built // by SessionClaimBuilder, and the comparison is OrdinalIgnoreCase to match // ManagementActor's — IsInRole compares claim VALUES ordinally, which would // make the two sides disagree on a differently-cased role name. - if (state.User.Identity?.IsAuthenticated != true) + if (user.Identity?.IsAuthenticated != true) return (false, new HashSet()); - var isAdministrator = state.User + var isAdministrator = user .FindAll(JwtTokenService.RoleClaimType) .Any(c => string.Equals(c.Value, Roles.Administrator, StringComparison.OrdinalIgnoreCase)); @@ -114,7 +144,7 @@ public sealed class SiteScopeService // holds for every caller, so a consumer that reads the id set without // first consulting IsSystemWide cannot silently narrow an Administrator // to a subset of sites. - foreach (var claim in state.User.FindAll(JwtTokenService.SiteIdClaimType)) + foreach (var claim in user.FindAll(JwtTokenService.SiteIdClaimType)) { if (int.TryParse(claim.Value, out var id)) ids.Add(id); diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Layout/PendingUpdatesIndicator.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Layout/PendingUpdatesIndicator.razor index 31cf4156..1ad42bf6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Layout/PendingUpdatesIndicator.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Layout/PendingUpdatesIndicator.razor @@ -1,9 +1,9 @@ @implements IDisposable +@using System.Security.Claims @using ZB.MOM.WW.ScadaBridge.CentralUI.Services @using ZB.MOM.WW.ScadaBridge.DeploymentManager.PendingUpdates @using ZB.MOM.WW.ScadaBridge.Security @inject IPendingUpdatesCache PendingUpdates -@inject SiteScopeService SiteScope @inject AuthenticationStateProvider AuthStateProvider @inject ILogger Logger @@ -12,7 +12,15 @@ and quiet by default: one muted line when the fleet is current, two lines with a warning pill when something is pending. Reads the process-wide single-flight IPendingUpdatesCache, so N circuits polling here cost at most one fleet - flatten per cache TTL. *@ + flatten per cache TTL. + + Roles and site scope are resolved from the CASCADED authentication state, once + per auth state — never per poll. This component lives in MainLayout, so it is + on every page of every circuit: an authoritative (fail-closed, DB-backed) + revalidation on each 30s tick would be exactly the per-circuit cost the shared + cache exists to avoid. Visibility still follows the session — it recomputes on + AuthenticationStateChanged, so a mid-session role revocation takes effect + immediately instead of persisting for the life of the circuit. *@ @if (_visible) {
@@ -20,20 +28,20 @@ { @* A read failure is shown as a failure, never as a clean bill of health; the exception message is the tooltip so it stays out of the rail. *@ - Updates unavailable + Updates unavailable } else if (_snapshot is null) { - Checking for updates… + Checking for updates… } else if (!_snapshot.HasPendingUpdates) { - Up to date + Up to date } else { - Pending updates + Pending updates @(StaleInstances + StaleArtifactSites) @@ -72,18 +80,32 @@ private static readonly TimeSpan PollInterval = TimeSpan.FromSeconds(30); private readonly PollGate _pollGate = new(); + private readonly object _timerLock = new(); private Timer? _timer; private volatile bool _disposed; - private bool _visible; + private volatile bool _visible; private bool _isDeployer; private bool _isAdministrator; + // null => system-wide (no narrowing). A single reference field rather than a + // (bool, set) pair so the poll thread reads one consistent value even if an + // auth-state change is applying concurrently. + private volatile IReadOnlySet? _scopedSiteIds; + private PendingUpdatesSnapshot? _snapshot; private string? _error; private string? _lastLoggedError; + /// + /// The ambient authentication state supplied by the app's + /// CascadingAuthenticationState. Preferred over asking the provider so the + /// principal already resolved for the page is reused rather than re-fetched. + /// + [CascadingParameter] + private Task? AuthenticationStateTask { get; set; } + private int StaleInstances => _snapshot?.StaleInstanceCount ?? 0; private int StaleArtifactSites => _snapshot?.StaleArtifactSiteCount ?? 0; @@ -100,46 +122,127 @@ private static string Plural(int count, string noun) => count == 1 ? $"{count} {noun}" : $"{count} {noun}s"; - protected override async Task OnInitializedAsync() + protected override void OnInitialized() + { + // A mid-session role change (revocation, or a re-login as someone else on + // the same circuit) must move this block, not leave it showing what the + // previous principal was allowed to see. + AuthStateProvider.AuthenticationStateChanged += OnAuthenticationStateChanged; + } + + protected override async Task OnParametersSetAsync() + { + // Non-forced on purpose: this is display gating, not a mutation, so the + // cached principal is the right input (same pattern as Templates.razor / + // FlowDebugger). Anything that ACTS on a site re-checks authoritatively. + var state = AuthenticationStateTask is not null + ? await AuthenticationStateTask + : await AuthStateProvider.GetAuthenticationStateAsync(); + + ApplyPrincipal(state.User); + } + + // Roles and site scope are both pure functions of the principal's claims, so + // one resolution answers "do I render?", "do I link?" and "which sites count?". + private void ApplyPrincipal(ClaimsPrincipal user) { // Role gating is a plain claim check rather than an AuthorizeView: the whole // component is hidden, and the two breakdown links are gated independently, - // so there is no policy to evaluate — just two booleans resolved once. - var state = await AuthStateProvider.GetAuthoritativeStateAsync(); - var roles = state.User.FindAll(JwtTokenService.RoleClaimType).ToList(); + // so there is no policy to evaluate — just two booleans. The comparison is + // OrdinalIgnoreCase to match SiteScopeService/ManagementActor. + var roles = user.FindAll(JwtTokenService.RoleClaimType).ToList(); _isDeployer = roles.Any(c => string.Equals(c.Value, Roles.Deployer, StringComparison.OrdinalIgnoreCase)); _isAdministrator = roles.Any(c => string.Equals(c.Value, Roles.Administrator, StringComparison.OrdinalIgnoreCase)); _visible = _isDeployer || _isAdministrator; + + var (isSystemWide, sites) = SiteScopeService.Resolve(user); + _scopedSiteIds = isSystemWide ? null : sites; + } + + private void OnAuthenticationStateChanged(Task task) + => _ = ApplyAuthenticationStateAsync(task); + + private async Task ApplyAuthenticationStateAsync(Task task) + { + if (_disposed) + { + return; + } + + try + { + var state = await task; + if (_disposed) + { + return; + } + + ApplyPrincipal(state.User); + + // A role GRANTED mid-session has to start the poll that first render + // skipped; a role revoked just stops rendering (LoadAsync no-ops). + if (_visible) + { + EnsurePollingStarted(); + } + } + catch (Exception ex) + { + // Never let an auth-state notification tear down the circuit: the worst + // case is that this block keeps its previous visibility. + Logger.LogWarning(ex, "Pending-updates indicator could not apply an authentication state change."); + return; + } + + await RenderStateAsync(); } protected override Task OnAfterRenderAsync(bool firstRender) { - if (!firstRender || !_visible || _disposed) + if (firstRender && _visible && !_disposed) { - return Task.CompletedTask; + EnsurePollingStarted(); } - // Deliberately NOT awaited here and deliberately not in OnInitializedAsync: - // a cold fleet flatten takes seconds, and this component sits in the layout — - // awaiting it would hold the first render of every page behind it. The rail - // paints "Checking for updates…" and fills in when the snapshot lands. - _ = LoadAsync(); - - _timer = new Timer( - _ => _ = _pollGate.RunAsync( - LoadAsync, - error => Logger.LogDebug(error, "Pending-updates indicator poll tick failed.")), - null, - PollInterval, - PollInterval); - return Task.CompletedTask; } + private void EnsurePollingStarted() + { + lock (_timerLock) + { + if (_disposed || _timer is not null) + { + return; + } + + _timer = new Timer(_ => _ = RunLoadAsync(), null, PollInterval, PollInterval); + } + + // Deliberately NOT awaited and deliberately not done in OnInitializedAsync: + // a cold fleet flatten takes seconds, and this component sits in the layout — + // awaiting it would hold the first render of every page behind it. The rail + // paints "Checking for updates…" and fills in when the snapshot lands. The + // first load goes through the same gate as a tick so nothing can escape as + // an unobserved faulted task. + _ = RunLoadAsync(); + } + + /// + /// Test seam: runs one poll cycle exactly as the timer tick does, through the + /// same gate. Exists so a test can pin the load-bearing cost property — that a + /// poll resolves no authentication state — without waiting out the interval. + /// + internal Task PollOnceAsync() => RunLoadAsync(); + + private Task RunLoadAsync() => _pollGate.RunAsync( + LoadAsync, + error => Logger.LogDebug(error, "Pending-updates indicator poll tick failed.")); + private async Task LoadAsync() { - if (_disposed) + if (_disposed || !_visible) { return; } @@ -149,12 +252,18 @@ var snapshot = await PendingUpdates.GetAsync(); // A site-scoped operator's totals must never include a site they cannot - // see — same rule Topology and the Sites page apply. - if (!await SiteScope.IsSystemWideAsync()) + // see — same rule Topology and the Sites page apply. The scope was + // resolved from the principal, so narrowing costs nothing here. + var scope = _scopedSiteIds; + if (scope is not null) { - snapshot = snapshot.ForSites(await SiteScope.PermittedSiteIdsAsync()); + snapshot = snapshot.ForSites(scope); } + // These writes happen off the renderer's synchronization context by + // design: the fleet read must not occupy the UI thread, PollGate's + // single-flight guarantees there is never a second concurrent writer, + // and only the RE-RENDER is marshalled back via InvokeAsync below. _snapshot = snapshot; _error = null; _lastLoggedError = null; @@ -192,14 +301,21 @@ } catch (InvalidOperationException) { - // Renderer already disposed — same race, different throw site. + // Same race, earlier throw site: dispatching to a renderer whose circuit + // is already tearing down throws this before the ObjectDisposedException + // path is reachable. } } public void Dispose() { _disposed = true; - _timer?.Dispose(); - _timer = null; + AuthStateProvider.AuthenticationStateChanged -= OnAuthenticationStateChanged; + + lock (_timerLock) + { + _timer?.Dispose(); + _timer = null; + } } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/SiteScopeServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/SiteScopeServiceTests.cs index e78fa568..04cd97bc 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/SiteScopeServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/SiteScopeServiceTests.cs @@ -161,4 +161,67 @@ public class SiteScopeServiceTests Assert.False(await svc.IsSystemWideAsync()); Assert.False(await svc.IsSiteAllowedAsync(2)); } + + // ── Resolve(ClaimsPrincipal): the same rule as a pure function ─────────── + // Display-gating callers (the rail pending-updates indicator) hold a principal + // already and must NOT pay for the instance methods' authoritative, fail-closed + // state refresh — which for a polling component would be a forced session + // revalidation per tick per circuit. These pin that the pure overload answers + // identically to the async path. + + private static ClaimsPrincipal Principal(params Claim[] claims) + => new(new ClaimsIdentity(claims, authenticationType: "TestCookie")); + + [Fact] + public void Resolve_Administrator_IsSystemWideWithAnEmptySet() + { + // Even carrying SiteId claims: an Administrator's ids are not collected at + // all, so "system-wide => empty set" holds for every caller. + var (isSystemWide, sites) = SiteScopeService.Resolve( + Principal(Role("Administrator"), SiteClaim(1), SiteClaim(2))); + + Assert.True(isSystemWide); + Assert.Empty(sites); + } + + [Fact] + public void Resolve_SiteClaims_YieldTheScopedSet() + { + var (isSystemWide, sites) = SiteScopeService.Resolve( + Principal(Role("Deployer"), SiteClaim(1), SiteClaim(3))); + + Assert.False(isSystemWide); + Assert.Equal([1, 3], sites.OrderBy(id => id)); + } + + [Fact] + public void Resolve_NoSiteClaims_IsSystemWide() + { + var (isSystemWide, sites) = SiteScopeService.Resolve(Principal(Role("Deployer"))); + + Assert.True(isSystemWide); + Assert.Empty(sites); + } + + [Fact] + public void Resolve_UnauthenticatedPrincipal_IsScopedToNothing() + { + // Fail closed: not system-wide AND no permitted sites. + var (isSystemWide, sites) = SiteScopeService.Resolve(new ClaimsPrincipal(new ClaimsIdentity())); + + Assert.False(isSystemWide); + Assert.Empty(sites); + } + + [Fact] + public async Task Resolve_AgreesWithTheAsyncPath() + { + var claims = new[] { Role("Deployer"), SiteClaim(2), SiteClaim(5) }; + var svc = ForUser(claims); + + var (isSystemWide, sites) = SiteScopeService.Resolve(Principal(claims)); + + Assert.Equal(await svc.IsSystemWideAsync(), isSystemWide); + Assert.Equal((await svc.PermittedSiteIdsAsync()).OrderBy(id => id), sites.OrderBy(id => id)); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Layout/PendingUpdatesIndicatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Layout/PendingUpdatesIndicatorTests.cs index ba2f4c82..b0e382b0 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Layout/PendingUpdatesIndicatorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Layout/PendingUpdatesIndicatorTests.cs @@ -1,5 +1,6 @@ using System.Security.Claims; using Bunit; +using Microsoft.AspNetCore.Components; using Microsoft.AspNetCore.Components.Authorization; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -20,6 +21,12 @@ namespace ZB.MOM.WW.ScadaBridge.CentralUI.Tests.Layout; /// (it advertises fleet state), its two breakdown links are gated independently by the /// role that can actually act on each (Topology ⇒ Deployer, Sites ⇒ Administrator), and /// a site-scoped operator's totals count only their permitted sites. +/// +/// A fourth property is pinned for cost rather than correctness: because this block +/// lives in MainLayout, roles and scope must be resolved once per authentication +/// state, NEVER once per 30s poll — a per-poll resolve would be a forced session +/// revalidation on every page of every circuit. +/// /// public class PendingUpdatesIndicatorTests : BunitContext { @@ -74,6 +81,47 @@ public class PendingUpdatesIndicatorTests : BunitContext return Render(); } + /// An auth provider that records how many times its state was asked for. + private sealed class CountingAuthStateProvider : AuthenticationStateProvider + { + private readonly ClaimsPrincipal _user; + + public CountingAuthStateProvider(ClaimsPrincipal user) => _user = user; + + public int Calls { get; private set; } + + public override Task GetAuthenticationStateAsync() + { + Calls++; + return Task.FromResult(new AuthenticationState(_user)); + } + } + + /// + /// Renders the indicator under an explicit cascaded — + /// the same cascade CascadingAuthenticationState supplies in the real app. + /// + private IRenderedComponent RenderCascaded( + IPendingUpdatesCache cache, + AuthenticationStateProvider provider, + ClaimsPrincipal cascadedUser) + { + Services.AddSingleton(provider); + Services.AddScoped(); + Services.AddSingleton(cache); + Services.AddSingleton>(NullLogger.Instance); + + var host = Render>>(parameters => parameters + .Add(p => p.Value, Task.FromResult(new AuthenticationState(cascadedUser))) + .Add(p => p.ChildContent, (RenderFragment)(builder => + { + builder.OpenComponent(0); + builder.CloseComponent(); + }))); + + return host.FindComponent(); + } + [Fact] public void Viewer_RendersNothing() { @@ -229,4 +277,96 @@ public class PendingUpdatesIndicatorTests : BunitContext instance.Dispose(); instance.Dispose(); } + + [Fact] + public void CascadedAuthenticationState_IsUsedInsteadOfAskingTheProvider() + { + // The provider would report an ANONYMOUS user, so the block can only render + // if the component read the cascaded principal — which is what the app's + // CascadingAuthenticationState supplies. + var anonymous = new CountingAuthStateProvider(new ClaimsPrincipal(new ClaimsIdentity())); + var deployer = new ClaimsPrincipal(new ClaimsIdentity([Role(Roles.Deployer)], "TestAuth")); + + var cut = RenderCascaded( + CacheReturning(Snapshot(SiteRow(1, staleInstances: 0))), + anonymous, + deployer); + + cut.WaitForAssertion(() => Assert.Contains("Up to date", cut.Markup)); + Assert.Equal(0, anonymous.Calls); + } + + [Fact] + public async Task Polling_ResolvesNoFurtherAuthenticationState() + { + // The regression this pins: SiteScopeService's instance methods resolve the + // AUTHORITATIVE state (a fail-closed, DB-backed group→role re-mapping). Doing + // that per tick, from a component that sits in the layout, would cost one or + // two forced session revalidations per circuit every 30s on every page. + var provider = new CountingAuthStateProvider( + new ClaimsPrincipal(new ClaimsIdentity([Role(Roles.Deployer), SiteClaim(1)], "TestAuth"))); + var cache = CacheReturning(Snapshot(SiteRow(1, staleInstances: 2))); + + Services.AddSingleton(provider); + Services.AddScoped(); + Services.AddSingleton(cache); + Services.AddSingleton>(NullLogger.Instance); + + var cut = Render(); + cut.WaitForAssertion(() => Assert.Contains("2 instances", cut.Markup)); + + var afterFirstLoad = provider.Calls; + + // Three more poll cycles, exactly as the timer drives them. + for (var i = 0; i < 3; i++) + { + await cut.Instance.PollOnceAsync(); + } + + // Each poll read the fleet cache; none of them re-resolved the principal. + _ = cache.Received(4).GetAsync(Arg.Any(), Arg.Any()); + Assert.Equal(afterFirstLoad, provider.Calls); + + // Scope narrowing still applies on every poll — it reads the resolved fields. + Assert.Contains("2 instances", cut.Markup); + } + + [Fact] + public void AuthenticationStateChange_HidesTheBlockWhenTheRoleIsRevoked() + { + var cache = CacheReturning(Snapshot(SiteRow(1, staleInstances: 0))); + var provider = new MutableAuthStateProvider( + new ClaimsPrincipal(new ClaimsIdentity([Role(Roles.Deployer)], "TestAuth"))); + + Services.AddSingleton(provider); + Services.AddScoped(); + Services.AddSingleton(cache); + Services.AddSingleton>(NullLogger.Instance); + + var cut = Render(); + cut.WaitForAssertion(() => Assert.Contains("Up to date", cut.Markup)); + + // Role revoked mid-session: the block must go, not linger for the life of + // the circuit showing what the previous principal could see. + provider.SetUser(new ClaimsPrincipal(new ClaimsIdentity([Role(Roles.Viewer)], "TestAuth"))); + + cut.WaitForAssertion(() => Assert.True(string.IsNullOrWhiteSpace(cut.Markup))); + } + + /// An auth provider whose principal can be swapped mid-test, raising the change event. + private sealed class MutableAuthStateProvider : AuthenticationStateProvider + { + private ClaimsPrincipal _user; + + public MutableAuthStateProvider(ClaimsPrincipal user) => _user = user; + + public override Task GetAuthenticationStateAsync() + => Task.FromResult(new AuthenticationState(_user)); + + public void SetUser(ClaimsPrincipal user) + { + _user = user; + NotifyAuthenticationStateChanged(GetAuthenticationStateAsync()); + } + } }