feat(apikeys): optional ExpiresUtc — create + verifier enforcement + sqlite v3 migration (archreview G-2)

ApiKeyRecord/ApiKeyListItem gain a nullable ExpiresUtc (NULL = never expires); ApiKeyFailure gains KeyExpired. ApiKeyVerifier rejects a key whose ExpiresUtc is at-or-before now (inclusive, injected clock), before the secret comparison. CreateKeyAsync gets an expiresUtc overload; rotate preserves existing expiry. SQLite schema bumps to v3: a nullable expires_utc column, added to fresh DBs via CREATE and to existing v1/v2 DBs via an idempotent guarded ALTER — donor v2 gateway-auth.db upgrades in place, no key invalidated. Version 0.1.3 -> 0.1.4 (not yet published; nuget push is human-gated). Consumers (HistorianGateway, mxaccessgw) bump to 0.1.4 to set/enforce expiry. 146 Auth.ApiKeys tests pass.
2026-07-09 06:04:54 -04:00
parent 497c0aac52
commit 378880f0ed
14 changed files with 381 additions and 32 deletions
@@ -87,6 +87,46 @@ public sealed class ApiKeyAdminCommandsTests : IAsyncLifetime
        Assert.Single(recent, e => e.EventType == "create-key");
    }

+    [Fact]
+    public async Task CreateKey_WithExpiresUtc_PersistsExpiryOnRecord()
+    {
+        // archreview G-2: the expiry overload threads ExpiresUtc onto the stored record.
+        ApiKeyAdminCommands commands = BuildCommands();
+        await commands.InitDbAsync(null, CancellationToken.None);
+        var expiry = new DateTimeOffset(2027, 1, 1, 0, 0, 0, TimeSpan.Zero);
+
+        await commands.CreateKeyAsync(
+            "key-exp",
+            "Service Exp",
+            new HashSet<string>(["read"], StringComparer.Ordinal),
+            constraintsJson: null,
+            expiresUtc: expiry,
+            remoteAddress: null,
+            CancellationToken.None);
+
+        ApiKeyRecord? found = await _read.FindByKeyIdAsync("key-exp", CancellationToken.None);
+        Assert.NotNull(found);
+        Assert.Equal(expiry, found!.ExpiresUtc);
+    }
+
+    [Fact]
+    public async Task CreateKey_ExpiryLessOverload_PersistsNullExpiry()
+    {
+        ApiKeyAdminCommands commands = BuildCommands();
+        await commands.InitDbAsync(null, CancellationToken.None);
+
+        await commands.CreateKeyAsync(
+            "key-1",
+            "Service A",
+            new HashSet<string>(["read"], StringComparer.Ordinal),
+            constraintsJson: null,
+            remoteAddress: null,
+            CancellationToken.None);
+
+        ApiKeyRecord? found = await _read.FindByKeyIdAsync("key-1", CancellationToken.None);
+        Assert.Null(found!.ExpiresUtc);
+    }
+
    [Fact]
    public async Task CreateKey_PersistsBareTokenPrefix_NotPrefixUnderscoreKeyId()
    {
@@ -20,7 +20,8 @@ public class ApiKeyVerifierTests

    private static ApiKeyRecord BuildRecord(
        byte[] secretHash,
-        DateTimeOffset? revokedUtc = null) => new(
+        DateTimeOffset? revokedUtc = null,
+        DateTimeOffset? expiresUtc = null) => new(
        KeyId: KeyId,
        KeyPrefix: TokenPrefix,
        SecretHash: secretHash,
@@ -29,7 +30,8 @@ public class ApiKeyVerifierTests
        ConstraintsJson: ConstraintsJson,
        CreatedUtc: DateTimeOffset.UnixEpoch,
        LastUsedUtc: null,
-        RevokedUtc: revokedUtc);
+        RevokedUtc: revokedUtc,
+        ExpiresUtc: expiresUtc);

    private static ApiKeyVerifier BuildVerifier(
        FakeApiKeyStore store,
@@ -126,6 +128,101 @@ public class ApiKeyVerifierTests
        Assert.False(store.MarkUsedCalled);
    }

