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
This commit is contained in:
Joseph Doherty
2026-11-09 12:30:04 -05:00
parent 8bb2679e43
commit 243c3fa619
5 changed files with 88 additions and 4 deletions
+2
View File
@@ -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 '<pat>'` 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<InvalidOperationException>` 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.
+3 -1
View File
@@ -10,7 +10,9 @@
ServiceAccount stays the default, so existing consumers need no change. -->
<!-- 0.4.1: Username canonicalised from the entry's UserNameAttribute; empty passwords refused
in both bind modes (ServiceAccount-mode behaviour change); template docs prefer EXAMPLE\{0}. -->
<Version>0.4.1</Version>
<!-- 0.4.2: typed ApiKeyPepperUnavailableException (: InvalidOperationException) for a missing pepper on
create/rotate; patched System.Security.Cryptography.Pkcs transitive pin (SqlServer package). -->
<Version>0.4.2</Version>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
</PropertyGroup>
@@ -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;
@@ -0,0 +1,34 @@
namespace ZB.MOM.WW.Auth.ApiKeys;
/// <summary>
/// Thrown when an API-key operation needs the secret pepper but
/// <see cref="IApiKeyPepperProvider.GetPepper"/> returned <c>null</c>, empty or whitespace
/// (create and rotate in <c>ApiKeyAdminCommands</c>).
/// </summary>
/// <remarks>
/// Derives from <see cref="InvalidOperationException"/>, which is what these paths threw before
/// this type existed, so existing <c>catch (InvalidOperationException)</c> blocks keep working.
/// The message stays <c>"pepper unavailable"</c>. Consumers can catch this type instead of
/// pre-checking <see cref="IApiKeyPepperProvider"/> themselves or matching the message. The verifier
/// does not throw it: a missing pepper surfaces there as <c>ApiKeyFailure.PepperUnavailable</c>.
/// </remarks>
public sealed class ApiKeyPepperUnavailableException : InvalidOperationException
{
/// <summary>Creates the exception with the default <c>"pepper unavailable"</c> message.</summary>
public ApiKeyPepperUnavailableException()
: base("pepper unavailable")
{
}
/// <summary>Creates the exception with a custom message.</summary>
public ApiKeyPepperUnavailableException(string message)
: base(message)
{
}
/// <summary>Creates the exception with a custom message and inner exception.</summary>
public ApiKeyPepperUnavailableException(string message, Exception innerException)
: base(message, innerException)
{
}
}
@@ -163,7 +163,7 @@ public sealed class ApiKeyAdminCommandsTests : IAsyncLifetime
int auditCountBefore = (await _audit.ListRecentAsync(50, CancellationToken.None)).Count;
await Assert.ThrowsAsync<InvalidOperationException>(() => commands.CreateKeyAsync(
await Assert.ThrowsAsync<ApiKeyPepperUnavailableException>(() => commands.CreateKeyAsync(
"key-x",
"No Pepper",
new HashSet<string>(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<string>(StringComparer.Ordinal), null, null, CancellationToken.None);
ApiKeyAdminCommands nopepper = BuildCommands(pepper: null);
InvalidOperationException create = await Assert.ThrowsAnyAsync<InvalidOperationException>(() =>
nopepper.CreateKeyAsync("key-bc2", "BC2", new HashSet<string>(StringComparer.Ordinal), null, null, CancellationToken.None));
InvalidOperationException rotate = await Assert.ThrowsAnyAsync<InvalidOperationException>(() =>
nopepper.RotateKeyAsync("key-bc", null, CancellationToken.None));
Assert.IsType<ApiKeyPepperUnavailableException>(create);
Assert.IsType<ApiKeyPepperUnavailableException>(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<ApiKeyPepperUnavailableException>(() => commands.CreateKeyAsync(
"key-w", "W", new HashSet<string>(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<InvalidOperationException>(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<InvalidOperationException>(() =>
await Assert.ThrowsAsync<ApiKeyPepperUnavailableException>(() =>
nopepper.RotateKeyAsync("key-1", null, CancellationToken.None));
// Assert: stored hash is unchanged.