diff --git a/docs/Authorization.md b/docs/Authorization.md index dc06ab5..b1fb269 100644 --- a/docs/Authorization.md +++ b/docs/Authorization.md @@ -197,7 +197,10 @@ 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). The dashboard API Keys page sets them too: its create form has a **Dashboard tags** field alongside the data-access -constraints, split on the same separators the other constraint fields use. +constraints, split on the same separators the other constraint fields use and +de-duplicated ordinal-ignore-case as the CLI does. The two differ on one point: a +stray separator drops an empty segment silently on the form rather than failing +the command, because a browser form has no exit code to fail with. That page's constraints column names `dashboard_tags` like any other member. It has to: `IsEmpty` counts the tags, so a key whose only recorded policy is a diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor index 870c6e6..62009d2 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor @@ -121,7 +121,8 @@ else aria-describedby="dashboardTagsHelp" @bind="CreateModel.DashboardTags" @bind:event="oninput">
- Comma- or newline-separated; mirrors apikey create-key --dashboard-tags. + Split on the same separators as the constraint fields above, and + de-duplicated case-insensitively — two spellings of a tag are one grant. Matched case-insensitively against the viewer grants in Dashboard:GroupToTag. Scopes dashboard event visibility only — never what the key may read, write, or browse. Empty leaves the key's sessions @@ -478,7 +479,7 @@ else { // Init-only rather than positional (it was bolted onto the record after the // eight-member constructor shipped), so it is attached here instead. - DashboardTags = ParseList(CreateModel.DashboardTags), + DashboardTags = ParseDashboardTags(CreateModel.DashboardTags), }); return true; @@ -575,6 +576,18 @@ else } } + // Dashboard tags alone are de-duplicated ordinal-ignore-case, which is how the enforcement site + // compares them — two spellings are one grant, and ApiKeyAdminCommandLineParser.ParseDashboardTags + // collapses them the same way, first spelling winning. Persisting both would not change who can + // see what (GatewaySession holds its tags in a case-insensitive set) but the constraints column + // would report one grant twice, and a security grant that reads as two is the wrong kind of + // wrong on an audit surface. The other five list fields keep ParseList's verbatim behaviour: + // their globs are matched literally, so near-duplicates there are not necessarily the same rule. + private static IReadOnlyList ParseDashboardTags(string? value) + { + return [.. ParseList(value).Distinct(StringComparer.OrdinalIgnoreCase)]; + } + private static IReadOnlyList ParseList(string? value) { return (value ?? string.Empty) diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/ApiKeysPageDashboardTagsTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/ApiKeysPageDashboardTagsTests.cs index fd98b5a..be5659a 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/ApiKeysPageDashboardTagsTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/ApiKeysPageDashboardTagsTests.cs @@ -111,6 +111,72 @@ public sealed class ApiKeysPageDashboardTagsTests Assert.False(request.Constraints.HasWriteConstraints); } + /// + /// Two spellings of one tag persist as one grant, matching how + /// ApiKeyAdminCommandLineParser.ParseDashboardTags collapses them and how the + /// enforcement site compares them. + /// + /// + /// Enforcement would survive the duplicate — a session holds its tags in a + /// case-insensitive set — but the API Keys page's constraints column would render + /// dashboard_tags=[team-a, TEAM-A] and report one grant as two, which is the + /// reading an audit surface can least afford. + /// + [Fact] + public void TryBuildCreateRequest_DeduplicatesDashboardTagsIgnoringCase() + { + ApiKeysPage page = new(); + page.CreateModel.DashboardTags = "team-a, TEAM-A, team-a"; + + bool built = page.TryBuildCreateRequest(out DashboardApiKeyManagementRequest? request, out string? error); + + Assert.True(built, error); + Assert.NotNull(request); + + // First spelling typed wins, so what the operator wrote is what the audit surface shows. + Assert.Equal(["team-a"], request.Constraints.DashboardTags); + } + + /// + /// A dedupe that spanned fields would be a bug of its own: the five glob lists are matched + /// literally, so near-duplicates there are not necessarily the same rule and must survive + /// verbatim. + /// + [Fact] + public void TryBuildCreateRequest_DoesNotDeduplicateTheOtherConstraintLists() + { + ApiKeysPage page = new(); + page.CreateModel.ReadSubtrees = "Area1, AREA1"; + + bool built = page.TryBuildCreateRequest(out DashboardApiKeyManagementRequest? request, out string? error); + + Assert.True(built, error); + Assert.NotNull(request); + Assert.Equal(["Area1", "AREA1"], request.Constraints.ReadSubtrees); + } + + /// + /// Reset clears the new field, so the next key minted in the same dialog does not + /// inherit the previous key's tag grant. + /// + [Fact] + public void Reset_ClearsDashboardTagsAlongsideTheOtherConstraintFields() + { + ApiKeysPage page = new(); + page.CreateModel.DashboardTags = "team-a"; + + // A sibling field pins the assertion to Reset itself: were Reset a no-op, both would + // survive and the failure would name the method rather than the one line. + page.CreateModel.ReadSubtrees = "Area1"; + page.CreateModel.KeyId = "leftover"; + + page.CreateModel.Reset(); + + Assert.Equal(string.Empty, page.CreateModel.DashboardTags); + Assert.Equal(string.Empty, page.CreateModel.ReadSubtrees); + Assert.Equal(string.Empty, page.CreateModel.KeyId); + } + /// An empty field leaves the key untagged rather than inventing a tag. [Fact] public void TryBuildCreateRequest_WhenDashboardTagsIsBlank_LeavesTheKeyUntagged()