Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ ASP.NET Core Data Protection (IXmlRepository)
### Wiring

- **Core:** `PersistKeysToStorage<TStorage>()` registers a singleton `IConfigureOptions<KeyManagementOptions>` that sets `XmlRepository` to a `StorageWrapper<TStorage>`, or to a `DeletableStorageWrapper<TStorage>` 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`.
- `DeletableStorageWrapper.DeleteElements` offers the parsed keys (malformed rows skipped), then passes the chosen `GetAll()` instances, ordered by `DeletionOrder`, to a single `Delete()` call. Each key is logged (`DeletedKeyFromStorage`) only after `Delete()` succeeds; 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 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<DapperDataProtectionConfig>` 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<IDbDataProtectionStorage>()`.
- **DB providers:** `PersistKeysWithDapperInSqlServer(...)` / `PersistKeysWithDapperInPostgreSQL(...)` throw `InvalidOperationException` when an `IDbDataProtectionStorage` is already registered (both providers share one unnamed `DapperDataProtectionConfig`), then configure `IOptions<DapperDataProtectionConfig>` 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<IDbDataProtectionStorage>()`.
- `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
Expand Down
14 changes: 12 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ builder.Services.AddDataProtection()
});
```

Call `PersistKeysWithDapperInSqlServer()` or `PersistKeysWithDapperInPostgreSQL()` only once: a
second call, to either provider, throws an `InvalidOperationException`.

