From f0de11c44607eb9970d38c4c6e6c235e455e4015 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:38:44 -0400 Subject: [PATCH] fix(centralui-auth): memoize non-authoritative circuit auth-state reads for 10 s (review 2026-10-21 07/P-3) GetAuthenticationStateAsync (display reads: current username, nav role checks) used to force a full revalidation + DB role re-map on every call. It now reuses the circuit's last revalidation for NonAuthoritativeRefreshInterval (10 s); GetAuthoritativeStateAsync (mutation gates, UI-13) still re-maps every time and fails closed. The revocation regression test now asserts through the authoritative read, which is what every gate uses. --- .../Auth/CookieAuthenticationStateProvider.cs | 37 +++++++- .../Auth/CircuitRevocationRegressionTests.cs | 5 +- .../CookieAuthenticationStateMemoTests.cs | 91 +++++++++++++++++++ 3 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/CookieAuthenticationStateMemoTests.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/CookieAuthenticationStateProvider.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/CookieAuthenticationStateProvider.cs index bd4997bd..6edc23e0 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/CookieAuthenticationStateProvider.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Auth/CookieAuthenticationStateProvider.cs @@ -15,28 +15,58 @@ namespace ZB.MOM.WW.ScadaBridge.CentralUI.Auth; /// against current server state. Fresh scopes avoid sharing the circuit's DbContext. /// Never reads IHttpContextAccessor after construction. /// +/// +/// +/// (mutation gates, UI-13) re-maps the stored LDAP +/// groups on every call and fails closed on a mapping fault. +/// +/// +/// (display reads — the current username for a save +/// button, a nav-menu role check) is memoized per circuit for +/// (review 2026-10-21 07/P-3): within that window +/// it returns the state of the last revalidation, authoritative or not, instead of running the +/// role mapping again. A revoked grant can therefore linger on a display surface for at most +/// that window, while every mutation still revalidates. The underlying mapping itself is +/// served from , so even a revalidation is normally DB-free. +/// +/// public class CookieAuthenticationStateProvider : ServerAuthenticationStateProvider { + /// How long a non-authoritative read may reuse the circuit's last revalidated state. + public static readonly TimeSpan NonAuthoritativeRefreshInterval = TimeSpan.FromSeconds(10); + private AuthenticationState _state; private readonly IServiceScopeFactory? _scopes; private readonly bool _autoLogin; + private readonly TimeProvider _clock; private readonly SemaphoreSlim _gate = new(1, 1); + // UTC ticks of the last completed revalidation; 0 = never. Read outside the gate. + private long _lastRefreshTicks; + /// Captures identity and the factory used for subsequent server-side revalidation. /// Supplies the user captured at construction. /// Factory for fresh scopes used during revalidation. /// Options indicating whether login is disabled (auto-login). - public CookieAuthenticationStateProvider(IHttpContextAccessor httpContextAccessor, IServiceScopeFactory? scopes = null, IOptions? autoLogin = null) + /// Clock for the non-authoritative memo window; defaults to the system clock. + public CookieAuthenticationStateProvider(IHttpContextAccessor httpContextAccessor, IServiceScopeFactory? scopes = null, IOptions? autoLogin = null, TimeProvider? clock = null) { _state = new AuthenticationState(httpContextAccessor.HttpContext?.User ?? new ClaimsPrincipal(new ClaimsIdentity())); _scopes = scopes; // Trusted startup configuration, already checked by DisableLoginGuard. The // auto-login handler deliberately uses the Cookies scheme with no LDAP groups. _autoLogin = autoLogin?.Value.DisableLogin == true; + _clock = clock ?? TimeProvider.System; } /// - public override Task GetAuthenticationStateAsync() => RefreshAsync(failClosed: false); + public override Task GetAuthenticationStateAsync() + { + var last = Interlocked.Read(ref _lastRefreshTicks); + if (last != 0 && _clock.GetUtcNow().UtcTicks - last < NonAuthoritativeRefreshInterval.Ticks) + return Task.FromResult(Volatile.Read(ref _state)); + return RefreshAsync(failClosed: false); + } /// Requires a current mapping before a mutation; mapping faults deny rather than reuse old grants. /// The authoritative authentication state. @@ -61,7 +91,8 @@ public class CookieAuthenticationStateProvider : ServerAuthenticationStateProvid _ => _state.User, }; changed = Authority(_state.User) != Authority(user); - _state = state = new AuthenticationState(user); + Volatile.Write(ref _state, state = new AuthenticationState(user)); + Interlocked.Exchange(ref _lastRefreshTicks, _clock.GetUtcNow().UtcTicks); } finally { _gate.Release(); } if (changed) NotifyAuthenticationStateChanged(Task.FromResult(state)); diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/CircuitRevocationRegressionTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/CircuitRevocationRegressionTests.cs index e2cc8d21..41107151 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/CircuitRevocationRegressionTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/CircuitRevocationRegressionTests.cs @@ -66,7 +66,10 @@ public class CircuitRevocationRegressionTests var sites = new SiteScopeService(auth); Assert.True(await sites.IsSiteAllowedAsync(1)); mapping = new RoleMappingResult([Roles.Viewer], ["2"], false); - var refreshed = await auth.GetAuthenticationStateAsync(); + // Authoritative read: a non-authoritative read may reuse the circuit's last + // revalidation for CookieAuthenticationStateProvider.NonAuthoritativeRefreshInterval + // (review 2026-10-21 07/P-3); every gate below is authoritative. + var refreshed = await auth.GetAuthoritativeStateAsync(); Assert.DoesNotContain(Roles.Deployer, refreshed.User.FindAll(JwtTokenService.RoleClaimType).Select(c => c.Value)); Assert.False(await sites.IsSiteAllowedAsync(1)); Assert.True(await sites.IsSiteAllowedAsync(2)); diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/CookieAuthenticationStateMemoTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/CookieAuthenticationStateMemoTests.cs new file mode 100644 index 00000000..823f008a --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Auth/CookieAuthenticationStateMemoTests.cs @@ -0,0 +1,91 @@ +using Microsoft.AspNetCore.Components.Authorization; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using NSubstitute; +using ZB.MOM.WW.Auth.Abstractions.Roles; +using ZB.MOM.WW.ScadaBridge.CentralUI.Auth; +using ZB.MOM.WW.ScadaBridge.Security; + +namespace ZB.MOM.WW.ScadaBridge.CentralUI.Tests.Auth; + +/// +/// Review 2026-10-21 07/P-3: a non-authoritative +/// no longer re-runs the role mapping on every read — it reuses the circuit's last revalidation +/// for — while +/// (mutation gates) +/// still re-maps every time. +/// +public class CookieAuthenticationStateMemoTests +{ + private sealed class MutableClock : TimeProvider + { + public DateTimeOffset Now { get; set; } = DateTimeOffset.UtcNow; + + public override DateTimeOffset GetUtcNow() => Now; + } + + private static (CookieAuthenticationStateProvider Auth, IGroupRoleMapper Mapper, MutableClock Clock, ServiceProvider Provider) Create() + { + var clock = new MutableClock(); + var mapping = new RoleMappingResult([Roles.Designer], [], false); + var mapper = Substitute.For>(); + mapper.MapAsync(Arg.Any>(), Arg.Any()) + .Returns(_ => new GroupRoleMapping(mapping.Roles, Scope: mapping)); + var initial = SessionClaimBuilder.Build("alice", "Alice", ["group"], mapping, clock.Now); + var services = new ServiceCollection().AddLogging(); + services.AddSingleton(mapper); + services.AddSingleton(clock); + services.AddSingleton(Options.Create(new SecurityOptions())); + services.AddScoped(); + services.AddSingleton(new HttpContextAccessor { HttpContext = new DefaultHttpContext { User = initial } }); + services.AddScoped(); + var provider = services.BuildServiceProvider(); + var auth = (CookieAuthenticationStateProvider)provider.CreateScope().ServiceProvider.GetRequiredService(); + return (auth, mapper, clock, provider); + } + + /// Repeated display reads within the window run the role mapping once. + /// A task that represents the asynchronous operation. + [Fact] + public async Task NonAuthoritativeReads_WithinWindow_MapOnce() + { + var (auth, mapper, _, provider) = Create(); + using var _p = provider; + + for (var i = 0; i < 5; i++) + Assert.Equal("alice", (await auth.GetCurrentUsernameAsync())); + + await mapper.Received(1).MapAsync(Arg.Any>(), Arg.Any()); + } + + /// After the window the next display read revalidates again. + /// A task that represents the asynchronous operation. + [Fact] + public async Task NonAuthoritativeRead_AfterWindow_Revalidates() + { + var (auth, mapper, clock, provider) = Create(); + using var _p = provider; + + await auth.GetAuthenticationStateAsync(); + clock.Now += CookieAuthenticationStateProvider.NonAuthoritativeRefreshInterval + TimeSpan.FromSeconds(1); + await auth.GetAuthenticationStateAsync(); + + await mapper.Received(2).MapAsync(Arg.Any>(), Arg.Any()); + } + + /// Authoritative reads (mutation gates) always re-map, even inside the window. + /// A task that represents the asynchronous operation. + [Fact] + public async Task AuthoritativeReads_AlwaysRemap() + { + var (auth, mapper, _, provider) = Create(); + using var _p = provider; + + await auth.GetAuthenticationStateAsync(); + await auth.GetAuthoritativeStateAsync(); + await auth.GetAuthoritativeStateAsync(); + + await mapper.Received(3).MapAsync(Arg.Any>(), Arg.Any()); + } +}