diff --git a/docs/plans/2026-08-11-dashboard-ui-sweeps.md b/docs/plans/2026-08-11-dashboard-ui-sweeps.md index f8bbcd3..31fedbe 100644 --- a/docs/plans/2026-08-11-dashboard-ui-sweeps.md +++ b/docs/plans/2026-08-11-dashboard-ui-sweeps.md @@ -350,3 +350,40 @@ margins). Bumped for family-pin alignment. **Verification.** Build 0 warnings / 0 errors; suite **879/879**; `staticwebassets.build.json` resolves `zb.mom.ww.theme/0.4.1`. No stale-HTTP-cache clear was needed — restore picked 0.4.1 directly. + +## 8. Follow-up: role-gate the side rail's Secrets link (family-wide nav task) + +Requested as a family-wide sweep: every app's UI should link to the Secrets management page, visible +to Administrator-role users only. + +**Found state.** The link already existed — `MainLayout.razor`, Admin section, `/admin/secrets`. What +did not exist was any gate: the rail rendered every item for every visitor, including a Viewer and +the anonymous-localhost read-only identity. The premise that there was an "existing role-gated nav +pattern" to follow was false; the rail's only `AuthorizeView` was the footer's signed-in/signed-out +split, so this introduces the pattern rather than extending it. + +Not an access hole — the mounted page carries `[Authorize(Policy = "secrets:manage")]`, so a Viewer +clicking through was denied. It was a dead link presented as a live one. + +**Gate chosen: the policy, not the role.** ``, +i.e. the same policy the page itself enforces, so nav visibility cannot drift from page access. The +sweep asked for a role literal (`DashboardRoles.Admin` = `"Administrator"`), and in this host the two +are equivalent: `GatewayOptionsValidator` constrains `Dashboard:GroupToRole` values to +`Administrator` or `Viewer`, so the shared library's other manage-granting roles (`secrets-manager`, +`secrets-reveal`) are unreachable here. The policy form was preferred because it stays correct if +that constraint ever relaxes — a role literal would then hide the link from users who can use the +page. + +**Deliberate asymmetry — API Keys stays ungated.** Its sibling item looks like the same case and is +not. `ApiKeysPage` renders for a Viewer with write affordances hidden (`@if (CanManageApiKeys)`), so +hiding its nav item would remove legitimate read access. The secrets page has no read-only mode. The +rule is "gate the link when the page denies the role outright", not "gate everything under Admin". + +**Coverage.** Three tests pin the policy's verdict per principal (Administrator admitted, Viewer +refused, unauthenticated refused) in `SecretsNavGateTests`, and `/admin/secrets` joins the canonical +route list in `GatewayApplicationTests` — it is the one nav destination mounted from an RCL rather +than declared here, so a routing regression could remove it without touching this repo's pages. +Not a rendering test: the suite has no component-testing harness, and adding one to assert a single +`AuthorizeView` would be a large dependency for a small claim. + +**Verification.** Build 0 warnings / 0 errors; suite **895/895** (892 + 3). diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Layout/MainLayout.razor b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Layout/MainLayout.razor index a6693d7..2a18a07 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Layout/MainLayout.razor +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Layout/MainLayout.razor @@ -1,4 +1,5 @@ @inherits LayoutComponentBase +@using ZB.MOM.WW.Secrets.Ui @* Thin layout: delegates the side-rail chassis (hamburger, brand, responsive collapse) to the shared ZB.MOM.WW.Theme . The nav is reproduced @@ -19,7 +20,20 @@ - + @* Gated on the SAME policy the mounted /admin/secrets page enforces, not on a role + literal, so nav visibility cannot drift from page access. In this host the two are + equivalent — GatewayOptionsValidator constrains Dashboard:GroupToRole values to + Administrator or Viewer, so the shared library's other manage-granting roles + (secrets-manager, secrets-reveal) are unreachable here — but the policy form stays + correct if that ever relaxes. Deliberately NOT applied to the API Keys item above: + that page renders read-only for Viewers, so hiding its link would remove legitimate + read access, whereas the secrets page denies a Viewer outright and its link would be + a dead end. *@ + + + + + diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SecretsNavGateTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SecretsNavGateTests.cs new file mode 100644 index 0000000..4bb3cc1 --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SecretsNavGateTests.cs @@ -0,0 +1,104 @@ +using System.Security.Claims; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.Auth.AspNetCore; +using ZB.MOM.WW.MxGateway.Server; +using ZB.MOM.WW.MxGateway.Server.Dashboard; +using ZB.MOM.WW.Secrets.Ui; + +namespace ZB.MOM.WW.MxGateway.Tests.Dashboard; + +/// +/// Covers the authorization decision behind the side rail's Secrets link. +/// +/// +/// +/// The rail gates that item with <AuthorizeView Policy="secrets:manage"> — the same +/// policy the mounted /admin/secrets page enforces — rather than a role literal, so nav +/// visibility cannot drift from page access. These tests pin the policy's verdict per principal, +/// which is the behaviour the gate delegates to. +/// +/// +/// This is deliberately not a rendering test: the suite has no component-testing harness, and +/// adding one to assert a single AuthorizeView would be a large dependency for a small +/// claim. What is asserted here is the part that can actually be wrong — which principals the +/// policy admits. The link's presence in MainLayout.razor and the route's existence are +/// covered separately (see GatewayApplicationTests, which asserts /admin/secrets is +/// mapped), so an unmapped route cannot masquerade as a working link. +/// +/// +public sealed class SecretsNavGateTests +{ + /// An Administrator sees the Secrets link, because the policy admits that role. + /// A task that represents the asynchronous operation. + [Fact] + public async Task ManagePolicy_AdmitsAdministrator() + { + await using WebApplication app = GatewayApplication.Build([]); + IAuthorizationService authorization = app.Services.GetRequiredService(); + + AuthorizationResult result = await authorization.AuthorizeAsync( + PrincipalWithRoles(DashboardRoles.Admin), + resource: null, + SecretsAuthorization.ManagePolicy); + + Assert.True(result.Succeeded); + } + + /// + /// A Viewer does not. This is the case the gate exists for: the secrets page denies a Viewer + /// outright, so an ungated link would be a dead end rather than a degraded-but-useful view — + /// which is why the sibling API Keys item is deliberately left ungated (that page does render + /// read-only for Viewers). + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ManagePolicy_RefusesViewer() + { + await using WebApplication app = GatewayApplication.Build([]); + IAuthorizationService authorization = app.Services.GetRequiredService(); + + AuthorizationResult result = await authorization.AuthorizeAsync( + PrincipalWithRoles(DashboardRoles.Viewer), + resource: null, + SecretsAuthorization.ManagePolicy); + + Assert.False(result.Succeeded); + } + + /// + /// An unauthenticated principal does not. Covers the anonymous-localhost path, which grants a + /// read-only identity without authenticating it. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ManagePolicy_RefusesUnauthenticated() + { + await using WebApplication app = GatewayApplication.Build([]); + IAuthorizationService authorization = app.Services.GetRequiredService(); + + AuthorizationResult result = await authorization.AuthorizeAsync( + new ClaimsPrincipal(new ClaimsIdentity()), + resource: null, + SecretsAuthorization.ManagePolicy); + + Assert.False(result.Succeeded); + } + + // Mirrors what DashboardAuthenticator issues: roles as ZbClaimTypes.Role (== ClaimTypes.Role), + // with the identity told to treat that claim as its role type. Constructing the identity with + // an authentication type is what makes it authenticated — without one, every policy that + // requires an authenticated user fails for the wrong reason and the role assertions above + // would pass vacuously. + private static ClaimsPrincipal PrincipalWithRoles(params string[] roles) + { + var identity = new ClaimsIdentity( + roles.Select(role => new Claim(ZbClaimTypes.Role, role)), + authenticationType: "Test", + nameType: ZbClaimTypes.Name, + roleType: ZbClaimTypes.Role); + + return new ClaimsPrincipal(identity); + } +} diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayApplicationTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayApplicationTests.cs index 2293e15..a20ffbc 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayApplicationTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayApplicationTests.cs @@ -205,6 +205,12 @@ public sealed class GatewayApplicationTests "/galaxy", "/apikeys", "/sessions/{SessionId}", + + // Mounted from the ZB.MOM.WW.Secrets.Ui RCL rather than declared here, so it is the + // one nav destination that a routing regression could remove without touching this + // repo's own pages. The side rail links to it (role-gated), which makes an unmapped + // route a visible dead link rather than a silent absence. + "/admin/secrets", ]; foreach (string canonical in canonicalRoutes) {