feat(auth.apikeys.sqlserver): zbauth schema v1 + concurrent-boot-safe migrator (retry on 1205/2714/1913/2627, newer-version fail-closed); live tests gated on AUTH_SQLSERVER_CONNSTR
Claude-Session: https://claude.ai/code/session_013L3XdmVii9i5sY4FChU8pB
This commit is contained in:
Joseph Doherty
@@ -28,9 +28,38 @@ public sealed class SqlServerApiKeyOptions
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(ConnectionString))
|
||||
throw new InvalidOperationException("SQL Server API-key store requires a ConnectionString.");
|
||||
if (string.IsNullOrEmpty(Schema) || !Schema.All(c => char.IsAsciiLetterOrDigit(c) || c == '_') || Schema.Length > 64)
|
||||
throw new InvalidOperationException($"SQL Server API-key store schema name '{Schema}' is not a safe identifier.");
|
||||
|
||||
ValidateIdentifier(Schema);
|
||||
|
||||
if (CommandTimeout <= TimeSpan.Zero)
|
||||
throw new InvalidOperationException($"SQL Server API-key store CommandTimeout must be positive (was {CommandTimeout}).");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Strict allow-list for a SQL identifier that will be interpolated into DDL or query text.
|
||||
/// Rejects everything that is not <c>[A-Za-z_][A-Za-z0-9_]{0,63}</c> — no brackets, quotes, dots
|
||||
/// or spaces — so no bracket-escape (<c>]]</c>) trick can break out of the quoting applied at the
|
||||
/// use site. Uses the ASCII-only character tests deliberately, so homoglyph and full-width
|
||||
/// look-alikes are rejected rather than accepted as "letters".
|
||||
/// </summary>
|
||||
/// <param name="schemaName">The candidate identifier.</param>
|
||||
/// <returns>The same value, once validated.</returns>
|
||||
/// <exception cref="InvalidOperationException">The value is not a plain identifier.</exception>
|
||||
/// <remarks>
|
||||
/// Public because <see cref="SqlServerAuthSchema.CreateSchemaDdl(string)"/> is public and must
|
||||
/// defend itself rather than trust that every caller routed through <see cref="Validate"/>.
|
||||
/// </remarks>
|
||||
public static string ValidateIdentifier(string schemaName)
|
||||
{
|
||||
if (string.IsNullOrEmpty(schemaName)
|
||||
|| schemaName.Length > 64
|
||||
|| (!char.IsAsciiLetter(schemaName[0]) && schemaName[0] != '_')
|
||||
|| !schemaName.All(c => char.IsAsciiLetterOrDigit(c) || c == '_'))
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
$"SQL Server API-key store schema name '{schemaName}' is not a safe identifier.");
|
||||
}
|
||||
|
||||
return schemaName;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,207 @@
|
||||
using System.Data;
|
||||
using System.Globalization;
|
||||
using Microsoft.Data.SqlClient;
|
||||
using ZB.MOM.WW.Auth.Abstractions.ApiKeys;
|
||||
using ZB.MOM.WW.Auth.ApiKeys.Sqlite;
|
||||
|
||||
namespace ZB.MOM.WW.Auth.ApiKeys.SqlServer;
|
||||
|
||||
/// <summary>
|
||||
/// Creates the shared SQL Server API-key schema and records the applied version. Idempotent: safe
|
||||
/// to run on every node at every startup, including concurrently — lost provisioning races surface
|
||||
/// as retryable <see cref="SqlException"/>s (see <see cref="IsRetryableMigrationErrorNumber"/>) and
|
||||
/// the retry re-runs the <c>IF NOT EXISTS</c> path against the objects the winner just created.
|
||||
/// Refuses to run against a database whose recorded version is newer than this build supports.
|
||||
/// </summary>
|
||||
/// <param name="connectionFactory">Factory for connections to the shared API-key database.</param>
|
||||
public sealed class SqlServerApiKeyStoreMigrator(AuthSqlServerConnectionFactory connectionFactory)
|
||||
: IApiKeyStoreMigrator
|
||||
{
|
||||
// Every node runs this at every startup, so a virgin database being provisioned by N nodes at
|
||||
// once is the NORMAL case, not a rare race — and an unretried failure aborts that node's whole
|
||||
// host at startup. Two failure shapes come out of that race, and retrying is correct for both
|
||||
// precisely because the migration is idempotent:
|
||||
//
|
||||
// - Deadlock victim (1205): concurrent Serializable DDL against the system catalogs can pick
|
||||
// a victim even though the IF NOT EXISTS guards make the logic itself safe.
|
||||
// - Lost CREATE / lost INSERT (2714, 1913, 2627): Serializable range locks do NOT serialize
|
||||
// the metadata existence checks against another session's in-flight CREATE — observed live
|
||||
// on the sibling shared secret store (ScadaBridge central-pair first boot, 2026-08-07):
|
||||
// both nodes passed the sys.schemas IF NOT EXISTS probe, the loser's CREATE SCHEMA then
|
||||
// died with 2714 "There is already an object named 'zbsecrets'". The object the loser tried
|
||||
// to create now exists, which is the desired end state — the retry's IF NOT EXISTS pass
|
||||
// skips it and proceeds.
|
||||
//
|
||||
// The retry cap keeps this from masking a GENUINE conflict (e.g. an unrelated pre-existing
|
||||
// object colliding with the schema name): that reproduces on every attempt, so after
|
||||
// MaxMigrationRetries the real SqlException propagates and the node fails closed.
|
||||
private const int MaxMigrationRetries = 5;
|
||||
|
||||
/// <summary>Deadlock victim.</summary>
|
||||
private const int SqlDeadlockVictimErrorNumber = 1205;
|
||||
|
||||
/// <summary>"There is already an object named '%ls'" — lost CREATE SCHEMA / CREATE TABLE race.</summary>
|
||||
private const int SqlObjectAlreadyExistsErrorNumber = 2714;
|
||||
|
||||
/// <summary>"An index or statistics with name '%ls' already exists" — lost CREATE INDEX race.</summary>
|
||||
private const int SqlIndexAlreadyExistsErrorNumber = 1913;
|
||||
|
||||
/// <summary>Primary-key violation — lost version-row MERGE-insert race.</summary>
|
||||
private const int SqlPrimaryKeyViolationErrorNumber = 2627;
|
||||
|
||||
/// <inheritdoc />
|
||||
/// <exception cref="AuthStoreMigrationException">
|
||||
/// The recorded schema version is newer than <see cref="SqlServerAuthSchema.CurrentVersion"/>.
|
||||
/// </exception>
|
||||
public async Task MigrateAsync(CancellationToken cancellationToken)
|
||||
{
|
||||
for (int attempt = 1; ; attempt++)
|
||||
{
|
||||
try
|
||||
{
|
||||
await MigrateOnceAsync(cancellationToken).ConfigureAwait(false);
|
||||
return;
|
||||
}
|
||||
catch (SqlException ex)
|
||||
when (IsRetryableMigrationError(ex) && attempt < MaxMigrationRetries)
|
||||
{
|
||||
await Task.Delay(BackoffFor(attempt), cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Back-off before retry attempt <paramref name="attempt"/> + 1: a growing base plus RANDOM
|
||||
/// jitter. The jitter is the load-bearing half. The sibling secret-store migrator this is ported
|
||||
/// from delays a flat <c>100 * attempt</c>, which is the SAME value for every victim of the same
|
||||
/// round — so N deadlock victims wake together and re-collide, exactly the lockstep its comment
|
||||
/// claims to prevent. Measured here against a live SQL Server with eight concurrent migrators on
|
||||
/// a virgin schema, the unjittered form exhausted the five-attempt budget on repeated 1205s in
|
||||
/// roughly 1 run in 60; with jitter it did not.
|
||||
/// </summary>
|
||||
/// <param name="attempt">The 1-based attempt that just failed.</param>
|
||||
/// <returns>How long to wait before the next attempt.</returns>
|
||||
private static TimeSpan BackoffFor(int attempt) =>
|
||||
TimeSpan.FromMilliseconds((100 * attempt) + Random.Shared.Next(0, 100 * attempt));
|
||||
|
||||
/// <summary>
|
||||
/// Whether any error carried by <paramref name="exception"/> is a retryable provisioning-race
|
||||
/// outcome. Scans the whole <see cref="SqlException.Errors"/> collection rather than only
|
||||
/// <see cref="SqlException.Number"/> because a failed DDL batch stacks follow-on errors (e.g.
|
||||
/// 2714 arrives with 2759 "CREATE SCHEMA failed due to previous errors") and the retryable one
|
||||
/// is not guaranteed to be first.
|
||||
/// </summary>
|
||||
/// <param name="exception">The exception the migration attempt failed with.</param>
|
||||
/// <returns><see langword="true"/> when a retry may succeed.</returns>
|
||||
internal static bool IsRetryableMigrationError(SqlException exception)
|
||||
{
|
||||
foreach (SqlError error in exception.Errors)
|
||||
{
|
||||
if (IsRetryableMigrationErrorNumber(error.Number))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return IsRetryableMigrationErrorNumber(exception.Number);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Classifies a SQL Server error number as a retryable outcome of N nodes provisioning the same
|
||||
/// (possibly virgin) database at once. Everything here is a race whose retry re-runs the
|
||||
/// idempotent <c>IF NOT EXISTS</c> path; anything else (login failures, missing database,
|
||||
/// permissions, syntax) reproduces identically on retry and must propagate. 2705 (duplicate
|
||||
/// column) is deliberately absent: every column lives inside a single <c>CREATE TABLE</c>
|
||||
/// statement, so no cross-session race can produce it.
|
||||
/// </summary>
|
||||
/// <param name="errorNumber">The <see cref="SqlError.Number"/> to classify.</param>
|
||||
/// <returns><see langword="true"/> when the number is a known provisioning-race outcome.</returns>
|
||||
internal static bool IsRetryableMigrationErrorNumber(int errorNumber) => errorNumber
|
||||
is SqlDeadlockVictimErrorNumber
|
||||
or SqlObjectAlreadyExistsErrorNumber
|
||||
or SqlIndexAlreadyExistsErrorNumber
|
||||
or SqlPrimaryKeyViolationErrorNumber;
|
||||
|
||||
private async Task MigrateOnceAsync(CancellationToken cancellationToken)
|
||||
{
|
||||
string schema = connectionFactory.Schema;
|
||||
|
||||
await using SqlConnection connection =
|
||||
await connectionFactory.OpenConnectionAsync(cancellationToken).ConfigureAwait(false);
|
||||
|
||||
// Serializable: unlike the single-writer SQLite store, EVERY node in the cluster runs this
|
||||
// migration at startup, so concurrent first-boots are the normal case rather than a rare
|
||||
// race. The isolation level narrows the race window but does NOT close it — the IF NOT
|
||||
// EXISTS metadata probes are not serialized against another session's in-flight CREATE, so
|
||||
// a loser can still fail with "object exists" (2714/1913) or a deadlock (1205). The retry
|
||||
// filter in MigrateAsync is what makes simultaneous provision converge: by the time the
|
||||
// loser retries, the winner's objects exist and every guard here skips them.
|
||||
await using SqlTransaction transaction = (SqlTransaction)
|
||||
await connection.BeginTransactionAsync(IsolationLevel.Serializable, cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
|
||||
int existingVersion =
|
||||
await ReadExistingSchemaVersionAsync(connection, transaction, schema, cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
|
||||
if (existingVersion > SqlServerAuthSchema.CurrentVersion)
|
||||
{
|
||||
throw new AuthStoreMigrationException(
|
||||
$"Shared API-key database schema version {existingVersion} is newer than supported version " +
|
||||
$"{SqlServerAuthSchema.CurrentVersion}. Upgrade this node before it writes to the shared store.");
|
||||
}
|
||||
|
||||
await using (SqlCommand ddl = connectionFactory.CreateCommand(
|
||||
connection, SqlServerAuthSchema.CreateSchemaDdl(schema), transaction))
|
||||
{
|
||||
await ddl.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
// MERGE rather than insert-if-missing: WHEN MATCHED re-stamps the row, so a future schema
|
||||
// version actually lands in the table instead of leaving the first-ever value behind.
|
||||
await using (SqlCommand version = connectionFactory.CreateCommand(
|
||||
connection,
|
||||
$"""
|
||||
MERGE [{schema}].[{SqlServerAuthSchema.SchemaVersionTable}] AS target
|
||||
USING (SELECT 1 AS id, @version AS version, @applied_utc AS applied_utc) AS source
|
||||
ON target.id = source.id
|
||||
WHEN MATCHED THEN
|
||||
UPDATE SET version = source.version, applied_utc = source.applied_utc
|
||||
WHEN NOT MATCHED THEN
|
||||
INSERT (id, version, applied_utc) VALUES (1, source.version, source.applied_utc);
|
||||
""",
|
||||
transaction))
|
||||
{
|
||||
version.Parameters.Add("@version", SqlDbType.Int).Value = SqlServerAuthSchema.CurrentVersion;
|
||||
version.Parameters.Add("@applied_utc", SqlDbType.DateTimeOffset).Value = DateTimeOffset.UtcNow;
|
||||
await version.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
await transaction.CommitAsync(cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
private async Task<int> ReadExistingSchemaVersionAsync(
|
||||
SqlConnection connection,
|
||||
SqlTransaction transaction,
|
||||
string schema,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
// OBJECT_ID returns NULL for an absent table, so this reads 0 on a virgin database without
|
||||
// needing the table to exist first.
|
||||
await using SqlCommand command = connectionFactory.CreateCommand(
|
||||
connection,
|
||||
$"""
|
||||
IF OBJECT_ID(N'[{schema}].[{SqlServerAuthSchema.SchemaVersionTable}]', N'U') IS NULL
|
||||
SELECT 0;
|
||||
ELSE
|
||||
SELECT ISNULL((SELECT version FROM [{schema}].[{SqlServerAuthSchema.SchemaVersionTable}] WHERE id = 1), 0);
|
||||
""",
|
||||
transaction);
|
||||
|
||||
object? version = await command.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false);
|
||||
|
||||
return version is null || version == DBNull.Value
|
||||
? 0
|
||||
: Convert.ToInt32(version, CultureInfo.InvariantCulture);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,116 @@
|
||||
namespace ZB.MOM.WW.Auth.ApiKeys.SqlServer;
|
||||
|
||||
/// <summary>
|
||||
/// Schema constants and DDL for the shared SQL Server API-key store. Mirrors
|
||||
/// <c>ZB.MOM.WW.Auth.ApiKeys.Sqlite.SqliteAuthSchema</c> column-for-column so a row means exactly
|
||||
/// the same thing in either store.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>Timestamps are <c>DATETIMEOFFSET(7)</c>, not ISO-8601 text.</b> The sibling
|
||||
/// <c>ZB.MOM.WW.Secrets</c> SQL Server store deliberately stores its timestamps as
|
||||
/// <c>nvarchar</c> because its rows <em>replicate</em> with a SQLite store and last-writer-wins
|
||||
/// compares those timestamps for equality, where a sub-tick round-trip difference would turn a tie
|
||||
/// into a spurious "newer". No such rationale applies here: this store never replicates with the
|
||||
/// SQLite one — a host picks exactly one provider, and the one-shot SQLite import is a copy, not an
|
||||
/// ongoing sync — so the native type is used, which sorts and range-scans correctly and keeps the
|
||||
/// tables readable to a DBA.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <c>SqliteAuthSchema.CurrentVersion</c> is 3 only because that store carries the donor
|
||||
/// MxAccessGateway's version history (v1/v2 were byte-identical; v3 added <c>expires_utc</c>). This
|
||||
/// schema is created whole, with <c>expires_utc</c> present from the start, so its own version line
|
||||
/// starts at 1.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public static class SqlServerAuthSchema
|
||||
{
|
||||
/// <summary>
|
||||
/// The schema version this build creates and supports. The migrator refuses a database whose
|
||||
/// recorded version is <em>newer</em> than this.
|
||||
/// </summary>
|
||||
public const int CurrentVersion = 1;
|
||||
|
||||
/// <summary>Default schema the API-key tables live in.</summary>
|
||||
public const string DefaultSchemaName = "zbauth";
|
||||
|
||||
/// <summary>Name of the single-row table tracking the applied schema version.</summary>
|
||||
public const string SchemaVersionTable = "schema_version";
|
||||
|
||||
/// <summary>Name of the table storing API-key records.</summary>
|
||||
public const string ApiKeysTable = "api_keys";
|
||||
|
||||
/// <summary>Name of the append-only audit table.</summary>
|
||||
public const string ApiKeyAuditTable = "api_key_audit";
|
||||
|
||||
/// <summary>
|
||||
/// Returns the idempotent DDL that provisions the schema, the three tables and the two
|
||||
/// supporting indexes in <paramref name="schemaName"/>.
|
||||
/// </summary>
|
||||
/// <param name="schemaName">The SQL schema name to create the objects in.</param>
|
||||
/// <returns>The DDL batch.</returns>
|
||||
/// <remarks>
|
||||
/// <paramref name="schemaName"/> is interpolated, not parameterized — T-SQL has no parameter
|
||||
/// form for an object name. It is re-validated here against the same strict identifier
|
||||
/// allow-list <see cref="SqlServerApiKeyOptions.Validate"/> enforces, and bracket-quoted below.
|
||||
/// The duplicate check is deliberate: this method is <c>public</c>, so it must defend itself
|
||||
/// rather than trust that every caller routed through the options type. The version row is NOT
|
||||
/// written here — <see cref="SqlServerApiKeyStoreMigrator"/> writes it with a MERGE that also
|
||||
/// updates on match, so a future upgrade re-stamps the stored version.
|
||||
/// </remarks>
|
||||
/// <exception cref="InvalidOperationException"><paramref name="schemaName"/> is not a plain identifier.</exception>
|
||||
public static string CreateSchemaDdl(string schemaName)
|
||||
{
|
||||
SqlServerApiKeyOptions.ValidateIdentifier(schemaName);
|
||||
|
||||
return $"""
|
||||
IF NOT EXISTS (SELECT 1 FROM sys.schemas WHERE name = N'{schemaName}')
|
||||
EXEC(N'CREATE SCHEMA [{schemaName}]');
|
||||
|
||||
IF OBJECT_ID(N'[{schemaName}].[{SchemaVersionTable}]', N'U') IS NULL
|
||||
CREATE TABLE [{schemaName}].[{SchemaVersionTable}] (
|
||||
id INT NOT NULL CONSTRAINT PK_{schemaName}_{SchemaVersionTable} PRIMARY KEY CHECK (id = 1),
|
||||
version INT NOT NULL,
|
||||
applied_utc DATETIMEOFFSET(7) NOT NULL
|
||||
);
|
||||
|
||||
IF OBJECT_ID(N'[{schemaName}].[{ApiKeysTable}]', N'U') IS NULL
|
||||
CREATE TABLE [{schemaName}].[{ApiKeysTable}] (
|
||||
key_id NVARCHAR(64) NOT NULL CONSTRAINT PK_{schemaName}_{ApiKeysTable} PRIMARY KEY,
|
||||
key_prefix NVARCHAR(16) NOT NULL,
|
||||
secret_hash VARBINARY(64) NOT NULL,
|
||||
display_name NVARCHAR(256) NOT NULL,
|
||||
scopes NVARCHAR(MAX) NOT NULL,
|
||||
constraints NVARCHAR(MAX) NULL,
|
||||
created_utc DATETIMEOFFSET(7) NOT NULL,
|
||||
last_used_utc DATETIMEOFFSET(7) NULL,
|
||||
revoked_utc DATETIMEOFFSET(7) NULL,
|
||||
expires_utc DATETIMEOFFSET(7) NULL
|
||||
);
|
||||
|
||||
IF OBJECT_ID(N'[{schemaName}].[{ApiKeyAuditTable}]', N'U') IS NULL
|
||||
CREATE TABLE [{schemaName}].[{ApiKeyAuditTable}] (
|
||||
audit_id BIGINT IDENTITY(1,1) NOT NULL CONSTRAINT PK_{schemaName}_{ApiKeyAuditTable} PRIMARY KEY,
|
||||
key_id NVARCHAR(64) NULL,
|
||||
event_type NVARCHAR(64) NOT NULL,
|
||||
remote_address NVARCHAR(64) NULL,
|
||||
created_utc DATETIMEOFFSET(7) NOT NULL,
|
||||
details NVARCHAR(MAX) NULL
|
||||
);
|
||||
|
||||
IF NOT EXISTS (
|
||||
SELECT 1 FROM sys.indexes
|
||||
WHERE name = N'ix_api_keys_revoked_utc'
|
||||
AND object_id = OBJECT_ID(N'[{schemaName}].[{ApiKeysTable}]'))
|
||||
CREATE INDEX ix_api_keys_revoked_utc
|
||||
ON [{schemaName}].[{ApiKeysTable}] (revoked_utc);
|
||||
|
||||
IF NOT EXISTS (
|
||||
SELECT 1 FROM sys.indexes
|
||||
WHERE name = N'ix_api_key_audit_key_id_created_utc'
|
||||
AND object_id = OBJECT_ID(N'[{schemaName}].[{ApiKeyAuditTable}]'))
|
||||
CREATE INDEX ix_api_key_audit_key_id_created_utc
|
||||
ON [{schemaName}].[{ApiKeyAuditTable}] (key_id, created_utc);
|
||||
""";
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user