+    // --- KeyExpired (archreview G-2) ---
+
+    private static readonly DateTimeOffset Now = new(2026, 6, 1, 12, 0, 0, TimeSpan.Zero);
+
+    private static ApiKeyVerifier BuildVerifierAt(
+        FakeApiKeyStore store, DateTimeOffset now) =>
+        new(new ApiKeyOptions { TokenPrefix = TokenPrefix },
+            store,
+            new FakePepperProvider(Pepper),
+            new FakeTimeProvider(now));
+
+    [Fact]
+    public async Task VerifyAsync_ExpiredKey_ReturnsKeyExpired()
+    {
+        byte[] hash = ApiKeySecretHasher.Hash(Secret, Pepper);
+        var store = new FakeApiKeyStore
+        {
+            Record = BuildRecord(hash, expiresUtc: Now - TimeSpan.FromSeconds(1)),
+        };
+        var verifier = BuildVerifierAt(store, Now);
+
+        ApiKeyVerification result =
+            await verifier.VerifyAsync(Header(KeyId, Secret), CancellationToken.None);
+
+        Assert.False(result.Succeeded);
+        Assert.Equal(ApiKeyFailure.KeyExpired, result.Failure);
+        Assert.Null(result.Identity);
+        // Expiry is decided before the secret comparison and usage write.
+        Assert.False(store.MarkUsedCalled);
+    }
+
+    [Fact]
+    public async Task VerifyAsync_ExpiryExactlyNow_IsExpired_Inclusive()
+    {
+        byte[] hash = ApiKeySecretHasher.Hash(Secret, Pepper);
+        var store = new FakeApiKeyStore { Record = BuildRecord(hash, expiresUtc: Now) };
+        var verifier = BuildVerifierAt(store, Now);
+
+        ApiKeyVerification result =
+            await verifier.VerifyAsync(Header(KeyId, Secret), CancellationToken.None);
+
+        Assert.False(result.Succeeded);
+        Assert.Equal(ApiKeyFailure.KeyExpired, result.Failure);
+    }
+
+    [Fact]
+    public async Task VerifyAsync_NotYetExpiredKey_Succeeds()
+    {
+        byte[] hash = ApiKeySecretHasher.Hash(Secret, Pepper);
+        var store = new FakeApiKeyStore
+        {
+            Record = BuildRecord(hash, expiresUtc: Now + TimeSpan.FromSeconds(1)),
+        };
+        var verifier = BuildVerifierAt(store, Now);
+
+        ApiKeyVerification result =
+            await verifier.VerifyAsync(Header(KeyId, Secret), CancellationToken.None);
+
+        Assert.True(result.Succeeded);
+        Assert.Null(result.Failure);
+        Assert.True(store.MarkUsedCalled);
+    }
+
+    [Fact]
+    public async Task VerifyAsync_NullExpiry_NeverExpires()
+    {
+        byte[] hash = ApiKeySecretHasher.Hash(Secret, Pepper);
+        var store = new FakeApiKeyStore { Record = BuildRecord(hash, expiresUtc: null) };
+        // A clock far in the future must still accept a never-expiring key.
+        var verifier = BuildVerifierAt(store, Now + TimeSpan.FromDays(3650));
+
+        ApiKeyVerification result =
+            await verifier.VerifyAsync(Header(KeyId, Secret), CancellationToken.None);
+
+        Assert.True(result.Succeeded);
+    }
+
+    [Fact]
+    public async Task VerifyAsync_RevokedTakesPrecedenceOverExpiry()
+    {
+        // A key that is both revoked and expired reports KeyRevoked (the earlier check).
+        byte[] hash = ApiKeySecretHasher.Hash(Secret, Pepper);
+        var store = new FakeApiKeyStore
+        {
+            Record = BuildRecord(hash, revokedUtc: Now - TimeSpan.FromDays(1), expiresUtc: Now - TimeSpan.FromDays(1)),
+        };
+        var verifier = BuildVerifierAt(store, Now);
+
+        ApiKeyVerification result =
+            await verifier.VerifyAsync(Header(KeyId, Secret), CancellationToken.None);
+
+        Assert.False(result.Succeeded);
+        Assert.Equal(ApiKeyFailure.KeyRevoked, result.Failure);
+    }
+
    // --- SecretMismatch ---

    [Fact]
