diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/SiteScopeService.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/SiteScopeService.cs index 7566251c..e100664d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/SiteScopeService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/SiteScopeService.cs @@ -94,14 +94,17 @@ public sealed class SiteScopeService /// 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. + /// Every instance method here resolves the state through the provider first, which + /// on CookieAuthenticationStateProvider re-maps groups to roles against the + /// database. Note that the non-authoritative overload is NOT a cheap alternative — + /// both entry points refresh, differing only in whether a mapping fault denies or + /// reuses the old grants — so the way to avoid the cost is to not ask the provider + /// at all. A display-gating caller that already HOLDS a principal (the cascading + /// the router resolved for the page) calls this + /// instead and triggers no refresh of its own; that is what makes the rule usable + /// from a component that re-decides on a timer. Anything that ACTS on a site must + /// still go through so the check is made against + /// freshly revalidated claims. /// /// /// The principal to classify. An unauthenticated principal is scoped to nothing. 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 1ad42bf6..05f7a8d1 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Layout/PendingUpdatesIndicator.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Layout/PendingUpdatesIndicator.razor @@ -14,13 +14,18 @@ IPendingUpdatesCache, so N circuits polling here cost at most one fleet 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. *@ + Roles and site scope are resolved from the CASCADED authentication state the + router already supplies (AddCascadingAuthenticationState), once per auth state + — never per poll. That matters because this component lives in MainLayout, so + it is on every page of every circuit: note that on this app's + CookieAuthenticationStateProvider BOTH entry points revalidate the session + against the database (they differ only in failClosed, not in whether they + refresh), so the saving is not "use the cheap overload" — it is that the + component never asks the provider at all on a tick, and reuses the principal + the router resolved rather than triggering a refresh of its own. 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) {
@@ -94,14 +99,21 @@ // auth-state change is applying concurrently. private volatile IReadOnlySet? _scopedSiteIds; + // Result of the provider FALLBACK below, kept so it is resolved at most once per + // auth state instead of on every parameter set. + private ClaimsPrincipal? _fallbackPrincipal; + 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. + /// CascadingAuthenticationState (registered by + /// AddCascadingAuthenticationState, so it is always present in the running + /// app). Preferred over asking the provider because this task is ALREADY resolved + /// for the page: reusing it means this component never triggers a session + /// revalidation of its own, whereas every provider entry point does refresh. /// [CascadingParameter] private Task? AuthenticationStateTask { get; set; } @@ -132,14 +144,32 @@ 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(); + ClaimsPrincipal user; + if (AuthenticationStateTask is not null) + { + user = (await AuthenticationStateTask).User; + } + else + { + // Fallback for a host with no cascade. AddCascadingAuthenticationState() is + // registered, so this is expected to be dead in production — and it is a + // REAL session refresh (non-forced differs only in failClosed), which is why + // it is taken at most once and then reused until the state changes. + _fallbackPrincipal ??= (await AuthStateProvider.GetAuthenticationStateAsync()).User; + user = _fallbackPrincipal; + } - ApplyPrincipal(state.User); + ApplyPrincipal(user); + + // MUST be here, not only in OnAfterRenderAsync: the await above can suspend, in + // which case the FIRST render runs with _visible still false and skips the + // OnAfterRender(firstRender: true) start. The render that follows the resolve is + // firstRender: false, so nothing else would ever start the poll and the block + // would sit on "Checking for updates…" for the life of the circuit. Idempotent. + if (_visible) + { + EnsurePollingStarted(); + } } // Roles and site scope are both pure functions of the principal's claims, so @@ -178,6 +208,7 @@ return; } + _fallbackPrincipal = state.User; ApplyPrincipal(state.User); // A role GRANTED mid-session has to start the poll that first render 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 b0e382b0..a2c52217 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Layout/PendingUpdatesIndicatorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Layout/PendingUpdatesIndicatorTests.cs @@ -369,4 +369,47 @@ public class PendingUpdatesIndicatorTests : BunitContext NotifyAuthenticationStateChanged(GetAuthenticationStateAsync()); } } + + [Fact] + public void Polling_StartsAfterAnAsynchronousAuthResolveCompletes() + { + // Regression: OnParametersSetAsync awaits the principal. On a real cookie + // session that await suspends, so the FIRST render happens with _visible + // still false and OnAfterRenderAsync(firstRender: true) skips starting the + // poll; the render that follows the resolve is firstRender: false. Without a + // start from OnParametersSetAsync the block sits on "Checking for updates…" + // for the life of the circuit. + var gate = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + var cache = CacheReturning(Snapshot(SiteRow(1, staleInstances: 4))); + + Services.AddSingleton(new DeferredAuthStateProvider(gate.Task)); + Services.AddScoped(); + Services.AddSingleton(cache); + Services.AddSingleton>(NullLogger.Instance); + + var cut = Render(); + + // Nothing resolved yet: no principal, so nothing rendered and nothing polled. + Assert.True(string.IsNullOrWhiteSpace(cut.Markup)); + + gate.SetResult(new AuthenticationState( + new ClaimsPrincipal(new ClaimsIdentity([Role(Roles.Deployer)], "TestAuth")))); + + cut.WaitForAssertion(() => + { + _ = cache.Received().GetAsync(Arg.Any(), Arg.Any()); + Assert.Contains("4 instances", cut.Markup); + }); + } + + /// An auth provider whose state resolves only when the supplied task completes. + private sealed class DeferredAuthStateProvider : AuthenticationStateProvider + { + private readonly Task _state; + + public DeferredAuthStateProvider(Task state) => _state = state; + + public override Task GetAuthenticationStateAsync() => _state; + } }