From 042e2efd1d7c7dafe9c6c20b1b5676e23977716d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 19 Aug 2026 13:45:12 -0400 Subject: [PATCH] =?UTF-8?q?fix(centralui):=20enforce=20site=20scoping=20on?= =?UTF-8?q?=20AlarmSummary=20=E2=80=94=20filter=20site=20list=20+=20deny?= =?UTF-8?q?=20out-of-scope=20alarm=20fetches,=20mirroring=20DebugView=20(a?= =?UTF-8?q?rch-review=202026-08-19=20P6-2=20High)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AlarmSummary.razor was gated RequireDeployment but never consulted SiteScopeService: the site picker listed every site unfiltered and GetSiteAlarmsAsync ran for whatever siteId the onchange event carried, so a Deployer scoped to one site could watch any site's live alarms (including CurrentValue/LimitValue process data). The page was missed by the ad0100f2 scoping survey. Fix mirrors EventLogs.razor/DebugView.razor: OnInitializedAsync filters _sites through SiteScope.FilterSitesAsync, and both the site-change handler and RefreshAsync (the shared fetch path used by the manual refresh button and the 15s poll timer) independently re-check the selected site against the filtered list before ever calling AlarmSummaryService, denying/resetting an out-of-scope id rather than merely hiding it from the dropdown. Tests: bUnit render tests added to AlarmSummaryRenderTests.cs proving (1) a scoped Deployer's picker omits the out-of-scope site and (2) a forged out-of-scope onchange value is denied — no row renders and GetSiteAlarmsAsync is never called for that site. AlarmSummaryVirtualizeTests.cs updated to register the now-required SiteScopeService. --- .../Pages/Monitoring/AlarmSummary.razor | 46 +++++++++++++++--- .../Monitoring/AlarmSummaryRenderTests.cs | 48 +++++++++++++++++++ .../Monitoring/AlarmSummaryVirtualizeTests.cs | 1 + 3 files changed, 88 insertions(+), 7 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/AlarmSummary.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/AlarmSummary.razor index 284486e2..b3bf3a26 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/AlarmSummary.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/AlarmSummary.razor @@ -11,6 +11,7 @@ @inject IAlarmSummaryService AlarmSummaryService @inject ISiteRepository SiteRepository @inject ISiteAlarmLiveCache LiveAlarmCache +@inject ZB.MOM.WW.ScadaBridge.CentralUI.Auth.SiteScopeService SiteScope
@@ -275,7 +276,9 @@ { try { - _sites = await SiteRepository.GetAllSitesAsync(); + // Site scoping (CentralUI-002, arch-review P6-2): a scoped Deployment + // user may only watch live alarms for the sites they are permitted on. + _sites = await SiteScope.FilterSitesAsync(await SiteRepository.GetAllSitesAsync()); } catch { @@ -283,17 +286,36 @@ } } + // _sites is already filtered, so membership IS the scope check — mirrors + // EventLogs.razor's SelectedSiteIsPermitted. + private bool IsSiteSelectionPermitted(int siteId) => _sites.Any(s => s.Id == siteId); + + private void ClearSiteSelection() + { + _selectedSiteId = null; + _rows = Array.Empty(); + _notReporting = Array.Empty(); + _rollup = new AlarmRollup(0, 0, 0, new Dictionary()); + StopTimer(); + DisposeLiveSubscription(); + } + private async Task OnSiteChangedAsync(ChangeEventArgs e) { var raw = e.Value?.ToString(); if (string.IsNullOrEmpty(raw) || !int.TryParse(raw, out var siteId)) { - _selectedSiteId = null; - _rows = Array.Empty(); - _notReporting = Array.Empty(); - _rollup = new AlarmRollup(0, 0, 0, new Dictionary()); - StopTimer(); - DisposeLiveSubscription(); + ClearSiteSelection(); + return; + } + + // Site scoping (CentralUI-002, arch-review P6-2): the dropdown is already + // filtered, but the selection must not be trusted on its own — re-check + // server-side so a forged onchange value naming an out-of-scope site is + // denied rather than merely hidden from the picker. + if (!IsSiteSelectionPermitted(siteId)) + { + ClearSiteSelection(); return; } @@ -319,6 +341,16 @@ return; } + // Site scoping (CentralUI-002, arch-review P6-2): re-check before every + // fetch, not just at selection time — defense in depth against + // _selectedSiteId ever being set by a path that skips OnSiteChangedAsync's + // check, so an out-of-scope site can never reach AlarmSummaryService. + if (!IsSiteSelectionPermitted(siteId)) + { + ClearSiteSelection(); + return; + } + _loading = true; try { diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Monitoring/AlarmSummaryRenderTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Monitoring/AlarmSummaryRenderTests.cs index 802ef522..8c513ba2 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Monitoring/AlarmSummaryRenderTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Monitoring/AlarmSummaryRenderTests.cs @@ -67,6 +67,7 @@ public class AlarmSummaryRenderTests : BunitContext Services.AddSingleton(_siteRepo); Services.AddSingleton(_liveCache); + Services.AddScoped(); var claims = new[] { @@ -278,6 +279,53 @@ public class AlarmSummaryRenderTests : BunitContext }); } + // ── arch-review P6-2: site scoping (a scoped Deployer must not watch another + // site's live alarms — the page was missed by the ad0100f2 scoping survey) ── + + [Fact] + public void SiteScoping_ScopedDeploymentUser_SeesOnlyPermittedSiteInPicker() + { + var scopedUser = new ClaimsPrincipal(new ClaimsIdentity(new[] + { + new Claim(JwtTokenService.UsernameClaimType, "scoped"), + new Claim(JwtTokenService.RoleClaimType, "Deployer"), + new Claim(JwtTokenService.SiteIdClaimType, "1"), // Site 1 only + }, "TestAuth")); + Services.AddSingleton(new TestAuthStateProvider(scopedUser)); + + var cut = Render(); + + var options = cut.FindAll("[data-test='alarm-summary-site'] option"); + // "Select site…" placeholder + Site 1 only — Site 2 must not appear. + Assert.Equal(2, options.Count); + Assert.DoesNotContain("Site 2", cut.Markup); + } + + [Fact] + public void SiteScoping_ForgedOutOfScopeSiteId_IsDeniedNotFetched() + { + // The picker never renders Site 2 for this user, but the change handler + // must independently reject an out-of-scope id arriving via any onchange + // value (e.g. a forged one, since Blazor Server processes the SignalR + // event server-side) rather than trusting it because it came through the + //