From 3a21cfc2f533fa8b73e416c4460ede5f9ffc67e5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 14 Sep 2026 14:29:36 -0400 Subject: [PATCH] fix(central-ui): pending-updates indicator starts polling once the async auth resolve completes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OnParametersSetAsync awaits the principal. On a real cookie session that await suspends, so the FIRST render ran with _visible still false and OnAfterRenderAsync(firstRender: true) skipped EnsurePollingStarted(); the render that followed the resolve was firstRender: false, and nothing else started the poll unless AuthenticationStateChanged happened to fire. The block therefore rendered and sat on 'Checking for updates…' for the life of the circuit. OnParametersSetAsync now starts polling itself when visible (the start is idempotent and the OnAfterRender call is kept). Also corrects the comments that called the non-forced GetAuthenticationStateAsync cheap: on CookieAuthenticationStateProvider both entry points revalidate against the database and differ only in failClosed, so the saving is not asking the provider on a tick and reusing the cascaded state the router already resolved. The provider fallback is a real refresh, is only taken when no cascade is present (AddCascadingAuthenticationState is registered, so it is dead in production), and is now resolved at most once per auth state rather than on every parameter set. Claude-Session: https://claude.ai/code/session_01Y5ef34iUUMjmwnMpL9xgmB --- .../Auth/SiteScopeService.cs | 19 +++--- .../Layout/PendingUpdatesIndicator.razor | 63 ++++++++++++++----- .../Layout/PendingUpdatesIndicatorTests.cs | 43 +++++++++++++ 3 files changed, 101 insertions(+), 24 deletions(-) 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; + } }