feat(management): any-of role gates; restrict ListSecuredWrites to Operator/Verifier/Admin (arch-review UA1)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -206,8 +206,9 @@ Per-leaf alarm rendering (leaf nodes are individual conditions for native alarms
|
||||
- **Stuck rows are visually badged** — a notification is stuck if it is `Pending` or `Retrying` and older than the configurable stuck-age threshold. Stuck detection is display-only; there is no automated escalation or alerting.
|
||||
- All queries are served from the central `Notifications` table — no remote per-site queries are needed, unlike the Parked Message Management page.
|
||||
|
||||
### Secured Writes (Operator / Verifier Roles)
|
||||
### Secured Writes (Operator / Verifier / Administrator Roles)
|
||||
- A **Secured Writes** page (`/operations/secured-writes`) drives the **two-person** authorization workflow for writes through the MxAccess Gateway: an **Operator** initiates the write, a separate **Verifier** approves it, and only an approved write reaches the site.
|
||||
- The **page itself is gated by `RequireSecuredWriteAccess`** — satisfied by any of `Operator` / `Verifier` / `Administrator`. Although the pending/history lists are read-only, they expose process-sensitive tag values, so the page is not open to every authenticated user; this aligns with the ManagementActor `ListSecuredWritesCommand` any-of gate (arch-review UA1). The submit and approve/reject sub-actions are further gated by `RequireOperator` / `RequireVerifier` respectively (below).
|
||||
- **Operator (submit)** — a submit form gated by `RequireOperator`: pick the site → an **MxGateway** connection on that site → the tag path → a typed value → an optional comment. Submission inserts a `Pending` `PendingSecuredWrite` row centrally; it does **not** write anything yet.
|
||||
- **Verifier (approve / reject)** — a pending queue gated by `RequireVerifier` with **Approve** / **Reject** (+comment) actions. Approve shows a confirmation of the exact site / connection / tag / value before firing. The verifier's **own submissions are disabled in the UI and rejected server-side** (no self-approval). On approve, central marks the row `Approved` and relays the write to the site MxGateway (records `Executed` / `Failed`); reject moves it to `Rejected` with a reason.
|
||||
- **History** — terminal rows (Executed / Failed / Rejected / Expired) with the full who/when/outcome trail (operator, verifier, comments, timestamps, any execution error).
|
||||
|
||||
@@ -147,7 +147,7 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B
|
||||
- **SubmitSecuredWriteCommand** (`Operator` role): validates the target connection exists and uses the **MxGateway** protocol (rejected otherwise), then inserts a `Pending` row capturing site / connection / tag / typed value / operator + comment. Nothing is written yet.
|
||||
- **ApproveSecuredWriteCommand** (`Verifier` role): enforces **no self-approval** (`VerifierUser ≠ OperatorUser`, checked **before** any state change) and a **compare-and-swap** race guard (`TryMarkApprovedAsync` — exactly one concurrent approver flips `Pending → Approved`; the loser is rejected). On winning, it validates the value type, decodes the value (guarded — a bad value fails the row deterministically rather than leaving it stuck `Approved`), and relays a `WriteTagRequest` to the site MxGateway via the Communication Layer, recording the outcome as `Executed` or `Failed`.
|
||||
- **RejectSecuredWriteCommand** (`Verifier` role): marks a `Pending` row `Rejected` with the verifier's reason.
|
||||
- **List / query secured writes**: pending queue + terminal history, global or per-site.
|
||||
- **ListSecuredWritesCommand** (any of `Operator` / `Verifier` / `Administrator`): pending queue + terminal history, global or per-site. Although read-only, the history table exposes process-sensitive tag values, so it is **not** open to any authenticated user — it is gated to the two Secured Write roles plus `Administrator` (any-of), aligning with the `RequireSecuredWriteAccess` Central UI policy (arch-review UA1).
|
||||
|
||||
### External Systems
|
||||
|
||||
@@ -215,6 +215,7 @@ Every incoming message carries the authenticated user's identity and roles. The
|
||||
- **Deployment** role required for: instance management (including instance alarm overrides and native alarm source overrides), deployments, debug view, debug snapshot, parked message queries, site event log queries. Site scoping is enforced for site-scoped Deployment users.
|
||||
- **Operator** role required for: submitting a secured write (`SubmitSecuredWriteCommand`).
|
||||
- **Verifier** role required for: approving / rejecting a secured write (`ApproveSecuredWriteCommand` / `RejectSecuredWriteCommand`). The no-self-approval rule (`Operator ≠ Verifier`) is enforced in the handler, independent of the role check.
|
||||
- **Any of Operator / Verifier / Administrator** required for: listing / querying secured writes (`ListSecuredWritesCommand`). Read-only, but the history exposes process-sensitive tag values, so it is gated any-of rather than open to every authenticated user (arch-review UA1). Enforced as an any-of gate (`GetRequiredRoles` returns the permitted set; the caller must hold at least one).
|
||||
- **Admin** role additionally required for: server-certificate trust/remove/list (`TrustServerCertCommand` / `RemoveServerCertCommand` / `ListServerCertsCommand`).
|
||||
- **Read-only access** (any authenticated role): health summary, health site, site event log queries, parked message queries.
|
||||
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
@page "/operations/secured-writes"
|
||||
@attribute [Authorize]
|
||||
@* arch-review UA1: the Secured Writes history exposes process-sensitive tag
|
||||
values, so the page is gated to the Secured Write roles + Administrator
|
||||
(any-of), aligning with the ManagementActor ListSecuredWrites gate. *@
|
||||
@attribute [Authorize(Policy = AuthorizationPolicies.RequireSecuredWriteAccess)]
|
||||
@using ZB.MOM.WW.ScadaBridge.CentralUI.Services
|
||||
@using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites
|
||||
@using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories
|
||||
|
||||
@@ -100,12 +100,14 @@ public class ManagementActor : ReceiveActor
|
||||
var correlationId = envelope.CorrelationId;
|
||||
var user = envelope.User;
|
||||
|
||||
// Check authorization
|
||||
var requiredRole = GetRequiredRole(envelope.Command);
|
||||
if (requiredRole != null && !user.Roles.Contains(requiredRole, StringComparer.OrdinalIgnoreCase))
|
||||
// Check authorization. A command may require ANY of a set of roles
|
||||
// (any-of): the caller is authorized when they hold at least one.
|
||||
var requiredRoles = GetRequiredRoles(envelope.Command);
|
||||
if (requiredRoles is not null
|
||||
&& !requiredRoles.Any(r => user.Roles.Contains(r, StringComparer.OrdinalIgnoreCase)))
|
||||
{
|
||||
sender.Tell(new ManagementUnauthorized(correlationId,
|
||||
$"Role '{requiredRole}' required for {envelope.Command.GetType().Name}"));
|
||||
$"One of roles [{string.Join(", ", requiredRoles)}] required for {envelope.Command.GetType().Name}"));
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -157,7 +159,28 @@ public class ManagementActor : ReceiveActor
|
||||
return new ManagementError(correlationId, clientMessage, "COMMAND_FAILED");
|
||||
}
|
||||
|
||||
private static string? GetRequiredRole(object command) => command switch
|
||||
// Static readonly role sets for the authorization gate. Single-role arms
|
||||
// stay one-element arrays so every arm has the uniform any-of shape.
|
||||
private static readonly string[] AdminOnly = [Roles.Administrator];
|
||||
private static readonly string[] DesignerOnly = [Roles.Designer];
|
||||
private static readonly string[] DeployerOnly = [Roles.Deployer];
|
||||
private static readonly string[] OperatorOnly = [Roles.Operator];
|
||||
private static readonly string[] VerifierOnly = [Roles.Verifier];
|
||||
|
||||
// arch-review UA1: the Secured Writes history table exposes
|
||||
// process-sensitive tag values, so reading it is restricted to the
|
||||
// two-person Secured Write roles plus Administrator (any-of) — NOT any
|
||||
// authenticated user like the other list/query commands.
|
||||
private static readonly string[] SecuredWriteReaders =
|
||||
[Roles.Operator, Roles.Verifier, Roles.Administrator];
|
||||
|
||||
/// <summary>
|
||||
/// Returns the set of roles that satisfy the authorization gate for
|
||||
/// <paramref name="command"/> (any-of semantics), or <c>null</c> when the
|
||||
/// command is available to any authenticated user. Internal so the
|
||||
/// authorization matrix can be asserted directly in tests.
|
||||
/// </summary>
|
||||
internal static IReadOnlyList<string>? GetRequiredRoles(object command) => command switch
|
||||
{
|
||||
// Administrator operations
|
||||
CreateSiteCommand or UpdateSiteCommand or DeleteSiteCommand
|
||||
@@ -180,7 +203,7 @@ public class ManagementActor : ReceiveActor
|
||||
// gate must match — otherwise a Designer blocked in the UI could still
|
||||
// rotate a production credential via the CLI/Management API.
|
||||
or UpdateSmtpConfigCommand
|
||||
or UpdateSmsConfigCommand => Roles.Administrator,
|
||||
or UpdateSmsConfigCommand => AdminOnly,
|
||||
|
||||
// Designer operations
|
||||
CreateAreaCommand or DeleteAreaCommand
|
||||
@@ -208,13 +231,13 @@ public class ManagementActor : ReceiveActor
|
||||
or CreateTemplateFolderCommand or RenameTemplateFolderCommand
|
||||
or MoveTemplateFolderCommand or ReorderTemplateFolderCommand or DeleteTemplateFolderCommand
|
||||
or MoveTemplateToFolderCommand
|
||||
or ExportBundleCommand => Roles.Designer,
|
||||
or ExportBundleCommand => DesignerOnly,
|
||||
|
||||
// Transport import operations (mirror the Central UI gating:
|
||||
// Administrator for inbound bundle handling because they mutate
|
||||
// cross-cutting configuration; Export stays Designer because it only
|
||||
// reads).
|
||||
PreviewBundleCommand or ImportBundleCommand => Roles.Administrator,
|
||||
PreviewBundleCommand or ImportBundleCommand => AdminOnly,
|
||||
|
||||
// Deployer operations
|
||||
CreateInstanceCommand or MgmtDeployInstanceCommand or MgmtEnableInstanceCommand
|
||||
@@ -226,16 +249,19 @@ public class ManagementActor : ReceiveActor
|
||||
or MgmtDeployArtifactsCommand
|
||||
or QueryDeploymentsCommand
|
||||
or RetryParkedMessageCommand or DiscardParkedMessageCommand
|
||||
or DebugSnapshotCommand => Roles.Deployer,
|
||||
or DebugSnapshotCommand => DeployerOnly,
|
||||
|
||||
// Two-person secured write. Submit is an Operator action;
|
||||
// approve/reject are Verifier actions. Separation of duties (a write may
|
||||
// not be verified by its submitter) is enforced inside the handler — the
|
||||
// role gate only ensures the caller holds the right coarse role.
|
||||
SubmitSecuredWriteCommand => Roles.Operator,
|
||||
RejectSecuredWriteCommand or ApproveSecuredWriteCommand => Roles.Verifier,
|
||||
// ListSecuredWritesCommand is read-only -> falls through to "any
|
||||
// authenticated user" below, like the other list/query commands.
|
||||
SubmitSecuredWriteCommand => OperatorOnly,
|
||||
RejectSecuredWriteCommand or ApproveSecuredWriteCommand => VerifierOnly,
|
||||
// ListSecuredWritesCommand is read-only, but the history table exposes
|
||||
// process-sensitive tag values, so it is NOT open to any authenticated
|
||||
// user — restricted to the Secured Write roles + Administrator
|
||||
// (arch-review UA1).
|
||||
ListSecuredWritesCommand => SecuredWriteReaders,
|
||||
|
||||
// Read-only queries -- any authenticated user
|
||||
_ => null
|
||||
|
||||
@@ -88,6 +88,24 @@ public static class AuthorizationPolicies
|
||||
/// </summary>
|
||||
public const string RequireVerifier = "RequireVerifier";
|
||||
|
||||
/// <summary>
|
||||
/// Read access to the Secured Writes surface (the Secured Writes page +
|
||||
/// its Pending/history list). The history table exposes process-sensitive
|
||||
/// tag values, so it is NOT open to any authenticated user — satisfied by
|
||||
/// any of {<see cref="Roles.Operator"/>, <see cref="Roles.Verifier"/>,
|
||||
/// <see cref="Roles.Administrator"/>} (arch-review UA1). Aligns with the
|
||||
/// ManagementActor's <c>ListSecuredWritesCommand</c> any-of gate.
|
||||
/// </summary>
|
||||
public const string RequireSecuredWriteAccess = "RequireSecuredWriteAccess";
|
||||
|
||||
/// <summary>
|
||||
/// Roles that satisfy <see cref="RequireSecuredWriteAccess"/>. Held in one
|
||||
/// place so the policy, the docs, and the ManagementActor gate stay in
|
||||
/// lockstep: {<c>Operator</c>, <c>Verifier</c>, <c>Administrator</c>}.
|
||||
/// </summary>
|
||||
public static readonly string[] SecuredWriteAccessRoles =
|
||||
{ Roles.Operator, Roles.Verifier, Roles.Administrator };
|
||||
|
||||
/// <summary>
|
||||
/// Read access to the Audit Log surface (Audit Log page,
|
||||
/// Configuration Audit Log page, Audit nav group). Granted to the
|
||||
@@ -155,6 +173,12 @@ public static class AuthorizationPolicies
|
||||
options.AddPolicy(RequireVerifier, policy =>
|
||||
policy.RequireClaim(JwtTokenService.RoleClaimType, Roles.Verifier));
|
||||
|
||||
// Any-of read gate for the Secured Writes surface (arch-review UA1):
|
||||
// succeeds when the principal holds any of Operator/Verifier/
|
||||
// Administrator, mirroring the ManagementActor ListSecuredWrites gate.
|
||||
options.AddPolicy(RequireSecuredWriteAccess, policy =>
|
||||
policy.RequireClaim(JwtTokenService.RoleClaimType, SecuredWriteAccessRoles));
|
||||
|
||||
// Multi-role permission policies — the policy succeeds when the
|
||||
// principal holds ANY of the mapped roles. RequireClaim with
|
||||
// multiple allowed values is the right primitive: it checks
|
||||
|
||||
@@ -6,6 +6,7 @@ using NSubstitute;
|
||||
using NSubstitute.ExceptionExtensions;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Schemas;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.SecuredWrites;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Scripts;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
@@ -86,6 +87,39 @@ public class ManagementActorTests : TestKit, IDisposable
|
||||
Assert.Contains("Administrator", response.Message);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// ListSecuredWrites role gate (arch-review UA1): read-only, but the
|
||||
// history table exposes process-sensitive tag values, so it is restricted
|
||||
// to the two-person Secured Write roles + Administrator (any-of).
|
||||
// ========================================================================
|
||||
|
||||
[Fact]
|
||||
public void ListSecuredWrites_ViewerOnly_Unauthorized()
|
||||
{
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new ListSecuredWritesCommand(null, null), "Viewer"));
|
||||
ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(5));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("Operator")]
|
||||
[InlineData("Verifier")]
|
||||
[InlineData("Administrator")]
|
||||
public void ListSecuredWrites_SecuredWriteRoles_Allowed(string role)
|
||||
{
|
||||
var securedWriteRepo = Substitute.For<ISecuredWriteRepository>();
|
||||
securedWriteRepo.QueryAsync(
|
||||
Arg.Any<string?>(), Arg.Any<string?>(), Arg.Any<int>(), Arg.Any<int>(),
|
||||
Arg.Any<CancellationToken>())
|
||||
.Returns(Task.FromResult<IReadOnlyList<PendingSecuredWrite>>(
|
||||
Array.Empty<PendingSecuredWrite>()));
|
||||
_services.AddScoped(_ => securedWriteRepo);
|
||||
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new ListSecuredWritesCommand(null, null), role));
|
||||
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DeploymentCommand_WithDesignRole_ReturnsUnauthorized()
|
||||
{
|
||||
|
||||
@@ -352,7 +352,10 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
|
||||
});
|
||||
|
||||
var actor = CreateActor();
|
||||
var envelope = Envelope(new ListSecuredWritesCommand("Pending", "SITE1"), "carol", "Viewer");
|
||||
// arch-review UA1: ListSecuredWrites is gated to the Secured Write
|
||||
// roles + Administrator, so use Operator here (this test asserts filter
|
||||
// forwarding, not the role gate — see ListSecuredWrites_* auth tests).
|
||||
var envelope = Envelope(new ListSecuredWritesCommand("Pending", "SITE1"), "carol", "Operator");
|
||||
|
||||
actor.Tell(envelope);
|
||||
|
||||
@@ -370,7 +373,8 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
|
||||
.Returns(new List<PendingSecuredWrite>());
|
||||
|
||||
var actor = CreateActor();
|
||||
var envelope = Envelope(new ListSecuredWritesCommand(null, null), "carol");
|
||||
// arch-review UA1: gated read — Operator satisfies the any-of gate.
|
||||
var envelope = Envelope(new ListSecuredWritesCommand(null, null), "carol", "Operator");
|
||||
|
||||
actor.Tell(envelope);
|
||||
|
||||
|
||||
@@ -1242,6 +1242,28 @@ public class AuthorizationPolicyTests
|
||||
Assert.False(await EvaluatePolicy(AuthorizationPolicies.RequireOperator, verifierOnly));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("Operator")]
|
||||
[InlineData("Verifier")]
|
||||
[InlineData("Administrator")]
|
||||
public async Task RequireSecuredWriteAccess_SecuredWriteRoles_Satisfy(string role)
|
||||
{
|
||||
// arch-review UA1: the Secured Writes history exposes process-sensitive
|
||||
// tag values, so read access is any-of {Operator, Verifier, Administrator}.
|
||||
var principal = CreatePrincipal(new[] { role });
|
||||
Assert.True(await EvaluatePolicy(AuthorizationPolicies.RequireSecuredWriteAccess, principal));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("Viewer")]
|
||||
[InlineData("Designer")]
|
||||
[InlineData("Deployer")]
|
||||
public async Task RequireSecuredWriteAccess_OtherRoles_Denied(string role)
|
||||
{
|
||||
var principal = CreatePrincipal(new[] { role });
|
||||
Assert.False(await EvaluatePolicy(AuthorizationPolicies.RequireSecuredWriteAccess, principal));
|
||||
}
|
||||
|
||||
private static ClaimsPrincipal CreatePrincipal(string[] roles, string[]? siteIds = null)
|
||||
{
|
||||
var claims = new List<Claim>();
|
||||
|
||||
Reference in New Issue
Block a user