feat(acl): API key minting — AdminUI page + admin service (#520)

Pulled forward from stage 7 because without it the feature is unusable and un-live-gateable: there was no way to create a key short of hand-inserting a row and computing an HMAC by hand. The minting logic is an IApiKeyAdminService in Core.Authorization, NOT logic in the Razor page — this repo has no bUnit, so anything only a page can reach is untestable, and minting credentials is not where that belongs. The page is a thin shell over it. Moved to Core.Authorization so both ends can reach them: ApiKeyOptions (was in Host) and OpcUaPrincipals (was in OpcUaServer.Security). The OPC UA server resolves principals; the AdminUI must refuse to mint a key claiming a reserved one. OpcUaServer already gained a Core reference in stage 1b, so no new edge — AdminUI gains one. ApiKeyOptions is now bound UNCONDITIONALLY rather than in the driver branch: a driver node needs the pepper to VERIFY keys and an admin node needs it to MINT them, and getting that wrong on either side is a silent authentication failure rather than a startup error. Validation rules, all covered: the reserved 'anonymous' principal is refused case-insensitively (so "ANONYMOUS" cannot slip past into a distinct principal that the evaluator would then match case-INsensitively against anonymous's grants); blank name/principal; a principal longer than the column, rejected rather than truncated, because silent truncation yields a key that authenticates and is granted nothing with no visible cause; a past expiry. Generation runs BEFORE the DbContext is opened so an unconfigured pepper surfaces as a clear error instead of a stored row whose hash nothing can verify. The page shows the plaintext ONCE and says so; it is held in component state and written nowhere else, because the server keeps only a hash. Revocation is flagged in the UI as taking effect only after the next deployment — driver-only nodes have no ConfigDb, so keys ride the artifact. FleetAdmin-only (Administrator), not ConfigEditor: this mints credentials. Core.Tests 68 green. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
2026-07-31 13:40:34 -04:00
parent dd939d3c97
commit 087a969ee6
10 changed files with 472 additions and 15 deletions
@@ -0,0 +1,141 @@
+using Microsoft.EntityFrameworkCore;
+using ZB.MOM.WW.OtOpcUa.Configuration;
+using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
+
+namespace ZB.MOM.WW.OtOpcUa.Core.Authorization;
+
+/// <summary>Create / list / revoke OPC UA data-plane API keys (#520).</summary>
+/// <remarks>
+///     A service rather than logic in the Razor page, because this repo has no bUnit — anything only a
+///     page can reach is untestable, and minting credentials is not where that belongs.
+/// </remarks>
+public interface IApiKeyAdminService
+{
+    /// <summary>Lists every key, newest first. Never returns a secret — there is none stored.</summary>
+    /// <param name="ct">The cancellation token.</param>
+    /// <returns>All key rows.</returns>
+    Task<IReadOnlyList<ApiKey>> ListAsync(CancellationToken ct);
+
+    /// <summary>
+    ///     Mints a new key. The returned plaintext is the ONLY time it exists — it is not recoverable
+    ///     from what is stored, so a caller that fails to show it has effectively discarded the key.
+    /// </summary>
+    /// <param name="name">Human label, e.g. the consuming system.</param>
+    /// <param name="principal">The principal the key authenticates as.</param>
+    /// <param name="expiresUtc">Optional expiry.</param>
+    /// <param name="notes">Optional operator notes.</param>
+    /// <param name="ct">The cancellation token.</param>
+    /// <returns>The presented key, shown once.</returns>
+    Task<string> CreateAsync(
+        string name, string principal, DateTime? expiresUtc, string? notes, CancellationToken ct);
+
+    /// <summary>Enables or disables a key.</summary>
+    /// <param name="apiKeyId">The public key id.</param>
+    /// <param name="enabled">Whether the key may authenticate.</param>
+    /// <param name="ct">The cancellation token.</param>
+    /// <returns>A task representing the operation.</returns>
+    Task SetEnabledAsync(string apiKeyId, bool enabled, CancellationToken ct);
+
+    /// <summary>Permanently deletes a key row.</summary>
+    /// <param name="apiKeyId">The public key id.</param>
+    /// <param name="ct">The cancellation token.</param>
+    /// <returns>A task representing the operation.</returns>
+    Task DeleteAsync(string apiKeyId, CancellationToken ct);
+}
+
+/// <summary>EF-backed <see cref="IApiKeyAdminService"/>.</summary>
+public sealed class ApiKeyAdminService : IApiKeyAdminService
+{
+    /// <summary>Longest accepted principal name, matching the column width.</summary>
+    public const int MaxPrincipalLength = 256;
+
+    private readonly IDbContextFactory<OtOpcUaConfigDbContext> _factory;
+    private readonly string _pepper;
+
+    /// <summary>Initializes a new instance of the <see cref="ApiKeyAdminService"/> class.</summary>
+    /// <param name="factory">The config DB context factory.</param>
+    /// <param name="pepper">The server-side pepper used to hash key secrets.</param>
+    public ApiKeyAdminService(IDbContextFactory<OtOpcUaConfigDbContext> factory, string pepper)
+    {
+        _factory = factory;
+        _pepper = pepper;
+    }
+
+    /// <inheritdoc />
+    public async Task<IReadOnlyList<ApiKey>> ListAsync(CancellationToken ct)
+    {
+        await using var db = await _factory.CreateDbContextAsync(ct).ConfigureAwait(false);
+        return await db.ApiKeys.AsNoTracking().OrderByDescending(k => k.CreatedUtc).ToListAsync(ct)
+            .ConfigureAwait(false);
+    }
+
+    /// <inheritdoc />
+    public async Task<string> CreateAsync(
+        string name, string principal, DateTime? expiresUtc, string? notes, CancellationToken ct)
+    {
+        name = (name ?? string.Empty).Trim();
+        principal = (principal ?? string.Empty).Trim();
+
+        if (name.Length == 0) throw new ArgumentException("A key name is required.", nameof(name));
+        if (principal.Length == 0)
+            throw new ArgumentException("A principal is required.", nameof(principal));
+        if (principal.Length > MaxPrincipalLength)
+            throw new ArgumentException(
+                $"Principal must be {MaxPrincipalLength} characters or fewer.", nameof(principal));
+
+        // Reserved: a key authenticating as 'anonymous' would silently inherit whatever anonymous was
+        // granted, and vice versa — two identities meant to be distinct sharing one grant set. The
+        // authenticator refuses it too; this is the authoring half so it can never be stored at all.
+        if (string.Equals(principal, OpcUaPrincipals.Anonymous, StringComparison.OrdinalIgnoreCase))
+            throw new ArgumentException(
+                $"'{OpcUaPrincipals.Anonymous}' is reserved for anonymous sessions and cannot be a key's principal.",
+                nameof(principal));
+
+        if (expiresUtc is { } exp && exp <= DateTime.UtcNow)
+            throw new ArgumentException("Expiry must be in the future.", nameof(expiresUtc));
+
+        // Generate BEFORE opening the context: this throws on an unconfigured pepper, and failing there
+        // is much clearer than a row that persists against a hash nothing can ever verify.
+        var (presented, apiKeyId, hash) = ApiKeySecret.Generate(_pepper);
+
+        await using var db = await _factory.CreateDbContextAsync(ct).ConfigureAwait(false);
+        db.ApiKeys.Add(new ApiKey
+        {
+            ApiKeyId = apiKeyId,
+            Name = name,
+            SecretHash = hash,
+            Principal = principal,
+            Enabled = true,
+            CreatedUtc = DateTime.UtcNow,
+            ExpiresUtc = expiresUtc,
+            Notes = string.IsNullOrWhiteSpace(notes) ? null : notes.Trim(),
+        });
+        await db.SaveChangesAsync(ct).ConfigureAwait(false);
+
+        return presented;
+    }
+
+    /// <inheritdoc />
+    public async Task SetEnabledAsync(string apiKeyId, bool enabled, CancellationToken ct)
+    {
+        await using var db = await _factory.CreateDbContextAsync(ct).ConfigureAwait(false);
+        var key = await db.ApiKeys.SingleOrDefaultAsync(k => k.ApiKeyId == apiKeyId, ct)
+            .ConfigureAwait(false);
+        if (key is null) return;
+
+        key.Enabled = enabled;
+        await db.SaveChangesAsync(ct).ConfigureAwait(false);
+    }
+
+    /// <inheritdoc />
+    public async Task DeleteAsync(string apiKeyId, CancellationToken ct)
+    {
+        await using var db = await _factory.CreateDbContextAsync(ct).ConfigureAwait(false);
+        var key = await db.ApiKeys.SingleOrDefaultAsync(k => k.ApiKeyId == apiKeyId, ct)
+            .ConfigureAwait(false);
+        if (key is null) return;
+
+        db.ApiKeys.Remove(key);
+        await db.SaveChangesAsync(ct).ConfigureAwait(false);
+    }
+}
@@ -1,4 +1,4 @@
-namespace ZB.MOM.WW.OtOpcUa.Host.OpcUa;
+namespace ZB.MOM.WW.OtOpcUa.Core.Authorization;

 /// <summary>
 ///     <c>Security:ApiKeys</c> — OPC UA data-plane API key settings (#520). The data plane is
@@ -0,0 +1,21 @@
+namespace ZB.MOM.WW.OtOpcUa.Core.Authorization;
+
+/// <summary>Reserved principal names for the OPC UA data plane (#520).</summary>
+/// <remarks>
+///     Lives in Core rather than beside the authenticator because BOTH ends need it: the OPC UA server
+///     resolves an anonymous session to it, and the AdminUI authoring path must refuse to mint a key
+///     that claims it.
+/// </remarks>
+public static class OpcUaPrincipals
+{
+    /// <summary>
+    ///     The principal an anonymous session authenticates as. <c>NodeAcl</c> rows naming it grant
+    ///     anonymous clients permissions exactly as they would a key's principal.
+    /// </summary>
+    /// <remarks>
+    ///     Reserved: an API key must not use this principal, or a credentialled client would silently
+    ///     inherit whatever anonymous was granted (and vice versa). Refused at authoring AND at
+    ///     authentication — belt and braces, because the two paths can be reached independently.
+    /// </remarks>
+    public const string Anonymous = "anonymous";
+}
@@ -20,6 +20,7 @@
            <NavRailItem Href="/reservations" Text="Reservations" />
            <NavRailItem Href="/certificates" Text="Certificates" />
            <NavRailItem Href="/role-grants" Text="Role grants" />
+            <NavRailItem Href="/apikeys" Text="API keys" />
            <NavRailItem Href="/admin/secrets" Text="Secrets" />
        </NavRailSection>
        <NavRailSection Title="Scripting" Key="scripting">
@@ -0,0 +1,206 @@
+@page "/apikeys"
+@attribute [Authorize(Policy = AdminUiPolicies.FleetAdmin)]
+@rendermode RenderMode.InteractiveServer
+@using ZB.MOM.WW.OtOpcUa.Configuration.Entities
+@using ZB.MOM.WW.OtOpcUa.Core.Authorization
+@inject IApiKeyAdminService Keys
+
+<div class="d-flex justify-content-between align-items-center mb-3">
+    <h4 class="mb-0">OPC UA API keys</h4>
+</div>
+
+<div class="alert alert-info" role="alert">
+    The OPC UA data plane is machine-to-machine. A client connects with an OPC UA
+    <strong>UserName</strong> token whose username is the literal <span class="mono">apikey</span> and
+    whose password is the key below. The key resolves to a <strong>principal</strong>; ACL rows naming
+    that principal are what grant it permissions.
+    <br />
+    <strong>Revoking a key takes effect on a node only after the next deployment</strong> — driver-only
+    nodes have no config database, so keys travel inside the deployment artifact.
+</div>
+
+@if (_created is not null)
+{
+    <div class="alert alert-success" role="alert">
+        <strong>Copy this key now — it is not stored and cannot be shown again.</strong>
+        <pre class="mono mt-2 mb-0" style="white-space:pre-wrap;word-break:break-all">@_created</pre>
+        <button class="btn btn-sm btn-secondary mt-2" @onclick="() => _created = null">Dismiss</button>
+    </div>
+}
+
+@if (!string.IsNullOrWhiteSpace(_error))
+{
+    <div class="alert alert-danger" role="alert">@_error</div>
+}
+
+<section class="panel mb-3">
+    <div class="panel-head">Create a key</div>
+    <div style="padding:1rem">
+        <div class="row g-2">
+            <div class="col-md-3">
+                <label class="form-label" for="k-name">Name</label>
+                <input id="k-name" class="form-control form-control-sm" @bind="_newName"
+                       placeholder="ScadaBridge DCL" />
+            </div>
+            <div class="col-md-3">
+                <label class="form-label" for="k-principal">Principal</label>
+                <input id="k-principal" class="form-control form-control-sm mono" @bind="_newPrincipal"
+                       placeholder="scadabridge-dcl" />
+            </div>
+            <div class="col-md-3">
+                <label class="form-label" for="k-expires">Expires (optional, UTC)</label>
+                <input id="k-expires" type="date" class="form-control form-control-sm" @bind="_newExpires" />
+            </div>
+            <div class="col-md-3 d-flex align-items-end">
+                <button class="btn btn-sm btn-primary" @onclick="CreateAsync" disabled="@_busy">Create key</button>
+            </div>
+        </div>
+    </div>
+</section>
+
+<section class="panel">
+    <div class="panel-head">Keys</div>
+    @if (_keys is null)
+    {
+        <div style="padding:1rem" class="text-muted">Loading…</div>
+    }
+    else if (_keys.Count == 0)
+    {
+        <div style="padding:1rem" class="text-muted">
+            No keys yet. Until one exists, only anonymous sessions can connect.
+        </div>
+    }
+    else
+    {
+        <table class="table table-sm mb-0">
+            <thead>
+                <tr>
+                    <th>Name</th>
+                    <th>Principal</th>
+                    <th>Key id</th>
+                    <th>Status</th>
+                    <th>Created (UTC)</th>
+                    <th>Expires (UTC)</th>
+                    <th></th>
+                </tr>
+            </thead>
+            <tbody>
+                @foreach (var k in _keys)
+                {
+                    <tr>
+                        <td>@k.Name</td>
+                        <td><span class="mono">@k.Principal</span></td>
+                        <td><span class="mono">@k.ApiKeyId</span></td>
+                        <td>
+                            @if (!k.Enabled)
+                            {
+                                <span class="badge bg-secondary">revoked</span>
+                            }
+                            else if (k.ExpiresUtc is { } e && e <= DateTime.UtcNow)
+                            {
+                                <span class="badge bg-warning text-dark">expired</span>
+                            }
+                            else
+                            {
+                                <span class="badge bg-success">active</span>
+                            }
+                        </td>
+                        <td>@k.CreatedUtc.ToString("yyyy-MM-dd HH:mm")</td>
+                        <td>@(k.ExpiresUtc?.ToString("yyyy-MM-dd") ?? "—")</td>
+                        <td class="text-end">
+                            <button class="btn btn-sm btn-outline-secondary"
+                                    @onclick="() => SetEnabledAsync(k.ApiKeyId, !k.Enabled)" disabled="@_busy">
+                                @(k.Enabled ? "Revoke" : "Re-enable")
+                            </button>
+                            <button class="btn btn-sm btn-outline-danger"
+                                    @onclick="() => DeleteAsync(k.ApiKeyId)" disabled="@_busy">Delete</button>
+                        </td>
+                    </tr>
+                }
+            </tbody>
+        </table>
+    }
+</section>
+
+@code {
+    private IReadOnlyList<ApiKey>? _keys;
+    private string _newName = "";
+    private string _newPrincipal = "";
+    private DateTime? _newExpires;
+    private string? _created;
+    private string? _error;
+    private bool _busy;
+
+    protected override async Task OnInitializedAsync() => await ReloadAsync();
+
+    private async Task ReloadAsync()
+    {
+        try
+        {
+            _keys = await Keys.ListAsync(default);
+        }
+        catch (Exception ex)
+        {
+            _error = ex.Message;
+            _keys = [];
+        }
+    }
+
+    private async Task CreateAsync()
+    {
+        _busy = true;
+        _error = null;
+        _created = null;
+        try
+        {
+            // Shown once, held only in this component's state. There is no way to recover it afterwards
+            // — the server stores a hash — so it is deliberately not written anywhere else.
+            _created = await Keys.CreateAsync(
+                _newName,
+                _newPrincipal,
+                // A date-only input means midnight; treat it as UTC rather than local so an operator
+                // near a timezone boundary does not get a key that expired before it was issued.
+                _newExpires is { } d ? DateTime.SpecifyKind(d, DateTimeKind.Utc) : null,
+                notes: null,
+                default);
+            _newName = "";
+            _newPrincipal = "";
+            _newExpires = null;
+            await ReloadAsync();
+        }
+        catch (Exception ex)
+        {
+            _error = ex.Message;
+        }
+        finally
+        {
+            _busy = false;
+        }
+    }
+
+    private async Task SetEnabledAsync(string apiKeyId, bool enabled)
+    {
+        _busy = true;
+        _error = null;
+        try
+        {
+            await Keys.SetEnabledAsync(apiKeyId, enabled, default);
+            await ReloadAsync();
+        }
+        catch (Exception ex) { _error = ex.Message; }
+        finally { _busy = false; }
+    }
+
+    private async Task DeleteAsync(string apiKeyId)
+    {
+        _busy = true;
+        _error = null;
+        try
+        {
+            await Keys.DeleteAsync(apiKeyId, default);
+            await ReloadAsync();
+        }
+        catch (Exception ex) { _error = ex.Message; }
+        finally { _busy = false; }
+    }
+}
@@ -23,6 +23,9 @@
        <ProjectReference Include="..\ZB.MOM.WW.OtOpcUa.ControlPlane\ZB.MOM.WW.OtOpcUa.ControlPlane.csproj"/>
        <ProjectReference Include="..\ZB.MOM.WW.OtOpcUa.Runtime\ZB.MOM.WW.OtOpcUa.Runtime.csproj"/>
        <ProjectReference Include="..\..\Core\ZB.MOM.WW.OtOpcUa.Configuration\ZB.MOM.WW.OtOpcUa.Configuration.csproj"/>
+        <!-- #520: API-key minting + the reserved-principal rule live in Core.Authorization, because
+             both ends need them — the OPC UA server resolves principals, the AdminUI authors keys. -->
+        <ProjectReference Include="..\..\Core\ZB.MOM.WW.OtOpcUa.Core\ZB.MOM.WW.OtOpcUa.Core.csproj"/>
        <ProjectReference Include="..\..\Core\ZB.MOM.WW.OtOpcUa.Core.Scripting\ZB.MOM.WW.OtOpcUa.Core.Scripting.csproj"/>
        <ProjectReference Include="..\..\Core\ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions\ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions.csproj"/>
    </ItemGroup>
@@ -2,6 +2,7 @@ using Akka.Actor;
 using Akka.Hosting;
 using Microsoft.AspNetCore.Server.Kestrel.Core;
 using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.Options;
 using Serilog;
 using ZB.MOM.WW.LocalDb.Replication;
@@ -20,6 +21,7 @@ using ZB.MOM.WW.OtOpcUa.ControlPlane;
 using ZB.MOM.WW.OtOpcUa.ControlPlane.Redundancy;
 using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
 using ZB.MOM.WW.OtOpcUa.Commons.Engines;
+using ZB.MOM.WW.OtOpcUa.Core.Authorization;
 using ZB.MOM.WW.OtOpcUa.Core.Scripting;
 using ZB.MOM.WW.OtOpcUa.Host;
 using ZB.MOM.WW.OtOpcUa.Host.Configuration;
@@ -132,6 +134,11 @@ builder.AddZbSerilog(o => o.ServiceName = "otopcua");
 // central over the Phase-3 fetch-and-cache path (ConfigSource:Mode=FetchAndCache). Registering it
 // unconditionally would throw on a driver-only node, since AddOtOpcUaConfigDb requires the string.
 // A fused admin+driver node keeps ConfigDb via its admin role — byte-for-byte unaffected.
+// #520 — bound unconditionally: a driver node needs the pepper to VERIFY keys, an admin node needs it
+// to MINT them, and getting this wrong on either side is a silent authentication failure rather than a
+// startup error.
+builder.Services.Configure<ApiKeyOptions>(builder.Configuration.GetSection(ApiKeyOptions.SectionName));
+
 if (hasAdmin)
    builder.Services.AddOtOpcUaConfigDb(builder.Configuration);
 else
@@ -344,7 +351,6 @@ if (hasDriver)
    // (static username "apikey", the otua_ credential as the password), not an LDAP user. The LDAP
    // registrations above are deliberately retained: they back ADMINUI LOGIN, which keeps LDAP.
    // Only IOpcUaUserAuthenticator moves.
-    builder.Services.Configure<ApiKeyOptions>(builder.Configuration.GetSection(ApiKeyOptions.SectionName));
    if (hasAdmin)
    {
        // ConfigDbApiKeyStore needs a ConfigDb, and Phase 4 gated AddOtOpcUaConfigDb on hasAdmin.
@@ -353,6 +359,11 @@ if (hasDriver)
        // Safe direction, and anonymous sessions are unaffected.
        builder.Services.AddSingleton<IApiKeyStore, ConfigDbApiKeyStore>();
        builder.Services.AddSingleton<IOpcUaUserAuthenticator, ApiKeyOpcUaUserAuthenticator>();
+
+        // Minting surface for the AdminUI. Same node condition as the store above: both need a ConfigDb.
+        builder.Services.AddSingleton<IApiKeyAdminService>(sp => new ApiKeyAdminService(
+            sp.GetRequiredService<IDbContextFactory<OtOpcUaConfigDbContext>>(),
+            sp.GetRequiredService<IOptions<ApiKeyOptions>>().Value.Pepper));
    }
    else
    {
@@ -3,6 +3,7 @@ using Microsoft.Extensions.Logging;
 using Opc.Ua;
 using Opc.Ua.Configuration;
 using Opc.Ua.Server;
+using ZB.MOM.WW.OtOpcUa.Core.Authorization;
 using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;

 namespace ZB.MOM.WW.OtOpcUa.OpcUaServer;
@@ -1,3 +1,5 @@
+using ZB.MOM.WW.OtOpcUa.Core.Authorization;
+
 namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;

 /// <summary>
@@ -47,19 +49,6 @@ public interface IOpcUaUserAuthenticator
        Task.FromResult(OpcUaUserAuthResult.Allow(OpcUaPrincipals.Anonymous, Array.Empty<string>()));
 }

-/// <summary>Reserved principal names for the OPC UA data plane (#520).</summary>
-public static class OpcUaPrincipals
-{
-    /// <summary>
-    ///     The principal an anonymous session authenticates as. <c>NodeAcl</c> rows naming it grant
-    ///     anonymous clients permissions exactly as they would a key's principal.
-    /// </summary>
-    /// <remarks>
-    ///     Reserved: an API key must not be created with this principal, or a key would silently inherit
-    ///     whatever anonymous was granted (and vice versa). Enforced at authoring time.
-    /// </remarks>
-    public const string Anonymous = "anonymous";
-}

 /// <summary>Outcome of a UserName authentication attempt. <see cref="Roles"/> populates the session identity's role set.</summary>
 public sealed record OpcUaUserAuthResult(
@@ -0,0 +1,84 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Authorization;
+
+namespace ZB.MOM.WW.OtOpcUa.Core.Tests.Authorization;
+
+/// <summary>
+///     Validation rules for minting API keys (#520). These run BEFORE any database work, so they are
+///     covered without a ConfigDb: a null context factory proves the guard rejected the input rather
+///     than reaching persistence (it would NullReferenceException if it got that far).
+/// </summary>
+public sealed class ApiKeyAdminServiceTests
+{
+    private static ApiKeyAdminService Sut(string pepper = "unit-test-pepper") => new(null!, pepper);
+
+    [Fact]
+    public async Task The_reserved_anonymous_principal_cannot_be_minted_as_a_key()
+    {
+        // Otherwise a credentialled client silently inherits whatever anonymous was granted. Rejected
+        // here AND at authentication, because the two paths are reachable independently.
+        var ex = await Should.ThrowAsync<ArgumentException>(() =>
+            Sut().CreateAsync("sneaky", OpcUaPrincipals.Anonymous, null, null, default));
+
+        ex.Message.ShouldContain("reserved");
+    }
+
+    [Fact]
+    public async Task The_reserved_principal_check_is_case_insensitive()
+    {
+        // "Anonymous" must not slip past a case-sensitive comparison and become a distinct principal
+        // that the evaluator would then match case-INsensitively against anonymous's grants.
+        await Should.ThrowAsync<ArgumentException>(() =>
+            Sut().CreateAsync("sneaky", "ANONYMOUS", null, null, default));
+    }
+
+    [Theory]
+    [InlineData("", "principal")]
+    [InlineData("   ", "principal")]
+    public async Task A_blank_principal_is_rejected(string principal, string expectedParam)
+    {
+        var ex = await Should.ThrowAsync<ArgumentException>(() =>
+            Sut().CreateAsync("name", principal, null, null, default));
+
+        ex.ParamName.ShouldBe(expectedParam);
+    }
+
+    [Theory]
+    [InlineData("")]
+    [InlineData("   ")]
+    public async Task A_blank_name_is_rejected(string name)
+    {
+        var ex = await Should.ThrowAsync<ArgumentException>(() =>
+            Sut().CreateAsync(name, "some-principal", null, null, default));
+
+        ex.ParamName.ShouldBe("name");
+    }
+
+    [Fact]
+    public async Task A_principal_longer_than_the_column_is_rejected_rather_than_truncated()
+    {
+        // Silent truncation would create a key whose principal does not match the ACL row the operator
+        // authored, i.e. a key that authenticates and is granted nothing, with no visible cause.
+        var tooLong = new string('p', ApiKeyAdminService.MaxPrincipalLength + 1);
+
+        await Should.ThrowAsync<ArgumentException>(() =>
+            Sut().CreateAsync("name", tooLong, null, null, default));
+    }
+
+    [Fact]
+    public async Task An_expiry_in_the_past_is_rejected()
+    {
+        await Should.ThrowAsync<ArgumentException>(() =>
+            Sut().CreateAsync("name", "p", DateTime.UtcNow.AddMinutes(-1), null, default));
+    }
+
+    [Fact]
+    public async Task An_unconfigured_pepper_fails_before_anything_is_persisted()
+    {
+        // Generate() runs before the DbContext is opened precisely so this surfaces as a clear pepper
+        // error rather than a stored row whose hash nothing could ever verify.
+        await Should.ThrowAsync<ArgumentException>(() =>
+            Sut(pepper: "").CreateAsync("name", "p", null, null, default));
+    }
+}