diff --git a/AGENTS.md b/AGENTS.md index 4065728..c60b036 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,9 +50,9 @@ ASP.NET Core Data Protection (IXmlRepository) - **Core:** `PersistKeysToStorage()` registers a singleton `IConfigureOptions` that sets `XmlRepository` to a `StorageWrapper`, or to a `DeletableStorageWrapper` when `TStorage` is assignable to `IDeletableDataProtectionStorage` (so `XmlKeyManager.CanDeleteKeys` reflects the storage). It does **not** register `TStorage`; the caller must. - `DeletableStorageWrapper.DeleteElements` offers the parsed keys (malformed rows skipped), then passes the chosen `GetAll()` instances, ordered by `DeletionOrder`, to a single `Delete()` call. A storage exception is logged (`ErrorDeletingKeysFromStorage`) and returns `false`. Note: in ASP.NET Core 10.0, `XmlKeyManager` exposes `DeleteKeys` but does not implement `IDeletableKeyManager`. -- `StorageWrapper` (internal) creates a new DI scope and resolves `TStorage` on every `GetAllElements` / `StoreElement` call, so scoped storage implementations work. It trims `FriendlyName`, normalizes empty or whitespace-only names to `null`, and wraps any `Insert` exception in `KeyInsertException`. `GetAllElements` logs and skips rows with malformed XML (`SkippingMalformedKey`) instead of failing the whole key ring. +- `StorageWrapper` (internal) creates a new DI scope and resolves `TStorage` on every `GetAllElements` / `StoreElement` call, so scoped storage implementations work. It trims `FriendlyName`, normalizes empty or whitespace-only names to `null`, and wraps any `Insert` exception in `KeyInsertException`. `GetAllElements` logs and skips rows with malformed or `null` XML (`SkippingMalformedKey`) instead of failing the whole key ring. - Never log or expose key XML (it may contain unencrypted key material); this includes `XmlException` messages, which can quote the input. -- **DB providers:** `PersistKeysWithDapperInSqlServer(...)` / `PersistKeysWithDapperInPostgreSQL(...)` configure `IOptions` with an internal per-provider `DapperDataProtectionConfigValidator` and `ValidateOnStart()`, register the connection source as a singleton (`SqlConnectionFactory`; `NpgsqlDataSource` as a keyed singleton under the internal `Constants.DataSourceServiceKey`, created by a factory so the container disposes it, and so it never collides with the application's own data source), register the repository as scoped `IDbDataProtectionStorage`, then call `PersistKeysToStorage()`. +- **DB providers:** `PersistKeysWithDapperInSqlServer(...)` / `PersistKeysWithDapperInPostgreSQL(...)` configure `IOptions` with an internal per-provider `DapperDataProtectionConfigValidator` and `ValidateOnStart()`, register the connection source as a keyed singleton, so it never collides with one registered by the application (`SqlConnectionFactory` under the internal `Constants.ConnectionFactoryServiceKey`; `NpgsqlDataSource` under the internal `Constants.DataSourceServiceKey`, created by a factory so the container disposes it), register the repository as scoped `IDbDataProtectionStorage`, then call `PersistKeysToStorage()`. - `IServiceProvider.UseDapperDataProtection()` (in the shared Dapper project) must be called at startup. It resolves the registrations and runs `InitializeDb()` when `DapperDataProtectionConfig.InitializeTable` is `true` (the default). Reading the options there also triggers validation when no host runs `ValidateOnStart`. ### Database Implementations @@ -67,9 +67,9 @@ Both repositories use raw SQL via Dapper. They differ in naming conventions: - The PostgreSQL `SELECT` aliases each snake_case column to its `DataProtectionKeyEntity` property name (`AS "{nameof(...)}"`). Never set the process-wide `Dapper.DefaultTypeMap.MatchNamesWithUnderscores`: it changes column mapping for every Dapper query in the host application. - `Delete()` requires `DataProtectionKeyEntity` instances (as returned by `GetAll()`) and deletes by `Id`, row by row in order, in one transaction. -- Schema and table names are validated at startup (SQL Server: `^[A-Za-z_][A-Za-z0-9_]*`, schema <= 128, table <= 112 to fit `IX_{table}_FriendlyName`; PostgreSQL: lowercase only, <= 63) and always quoted (`[...]` / `"..."`, with the closing quote escaped). PostgreSQL names are lowercase because earlier versions did not quote them: a mixed-case name would silently target a new, empty table. +- Schema and table names are validated at startup (SQL Server: `^[A-Za-z_][A-Za-z0-9_]*`, schema <= 128, table <= 112 to fit `IX_{table}_FriendlyName`; PostgreSQL: lowercase only, schema + table <= 45 to fit `ix_{schema}_{table}_friendly_name` in 63) and always quoted (`[...]` / `"..."`, with the closing quote escaped). PostgreSQL names are lowercase because earlier versions did not quote them: a mixed-case name would silently target a new, empty table. - SQL Server passes names as parameters wherever T-SQL accepts them (existence checks, `QUOTENAME`, the duplicate-index error message). -- `InitializeDb()` creates a missing schema. It checks existence first (`SCHEMA_ID` / `pg_namespace`), because PostgreSQL's `CREATE SCHEMA IF NOT EXISTS` requires the database CREATE privilege even when the schema exists. +- `InitializeDb()` creates the schema, the table and the index only when missing. On PostgreSQL it checks existence first (`pg_namespace`, `pg_tables`, `pg_indexes`), because PostgreSQL checks privileges before `IF NOT EXISTS`: a user without CREATE on the database or schema, or without ownership of the table, would otherwise fail on every start. - Defaults are in each provider's `Constants.cs`. Keep the table DDL in sync with the "Database schema" section of `README.md`. ### Testing diff --git a/README.md b/README.md index a357e43..76c46db 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,10 @@ app.Services.UseDapperDataProtection(); app.Run(); ``` +The provider registers its `SqlConnectionFactory` as a keyed service. A `SqlConnectionFactory` +registered by the application is neither replaced nor used for the keys, whatever the registration +order. + ### PostgreSQL ```csharp @@ -146,8 +150,9 @@ builder.Services.AddDataProtection() `UseDapperDataProtection()` is an extension on `IServiceProvider`. Call it once at startup (`app.Services.UseDapperDataProtection()`): it validates the service registrations and, when `InitializeTable` is `true`, creates the schema, the table and the friendly-name unique index if -they are missing. The schema is created only when it does not exist, so the database user needs the -permission to create schemas only in that case. The index is also created on an existing table that lacks it; if that table already +they are missing. Each object is created only when it does not exist, so the database user needs the +permission to create it only in that case: once the schema, the table and the index exist, a user +that can only read, insert and delete rows of the table is enough. The index is also created on an existing table that lacks it; if that table already contains duplicate non-null friendly names, initialization fails and the duplicates must be removed before retrying. Initialization is safe when several instances start at the same time: it runs in a transaction that holds a database-wide lock (`sp_getapplock` on SQL Server, @@ -164,7 +169,7 @@ the option and the rule: | | SQL Server | PostgreSQL | |---|---|---| | Allowed characters | letters, digits and `_`; must not start with a digit | lowercase letters, digits and `_`; must not start with a digit | -| Maximum length | 128 for the schema, 112 for the table (room for the `IX_{TableName}_FriendlyName` index name) | 63 | +| Maximum length | 128 for the schema, 112 for the table (room for the `IX_{TableName}_FriendlyName` index name) | 45 for the schema and the table together (room for the `ix_{SchemaName}_{TableName}_friendly_name` index name within the 63-character identifier limit) | The names are always quoted in the generated SQL, so reserved words such as `order` or `user` are allowed. @@ -345,7 +350,7 @@ Protection key repository. The adapter maps the framework's XML-element calls to service provider for every operation. When `TStorage` implements `IDeletableDataProtectionStorage`, the adapter also implements the framework's `IDeletableXmlRepository` and maps deletions to `Delete()`. -When the key ring is loaded, a stored key whose XML is malformed is skipped and logged as an error +When the key ring is loaded, a stored key whose XML is malformed or missing is skipped and logged as an error (event `SkippingMalformedKey`, with the friendly name and the parse position; the XML itself is never logged). The remaining keys stay usable, but data protected with the skipped key can no longer be unprotected, and if no valid key remains, Data Protection generates a new one. @@ -389,6 +394,12 @@ with an `OptionsValidationException`. The rules are listed in - **PostgreSQL:** names must be lowercase. v1 did not quote the names, so PostgreSQL folded them to lowercase: a table configured as `DpKeys` was created as `dpkeys`. Replace the configured name with its lowercase form; the database does not need to change. +- **PostgreSQL:** the schema and table names together must be at most 45 characters long. v1 + accepted longer names, but PostgreSQL silently truncated the derived index name to 63 + characters, so two tables could end up with the same index name. Rename the table + (`ALTER TABLE ... RENAME TO ...`) and its index to `ix_{SchemaName}_{TableName}_friendly_name` + (`ALTER INDEX ... RENAME TO ...`), then update the configuration. If the index is not renamed, + `InitializeDb()` creates a second unique index with the new name. #### The PostgreSQL provider no longer enables Dapper's underscore mapping @@ -422,6 +433,24 @@ v1 registered its `NpgsqlDataSource` as an unkeyed singleton. v2 registers it as A malformed connection string now throws when the data source is first resolved, in `UseDapperDataProtection()`, instead of in `PersistKeysWithDapperInPostgreSQL()`. +#### The SQL Server provider registers its connection factory as a keyed service + +v1 registered its `SqlConnectionFactory` as an unkeyed singleton. v2 registers it as a keyed service +(see [SQL Server](#sql-server)). This affects: + +- Applications that resolve `SqlConnectionFactory` from dependency injection and received the keys + connection factory: register your own `SqlConnectionFactory`. +- Applications that register `SQLServerDataProtectionRepository` themselves instead of calling + `PersistKeysWithDapperInSqlServer()`: the constructor now resolves the provider's keyed connection + factory. Call `PersistKeysWithDapperInSqlServer()`, or register the repository with a factory + that passes the connection factory explicitly: + + ```csharp + builder.Services.AddScoped(sp => new SQLServerDataProtectionRepository( + new SqlConnectionFactory(connectionString), + sp.GetRequiredService>())); + ``` + #### `StorageWrapper` is internal Affects code that created `StorageWrapper` directly, for example to assign @@ -461,9 +490,12 @@ These need no code change, but can affect a running application: - **Missing schema created.** When `InitializeTable` is `true`, `InitializeDb()` creates a missing schema instead of failing. The database user needs the permission to create schemas only in that case. +- **PostgreSQL initialization with a restricted user.** `InitializeDb()` creates the table and the + index only when they are missing. In v1, `CREATE TABLE IF NOT EXISTS` failed on every start for a + user without the `CREATE` privilege on the schema, even when the table existed. - **Blank friendly names stored as `NULL`.** An empty or whitespace-only friendly name is passed to `Insert` as `null`; v1 passed an empty string. Custom storages must accept `null`. -- **Malformed keys skipped.** A stored key whose XML is malformed is logged +- **Malformed keys skipped.** A stored key whose XML is malformed or `null` is logged (`SkippingMalformedKey`) and skipped; in v1 it made loading the whole key ring fail. - **No key XML in `KeyInsertException`.** The exception message names the key by friendly name only; the storage exception is the `InnerException`. diff --git a/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/DapperDataProtectionConfigValidator.cs b/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/DapperDataProtectionConfigValidator.cs index fb03099..8b6bedc 100644 --- a/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/DapperDataProtectionConfigValidator.cs +++ b/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/DapperDataProtectionConfigValidator.cs @@ -19,23 +19,40 @@ internal sealed partial class DapperDataProtectionConfigValidator : IValidateOpt /// internal const int MaxNameLength = 63; + /// + /// Maximum combined length of the schema and table names, which leaves room for the derived index name + /// ix_{SchemaName}_{TableName}_friendly_name, the longest name derived from them. + /// PostgreSQL would silently truncate a longer name, so two tables could get the same index name. + /// + internal static readonly int MaxCombinedNameLength = + MaxNameLength - PostgreSQLDataProtectionRepository.GetIndexName(string.Empty, string.Empty).Length; + /// public ValidateOptionsResult Validate(string? name, DapperDataProtectionConfig options) { var failures = new List(); Validate(nameof(DapperDataProtectionConfig.SchemaName), options.SchemaName, failures); Validate(nameof(DapperDataProtectionConfig.TableName), options.TableName, failures); + if (failures.Count == 0 && options.SchemaName.Length + options.TableName.Length > MaxCombinedNameLength) + { + failures.Add( + $"{nameof(DapperDataProtectionConfig)}.{nameof(DapperDataProtectionConfig.SchemaName)} '{options.SchemaName}' " + + $"and {nameof(DapperDataProtectionConfig)}.{nameof(DapperDataProtectionConfig.TableName)} '{options.TableName}' " + + $"are too long: together they must be at most {MaxCombinedNameLength} characters long, so that the " + + $"derived index name fits the PostgreSQL limit of {MaxNameLength} characters."); + } + return failures.Count == 0 ? ValidateOptionsResult.Success : ValidateOptionsResult.Fail(failures); } private static void Validate(string optionName, string? value, List failures) { - if (value is null || value.Length > MaxNameLength || !IdentifierRegex().IsMatch(value)) + if (value is null || !IdentifierRegex().IsMatch(value)) { failures.Add( $"{nameof(DapperDataProtectionConfig)}.{optionName} '{value}' is not a valid PostgreSQL name: " + - "it must start with a lowercase letter or an underscore, contain only lowercase letters, digits " + - $"and underscores, and be at most {MaxNameLength} characters long."); + "it must start with a lowercase letter or an underscore and contain only lowercase letters, digits " + + "and underscores."); } } diff --git a/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/PostgreSQLDataProtectionRepository.cs b/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/PostgreSQLDataProtectionRepository.cs index d427a46..1b0cb5f 100644 --- a/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/PostgreSQLDataProtectionRepository.cs +++ b/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/PostgreSQLDataProtectionRepository.cs @@ -123,8 +123,9 @@ public void InitializeDb() connection.Execute("SELECT pg_advisory_xact_lock(hashtext(@LockName));", new { LockName = InitializeDbLockName }, transaction); - // CREATE SCHEMA IF NOT EXISTS requires the CREATE privilege on the database even when the schema - // exists, so it is executed only when the schema is missing. + // PostgreSQL checks privileges before IF NOT EXISTS: CREATE SCHEMA requires the CREATE privilege on the + // database, CREATE TABLE the CREATE privilege on the schema and CREATE INDEX the ownership of the table, + // even when the object exists. Each object is therefore created only when it is missing. var schemaExists = connection.ExecuteScalar( "SELECT EXISTS(SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = @SchemaName);", new { _config.SchemaName }, @@ -135,24 +136,49 @@ public void InitializeDb() transaction: transaction); } - connection.Execute($""" - CREATE TABLE IF NOT EXISTS {_tableName} - ( - id INTEGER generated always as identity, - insert_date timestamp with time zone NOT NULL default NOW(), - friendly_name character varying(256) COLLATE pg_catalog."default" NULL, - xml text COLLATE pg_catalog."default" NOT NULL, - CONSTRAINT {QuoteIdentifier($"pk_{_config.SchemaName}_{_config.TableName}")} PRIMARY KEY (id) - ); - - CREATE UNIQUE INDEX IF NOT EXISTS {QuoteIdentifier($"ix_{_config.SchemaName}_{_config.TableName}_friendly_name")} - ON {_tableName} USING btree - (friendly_name COLLATE pg_catalog."default" ASC NULLS LAST) - TABLESPACE pg_default; - """, - transaction: transaction); + var tableExists = connection.ExecuteScalar( + "SELECT EXISTS(SELECT 1 FROM pg_catalog.pg_tables WHERE schemaname = @SchemaName AND tablename = @TableName);", + new { _config.SchemaName, _config.TableName }, + transaction); + if (!tableExists) + { + connection.Execute($""" + CREATE TABLE IF NOT EXISTS {_tableName} + ( + id INTEGER generated always as identity, + insert_date timestamp with time zone NOT NULL default NOW(), + friendly_name character varying(256) COLLATE pg_catalog."default" NULL, + xml text COLLATE pg_catalog."default" NOT NULL, + CONSTRAINT {QuoteIdentifier($"pk_{_config.SchemaName}_{_config.TableName}")} PRIMARY KEY (id) + ); + """, + transaction: transaction); + } + + var indexName = GetIndexName(_config.SchemaName, _config.TableName); + var indexExists = connection.ExecuteScalar( + "SELECT EXISTS(SELECT 1 FROM pg_catalog.pg_indexes WHERE schemaname = @SchemaName AND indexname = @IndexName);", + new { _config.SchemaName, IndexName = indexName }, + transaction); + if (!indexExists) + { + connection.Execute($""" + CREATE UNIQUE INDEX IF NOT EXISTS {QuoteIdentifier(indexName)} + ON {_tableName} USING btree + (friendly_name COLLATE pg_catalog."default" ASC NULLS LAST) + TABLESPACE pg_default; + """, + transaction: transaction); + } + transaction.Commit(); } + /// + /// Returns the name of the friendly-name unique index, the longest name derived from the schema and table names. + /// + internal static string GetIndexName(string schemaName, string tableName) => + $"ix_{schemaName}_{tableName}_friendly_name"; + private static string QuoteIdentifier(string name) => $"\"{name.Replace("\"", "\"\"")}\""; } \ No newline at end of file diff --git a/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/Constants.cs b/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/Constants.cs index 9f4e413..219d16c 100644 --- a/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/Constants.cs +++ b/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/Constants.cs @@ -13,4 +13,9 @@ public static class Constants /// default keys table name /// public const string TableName = "DataProtectionKeys"; + /// + /// service key of the registered by the provider, + /// so that it does not collide with a connection factory registered by the application + /// + internal const string ConnectionFactoryServiceKey = "AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer"; } \ No newline at end of file diff --git a/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/DapperDataProtectionExtensions.cs b/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/DapperDataProtectionExtensions.cs index 5047386..b8d14e7 100644 --- a/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/DapperDataProtectionExtensions.cs +++ b/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/DapperDataProtectionExtensions.cs @@ -41,7 +41,9 @@ public static IDataProtectionBuilder PersistKeysWithDapperInSqlServer(this IData builder.Services.TryAddEnumerable(ServiceDescriptor .Singleton, DapperDataProtectionConfigValidator>()); - builder.Services.AddSingleton(_=>new SqlConnectionFactory(connectionString)); + // Keyed, so that a connection factory registered by the application is neither replaced nor used. + builder.Services.AddKeyedSingleton(Constants.ConnectionFactoryServiceKey, + (_, _) => new SqlConnectionFactory(connectionString)); builder.Services.AddScoped(); builder.PersistKeysToStorage(); return builder; diff --git a/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/SQLServerDataProtectionRepository.cs b/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/SQLServerDataProtectionRepository.cs index b1e8417..577f954 100644 --- a/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/SQLServerDataProtectionRepository.cs +++ b/src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/SQLServerDataProtectionRepository.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Threading.Tasks; using Dapper; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; namespace AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer; @@ -30,7 +31,8 @@ private string GetAllQuery() => /// /// /// repository configuration - public SQLServerDataProtectionRepository(SqlConnectionFactory connectionFactory, + public SQLServerDataProtectionRepository( + [FromKeyedServices(Constants.ConnectionFactoryServiceKey)] SqlConnectionFactory connectionFactory, IOptionsSnapshot options) { _connectionFactory = connectionFactory; diff --git a/src/AspNetCore.DataProtection.CustomStorage/StorageWrapper.cs b/src/AspNetCore.DataProtection.CustomStorage/StorageWrapper.cs index 80e5b1c..e55e723 100644 --- a/src/AspNetCore.DataProtection.CustomStorage/StorageWrapper.cs +++ b/src/AspNetCore.DataProtection.CustomStorage/StorageWrapper.cs @@ -67,7 +67,8 @@ public IReadOnlyCollection GetAllElements() _logger.ReadingXmlFromKey(key.FriendlyName); try { - return XElement.Parse(key.Xml); + // Xml is required but not enforced at runtime: a missing value is handled as an empty, malformed document. + return XElement.Parse(key.Xml ?? string.Empty); } catch (XmlException e) { diff --git a/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDapperDataProtectionConfigValidatorTests.cs b/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDapperDataProtectionConfigValidatorTests.cs index 0ab82fb..4114721 100644 --- a/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDapperDataProtectionConfigValidatorTests.cs +++ b/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDapperDataProtectionConfigValidatorTests.cs @@ -51,14 +51,28 @@ public void Validate_MixedCaseName_FailsExplainingLowercaseRequirement() } [Theory] - [InlineData(63, true)] - [InlineData(64, false)] - public void Validate_NameLength_IsLimitedToPostgreSqlIdentifierLength(int length, bool succeeded) + [InlineData(45, true)] + [InlineData(46, false)] + public void Validate_CombinedNameLength_LeavesRoomForDerivedIndexName(int combinedLength, bool succeeded) { - var name = new string('n', length); + var schemaName = new string('s', 10); + var tableName = new string('t', combinedLength - schemaName.Length); - _sut.Validate(null, Config(name, Constants.TableName)).Succeeded.Should().Be(succeeded); - _sut.Validate(null, Config(Constants.DefaultSchema, name)).Succeeded.Should().Be(succeeded); + PostgreSQLDataProtectionRepository.GetIndexName(schemaName, tableName).Length + .Should().Be(succeeded ? DapperDataProtectionConfigValidator.MaxNameLength : DapperDataProtectionConfigValidator.MaxNameLength + 1); + _sut.Validate(null, Config(schemaName, tableName)).Succeeded.Should().Be(succeeded); + _sut.Validate(null, Config(tableName, schemaName)).Succeeded.Should().Be(succeeded); + } + + [Fact] + public void Validate_CombinedNameTooLong_FailsNamingBothOptions() + { + var result = _sut.Validate(null, Config(Constants.DefaultSchema, new string('t', 64))); + + result.Failed.Should().BeTrue(); + result.FailureMessage.Should().Contain($"{nameof(DapperDataProtectionConfig)}.{nameof(DapperDataProtectionConfig.SchemaName)}") + .And.Contain($"{nameof(DapperDataProtectionConfig)}.{nameof(DapperDataProtectionConfig.TableName)}") + .And.Contain(DapperDataProtectionConfigValidator.MaxCombinedNameLength.ToString()); } public static TheoryData InvalidNames() => diff --git a/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDataProtectionRepositoryTests.cs b/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDataProtectionRepositoryTests.cs index eb334a9..bce53d3 100644 --- a/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDataProtectionRepositoryTests.cs +++ b/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDataProtectionRepositoryTests.cs @@ -205,6 +205,36 @@ public void InitializeDb_ReservedWordSchemaName_PersistsKeys() CountRows(schemaName, tableName).Should().Be(0); } + [Fact] + public void InitializeDb_ExistingTableAndRoleWithoutCreatePrivilege_Succeeds() + { + // Since PostgreSQL 15 the PUBLIC role has no CREATE privilege on the public schema, + // so the role can only use the table it has been granted. + var tableName = PostgreSqlContainerFixture.NewTableName(); + CreateSut(Constants.DefaultSchema, tableName).InitializeDb(); + var roleName = $"role_{tableName}"; + using (var connection = _fixture.NpgsqlDataSource.OpenConnection()) + { + connection.Execute( + $""" + CREATE ROLE {roleName} LOGIN PASSWORD '{roleName}'; + GRANT SELECT, INSERT, DELETE ON {Constants.DefaultSchema}.{tableName} TO {roleName}; + """); + } + var connectionString = new NpgsqlConnectionStringBuilder(_fixture.ConnectionString) + { + Username = roleName, + Password = roleName + }.ConnectionString; + using var dataSource = NpgsqlDataSource.Create(connectionString); + var sut = CreateSut(Constants.DefaultSchema, tableName, dataSource); + + sut.Invoking(x => x.InitializeDb()).Should().NotThrow(); + sut.Insert(new DataProtectionKey { FriendlyName = "RestrictedRoleKey", Xml = "Xml" }); + + CountRows(Constants.DefaultSchema, tableName).Should().Be(1); + } + [Fact] public void Delete_NullKeys_ThrowsArgumentNullException() { @@ -279,11 +309,12 @@ CREATE TRIGGER tr_{tableName}_fail_delete BEFORE DELETE ON {Constants.DefaultSch sut.GetAll().Select(x => x.FriendlyName).Should().BeEquivalentTo("DeleteFirst", "DeleteFails"); } - private PostgreSQLDataProtectionRepository CreateSut(string schemaName, string tableName) + private PostgreSQLDataProtectionRepository CreateSut(string schemaName, string tableName, + NpgsqlDataSource? dataSource = null) { var options = Substitute.For>(); options.Value.Returns(new DapperDataProtectionConfig { SchemaName = schemaName, TableName = tableName }); - return new PostgreSQLDataProtectionRepository(_fixture.NpgsqlDataSource, options); + return new PostgreSQLDataProtectionRepository(dataSource ?? _fixture.NpgsqlDataSource, options); } private long CountRows(string schemaName, string tableName) diff --git a/tests/AspNetCore.DataProtection.CustomStorage.Tests/SQLServer/SqlServerDapperDataProtectionExtensionsTests.cs b/tests/AspNetCore.DataProtection.CustomStorage.Tests/SQLServer/SqlServerDapperDataProtectionExtensionsTests.cs index 010fedb..c6e3c16 100644 --- a/tests/AspNetCore.DataProtection.CustomStorage.Tests/SQLServer/SqlServerDapperDataProtectionExtensionsTests.cs +++ b/tests/AspNetCore.DataProtection.CustomStorage.Tests/SQLServer/SqlServerDapperDataProtectionExtensionsTests.cs @@ -13,6 +13,33 @@ public class SqlServerDapperDataProtectionExtensionsTests { private const string ConnectionString = "Server=localhost;Database=test"; + [Fact] + public void PersistKeysWithDapperInSqlServer_DoesNotRegisterUnkeyedConnectionFactory() + { + var services = new ServiceCollection(); + + services.AddDataProtection().PersistKeysWithDapperInSqlServer(ConnectionString); + + services.Should().NotContain(d => d.ServiceType == typeof(SqlConnectionFactory) && !d.IsKeyedService); + } + + [Fact] + public void PersistKeysWithDapperInSqlServer_ApplicationConnectionFactoryRegistered_KeepsBothFactories() + { + var applicationFactory = new SqlConnectionFactory("Server=localhost;Database=app"); + var services = new ServiceCollection(); + services.AddSingleton(applicationFactory); + services.AddDataProtection().PersistKeysWithDapperInSqlServer(ConnectionString); + using var provider = services.BuildServiceProvider(); + + provider.GetRequiredService().Should().BeSameAs(applicationFactory); + provider.GetRequiredKeyedService(Constants.ConnectionFactoryServiceKey) + .CreateConnection().ConnectionString.Should().Be(ConnectionString); + using var scope = provider.CreateScope(); + scope.ServiceProvider.GetRequiredService() + .Should().BeOfType(); + } + [Fact] public void PersistKeysWithDapperInSqlServer_DefaultConfig_PassesStartupValidation() { diff --git a/tests/AspNetCore.DataProtection.CustomStorage.Tests/StorageWrapperTests.cs b/tests/AspNetCore.DataProtection.CustomStorage.Tests/StorageWrapperTests.cs index 7ae9008..056ed6a 100644 --- a/tests/AspNetCore.DataProtection.CustomStorage.Tests/StorageWrapperTests.cs +++ b/tests/AspNetCore.DataProtection.CustomStorage.Tests/StorageWrapperTests.cs @@ -188,6 +188,22 @@ public void GetAllElements_RowWithMalformedXml_LogsErrorNamingRowWithoutKeyXml() _loggerFactory.Entries.Should().NotContain(e => e.Contains(KeyMaterial)); } + [Fact] + public void GetAllElements_RowWithNullXml_SkipsRowAndLogsError() + { + _storage.GetAll().Returns([ + new DataProtectionKey { FriendlyName = MalformedFriendlyName, Xml = null! }, + new DataProtectionKey { FriendlyName = KeyFriendlyName, Xml = KeyElement.ToString() } + ]); + + var elements = _sut.GetAllElements(); + + elements.Should().ContainSingle() + .Which.ToString().Should().Be(KeyElement.ToString()); + _loggerFactory.Entries.Should().ContainSingle(e => e.Level == LogLevel.Error) + .Which.Should().Match(e => e.EventId.Name == "SkippingMalformedKey" && e.Contains(MalformedFriendlyName)); + } + [Fact] public void GetAllElements_EmptyStorage_ReturnsEmpty() {