| Option | Type | Default (SQL Server) | Default (PostgreSQL) | Description |
|--------|------|----------------------|----------------------|-------------|
| `SchemaName` | `string` | `dbo` | `public` | Schema that contains the keys table. See [Schema and table names](#schema-and-table-names). |
Expand Down Expand Up @@ -339,8 +342,8 @@ if (keyManager is XmlKeyManager { CanDeleteKeys: true } xmlKeyManager)
The selected keys are deleted with a single `Delete` call. The database providers delete the rows
by `Id`, in deletion order, in one transaction: either every selected key is deleted or none is.
If the storage throws, the error is logged (event `ErrorDeletingKeysFromStorage`) and `DeleteKeys`
returns `false`. Each deleted key is logged by friendly name (event `DeletingKeyFromStorage`); the
key XML is never logged. A stored key whose XML is malformed is not offered for deletion.
returns `false`. Once the deletion has succeeded, each deleted key is logged by friendly name (event
`DeletedKeyFromStorage`); the key XML is never logged. A stored key whose XML is malformed is not offered for deletion.

## How it works

Expand Down Expand Up @@ -451,6 +454,13 @@ v1 registered its `SqlConnectionFactory` as an unkeyed singleton. v2 registers i
sp.GetRequiredService<IOptionsSnapshot<DapperDataProtectionConfig>>()));
```

#### The Dapper providers can be registered only once

v1 accepted several calls to `PersistKeysWithDapperInSqlServer()` and
`PersistKeysWithDapperInPostgreSQL()`: the last call silently replaced the storage of the previous
ones, which shared a single configuration. v2 throws an `InvalidOperationException` when an
`IDbDataProtectionStorage` is already registered. Keep a single call.

#### `StorageWrapper<TStorage>` is internal

Affects code that created `StorageWrapper<TStorage>` directly, for example to assign
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Linq;
using Microsoft.AspNetCore.DataProtection;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
Expand All @@ -20,11 +21,19 @@ public static class DapperDataProtectionExtensions
/// <param name="connectionString"></param>
/// <param name="configAction"></param>
/// <returns>The value <paramref name="builder"/>.</returns>
/// <exception cref="InvalidOperationException">an <see cref="IDbDataProtectionStorage"/> is already registered</exception>
public static IDataProtectionBuilder PersistKeysWithDapperInPostgreSQL(this IDataProtectionBuilder builder,
string connectionString,
Action<DapperDataProtectionConfig>? configAction = null)
{
ArgumentException.ThrowIfNullOrWhiteSpace(connectionString);
// A second registration would silently replace the first one and share its options.
if (builder.Services.Any(d => d.ServiceType == typeof(IDbDataProtectionStorage)))
{
throw new InvalidOperationException(
$"An {nameof(IDbDataProtectionStorage)} is already registered: call {nameof(PersistKeysWithDapperInPostgreSQL)} " +
"or PersistKeysWithDapperInSqlServer only once.");
}

var config = new DapperDataProtectionConfig
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,7 @@ public class PostgreSQLDataProtectionRepository : IDbDataProtectionStorage
private readonly NpgsqlDataSource _dataSource;
private readonly DapperDataProtectionConfig _config;
private readonly string _tableName;
private string GetAllQuery() =>
$"""
SELECT
id AS "{nameof(DataProtectionKeyEntity.Id)}",
insert_date AS "{nameof(DataProtectionKeyEntity.InsertDate)}",
friendly_name AS "{nameof(DataProtectionKeyEntity.FriendlyName)}",
xml AS "{nameof(DataProtectionKeyEntity.Xml)}"
FROM {_tableName};
""";
private readonly string _getAllQuery;

/// <summary>
/// create an instance of <see cref="PostgreSQLDataProtectionRepository"/>
Expand All @@ -38,20 +30,29 @@ public PostgreSQLDataProtectionRepository([FromKeyedServices(Constants.DataSourc
_dataSource = dataSource;
_config = options.Value;
_tableName = $"{QuoteIdentifier(_config.SchemaName)}.{QuoteIdentifier(_config.TableName)}";
_getAllQuery =
$"""
SELECT
id AS "{nameof(DataProtectionKeyEntity.Id)}",
insert_date AS "{nameof(DataProtectionKeyEntity.InsertDate)}",
friendly_name AS "{nameof(DataProtectionKeyEntity.FriendlyName)}",
xml AS "{nameof(DataProtectionKeyEntity.Xml)}"
FROM {_tableName};
""";
}

/// <inheritdoc />
public IEnumerable<DataProtectionKey> GetAll()
{
using var connection = _dataSource.CreateConnection();
return connection.Query<DataProtectionKeyEntity>(GetAllQuery());
return connection.Query<DataProtectionKeyEntity>(_getAllQuery);
}

/// <inheritdoc />
public async Task<IEnumerable<DataProtectionKeyEntity>> GetAllAsync()
{
await using var connection = _dataSource.CreateConnection();
return await connection.QueryAsync<DataProtectionKeyEntity>(GetAllQuery());
return await connection.QueryAsync<DataProtectionKeyEntity>(_getAllQuery);
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Linq;
using Microsoft.AspNetCore.DataProtection;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
Expand All @@ -18,11 +19,19 @@ public static class DapperDataProtectionExtensions
/// <param name="connectionString"></param>
/// <param name="configAction"></param>
/// <returns>The value <paramref name="builder"/>.</returns>
/// <exception cref="InvalidOperationException">an <see cref="IDbDataProtectionStorage"/> is already registered</exception>
public static IDataProtectionBuilder PersistKeysWithDapperInSqlServer(this IDataProtectionBuilder builder,
string connectionString,
Action<DapperDataProtectionConfig>? configAction = null)
{
ArgumentException.ThrowIfNullOrWhiteSpace(connectionString);
// A second registration would silently replace the first one and share its options.
if (builder.Services.Any(d => d.ServiceType == typeof(IDbDataProtectionStorage)))
{
throw new InvalidOperationException(
$"An {nameof(IDbDataProtectionStorage)} is already registered: call {nameof(PersistKeysWithDapperInSqlServer)} " +
"or PersistKeysWithDapperInPostgreSQL only once.");
}

var config = new DapperDataProtectionConfig
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,8 @@ public class SQLServerDataProtectionRepository : IDbDataProtectionStorage
private readonly SqlConnectionFactory _connectionFactory;
private readonly DapperDataProtectionConfig _config;
private readonly string _tableName;
private readonly string _getAllQuery;

private string GetAllQuery() =>
$"""
SELECT
[{nameof(DataProtectionKeyEntity.Id)}],
[{nameof(DataProtectionKeyEntity.InsertDate)}],
[{nameof(DataProtectionKeyEntity.FriendlyName)}],
[{nameof(DataProtectionKeyEntity.Xml)}]
FROM {_tableName};
""";
/// <summary>
/// create an instance of <see cref="SQLServerDataProtectionRepository"/>
/// </summary>
Expand All @@ -38,20 +30,29 @@ public SQLServerDataProtectionRepository(
_connectionFactory = connectionFactory;
_config = options.Value;
_tableName = $"{QuoteIdentifier(_config.SchemaName)}.{QuoteIdentifier(_config.TableName)}";
_getAllQuery =
$"""
SELECT
[{nameof(DataProtectionKeyEntity.Id)}],
[{nameof(DataProtectionKeyEntity.InsertDate)}],
[{nameof(DataProtectionKeyEntity.FriendlyName)}],
[{nameof(DataProtectionKeyEntity.Xml)}]
FROM {_tableName};
""";
}

/// <inheritdoc />
public IEnumerable<DataProtectionKey> GetAll()
{
using var connection = _connectionFactory.CreateConnection();
return connection.Query<DataProtectionKeyEntity>(GetAllQuery());
return connection.Query<DataProtectionKeyEntity>(_getAllQuery);
}

/// <inheritdoc />
public async Task<IEnumerable<DataProtectionKeyEntity>> GetAllAsync()
{
await using var connection = _connectionFactory.CreateConnection();
return await connection.QueryAsync<DataProtectionKeyEntity>(GetAllQuery());
return await connection.QueryAsync<DataProtectionKeyEntity>(_getAllQuery);
}

/// <inheritdoc />
Expand All @@ -63,8 +64,8 @@ public void Insert(DataProtectionKey key)
connection.Execute(
$"""
INSERT INTO {_tableName}(
{nameof(DataProtectionKeyEntity.FriendlyName)},
{nameof(DataProtectionKeyEntity.Xml)})
[{nameof(DataProtectionKeyEntity.FriendlyName)}],
[{nameof(DataProtectionKeyEntity.Xml)}])
VALUES (
@{nameof(DataProtectionKeyEntity.FriendlyName)},
@{nameof(DataProtectionKeyEntity.Xml)})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,22 @@ public bool DeleteElements(Action<IReadOnlyCollection<IDeletableElement>> choose
return true;
}

foreach (var key in keysToDelete)
{
_logger.LogDeletingKeyFromStorage(key.FriendlyName, typeof(TStorage).Name);
}

try
{
storage.Delete(keysToDelete);
return true;
}
catch (Exception e)
{
_logger.LogErrorDeletingKeysFromStorage(e, keysToDelete.Count, typeof(TStorage).Name);
return false;
}

foreach (var key in keysToDelete)
{
_logger.LogDeletedKeyFromStorage(key.FriendlyName, typeof(TStorage).Name);
}

return true;
}

private sealed class DeletableElement(DataProtectionKey key, XElement element) : IDeletableElement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ internal static partial class LoggingExtensions
public static partial void LogSkippingMalformedKey(this ILogger logger, string? friendlyName, int lineNumber, int linePosition);

/// <summary>
/// Logs an information message for each key about to be deleted from storage.
/// Logs an information message for each key deleted from storage, once the deletion has succeeded.
/// The key XML is never logged because it may contain unencrypted key material.
/// </summary>
[LoggerMessage(6, LogLevel.Information, "Deleting key '{FriendlyName}' with '{storage}'.", EventName = "DeletingKeyFromStorage")]
public static partial void LogDeletingKeyFromStorage(this ILogger logger, string? friendlyName, string storage);
[LoggerMessage(6, LogLevel.Information, "Deleted key '{FriendlyName}' with '{storage}'.", EventName = "DeletedKeyFromStorage")]
public static partial void LogDeletedKeyFromStorage(this ILogger logger, string? friendlyName, string storage);

/// <summary>
/// Logs an error message when deleting keys from storage fails.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public void DeleteElements_ElementsChosen_LogsDeletedKeysWithoutKeyXml()
{
_sut.DeleteElements(elements => elements.First().DeletionOrder = 1);

_loggerFactory.Entries.Should().ContainSingle(e => e.EventId.Name == "DeletingKeyFromStorage")
_loggerFactory.Entries.Should().ContainSingle(e => e.EventId.Name == "DeletedKeyFromStorage")
.Which.Contains(_first.FriendlyName!).Should().BeTrue();
_loggerFactory.Entries.Should().NotContain(e => e.Contains(KeyMaterial));
}
Expand All @@ -119,6 +119,7 @@ public void DeleteElements_StorageFails_ReturnsFalseAndLogsErrorWithoutKeyXml()
result.Should().BeFalse();
_loggerFactory.Entries.Should().ContainSingle(e => e.Level == LogLevel.Error)
.Which.Exception.Should().BeSameAs(storageException);
_loggerFactory.Entries.Should().NotContain(e => e.EventId.Name == "DeletedKeyFromStorage");
_loggerFactory.Entries.Should().NotContain(e => e.Contains(KeyMaterial));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,26 @@ public void PersistKeysWithDapperInPostgreSQL_DoesNotEnableMatchNamesWithUndersc
DefaultTypeMap.MatchNamesWithUnderscores.Should().BeFalse();
}

[Fact]
public void PersistKeysWithDapperInPostgreSQL_CalledTwice_ThrowsInvalidOperationException()
{
var builder = new ServiceCollection().AddDataProtection().PersistKeysWithDapperInPostgreSQL("Host=localhost;Database=test");

builder.Invoking(x => x.PersistKeysWithDapperInPostgreSQL("Host=localhost;Database=test"))
.Should().Throw<InvalidOperationException>()
.WithMessage($"*{nameof(IDbDataProtectionStorage)}*");
}

[Fact]
public void PersistKeysWithDapperInPostgreSQL_AfterSqlServerProvider_ThrowsInvalidOperationException()
{
var builder = new ServiceCollection().AddDataProtection();
Dapper.SQLServer.DapperDataProtectionExtensions.PersistKeysWithDapperInSqlServer(builder, "Server=localhost;Database=test");

builder.Invoking(x => x.PersistKeysWithDapperInPostgreSQL("Host=localhost;Database=test"))
.Should().Throw<InvalidOperationException>();
}

[Fact]
public void PersistKeysWithDapperInPostgreSQL_DefaultConfig_PassesStartupValidation()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,26 @@ public void PersistKeysWithDapperInSqlServer_ApplicationConnectionFactoryRegiste
.Should().BeOfType<SQLServerDataProtectionRepository>();
}

[Fact]
public void PersistKeysWithDapperInSqlServer_CalledTwice_ThrowsInvalidOperationException()
{
var builder = new ServiceCollection().AddDataProtection().PersistKeysWithDapperInSqlServer(ConnectionString);

builder.Invoking(x => x.PersistKeysWithDapperInSqlServer(ConnectionString))
.Should().Throw<InvalidOperationException>()
.WithMessage($"*{nameof(IDbDataProtectionStorage)}*");
}

[Fact]
public void PersistKeysWithDapperInSqlServer_AfterPostgreSqlProvider_ThrowsInvalidOperationException()
{
var builder = new ServiceCollection().AddDataProtection();
Dapper.PostgreSQL.DapperDataProtectionExtensions.PersistKeysWithDapperInPostgreSQL(builder, "Host=localhost;Database=test");

builder.Invoking(x => x.PersistKeysWithDapperInSqlServer(ConnectionString))
.Should().Throw<InvalidOperationException>();
}

[Fact]
public void PersistKeysWithDapperInSqlServer_DefaultConfig_PassesStartupValidation()
{
Expand Down
Loading