From 243c3fa6196cfbe8714fb479e5288bb7a47d8317 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 9 Nov 2026 12:30:04 -0500 Subject: [PATCH] feat(auth): typed ApiKeyPepperUnavailableException for missing API-key pepper; bump to 0.4.2 (scadaproj#23) Derives from InvalidOperationException so existing catches keep working. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV --- ZB.MOM.WW.Auth/CLAUDE.md | 2 + ZB.MOM.WW.Auth/Directory.Build.props | 4 +- .../Admin/ApiKeyAdminCommands.cs | 2 +- .../ApiKeyPepperUnavailableException.cs | 34 +++++++++++++ .../ApiKeyAdminCommandsTests.cs | 50 ++++++++++++++++++- 5 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.ApiKeys/ApiKeyPepperUnavailableException.cs diff --git a/ZB.MOM.WW.Auth/CLAUDE.md b/ZB.MOM.WW.Auth/CLAUDE.md index 20cdac1..59b50fb 100644 --- a/ZB.MOM.WW.Auth/CLAUDE.md +++ b/ZB.MOM.WW.Auth/CLAUDE.md @@ -41,3 +41,5 @@ MxAccessGateway & HistorianGateway → the original four; ScadaBridge → those **`0.3.0` (2026-09-02) — `ZB.MOM.WW.Auth.ApiKeys.SqlServer`, a SQL Server provider for the API-key store** (design `ScadaBridge/docs/plans/2026-09-02-inbound-api-key-shared-store-design.md`; **5 pkgs published to the feed**, `ApiKeys.SqlServer` search-verified from the ScadaBridge checkout; family `Directory.Build.props` now `0.3.0`). Motivation: `Auth.ApiKeys`' SQLite store is a per-node file, so on a two-node active/standby host (ScadaBridge central) a key minted on one node exists only there and a failover 401s every inbound caller (ScadaBridge deferred-work row 108). The new package implements `IApiKeyStore`/`IApiKeyAdminStore`/`IApiKeyAuditStore` over `Microsoft.Data.SqlClient` in a `zbauth` schema (v1: `schema_version`, `api_keys`, `api_key_audit`; all timestamps `DATETIMEOFFSET(7)`, tick-exact) with a concurrent-boot-safe migrator (Serializable tx, retry on 1205/2714/1913/2627 with **jittered** backoff — the Secrets replicator's flat `100*attempt` backoff exhausted five retries about once per sixty 8-way boots in the port's soak test, so the same latent defect is an open follow-up there → #18) and a `SqliteApiKeyStoreImporter` that copies an existing SQLite file into the schema insert-where-absent (idempotent, so both nodes of a pair can import their own file and the result is the union). Wiring: `services.AddZbApiKeyAuthSqlServer(config, apiKeySection, sqlServerSection)` registers the SQL Server stores + migrator **before** delegating to `AddZbApiKeyAuth`, whose `TryAdd*` registrations then yield (same displacement mechanism as Secrets 0.5.0). One `Abstractions` addition, additive: `IApiKeyStoreMigrator`, which `SqliteAuthStoreMigrator` now implements and `ApiKeyAdminCommands` accepts via a new ctor (old `SqliteAuthStoreMigrator` ctor retained — the three `0.2.1` consumers compile unchanged). Store semantics are pinned to the SQLite contract tests re-run live (mark-used skips revoked keys, delete only removes revoked keys, rotate clears last-used/revoked, list newest-first). 92 tests in the new project, 351 across the solution, 0 failed. A DBA-run copy of the DDL ships at `docs/sql/zbauth-schema.sql`. **`tools/LdapProbe`** (`ldapprobe`, not packable, outside the Auth solution) is the diagnostic that found the above: it drives the **real** `LdapAuthService` so its verdict is what the four apps produce, then separately queries the directory for what the library cannot report — full group DNs, **nested** groups via `LDAP_MATCHING_RULE_IN_CHAIN`, the **primary group** (which AD omits from `memberOf`, so it can never be a role group), group type (security vs distribution), decoded `userAccountControl`, referral counts, and the TLS chain with key sizes. `--list-groups ''` enumerates every group in the directory matching a pattern, which a membership dump structurally cannot do. Findings recorded in [`ldap_setup.md`](ldap_setup.md), including the **`UG_WW_*` Wonderware group convention (39 groups)** that partly overturns that doc's earlier "no group→role convention exists" claim — it was true of Ignition, false of the domain. + +**`0.4.2` (unpublished, scadaproj#23 + #22):** `ZB.MOM.WW.Auth.ApiKeys.ApiKeyPepperUnavailableException : InvalidOperationException` (message still `"pepper unavailable"`) is now thrown by `ApiKeyAdminCommands` create/rotate when the pepper is missing, so consumers can catch it by type; existing `catch (InvalidOperationException)` blocks keep working (only exact-type assertions like xunit `ThrowsAsync` need updating). **Release note: consumers can drop their own `IApiKeyPepperProvider` pre-checks** — mxaccessgw's `DashboardApiKeyManagementService` and any similar code. Also pins `System.Security.Cryptography.Pkcs 9.0.21` (transitive, via SqlClient 6.0.2) to clear NuGetAudit NU1902/NU1903. Not yet published to the feed. diff --git a/ZB.MOM.WW.Auth/Directory.Build.props b/ZB.MOM.WW.Auth/Directory.Build.props index b89fd0a..d1d9632 100644 --- a/ZB.MOM.WW.Auth/Directory.Build.props +++ b/ZB.MOM.WW.Auth/Directory.Build.props @@ -10,7 +10,9 @@ ServiceAccount stays the default, so existing consumers need no change. --> - 0.4.1 + + 0.4.2 true diff --git a/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.ApiKeys/Admin/ApiKeyAdminCommands.cs b/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.ApiKeys/Admin/ApiKeyAdminCommands.cs index 4e871bc..9a40fee 100644 --- a/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.ApiKeys/Admin/ApiKeyAdminCommands.cs +++ b/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.ApiKeys/Admin/ApiKeyAdminCommands.cs @@ -285,7 +285,7 @@ public sealed class ApiKeyAdminCommands string? pepper = _pepperProvider.GetPepper(); if (string.IsNullOrWhiteSpace(pepper)) { - throw new InvalidOperationException("pepper unavailable"); + throw new ApiKeyPepperUnavailableException(); } return pepper; diff --git a/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.ApiKeys/ApiKeyPepperUnavailableException.cs b/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.ApiKeys/ApiKeyPepperUnavailableException.cs new file mode 100644 index 0000000..e2429b0 --- /dev/null +++ b/ZB.MOM.WW.Auth/src/ZB.MOM.WW.Auth.ApiKeys/ApiKeyPepperUnavailableException.cs @@ -0,0 +1,34 @@ +namespace ZB.MOM.WW.Auth.ApiKeys; + +/// +/// Thrown when an API-key operation needs the secret pepper but +/// returned null, empty or whitespace +/// (create and rotate in ApiKeyAdminCommands). +/// +/// +/// Derives from , which is what these paths threw before +/// this type existed, so existing catch (InvalidOperationException) blocks keep working. +/// The message stays "pepper unavailable". Consumers can catch this type instead of +/// pre-checking themselves or matching the message. The verifier +/// does not throw it: a missing pepper surfaces there as ApiKeyFailure.PepperUnavailable. +/// +public sealed class ApiKeyPepperUnavailableException : InvalidOperationException +{ + /// Creates the exception with the default "pepper unavailable" message. + public ApiKeyPepperUnavailableException() + : base("pepper unavailable") + { + } + + /// Creates the exception with a custom message. + public ApiKeyPepperUnavailableException(string message) + : base(message) + { + } + + /// Creates the exception with a custom message and inner exception. + public ApiKeyPepperUnavailableException(string message, Exception innerException) + : base(message, innerException) + { + } +} diff --git a/ZB.MOM.WW.Auth/tests/ZB.MOM.WW.Auth.ApiKeys.Tests/ApiKeyAdminCommandsTests.cs b/ZB.MOM.WW.Auth/tests/ZB.MOM.WW.Auth.ApiKeys.Tests/ApiKeyAdminCommandsTests.cs index f993e9f..4165c7c 100644 --- a/ZB.MOM.WW.Auth/tests/ZB.MOM.WW.Auth.ApiKeys.Tests/ApiKeyAdminCommandsTests.cs +++ b/ZB.MOM.WW.Auth/tests/ZB.MOM.WW.Auth.ApiKeys.Tests/ApiKeyAdminCommandsTests.cs @@ -163,7 +163,7 @@ public sealed class ApiKeyAdminCommandsTests : IAsyncLifetime int auditCountBefore = (await _audit.ListRecentAsync(50, CancellationToken.None)).Count; - await Assert.ThrowsAsync(() => commands.CreateKeyAsync( + await Assert.ThrowsAsync(() => commands.CreateKeyAsync( "key-x", "No Pepper", new HashSet(StringComparer.Ordinal), @@ -297,6 +297,52 @@ public sealed class ApiKeyAdminCommandsTests : IAsyncLifetime Assert.Equal("not-found", auditRow.Details); } + [Fact] + public async Task PepperUnavailable_IsStillCatchableAsInvalidOperationException_WithOriginalMessage() + { + // Back-compat: consumers that caught InvalidOperationException before the typed exception + // existed must keep working, for both create and rotate. + ApiKeyAdminCommands setup = BuildCommands(pepper: Pepper); + await setup.InitDbAsync(null, CancellationToken.None); + await setup.CreateKeyAsync("key-bc", "BC", new HashSet(StringComparer.Ordinal), null, null, CancellationToken.None); + ApiKeyAdminCommands nopepper = BuildCommands(pepper: null); + + InvalidOperationException create = await Assert.ThrowsAnyAsync(() => + nopepper.CreateKeyAsync("key-bc2", "BC2", new HashSet(StringComparer.Ordinal), null, null, CancellationToken.None)); + InvalidOperationException rotate = await Assert.ThrowsAnyAsync(() => + nopepper.RotateKeyAsync("key-bc", null, CancellationToken.None)); + + Assert.IsType(create); + Assert.IsType(rotate); + Assert.Equal("pepper unavailable", create.Message); + Assert.Equal("pepper unavailable", rotate.Message); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public async Task CreateKey_NullEmptyOrWhitespacePepper_ThrowsTypedException(string? pepper) + { + ApiKeyAdminCommands commands = BuildCommands(pepper); + await new ApiKeyAdminCommands(_options, _admin, _audit, new FakePepperProvider(Pepper), _migrator) + .InitDbAsync(null, CancellationToken.None); + + await Assert.ThrowsAsync(() => commands.CreateKeyAsync( + "key-w", "W", new HashSet(StringComparer.Ordinal), null, null, CancellationToken.None)); + } + + [Fact] + public void PepperUnavailableException_ConstructorOverloads_AreInvalidOperationExceptions() + { + var inner = new Exception("inner"); + Assert.Equal("pepper unavailable", new ApiKeyPepperUnavailableException().Message); + Assert.Equal("custom", new ApiKeyPepperUnavailableException("custom").Message); + var withInner = new ApiKeyPepperUnavailableException("m", inner); + Assert.Same(inner, withInner.InnerException); + Assert.IsAssignableFrom(withInner); + } + [Fact] public async Task RotateKey_PepperUnavailable_Throws_HashUnchanged_AndAppendsNoAudit() { @@ -319,7 +365,7 @@ public sealed class ApiKeyAdminCommandsTests : IAsyncLifetime // Act: rotate with no pepper available. ApiKeyAdminCommands nopepper = BuildCommands(pepper: null); - await Assert.ThrowsAsync(() => + await Assert.ThrowsAsync(() => nopepper.RotateKeyAsync("key-1", null, CancellationToken.None)); // Assert: stored hash is unchanged.