From c9f9bba542385d5d7092ada80db5a9dbbf876898 Mon Sep 17 00:00:00 2001 From: giammin Date: Thu, 22 Oct 2026 23:17:02 +0200 Subject: [PATCH] fix(postgresql): register NpgsqlDataSource as a keyed service The provider registered its data source as an unkeyed NpgsqlDataSource singleton, which collided with a data source registered by the application: depending on the registration order, either the application queries went to the keys database or the keys went to the application database. The instance registration also prevented the container from disposing it. Register the data source as a keyed singleton created by a factory, so the container owns and disposes it, and resolve it in the repository with [FromKeyedServices]. The public repository constructor is unchanged. A malformed connection string now fails when the data source is first resolved (UseDapperDataProtection) instead of at registration. Closes #19 --- AGENTS.md | 2 +- README.md | 4 +++ .../Constants.cs | 5 +++ .../DapperDataProtectionExtensions.cs | 7 ++-- .../PostgreSQLDataProtectionRepository.cs | 3 +- ...eSqlDapperDataProtectionExtensionsTests.cs | 29 +++++++++++++-- .../PostgreSQL/PostgreSqlEndToEndTests.cs | 36 ++++++++++++++++++- 7 files changed, 77 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b71f40c..61d2d08 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -52,7 +52,7 @@ ASP.NET Core Data Protection (IXmlRepository) - `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. - 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`, register the connection source as a singleton (`SqlConnectionFactory` / `NpgsqlDataSource`), register the repository as scoped `IDbDataProtectionStorage`, then call `PersistKeysToStorage()`. +- **DB providers:** `PersistKeysWithDapperInSqlServer(...)` / `PersistKeysWithDapperInPostgreSQL(...)` configure `IOptions`, 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()`. - `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). ### Database Implementations diff --git a/README.md b/README.md index ba5714b..b1ece35 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,10 @@ app.Services.UseDapperDataProtection(); app.Run(); ``` +The provider registers its `NpgsqlDataSource` as a keyed service and disposes it with the service +provider. An `NpgsqlDataSource` registered by the application (for example with +`AddNpgsqlDataSource`) is neither replaced nor used for the keys, whatever the registration order. + ## Configuration Both providers accept an optional `Action` to override the defaults: diff --git a/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/Constants.cs b/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/Constants.cs index 488146d..5d4d6d7 100644 --- a/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/Constants.cs +++ b/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/Constants.cs @@ -13,4 +13,9 @@ public static class Constants /// default keys table name /// public const string TableName = "data_protection_keys"; + /// + /// service key of the registered by the provider, + /// so that it does not collide with a data source registered by the application + /// + internal const string DataSourceServiceKey = "AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL"; } \ No newline at end of file diff --git a/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/DapperDataProtectionExtensions.cs b/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/DapperDataProtectionExtensions.cs index 88b442d..40e6e71 100644 --- a/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/DapperDataProtectionExtensions.cs +++ b/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/DapperDataProtectionExtensions.cs @@ -37,9 +37,10 @@ public static IDataProtectionBuilder PersistKeysWithDapperInPostgreSQL(this IDat c.TableName = config.TableName; }); - var npgsqlDataSource = new NpgsqlDataSourceBuilder(connectionString).Build(); - - builder.Services.AddSingleton(npgsqlDataSource); + // Keyed, so that a data source registered by the application is neither replaced nor used; + // created by a factory, so that the container owns and disposes it. + builder.Services.AddKeyedSingleton(Constants.DataSourceServiceKey, + (_, _) => new NpgsqlDataSourceBuilder(connectionString).Build()); builder.Services.AddScoped(); builder.PersistKeysToStorage(); diff --git a/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/PostgreSQLDataProtectionRepository.cs b/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/PostgreSQLDataProtectionRepository.cs index 0c41577..ad4fc19 100644 --- a/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/PostgreSQLDataProtectionRepository.cs +++ b/src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/PostgreSQLDataProtectionRepository.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Threading.Tasks; using Dapper; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Npgsql; @@ -28,7 +29,7 @@ private string GetAllQuery() => /// /// the db connection /// repository configuration - public PostgreSQLDataProtectionRepository(NpgsqlDataSource dataSource, + public PostgreSQLDataProtectionRepository([FromKeyedServices(Constants.DataSourceServiceKey)] NpgsqlDataSource dataSource, IOptionsSnapshot options) { _dataSource = dataSource; diff --git a/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDapperDataProtectionExtensionsTests.cs b/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDapperDataProtectionExtensionsTests.cs index e05ded0..376c77a 100644 --- a/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDapperDataProtectionExtensionsTests.cs +++ b/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlDapperDataProtectionExtensionsTests.cs @@ -1,4 +1,4 @@ -using System.Linq; +using System; using AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL; using Dapper; using FluentAssertions; @@ -19,14 +19,37 @@ public void PersistKeysWithDapperInPostgreSQL_PreservesIncludeErrorDetail(string { var services = new ServiceCollection(); services.AddDataProtection().PersistKeysWithDapperInPostgreSQL(connectionString); + using var provider = services.BuildServiceProvider(); - var dataSource = services.Single(d => d.ServiceType == typeof(NpgsqlDataSource)) - .ImplementationInstance.Should().BeAssignableTo().Subject; + var dataSource = provider.GetRequiredKeyedService(Constants.DataSourceServiceKey); new NpgsqlConnectionStringBuilder(dataSource.ConnectionString).IncludeErrorDetail .Should().Be(expected); } + [Fact] + public void PersistKeysWithDapperInPostgreSQL_DoesNotRegisterUnkeyedDataSource() + { + var services = new ServiceCollection(); + + services.AddDataProtection().PersistKeysWithDapperInPostgreSQL("Host=localhost;Database=test"); + + services.Should().NotContain(d => d.ServiceType == typeof(NpgsqlDataSource) && !d.IsKeyedService); + } + + [Fact] + public void PersistKeysWithDapperInPostgreSQL_ServiceProviderDisposed_DisposesDataSource() + { + var services = new ServiceCollection(); + services.AddDataProtection().PersistKeysWithDapperInPostgreSQL("Host=localhost;Database=test"); + var provider = services.BuildServiceProvider(); + var dataSource = provider.GetRequiredKeyedService(Constants.DataSourceServiceKey); + + provider.Dispose(); + + dataSource.Invoking(d => d.OpenConnection()).Should().Throw(); + } + [Fact] public void PersistKeysWithDapperInPostgreSQL_DoesNotEnableMatchNamesWithUnderscores() { diff --git a/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlEndToEndTests.cs b/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlEndToEndTests.cs index 583dd9b..36b6634 100644 --- a/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlEndToEndTests.cs +++ b/tests/AspNetCore.DataProtection.CustomStorage.Tests/PostgreSQL/PostgreSqlEndToEndTests.cs @@ -10,6 +10,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Npgsql; using Xunit; namespace AspNetCore.DataProtection.CustomStorage.Tests.PostgreSQL; @@ -73,16 +74,49 @@ public void DeleteKeys_KeySelected_DeletesItFromTable() GetStoredFriendlyNames(tableName).Should().Equal($"key-{kept.KeyId:D}"); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Protect_ApplicationRegistersOwnDataSource_DataSourcesStaySeparate(bool applicationRegistersFirst) + { + var tableName = NewTableName(); + // The application data source targets a database that does not exist, so that any key + // operation routed through it fails. + var applicationConnectionString = new NpgsqlConnectionStringBuilder(_fixture.ConnectionString) + { + Database = "application_database_does_not_exist" + }.ConnectionString; + using var applicationDataSource = NpgsqlDataSource.Create(applicationConnectionString); + using var services = BuildServiceProvider(tableName, applicationDataSource, applicationRegistersFirst); + var protector = services.GetRequiredService().CreateProtector(Purpose); + + var protectedPayload = protector.Protect("payload"); + + services.GetRequiredService().Should().BeSameAs(applicationDataSource); + protector.Unprotect(protectedPayload).Should().Be("payload"); + GetStoredFriendlyNames(tableName).Should().ContainSingle(); + } + // PostgreSQL folds unquoted identifiers to lower case; keep the name short enough for derived index names. private static string NewTableName() => $"keys_{Guid.NewGuid().ToString("N")[..12]}"; - private ServiceProvider BuildServiceProvider(string tableName) + private ServiceProvider BuildServiceProvider(string tableName, + NpgsqlDataSource? applicationDataSource = null, + bool applicationRegistersFirst = false) { var services = new ServiceCollection(); services.AddSingleton(NullLoggerFactory.Instance); + if (applicationDataSource is not null && applicationRegistersFirst) + { + services.AddSingleton(applicationDataSource); + } services.AddDataProtection() .SetApplicationName(ApplicationName) .PersistKeysWithDapperInPostgreSQL(_fixture.ConnectionString, config => config.TableName = tableName); + if (applicationDataSource is not null && !applicationRegistersFirst) + { + services.AddSingleton(applicationDataSource); + } var provider = services.BuildServiceProvider(); provider.UseDapperDataProtection();