@@ -41,6 +41,34 @@ public sealed class SqliteApiKeyAdminStoreTests : IAsyncLifetime
        Assert.Null(found.RevokedUtc);
    }

+    [Fact]
+    public async Task Create_WithExpiresUtc_RoundTripsThroughFindAndList()
+    {
+        var expiry = new DateTimeOffset(2026, 12, 31, 23, 59, 59, TimeSpan.Zero);
+        ApiKeyRecord record = SampleRecord("key-exp") with { ExpiresUtc = expiry };
+
+        await _admin.CreateAsync(record, CancellationToken.None);
+
+        ApiKeyRecord? found = await _read.FindByKeyIdAsync("key-exp", CancellationToken.None);
+        Assert.Equal(expiry, found!.ExpiresUtc);
+
+        IReadOnlyList<ApiKeyListItem> listed = await _admin.ListAsync(CancellationToken.None);
+        ApiKeyListItem item = Assert.Single(listed, k => k.KeyId == "key-exp");
+        Assert.Equal(expiry, item.ExpiresUtc);
+    }
+
+    [Fact]
+    public async Task Create_WithoutExpiresUtc_ReadsBackAsNull()
+    {
+        await _admin.CreateAsync(SampleRecord("key-1"), CancellationToken.None);
+
+        ApiKeyRecord? found = await _read.FindByKeyIdAsync("key-1", CancellationToken.None);
+        Assert.Null(found!.ExpiresUtc);
+
+        IReadOnlyList<ApiKeyListItem> listed = await _admin.ListAsync(CancellationToken.None);
+        Assert.Null(Assert.Single(listed, k => k.KeyId == "key-1").ExpiresUtc);
+    }
+
    // --- Revoke ---

    [Fact]
@@ -35,24 +35,93 @@ public sealed class SqliteMigratorTests : IDisposable
    }

    [Fact]
-    public void CurrentVersion_Is2_ToMatchDonorGatewayDeployedSchema() =>
-        // The store was extracted from MxAccessGateway, whose deployed gateway-auth.db is
-        // stamped version 2. The library must stamp 2 (not reset to 1) so it does not refuse
-        // those existing databases on first boot. Locking this invariant.
-        Assert.Equal(2, SqliteAuthSchema.CurrentVersion);
+    public void CurrentVersion_Is3_ForExpiresUtcColumn() =>
+        // v3 adds the nullable expires_utc column (archreview G-2). The value must stay >= 2 so the
+        // migrator does not refuse a donor MxAccessGateway gateway-auth.db (stamped 2) on first boot:
+        // 2 <= 3, so such a DB is accepted and upgraded in place. Locking this invariant.
+        Assert.Equal(3, SqliteAuthSchema.CurrentVersion);

    [Fact]
