diff --git a/code-reviews/Admin/findings.md b/code-reviews/Admin/findings.md index 42c153a..76f76be 100644 --- a/code-reviews/Admin/findings.md +++ b/code-reviews/Admin/findings.md @@ -123,13 +123,13 @@ | Severity | Medium | | Category | Design-document adherence | | Location | `Components/Pages/Clusters/NewCluster.razor:91,95-96` | -| Status | Open | +| Status | Resolved | **Description:** `NewCluster.CreateAsync` hardcodes `CreatedBy = "admin-ui"` (both on the `ServerCluster` row and the draft generation) instead of the signed-in operator principal name. `admin-ui.md` section "Audit" requires "the operator principal" be recorded on every write. The audit trail therefore cannot attribute cluster creation to a person. The same literal would apply to any anonymous creation that Admin-001/002 currently permit. **Recommendation:** Pass the authenticated user identity (`ClaimTypes.Name` / `NameIdentifier` from the cascaded `AuthenticationState`) as `createdBy`. Apply the same pattern to every other Admin write path that records a `CreatedBy`/`PublishedBy`/`ReleasedBy` field. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — `NewCluster.razor` and `ClusterDetail.razor` (the two pages that call `ClusterService.CreateAsync` / `GenerationService.CreateDraftAsync` with a hardcoded literal) now resolve `ClaimTypes.Name` / `ClaimTypes.NameIdentifier` from the cascaded `AuthenticationState` and pass the operator principal name as `createdBy`; the fallback is `"unknown"` (defensive, should never occur on an `[Authorize]`-gated page). ### Admin-008 diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/ClusterDetail.razor b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/ClusterDetail.razor index 252a6f3..4be8f20 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/ClusterDetail.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/ClusterDetail.razor @@ -1,5 +1,6 @@ @page "/clusters/{ClusterId}" @attribute [Microsoft.AspNetCore.Authorization.Authorize] +@using System.Security.Claims @using Microsoft.AspNetCore.Components.Web @using Microsoft.AspNetCore.SignalR.Client @using ZB.MOM.WW.OtOpcUa.Admin.Hubs @@ -200,7 +201,12 @@ else _busy = true; try { - var draft = await GenerationSvc.CreateDraftAsync(ClusterId, createdBy: "admin-ui", CancellationToken.None); + // Admin-007: record the authenticated operator's name, not a static literal. + var user = AuthState is not null ? (await AuthState).User : null; + var operatorName = user?.FindFirstValue(ClaimTypes.Name) + ?? user?.FindFirstValue(ClaimTypes.NameIdentifier) + ?? "unknown"; + var draft = await GenerationSvc.CreateDraftAsync(ClusterId, createdBy: operatorName, CancellationToken.None); Nav.NavigateTo($"/clusters/{ClusterId}/draft/{draft.GenerationId}"); } finally { _busy = false; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/NewCluster.razor b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/NewCluster.razor index 4a585b1..37aa5ea 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/NewCluster.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/NewCluster.razor @@ -4,6 +4,8 @@ and its CreateAsync write path exploitable by any caller. *@ @attribute [Microsoft.AspNetCore.Authorization.Authorize(Policy = "CanPublish")] @using System.ComponentModel.DataAnnotations +@using System.Security.Claims +@using Microsoft.AspNetCore.Components.Authorization @using Microsoft.AspNetCore.Components.Web @using ZB.MOM.WW.OtOpcUa.Admin.Services @using ZB.MOM.WW.OtOpcUa.Configuration.Entities @@ -73,6 +75,10 @@ public RedundancyMode RedundancyMode { get; set; } = RedundancyMode.None; } + // Admin-007: record the authenticated operator's identity on every write path, not + // the static literal "admin-ui" which produced an unattributable audit trail. + [CascadingParameter] private Task? AuthState { get; set; } + private Input _input = new(); private bool _submitting; private string? _error; @@ -84,6 +90,14 @@ try { + // Resolve the signed-in principal name. The page is [Authorize(Policy="CanPublish")] + // so AuthState will always be available with an authenticated user here; fall back to + // "unknown" only as a defensive last resort (should never happen in practice). + var user = AuthState is not null ? (await AuthState).User : null; + var operatorName = user?.FindFirstValue(ClaimTypes.Name) + ?? user?.FindFirstValue(ClaimTypes.NameIdentifier) + ?? "unknown"; + var cluster = new ServerCluster { ClusterId = _input.ClusterId, @@ -93,11 +107,11 @@ RedundancyMode = _input.RedundancyMode, NodeCount = _input.RedundancyMode == RedundancyMode.None ? (byte)1 : (byte)2, Enabled = true, - CreatedBy = "admin-ui", + CreatedBy = operatorName, }; - await ClusterSvc.CreateAsync(cluster, createdBy: "admin-ui", CancellationToken.None); - await GenerationSvc.CreateDraftAsync(cluster.ClusterId, createdBy: "admin-ui", CancellationToken.None); + await ClusterSvc.CreateAsync(cluster, createdBy: operatorName, CancellationToken.None); + await GenerationSvc.CreateDraftAsync(cluster.ClusterId, createdBy: operatorName, CancellationToken.None); Nav.NavigateTo($"/clusters/{cluster.ClusterId}"); }