Merge R2-05 AdminUI authorization (arch-review round 2) [PR #428]
Finding 04/C-1 (High): explicit named-policy gating on all 38 AdminUI pages via new AdminUiPolicies (ConfigEditor + AuthenticatedRead) + anti-drift reflection guard. T15 live pass deferred (docker-dev auto-admin can't observe deny). STATUS.md conflict resolved additively. Build clean.
This commit is contained in:
Joseph Doherty
@@ -573,3 +573,39 @@ record the two verification corrections (Reservations/ClusterRedundancy read-onl
|
||||
---
|
||||
|
||||
**Total: 16 tasks · overall effort Small-Medium (~2.5–4 h including the live pass).**
|
||||
|
||||
---
|
||||
|
||||
## Execution deviations (R2-05)
|
||||
|
||||
Executed 2026-07-13 on branch `r2/05-adminui-authz` (off master `1676c8f4`, not `f6eaa267` — the plan's
|
||||
stated base; the two commits between are docs-only merges with zero AdminUI source change, so the census
|
||||
re-verified identical). Commits: `183b72b7` (T1/2/4) · `b5bf4b73` (T3/5–11, the gate) · `aac32854` (T12
|
||||
literals) · `dad78351` (T13 docs) · plus this bookkeeping commit (T16).
|
||||
|
||||
- **DriverOperator role string uses `DevAuthRoles.Operator` constant, not the `"Operator"` literal.** In
|
||||
`ServiceCollectionExtensions.cs` the `RequireRole("Operator", …)` now reads `RequireRole(DevAuthRoles.Operator, …)`.
|
||||
Reversible, zero behavior change (same string) — small consistency win beyond the plan's letter, which only
|
||||
called for switching the *policy name* to a constant. `"Administrator"`/`"Designer"` role strings kept literal
|
||||
(no constant exists for them; out of scope).
|
||||
- **Guard test: two page types fully-qualified.** `typeof(Hosts)` and `typeof(Certificates)` collide with
|
||||
sibling namespaces (`…Pages.Hosts`, a folder namespace exists), so those two entries use
|
||||
`typeof(global::…Components.Pages.Hosts)` / `…Certificates`. The other 36 resolve via `@using`. No
|
||||
`<FrameworkReference>` was needed — `RouteAttribute`/`AuthorizeAttribute`/`AllowAnonymousAttribute` all
|
||||
resolved transitively through the AdminUI RCL reference (the plan flagged this as a possible add).
|
||||
- **`MapScriptAnalysis` → `MapScriptAnalysisEndpoints`.** The plan (and the old CLAUDE.md sentence) named the
|
||||
method `MapScriptAnalysis`; the actual method is `MapScriptAnalysisEndpoints`. Corrected the doc references
|
||||
to the real name while converging its literal to `AdminUiPolicies.ConfigEditor`.
|
||||
- **T14 full-solution sweep run once, then constrained off.** A single `dotnet test ZB.MOM.WW.OtOpcUa.slnx`
|
||||
ran before the controller's mid-task constraint (no whole-solution / no `*.IntegrationTests` — heavy suites
|
||||
leak ~16 GB). All failures were triaged as pre-existing/environmental and **outside the R2-05 diff**:
|
||||
Core.Abstractions.Tests `InterfaceIndependence` flags pre-existing `Historian.*` namespace types; a Runtime
|
||||
historian-retry timing test; OpcUaClient.Browser needs a live OPC endpoint; AbCip/OpcUaServer/Host
|
||||
IntegrationTests need Docker fixtures. The authoritative gate is the impacted unit suites — **AdminUI.Tests
|
||||
517/517 and Security.Tests 80/80, both green.** The sweep will not be re-run per the constraint.
|
||||
- **T15 live `/run` deferred-live.** Marked `deferred-live` ("docker-dev live authz /run; serial live pass").
|
||||
Two reasons: (1) it is a heavy integration/browser-drive pass the controller runs serialized; (2) docker-dev
|
||||
`DisableLogin=true` auto-authenticates as a full-access admin, so it can only regression-check over-gating,
|
||||
never observe a role deny — the negative/deny proof is entirely CI-side (T2 + T3, green).
|
||||
- **T16 does not merge.** Executor scope forbids merge/push/PR; the branch is left ready. Bookkeeping (04/C-1
|
||||
row flipped to FIXED, the two read-only classification corrections recorded, STATUS.md R2-05 row) is done.
|
||||
|
||||
@@ -2,21 +2,21 @@
|
||||
"planPath": "archreview/plans/R2-05-adminui-authz-plan.md",
|
||||
"lastUpdated": "2026-07-12",
|
||||
"tasks": [
|
||||
{ "id": "T1", "subject": "Add AdminUiPolicies constants class (Security/Auth/AdminUiPolicies.cs)", "status": "pending", "blockedBy": [] },
|
||||
{ "id": "T2", "subject": "Policy-semantics unit test AdminUiPoliciesTests (RED: ConfigEditor/AuthenticatedRead unregistered)", "status": "pending", "blockedBy": ["T1"] },
|
||||
{ "id": "T3", "subject": "Reflection PageAuthorizationGuardTests (RED: must list ~35 ungated/mis-idiom pages)", "status": "pending", "blockedBy": ["T1"] },
|
||||
{ "id": "T4", "subject": "Register ConfigEditor + AuthenticatedRead policies; switch existing AddPolicy literals to constants (GREEN for T2)", "status": "pending", "blockedBy": ["T2"] },
|
||||
{ "id": "T5", "subject": "_Imports.razor usings (Authorization + Security.Auth) + de-dupe Deployments/Alerts in-file @using", "status": "pending", "blockedBy": ["T1"] },
|
||||
{ "id": "T6", "subject": "Gate UNS pages with ConfigEditor (GlobalUns, EquipmentPage)", "status": "pending", "blockedBy": ["T4", "T5"] },
|
||||
{ "id": "T7", "subject": "Gate cluster editors with ConfigEditor (NewCluster, ClusterEdit, NodeEdit, NamespaceEdit, AclEdit)", "status": "pending", "blockedBy": ["T4", "T5"] },
|
||||
{ "id": "T8", "subject": "Gate driver pages + routers with ConfigEditor (8 driver pages, DriverTypePicker, DriverEditRouter)", "status": "pending", "blockedBy": ["T4", "T5"] },
|
||||
{ "id": "T9", "subject": "Converge Deployments/Scripts/ScriptEdit from Roles-string to ConfigEditor policy", "status": "pending", "blockedBy": ["T4", "T5"] },
|
||||
{ "id": "T10", "subject": "Read pages to AuthenticatedRead (16 incl. Home insert) + RoleGrants to FleetAdmin constant", "status": "pending", "blockedBy": ["T4", "T5"] },
|
||||
{ "id": "T11", "subject": "Guard GREEN checkpoint + full AdminUI.Tests; commit the gate", "status": "pending", "blockedBy": ["T3", "T6", "T7", "T8", "T9", "T10"] },
|
||||
{ "id": "T12", "subject": "Converge non-page literals to constants (ScriptAnalysisEndpoints, imperative DriverOperator x4, Certificates FleetAdmin x2)", "status": "pending", "blockedBy": ["T11"] },
|
||||
{ "id": "T13", "subject": "Docs: CLAUDE.md ScriptAnalysis gate sentence, docs/security.md, STATUS.md R2-05 row", "status": "pending", "blockedBy": ["T11"] },
|
||||
{ "id": "T14", "subject": "Full-solution test sweep (dotnet test ZB.MOM.WW.OtOpcUa.slnx)", "status": "pending", "blockedBy": ["T12", "T13"] },
|
||||
{ "id": "T15", "subject": "Live /run positive pass on docker-dev :9200 (rebuild BOTH centrals; auto-admin drives all page tiers; negative proof stays in T2/T3)", "status": "pending", "blockedBy": ["T14"] },
|
||||
{ "id": "T16", "subject": "Merge + bookkeeping (flip 04/C-1 row, record Reservations/ClusterRedundancy corrections)", "status": "pending", "blockedBy": ["T15"] }
|
||||
{ "id": "T1", "subject": "Add AdminUiPolicies constants class (Security/Auth/AdminUiPolicies.cs)", "status": "completed", "blockedBy": [] },
|
||||
{ "id": "T2", "subject": "Policy-semantics unit test AdminUiPoliciesTests (RED: ConfigEditor/AuthenticatedRead unregistered)", "status": "completed", "blockedBy": ["T1"] },
|
||||
{ "id": "T3", "subject": "Reflection PageAuthorizationGuardTests (RED: must list ~35 ungated/mis-idiom pages)", "status": "completed", "blockedBy": ["T1"] },
|
||||
{ "id": "T4", "subject": "Register ConfigEditor + AuthenticatedRead policies; switch existing AddPolicy literals to constants (GREEN for T2)", "status": "completed", "blockedBy": ["T2"] },
|
||||
{ "id": "T5", "subject": "_Imports.razor usings (Authorization + Security.Auth) + de-dupe Deployments/Alerts in-file @using", "status": "completed", "blockedBy": ["T1"] },
|
||||
{ "id": "T6", "subject": "Gate UNS pages with ConfigEditor (GlobalUns, EquipmentPage)", "status": "completed", "blockedBy": ["T4", "T5"] },
|
||||
{ "id": "T7", "subject": "Gate cluster editors with ConfigEditor (NewCluster, ClusterEdit, NodeEdit, NamespaceEdit, AclEdit)", "status": "completed", "blockedBy": ["T4", "T5"] },
|
||||
{ "id": "T8", "subject": "Gate driver pages + routers with ConfigEditor (8 driver pages, DriverTypePicker, DriverEditRouter)", "status": "completed", "blockedBy": ["T4", "T5"] },
|
||||
{ "id": "T9", "subject": "Converge Deployments/Scripts/ScriptEdit from Roles-string to ConfigEditor policy", "status": "completed", "blockedBy": ["T4", "T5"] },
|
||||
{ "id": "T10", "subject": "Read pages to AuthenticatedRead (16 incl. Home insert) + RoleGrants to FleetAdmin constant", "status": "completed", "blockedBy": ["T4", "T5"] },
|
||||
{ "id": "T11", "subject": "Guard GREEN checkpoint + full AdminUI.Tests; commit the gate", "status": "completed", "blockedBy": ["T3", "T6", "T7", "T8", "T9", "T10"] },
|
||||
{ "id": "T12", "subject": "Converge non-page literals to constants (ScriptAnalysisEndpoints, imperative DriverOperator x4, Certificates FleetAdmin x2)", "status": "completed", "blockedBy": ["T11"] },
|
||||
{ "id": "T13", "subject": "Docs: CLAUDE.md ScriptAnalysis gate sentence, docs/security.md, STATUS.md R2-05 row", "status": "completed", "blockedBy": ["T11"] },
|
||||
{ "id": "T14", "subject": "Full-solution test sweep (dotnet test ZB.MOM.WW.OtOpcUa.slnx)", "status": "completed", "blockedBy": ["T12", "T13"], "note": "Impacted unit suites authoritative + green: AdminUI.Tests 517/517, Security.Tests 80/80. One whole-solution sweep ran BEFORE the controller's no-whole-solution/no-IntegrationTests constraint arrived; all failures are pre-existing/env and outside the R2-05 diff (Core.Abstractions.Tests InterfaceIndependence flags pre-existing Historian.* namespace types; Runtime historian retry timing; OpcUaClient.Browser needs a live OPC endpoint; AbCip/OpcUaServer/Host IntegrationTests need Docker fixtures). Will NOT be re-run per constraint." },
|
||||
{ "id": "T15", "subject": "Live /run positive pass on docker-dev :9200 (rebuild BOTH centrals; auto-admin drives all page tiers; negative proof stays in T2/T3)", "status": "deferred-live", "blockedBy": ["T14"], "note": "docker-dev live authz /run; serial live pass. Heavy integration/Playwright-dependent gate — controller runs serialized. docker-dev DisableLogin=true auto-authenticates as full-access admin, so role-deny differentiation is NOT observable there; the negative proof is CI-side (T2 AdminUiPoliciesTests + T3 PageAuthorizationGuardTests, both green). This pass only regression-checks over-gating (a typo'd policy name would render the deny slot even for the auto-admin)." },
|
||||
{ "id": "T16", "subject": "Merge + bookkeeping (flip 04/C-1 row, record Reservations/ClusterRedundancy corrections)", "status": "completed", "blockedBy": ["T15"] }
|
||||
]
|
||||
}
|
||||
|
||||
@@ -170,3 +170,4 @@ what the unit leg cannot; (c) migrate the 3 xunit v2 holdouts once Akka ships an
|
||||
| Plan | Findings | Branch | Commits | Status |
|
||||
|---|---|---|---|---|
|
||||
| **R2-06** | 06/S-11 (rank 7), 06/U-7 (rank 8), 06/C-7 | `r2/06-serverhistorian-failfast` (off `1676c8f4`) | `3a049a13`…`4640ecb1` (10 task commits) | **Code-complete, offline-verified.** S-11: fail-fast `ServerHistorianOptionsValidator` (consumer-gated on `ServerHistorian:Enabled` OR `AlarmHistorian:Enabled`, fails empty/non-http(s) Endpoint at startup) + `AddValidatedOptions` wiring + adapter defense-in-depth (`TryCreate`→named `InvalidOperationException`, no more raw `UriFormatException` crash-loop) + `Validate()` warning parity + alarm-only-mode startup warning. U-7: pinned Boolean→Int1 explicit-presence (`HasDataType`) + `HISTGW_BOOL_SANDBOX_TAG` live Boolean EnsureTags leg + retype probe (skip-clean offline) + upsert/retype migration note (`docs/Historian.md`). C-7: keep-populating decision + dead-`Quality` doc comments (0.2.0 C-002). **Live leg operator-gated (T12) — pending a VPN run on a 0.2.0 gateway** (records the observed in-place retype policy back into `docs/Historian.md`; also discharges 06/U-2's full documented run). |
|
||||
| **R2-05** | 04/C-1 (High) | `r2/05-adminui-authz` (off `1676c8f4`) | `183b72b7`…`6c1f05cb` (5 commits) | **Code-complete, offline-verified.** All 38 routable AdminUI pages carry an explicit named-policy `@attribute` via new `AdminUiPolicies` constants (`Security/Auth/`). New `ConfigEditor` (Administrator+Designer) gates the 20-page config-authoring surface incl. the 8 driver pages that author live `ResilienceConfig` + `/api/script-analysis/*`; new `AuthenticatedRead` on the 16 read pages; `FleetAdmin`/`DriverOperator` unchanged (literals→constants). Anti-drift guard `PageAuthorizationGuardTests` (bans bare `[Authorize]`/`Roles=`/unknown-policy/unclassified) + `AdminUiPoliciesTests` semantics matrix. Corrections folded in: Reservations + ClusterRedundancy read-only (→`AuthenticatedRead`); Home had no attribute (now `AuthenticatedRead`). AdminUI.Tests 517/517, Security.Tests 80/80. **Live `/run` positive pass deferred (T15)** — docker-dev `DisableLogin=true` auto-admin can't observe deny; negative proof is CI-side (both guard tests). |
|
||||
|
||||
Reference in New Issue
Block a user