-    public async Task MigrateAsync_AgainstExistingVersion2Db_DoesNotThrow_AndStaysAt2()
+    public async Task MigrateAsync_AgainstExistingVersion2Db_DoesNotThrow_AndUpgradesTo3()
    {
-        // The deployed-gateway scenario: a database already provisioned at version 2.
+        // The deployed-gateway scenario: a database already provisioned at the donor's version 2.
+        // Migrating must accept it (2 <= 3), add the expires_utc column, and re-stamp to 3 — no key
+        // is invalidated, the table survives.
        var migrator = new SqliteAuthStoreMigrator(Factory);
        await migrator.MigrateAsync(CancellationToken.None);
        await SetVersionAsync(2);

        await migrator.MigrateAsync(CancellationToken.None); // must not throw

-        Assert.Equal(2, await ReadVersionAsync());
+        Assert.Equal(3, await ReadVersionAsync());
        Assert.True(await TableExistsAsync(SqliteAuthSchema.ApiKeysTable));
+        Assert.True(await ColumnExistsAsync(SqliteAuthSchema.ApiKeysTable, SqliteAuthSchema.ExpiresUtcColumn));
+    }
+
+    [Fact]
+    public async Task MigrateAsync_PreV3ApiKeysTableWithoutExpiresUtc_AddsColumn_AndPreservesRows()
+    {
+        // Simulate a genuine pre-expiry (v2) database: an api_keys table WITHOUT expires_utc, holding
+        // an existing key. The forward migration must add the column (existing row → NULL) without
+        // dropping the row.
+        await CreateLegacyV2SchemaWithKeyAsync("legacy-key");
+
+        Assert.False(await ColumnExistsAsync(SqliteAuthSchema.ApiKeysTable, SqliteAuthSchema.ExpiresUtcColumn));
+
+        await new SqliteAuthStoreMigrator(Factory).MigrateAsync(CancellationToken.None);
+
+        Assert.True(await ColumnExistsAsync(SqliteAuthSchema.ApiKeysTable, SqliteAuthSchema.ExpiresUtcColumn));
+        Assert.Equal(3, await ReadVersionAsync());
+
+        // The pre-existing key survives and its new expires_utc is NULL (never-expires).
+        var read = new SqliteApiKeyStore(Factory);
+        var found = await read.FindByKeyIdAsync("legacy-key", CancellationToken.None);
+        Assert.NotNull(found);
+        Assert.Null(found!.ExpiresUtc);
+    }
+
+    private async Task CreateLegacyV2SchemaWithKeyAsync(string keyId)
+    {
+        await using SqliteConnection connection =
+            await Factory.OpenConnectionAsync(CancellationToken.None);
+        await using SqliteCommand command = connection.CreateCommand();
+        command.CommandText = $"""
+            CREATE TABLE api_keys (
+                key_id TEXT PRIMARY KEY,
+                key_prefix TEXT NOT NULL,
+                secret_hash BLOB NOT NULL,
+                display_name TEXT NOT NULL,
+                scopes TEXT NOT NULL,
+                constraints TEXT NULL,
+                created_utc TEXT NOT NULL,
+                last_used_utc TEXT NULL,
+                revoked_utc TEXT NULL
+            );
+            {SqliteAuthSchema.CreateSchemaVersionTable}
+            INSERT INTO schema_version (id, version, applied_utc) VALUES (1, 2, '2026-01-01T00:00:00.0000000+00:00');
+            INSERT INTO api_keys (key_id, key_prefix, secret_hash, display_name, scopes, created_utc)
+            VALUES ($key_id, 'mxgw', X'01020304', 'Legacy', 'read', '2026-01-01T00:00:00.0000000+00:00');
+            """;
+        command.Parameters.AddWithValue("$key_id", keyId);
+        await command.ExecuteNonQueryAsync(CancellationToken.None);
+    }
+
+    private async Task<bool> ColumnExistsAsync(string table, string column)
+    {
+        await using SqliteConnection connection =
+            await Factory.OpenConnectionAsync(CancellationToken.None);
+        await using SqliteCommand command = connection.CreateCommand();
+        command.CommandText = $"PRAGMA table_info({table});";
+        await using SqliteDataReader reader = await command.ExecuteReaderAsync(CancellationToken.None);
+        while (await reader.ReadAsync(CancellationToken.None))
+        {
+            if (string.Equals(reader.GetString(1), column, StringComparison.OrdinalIgnoreCase))
+            {
+                return true;
+            }
+        }
+
+        return false;
    }

    [Fact]