fix(central-ui): pending-updates indicator resolves roles/scope once per auth state, not per poll
SiteScopeService's instance methods resolve the AUTHORITATIVE state — a fail-closed, DB-backed group-to-role re-mapping. The indicator called them on every 30s tick, and it lives in MainLayout, so that was one or two forced session revalidations per open circuit per page, forever: exactly the per-circuit cost IPendingUpdatesCache exists to avoid. Extracts the pure rule as SiteScopeService.Resolve(ClaimsPrincipal) (no I/O; ResolveAsync now delegates to it, no behaviour change for existing callers). The indicator takes the cascaded AuthenticationState — falling back to a NON-forced GetAuthenticationStateAsync, as Templates/FlowDebugger do — and computes roles plus scope once per auth state, re-running on AuthenticationStateChanged so a mid-session revocation hides the block instead of leaving it stale for the life of the circuit. Also routes the first load through PollGate, adds aria-hidden to the icons, and comments the off-renderer field writes and the InvalidOperationException catch. Claude-Session: https://claude.ai/code/session_01Y5ef34iUUMjmwnMpL9xgmB
This commit is contained in:
@@ -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));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
/// <para>
|
||||
/// 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.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public class PendingUpdatesIndicatorTests : BunitContext
|
||||
{
|
||||
@@ -74,6 +81,47 @@ public class PendingUpdatesIndicatorTests : BunitContext
|
||||
return Render<PendingUpdatesIndicator>();
|
||||
}
|
||||
|
||||
/// <summary>An auth provider that records how many times its state was asked for.</summary>
|
||||
private sealed class CountingAuthStateProvider : AuthenticationStateProvider
|
||||
{
|
||||
private readonly ClaimsPrincipal _user;
|
||||
|
||||
public CountingAuthStateProvider(ClaimsPrincipal user) => _user = user;
|
||||
|
||||
public int Calls { get; private set; }
|
||||
|
||||
public override Task<AuthenticationState> GetAuthenticationStateAsync()
|
||||
{
|
||||
Calls++;
|
||||
return Task.FromResult(new AuthenticationState(_user));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Renders the indicator under an explicit cascaded <see cref="AuthenticationState"/> —
|
||||
/// the same cascade <c>CascadingAuthenticationState</c> supplies in the real app.
|
||||
/// </summary>
|
||||
private IRenderedComponent<PendingUpdatesIndicator> RenderCascaded(
|
||||
IPendingUpdatesCache cache,
|
||||
AuthenticationStateProvider provider,
|
||||
ClaimsPrincipal cascadedUser)
|
||||
{
|
||||
Services.AddSingleton(provider);
|
||||
Services.AddScoped<SiteScopeService>();
|
||||
Services.AddSingleton(cache);
|
||||
Services.AddSingleton<ILogger<PendingUpdatesIndicator>>(NullLogger<PendingUpdatesIndicator>.Instance);
|
||||
|
||||
var host = Render<CascadingValue<Task<AuthenticationState>>>(parameters => parameters
|
||||
.Add(p => p.Value, Task.FromResult(new AuthenticationState(cascadedUser)))
|
||||
.Add(p => p.ChildContent, (RenderFragment)(builder =>
|
||||
{
|
||||
builder.OpenComponent<PendingUpdatesIndicator>(0);
|
||||
builder.CloseComponent();
|
||||
})));
|
||||
|
||||
return host.FindComponent<PendingUpdatesIndicator>();
|
||||
}
|
||||
|
||||
[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<AuthenticationStateProvider>(provider);
|
||||
Services.AddScoped<SiteScopeService>();
|
||||
Services.AddSingleton(cache);
|
||||
Services.AddSingleton<ILogger<PendingUpdatesIndicator>>(NullLogger<PendingUpdatesIndicator>.Instance);
|
||||
|
||||
var cut = Render<PendingUpdatesIndicator>();
|
||||
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<bool>(), Arg.Any<CancellationToken>());
|
||||
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<AuthenticationStateProvider>(provider);
|
||||
Services.AddScoped<SiteScopeService>();
|
||||
Services.AddSingleton(cache);
|
||||
Services.AddSingleton<ILogger<PendingUpdatesIndicator>>(NullLogger<PendingUpdatesIndicator>.Instance);
|
||||
|
||||
var cut = Render<PendingUpdatesIndicator>();
|
||||
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)));
|
||||
}
|
||||
|
||||
/// <summary>An auth provider whose principal can be swapped mid-test, raising the change event.</summary>
|
||||
private sealed class MutableAuthStateProvider : AuthenticationStateProvider
|
||||
{
|
||||
private ClaimsPrincipal _user;
|
||||
|
||||
public MutableAuthStateProvider(ClaimsPrincipal user) => _user = user;
|
||||
|
||||
public override Task<AuthenticationState> GetAuthenticationStateAsync()
|
||||
=> Task.FromResult(new AuthenticationState(_user));
|
||||
|
||||
public void SetUser(ClaimsPrincipal user)
|
||||
{
|
||||
_user = user;
|
||||
NotifyAuthenticationStateChanged(GetAuthenticationStateAsync());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user