From a212e145acea6e5477945f713ce4fd8ca4a09a86 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 17 Aug 2026 03:58:14 -0400 Subject: [PATCH] feat(security): DashboardTags on API-key constraints; sessions inherit owner tags (SEC-25 groundwork) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a dashboard event-visibility tag to ApiKeyConstraints, riding in the existing constraints JSON blob so no auth-store schema migration is needed (design docs/plans/2026-07-10-dashboard-session-acl-tst15.md sections 3/3.1, open call settled per its own recommendation). The tag is visibility-only: no read, write, browse, or subscribe path consults it, and HasRead/HasWriteConstraints ignore it. GatewaySession gains an immutable, ordinal-ignore-case Tags set stamped at construction from the owning API key, forwarded by MxAccessGatewayService.OpenSession from the resolved ApiKeyIdentity — never from the wire request, so a client cannot label its own session with another tenant's tag. ISessionManager gains a tag-carrying OpenSessionAsync overload whose default implementation forwards to the tagless one, so an implementation that does not model tags opens an untagged (least visible) session. apikey create-key gains --dashboard-tags team-a,team-b (repeatable, trimmed, de-duplicated; an empty segment is rejected rather than dropped) and list-keys prints the tags column. No enforcement yet — the EventsHub ACL that consumes the tag is a later change. --- docs/Authentication.md | 17 ++- docs/Authorization.md | 24 ++++ .../Grpc/MxAccessGatewayService.cs | 8 +- .../Authentication/ApiKeyAdminCliRunner.cs | 7 +- .../ApiKeyAdminCommandLineParser.cs | 40 ++++++- .../ApiKeyConstraintSerializer.cs | 5 + .../Authentication/ApiKeyConstraints.cs | 40 ++++++- .../Sessions/GatewaySession.cs | 30 ++++- .../Sessions/ISessionManager.cs | 27 +++++ .../Sessions/SessionManager.cs | 17 ++- .../Grpc/MxAccessGatewayServiceTests.cs | 55 +++++++++ .../Gateway/Sessions/SessionManagerTests.cs | 49 ++++++++ .../ApiKeyAdminCommandLineParserTests.cs | 58 +++++++++ .../ApiKeyConstraintSerializerTests.cs | 110 ++++++++++++++++++ 14 files changed, 477 insertions(+), 10 deletions(-) create mode 100644 src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyConstraintSerializerTests.cs diff --git a/docs/Authentication.md b/docs/Authentication.md index b7692d2..6b05974 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -263,6 +263,7 @@ mxgateway apikey init-db mxgateway apikey create-key --key-id ops.alice --display-name "Alice (ops)" --scopes read,write mxgateway apikey create-key --key-id area1.reader --display-name "Area 1 reader" --scopes invoke:read,metadata:read --read-subtree "Area1/*" --browse-subtree "Area1/*" mxgateway apikey create-key --key-id ops.temp --display-name "Temp contractor" --scopes invoke:read --expires 90d +mxgateway apikey create-key --key-id team-a.svc --display-name "Team A service" --scopes session:open,invoke:read --dashboard-tags team-a mxgateway apikey create-key --key-id ops.audit --display-name "Audit window" --scopes metadata:read --expires 2027-01-01T00:00:00Z mxgateway apikey list-keys --json mxgateway apikey revoke-key --key-id ops.alice @@ -272,8 +273,20 @@ mxgateway apikey rotate-key --key-id ops.alice Constraint flags are optional. `--read-subtree`, `--write-subtree`, `--read-tag-glob`, `--write-tag-glob`, and `--browse-subtree` are repeatable. `--max-write-classification` accepts one integer. `--read-alarm-only` and -`--read-historized-only` are boolean flags. Existing rows with null constraints -remain fully unconstrained after migration. +`--read-historized-only` are boolean flags. `--dashboard-tags` takes a +comma-separated list (`--dashboard-tags team-a,team-b`) and is repeatable; its +segments are trimmed and de-duplicated ordinal-ignore-case, and an empty segment +is rejected rather than dropped so a stray comma cannot silently persist a grant +the operator did not write. Existing rows with null constraints remain fully +unconstrained after migration; rows written before `--dashboard-tags` existed +deserialize as untagged, unchanged in every other respect. + +`--dashboard-tags` is *not* a data-access constraint — it only labels the key for +dashboard event visibility, and sessions the key opens inherit it. See +[Authorization](./Authorization.md#constraint-enforcement). + +`list-keys` prints the tags as a trailing tab-separated column (`-` when +untagged); the values are operator-chosen labels, not key material. Key ids are restricted by the parser to ASCII letters, digits, periods, and hyphens so they remain safe to embed in the token format and in URL paths used by diff --git a/docs/Authorization.md b/docs/Authorization.md index abb8dc3..2762fa5 100644 --- a/docs/Authorization.md +++ b/docs/Authorization.md @@ -178,6 +178,30 @@ Supported constraints are: | `browse_subtrees` | Contained-path globs used to filter Galaxy browse results and deploy-event counts. | | `read_alarm_only` | Read/subscription commands must target objects with alarm-bearing attributes. | | `read_historized_only` | Read/subscription commands must target objects with historized attributes. | +| `dashboard_tags` | Dashboard event-visibility tags. **Not a data-access constraint** — see below. | + +`dashboard_tags` is the one member of the blob that constrains nothing on the +gRPC data path. No read, write, browse, or subscribe check consults it, and +`HasReadConstraints` / `HasWriteConstraints` deliberately ignore it: adding a tag +neither widens nor narrows what a key may read or write. It rides in the same +serialized blob only to avoid an auth-store schema migration +(`docs/plans/2026-07-10-dashboard-session-acl-tst15.md` §3.1). + +Its sole purpose is dashboard event visibility. A session records the tags of the +API key that opened it (`GatewaySession.Tags`, immutable for the session's life, +compared ordinal-ignore-case). The tags come from the owning key, never from the +client's `OpenSession` request, so a client cannot label its own session with +another tenant's tag. A key with no tags opens untagged sessions. + +Tags are set at key creation with +`apikey create-key --dashboard-tags team-a,team-b` (repeatable; segments are +trimmed and de-duplicated ordinal-ignore-case). Keys created from the dashboard +API Keys page are currently always untagged. + +The tag is carried end to end today; the dashboard ACL that consumes it — scoping +a Viewer's `EventsHub` subscriptions to the sessions their LDAP groups are +granted — is a separate change. Until it lands, the tag affects nothing at +runtime. Glob matching is anchored, case-insensitive, and supports `*` and `?`. Subtree and tag glob lists are alternatives: matching either list allows that diff --git a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs index fa77d2c..ced3554 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs @@ -32,11 +32,17 @@ public sealed class MxAccessGatewayService( try { requestValidator.ValidateOpenSession(request); + + // The session's owner id and its dashboard-visibility tags both come from the resolved + // API key identity, never from the request: the key is the tenant principal, so a + // client cannot label its own session with another tenant's tag (SEC-25). + ApiKeyIdentity? owner = identityAccessor.Current; GatewaySession session = await sessionManager .OpenSessionAsync( SessionOpenRequest.FromContract(request), ResolveClientIdentity(), - identityAccessor.Current?.KeyId, + owner?.KeyId, + owner?.EffectiveConstraints.DashboardTags, context.CancellationToken) .ConfigureAwait(false); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCliRunner.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCliRunner.cs index e852397..0a8cefc 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCliRunner.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCliRunner.cs @@ -143,8 +143,13 @@ public sealed class ApiKeyAdminCliRunner(ApiKeyAdminCommands commands) string expiry = key.ExpiresUtc is { } expires ? expires.ToUniversalTime().ToString("u", System.Globalization.CultureInfo.InvariantCulture) : "-"; + // Dashboard tags are operator-facing labels, not key material, so they are safe to + // print alongside the scopes; "-" keeps the column aligned for an untagged key. + string dashboardTags = key.Constraints.DashboardTags.Count > 0 + ? string.Join(',', key.Constraints.DashboardTags) + : "-"; await output.WriteLineAsync( - $"{key.KeyId}\t{key.DisplayName}\t{status}\t{expiry}\t{string.Join(',', key.Scopes)}") + $"{key.KeyId}\t{key.DisplayName}\t{status}\t{expiry}\t{string.Join(',', key.Scopes)}\t{dashboardTags}") .ConfigureAwait(false); } } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommandLineParser.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommandLineParser.cs index 041094f..607efd6 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommandLineParser.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommandLineParser.cs @@ -233,7 +233,45 @@ public static class ApiKeyAdminCommandLineParser MaxWriteClassification: ParseNullableInt(GetOption(options, "max-write-classification")), BrowseSubtrees: GetOptions(options, "browse-subtree"), ReadAlarmOnly: HasFlag(options, "read-alarm-only"), - ReadHistorizedOnly: HasFlag(options, "read-historized-only")); + ReadHistorizedOnly: HasFlag(options, "read-historized-only")) + { + DashboardTags = ParseDashboardTags(options), + }; + } + + // --dashboard-tags takes a comma-separated list ("team-a,team-b"); repeating the flag unions + // its values. Segments are trimmed and de-duplicated ordinal-ignore-case, matching how the + // enforcement site compares them. An empty segment is rejected rather than dropped: a stray + // comma otherwise silently persists a grant the operator did not mean to write. + private static IReadOnlyList ParseDashboardTags(Dictionary> options) + { + if (!options.TryGetValue("dashboard-tags", out List? values)) + { + return Array.Empty(); + } + + List tags = []; + HashSet seen = new(StringComparer.OrdinalIgnoreCase); + + foreach (string? raw in values) + { + foreach (string segment in (raw ?? string.Empty).Split(',')) + { + string tag = segment.Trim(); + if (tag.Length == 0) + { + throw new FormatException( + "--dashboard-tags must be a comma-separated list of non-empty tags."); + } + + if (seen.Add(tag)) + { + tags.Add(tag); + } + } + } + + return tags.Count == 0 ? Array.Empty() : tags; } // Parses the optional --expires value into an absolute UTC expiry. Accepts a relative diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyConstraintSerializer.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyConstraintSerializer.cs index 73b5575..c0a5418 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyConstraintSerializer.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyConstraintSerializer.cs @@ -22,6 +22,11 @@ public static class ApiKeyConstraintSerializer /// Deserializes API key constraints from JSON, or returns empty constraints if JSON is null or whitespace. /// The JSON string to deserialize. /// The deserialized constraints, or when is null/whitespace. + /// + /// Members absent from the JSON take their default: rows persisted before + /// existed carry no dashboard_tags + /// member and deserialize to an untagged key, unchanged in every other respect. + /// public static ApiKeyConstraints Deserialize(string? json) { if (string.IsNullOrWhiteSpace(json)) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyConstraints.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyConstraints.cs index 6f0f1ee..1cf8418 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyConstraints.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyConstraints.cs @@ -10,6 +10,38 @@ public sealed record ApiKeyConstraints( bool ReadAlarmOnly, bool ReadHistorizedOnly) { + private readonly IReadOnlyList _dashboardTags = Array.Empty(); + + /// + /// Gets the dashboard event-visibility tags granted to this key (SEC-25). + /// + /// + /// + /// This is dashboard event-visibility only. It is never a + /// data-access constraint: no read, write, browse, or subscribe path consults it, and + /// adding a tag neither widens nor narrows what the key may read or write. Sessions + /// opened by the key inherit these tags (GatewaySession.Tags), and a dashboard + /// Viewer may observe a session's mirrored event metadata only when their granted tags + /// intersect the session's. It rides in the same serialized constraints blob purely to + /// avoid an auth-store schema migration — see + /// docs/plans/2026-07-10-dashboard-session-acl-tst15.md §3.1. + /// + /// + /// Tag values are stored exactly as supplied; comparisons are ordinal-ignore-case at the + /// enforcement site, so Team-A and team-a name the same tag. An empty list + /// means untagged. + /// + /// + public IReadOnlyList DashboardTags + { + get => _dashboardTags; + + // Defensive copy: the tag set is a security-relevant grant, so the record must not alias a + // caller-owned list that could be mutated after construction. A null or empty value (an old + // persisted row has no dashboard_tags member at all) normalizes to untagged. + init => _dashboardTags = value is { Count: > 0 } ? [.. value] : Array.Empty(); + } + /// Gets an empty constraints instance with no restrictions. public static ApiKeyConstraints Empty { get; } = new( ReadSubtrees: Array.Empty(), @@ -22,6 +54,11 @@ public sealed record ApiKeyConstraints( ReadHistorizedOnly: false); /// Gets a value indicating whether the constraints are empty (no restrictions). + /// + /// counts here even though it restricts nothing: an empty + /// instance is not persisted at all (ApiKeyConstraintSerializer.Serialize returns + /// null), so a key whose only per-key policy is a dashboard tag must still round-trip. + /// public bool IsEmpty => ReadSubtrees.Count == 0 && WriteSubtrees.Count == 0 @@ -30,7 +67,8 @@ public sealed record ApiKeyConstraints( && MaxWriteClassification is null && BrowseSubtrees.Count == 0 && !ReadAlarmOnly - && !ReadHistorizedOnly; + && !ReadHistorizedOnly + && DashboardTags.Count == 0; /// Gets a value indicating whether any read constraints are defined. public bool HasReadConstraints => diff --git a/src/ZB.MOM.WW.MxGateway.Server/Sessions/GatewaySession.cs b/src/ZB.MOM.WW.MxGateway.Server/Sessions/GatewaySession.cs index f7c8808..5802ff9 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Sessions/GatewaySession.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Sessions/GatewaySession.cs @@ -1,3 +1,4 @@ +using System.Collections.Frozen; using System.Diagnostics; using System.Runtime.CompilerServices; using Microsoft.Extensions.Logging; @@ -12,6 +13,10 @@ namespace ZB.MOM.WW.MxGateway.Server.Sessions; public sealed class GatewaySession { + // Shared untagged sentinel: most sessions carry no dashboard tags. Frozen so the exposed set + // cannot be mutated by a cast — the tag set is a visibility grant, not a scratch collection. + private static readonly IReadOnlySet EmptyTags = FrozenSet.Empty; + private readonly object _syncRoot = new(); private readonly SemaphoreSlim _closeLock = new(1, 1); private readonly SessionEventStreaming _eventStreaming; @@ -149,6 +154,12 @@ public sealed class GatewaySession /// using 's clock so the timer /// is unit-testable. /// + /// + /// Dashboard event-visibility tags inherited from the owning API key (SEC-25). Copied into + /// the immutable set; or empty means untagged. + /// The tags come from the owner key, never from the client's wire request, so a client + /// cannot label its own session with another tenant's tag. + /// public GatewaySession( string sessionId, string backendName, @@ -167,7 +178,8 @@ public sealed class GatewaySession TimeSpan detachGrace = default, TimeSpan workerReadyWaitTimeout = default, ArrayAddressNormalizer? addressNormalizer = null, - TimeSpan faultedGrace = default) + TimeSpan faultedGrace = default, + IReadOnlyList? ownerDashboardTags = null) { if (string.IsNullOrWhiteSpace(sessionId)) { @@ -195,6 +207,9 @@ public sealed class GatewaySession Nonce = nonce; ClientIdentity = clientIdentity; OwnerKeyId = ownerKeyId; + Tags = ownerDashboardTags is { Count: > 0 } + ? ownerDashboardTags.ToFrozenSet(StringComparer.OrdinalIgnoreCase) + : EmptyTags; ClientSessionName = clientSessionName; ClientCorrelationId = clientCorrelationId; CommandTimeout = commandTimeout; @@ -241,6 +256,19 @@ public sealed class GatewaySession /// public string? OwnerKeyId { get; } + /// + /// Gets the dashboard event-visibility tags this session inherited from its owning API key + /// (SEC-25). An empty set means untagged. + /// + /// + /// Immutable for the session's life — assigned once at construction from the owner key's + /// ApiKeyConstraints.DashboardTags — so a dashboard subscription decided at join time + /// never has to be re-evaluated. The set compares ordinal-ignore-case. These tags gate + /// nothing on the gRPC data path; they exist only so the dashboard can scope which sessions' + /// mirrored event metadata a Viewer may observe. + /// + public IReadOnlySet Tags { get; } + /// /// Gets the client-supplied session name. /// diff --git a/src/ZB.MOM.WW.MxGateway.Server/Sessions/ISessionManager.cs b/src/ZB.MOM.WW.MxGateway.Server/Sessions/ISessionManager.cs index 9cb0191..0470f5e 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Sessions/ISessionManager.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Sessions/ISessionManager.cs @@ -17,6 +17,33 @@ public interface ISessionManager string? ownerKeyId, CancellationToken cancellationToken); + /// + /// Opens a new gateway session, stamping the owning API key's dashboard event-visibility + /// tags onto it (SEC-25). + /// + /// Request payload. + /// Client identity string. + /// API key identifier of the caller creating the session. + /// + /// The owner key's ApiKeyConstraints.DashboardTags. Null or empty opens an untagged + /// session. Never sourced from the client's wire request — see + /// docs/plans/2026-07-10-dashboard-session-acl-tst15.md §3.1. + /// + /// Token to cancel the asynchronous operation. + /// The newly opened session. + /// + /// The default implementation forwards to the tagless overload, so an implementation that + /// does not model tags (unit-test fakes) opens an untagged session. That is the + /// fail-closed direction: untagged sessions are the least dashboard-visible ones. + /// + Task OpenSessionAsync( + SessionOpenRequest request, + string? clientIdentity, + string? ownerKeyId, + IReadOnlyList? ownerDashboardTags, + CancellationToken cancellationToken) + => OpenSessionAsync(request, clientIdentity, ownerKeyId, cancellationToken); + /// Attempts to retrieve a session by ID. /// Identifier of the session. /// The retrieved session, if found. diff --git a/src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionManager.cs b/src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionManager.cs index 0cf27b6..a58aec0 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionManager.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionManager.cs @@ -87,11 +87,20 @@ public sealed class SessionManager : ISessionManager _sessionSlots = new SemaphoreSlim(_options.Sessions.MaxSessions, _options.Sessions.MaxSessions); } + /// + public Task OpenSessionAsync( + SessionOpenRequest request, + string? clientIdentity, + string? ownerKeyId, + CancellationToken cancellationToken) + => OpenSessionAsync(request, clientIdentity, ownerKeyId, ownerDashboardTags: null, cancellationToken); + /// public async Task OpenSessionAsync( SessionOpenRequest request, string? clientIdentity, string? ownerKeyId, + IReadOnlyList? ownerDashboardTags, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(request); @@ -101,7 +110,7 @@ public sealed class SessionManager : ISessionManager bool sessionOpenedRecorded = false; try { - session = CreateSession(request, clientIdentity, ownerKeyId); + session = CreateSession(request, clientIdentity, ownerKeyId, ownerDashboardTags); if (!_registry.TryAdd(session)) { throw new SessionManagerException( @@ -494,7 +503,8 @@ public sealed class SessionManager : ISessionManager private GatewaySession CreateSession( SessionOpenRequest request, string? clientIdentity, - string? ownerKeyId) + string? ownerKeyId, + IReadOnlyList? ownerDashboardTags) { string sessionUid = Guid.NewGuid().ToString("N"); string sessionId = $"session-{sessionUid}"; @@ -541,7 +551,8 @@ public sealed class SessionManager : ISessionManager TimeSpan.FromSeconds(Math.Max(0, _options.Sessions.DetachGraceSeconds)), TimeSpan.FromMilliseconds(Math.Max(0, _options.Sessions.WorkerReadyWaitTimeoutMs)), _addressNormalizer, - TimeSpan.FromSeconds(Math.Max(0, _options.Sessions.FaultedGraceSeconds))); + TimeSpan.FromSeconds(Math.Max(0, _options.Sessions.FaultedGraceSeconds)), + ownerDashboardTags); } private static string CreateClientCorrelationId( diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs index cfc06a3..6a82b5d 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs @@ -50,6 +50,45 @@ public sealed class MxAccessGatewayServiceTests Assert.Equal("operator-session", sessionManager.LastOpenRequest?.ClientSessionName); } + /// + /// Verifies OpenSession forwards the calling key's dashboard-visibility tags, so the + /// session's tags are derived from the owning API key rather than the wire request (SEC-25). + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task OpenSession_WithTaggedKey_ForwardsOwnerDashboardTags() + { + GatewayRequestIdentityAccessor identityAccessor = new(); + FakeSessionManager sessionManager = new(); + MxAccessGatewayService service = CreateService(sessionManager, identityAccessor); + ApiKeyIdentity identity = CreateIdentity() with + { + Constraints = ApiKeyConstraints.Empty with { DashboardTags = ["team-a"] }, + }; + + using IDisposable identityScope = identityAccessor.Push(identity); + await service.OpenSession(new OpenSessionRequest(), new TestServerCallContext()); + + Assert.Equal(["team-a"], sessionManager.LastOwnerDashboardTags); + } + + /// + /// Verifies an unauthenticated OpenSession (no resolved key identity) opens an untagged + /// session — the fail-closed state for dashboard event visibility. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task OpenSession_WithoutIdentity_ForwardsNoDashboardTags() + { + FakeSessionManager sessionManager = new(); + MxAccessGatewayService service = CreateService(sessionManager, new GatewayRequestIdentityAccessor()); + + await service.OpenSession(new OpenSessionRequest(), new TestServerCallContext()); + + Assert.Null(sessionManager.LastOwnerDashboardTags); + Assert.Null(sessionManager.LastOwnerKeyId); + } + /// /// Verifies that Invoke maps a genuinely missing session to NotFound via the /// service's own ResolveSession lookup. No InvokeException is @@ -517,6 +556,9 @@ public sealed class MxAccessGatewayServiceTests /// The last owner key id passed to OpenSessionAsync. public string? LastOwnerKeyId { get; private set; } + /// The last owner dashboard tags passed to OpenSessionAsync. + public IReadOnlyList? LastOwnerDashboardTags { get; private set; } + /// The last session ID the event stream service was asked to stream. public string? LastReadEventsSessionId { get; private set; } @@ -564,6 +606,19 @@ public sealed class MxAccessGatewayServiceTests return Task.FromResult(OpenSessionResult ?? CreateSession("session-1", processId: 1234)); } + /// + public Task OpenSessionAsync( + SessionOpenRequest request, + string? clientIdentity, + string? ownerKeyId, + IReadOnlyList? ownerDashboardTags, + CancellationToken cancellationToken) + { + LastOwnerDashboardTags = ownerDashboardTags; + + return OpenSessionAsync(request, clientIdentity, ownerKeyId, cancellationToken); + } + /// public bool TryGetSession( string sessionId, diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs index f4e16f1..55c2cb6 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs @@ -109,6 +109,55 @@ public sealed class SessionManagerTests Assert.Null(session.OwnerKeyId); } + /// + /// Verifies a session inherits the owning API key's dashboard-visibility tags (SEC-25), + /// compared ordinal-ignore-case so a differently cased grant still matches. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task OpenSessionAsync_WithOwnerDashboardTags_CopiesTagsOntoSession() + { + SessionManager manager = CreateManager(new FakeSessionWorkerClientFactory(new FakeWorkerClient())); + + GatewaySession session = await manager.OpenSessionAsync( + CreateOpenRequest(), + clientIdentity: "MyKey Display", + ownerKeyId: "key-abc123", + ownerDashboardTags: ["team-a", "team-b"], + CancellationToken.None); + + Assert.Equal(["team-a", "team-b"], session.Tags.OrderBy(tag => tag, StringComparer.Ordinal)); + Assert.Contains("TEAM-A", session.Tags); + } + + /// + /// Verifies a session opened by a key with no dashboard tags is untagged, which is the + /// fail-closed state for dashboard event visibility. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task OpenSessionAsync_WithoutOwnerDashboardTags_LeavesSessionUntagged() + { + SessionManager manager = CreateManager(new FakeSessionWorkerClientFactory(new FakeWorkerClient())); + + GatewaySession session = await manager.OpenSessionAsync( + CreateOpenRequest(), + clientIdentity: "MyKey Display", + ownerKeyId: "key-abc123", + ownerDashboardTags: null, + CancellationToken.None); + + Assert.Empty(session.Tags); + + GatewaySession tagless = await manager.OpenSessionAsync( + CreateOpenRequest(), + "client-1", + ownerKeyId: null, + CancellationToken.None); + + Assert.Empty(tagless.Tags); + } + /// Verifies that opening a session sets the initial lease expiry from the configured default lease. /// A task that represents the asynchronous operation. [Fact] diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCommandLineParserTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCommandLineParserTests.cs index f7b94bb..467e7d0 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCommandLineParserTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCommandLineParserTests.cs @@ -196,6 +196,64 @@ public sealed class ApiKeyAdminCommandLineParserTests Assert.True(constraints.ReadHistorizedOnly); } + /// + /// Verifies --dashboard-tags parses a comma-separated list, trimming segments and unioning + /// repeated occurrences of the flag without duplicating a tag that differs only by case. + /// + [Fact] + public void Parse_CreateKeyCommand_WithDashboardTags_ParsesTrimmedTagList() + { + ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse( + [ + "apikey", + "create-key", + "--key-id", + "operator01", + "--display-name", + "Operator", + "--dashboard-tags", + " team-a , team-b ", + "--dashboard-tags", + "TEAM-A,team-c" + ]); + + Assert.True(result.IsApiKeyCommand); + Assert.Null(result.Error); + Assert.NotNull(result.Command); + Assert.Equal(["team-a", "team-b", "team-c"], result.Command.Constraints.DashboardTags); + } + + /// Verifies a create-key command without --dashboard-tags leaves the key untagged. + [Fact] + public void Parse_CreateKeyCommand_WithoutDashboardTags_LeavesKeyUntagged() + { + ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse( + ["apikey", "create-key", "--key-id", "operator01", "--display-name", "Operator"]); + + Assert.NotNull(result.Command); + Assert.Empty(result.Command.Constraints.DashboardTags); + } + + /// + /// Verifies an empty tag segment is rejected rather than dropped: a stray comma must not + /// silently persist a grant the operator did not write. + /// + [Theory] + [InlineData("team-a,,team-b")] + [InlineData("team-a, ")] + [InlineData("")] + public void Parse_CreateKeyCommand_WithEmptyDashboardTag_Fails(string tags) + { + ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse( + ["apikey", "create-key", "--key-id", "operator01", "--display-name", "Operator", + $"--dashboard-tags={tags}"]); + + Assert.True(result.IsApiKeyCommand); + Assert.Null(result.Command); + Assert.NotNull(result.Error); + Assert.Contains("--dashboard-tags", result.Error, StringComparison.Ordinal); + } + /// Verifies that create-key command without display name returns error. [Fact] public void Parse_CreateKeyWithoutDisplayName_ReturnsError() diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyConstraintSerializerTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyConstraintSerializerTests.cs new file mode 100644 index 0000000..4084f41 --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyConstraintSerializerTests.cs @@ -0,0 +1,110 @@ +using ZB.MOM.WW.MxGateway.Server.Security.Authentication; + +namespace ZB.MOM.WW.MxGateway.Tests.Security.Authentication; + +public sealed class ApiKeyConstraintSerializerTests +{ + /// Verifies that dashboard tags survive a serialize/deserialize round trip. + [Fact] + public void RoundTrip_WithDashboardTags_PreservesTags() + { + ApiKeyConstraints constraints = ApiKeyConstraints.Empty with + { + ReadSubtrees = ["Area1/*"], + DashboardTags = ["team-a", "team-b"], + }; + + string? json = ApiKeyConstraintSerializer.Serialize(constraints); + + Assert.NotNull(json); + Assert.Contains("dashboard_tags", json, StringComparison.Ordinal); + + ApiKeyConstraints restored = ApiKeyConstraintSerializer.Deserialize(json); + + Assert.Equal(["team-a", "team-b"], restored.DashboardTags); + Assert.Equal(["Area1/*"], restored.ReadSubtrees); + } + + /// + /// Verifies a key whose only per-key policy is a dashboard tag is still persisted: the + /// serializer drops empty constraints entirely, so the tag must count as non-empty. + /// + [Fact] + public void Serialize_WithOnlyDashboardTags_IsNotTreatedAsEmpty() + { + ApiKeyConstraints constraints = ApiKeyConstraints.Empty with { DashboardTags = ["team-a"] }; + + Assert.False(constraints.IsEmpty); + Assert.NotNull(ApiKeyConstraintSerializer.Serialize(constraints)); + } + + /// Verifies that dashboard tags never register as read or write (data-access) constraints. + [Fact] + public void DashboardTags_AreNotDataAccessConstraints() + { + ApiKeyConstraints constraints = ApiKeyConstraints.Empty with { DashboardTags = ["team-a"] }; + + Assert.False(constraints.HasReadConstraints); + Assert.False(constraints.HasWriteConstraints); + } + + /// + /// Verifies a row persisted before the dashboard-tag field existed still deserializes, with + /// every pre-existing constraint intact and an untagged (empty, never null) tag list. + /// + [Fact] + public void Deserialize_LegacyJsonWithoutDashboardTags_YieldsUntaggedConstraints() + { + const string LegacyJson = """ + { + "read_subtrees": ["Area1/*"], + "write_subtrees": [], + "read_tag_globs": [], + "write_tag_globs": ["Pump_*"], + "max_write_classification": 2, + "browse_subtrees": ["Area1/*"], + "read_alarm_only": true, + "read_historized_only": false + } + """; + + ApiKeyConstraints constraints = ApiKeyConstraintSerializer.Deserialize(LegacyJson); + + Assert.Empty(constraints.DashboardTags); + Assert.Equal(["Area1/*"], constraints.ReadSubtrees); + Assert.Equal(["Pump_*"], constraints.WriteTagGlobs); + Assert.Equal(2, constraints.MaxWriteClassification); + Assert.Equal(["Area1/*"], constraints.BrowseSubtrees); + Assert.True(constraints.ReadAlarmOnly); + Assert.False(constraints.ReadHistorizedOnly); + } + + /// Verifies an explicit JSON null for the tag list normalizes to untagged rather than null. + [Fact] + public void Deserialize_ExplicitNullDashboardTags_YieldsEmptyList() + { + const string Json = """ + { + "read_subtrees": [], + "write_subtrees": [], + "read_tag_globs": [], + "write_tag_globs": [], + "max_write_classification": null, + "browse_subtrees": [], + "read_alarm_only": false, + "read_historized_only": false, + "dashboard_tags": null + } + """; + + Assert.Empty(ApiKeyConstraintSerializer.Deserialize(Json).DashboardTags); + } + + /// Verifies null or whitespace constraint JSON deserializes to the untagged empty instance. + [Fact] + public void Deserialize_NullJson_YieldsEmptyConstraints() + { + Assert.Same(ApiKeyConstraints.Empty, ApiKeyConstraintSerializer.Deserialize(null)); + Assert.Empty(ApiKeyConstraints.Empty.DashboardTags); + } +}