From 7647b7d06e4c93037d006ce2c25ab2ae1e8ee6e6 Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Wed, 6 May 2026 21:22:52 +0100 Subject: [PATCH 01/12] style(api): fix newline at end of file in snapshot files --- .../Features/Snapshots/Contracts/PreviewSnapshotResponse.cs | 2 +- .../Features/Snapshots/Contracts/PublishSnapshotRequest.cs | 2 +- .../Features/Snapshots/PreviewSnapshotHandler.cs | 2 +- src/GroundControl.Api/Features/Snapshots/SnapshotResolver.cs | 2 +- src/GroundControl.Api/Properties/Usings.cs | 2 +- .../Snapshots/PreviewSnapshotHandlerTests.cs | 2 +- .../GroundControl.Api.Tests/Snapshots/SnapshotResolverTests.cs | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/GroundControl.Api/Features/Snapshots/Contracts/PreviewSnapshotResponse.cs b/src/GroundControl.Api/Features/Snapshots/Contracts/PreviewSnapshotResponse.cs index c342bc34..cb177baa 100644 --- a/src/GroundControl.Api/Features/Snapshots/Contracts/PreviewSnapshotResponse.cs +++ b/src/GroundControl.Api/Features/Snapshots/Contracts/PreviewSnapshotResponse.cs @@ -35,4 +35,4 @@ internal sealed record PreviewSnapshotResponse /// ?decrypt=true. /// public required IReadOnlyList Entries { get; init; } -} +} \ No newline at end of file diff --git a/src/GroundControl.Api/Features/Snapshots/Contracts/PublishSnapshotRequest.cs b/src/GroundControl.Api/Features/Snapshots/Contracts/PublishSnapshotRequest.cs index 53efba00..02d27a01 100644 --- a/src/GroundControl.Api/Features/Snapshots/Contracts/PublishSnapshotRequest.cs +++ b/src/GroundControl.Api/Features/Snapshots/Contracts/PublishSnapshotRequest.cs @@ -21,4 +21,4 @@ internal sealed record PublishSnapshotRequest /// [MaxLength(128)] public string? ExpectedHash { get; init; } -} +} \ No newline at end of file diff --git a/src/GroundControl.Api/Features/Snapshots/PreviewSnapshotHandler.cs b/src/GroundControl.Api/Features/Snapshots/PreviewSnapshotHandler.cs index 99d07853..d277b34a 100644 --- a/src/GroundControl.Api/Features/Snapshots/PreviewSnapshotHandler.cs +++ b/src/GroundControl.Api/Features/Snapshots/PreviewSnapshotHandler.cs @@ -102,4 +102,4 @@ private ResolvedEntryResponse MapEntry(ResolvedEntry entry, bool canDecrypt) }).ToList(), }; } -} +} \ No newline at end of file diff --git a/src/GroundControl.Api/Features/Snapshots/SnapshotResolver.cs b/src/GroundControl.Api/Features/Snapshots/SnapshotResolver.cs index 07c4953b..07e4572f 100644 --- a/src/GroundControl.Api/Features/Snapshots/SnapshotResolver.cs +++ b/src/GroundControl.Api/Features/Snapshots/SnapshotResolver.cs @@ -272,4 +272,4 @@ internal sealed record SnapshotResolveResult /// and within the BSON size limit). /// public bool IsPublishable => UnresolvedPlaceholders.Count == 0 && BsonSizeBytes <= SnapshotResolver.MaxBsonSizeBytes; -} +} \ No newline at end of file diff --git a/src/GroundControl.Api/Properties/Usings.cs b/src/GroundControl.Api/Properties/Usings.cs index 82429fdf..9c30fbc6 100644 --- a/src/GroundControl.Api/Properties/Usings.cs +++ b/src/GroundControl.Api/Properties/Usings.cs @@ -4,4 +4,4 @@ global using GroundControl.Api.Core.Validation; global using GroundControl.Api.Extensions.Http; global using GroundControl.Api.Extensions.Options; -global using GroundControl.Api.Extensions.Threading; +global using GroundControl.Api.Extensions.Threading; \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/Snapshots/PreviewSnapshotHandlerTests.cs b/tests/GroundControl.Api.Tests/Snapshots/PreviewSnapshotHandlerTests.cs index bcd4050e..b7cb7336 100644 --- a/tests/GroundControl.Api.Tests/Snapshots/PreviewSnapshotHandlerTests.cs +++ b/tests/GroundControl.Api.Tests/Snapshots/PreviewSnapshotHandlerTests.cs @@ -337,4 +337,4 @@ private static async Task CreateConfigEntryAsync(HttpClient apiClient, string ke var response = await apiClient.PostAsJsonAsync("/api/config-entries", request, WebJsonSerializerOptions, TestCancellationToken); response.StatusCode.ShouldBe(HttpStatusCode.Created); } -} +} \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/Snapshots/SnapshotResolverTests.cs b/tests/GroundControl.Api.Tests/Snapshots/SnapshotResolverTests.cs index cbde7dde..e0d1f76d 100644 --- a/tests/GroundControl.Api.Tests/Snapshots/SnapshotResolverTests.cs +++ b/tests/GroundControl.Api.Tests/Snapshots/SnapshotResolverTests.cs @@ -438,4 +438,4 @@ private static async Task UpdateConfigEntryAsync(HttpClient apiClient, Guid entr var response = await apiClient.SendAsync(request, TestCancellationToken); response.StatusCode.ShouldBe(HttpStatusCode.OK); } -} +} \ No newline at end of file From 484fcbf41a0dd8af354b74b52830932b1c8758b0 Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Wed, 6 May 2026 21:58:47 +0100 Subject: [PATCH 02/12] fix(api): wire certificate decryption for Mode=Certificate and Mode=Redis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CertificateKeyEncryptionConfigurator only set XmlEncryptor; decryption fell back to a default that looks up certificates in the OS cert store, but the providers load with EphemeralKeySet, so any cross-host scenario silently lost access to existing key XML at startup. DataProtectionModule now eagerly loads the cert provider and calls UnprotectKeysWithAnyCertificate(current + previous), the only public surface for wiring XmlKeyDecryptionOptions. Adds DataProtection:PreviousCertificatePaths and DataProtection:PreviousAzureBlobUrls to support certificate rotation without stranding key XML written under the previous cert. Also switches IDataProtectionCertificateProvider from async to sync — every consumer is sync at the DataProtection wiring boundary, and AzureBlob now uses BlobClient.DownloadContent (sync) rather than blocking on DownloadContentAsync. --- .../AzureBlobCertificateProvider.cs | 63 +++++++++++++------ .../CertificateKeyEncryptionConfigurator.cs | 32 +++++----- .../Certificate/CertificateStartupLogger.cs | 10 +-- .../FileSystemCertificateProvider.cs | 53 ++++++++++------ .../IDataProtectionCertificateProvider.cs | 13 ++-- .../DataProtection/DataProtectionModule.cs | 24 ++++++- 6 files changed, 129 insertions(+), 66 deletions(-) diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs index 776425fe..2c511533 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs @@ -7,39 +7,62 @@ namespace GroundControl.Api.Core.DataProtection.Certificate; /// /// Downloads X.509 certificates from Azure Blob Storage using . /// -internal sealed partial class AzureBlobCertificateProvider( - IConfiguration configuration, - ILogger logger) : IDataProtectionCertificateProvider +/// +/// Uses the Azure SDK's synchronous BlobClient.DownloadContent API rather than +/// blocking on the async overload. The provider is invoked once at host startup, where the +/// blocking download is acceptable, and avoiding sync-over-async eliminates any deadlock risk +/// regardless of synchronization context. +/// +internal sealed partial class AzureBlobCertificateProvider : IDataProtectionCertificateProvider { + private readonly IConfiguration _configuration; + private readonly ILogger _logger; + + public AzureBlobCertificateProvider(IConfiguration configuration, + ILogger logger) + { + _configuration = configuration; + _logger = logger; + } + private static readonly DefaultAzureCredential Credential = new(); /// - public async Task GetCurrentCertificateAsync(CancellationToken cancellationToken = default) + public X509Certificate2 GetCurrentCertificate() + { + var blobUrl = _configuration["DataProtection:AzureBlobUrl"] ?? throw new InvalidOperationException("DataProtection:AzureBlobUrl is required."); + return DownloadCertificate(blobUrl, "AzureBlob"); + } + + /// + public IReadOnlyList GetPreviousCertificates() { - var blobUrl = configuration["DataProtection:AzureBlobUrl"] - ?? throw new InvalidOperationException("DataProtection:AzureBlobUrl is required."); + var urls = _configuration.GetSection("DataProtection:PreviousAzureBlobUrls").Get() ?? []; + if (urls.Length == 0) + { + return []; + } - var password = configuration["DataProtection:CertificatePassword"]; + var certificates = new List(urls.Length); + certificates.AddRange(urls.Select(url => DownloadCertificate(url, "AzureBlob (previous)"))); + + return certificates; + } + + private X509Certificate2 DownloadCertificate(string blobUrl, string source) + { + var password = _configuration["DataProtection:CertificatePassword"]; var client = new BlobClient(new Uri(blobUrl), Credential); - var response = await client.DownloadContentAsync(cancellationToken).ConfigureAwait(false); + var response = client.DownloadContent(); var pfxBytes = response.Value.Content.ToArray(); - var certificate = X509CertificateLoader.LoadPkcs12( - pfxBytes, - password, - X509KeyStorageFlags.EphemeralKeySet); - - LogCertificateLoaded(logger, "AzureBlob", certificate.Thumbprint); + var certificate = X509CertificateLoader.LoadPkcs12(pfxBytes, password, X509KeyStorageFlags.EphemeralKeySet); + LogCertificateLoaded(_logger, source, certificate.Thumbprint); return certificate; } - /// - public Task> GetPreviousCertificatesAsync( - CancellationToken cancellationToken = default) - => Task.FromResult>([]); - [LoggerMessage(1, LogLevel.Information, "Loaded certificate from {Source} with thumbprint {Thumbprint}.")] private static partial void LogCertificateLoaded(ILogger logger, string source, string thumbprint); -} \ No newline at end of file +} diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateKeyEncryptionConfigurator.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateKeyEncryptionConfigurator.cs index 964277b7..9fce602d 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateKeyEncryptionConfigurator.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateKeyEncryptionConfigurator.cs @@ -8,25 +8,25 @@ namespace GroundControl.Api.Core.DataProtection.Certificate; /// Configures Data Protection key encryption using X.509 certificates resolved from DI. /// /// -/// -/// This defers certificate loading from service registration time to the first resolution of -/// . ASP.NET Core Data Protection is synchronous by design -/// (see aspnetcore#3548), so the blocking call to the async certificate provider is unavoidable. -/// It is safe because ASP.NET Core has no SynchronizationContext. -/// -/// -/// The certificate provider is resolved from DI with proper logging, replacing the previous -/// approach of manually constructing providers with NullLoggerFactory. -/// +/// Defers certificate loading from service registration time to the first resolution of +/// . The certificate provider is resolved from DI so the +/// configured logger is used instead of NullLoggerFactory. /// -internal sealed class CertificateKeyEncryptionConfigurator( - IDataProtectionCertificateProvider certificateProvider, - ILoggerFactory loggerFactory) : IConfigureOptions +internal sealed class CertificateKeyEncryptionConfigurator : IConfigureOptions { + private readonly IDataProtectionCertificateProvider _certificateProvider; + private readonly ILoggerFactory _loggerFactory; + + public CertificateKeyEncryptionConfigurator(IDataProtectionCertificateProvider certificateProvider, ILoggerFactory loggerFactory) + { + _certificateProvider = certificateProvider; + _loggerFactory = loggerFactory; + } + /// public void Configure(KeyManagementOptions options) { - var certificate = certificateProvider.GetCurrentCertificateAsync().GetAwaiter().GetResult(); - options.XmlEncryptor = new CertificateXmlEncryptor(certificate, loggerFactory); + var certificate = _certificateProvider.GetCurrentCertificate(); + options.XmlEncryptor = new CertificateXmlEncryptor(certificate, _loggerFactory); } -} \ No newline at end of file +} diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateStartupLogger.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateStartupLogger.cs index df1eeedd..dacccd99 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateStartupLogger.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateStartupLogger.cs @@ -4,15 +4,15 @@ namespace GroundControl.Api.Core.DataProtection.Certificate; /// Loads the Data Protection certificate at startup to verify it is accessible /// and to log the certificate thumbprint. /// -internal sealed partial class CertificateStartupLogger( - IDataProtectionCertificateProvider provider, - ILogger logger) : IHostedService +internal sealed partial class CertificateStartupLogger(IDataProtectionCertificateProvider provider, ILogger logger) : IHostedService { /// - public async Task StartAsync(CancellationToken cancellationToken) + public Task StartAsync(CancellationToken cancellationToken) { - using var certificate = await provider.GetCurrentCertificateAsync(cancellationToken).ConfigureAwait(false); + using var certificate = provider.GetCurrentCertificate(); LogCertificateReady(logger, certificate.Thumbprint); + + return Task.CompletedTask; } /// diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateProvider.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateProvider.cs index a109b2dd..e78635e5 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateProvider.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateProvider.cs @@ -5,38 +5,53 @@ namespace GroundControl.Api.Core.DataProtection.Certificate; /// /// Loads X.509 certificates from the local file system. /// -internal sealed partial class FileSystemCertificateProvider( - IConfiguration configuration, - ILogger logger) : IDataProtectionCertificateProvider +internal sealed partial class FileSystemCertificateProvider : IDataProtectionCertificateProvider { + private readonly IConfiguration _configuration; + private readonly ILogger _logger; + + public FileSystemCertificateProvider(IConfiguration configuration, ILogger logger) + { + _configuration = configuration; + _logger = logger; + } + + /// + public X509Certificate2 GetCurrentCertificate() + { + var path = _configuration["DataProtection:CertificatePath"] ?? throw new InvalidOperationException("DataProtection:CertificatePath is required."); + return LoadCertificate(path, _configuration["DataProtection:CertificatePassword"], "FileSystem"); + } + /// - public Task GetCurrentCertificateAsync(CancellationToken cancellationToken = default) + public IReadOnlyList GetPreviousCertificates() { - var path = configuration["DataProtection:CertificatePath"] - ?? throw new InvalidOperationException("DataProtection:CertificatePath is required."); + var paths = _configuration.GetSection("DataProtection:PreviousCertificatePaths").Get() ?? []; + if (paths.Length == 0) + { + return []; + } - var password = configuration["DataProtection:CertificatePassword"]; + var password = _configuration["DataProtection:CertificatePassword"]; + var certificates = new List(paths.Length); + certificates.AddRange(paths.Select(path => LoadCertificate(path, password, "FileSystem (previous)"))); + + return certificates; + } + private X509Certificate2 LoadCertificate(string path, string? password, string source) + { if (!File.Exists(path)) { throw new FileNotFoundException($"Certificate not found at path: {path}"); } - var certificate = X509CertificateLoader.LoadPkcs12FromFile( - path, - password, - X509KeyStorageFlags.EphemeralKeySet); - - LogCertificateLoaded(logger, "FileSystem", certificate.Thumbprint); + var certificate = X509CertificateLoader.LoadPkcs12FromFile(path, password, X509KeyStorageFlags.EphemeralKeySet); + LogCertificateLoaded(_logger, source, certificate.Thumbprint); - return Task.FromResult(certificate); + return certificate; } - /// - public Task> GetPreviousCertificatesAsync( - CancellationToken cancellationToken = default) - => Task.FromResult>([]); - [LoggerMessage(1, LogLevel.Information, "Loaded certificate from {Source} with thumbprint {Thumbprint}.")] private static partial void LogCertificateLoaded(ILogger logger, string source, string thumbprint); } \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/IDataProtectionCertificateProvider.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/IDataProtectionCertificateProvider.cs index 1a16ede5..a057896e 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/IDataProtectionCertificateProvider.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/IDataProtectionCertificateProvider.cs @@ -5,19 +5,24 @@ namespace GroundControl.Api.Core.DataProtection.Certificate; /// /// Provides X.509 certificates for Data Protection key ring encryption. /// +/// +/// Implementations are invoked once at host startup, where ASP.NET Core has no +/// SynchronizationContext. The interface is synchronous because every consumer +/// (XmlEncryptor configuration, UnprotectKeysWithAnyCertificate wiring) +/// is itself synchronous, and certificate loading is a one-shot operation that does not +/// benefit from cancellation. +/// public interface IDataProtectionCertificateProvider { /// /// Gets the current certificate used to protect newly created Data Protection keys. /// - /// A token to cancel the operation. /// The active X.509 certificate. - Task GetCurrentCertificateAsync(CancellationToken cancellationToken = default); + X509Certificate2 GetCurrentCertificate(); /// /// Gets previous certificates that can still decrypt keys protected with older certificates. /// - /// A token to cancel the operation. /// A list of previous certificates for key rotation, or an empty list if none exist. - Task> GetPreviousCertificatesAsync(CancellationToken cancellationToken = default); + IReadOnlyList GetPreviousCertificates(); } \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs b/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs index fba00cf1..1ae24a24 100644 --- a/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs +++ b/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs @@ -4,6 +4,7 @@ using GroundControl.Host.Api; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.DataProtection.KeyManagement; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; namespace GroundControl.Api.Core.DataProtection; @@ -20,8 +21,8 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) if (options.CertificateProvider.HasValue) { - RegisterCertificateProvider(builder.Services, options.CertificateProvider.Value) - .AddHostedService(); + RegisterCertificateProvider(builder.Services, options.CertificateProvider.Value); + builder.Services.AddHostedService(); } var keyRingConfigurator = CreateKeyRingConfigurator(options.Mode); @@ -30,6 +31,17 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) if (options.Mode is DataProtectionMode.Certificate or DataProtectionMode.Redis) { builder.Services.AddSingleton, CertificateKeyEncryptionConfigurator>(); + + // The decryption side of certificate-based key ring protection cannot be wired through + // IConfigureOptions because XmlKeyDecryptionOptions is internal to ASP.NET Core. The only + // public surface is UnprotectKeysWithAnyCertificate, which captures the certificates at + // registration time. Loading them here and supplying both current and previous certs is + // required for cross-instance and cross-restart decryption to work, and for safe + // certificate rotation. + var startupCertificateProvider = CreateCertificateProvider(builder.Configuration, options.CertificateProvider!.Value); + var currentCertificate = startupCertificateProvider.GetCurrentCertificate(); + var previousCertificates = startupCertificateProvider.GetPreviousCertificates(); + dataProtectionBuilder.UnprotectKeysWithAnyCertificate([currentCertificate, .. previousCertificates]); } builder.Services.AddSingleton(); @@ -52,4 +64,12 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) _ => throw new InvalidOperationException( $"Unknown DataProtection:CertificateProvider '{mode}'. Supported values: {string.Join(", ", Enum.GetNames())}.") }; + + private static IDataProtectionCertificateProvider CreateCertificateProvider(IConfiguration configuration, CertificateProviderMode mode) => mode switch + { + CertificateProviderMode.FileSystem => new FileSystemCertificateProvider(configuration, NullLogger.Instance), + CertificateProviderMode.AzureBlob => new AzureBlobCertificateProvider(configuration, NullLogger.Instance), + _ => throw new InvalidOperationException( + $"Unknown DataProtection:CertificateProvider '{mode}'. Supported values: {string.Join(", ", Enum.GetNames())}.") + }; } \ No newline at end of file From 4684e333b5dfc82956d1e001cae12f9aad616139 Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Wed, 6 May 2026 21:59:04 +0100 Subject: [PATCH 03/12] test(api): add Data Protection lifecycle integration tests Covers key persistence across API host restarts, key rotation, and certificate rotation for both FileSystem and Redis modes. Adds a Testcontainers-backed Redis fixture and a shared base class for the new lifecycle suite. Updates the existing FileSystem provider and CertificateKeyEncryptionConfigurator unit tests for the new synchronous IDataProtectionCertificateProvider interface. --- Directory.Packages.props | 1 + ...rtificateKeyEncryptionConfiguratorTests.cs | 10 +- .../FileSystemCertificateProviderTests.cs | 56 +++++++---- .../Lifecycle/CertificateRotationTests.cs | 94 +++++++++++++++++++ .../DataProtectionLifecycleTestBase.cs | 93 ++++++++++++++++++ .../FileSystemKeyPersistenceTests.cs | 58 ++++++++++++ .../Lifecycle/FileSystemKeyRotationTests.cs | 75 +++++++++++++++ .../Lifecycle/Redis/RedisAssemblyFixture.cs | 4 + .../Redis/RedisCertificateRotationTests.cs | 74 +++++++++++++++ .../Lifecycle/Redis/RedisFixture.cs | 20 ++++ .../Redis/RedisKeyPersistenceTests.cs | 67 +++++++++++++ .../Lifecycle/Redis/RedisKeyRotationTests.cs | 82 ++++++++++++++++ .../Lifecycle/SelfSignedCertificate.cs | 36 +++++++ .../GroundControl.Api.Tests.csproj | 1 + 14 files changed, 649 insertions(+), 22 deletions(-) create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/DataProtectionLifecycleTestBase.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemKeyPersistenceTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemKeyRotationTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisAssemblyFixture.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisFixture.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyPersistenceTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyRotationTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/SelfSignedCertificate.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 3379297c..f288a302 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -58,6 +58,7 @@ + diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/CertificateKeyEncryptionConfiguratorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/CertificateKeyEncryptionConfiguratorTests.cs index 0c2d92e0..41083350 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/CertificateKeyEncryptionConfiguratorTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/CertificateKeyEncryptionConfiguratorTests.cs @@ -19,8 +19,7 @@ public void Configure_SetsXmlEncryptorOnKeyManagementOptions() // Arrange var certificate = CreateSelfSignedCertificate(); var provider = Substitute.For(); - provider.GetCurrentCertificateAsync(Arg.Any()) - .Returns(certificate); + provider.GetCurrentCertificate().Returns(certificate); var configurator = new CertificateKeyEncryptionConfigurator(provider, NullLoggerFactory.Instance); var options = new KeyManagementOptions(); @@ -33,13 +32,12 @@ public void Configure_SetsXmlEncryptorOnKeyManagementOptions() } [Fact] - public void Configure_CallsGetCurrentCertificateAsync() + public void Configure_CallsGetCurrentCertificate() { // Arrange var certificate = CreateSelfSignedCertificate(); var provider = Substitute.For(); - provider.GetCurrentCertificateAsync(Arg.Any()) - .Returns(certificate); + provider.GetCurrentCertificate().Returns(certificate); var configurator = new CertificateKeyEncryptionConfigurator(provider, NullLoggerFactory.Instance); var options = new KeyManagementOptions(); @@ -48,7 +46,7 @@ public void Configure_CallsGetCurrentCertificateAsync() configurator.Configure(options); // Assert - provider.Received(1).GetCurrentCertificateAsync(Arg.Any()); + provider.Received(1).GetCurrentCertificate(); } public void Dispose() diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemCertificateProviderTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemCertificateProviderTests.cs index a45eed90..9886b975 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemCertificateProviderTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemCertificateProviderTests.cs @@ -20,7 +20,7 @@ public FileSystemCertificateProviderTests() } [Fact] - public async Task GetCurrentCertificateAsync_LoadsValidPfxWithoutPassword() + public void GetCurrentCertificate_LoadsValidPfxWithoutPassword() { // Arrange var pfxPath = CreateTestCertificate(password: null); @@ -28,7 +28,7 @@ public async Task GetCurrentCertificateAsync_LoadsValidPfxWithoutPassword() var provider = new FileSystemCertificateProvider(configuration, _logger); // Act - var certificate = await provider.GetCurrentCertificateAsync(TestContext.Current.CancellationToken); + var certificate = provider.GetCurrentCertificate(); // Assert certificate.ShouldNotBeNull(); @@ -37,7 +37,7 @@ public async Task GetCurrentCertificateAsync_LoadsValidPfxWithoutPassword() } [Fact] - public async Task GetCurrentCertificateAsync_LoadsValidPfxWithPassword() + public void GetCurrentCertificate_LoadsValidPfxWithPassword() { // Arrange var password = "test-password-123"; @@ -46,7 +46,7 @@ public async Task GetCurrentCertificateAsync_LoadsValidPfxWithPassword() var provider = new FileSystemCertificateProvider(configuration, _logger); // Act - var certificate = await provider.GetCurrentCertificateAsync(TestContext.Current.CancellationToken); + var certificate = provider.GetCurrentCertificate(); // Assert certificate.ShouldNotBeNull(); @@ -55,19 +55,18 @@ public async Task GetCurrentCertificateAsync_LoadsValidPfxWithPassword() } [Fact] - public async Task GetCurrentCertificateAsync_ThrowsFileNotFoundException_WhenPathDoesNotExist() + public void GetCurrentCertificate_ThrowsFileNotFoundException_WhenPathDoesNotExist() { // Arrange var configuration = BuildConfiguration("/nonexistent/path/cert.pfx", password: null); var provider = new FileSystemCertificateProvider(configuration, _logger); // Act & Assert - await Should.ThrowAsync( - () => provider.GetCurrentCertificateAsync(TestContext.Current.CancellationToken)); + Should.Throw(() => provider.GetCurrentCertificate()); } [Fact] - public async Task GetCurrentCertificateAsync_ThrowsCryptographicException_WhenPasswordIsWrong() + public void GetCurrentCertificate_ThrowsCryptographicException_WhenPasswordIsWrong() { // Arrange var pfxPath = CreateTestCertificate(password: "correct-password"); @@ -75,38 +74,55 @@ public async Task GetCurrentCertificateAsync_ThrowsCryptographicException_WhenPa var provider = new FileSystemCertificateProvider(configuration, _logger); // Act & Assert - await Should.ThrowAsync( - () => provider.GetCurrentCertificateAsync(TestContext.Current.CancellationToken)); + Should.Throw(() => provider.GetCurrentCertificate()); } [Fact] - public async Task GetCurrentCertificateAsync_ThrowsInvalidOperationException_WhenPathNotConfigured() + public void GetCurrentCertificate_ThrowsInvalidOperationException_WhenPathNotConfigured() { // Arrange var configuration = new ConfigurationBuilder().Build(); var provider = new FileSystemCertificateProvider(configuration, _logger); // Act & Assert - var exception = await Should.ThrowAsync( - () => provider.GetCurrentCertificateAsync(TestContext.Current.CancellationToken)); + var exception = Should.Throw(() => provider.GetCurrentCertificate()); exception.Message.ShouldContain("DataProtection:CertificatePath"); } [Fact] - public async Task GetPreviousCertificatesAsync_ReturnsEmptyList() + public void GetPreviousCertificates_ReturnsEmptyList_WhenNotConfigured() { // Arrange var configuration = new ConfigurationBuilder().Build(); var provider = new FileSystemCertificateProvider(configuration, _logger); // Act - var result = await provider.GetPreviousCertificatesAsync(TestContext.Current.CancellationToken); + var result = provider.GetPreviousCertificates(); // Assert result.ShouldBeEmpty(); } - private static IConfiguration BuildConfiguration(string path, string? password) + [Fact] + public void GetPreviousCertificates_LoadsConfiguredPaths() + { + // Arrange + var currentPath = CreateTestCertificate(password: null); + var firstPreviousPath = CreateTestCertificate(password: null); + var secondPreviousPath = CreateTestCertificate(password: null); + var configuration = BuildConfiguration(currentPath, password: null, previousPaths: [firstPreviousPath, secondPreviousPath]); + var provider = new FileSystemCertificateProvider(configuration, _logger); + + // Act + var result = provider.GetPreviousCertificates(); + + // Assert + result.Count.ShouldBe(2); + result[0].HasPrivateKey.ShouldBeTrue(); + result[1].HasPrivateKey.ShouldBeTrue(); + } + + private static IConfiguration BuildConfiguration(string path, string? password, IReadOnlyList? previousPaths = null) { var configValues = new Dictionary { @@ -118,6 +134,14 @@ private static IConfiguration BuildConfiguration(string path, string? password) configValues["DataProtection:CertificatePassword"] = password; } + if (previousPaths is not null) + { + for (var i = 0; i < previousPaths.Count; i++) + { + configValues[$"DataProtection:PreviousCertificatePaths:{i}"] = previousPaths[i]; + } + } + return new ConfigurationBuilder() .AddInMemoryCollection(configValues) .Build(); diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs new file mode 100644 index 00000000..cda69c69 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs @@ -0,0 +1,94 @@ +using GroundControl.Api.Features.ConfigEntries.Contracts; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle; + +/// +/// Verifies that rotating the X.509 certificate that protects the Data Protection key ring +/// does not strand sensitive values written before the rotation. Factory A boots with cert +/// C1 as the current certificate; Factory B boots with C2 as current and C1 in +/// DataProtection:PreviousCertificatePaths so the key XML protected by C1 remains +/// decryptable. +/// +public sealed class CertificateRotationTests : DataProtectionLifecycleTestBase +{ + private readonly string _keyStorePath; + private readonly string _certificateDir; + + public CertificateRotationTests(MongoFixture mongoFixture) + : base(mongoFixture) + { + _keyStorePath = AllocateTempDirectory("gc-keys"); + _certificateDir = AllocateTempDirectory("gc-certs"); + } + + [Fact] + public async Task PreRotationEntry_RemainsDecryptable_AfterCertificateSwap() + { + // Arrange — Cert C1 protects the key ring while Factory A creates a sensitive entry. + var c1Path = SelfSignedCertificate.CreatePfxFile(_certificateDir, "c1.pfx"); + Guid preRotationId; + + await using (var factoryA = CreateLifecycleFactory(currentCertificatePath: c1Path, previousCertificatePaths: [])) + using (var clientA = factoryA.CreateClient()) + { + var created = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "before-cert-rotation"); + preRotationId = created.Id; + + Directory.GetFiles(_keyStorePath, "*.xml").ShouldNotBeEmpty( + "key ring XML should have been written under C1 before the rotation"); + } + + // Act — Cert C2 is now current with C1 in the previous list; Factory B reads the entry. + var c2Path = SelfSignedCertificate.CreatePfxFile(_certificateDir, "c2.pfx"); + await using var factoryB = CreateLifecycleFactory(currentCertificatePath: c2Path, previousCertificatePaths: [c1Path]); + using var clientB = factoryB.CreateClient(); + + var response = await clientB.GetAsync($"/api/config-entries/{preRotationId}?decrypt=true", TestCancellationToken); + + // Assert — entry written under C1 must still decrypt now that C2 is the current cert + // and C1 has been moved to the previous list. + response.IsSuccessStatusCode.ShouldBeTrue($"GET should succeed, but returned {response.StatusCode}."); + var body = await ReadRequiredJsonAsync(response, TestCancellationToken); + body.Values.ShouldHaveSingleItem().Value.ShouldBe("before-cert-rotation"); + } + + [Fact] + public async Task PostRotationEntry_RoundTrips_UnderNewCertificateOnly() + { + // Arrange — Boot directly with C2 only; nothing was ever encrypted with C1. + var c2Path = SelfSignedCertificate.CreatePfxFile(_certificateDir, "c2-only.pfx"); + + // Act + await using var factory = CreateLifecycleFactory(currentCertificatePath: c2Path, previousCertificatePaths: []); + using var client = factory.CreateClient(); + + var created = await CreateSensitiveConfigEntryAsync(client, "api.token", "after-cert-rotation"); + var response = await client.GetAsync($"/api/config-entries/{created.Id}?decrypt=true", TestCancellationToken); + + // Assert + response.IsSuccessStatusCode.ShouldBeTrue(); + var body = await ReadRequiredJsonAsync(response, TestCancellationToken); + body.Values.ShouldHaveSingleItem().Value.ShouldBe("after-cert-rotation"); + } + + private GroundControlApiFactory CreateLifecycleFactory(string currentCertificatePath, IReadOnlyList previousCertificatePaths) + { + var config = new Dictionary + { + ["Persistence:MongoDb:DatabaseName"] = DatabaseName, + ["DataProtection:Mode"] = "Certificate", + ["DataProtection:CertificateProvider"] = "FileSystem", + ["DataProtection:CertificatePath"] = currentCertificatePath, + ["DataProtection:KeyStorePath"] = _keyStorePath + }; + + for (var i = 0; i < previousCertificatePaths.Count; i++) + { + config[$"DataProtection:PreviousCertificatePaths:{i}"] = previousCertificatePaths[i]; + } + + return CreateFactory(config); + } +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/DataProtectionLifecycleTestBase.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/DataProtectionLifecycleTestBase.cs new file mode 100644 index 00000000..9aa902a5 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/DataProtectionLifecycleTestBase.cs @@ -0,0 +1,93 @@ +using System.Net.Http.Json; +using GroundControl.Api.Features.ConfigEntries.Contracts; +using GroundControl.Persistence.Contracts; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle; + +/// +/// Shared infrastructure for Data Protection lifecycle integration tests. Provides a stable +/// MongoDB database name (so two factory instantiations can simulate a host restart against +/// the same persisted state), a small helper to allocate temp directories with deferred +/// cleanup, and a helper for the most common HTTP shape these tests perform — creating a +/// sensitive config entry and parsing the response. +/// +public abstract class DataProtectionLifecycleTestBase : ApiHandlerTestBase, IDisposable +{ + private readonly List _tempDirectories = []; + + protected DataProtectionLifecycleTestBase(MongoFixture mongoFixture) + : base(mongoFixture) + { + } + + /// + /// Database name reused by every factory the test creates so a "second host" sees the + /// MongoDB state written by the first. + /// + protected string DatabaseName { get; } = $"groundcontrol_test_{Guid.CreateVersion7():N}"; + + /// + /// Returns a fresh path under the system temp directory and registers it for cleanup + /// when the test class is disposed. The directory itself is created lazily by the code + /// that writes to it (e.g. ASP.NET Data Protection key persistence, certificate file writes). + /// + protected string AllocateTempDirectory(string prefix) + { + var path = Path.Combine(Path.GetTempPath(), $"{prefix}-{Guid.NewGuid():N}"); + _tempDirectories.Add(path); + return path; + } + + /// + /// POSTs a sensitive and returns the parsed response. The owning + /// project id is generated per call because these tests do not need a real owner — they + /// only exercise persistence and decryption of the sensitive value. + /// + internal static async Task CreateSensitiveConfigEntryAsync(HttpClient client, string key, string value) + { + var request = new CreateConfigEntryRequest + { + Key = key, + OwnerId = Guid.CreateVersion7(), + OwnerType = ConfigEntryOwnerType.Project, + ValueType = "String", + Values = [new ScopedValueRequest { Value = value }], + IsSensitive = true + }; + + var response = await client.PostAsJsonAsync("/api/config-entries", request, WebJsonSerializerOptions, TestCancellationToken); + response.EnsureSuccessStatusCode(); + return await ReadRequiredJsonAsync(response, TestCancellationToken); + } + + public void Dispose() + { + Dispose(disposing: true); + GC.SuppressFinalize(this); + } + + protected virtual void Dispose(bool disposing) + { + if (!disposing) + { + return; + } + + foreach (var path in _tempDirectories) + { + if (!Directory.Exists(path)) + { + continue; + } + + try + { + Directory.Delete(path, recursive: true); + } + catch (IOException) + { + // Disk locks (AV scans, lingering file handles) should not fail an otherwise green test. + } + } + } +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemKeyPersistenceTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemKeyPersistenceTests.cs new file mode 100644 index 00000000..d855bca0 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemKeyPersistenceTests.cs @@ -0,0 +1,58 @@ +using GroundControl.Api.Features.ConfigEntries.Contracts; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle; + +/// +/// Verifies that sensitive configuration values written under one API host instance +/// remain decryptable after the host is disposed and a fresh host is started against +/// the same on-disk key store and MongoDB database. This is the foundational guarantee +/// for any deployment that survives a process restart. +/// +public sealed class FileSystemKeyPersistenceTests : DataProtectionLifecycleTestBase +{ + private readonly string _keyStorePath; + + public FileSystemKeyPersistenceTests(MongoFixture mongoFixture) + : base(mongoFixture) + { + _keyStorePath = AllocateTempDirectory("gc-keys"); + } + + [Fact] + public async Task SensitiveValue_RemainsDecryptable_AfterApiHostRestart() + { + // Arrange — Factory A creates a sensitive entry under the shared key store + DB + Guid createdId; + + await using (var factoryA = CreateLifecycleFactory()) + using (var clientA = factoryA.CreateClient()) + { + var created = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "s3cret!"); + createdId = created.Id; + + Directory.GetFiles(_keyStorePath, "*.xml").ShouldNotBeEmpty( + "FileSystem mode should have persisted at least one key XML file before restart"); + } + + // Act — Factory B is a fresh host pointed at the same key store + DB + await using var factoryB = CreateLifecycleFactory(); + using var clientB = factoryB.CreateClient(); + + var response = await clientB.GetAsync($"/api/config-entries/{createdId}?decrypt=true", TestCancellationToken); + + // Assert — Factory B reads back the plaintext written under Factory A + response.IsSuccessStatusCode.ShouldBeTrue(); + var body = await ReadRequiredJsonAsync(response, TestCancellationToken); + body.IsSensitive.ShouldBeTrue(); + body.Values.ShouldHaveSingleItem().Value.ShouldBe("s3cret!"); + } + + private GroundControlApiFactory CreateLifecycleFactory() => CreateFactory(new Dictionary + { + ["Persistence:MongoDb:DatabaseName"] = DatabaseName, + ["DataProtection:Mode"] = "FileSystem", + ["DataProtection:KeyStorePath"] = _keyStorePath + }); +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemKeyRotationTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemKeyRotationTests.cs new file mode 100644 index 00000000..b72157d9 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemKeyRotationTests.cs @@ -0,0 +1,75 @@ +using GroundControl.Api.Features.ConfigEntries.Contracts; +using Microsoft.AspNetCore.DataProtection.KeyManagement; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle; + +/// +/// Verifies that adding a new Data Protection key to the ring does not render existing +/// sensitive values unreadable. The key id is embedded in the ciphertext, so old values +/// must continue to decrypt after a rotation; new values are protected with the new key. +/// +public sealed class FileSystemKeyRotationTests : DataProtectionLifecycleTestBase +{ + private readonly string _keyStorePath; + + public FileSystemKeyRotationTests(MongoFixture mongoFixture) + : base(mongoFixture) + { + _keyStorePath = AllocateTempDirectory("gc-keys"); + } + + [Fact] + public async Task PreRotationEntry_RemainsDecryptable_AfterNewKeyAddedAndHostRestart() + { + Guid preRotationId; + + // Arrange — Factory A creates a sensitive entry under K1 then forces a rotation by + // adding K2 to the ring with an activation date in the past so it is immediately eligible. + await using (var factoryA = CreateLifecycleFactory()) + using (var clientA = factoryA.CreateClient()) + { + var preRotation = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "before-rotation"); + preRotationId = preRotation.Id; + + var keyCountBeforeRotation = Directory.GetFiles(_keyStorePath, "*.xml").Length; + keyCountBeforeRotation.ShouldBeGreaterThanOrEqualTo(1); + + var keyManager = factoryA.Services.GetRequiredService(); + keyManager.CreateNewKey( + activationDate: DateTimeOffset.UtcNow.AddHours(-1), + expirationDate: DateTimeOffset.UtcNow.AddDays(90)); + + Directory.GetFiles(_keyStorePath, "*.xml").Length.ShouldBeGreaterThan( + keyCountBeforeRotation, + "creating a new key should have written an additional key XML file"); + } + + // Act — Factory B is a fresh host pointed at the same key store. It should both + // (a) decrypt the pre-rotation entry and (b) create new entries under the new key. + await using var factoryB = CreateLifecycleFactory(); + using var clientB = factoryB.CreateClient(); + + var preRotationResponse = await clientB.GetAsync($"/api/config-entries/{preRotationId}?decrypt=true", TestCancellationToken); + var postRotation = await CreateSensitiveConfigEntryAsync(clientB, "api.token", "after-rotation"); + var postRotationResponse = await clientB.GetAsync($"/api/config-entries/{postRotation.Id}?decrypt=true", TestCancellationToken); + + // Assert — both entries decrypt to their original plaintext + preRotationResponse.IsSuccessStatusCode.ShouldBeTrue(); + var preBody = await ReadRequiredJsonAsync(preRotationResponse, TestCancellationToken); + preBody.Values.ShouldHaveSingleItem().Value.ShouldBe("before-rotation"); + + postRotationResponse.IsSuccessStatusCode.ShouldBeTrue(); + var postBody = await ReadRequiredJsonAsync(postRotationResponse, TestCancellationToken); + postBody.Values.ShouldHaveSingleItem().Value.ShouldBe("after-rotation"); + } + + private GroundControlApiFactory CreateLifecycleFactory() => CreateFactory(new Dictionary + { + ["Persistence:MongoDb:DatabaseName"] = DatabaseName, + ["DataProtection:Mode"] = "FileSystem", + ["DataProtection:KeyStorePath"] = _keyStorePath + }); +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisAssemblyFixture.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisAssemblyFixture.cs new file mode 100644 index 00000000..6b262711 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisAssemblyFixture.cs @@ -0,0 +1,4 @@ +using GroundControl.Api.Tests.Core.DataProtection.Lifecycle.Redis; +using Xunit; + +[assembly: AssemblyFixture(typeof(RedisFixture))] \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs new file mode 100644 index 00000000..bf638f98 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs @@ -0,0 +1,74 @@ +using GroundControl.Api.Features.ConfigEntries.Contracts; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle.Redis; + +/// +/// Redis equivalent of : rotating the X.509 certificate +/// that protects a Redis-backed key ring must not strand sensitive values written before the +/// rotation. Mode=Redis shares the certificate-protection wiring with Mode=Certificate, so +/// this test guards against regressions on either mode. +/// +public sealed class RedisCertificateRotationTests : DataProtectionLifecycleTestBase +{ + private readonly RedisFixture _redisFixture; + private readonly string _redisKeyName = $"groundcontrol-test-keys-{Guid.NewGuid():N}"; + private readonly string _certificateDir; + + public RedisCertificateRotationTests(MongoFixture mongoFixture, RedisFixture redisFixture) + : base(mongoFixture) + { + _redisFixture = redisFixture; + _certificateDir = AllocateTempDirectory("gc-certs"); + } + + [Fact] + public async Task PreRotationEntry_RemainsDecryptable_AfterCertificateSwap() + { + // Arrange — Cert C1 protects the Redis-backed key ring while Factory A writes a + // sensitive entry; the resulting key XML lives in Redis encrypted with C1. + var c1Path = SelfSignedCertificate.CreatePfxFile(_certificateDir, "c1.pfx"); + Guid preRotationId; + + await using (var factoryA = CreateLifecycleFactory(currentCertificatePath: c1Path, previousCertificatePaths: [])) + { + using var clientA = factoryA.CreateClient(); + var created = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "before-cert-rotation"); + preRotationId = created.Id; + } + + // Act — Cert C2 is now current with C1 in the previous list; Factory B reads the entry. + var c2Path = SelfSignedCertificate.CreatePfxFile(_certificateDir, "c2.pfx"); + await using var factoryB = CreateLifecycleFactory(currentCertificatePath: c2Path, previousCertificatePaths: [c1Path]); + using var clientB = factoryB.CreateClient(); + + var response = await clientB.GetAsync($"/api/config-entries/{preRotationId}?decrypt=true", TestCancellationToken); + + // Assert — entry written under C1 must still decrypt now that C2 is the current cert + // and C1 has been moved to the previous list. + response.IsSuccessStatusCode.ShouldBeTrue($"GET should succeed, but returned {response.StatusCode}."); + var body = await ReadRequiredJsonAsync(response, TestCancellationToken); + body.Values.ShouldHaveSingleItem().Value.ShouldBe("before-cert-rotation"); + } + + private GroundControlApiFactory CreateLifecycleFactory(string currentCertificatePath, IReadOnlyList previousCertificatePaths) + { + var config = new Dictionary + { + ["Persistence:MongoDb:DatabaseName"] = DatabaseName, + ["DataProtection:Mode"] = "Redis", + ["DataProtection:CertificateProvider"] = "FileSystem", + ["DataProtection:CertificatePath"] = currentCertificatePath, + ["DataProtection:Redis:ConnectionString"] = _redisFixture.ConnectionString, + ["DataProtection:Redis:KeyName"] = _redisKeyName + }; + + for (var i = 0; i < previousCertificatePaths.Count; i++) + { + config[$"DataProtection:PreviousCertificatePaths:{i}"] = previousCertificatePaths[i]; + } + + return CreateFactory(config); + } +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisFixture.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisFixture.cs new file mode 100644 index 00000000..84e3f96d --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisFixture.cs @@ -0,0 +1,20 @@ +using Testcontainers.Redis; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle.Redis; + +/// +/// Shared assembly-level fixture that boots a Redis container for Data Protection +/// lifecycle tests. The same container is reused across tests to amortise startup cost; +/// each test isolates itself by using a unique KeyName on the same Redis instance. +/// +public sealed class RedisFixture : IAsyncLifetime +{ + private readonly RedisContainer _container = new RedisBuilder("redis:7-alpine").Build(); + + public string ConnectionString => _container.GetConnectionString(); + + public async ValueTask InitializeAsync() => await _container.StartAsync().ConfigureAwait(false); + + public async ValueTask DisposeAsync() => await _container.DisposeAsync().ConfigureAwait(false); +} \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyPersistenceTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyPersistenceTests.cs new file mode 100644 index 00000000..84e1cff8 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyPersistenceTests.cs @@ -0,0 +1,67 @@ +using GroundControl.Api.Features.ConfigEntries.Contracts; +using Shouldly; +using StackExchange.Redis; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle.Redis; + +/// +/// Verifies that the Redis-backed Data Protection key ring lets a freshly started API host +/// decrypt sensitive values that were written under a previous host instance, both for the +/// horizontally-scaled deployment scenario (two instances sharing one Redis) and the simple +/// "process restart" scenario. +/// +public sealed class RedisKeyPersistenceTests : DataProtectionLifecycleTestBase +{ + private readonly RedisFixture _redisFixture; + private readonly string _redisKeyName = $"groundcontrol-test-keys-{Guid.NewGuid():N}"; + private readonly string _certificatePath; + + public RedisKeyPersistenceTests(MongoFixture mongoFixture, RedisFixture redisFixture) + : base(mongoFixture) + { + _redisFixture = redisFixture; + _certificatePath = SelfSignedCertificate.CreatePfxFile(AllocateTempDirectory("gc-certs"), "dp.pfx", password: null); + } + + [Fact] + public async Task SensitiveValue_RemainsDecryptable_AfterApiHostRestart() + { + // Arrange — Factory A protects a value under the Redis-backed key ring. + Guid createdId; + + await using (var factoryA = CreateLifecycleFactory()) + using (var clientA = factoryA.CreateClient()) + { + var created = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "redis-secret"); + createdId = created.Id; + + // Sanity — at least one key XML must now live in Redis under the configured key name. + await using var multiplexer = await ConnectionMultiplexer.ConnectAsync(_redisFixture.ConnectionString); + (await multiplexer.GetDatabase().ListLengthAsync(_redisKeyName)) + .ShouldBeGreaterThan(0, $"Redis list '{_redisKeyName}' should hold at least one key XML entry"); + } + + // Act — Factory B is a fresh host pointed at the same Redis + Mongo + cert. + await using var factoryB = CreateLifecycleFactory(); + using var clientB = factoryB.CreateClient(); + + var response = await clientB.GetAsync($"/api/config-entries/{createdId}?decrypt=true", TestCancellationToken); + + // Assert — Factory B reads back the plaintext written under Factory A. + response.IsSuccessStatusCode.ShouldBeTrue(); + var body = await ReadRequiredJsonAsync(response, TestCancellationToken); + body.IsSensitive.ShouldBeTrue(); + body.Values.ShouldHaveSingleItem().Value.ShouldBe("redis-secret"); + } + + private GroundControlApiFactory CreateLifecycleFactory() => CreateFactory(new Dictionary + { + ["Persistence:MongoDb:DatabaseName"] = DatabaseName, + ["DataProtection:Mode"] = "Redis", + ["DataProtection:CertificateProvider"] = "FileSystem", + ["DataProtection:CertificatePath"] = _certificatePath, + ["DataProtection:Redis:ConnectionString"] = _redisFixture.ConnectionString, + ["DataProtection:Redis:KeyName"] = _redisKeyName + }); +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyRotationTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyRotationTests.cs new file mode 100644 index 00000000..12d0d594 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyRotationTests.cs @@ -0,0 +1,82 @@ +using GroundControl.Api.Features.ConfigEntries.Contracts; +using Microsoft.AspNetCore.DataProtection.KeyManagement; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; +using StackExchange.Redis; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle.Redis; + +/// +/// Redis equivalent of the FileSystem key-rotation test. Adding a new Data Protection key +/// to a Redis-backed ring must not strand existing sensitive values; the key id is embedded +/// in the ciphertext, so old values continue to decrypt while new values are protected with +/// the freshly added key. +/// +public sealed class RedisKeyRotationTests : DataProtectionLifecycleTestBase +{ + private readonly RedisFixture _redisFixture; + private readonly string _redisKeyName = $"groundcontrol-test-keys-{Guid.NewGuid():N}"; + private readonly string _certificatePath; + + public RedisKeyRotationTests(MongoFixture mongoFixture, RedisFixture redisFixture) + : base(mongoFixture) + { + _redisFixture = redisFixture; + _certificatePath = SelfSignedCertificate.CreatePfxFile(AllocateTempDirectory("gc-certs"), "dp.pfx", password: null); + } + + [Fact] + public async Task PreRotationEntry_RemainsDecryptable_AfterNewKeyAddedAndHostRestart() + { + Guid preRotationId; + + // Arrange — Factory A creates a sensitive entry under K1 then forces a rotation by + // adding K2 with an activation date in the past so the next host picks it up immediately. + await using (var factoryA = CreateLifecycleFactory()) + using (var clientA = factoryA.CreateClient()) + { + var preRotation = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "before-rotation"); + preRotationId = preRotation.Id; + + await using var multiplexer = await ConnectionMultiplexer.ConnectAsync(_redisFixture.ConnectionString); + var keyCountBefore = await multiplexer.GetDatabase().ListLengthAsync(_redisKeyName); + keyCountBefore.ShouldBeGreaterThanOrEqualTo(1); + + var keyManager = factoryA.Services.GetRequiredService(); + keyManager.CreateNewKey( + activationDate: DateTimeOffset.UtcNow.AddHours(-1), + expirationDate: DateTimeOffset.UtcNow.AddDays(90)); + + (await multiplexer.GetDatabase().ListLengthAsync(_redisKeyName)) + .ShouldBeGreaterThan(keyCountBefore, "creating a new key should append a new entry to Redis"); + } + + // Act — Factory B is a fresh host pointed at the same Redis instance. + await using var factoryB = CreateLifecycleFactory(); + using var clientB = factoryB.CreateClient(); + + var preRotationResponse = await clientB.GetAsync($"/api/config-entries/{preRotationId}?decrypt=true", TestCancellationToken); + var postRotation = await CreateSensitiveConfigEntryAsync(clientB, "api.token", "after-rotation"); + var postRotationResponse = await clientB.GetAsync($"/api/config-entries/{postRotation.Id}?decrypt=true", TestCancellationToken); + + // Assert — both pre- and post-rotation entries decrypt correctly under Factory B. + preRotationResponse.IsSuccessStatusCode.ShouldBeTrue(); + var preBody = await ReadRequiredJsonAsync(preRotationResponse, TestCancellationToken); + preBody.Values.ShouldHaveSingleItem().Value.ShouldBe("before-rotation"); + + postRotationResponse.IsSuccessStatusCode.ShouldBeTrue(); + var postBody = await ReadRequiredJsonAsync(postRotationResponse, TestCancellationToken); + postBody.Values.ShouldHaveSingleItem().Value.ShouldBe("after-rotation"); + } + + private GroundControlApiFactory CreateLifecycleFactory() => CreateFactory(new Dictionary + { + ["Persistence:MongoDb:DatabaseName"] = DatabaseName, + ["DataProtection:Mode"] = "Redis", + ["DataProtection:CertificateProvider"] = "FileSystem", + ["DataProtection:CertificatePath"] = _certificatePath, + ["DataProtection:Redis:ConnectionString"] = _redisFixture.ConnectionString, + ["DataProtection:Redis:KeyName"] = _redisKeyName + }); +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/SelfSignedCertificate.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/SelfSignedCertificate.cs new file mode 100644 index 00000000..782d244a --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/SelfSignedCertificate.cs @@ -0,0 +1,36 @@ +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle; + +/// +/// Generates self-signed X.509 certificates for Data Protection lifecycle tests. +/// +internal static class SelfSignedCertificate +{ + private static X509Certificate2 Create(string subjectName = "CN=GroundControl Test", DateTimeOffset? notBefore = null, DateTimeOffset? notAfter = null) + { + using var rsa = RSA.Create(2048); + var request = new CertificateRequest( + subjectName, + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + return request.CreateSelfSigned( + notBefore ?? DateTimeOffset.UtcNow.AddMinutes(-1), + notAfter ?? DateTimeOffset.UtcNow.AddYears(1)); + } + + public static string CreatePfxFile(string directory, string fileName, string? password = null) + { + Directory.CreateDirectory(directory); + using var certificate = Create(); + + var path = Path.Combine(directory, fileName); + var pfxBytes = certificate.Export(X509ContentType.Pfx, password); + File.WriteAllBytes(path, pfxBytes); + + return path; + } +} \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/GroundControl.Api.Tests.csproj b/tests/GroundControl.Api.Tests/GroundControl.Api.Tests.csproj index e988777f..2f7bb4d3 100644 --- a/tests/GroundControl.Api.Tests/GroundControl.Api.Tests.csproj +++ b/tests/GroundControl.Api.Tests/GroundControl.Api.Tests.csproj @@ -7,6 +7,7 @@ + From 1ee67cff8846c482f0ba762731380f5de97b4b6f Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Wed, 6 May 2026 21:59:15 +0100 Subject: [PATCH 04/12] docs: document Data Protection certificate rotation Updates the configuration guide, security model, and deployment architecture doc to reflect the actual flat configuration keys (CertificatePath, AzureBlobUrl) and the new PreviousCertificatePaths / PreviousAzureBlobUrls arrays. Replaces aspirational nested config examples and adds the rotation workflow. --- docs/design-docs/Deployment-Architecture.md | 23 ++++----- docs/design-docs/Security-Model.md | 57 +++++++++------------ docs/guide/server/configuration.md | 29 ++++++++--- 3 files changed, 57 insertions(+), 52 deletions(-) diff --git a/docs/design-docs/Deployment-Architecture.md b/docs/design-docs/Deployment-Architecture.md index 3a130d2c..14927571 100644 --- a/docs/design-docs/Deployment-Architecture.md +++ b/docs/design-docs/Deployment-Architecture.md @@ -330,18 +330,17 @@ Application configuration for GroundControl server: | `DataProtection:Mode` | `FileSystem` | Key ring configurator: `FileSystem`, `Certificate`, `Redis`, `Azure` | | `DataProtection:KeyStorePath` | `./keys` | File system path for key ring storage (`FileSystem`, `Certificate`) | | `DataProtection:UseDpapi` | `false` | Use DPAPI for key protection (`FileSystem` on Windows only) | -| `DataProtection:CertificateProvider` | `FileSystem` | Certificate provider: `FileSystem`, `AzureBlob` | -| `DataProtection:Certificate:FileSystem:CurrentPath` | — | Current X.509 certificate path (`.pfx`) | -| `DataProtection:Certificate:FileSystem:Password` | — | Certificate password (prefer environment variable or secrets manager) | -| `DataProtection:Certificate:FileSystem:PreviousPaths` | `[]` | Previous certificate paths for decrypting old keys during rotation | -| `DataProtection:Certificate:AzureBlob:ContainerUri` | — | Azure Blob Storage container URI for certificates | -| `DataProtection:Certificate:AzureBlob:CurrentBlobName` | — | Blob name of the current certificate | -| `DataProtection:Certificate:AzureBlob:PreviousBlobNames` | `[]` | Blob names of previous certificates | -| `DataProtection:Certificate:AzureBlob:Password` | — | Certificate password (shared across blobs) | -| `DataProtection:Redis:ConnectionString` | — | Redis connection string (`Redis` configurator) | -| `DataProtection:Redis:KeyName` | `GroundControl-DP-Keys` | Redis key name for key ring storage | -| `DataProtection:Azure:BlobStorageUri` | — | Azure Blob Storage URI for key ring storage | -| `DataProtection:Azure:KeyVaultKeyUri` | — | Azure Key Vault key URI for key ring protection | +| `DataProtection:CertificateProvider` | — | Certificate provider: `FileSystem` or `AzureBlob` (required for `Certificate` and `Redis` modes) | +| `DataProtection:CertificatePath` | — | Path to the current `.pfx` certificate (`FileSystem` provider) | +| `DataProtection:CertificatePassword` | — | Certificate password — shared by current and previous certificates and by the `AzureBlob` provider (prefer environment variable or secrets manager) | +| `DataProtection:PreviousCertificatePaths` | `[]` | Optional array of `.pfx` paths for previously-used certificates, kept available for decrypting key XML written under the prior cert until rotation completes | +| `DataProtection:AzureBlobUrl` | — | Azure Blob URL for the current certificate (`AzureBlob` provider) | +| `DataProtection:PreviousAzureBlobUrls` | `[]` | Optional array of blob URLs for previously-used certificates (`AzureBlob` provider). Same semantics as `PreviousCertificatePaths` | +| `DataProtection:Redis:ConnectionString` | — | Redis connection string (`Redis` mode) | +| `DataProtection:Redis:KeyName` | `groundcontrol-data-protection` | Redis key name for key ring storage | +| `DataProtection:Redis:ConnectTimeoutMs` | `5000` | Redis connection timeout | +| `DataProtection:Azure:BlobUri` | — | Azure Blob Storage URI for key ring storage (`Azure` mode) | +| `DataProtection:Azure:KeyVaultKeyId` | — | Azure Key Vault key URI for key ring protection (`Azure` mode) | | `DataProtection:KeyRotation:Enabled` | `false` | Enable automatic key rotation | | `DataProtection:KeyRotation:KeyLifetime` | `90` | Days before a new key is generated (when rotation is enabled) | | `Cache:PrewarmOnStartup` | `false` | Load all active snapshots into cache at startup (trades memory for guaranteed first-request cache hits) | diff --git a/docs/design-docs/Security-Model.md b/docs/design-docs/Security-Model.md index 9d3b80ee..37820048 100644 --- a/docs/design-docs/Security-Model.md +++ b/docs/design-docs/Security-Model.md @@ -205,7 +205,7 @@ IDataProtectionCertificateProvider └── GetPreviousCertificatesAsync() → IReadOnlyList ``` -`GetCurrentCertificateAsync()` returns the active certificate used by `ProtectKeysWithCertificate()` to encrypt new key ring entries. `GetPreviousCertificatesAsync()` returns any retired certificates passed to `UnprotectKeysWithAnyCertificate()` for decrypting old key ring entries during rotation. Selected via `DataProtection:Certificate:Provider`. +`GetCurrentCertificateAsync()` returns the active certificate used by `CertificateXmlEncryptor` to encrypt new key ring entries. `GetPreviousCertificatesAsync()` returns any retired certificates that, together with the current certificate, are passed to `UnprotectKeysWithAnyCertificate()` so old key ring entries remain decryptable during and after rotation. Selected via `DataProtection:CertificateProvider`. **Implementations:** @@ -222,14 +222,11 @@ IDataProtectionCertificateProvider ```json { "DataProtection": { - "Certificate": { - "Provider": "FileSystem", - "FileSystem": { - "CurrentPath": "/certs/dp-2026.pfx", - "Password": "...", - "PreviousPaths": ["/certs/dp-2024.pfx"] - } - } + "Mode": "Certificate", + "CertificateProvider": "FileSystem", + "CertificatePath": "/certs/dp-2026.pfx", + "CertificatePassword": "...", + "PreviousCertificatePaths": [ "/certs/dp-2024.pfx" ] } } ``` @@ -239,20 +236,16 @@ IDataProtectionCertificateProvider ```json { "DataProtection": { - "Certificate": { - "Provider": "AzureBlob", - "AzureBlob": { - "ContainerUri": "https://account.blob.core.windows.net/certificates", - "CurrentBlobName": "dp-2026.pfx", - "PreviousBlobNames": ["dp-2024.pfx"], - "Password": "..." - } - } + "Mode": "Certificate", + "CertificateProvider": "AzureBlob", + "AzureBlobUrl": "https://account.blob.core.windows.net/certificates/dp-2026.pfx", + "PreviousAzureBlobUrls": [ "https://account.blob.core.windows.net/certificates/dp-2024.pfx" ], + "CertificatePassword": "..." } } ``` -The `AzureBlobCertificateProvider` uses `DefaultAzureCredential` for authentication to the storage account. +The `AzureBlobCertificateProvider` uses `DefaultAzureCredential` for authentication to the storage account. `PreviousAzureBlobUrls` is the AzureBlob equivalent of `PreviousCertificatePaths`; certificates downloaded from those URLs are added to the decryption pipeline so key XML written under a previous certificate remains decryptable during and after rotation. ### Certificate Lifecycle (Key Ring Protection) @@ -277,27 +270,23 @@ However, there is a practical caveat: if configured via **thumbprint** (`Protect 1. Generate a new X.509 certificate. 2. Make the new certificate available to the `IDataProtectionCertificateProvider` (e.g., deploy the file, upload to blob storage). -3. Update configuration: register the new certificate as current and move the old certificate to the previous certificates list (provider-specific config). -4. Perform a rolling restart. The key ring configurator calls `IDataProtectionCertificateProvider` to obtain certificates, then: - - `ProtectKeysWithCertificate(currentCert)` — new data protection keys are encrypted with the new certificate. - - `UnprotectKeysWithAnyCertificate(previousCerts)` — existing keys encrypted with old certificates remain decryptable. -5. After all old data protection keys have expired (90+ days) or been re-encrypted, remove the old certificate from the provider's previous certificates configuration. +3. Update configuration: set the new certificate as `DataProtection:CertificatePath` and add the old certificate to `DataProtection:PreviousCertificatePaths`. +4. Perform a rolling restart. At startup `DataProtectionModule` resolves the provider, loads the current and previous certificates, and: + - Sets `KeyManagementOptions.XmlEncryptor = new CertificateXmlEncryptor(current)` — new data protection keys are encrypted with the new certificate. + - Calls `dataProtectionBuilder.UnprotectKeysWithAnyCertificate(current, ...previous)` — every existing key encrypted under the current or any previous certificate remains decryptable. +5. After all data protection keys protected by the old certificate have expired (90+ days) or been re-encrypted, remove that path from `PreviousCertificatePaths`. **Configuration example (certificate rotation in progress, FileSystem provider):** ```json { "DataProtection": { - "KeyRing": "Certificate", - "KeyStorePath": "./keys", - "Certificate": { - "Provider": "FileSystem", - "FileSystem": { - "CurrentPath": "/certs/dp-2026.pfx", - "Password": "...", - "PreviousPaths": ["/certs/dp-2024.pfx"] - } - }, + "Mode": "Certificate", + "KeyStorePath": "/keys", + "CertificateProvider": "FileSystem", + "CertificatePath": "/certs/dp-2026.pfx", + "CertificatePassword": "...", + "PreviousCertificatePaths": [ "/certs/dp-2024.pfx" ], "KeyRotation": { "Enabled": true, "KeyLifetime": 90 diff --git a/docs/guide/server/configuration.md b/docs/guide/server/configuration.md index 79102990..7724b9cf 100644 --- a/docs/guide/server/configuration.md +++ b/docs/guide/server/configuration.md @@ -62,14 +62,31 @@ Keys are stored as XML files in `KeyStorePath`. Suitable for single-instance dep ### Certificate mode -Keys are stored on the file system and protected with an X.509 certificate. +Keys are stored on the file system and the key XML is encrypted at rest with an X.509 certificate. The same configuration applies under Redis mode (which uses Redis storage but the same certificate-based key protection). | Setting | Description | |---|---| -| `DataProtection:CertificateProvider` | `FileSystem` or `AzureBlob` | -| `DataProtection:CertificatePath` | Path to the .pfx certificate file (FileSystem provider) | -| `DataProtection:CertificatePassword` | Certificate password (FileSystem provider) | -| `DataProtection:CertificateAzureBlobUrl` | Blob URL for certificate download (AzureBlob provider) | +| `DataProtection:CertificateProvider` | `FileSystem` or `AzureBlob`. | +| `DataProtection:CertificatePath` | Path to the current `.pfx` certificate file (`FileSystem` provider). | +| `DataProtection:CertificatePassword` | Certificate password. Shared by current and previous certificates and by the `AzureBlob` provider. | +| `DataProtection:PreviousCertificatePaths` | Optional array of `.pfx` paths for certificates that previously protected the key ring (`FileSystem` provider). Required during certificate rotation so existing key XML remains decryptable until rotated out. | +| `DataProtection:AzureBlobUrl` | Blob URL for the current certificate (`AzureBlob` provider). | +| `DataProtection:PreviousAzureBlobUrls` | Optional array of blob URLs for previously-used certificates (`AzureBlob` provider). Same semantics as `PreviousCertificatePaths`. | + +```json +{ + "DataProtection": { + "Mode": "Certificate", + "KeyStorePath": "/keys", + "CertificateProvider": "FileSystem", + "CertificatePath": "/certs/dp-current.pfx", + "CertificatePassword": "", + "PreviousCertificatePaths": [ "/certs/dp-previous.pfx" ] + } +} +``` + +> **Certificate rotation:** generate the new cert, deploy it as `CertificatePath`, move the old cert into `PreviousCertificatePaths`, and perform a rolling restart. New key ring entries are encrypted with the new cert; entries written under the previous cert remain decryptable as long as that cert stays in the previous list. Remove a cert from `PreviousCertificatePaths` only after every key encrypted with it has expired (90+ days by default) or been re-encrypted — otherwise the data those keys protect becomes permanently unreadable. ### Redis mode @@ -81,7 +98,7 @@ Keys are stored in Redis and protected with an X.509 certificate. Suitable for m | `DataProtection:Redis:KeyName` | `groundcontrol-data-protection` | Redis key name for the key ring. | | `DataProtection:Redis:ConnectTimeoutMs` | `5000` | Connection timeout in milliseconds. | -Also requires a certificate provider (`CertificateProvider`, `CertificatePath`/`CertificateAzureBlobUrl`). +Also requires the same certificate settings as Certificate mode (`CertificateProvider`, `CertificatePath`/`AzureBlobUrl`, optional `PreviousCertificatePaths`). ### Azure mode From 7bb1e4e50e3e391d009ea8ee7f3432b2ddf211a1 Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Wed, 6 May 2026 22:17:37 +0100 Subject: [PATCH 05/12] refactor(api): replace magic-string config reads in Data Protection with options classes Move certificate-provider configuration off raw IConfiguration["..."] reads onto strongly-typed FileSystemCertificateOptions and AzureBlobCertificateOptions with source-generated [OptionsValidator] validators, mirroring the existing Redis/Azure sub-options pattern. The certificate providers now inject IOptions instead of IConfiguration; the dead duplicate properties on DataProtectionOptions are removed; and the parent validator is reduced to a thin mode-conditional dispatcher. Also fix a latent bug where Redis/Azure validation failures were silently swallowed by an inverted TryValidate condition. Schema change (feature is new on this branch): - DataProtection:CertificatePath -> DataProtection:FileSystemCertificate:Path - DataProtection:CertificatePassword -> DataProtection:FileSystemCertificate:Password or DataProtection:AzureBlobCertificate:Password - DataProtection:PreviousCertificatePaths -> DataProtection:FileSystemCertificate:PreviousPaths - DataProtection:AzureBlobUrl -> DataProtection:AzureBlobCertificate:BlobUri - DataProtection:PreviousAzureBlobUrls -> DataProtection:AzureBlobCertificate:PreviousBlobUris --- docs/design-docs/Deployment-Architecture.md | 11 ++-- docs/design-docs/Security-Model.md | 30 +++++---- docs/guide/server/configuration.md | 27 ++++---- .../AzureBlobCertificateOptions.cs | 30 +++++++++ .../AzureBlobCertificateProvider.cs | 36 +++++----- .../FileSystemCertificateOptions.cs | 30 +++++++++ .../FileSystemCertificateProvider.cs | 17 ++--- .../DataProtection/DataProtectionModule.cs | 38 +++++++---- .../DataProtection/DataProtectionOptions.cs | 64 +++++++++--------- .../FileSystemCertificateProviderTests.cs | 66 +++++++------------ .../Lifecycle/CertificateRotationTests.cs | 8 +-- .../Redis/RedisCertificateRotationTests.cs | 4 +- .../Redis/RedisKeyPersistenceTests.cs | 2 +- .../Lifecycle/Redis/RedisKeyRotationTests.cs | 2 +- 14 files changed, 208 insertions(+), 157 deletions(-) create mode 100644 src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateOptions.cs create mode 100644 src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateOptions.cs diff --git a/docs/design-docs/Deployment-Architecture.md b/docs/design-docs/Deployment-Architecture.md index 14927571..2346b9f8 100644 --- a/docs/design-docs/Deployment-Architecture.md +++ b/docs/design-docs/Deployment-Architecture.md @@ -331,11 +331,12 @@ Application configuration for GroundControl server: | `DataProtection:KeyStorePath` | `./keys` | File system path for key ring storage (`FileSystem`, `Certificate`) | | `DataProtection:UseDpapi` | `false` | Use DPAPI for key protection (`FileSystem` on Windows only) | | `DataProtection:CertificateProvider` | — | Certificate provider: `FileSystem` or `AzureBlob` (required for `Certificate` and `Redis` modes) | -| `DataProtection:CertificatePath` | — | Path to the current `.pfx` certificate (`FileSystem` provider) | -| `DataProtection:CertificatePassword` | — | Certificate password — shared by current and previous certificates and by the `AzureBlob` provider (prefer environment variable or secrets manager) | -| `DataProtection:PreviousCertificatePaths` | `[]` | Optional array of `.pfx` paths for previously-used certificates, kept available for decrypting key XML written under the prior cert until rotation completes | -| `DataProtection:AzureBlobUrl` | — | Azure Blob URL for the current certificate (`AzureBlob` provider) | -| `DataProtection:PreviousAzureBlobUrls` | `[]` | Optional array of blob URLs for previously-used certificates (`AzureBlob` provider). Same semantics as `PreviousCertificatePaths` | +| `DataProtection:FileSystemCertificate:Path` | — | Path to the current `.pfx` certificate (`FileSystem` provider) | +| `DataProtection:FileSystemCertificate:Password` | — | Password for the current and previous file-system certificates (prefer environment variable or secrets manager) | +| `DataProtection:FileSystemCertificate:PreviousPaths` | `[]` | Optional array of `.pfx` paths for previously-used certificates, kept available for decrypting key XML written under the prior cert until rotation completes | +| `DataProtection:AzureBlobCertificate:BlobUri` | — | Azure Blob URI for the current certificate (`AzureBlob` provider) | +| `DataProtection:AzureBlobCertificate:Password` | — | Password for the current and previous Azure Blob certificates (prefer environment variable or secrets manager) | +| `DataProtection:AzureBlobCertificate:PreviousBlobUris` | `[]` | Optional array of blob URIs for previously-used certificates (`AzureBlob` provider). Same semantics as `FileSystemCertificate:PreviousPaths` | | `DataProtection:Redis:ConnectionString` | — | Redis connection string (`Redis` mode) | | `DataProtection:Redis:KeyName` | `groundcontrol-data-protection` | Redis key name for key ring storage | | `DataProtection:Redis:ConnectTimeoutMs` | `5000` | Redis connection timeout | diff --git a/docs/design-docs/Security-Model.md b/docs/design-docs/Security-Model.md index 37820048..7971071f 100644 --- a/docs/design-docs/Security-Model.md +++ b/docs/design-docs/Security-Model.md @@ -224,9 +224,11 @@ IDataProtectionCertificateProvider "DataProtection": { "Mode": "Certificate", "CertificateProvider": "FileSystem", - "CertificatePath": "/certs/dp-2026.pfx", - "CertificatePassword": "...", - "PreviousCertificatePaths": [ "/certs/dp-2024.pfx" ] + "FileSystemCertificate": { + "Path": "/certs/dp-2026.pfx", + "Password": "...", + "PreviousPaths": [ "/certs/dp-2024.pfx" ] + } } } ``` @@ -238,14 +240,16 @@ IDataProtectionCertificateProvider "DataProtection": { "Mode": "Certificate", "CertificateProvider": "AzureBlob", - "AzureBlobUrl": "https://account.blob.core.windows.net/certificates/dp-2026.pfx", - "PreviousAzureBlobUrls": [ "https://account.blob.core.windows.net/certificates/dp-2024.pfx" ], - "CertificatePassword": "..." + "AzureBlobCertificate": { + "BlobUri": "https://account.blob.core.windows.net/certificates/dp-2026.pfx", + "Password": "...", + "PreviousBlobUris": [ "https://account.blob.core.windows.net/certificates/dp-2024.pfx" ] + } } } ``` -The `AzureBlobCertificateProvider` uses `DefaultAzureCredential` for authentication to the storage account. `PreviousAzureBlobUrls` is the AzureBlob equivalent of `PreviousCertificatePaths`; certificates downloaded from those URLs are added to the decryption pipeline so key XML written under a previous certificate remains decryptable during and after rotation. +The `AzureBlobCertificateProvider` uses `DefaultAzureCredential` for authentication to the storage account. `AzureBlobCertificate:PreviousBlobUris` is the AzureBlob equivalent of `FileSystemCertificate:PreviousPaths`; certificates downloaded from those URIs are added to the decryption pipeline so key XML written under a previous certificate remains decryptable during and after rotation. ### Certificate Lifecycle (Key Ring Protection) @@ -270,11 +274,11 @@ However, there is a practical caveat: if configured via **thumbprint** (`Protect 1. Generate a new X.509 certificate. 2. Make the new certificate available to the `IDataProtectionCertificateProvider` (e.g., deploy the file, upload to blob storage). -3. Update configuration: set the new certificate as `DataProtection:CertificatePath` and add the old certificate to `DataProtection:PreviousCertificatePaths`. +3. Update configuration: set the new certificate as `DataProtection:FileSystemCertificate:Path` (or `DataProtection:AzureBlobCertificate:BlobUri`) and add the old certificate to `DataProtection:FileSystemCertificate:PreviousPaths` (or `DataProtection:AzureBlobCertificate:PreviousBlobUris`). 4. Perform a rolling restart. At startup `DataProtectionModule` resolves the provider, loads the current and previous certificates, and: - Sets `KeyManagementOptions.XmlEncryptor = new CertificateXmlEncryptor(current)` — new data protection keys are encrypted with the new certificate. - Calls `dataProtectionBuilder.UnprotectKeysWithAnyCertificate(current, ...previous)` — every existing key encrypted under the current or any previous certificate remains decryptable. -5. After all data protection keys protected by the old certificate have expired (90+ days) or been re-encrypted, remove that path from `PreviousCertificatePaths`. +5. After all data protection keys protected by the old certificate have expired (90+ days) or been re-encrypted, remove that path from `PreviousPaths`/`PreviousBlobUris`. **Configuration example (certificate rotation in progress, FileSystem provider):** @@ -284,9 +288,11 @@ However, there is a practical caveat: if configured via **thumbprint** (`Protect "Mode": "Certificate", "KeyStorePath": "/keys", "CertificateProvider": "FileSystem", - "CertificatePath": "/certs/dp-2026.pfx", - "CertificatePassword": "...", - "PreviousCertificatePaths": [ "/certs/dp-2024.pfx" ], + "FileSystemCertificate": { + "Path": "/certs/dp-2026.pfx", + "Password": "...", + "PreviousPaths": [ "/certs/dp-2024.pfx" ] + }, "KeyRotation": { "Enabled": true, "KeyLifetime": 90 diff --git a/docs/guide/server/configuration.md b/docs/guide/server/configuration.md index 7724b9cf..a5ef98b5 100644 --- a/docs/guide/server/configuration.md +++ b/docs/guide/server/configuration.md @@ -67,11 +67,12 @@ Keys are stored on the file system and the key XML is encrypted at rest with an | Setting | Description | |---|---| | `DataProtection:CertificateProvider` | `FileSystem` or `AzureBlob`. | -| `DataProtection:CertificatePath` | Path to the current `.pfx` certificate file (`FileSystem` provider). | -| `DataProtection:CertificatePassword` | Certificate password. Shared by current and previous certificates and by the `AzureBlob` provider. | -| `DataProtection:PreviousCertificatePaths` | Optional array of `.pfx` paths for certificates that previously protected the key ring (`FileSystem` provider). Required during certificate rotation so existing key XML remains decryptable until rotated out. | -| `DataProtection:AzureBlobUrl` | Blob URL for the current certificate (`AzureBlob` provider). | -| `DataProtection:PreviousAzureBlobUrls` | Optional array of blob URLs for previously-used certificates (`AzureBlob` provider). Same semantics as `PreviousCertificatePaths`. | +| `DataProtection:FileSystemCertificate:Path` | Path to the current `.pfx` certificate file (`FileSystem` provider). | +| `DataProtection:FileSystemCertificate:Password` | Certificate password used for the current and previous file-system certificates. | +| `DataProtection:FileSystemCertificate:PreviousPaths` | Optional array of `.pfx` paths for certificates that previously protected the key ring (`FileSystem` provider). Required during certificate rotation so existing key XML remains decryptable until rotated out. | +| `DataProtection:AzureBlobCertificate:BlobUri` | Blob URI for the current certificate (`AzureBlob` provider). | +| `DataProtection:AzureBlobCertificate:Password` | Certificate password used for the current and previous Azure Blob certificates. | +| `DataProtection:AzureBlobCertificate:PreviousBlobUris` | Optional array of blob URIs for previously-used certificates (`AzureBlob` provider). Same semantics as `FileSystemCertificate:PreviousPaths`. | ```json { @@ -79,14 +80,16 @@ Keys are stored on the file system and the key XML is encrypted at rest with an "Mode": "Certificate", "KeyStorePath": "/keys", "CertificateProvider": "FileSystem", - "CertificatePath": "/certs/dp-current.pfx", - "CertificatePassword": "", - "PreviousCertificatePaths": [ "/certs/dp-previous.pfx" ] + "FileSystemCertificate": { + "Path": "/certs/dp-current.pfx", + "Password": "", + "PreviousPaths": [ "/certs/dp-previous.pfx" ] + } } } ``` -> **Certificate rotation:** generate the new cert, deploy it as `CertificatePath`, move the old cert into `PreviousCertificatePaths`, and perform a rolling restart. New key ring entries are encrypted with the new cert; entries written under the previous cert remain decryptable as long as that cert stays in the previous list. Remove a cert from `PreviousCertificatePaths` only after every key encrypted with it has expired (90+ days by default) or been re-encrypted — otherwise the data those keys protect becomes permanently unreadable. +> **Certificate rotation:** generate the new cert, deploy it as `FileSystemCertificate:Path`, move the old cert into `FileSystemCertificate:PreviousPaths`, and perform a rolling restart. New key ring entries are encrypted with the new cert; entries written under the previous cert remain decryptable as long as that cert stays in the previous list. Remove a cert from `PreviousPaths` only after every key encrypted with it has expired (90+ days by default) or been re-encrypted — otherwise the data those keys protect becomes permanently unreadable. ### Redis mode @@ -98,7 +101,7 @@ Keys are stored in Redis and protected with an X.509 certificate. Suitable for m | `DataProtection:Redis:KeyName` | `groundcontrol-data-protection` | Redis key name for the key ring. | | `DataProtection:Redis:ConnectTimeoutMs` | `5000` | Connection timeout in milliseconds. | -Also requires the same certificate settings as Certificate mode (`CertificateProvider`, `CertificatePath`/`AzureBlobUrl`, optional `PreviousCertificatePaths`). +Also requires the same certificate settings as Certificate mode (`CertificateProvider`, `FileSystemCertificate:Path` / `AzureBlobCertificate:BlobUri`, optional previous-certificate paths/URIs). ### Azure mode @@ -175,8 +178,8 @@ export Authentication__Seed__AdminPassword="YourSecurePassword123!" export DataProtection__Mode="Redis" export DataProtection__Redis__ConnectionString="redis:6379" export DataProtection__CertificateProvider="FileSystem" -export DataProtection__CertificatePath="/certs/dp.pfx" -export DataProtection__CertificatePassword="certpass" +export DataProtection__FileSystemCertificate__Path="/certs/dp.pfx" +export DataProtection__FileSystemCertificate__Password="certpass" # Change notification export ChangeNotifier__Mode="MongoChangeStream" diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateOptions.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateOptions.cs new file mode 100644 index 00000000..54de98de --- /dev/null +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateOptions.cs @@ -0,0 +1,30 @@ +using System.ComponentModel.DataAnnotations; +using Microsoft.Extensions.Options; + +namespace GroundControl.Api.Core.DataProtection.Certificate; + +/// +/// Options for the . +/// +internal sealed partial class AzureBlobCertificateOptions +{ + /// + /// Gets or sets the Azure Blob Storage URI of the current X.509 certificate (PFX/PKCS#12). + /// + [Required] + public Uri? BlobUri { get; set; } + + /// + /// Gets or sets the password used to load the current and previous certificates. + /// + public string? Password { get; set; } + + /// + /// Gets or sets blob URIs of previous certificates retained for decrypting key XML + /// produced under earlier certificates during rotation. + /// + public Uri[] PreviousBlobUris { get; set; } = []; + + [OptionsValidator] + public sealed partial class Validator : IValidateOptions; +} \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs index 2c511533..ae64aecf 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs @@ -1,6 +1,7 @@ using System.Security.Cryptography.X509Certificates; using Azure.Identity; using Azure.Storage.Blobs; +using Microsoft.Extensions.Options; namespace GroundControl.Api.Core.DataProtection.Certificate; @@ -15,49 +16,42 @@ namespace GroundControl.Api.Core.DataProtection.Certificate; /// internal sealed partial class AzureBlobCertificateProvider : IDataProtectionCertificateProvider { - private readonly IConfiguration _configuration; + private static readonly DefaultAzureCredential Credential = new(); + + private readonly AzureBlobCertificateOptions _options; private readonly ILogger _logger; - public AzureBlobCertificateProvider(IConfiguration configuration, - ILogger logger) + public AzureBlobCertificateProvider(IOptions options, ILogger logger) { - _configuration = configuration; + _options = options.Value; _logger = logger; } - private static readonly DefaultAzureCredential Credential = new(); - /// - public X509Certificate2 GetCurrentCertificate() - { - var blobUrl = _configuration["DataProtection:AzureBlobUrl"] ?? throw new InvalidOperationException("DataProtection:AzureBlobUrl is required."); - return DownloadCertificate(blobUrl, "AzureBlob"); - } + public X509Certificate2 GetCurrentCertificate() => DownloadCertificate(_options.BlobUri!, "AzureBlob"); /// public IReadOnlyList GetPreviousCertificates() { - var urls = _configuration.GetSection("DataProtection:PreviousAzureBlobUrls").Get() ?? []; - if (urls.Length == 0) + var uris = _options.PreviousBlobUris; + if (uris.Length == 0) { return []; } - var certificates = new List(urls.Length); - certificates.AddRange(urls.Select(url => DownloadCertificate(url, "AzureBlob (previous)"))); + var certificates = new List(uris.Length); + certificates.AddRange(uris.Select(uri => DownloadCertificate(uri, "AzureBlob (previous)"))); return certificates; } - private X509Certificate2 DownloadCertificate(string blobUrl, string source) + private X509Certificate2 DownloadCertificate(Uri blobUri, string source) { - var password = _configuration["DataProtection:CertificatePassword"]; - - var client = new BlobClient(new Uri(blobUrl), Credential); + var client = new BlobClient(blobUri, Credential); var response = client.DownloadContent(); var pfxBytes = response.Value.Content.ToArray(); - var certificate = X509CertificateLoader.LoadPkcs12(pfxBytes, password, X509KeyStorageFlags.EphemeralKeySet); + var certificate = X509CertificateLoader.LoadPkcs12(pfxBytes, _options.Password, X509KeyStorageFlags.EphemeralKeySet); LogCertificateLoaded(_logger, source, certificate.Thumbprint); return certificate; @@ -65,4 +59,4 @@ private X509Certificate2 DownloadCertificate(string blobUrl, string source) [LoggerMessage(1, LogLevel.Information, "Loaded certificate from {Source} with thumbprint {Thumbprint}.")] private static partial void LogCertificateLoaded(ILogger logger, string source, string thumbprint); -} +} \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateOptions.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateOptions.cs new file mode 100644 index 00000000..cf4d1a72 --- /dev/null +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateOptions.cs @@ -0,0 +1,30 @@ +using System.ComponentModel.DataAnnotations; +using Microsoft.Extensions.Options; + +namespace GroundControl.Api.Core.DataProtection.Certificate; + +/// +/// Options for the . +/// +internal sealed partial class FileSystemCertificateOptions +{ + /// + /// Gets or sets the file system path to the current X.509 certificate (PFX/PKCS#12). + /// + [Required] + public string Path { get; set; } = string.Empty; + + /// + /// Gets or sets the password used to load the current and previous certificates. + /// + public string? Password { get; set; } + + /// + /// Gets or sets file system paths to previous certificates retained for decrypting key XML + /// produced under earlier certificates during rotation. + /// + public string[] PreviousPaths { get; set; } = []; + + [OptionsValidator] + public sealed partial class Validator : IValidateOptions; +} \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateProvider.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateProvider.cs index e78635e5..fbfe92c7 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateProvider.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/FileSystemCertificateProvider.cs @@ -1,4 +1,5 @@ using System.Security.Cryptography.X509Certificates; +using Microsoft.Extensions.Options; namespace GroundControl.Api.Core.DataProtection.Certificate; @@ -7,32 +8,28 @@ namespace GroundControl.Api.Core.DataProtection.Certificate; /// internal sealed partial class FileSystemCertificateProvider : IDataProtectionCertificateProvider { - private readonly IConfiguration _configuration; + private readonly FileSystemCertificateOptions _options; private readonly ILogger _logger; - public FileSystemCertificateProvider(IConfiguration configuration, ILogger logger) + public FileSystemCertificateProvider(IOptions options, ILogger logger) { - _configuration = configuration; + _options = options.Value; _logger = logger; } /// - public X509Certificate2 GetCurrentCertificate() - { - var path = _configuration["DataProtection:CertificatePath"] ?? throw new InvalidOperationException("DataProtection:CertificatePath is required."); - return LoadCertificate(path, _configuration["DataProtection:CertificatePassword"], "FileSystem"); - } + public X509Certificate2 GetCurrentCertificate() => LoadCertificate(_options.Path, _options.Password, "FileSystem"); /// public IReadOnlyList GetPreviousCertificates() { - var paths = _configuration.GetSection("DataProtection:PreviousCertificatePaths").Get() ?? []; + var paths = _options.PreviousPaths; if (paths.Length == 0) { return []; } - var password = _configuration["DataProtection:CertificatePassword"]; + var password = _options.Password; var certificates = new List(paths.Length); certificates.AddRange(paths.Select(path => LoadCertificate(path, password, "FileSystem (previous)"))); diff --git a/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs b/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs index 1ae24a24..484de687 100644 --- a/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs +++ b/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs @@ -21,7 +21,7 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) if (options.CertificateProvider.HasValue) { - RegisterCertificateProvider(builder.Services, options.CertificateProvider.Value); + RegisterCertificateProvider(builder.Services, options); builder.Services.AddHostedService(); } @@ -38,7 +38,7 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) // registration time. Loading them here and supplying both current and previous certs is // required for cross-instance and cross-restart decryption to work, and for safe // certificate rotation. - var startupCertificateProvider = CreateCertificateProvider(builder.Configuration, options.CertificateProvider!.Value); + var startupCertificateProvider = CreateCertificateProvider(options); var currentCertificate = startupCertificateProvider.GetCurrentCertificate(); var previousCertificates = startupCertificateProvider.GetPreviousCertificates(); dataProtectionBuilder.UnprotectKeysWithAnyCertificate([currentCertificate, .. previousCertificates]); @@ -54,22 +54,34 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) DataProtectionMode.Redis => new RedisKeyRingConfigurator(), DataProtectionMode.Azure => new AzureKeyRingConfigurator(), _ => throw new InvalidOperationException( - $"Unknown DataProtection:Mode '{mode}'. Supported values: {string.Join(", ", Enum.GetNames())}.") + $"Unknown {nameof(DataProtectionOptions)}:{nameof(DataProtectionOptions.Mode)} '{mode}'. Supported values: {string.Join(", ", Enum.GetNames())}.") }; - private static IServiceCollection RegisterCertificateProvider(IServiceCollection services, CertificateProviderMode mode) => mode switch + private static void RegisterCertificateProvider(IServiceCollection services, DataProtectionOptions options) { - CertificateProviderMode.FileSystem => services.AddSingleton(), - CertificateProviderMode.AzureBlob => services.AddSingleton(), - _ => throw new InvalidOperationException( - $"Unknown DataProtection:CertificateProvider '{mode}'. Supported values: {string.Join(", ", Enum.GetNames())}.") - }; + switch (options.CertificateProvider!.Value) + { + case CertificateProviderMode.FileSystem: + services.AddSingleton(Options.Create(options.FileSystemCertificate)); + services.AddSingleton(); + break; + + case CertificateProviderMode.AzureBlob: + services.AddSingleton(Options.Create(options.AzureBlobCertificate)); + services.AddSingleton(); + break; + + default: + throw new InvalidOperationException( + $"Unknown {nameof(DataProtectionOptions)}:{nameof(DataProtectionOptions.CertificateProvider)} '{options.CertificateProvider}'. Supported values: {string.Join(", ", Enum.GetNames())}."); + } + } - private static IDataProtectionCertificateProvider CreateCertificateProvider(IConfiguration configuration, CertificateProviderMode mode) => mode switch + private static IDataProtectionCertificateProvider CreateCertificateProvider(DataProtectionOptions options) => options.CertificateProvider switch { - CertificateProviderMode.FileSystem => new FileSystemCertificateProvider(configuration, NullLogger.Instance), - CertificateProviderMode.AzureBlob => new AzureBlobCertificateProvider(configuration, NullLogger.Instance), + CertificateProviderMode.FileSystem => new FileSystemCertificateProvider(Options.Create(options.FileSystemCertificate), NullLogger.Instance), + CertificateProviderMode.AzureBlob => new AzureBlobCertificateProvider(Options.Create(options.AzureBlobCertificate), NullLogger.Instance), _ => throw new InvalidOperationException( - $"Unknown DataProtection:CertificateProvider '{mode}'. Supported values: {string.Join(", ", Enum.GetNames())}.") + $"Unknown {nameof(DataProtectionOptions)}:{nameof(DataProtectionOptions.CertificateProvider)} '{options.CertificateProvider}'. Supported values: {string.Join(", ", Enum.GetNames())}.") }; } \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs b/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs index 2621f6b6..f2204618 100644 --- a/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs +++ b/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs @@ -1,3 +1,4 @@ +using GroundControl.Api.Core.DataProtection.Certificate; using Microsoft.Extensions.Options; namespace GroundControl.Api.Core.DataProtection; @@ -32,19 +33,14 @@ internal sealed class DataProtectionOptions public bool UseDpapi { get; set; } /// - /// Gets or sets the file system path to the X.509 certificate. + /// Gets or sets options for the file system certificate provider. /// - public string? CertificatePath { get; set; } + public FileSystemCertificateOptions FileSystemCertificate { get; set; } = new(); /// - /// Gets or sets the certificate password. + /// Gets or sets options for the Azure Blob certificate provider. /// - public string? CertificatePassword { get; set; } - - /// - /// Gets or sets the Azure Blob URL for certificate download. - /// - public Uri? CertificateAzureBlobUrl { get; set; } + public AzureBlobCertificateOptions AzureBlobCertificate { get; set; } = new(); /// /// Gets or sets the Redis-specific options. @@ -57,7 +53,8 @@ internal sealed class DataProtectionOptions public AzureOptions Azure { get; set; } = new(); /// - /// Validates including cross-property constraints. + /// Validates by enforcing mode/provider consistency + /// and dispatching to source-generated validators for the active sub-options. /// internal sealed class Validator : IValidateOptions { @@ -66,43 +63,42 @@ public ValidateOptionsResult Validate(string? name, DataProtectionOptions option { var failures = new List(); - if (string.IsNullOrEmpty(options.KeyStorePath)) - { - failures.Add("DataProtection:KeyStorePath is required."); - } - - if (options.Mode is DataProtectionMode.Certificate or DataProtectionMode.Redis) + if (string.IsNullOrWhiteSpace(options.KeyStorePath)) { - if (!options.CertificateProvider.HasValue) - { - failures.Add($"DataProtection:CertificateProvider must be configured when Mode is '{options.Mode}'."); - } + failures.Add($"{nameof(DataProtectionOptions)}:{nameof(KeyStorePath)} is required."); } - if (options.CertificateProvider is CertificateProviderMode.FileSystem && string.IsNullOrWhiteSpace(options.CertificatePath)) + if (options.Mode is DataProtectionMode.Certificate or DataProtectionMode.Redis && !options.CertificateProvider.HasValue) { - failures.Add("DataProtection:CertificatePath is required when CertificateProvider is 'FileSystem'."); + failures.Add($"{nameof(DataProtectionOptions)}:{nameof(CertificateProvider)} must be configured when Mode is '{options.Mode}'."); } - if (options is { CertificateProvider: CertificateProviderMode.AzureBlob, CertificateAzureBlobUrl: null }) + switch (options.CertificateProvider) { - failures.Add("DataProtection:AzureBlobUrl is required when CertificateProvider is 'AzureBlob'."); + case CertificateProviderMode.FileSystem: + if (!FileSystemCertificateOptions.Validator.TryValidate(options.FileSystemCertificate, out var fsFailures, nameof(FileSystemCertificate))) + { + failures.AddRange(fsFailures); + } + break; + + case CertificateProviderMode.AzureBlob: + if (!AzureBlobCertificateOptions.Validator.TryValidate(options.AzureBlobCertificate, out var blobFailures, nameof(AzureBlobCertificate))) + { + failures.AddRange(blobFailures); + } + break; } - if (options.Mode is DataProtectionMode.Redis) + switch (options.Mode) { - if (RedisOptions.Validator.TryValidate(options.Redis, out var redisFailures, nameof(options.Redis))) - { + case DataProtectionMode.Redis when !RedisOptions.Validator.TryValidate(options.Redis, out var redisFailures, nameof(Redis)): failures.AddRange(redisFailures); - } - } + break; - if (options.Mode is DataProtectionMode.Azure) - { - if (AzureOptions.Validator.TryValidate(options.Azure, out var azureFailures, nameof(options.Azure))) - { + case DataProtectionMode.Azure when !AzureOptions.Validator.TryValidate(options.Azure, out var azureFailures, nameof(Azure)): failures.AddRange(azureFailures); - } + break; } return failures.Count > 0 ? ValidateOptionsResult.Fail(failures) : ValidateOptionsResult.Success; diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemCertificateProviderTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemCertificateProviderTests.cs index 9886b975..7118528e 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemCertificateProviderTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemCertificateProviderTests.cs @@ -1,9 +1,9 @@ using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using GroundControl.Api.Core.DataProtection.Certificate; -using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Shouldly; using Xunit; @@ -24,8 +24,7 @@ public void GetCurrentCertificate_LoadsValidPfxWithoutPassword() { // Arrange var pfxPath = CreateTestCertificate(password: null); - var configuration = BuildConfiguration(pfxPath, password: null); - var provider = new FileSystemCertificateProvider(configuration, _logger); + var provider = new FileSystemCertificateProvider(BuildOptions(pfxPath, password: null), _logger); // Act var certificate = provider.GetCurrentCertificate(); @@ -42,8 +41,7 @@ public void GetCurrentCertificate_LoadsValidPfxWithPassword() // Arrange var password = "test-password-123"; var pfxPath = CreateTestCertificate(password); - var configuration = BuildConfiguration(pfxPath, password); - var provider = new FileSystemCertificateProvider(configuration, _logger); + var provider = new FileSystemCertificateProvider(BuildOptions(pfxPath, password), _logger); // Act var certificate = provider.GetCurrentCertificate(); @@ -58,8 +56,7 @@ public void GetCurrentCertificate_LoadsValidPfxWithPassword() public void GetCurrentCertificate_ThrowsFileNotFoundException_WhenPathDoesNotExist() { // Arrange - var configuration = BuildConfiguration("/nonexistent/path/cert.pfx", password: null); - var provider = new FileSystemCertificateProvider(configuration, _logger); + var provider = new FileSystemCertificateProvider(BuildOptions("/nonexistent/path/cert.pfx", password: null), _logger); // Act & Assert Should.Throw(() => provider.GetCurrentCertificate()); @@ -70,31 +67,33 @@ public void GetCurrentCertificate_ThrowsCryptographicException_WhenPasswordIsWro { // Arrange var pfxPath = CreateTestCertificate(password: "correct-password"); - var configuration = BuildConfiguration(pfxPath, password: "wrong-password"); - var provider = new FileSystemCertificateProvider(configuration, _logger); + var provider = new FileSystemCertificateProvider(BuildOptions(pfxPath, password: "wrong-password"), _logger); // Act & Assert Should.Throw(() => provider.GetCurrentCertificate()); } [Fact] - public void GetCurrentCertificate_ThrowsInvalidOperationException_WhenPathNotConfigured() + public void Validator_FailsValidation_WhenPathIsEmpty() { // Arrange - var configuration = new ConfigurationBuilder().Build(); - var provider = new FileSystemCertificateProvider(configuration, _logger); + var options = new FileSystemCertificateOptions(); + var validator = new FileSystemCertificateOptions.Validator(); - // Act & Assert - var exception = Should.Throw(() => provider.GetCurrentCertificate()); - exception.Message.ShouldContain("DataProtection:CertificatePath"); + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.FailureMessage.ShouldContain(nameof(FileSystemCertificateOptions.Path)); } [Fact] public void GetPreviousCertificates_ReturnsEmptyList_WhenNotConfigured() { // Arrange - var configuration = new ConfigurationBuilder().Build(); - var provider = new FileSystemCertificateProvider(configuration, _logger); + var pfxPath = CreateTestCertificate(password: null); + var provider = new FileSystemCertificateProvider(BuildOptions(pfxPath, password: null), _logger); // Act var result = provider.GetPreviousCertificates(); @@ -110,8 +109,8 @@ public void GetPreviousCertificates_LoadsConfiguredPaths() var currentPath = CreateTestCertificate(password: null); var firstPreviousPath = CreateTestCertificate(password: null); var secondPreviousPath = CreateTestCertificate(password: null); - var configuration = BuildConfiguration(currentPath, password: null, previousPaths: [firstPreviousPath, secondPreviousPath]); - var provider = new FileSystemCertificateProvider(configuration, _logger); + var options = BuildOptions(currentPath, password: null, previousPaths: [firstPreviousPath, secondPreviousPath]); + var provider = new FileSystemCertificateProvider(options, _logger); // Act var result = provider.GetPreviousCertificates(); @@ -122,30 +121,13 @@ public void GetPreviousCertificates_LoadsConfiguredPaths() result[1].HasPrivateKey.ShouldBeTrue(); } - private static IConfiguration BuildConfiguration(string path, string? password, IReadOnlyList? previousPaths = null) - { - var configValues = new Dictionary - { - ["DataProtection:CertificatePath"] = path - }; - - if (password is not null) - { - configValues["DataProtection:CertificatePassword"] = password; - } - - if (previousPaths is not null) + private static IOptions BuildOptions(string path, string? password, IReadOnlyList? previousPaths = null) => + Options.Create(new FileSystemCertificateOptions { - for (var i = 0; i < previousPaths.Count; i++) - { - configValues[$"DataProtection:PreviousCertificatePaths:{i}"] = previousPaths[i]; - } - } - - return new ConfigurationBuilder() - .AddInMemoryCollection(configValues) - .Build(); - } + Path = path, + Password = password, + PreviousPaths = previousPaths is null ? [] : [.. previousPaths] + }); private string CreateTestCertificate(string? password) { diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs index cda69c69..30833d54 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs @@ -8,8 +8,8 @@ namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle; /// Verifies that rotating the X.509 certificate that protects the Data Protection key ring /// does not strand sensitive values written before the rotation. Factory A boots with cert /// C1 as the current certificate; Factory B boots with C2 as current and C1 in -/// DataProtection:PreviousCertificatePaths so the key XML protected by C1 remains -/// decryptable. +/// DataProtection:FileSystemCertificate:PreviousPaths so the key XML protected by C1 +/// remains decryptable. /// public sealed class CertificateRotationTests : DataProtectionLifecycleTestBase { @@ -80,13 +80,13 @@ private GroundControlApiFactory CreateLifecycleFactory(string currentCertificate ["Persistence:MongoDb:DatabaseName"] = DatabaseName, ["DataProtection:Mode"] = "Certificate", ["DataProtection:CertificateProvider"] = "FileSystem", - ["DataProtection:CertificatePath"] = currentCertificatePath, + ["DataProtection:FileSystemCertificate:Path"] = currentCertificatePath, ["DataProtection:KeyStorePath"] = _keyStorePath }; for (var i = 0; i < previousCertificatePaths.Count; i++) { - config[$"DataProtection:PreviousCertificatePaths:{i}"] = previousCertificatePaths[i]; + config[$"DataProtection:FileSystemCertificate:PreviousPaths:{i}"] = previousCertificatePaths[i]; } return CreateFactory(config); diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs index bf638f98..570857c9 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs @@ -59,14 +59,14 @@ private GroundControlApiFactory CreateLifecycleFactory(string currentCertificate ["Persistence:MongoDb:DatabaseName"] = DatabaseName, ["DataProtection:Mode"] = "Redis", ["DataProtection:CertificateProvider"] = "FileSystem", - ["DataProtection:CertificatePath"] = currentCertificatePath, + ["DataProtection:FileSystemCertificate:Path"] = currentCertificatePath, ["DataProtection:Redis:ConnectionString"] = _redisFixture.ConnectionString, ["DataProtection:Redis:KeyName"] = _redisKeyName }; for (var i = 0; i < previousCertificatePaths.Count; i++) { - config[$"DataProtection:PreviousCertificatePaths:{i}"] = previousCertificatePaths[i]; + config[$"DataProtection:FileSystemCertificate:PreviousPaths:{i}"] = previousCertificatePaths[i]; } return CreateFactory(config); diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyPersistenceTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyPersistenceTests.cs index 84e1cff8..6672b141 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyPersistenceTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyPersistenceTests.cs @@ -60,7 +60,7 @@ private GroundControlApiFactory CreateLifecycleFactory() => CreateFactory(new Di ["Persistence:MongoDb:DatabaseName"] = DatabaseName, ["DataProtection:Mode"] = "Redis", ["DataProtection:CertificateProvider"] = "FileSystem", - ["DataProtection:CertificatePath"] = _certificatePath, + ["DataProtection:FileSystemCertificate:Path"] = _certificatePath, ["DataProtection:Redis:ConnectionString"] = _redisFixture.ConnectionString, ["DataProtection:Redis:KeyName"] = _redisKeyName }); diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyRotationTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyRotationTests.cs index 12d0d594..38e2cc8f 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyRotationTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisKeyRotationTests.cs @@ -75,7 +75,7 @@ private GroundControlApiFactory CreateLifecycleFactory() => CreateFactory(new Di ["Persistence:MongoDb:DatabaseName"] = DatabaseName, ["DataProtection:Mode"] = "Redis", ["DataProtection:CertificateProvider"] = "FileSystem", - ["DataProtection:CertificatePath"] = _certificatePath, + ["DataProtection:FileSystemCertificate:Path"] = _certificatePath, ["DataProtection:Redis:ConnectionString"] = _redisFixture.ConnectionString, ["DataProtection:Redis:KeyName"] = _redisKeyName }); From 990268052d3498152f8fdb116c9b1c8c888c0cd3 Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Wed, 6 May 2026 23:15:40 +0100 Subject: [PATCH 06/12] feat(api): make Azure credential type configurable for Data Protection Replace hard-coded DefaultAzureCredential in AzureKeyRingConfigurator and AzureBlobCertificateProvider with a configurable AzureCredentialType (Default, ManagedIdentity, WorkloadIdentity, ClientSecret, AzureCli, Environment) bound under DataProtection:AzureCredential, with cross-field validation per type. The credential is built once in DataProtectionModule and shared across the key ring configurator and the cert provider. --- docs/design-docs/Deployment-Architecture.md | 4 +- docs/design-docs/Security-Model.md | 5 +- .../DataProtection/AzureCredentialFactory.cs | 87 ++++++++++++++++++ .../DataProtection/AzureCredentialOptions.cs | 88 +++++++++++++++++++ .../DataProtection/AzureCredentialType.cs | 46 ++++++++++ .../AzureBlobCertificateProvider.cs | 16 ++-- .../DataProtection/DataProtectionModule.cs | 27 ++++-- .../DataProtection/DataProtectionOptions.cs | 15 ++++ .../KeyRing/AzureKeyRingConfigurator.cs | 10 +-- .../AzureKeyRingConfiguratorTests.cs | 5 +- 10 files changed, 281 insertions(+), 22 deletions(-) create mode 100644 src/GroundControl.Api/Core/DataProtection/AzureCredentialFactory.cs create mode 100644 src/GroundControl.Api/Core/DataProtection/AzureCredentialOptions.cs create mode 100644 src/GroundControl.Api/Core/DataProtection/AzureCredentialType.cs diff --git a/docs/design-docs/Deployment-Architecture.md b/docs/design-docs/Deployment-Architecture.md index 2346b9f8..a0c10e20 100644 --- a/docs/design-docs/Deployment-Architecture.md +++ b/docs/design-docs/Deployment-Architecture.md @@ -180,9 +180,11 @@ environment: - DataProtection__Mode=Azure - DataProtection__Azure__BlobStorageUri=https://.blob.core.windows.net/data-protection/keys.xml - DataProtection__Azure__KeyVaultKeyUri=https://.vault.azure.net/keys/groundcontrol-dp + - DataProtection__AzureCredential__Mode=ManagedIdentity + - DataProtection__AzureCredential__ClientId= # omit for system-assigned MI ``` -Azure Key Vault handles key ring protection natively — no manual certificate management is needed. Use `DefaultAzureCredential` (Managed Identity in AKS) for authentication to both Blob Storage and Key Vault. All regions share the same blob container and Key Vault key, ensuring a consistent key ring. +Azure Key Vault handles key ring protection natively — no manual certificate management is needed. The credential type is configured via `DataProtection:AzureCredential:Mode` and applies to both Blob Storage and Key Vault. Supported modes: `Default` (chained `DefaultAzureCredential`, the recommended starting point), `ManagedIdentity` (system- or user-assigned), `WorkloadIdentity` (AKS workload identity), `ClientSecret`, `AzureCli` (local development), and `Environment`. All regions share the same blob container and Key Vault key, ensuring a consistent key ring. --- diff --git a/docs/design-docs/Security-Model.md b/docs/design-docs/Security-Model.md index 7971071f..4b5665d8 100644 --- a/docs/design-docs/Security-Model.md +++ b/docs/design-docs/Security-Model.md @@ -244,12 +244,15 @@ IDataProtectionCertificateProvider "BlobUri": "https://account.blob.core.windows.net/certificates/dp-2026.pfx", "Password": "...", "PreviousBlobUris": [ "https://account.blob.core.windows.net/certificates/dp-2024.pfx" ] + }, + "AzureCredential": { + "Mode": "ManagedIdentity" } } } ``` -The `AzureBlobCertificateProvider` uses `DefaultAzureCredential` for authentication to the storage account. `AzureBlobCertificate:PreviousBlobUris` is the AzureBlob equivalent of `FileSystemCertificate:PreviousPaths`; certificates downloaded from those URIs are added to the decryption pipeline so key XML written under a previous certificate remains decryptable during and after rotation. +The `AzureBlobCertificateProvider` authenticates to the storage account using the credential configured under `DataProtection:AzureCredential` (the same credential used by `DataProtection:Mode = Azure`). Supported modes are `Default`, `ManagedIdentity`, `WorkloadIdentity`, `ClientSecret`, `AzureCli`, and `Environment`; see `AzureCredentialOptions` for per-mode required fields. `AzureBlobCertificate:PreviousBlobUris` is the AzureBlob equivalent of `FileSystemCertificate:PreviousPaths`; certificates downloaded from those URIs are added to the decryption pipeline so key XML written under a previous certificate remains decryptable during and after rotation. ### Certificate Lifecycle (Key Ring Protection) diff --git a/src/GroundControl.Api/Core/DataProtection/AzureCredentialFactory.cs b/src/GroundControl.Api/Core/DataProtection/AzureCredentialFactory.cs new file mode 100644 index 00000000..14482bb9 --- /dev/null +++ b/src/GroundControl.Api/Core/DataProtection/AzureCredentialFactory.cs @@ -0,0 +1,87 @@ +using Azure.Core; +using Azure.Identity; + +namespace GroundControl.Api.Core.DataProtection; + +/// +/// Builds an Azure SDK from . +/// +internal static class AzureCredentialFactory +{ + /// + /// Creates a matching the configured + /// . The caller must validate + /// before invoking; required fields are not re-checked here. + /// + public static TokenCredential Create(AzureCredentialOptions options) => options.Mode switch + { + AzureCredentialType.Default => new DefaultAzureCredential(BuildDefaultOptions(options)), + AzureCredentialType.ManagedIdentity => CreateManagedIdentity(options), + AzureCredentialType.WorkloadIdentity => CreateWorkloadIdentity(options), + AzureCredentialType.ClientSecret => new ClientSecretCredential(options.TenantId!, options.ClientId!, options.ClientSecret!, BuildTokenCredentialOptions(options)), + AzureCredentialType.AzureCli => new AzureCliCredential(BuildTokenCredentialOptions(options)), + AzureCredentialType.Environment => new EnvironmentCredential(BuildTokenCredentialOptions(options)), + _ => throw new InvalidOperationException( + $"Unknown {nameof(AzureCredentialOptions)}:{nameof(AzureCredentialOptions.Mode)} '{options.Mode}'. Supported values: {string.Join(", ", Enum.GetNames())}.") + }; + + private static ManagedIdentityCredential CreateManagedIdentity(AzureCredentialOptions options) + { + var managedIdentityId = string.IsNullOrWhiteSpace(options.ClientId) + ? ManagedIdentityId.SystemAssigned + : ManagedIdentityId.FromUserAssignedClientId(options.ClientId); + + var credentialOptions = new ManagedIdentityCredentialOptions(managedIdentityId); + if (options.AuthorityHost is not null) + { + credentialOptions.AuthorityHost = options.AuthorityHost; + } + + return new ManagedIdentityCredential(credentialOptions); + } + + private static WorkloadIdentityCredential CreateWorkloadIdentity(AzureCredentialOptions options) + { + var credentialOptions = new WorkloadIdentityCredentialOptions + { + TenantId = options.TenantId, + ClientId = options.ClientId, + TokenFilePath = options.TokenFilePath + }; + + if (options.AuthorityHost is not null) + { + credentialOptions.AuthorityHost = options.AuthorityHost; + } + + return new WorkloadIdentityCredential(credentialOptions); + } + + private static DefaultAzureCredentialOptions BuildDefaultOptions(AzureCredentialOptions options) + { + var defaultOptions = new DefaultAzureCredentialOptions + { + TenantId = options.TenantId, + ManagedIdentityClientId = options.ClientId + }; + + if (options.AuthorityHost is not null) + { + defaultOptions.AuthorityHost = options.AuthorityHost; + } + + return defaultOptions; + } + + private static T BuildTokenCredentialOptions(AzureCredentialOptions options) + where T : TokenCredentialOptions, new() + { + var credentialOptions = new T(); + if (options.AuthorityHost is not null) + { + credentialOptions.AuthorityHost = options.AuthorityHost; + } + + return credentialOptions; + } +} \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/AzureCredentialOptions.cs b/src/GroundControl.Api/Core/DataProtection/AzureCredentialOptions.cs new file mode 100644 index 00000000..95875b98 --- /dev/null +++ b/src/GroundControl.Api/Core/DataProtection/AzureCredentialOptions.cs @@ -0,0 +1,88 @@ +using Microsoft.Extensions.Options; + +namespace GroundControl.Api.Core.DataProtection; + +/// +/// Options describing how the Data Protection module authenticates to Azure. Used by the +/// Azure key ring configurator and the Azure Blob certificate provider. +/// +internal sealed class AzureCredentialOptions +{ + /// + /// Gets or sets the credential type used to authenticate to Azure. + /// + public AzureCredentialType Mode { get; set; } = AzureCredentialType.Default; + + /// + /// Gets or sets the Microsoft Entra tenant id. Required for + /// and + /// . + /// + public string? TenantId { get; set; } + + /// + /// Gets or sets the Microsoft Entra application (client) id. Required for + /// and + /// ; optional for + /// (selects a user-assigned identity). + /// + public string? ClientId { get; set; } + + /// + /// Gets or sets the client secret. Required for . + /// + public string? ClientSecret { get; set; } + + /// + /// Gets or sets the path to a federated token file. Optional for + /// ; when omitted the credential reads the + /// path from the AZURE_FEDERATED_TOKEN_FILE environment variable. + /// + public string? TokenFilePath { get; set; } + + /// + /// Gets or sets the Microsoft Entra authority host URI. Use for sovereign clouds (for + /// example https://login.microsoftonline.us/). When unset, the SDK default + /// (https://login.microsoftonline.com/) is used. + /// + public Uri? AuthorityHost { get; set; } + + /// + /// Validates . Source-generated validators only inspect + /// data annotations; this validator enforces the cross-field rules each + /// requires. + /// + internal sealed class Validator : IValidateOptions + { + /// + public ValidateOptionsResult Validate(string? name, AzureCredentialOptions options) + { + var failures = new List(); + var prefix = name is null ? nameof(AzureCredentialOptions) : $"{name}"; + + switch (options.Mode) + { + case AzureCredentialType.ClientSecret: + RequireNonEmpty(options.TenantId, nameof(TenantId), prefix, options.Mode, failures); + RequireNonEmpty(options.ClientId, nameof(ClientId), prefix, options.Mode, failures); + RequireNonEmpty(options.ClientSecret, nameof(ClientSecret), prefix, options.Mode, failures); + break; + + case AzureCredentialType.WorkloadIdentity: + RequireNonEmpty(options.TenantId, nameof(TenantId), prefix, options.Mode, failures); + RequireNonEmpty(options.ClientId, nameof(ClientId), prefix, options.Mode, failures); + break; + } + + return failures.Count > 0 ? ValidateOptionsResult.Fail(failures) : ValidateOptionsResult.Success; + } + + private static void RequireNonEmpty(string? value, string member, string prefix, AzureCredentialType mode, List failures) + { + if (string.IsNullOrWhiteSpace(value)) + { + failures.Add($"{prefix}:{member} is required when {nameof(Mode)} is '{mode}'."); + } + } + } +} \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/AzureCredentialType.cs b/src/GroundControl.Api/Core/DataProtection/AzureCredentialType.cs new file mode 100644 index 00000000..5968e0a1 --- /dev/null +++ b/src/GroundControl.Api/Core/DataProtection/AzureCredentialType.cs @@ -0,0 +1,46 @@ +namespace GroundControl.Api.Core.DataProtection; + +/// +/// Defines how the Data Protection module authenticates to Azure when persisting key rings to +/// Azure Blob Storage / Key Vault, or downloading X.509 certificates from Azure Blob Storage. +/// +internal enum AzureCredentialType +{ + /// + /// Uses , which probes a chain of credential + /// sources (environment variables, workload identity, managed identity, Azure CLI, etc.) until + /// one succeeds. Recommended for AKS / App Service deployments using Managed Identity. + /// + Default, + + /// + /// Uses . When + /// is set, authenticates as that user-assigned + /// managed identity; otherwise authenticates as the system-assigned managed identity. + /// + ManagedIdentity, + + /// + /// Uses for AKS workload identity. + /// + WorkloadIdentity, + + /// + /// Uses . Requires + /// , , + /// and . + /// + ClientSecret, + + /// + /// Uses for local development against an + /// authenticated Azure CLI session. + /// + AzureCli, + + /// + /// Uses , reading credentials from the + /// well-known AZURE_* environment variables. + /// + Environment +} \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs index ae64aecf..c70382f4 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs @@ -1,12 +1,13 @@ using System.Security.Cryptography.X509Certificates; -using Azure.Identity; +using Azure.Core; using Azure.Storage.Blobs; using Microsoft.Extensions.Options; namespace GroundControl.Api.Core.DataProtection.Certificate; /// -/// Downloads X.509 certificates from Azure Blob Storage using . +/// Downloads X.509 certificates from Azure Blob Storage using an injected +/// . /// /// /// Uses the Azure SDK's synchronous BlobClient.DownloadContent API rather than @@ -16,14 +17,17 @@ namespace GroundControl.Api.Core.DataProtection.Certificate; /// internal sealed partial class AzureBlobCertificateProvider : IDataProtectionCertificateProvider { - private static readonly DefaultAzureCredential Credential = new(); - private readonly AzureBlobCertificateOptions _options; + private readonly TokenCredential _credential; private readonly ILogger _logger; - public AzureBlobCertificateProvider(IOptions options, ILogger logger) + public AzureBlobCertificateProvider( + IOptions options, + TokenCredential credential, + ILogger logger) { _options = options.Value; + _credential = credential; _logger = logger; } @@ -47,7 +51,7 @@ public IReadOnlyList GetPreviousCertificates() private X509Certificate2 DownloadCertificate(Uri blobUri, string source) { - var client = new BlobClient(blobUri, Credential); + var client = new BlobClient(blobUri, _credential); var response = client.DownloadContent(); var pfxBytes = response.Value.Content.ToArray(); diff --git a/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs b/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs index 484de687..2f69c3a8 100644 --- a/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs +++ b/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs @@ -1,3 +1,4 @@ +using Azure.Core; using GroundControl.Api.Core.DataProtection.Certificate; using GroundControl.Api.Core.DataProtection.KeyRing; using GroundControl.Api.Shared.Security.Protection; @@ -15,6 +16,12 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) { DataProtectionOptions.Validator.ThrowIfInvalid(options); + var azureCredential = NeedsAzureCredential(options) ? AzureCredentialFactory.Create(options.AzureCredential) : null; + if (azureCredential is not null) + { + builder.Services.AddSingleton(azureCredential); + } + var dataProtectionBuilder = builder.Services .AddDataProtection() .SetApplicationName(builder.Environment.ApplicationName); @@ -25,7 +32,7 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) builder.Services.AddHostedService(); } - var keyRingConfigurator = CreateKeyRingConfigurator(options.Mode); + var keyRingConfigurator = CreateKeyRingConfigurator(options.Mode, azureCredential); keyRingConfigurator.Configure(dataProtectionBuilder, options); if (options.Mode is DataProtectionMode.Certificate or DataProtectionMode.Redis) @@ -38,7 +45,7 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) // registration time. Loading them here and supplying both current and previous certs is // required for cross-instance and cross-restart decryption to work, and for safe // certificate rotation. - var startupCertificateProvider = CreateCertificateProvider(options); + var startupCertificateProvider = CreateCertificateProvider(options, azureCredential); var currentCertificate = startupCertificateProvider.GetCurrentCertificate(); var previousCertificates = startupCertificateProvider.GetPreviousCertificates(); dataProtectionBuilder.UnprotectKeysWithAnyCertificate([currentCertificate, .. previousCertificates]); @@ -47,12 +54,16 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) builder.Services.AddSingleton(); } - private static IKeyRingConfigurator CreateKeyRingConfigurator(DataProtectionMode mode) => mode switch + private static bool NeedsAzureCredential(DataProtectionOptions options) => + options.Mode is DataProtectionMode.Azure || options.CertificateProvider is CertificateProviderMode.AzureBlob; + + private static IKeyRingConfigurator CreateKeyRingConfigurator(DataProtectionMode mode, TokenCredential? azureCredential) => mode switch { DataProtectionMode.FileSystem => new FileSystemKeyRingConfigurator(), DataProtectionMode.Certificate => new CertificateKeyRingConfigurator(), DataProtectionMode.Redis => new RedisKeyRingConfigurator(), - DataProtectionMode.Azure => new AzureKeyRingConfigurator(), + DataProtectionMode.Azure => new AzureKeyRingConfigurator(azureCredential ?? throw new InvalidOperationException( + $"An Azure credential is required when {nameof(DataProtectionOptions)}:{nameof(DataProtectionOptions.Mode)} is '{mode}'.")), _ => throw new InvalidOperationException( $"Unknown {nameof(DataProtectionOptions)}:{nameof(DataProtectionOptions.Mode)} '{mode}'. Supported values: {string.Join(", ", Enum.GetNames())}.") }; @@ -77,10 +88,14 @@ private static void RegisterCertificateProvider(IServiceCollection services, Dat } } - private static IDataProtectionCertificateProvider CreateCertificateProvider(DataProtectionOptions options) => options.CertificateProvider switch + private static IDataProtectionCertificateProvider CreateCertificateProvider(DataProtectionOptions options, TokenCredential? azureCredential) => options.CertificateProvider switch { CertificateProviderMode.FileSystem => new FileSystemCertificateProvider(Options.Create(options.FileSystemCertificate), NullLogger.Instance), - CertificateProviderMode.AzureBlob => new AzureBlobCertificateProvider(Options.Create(options.AzureBlobCertificate), NullLogger.Instance), + CertificateProviderMode.AzureBlob => new AzureBlobCertificateProvider( + Options.Create(options.AzureBlobCertificate), + azureCredential ?? throw new InvalidOperationException( + $"An Azure credential is required when {nameof(DataProtectionOptions)}:{nameof(DataProtectionOptions.CertificateProvider)} is '{options.CertificateProvider}'."), + NullLogger.Instance), _ => throw new InvalidOperationException( $"Unknown {nameof(DataProtectionOptions)}:{nameof(DataProtectionOptions.CertificateProvider)} '{options.CertificateProvider}'. Supported values: {string.Join(", ", Enum.GetNames())}.") }; diff --git a/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs b/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs index f2204618..4e500432 100644 --- a/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs +++ b/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs @@ -52,6 +52,13 @@ internal sealed class DataProtectionOptions /// public AzureOptions Azure { get; set; } = new(); + /// + /// Gets or sets the credential used to authenticate to Azure. Applies to the Azure key ring + /// () and to the Azure Blob certificate provider + /// (); both share the same credential. + /// + public AzureCredentialOptions AzureCredential { get; set; } = new(); + /// /// Validates by enforcing mode/provider consistency /// and dispatching to source-generated validators for the active sub-options. @@ -90,6 +97,14 @@ public ValidateOptionsResult Validate(string? name, DataProtectionOptions option break; } + if (options.Mode is DataProtectionMode.Azure || options.CertificateProvider is CertificateProviderMode.AzureBlob) + { + if (!AzureCredentialOptions.Validator.TryValidate(options.AzureCredential, out var credentialFailures, nameof(AzureCredential))) + { + failures.AddRange(credentialFailures); + } + } + switch (options.Mode) { case DataProtectionMode.Redis when !RedisOptions.Validator.TryValidate(options.Redis, out var redisFailures, nameof(Redis)): diff --git a/src/GroundControl.Api/Core/DataProtection/KeyRing/AzureKeyRingConfigurator.cs b/src/GroundControl.Api/Core/DataProtection/KeyRing/AzureKeyRingConfigurator.cs index 29a463df..32c45a5e 100644 --- a/src/GroundControl.Api/Core/DataProtection/KeyRing/AzureKeyRingConfigurator.cs +++ b/src/GroundControl.Api/Core/DataProtection/KeyRing/AzureKeyRingConfigurator.cs @@ -1,4 +1,4 @@ -using Azure.Identity; +using Azure.Core; using Microsoft.AspNetCore.DataProtection; namespace GroundControl.Api.Core.DataProtection.KeyRing; @@ -6,17 +6,15 @@ namespace GroundControl.Api.Core.DataProtection.KeyRing; /// /// Persists Data Protection keys to Azure Blob Storage and protects them with Azure Key Vault. /// -internal sealed class AzureKeyRingConfigurator : IKeyRingConfigurator +internal sealed class AzureKeyRingConfigurator(TokenCredential credential) : IKeyRingConfigurator { - private static readonly DefaultAzureCredential Credential = new(); - /// public void Configure(IDataProtectionBuilder builder, DataProtectionOptions options) { AzureOptions.Validator.ThrowIfInvalid(options.Azure); builder - .PersistKeysToAzureBlobStorage(options.Azure.BlobUri, Credential) - .ProtectKeysWithAzureKeyVault(options.Azure.KeyVaultKeyId, Credential); + .PersistKeysToAzureBlobStorage(options.Azure.BlobUri, credential) + .ProtectKeysWithAzureKeyVault(options.Azure.KeyVaultKeyId, credential); } } \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/AzureKeyRingConfiguratorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/AzureKeyRingConfiguratorTests.cs index 26c6415b..ccd39c68 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/AzureKeyRingConfiguratorTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/AzureKeyRingConfiguratorTests.cs @@ -1,3 +1,4 @@ +using Azure.Identity; using GroundControl.Api.Core.DataProtection; using GroundControl.Api.Core.DataProtection.KeyRing; using Microsoft.AspNetCore.DataProtection; @@ -24,7 +25,7 @@ public void Configure_OptionsValidationException_WhenBlobUriNotConfigured() var builder = services.AddDataProtection() .SetApplicationName("GroundControl.Tests"); - var configurator = new AzureKeyRingConfigurator(); + var configurator = new AzureKeyRingConfigurator(new DefaultAzureCredential()); // Act & Assert var exception = Should.Throw(() => configurator.Configure(builder, options)); @@ -49,7 +50,7 @@ public void Configure_OptionsValidationException_WhenKeyVaultKeyIdNotConfigured( var dpBuilder = services.AddDataProtection() .SetApplicationName("GroundControl.Tests"); - var configurator = new AzureKeyRingConfigurator(); + var configurator = new AzureKeyRingConfigurator(new DefaultAzureCredential()); // Act & Assert var exception = Should.Throw(() => configurator.Configure(dpBuilder, options)); From 964d6c44d4f010b4df957b0fc539c3e7170aa812 Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Thu, 7 May 2026 09:46:48 +0100 Subject: [PATCH 07/12] build: pin Snappier package version to mitigate vulnerability --- Directory.Packages.props | 8 +++++--- src/GroundControl.AppHost/GroundControl.AppHost.csproj | 5 ++++- .../GroundControl.Persistence.MongoDb.csproj | 3 +++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index f288a302..7494dd25 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -14,8 +14,8 @@ - - + + @@ -47,7 +47,9 @@ - + + + diff --git a/src/GroundControl.AppHost/GroundControl.AppHost.csproj b/src/GroundControl.AppHost/GroundControl.AppHost.csproj index cb17c69d..d3dd7beb 100644 --- a/src/GroundControl.AppHost/GroundControl.AppHost.csproj +++ b/src/GroundControl.AppHost/GroundControl.AppHost.csproj @@ -16,6 +16,9 @@ + + + - + \ No newline at end of file diff --git a/src/GroundControl.Persistence.MongoDb/GroundControl.Persistence.MongoDb.csproj b/src/GroundControl.Persistence.MongoDb/GroundControl.Persistence.MongoDb.csproj index 0fb194e8..d5934c6a 100644 --- a/src/GroundControl.Persistence.MongoDb/GroundControl.Persistence.MongoDb.csproj +++ b/src/GroundControl.Persistence.MongoDb/GroundControl.Persistence.MongoDb.csproj @@ -16,6 +16,9 @@ + + + From 004aa4266bd11db5901aa0a43c064bc43c2b675c Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Thu, 7 May 2026 10:05:18 +0100 Subject: [PATCH 08/12] refactor(api): wire Data Protection cert decryption through DI via custom IXmlDecryptor Replace the eager UnprotectKeysWithAnyCertificate + manual provider instantiation with a custom IXmlEncryptor/IXmlDecryptor pair. The encryptor delegates the cryptographic work to the framework's CertificateXmlEncryptor and only swaps the recorded DecryptorType to GroundControlCertificateXmlDecryptor; the decryptor takes the IServiceProvider that ASP.NET Core's SimpleActivator hands it and resolves IDataProtectionCertificateProvider lazily on first decrypt. Eliminates the bootstrap-container question, the NullLogger workaround, and the triple-cert-load on startup. The decryptor falls back to EncryptedXml.DecryptEncryptedKey when no matching cert is found, so the existing rotation tests still pass. --- .../CertificateKeyEncryptionConfigurator.cs | 24 +-- .../GroundControlCertificateXmlDecryptor.cs | 171 ++++++++++++++++++ .../GroundControlCertificateXmlEncryptor.cs | 42 +++++ .../DataProtection/DataProtectionModule.cs | 30 +-- ...rtificateKeyEncryptionConfiguratorTests.cs | 25 +-- ...oundControlCertificateXmlRoundTripTests.cs | 110 +++++++++++ 6 files changed, 339 insertions(+), 63 deletions(-) create mode 100644 src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlDecryptor.cs create mode 100644 src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlRoundTripTests.cs diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateKeyEncryptionConfigurator.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateKeyEncryptionConfigurator.cs index 9fce602d..d727bceb 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateKeyEncryptionConfigurator.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateKeyEncryptionConfigurator.cs @@ -1,32 +1,20 @@ using Microsoft.AspNetCore.DataProtection.KeyManagement; -using Microsoft.AspNetCore.DataProtection.XmlEncryption; using Microsoft.Extensions.Options; namespace GroundControl.Api.Core.DataProtection.Certificate; /// -/// Configures Data Protection key encryption using X.509 certificates resolved from DI. +/// Configures Data Protection key encryption to use . /// -/// -/// Defers certificate loading from service registration time to the first resolution of -/// . The certificate provider is resolved from DI so the -/// configured logger is used instead of NullLoggerFactory. -/// internal sealed class CertificateKeyEncryptionConfigurator : IConfigureOptions { - private readonly IDataProtectionCertificateProvider _certificateProvider; - private readonly ILoggerFactory _loggerFactory; + private readonly GroundControlCertificateXmlEncryptor _encryptor; - public CertificateKeyEncryptionConfigurator(IDataProtectionCertificateProvider certificateProvider, ILoggerFactory loggerFactory) + public CertificateKeyEncryptionConfigurator(GroundControlCertificateXmlEncryptor encryptor) { - _certificateProvider = certificateProvider; - _loggerFactory = loggerFactory; + _encryptor = encryptor; } /// - public void Configure(KeyManagementOptions options) - { - var certificate = _certificateProvider.GetCurrentCertificate(); - options.XmlEncryptor = new CertificateXmlEncryptor(certificate, _loggerFactory); - } -} + public void Configure(KeyManagementOptions options) => options.XmlEncryptor = _encryptor; +} \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlDecryptor.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlDecryptor.cs new file mode 100644 index 00000000..ff67a530 --- /dev/null +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlDecryptor.cs @@ -0,0 +1,171 @@ +using System.Diagnostics.CodeAnalysis; +using System.Security.Cryptography.X509Certificates; +using System.Security.Cryptography.Xml; +using System.Xml; +using System.Xml.Linq; +using Microsoft.AspNetCore.DataProtection.XmlEncryption; + +namespace GroundControl.Api.Core.DataProtection.Certificate; + +/// +/// Decrypts Data Protection key XML produced by +/// by resolving the matching certificate through . +/// +/// +/// Activated by ASP.NET Core's SimpleActivator the first time a key needs to be decrypted. +/// That activator only supports a parameterless ctor or one that takes a single +/// , so we accept the service provider and resolve our actual +/// dependencies from it. +/// +/// The cryptographic recipe mirrors EncryptedXmlDecryptor from the framework: +/// is subclassed and +/// is overridden to match the +/// embedded recipient certificate against our provider's current and previous certificates by +/// thumbprint, then unwrap the symmetric key using the matched certificate's private key. +/// +internal sealed partial class GroundControlCertificateXmlDecryptor : IXmlDecryptor +{ + private readonly IDataProtectionCertificateProvider _provider; + private readonly ILogger _logger; + + [SuppressMessage("ReSharper", "UnusedMember.Global", Justification = "Required by ASP.NET Core.")] + public GroundControlCertificateXmlDecryptor(IServiceProvider services) + { + ArgumentNullException.ThrowIfNull(services); + _provider = services.GetRequiredService(); + _logger = services.GetRequiredService>(); + } + + // Test-only ctor that lets unit tests inject collaborators directly without building a service provider. + internal GroundControlCertificateXmlDecryptor( + IDataProtectionCertificateProvider provider, + ILogger logger) + { + _provider = provider; + _logger = logger; + } + + /// + [UnconditionalSuppressMessage("AOT", "IL2026:RequiresUnreferencedCode", Justification = "The common algorithms are preserved by the DynamicDependency attribute.")] + [UnconditionalSuppressMessage("AOT", "IL3050:RequiresDynamicCode", Justification = "Only XSLTs require dynamic code; the EncryptedXml usage here doesn't use XSLTs.")] + public XElement Decrypt(XElement encryptedElement) + { + ArgumentNullException.ThrowIfNull(encryptedElement); + + var xmlDocument = new XmlDocument(); + xmlDocument.Load(new XElement("root", encryptedElement).CreateReader()); + + var encryptedXml = new ProviderBackedEncryptedXml(xmlDocument, this); + encryptedXml.DecryptDocument(); + + return XElement.Load(xmlDocument.DocumentElement!.FirstChild!.CreateNavigator()!.ReadSubtree()); + } + + private X509Certificate2? FindCertificateWithPrivateKey(string thumbprint) + { + var current = _provider.GetCurrentCertificate(); + if (ThumbprintsMatch(current, thumbprint)) + { + return current.HasPrivateKey ? current : null; + } + + foreach (var previous in _provider.GetPreviousCertificates()) + { + if (ThumbprintsMatch(previous, thumbprint)) + { + return previous.HasPrivateKey ? previous : null; + } + } + + LogNoMatchingCertificate(_logger, thumbprint); + return null; + } + + private static bool ThumbprintsMatch(X509Certificate2 certificate, string thumbprint) + => string.Equals(certificate.Thumbprint, thumbprint, StringComparison.OrdinalIgnoreCase); + + [LoggerMessage(1, LogLevel.Warning, + "No Data Protection certificate available with thumbprint {Thumbprint}; key XML cannot be decrypted with the current provider configuration.")] + private static partial void LogNoMatchingCertificate(ILogger logger, string thumbprint); + + private sealed class ProviderBackedEncryptedXml : EncryptedXml + { + private readonly GroundControlCertificateXmlDecryptor _outer; + + public ProviderBackedEncryptedXml(XmlDocument document, GroundControlCertificateXmlDecryptor outer) + : base(document) + { + _outer = outer; + } + + public override byte[]? DecryptEncryptedKey(EncryptedKey encryptedKey) + { + ArgumentNullException.ThrowIfNull(encryptedKey); + + var keyInfoEnum = encryptedKey.KeyInfo.GetEnumerator(); + try + { + while (keyInfoEnum.MoveNext()) + { + if (keyInfoEnum.Current is not KeyInfoX509Data keyInfoX509Data) + { + continue; + } + + var unwrapped = TryDecryptKey(encryptedKey, keyInfoX509Data); + if (unwrapped is not null) + { + return unwrapped; + } + } + + return base.DecryptEncryptedKey(encryptedKey); + } + finally + { + (keyInfoEnum as IDisposable)?.Dispose(); + } + } + + private byte[]? TryDecryptKey(EncryptedKey encryptedKey, KeyInfoX509Data keyInfo) + { + var certificateEnum = keyInfo.Certificates?.GetEnumerator(); + try + { + if (certificateEnum is null) + { + return null; + } + + while (certificateEnum.MoveNext()) + { + if (certificateEnum.Current is not X509Certificate2 embeddedCertificate) + { + continue; + } + + var matchingCertificate = _outer.FindCertificateWithPrivateKey(embeddedCertificate.Thumbprint); + if (matchingCertificate is null) + { + continue; + } + + using var privateKey = matchingCertificate.GetRSAPrivateKey(); + if (privateKey is null) + { + continue; + } + + var useOaep = encryptedKey.EncryptionMethod?.KeyAlgorithm == XmlEncRSAOAEPUrl; + return DecryptKey(encryptedKey.CipherData.CipherValue!, privateKey, useOaep); + } + + return null; + } + finally + { + (certificateEnum as IDisposable)?.Dispose(); + } + } + } +} \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs new file mode 100644 index 00000000..97886672 --- /dev/null +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs @@ -0,0 +1,42 @@ +using System.Xml.Linq; +using Microsoft.AspNetCore.DataProtection.XmlEncryption; + +namespace GroundControl.Api.Core.DataProtection.Certificate; + +/// +/// Encrypts Data Protection key XML using the current X.509 certificate resolved through DI, +/// and pins as the decryptor type so the +/// decryption side can resolve certificates lazily through DI as well. +/// +/// +/// Delegates the actual cryptographic work to from the +/// framework. The only customisation is the : +/// the framework would normally pin EncryptedXmlDecryptor, which only consults the +/// internal XmlKeyDecryptionOptions. Pointing at our own decryptor lets us look up +/// certificates through at first decrypt, +/// removing the need for UnprotectKeysWithAnyCertificate and any eager loading at +/// service registration time. +/// +internal sealed class GroundControlCertificateXmlEncryptor : IXmlEncryptor +{ + private readonly IDataProtectionCertificateProvider _provider; + private readonly ILoggerFactory _loggerFactory; + + public GroundControlCertificateXmlEncryptor(IDataProtectionCertificateProvider provider, ILoggerFactory loggerFactory) + { + _provider = provider; + _loggerFactory = loggerFactory; + } + + /// + public EncryptedXmlInfo Encrypt(XElement plaintextElement) + { + ArgumentNullException.ThrowIfNull(plaintextElement); + + var certificate = _provider.GetCurrentCertificate(); + var inner = new CertificateXmlEncryptor(certificate, _loggerFactory); + var produced = inner.Encrypt(plaintextElement); + + return new EncryptedXmlInfo(produced.EncryptedElement, typeof(GroundControlCertificateXmlDecryptor)); + } +} \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs b/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs index 2f69c3a8..17618417 100644 --- a/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs +++ b/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs @@ -5,7 +5,6 @@ using GroundControl.Host.Api; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.DataProtection.KeyManagement; -using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; namespace GroundControl.Api.Core.DataProtection; @@ -37,18 +36,13 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) if (options.Mode is DataProtectionMode.Certificate or DataProtectionMode.Redis) { + // GroundControlCertificateXmlEncryptor pins GroundControlCertificateXmlDecryptor into + // the persisted key XML, so the decryption side runs through DI on first use — no + // eager certificate load, no UnprotectKeysWithAnyCertificate, no XmlKeyDecryptionOptions + // plumbing. The decryptor itself is activated by the Data Protection IActivator and + // does not need explicit DI registration; only its constructor dependencies do. + builder.Services.AddSingleton(); builder.Services.AddSingleton, CertificateKeyEncryptionConfigurator>(); - - // The decryption side of certificate-based key ring protection cannot be wired through - // IConfigureOptions because XmlKeyDecryptionOptions is internal to ASP.NET Core. The only - // public surface is UnprotectKeysWithAnyCertificate, which captures the certificates at - // registration time. Loading them here and supplying both current and previous certs is - // required for cross-instance and cross-restart decryption to work, and for safe - // certificate rotation. - var startupCertificateProvider = CreateCertificateProvider(options, azureCredential); - var currentCertificate = startupCertificateProvider.GetCurrentCertificate(); - var previousCertificates = startupCertificateProvider.GetPreviousCertificates(); - dataProtectionBuilder.UnprotectKeysWithAnyCertificate([currentCertificate, .. previousCertificates]); } builder.Services.AddSingleton(); @@ -87,16 +81,4 @@ private static void RegisterCertificateProvider(IServiceCollection services, Dat $"Unknown {nameof(DataProtectionOptions)}:{nameof(DataProtectionOptions.CertificateProvider)} '{options.CertificateProvider}'. Supported values: {string.Join(", ", Enum.GetNames())}."); } } - - private static IDataProtectionCertificateProvider CreateCertificateProvider(DataProtectionOptions options, TokenCredential? azureCredential) => options.CertificateProvider switch - { - CertificateProviderMode.FileSystem => new FileSystemCertificateProvider(Options.Create(options.FileSystemCertificate), NullLogger.Instance), - CertificateProviderMode.AzureBlob => new AzureBlobCertificateProvider( - Options.Create(options.AzureBlobCertificate), - azureCredential ?? throw new InvalidOperationException( - $"An Azure credential is required when {nameof(DataProtectionOptions)}:{nameof(DataProtectionOptions.CertificateProvider)} is '{options.CertificateProvider}'."), - NullLogger.Instance), - _ => throw new InvalidOperationException( - $"Unknown {nameof(DataProtectionOptions)}:{nameof(DataProtectionOptions.CertificateProvider)} '{options.CertificateProvider}'. Supported values: {string.Join(", ", Enum.GetNames())}.") - }; } \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/CertificateKeyEncryptionConfiguratorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/CertificateKeyEncryptionConfiguratorTests.cs index 41083350..78196a4e 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/CertificateKeyEncryptionConfiguratorTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/CertificateKeyEncryptionConfiguratorTests.cs @@ -14,39 +14,22 @@ public sealed class CertificateKeyEncryptionConfiguratorTests : IDisposable private readonly List _certificates = []; [Fact] - public void Configure_SetsXmlEncryptorOnKeyManagementOptions() + public void Configure_SetsTheGroundControlEncryptorOnKeyManagementOptions() { // Arrange var certificate = CreateSelfSignedCertificate(); var provider = Substitute.For(); provider.GetCurrentCertificate().Returns(certificate); - var configurator = new CertificateKeyEncryptionConfigurator(provider, NullLoggerFactory.Instance); + var encryptor = new GroundControlCertificateXmlEncryptor(provider, NullLoggerFactory.Instance); + var configurator = new CertificateKeyEncryptionConfigurator(encryptor); var options = new KeyManagementOptions(); // Act configurator.Configure(options); // Assert - options.XmlEncryptor.ShouldNotBeNull(); - } - - [Fact] - public void Configure_CallsGetCurrentCertificate() - { - // Arrange - var certificate = CreateSelfSignedCertificate(); - var provider = Substitute.For(); - provider.GetCurrentCertificate().Returns(certificate); - - var configurator = new CertificateKeyEncryptionConfigurator(provider, NullLoggerFactory.Instance); - var options = new KeyManagementOptions(); - - // Act - configurator.Configure(options); - - // Assert - provider.Received(1).GetCurrentCertificate(); + options.XmlEncryptor.ShouldBeSameAs(encryptor); } public void Dispose() diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlRoundTripTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlRoundTripTests.cs new file mode 100644 index 00000000..7940e680 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlRoundTripTests.cs @@ -0,0 +1,110 @@ +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using System.Xml.Linq; +using GroundControl.Api.Core.DataProtection.Certificate; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection; + +/// +/// Verifies that and +/// round-trip XML payloads, including across +/// certificate rotation where the cert that protected a payload has moved into the previous list. +/// +public sealed class GroundControlCertificateXmlRoundTripTests : IDisposable +{ + private readonly List _certificates = []; + + [Fact] + public void Encrypt_PinsTheGroundControlDecryptorTypeIntoTheKeyMetadata() + { + // Arrange + var certificate = CreateSelfSignedCertificate(); + var provider = Substitute.For(); + provider.GetCurrentCertificate().Returns(certificate); + + var encryptor = new GroundControlCertificateXmlEncryptor(provider, NullLoggerFactory.Instance); + var plaintext = new XElement("secret", "round-trip-payload"); + + // Act + var encrypted = encryptor.Encrypt(plaintext); + + // Assert + encrypted.DecryptorType.ShouldBe(typeof(GroundControlCertificateXmlDecryptor)); + encrypted.EncryptedElement.ShouldNotBeNull(); + } + + [Fact] + public void EncryptThenDecrypt_RestoresOriginalElement_WithCurrentCertificate() + { + // Arrange + var certificate = CreateSelfSignedCertificate(); + var provider = Substitute.For(); + provider.GetCurrentCertificate().Returns(certificate); + provider.GetPreviousCertificates().Returns([]); + + var encryptor = new GroundControlCertificateXmlEncryptor(provider, NullLoggerFactory.Instance); + var decryptor = new GroundControlCertificateXmlDecryptor(provider, NullLogger.Instance); + var plaintext = new XElement("secret", new XAttribute("kind", "value"), "before-rotation"); + + // Act + var encrypted = encryptor.Encrypt(plaintext); + var decrypted = decryptor.Decrypt(encrypted.EncryptedElement); + + // Assert + XNode.DeepEquals(decrypted, plaintext).ShouldBeTrue("decrypted element should be structurally equal to the original"); + } + + [Fact] + public void DecryptUnderRotatedKey_FindsCertificateInPreviousList() + { + // Arrange — Encrypt under c1 while it is current. + var c1 = CreateSelfSignedCertificate(); + var encryptionProvider = Substitute.For(); + encryptionProvider.GetCurrentCertificate().Returns(c1); + + var encryptor = new GroundControlCertificateXmlEncryptor(encryptionProvider, NullLoggerFactory.Instance); + var plaintext = new XElement("secret", "encrypted-under-c1"); + var encrypted = encryptor.Encrypt(plaintext); + + // Act — Now c2 is current and c1 has been moved to previous; decrypt the c1 payload. + var c2 = CreateSelfSignedCertificate(); + var rotatedProvider = Substitute.For(); + rotatedProvider.GetCurrentCertificate().Returns(c2); + rotatedProvider.GetPreviousCertificates().Returns([c1]); + + var decryptor = new GroundControlCertificateXmlDecryptor(rotatedProvider, NullLogger.Instance); + var decrypted = decryptor.Decrypt(encrypted.EncryptedElement); + + // Assert + XNode.DeepEquals(decrypted, plaintext).ShouldBeTrue(); + } + + public void Dispose() + { + foreach (var cert in _certificates) + { + cert.Dispose(); + } + } + + private X509Certificate2 CreateSelfSignedCertificate() + { + using var rsa = RSA.Create(2048); + var request = new CertificateRequest( + "CN=GroundControl Test Certificate", + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + var certificate = request.CreateSelfSigned( + DateTimeOffset.UtcNow, + DateTimeOffset.UtcNow.AddYears(1)); + + _certificates.Add(certificate); + return certificate; + } +} \ No newline at end of file From 3cadc0a7ced4e6d3812e3732b9a129b677933335 Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Thu, 7 May 2026 10:09:42 +0100 Subject: [PATCH 09/12] perf(api): cache inner CertificateXmlEncryptor per certificate GroundControlCertificateXmlEncryptor previously constructed a fresh framework CertificateXmlEncryptor on every Encrypt call. Cache it in a single-slot keyed by thumbprint so we only rebuild when the underlying certificate changes. Single-reference assignment is atomic, so no lock is needed; concurrent first-time writes may construct a duplicate inner encryptor (harmless). Also adds a negative round-trip test asserting that decryption with no matching certificate throws CryptographicException and emits the "no matching cert" warning naming the orphaned thumbprint. --- .../GroundControlCertificateXmlEncryptor.cs | 18 +++++++++- ...oundControlCertificateXmlRoundTripTests.cs | 33 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs index 97886672..34cbedce 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs @@ -1,3 +1,4 @@ +using System.Security.Cryptography.X509Certificates; using System.Xml.Linq; using Microsoft.AspNetCore.DataProtection.XmlEncryption; @@ -21,6 +22,7 @@ internal sealed class GroundControlCertificateXmlEncryptor : IXmlEncryptor { private readonly IDataProtectionCertificateProvider _provider; private readonly ILoggerFactory _loggerFactory; + private InnerEncryptorCache? _cache; public GroundControlCertificateXmlEncryptor(IDataProtectionCertificateProvider provider, ILoggerFactory loggerFactory) { @@ -34,9 +36,23 @@ public EncryptedXmlInfo Encrypt(XElement plaintextElement) ArgumentNullException.ThrowIfNull(plaintextElement); var certificate = _provider.GetCurrentCertificate(); - var inner = new CertificateXmlEncryptor(certificate, _loggerFactory); + var inner = ResolveInnerEncryptor(certificate); var produced = inner.Encrypt(plaintextElement); return new EncryptedXmlInfo(produced.EncryptedElement, typeof(GroundControlCertificateXmlDecryptor)); } + + private CertificateXmlEncryptor ResolveInnerEncryptor(X509Certificate2 certificate) + { + var existing = _cache; + if (existing is not null && string.Equals(existing.Thumbprint, certificate.Thumbprint, StringComparison.OrdinalIgnoreCase)) + { + return existing.Encryptor; + } + + _cache = new InnerEncryptorCache(certificate.Thumbprint, new CertificateXmlEncryptor(certificate, _loggerFactory)); + return _cache.Encryptor; + } + + private sealed record InnerEncryptorCache(string Thumbprint, CertificateXmlEncryptor Encryptor); } \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlRoundTripTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlRoundTripTests.cs index 7940e680..bbbf52d5 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlRoundTripTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlRoundTripTests.cs @@ -2,7 +2,9 @@ using System.Security.Cryptography.X509Certificates; using System.Xml.Linq; using GroundControl.Api.Core.DataProtection.Certificate; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Logging.Testing; using NSubstitute; using Shouldly; using Xunit; @@ -83,6 +85,37 @@ public void DecryptUnderRotatedKey_FindsCertificateInPreviousList() XNode.DeepEquals(decrypted, plaintext).ShouldBeTrue(); } + [Fact] + public void Decrypt_WhenNoMatchingCertificate_ThrowsAndLogsWarning() + { + // Arrange — Encrypt under c1, then build a provider that knows nothing about c1. + var c1 = CreateSelfSignedCertificate(); + var encryptionProvider = Substitute.For(); + encryptionProvider.GetCurrentCertificate().Returns(c1); + + var encryptor = new GroundControlCertificateXmlEncryptor(encryptionProvider, NullLoggerFactory.Instance); + var encrypted = encryptor.Encrypt(new XElement("secret", "orphaned-payload")); + + var unrelated = CreateSelfSignedCertificate(); + var unrelatedProvider = Substitute.For(); + unrelatedProvider.GetCurrentCertificate().Returns(unrelated); + unrelatedProvider.GetPreviousCertificates().Returns([]); + + var collector = new FakeLogCollector(); + var logger = new FakeLogger(collector); + var decryptor = new GroundControlCertificateXmlDecryptor(unrelatedProvider, logger); + + // Act & Assert — decryption falls through to base EncryptedXml which can't find the cert + // in the OS store either, so a CryptographicException surfaces. Before the throw we should + // have logged a warning naming the orphaned thumbprint. + Should.Throw(() => decryptor.Decrypt(encrypted.EncryptedElement)); + + var snapshot = collector.GetSnapshot(); + var warning = snapshot.ShouldHaveSingleItem(); + warning.Level.ShouldBe(LogLevel.Warning); + warning.Message.ShouldContain(c1.Thumbprint, Case.Insensitive); + } + public void Dispose() { foreach (var cert in _certificates) From 6cb6639197be29a31df4bdec59e23554eae6e7bd Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Thu, 7 May 2026 12:37:59 +0100 Subject: [PATCH 10/12] refactor(api): remove redundant CertificateStartupLogger The hosted service existed to (a) log the active cert thumbprint at startup and (b) act as a fail-fast checkpoint by eagerly loading the cert. After the IXmlEncryptor/IXmlDecryptor refactor: - Logging is already covered by FileSystemCertificateProvider and AzureBlobCertificateProvider, which log thumbprints on every load. - Cert loading is now fully lazy through the DI-resolved provider, so removing the logger means the host no longer fail-fasts on a missing cert; misconfig will surface on the first request that protects or unprotects a sensitive value instead. --- .../Certificate/CertificateStartupLogger.cs | 23 ------------------- .../DataProtection/DataProtectionModule.cs | 1 - 2 files changed, 24 deletions(-) delete mode 100644 src/GroundControl.Api/Core/DataProtection/Certificate/CertificateStartupLogger.cs diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateStartupLogger.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateStartupLogger.cs deleted file mode 100644 index dacccd99..00000000 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/CertificateStartupLogger.cs +++ /dev/null @@ -1,23 +0,0 @@ -namespace GroundControl.Api.Core.DataProtection.Certificate; - -/// -/// Loads the Data Protection certificate at startup to verify it is accessible -/// and to log the certificate thumbprint. -/// -internal sealed partial class CertificateStartupLogger(IDataProtectionCertificateProvider provider, ILogger logger) : IHostedService -{ - /// - public Task StartAsync(CancellationToken cancellationToken) - { - using var certificate = provider.GetCurrentCertificate(); - LogCertificateReady(logger, certificate.Thumbprint); - - return Task.CompletedTask; - } - - /// - public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; - - [LoggerMessage(1, LogLevel.Information, "Data Protection certificate ready with thumbprint {Thumbprint}.")] - private static partial void LogCertificateReady(ILogger logger, string thumbprint); -} \ No newline at end of file diff --git a/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs b/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs index 17618417..ea46833f 100644 --- a/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs +++ b/src/GroundControl.Api/Core/DataProtection/DataProtectionModule.cs @@ -28,7 +28,6 @@ public void OnServiceConfiguration(WebApplicationBuilder builder) if (options.CertificateProvider.HasValue) { RegisterCertificateProvider(builder.Services, options); - builder.Services.AddHostedService(); } var keyRingConfigurator = CreateKeyRingConfigurator(options.Mode, azureCredential); From a08dc9f62881255998374674b1f2aa9d250cde78 Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Thu, 7 May 2026 17:15:53 +0100 Subject: [PATCH 11/12] refactor(api): add BlobClient factory seam to AzureBlobCertificateProvider Production wiring is unchanged: the public ctor still constructs a real BlobClient against the supplied URI and TokenCredential. The internal overload accepts a Func so unit tests can mock the download path without depending on Azurite or live Azure. ActivatorUtilities only considers public constructors, so DI keeps picking the production ctor and ignores the seam. --- .../Certificate/AzureBlobCertificateProvider.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs index c70382f4..b42ba23d 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/AzureBlobCertificateProvider.cs @@ -20,15 +20,26 @@ internal sealed partial class AzureBlobCertificateProvider : IDataProtectionCert private readonly AzureBlobCertificateOptions _options; private readonly TokenCredential _credential; private readonly ILogger _logger; + private readonly Func _blobClientFactory; - public AzureBlobCertificateProvider( + public AzureBlobCertificateProvider(IOptions options, TokenCredential credential, ILogger logger) + : this(options, credential, logger, static (uri, cred) => new BlobClient(uri, cred)) + { + } + + /// + /// Initializes a new instance of the class for testing, allowing injection of a custom BlobClient factory. + /// + internal AzureBlobCertificateProvider( IOptions options, TokenCredential credential, - ILogger logger) + ILogger logger, + Func blobClientFactory) { _options = options.Value; _credential = credential; _logger = logger; + _blobClientFactory = blobClientFactory; } /// @@ -51,7 +62,7 @@ public IReadOnlyList GetPreviousCertificates() private X509Certificate2 DownloadCertificate(Uri blobUri, string source) { - var client = new BlobClient(blobUri, _credential); + var client = _blobClientFactory(blobUri, _credential); var response = client.DownloadContent(); var pfxBytes = response.Value.Content.ToArray(); From d0a8334ae773427e1edabf7d242da0e08200e941 Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Thu, 7 May 2026 17:16:11 +0100 Subject: [PATCH 12/12] test(api): expand Data Protection test coverage Fill the gaps in Data Protection coverage that the existing lifecycle suite did not exercise: - options validators (DataProtectionOptions, AzureCredentialOptions) - AzureCredentialFactory mode-to-credential mapping - DataProtectionModule DI registration across every mode/provider combo - GroundControlCertificateXmlEncryptor inner-encryptor cache identity and concurrency - GroundControlCertificateXmlDecryptor edge cases (null args, missing private key, SimpleActivator-compatible IServiceProvider ctor) - AzureBlobCertificateProvider download/load via the new factory seam - FileSystem DPAPI Windows / non-Windows branches - Redis connection-failure wrap, Azure registration happy path - DataProtectionValueProtector empty/unicode/large payloads, malformed input, multi-protector interop, concurrent round-trip - DataProtectionOptions configuration binding (JSON + env vars) - FileSystem and Redis scaled-out scenarios (live peer reads) - Certificate rotation negative paths (premature old-cert removal) Adds Azure.Storage.Blobs to the test project so AzureBlobCertificateProviderTests can construct mocked BlobDownloadResult responses via BlobsModelFactory. --- .../GroundControlCertificateXmlEncryptor.cs | 8 +- .../DataProtection/DataProtectionOptions.cs | 15 +- .../GroundControl.AppHost.csproj | 8 +- .../AzureBlobCertificateProviderTests.cs | 217 ++++++++++++ .../AzureCredentialFactoryTests.cs | 209 +++++++++++ .../AzureCredentialOptionsValidatorTests.cs | 169 +++++++++ .../AzureKeyRingConfiguratorTests.cs | 37 ++ .../DataProtectionModuleTests.cs | 240 +++++++++++++ .../DataProtectionOptionsBindingTests.cs | 214 +++++++++++ .../DataProtectionOptionsValidatorTests.cs | 331 ++++++++++++++++++ .../DataProtectionValueProtectorTests.cs | 112 ++++++ .../FileSystemKeyRingConfiguratorTests.cs | 83 +++++ ...lCertificateXmlDecryptorActivationTests.cs | 102 ++++++ ...oundControlCertificateXmlDecryptorTests.cs | 109 ++++++ ...oundControlCertificateXmlEncryptorTests.cs | 144 ++++++++ .../Lifecycle/CertificateRotationTests.cs | 31 ++ .../Lifecycle/FileSystemScaledOutTests.cs | 77 ++++ .../Redis/RedisCertificateRotationTests.cs | 50 +++ .../Lifecycle/Redis/RedisScaledOutTests.cs | 55 +++ .../RedisKeyRingConfiguratorTests.cs | 31 ++ .../GroundControl.Api.Tests.csproj | 1 + 21 files changed, 2229 insertions(+), 14 deletions(-) create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/AzureBlobCertificateProviderTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/AzureCredentialFactoryTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/AzureCredentialOptionsValidatorTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionModuleTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionOptionsBindingTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionOptionsValidatorTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlDecryptorActivationTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlDecryptorTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlEncryptorTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemScaledOutTests.cs create mode 100644 tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisScaledOutTests.cs diff --git a/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs b/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs index 34cbedce..7696d3b8 100644 --- a/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs +++ b/src/GroundControl.Api/Core/DataProtection/Certificate/GroundControlCertificateXmlEncryptor.cs @@ -44,14 +44,16 @@ public EncryptedXmlInfo Encrypt(XElement plaintextElement) private CertificateXmlEncryptor ResolveInnerEncryptor(X509Certificate2 certificate) { - var existing = _cache; + var existing = Volatile.Read(ref _cache); if (existing is not null && string.Equals(existing.Thumbprint, certificate.Thumbprint, StringComparison.OrdinalIgnoreCase)) { return existing.Encryptor; } - _cache = new InnerEncryptorCache(certificate.Thumbprint, new CertificateXmlEncryptor(certificate, _loggerFactory)); - return _cache.Encryptor; + var fresh = new InnerEncryptorCache(certificate.Thumbprint, new CertificateXmlEncryptor(certificate, _loggerFactory)); + Volatile.Write(ref _cache, fresh); + + return fresh.Encryptor; } private sealed record InnerEncryptorCache(string Thumbprint, CertificateXmlEncryptor Encryptor); diff --git a/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs b/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs index 4e500432..73f31522 100644 --- a/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs +++ b/src/GroundControl.Api/Core/DataProtection/DataProtectionOptions.cs @@ -69,28 +69,29 @@ internal sealed class Validator : IValidateOptions public ValidateOptionsResult Validate(string? name, DataProtectionOptions options) { var failures = new List(); + var prefix = string.IsNullOrEmpty(name) ? nameof(DataProtectionOptions) : name; if (string.IsNullOrWhiteSpace(options.KeyStorePath)) { - failures.Add($"{nameof(DataProtectionOptions)}:{nameof(KeyStorePath)} is required."); + failures.Add($"{prefix}:{nameof(KeyStorePath)} is required."); } if (options.Mode is DataProtectionMode.Certificate or DataProtectionMode.Redis && !options.CertificateProvider.HasValue) { - failures.Add($"{nameof(DataProtectionOptions)}:{nameof(CertificateProvider)} must be configured when Mode is '{options.Mode}'."); + failures.Add($"{prefix}:{nameof(CertificateProvider)} must be configured when Mode is '{options.Mode}'."); } switch (options.CertificateProvider) { case CertificateProviderMode.FileSystem: - if (!FileSystemCertificateOptions.Validator.TryValidate(options.FileSystemCertificate, out var fsFailures, nameof(FileSystemCertificate))) + if (!FileSystemCertificateOptions.Validator.TryValidate(options.FileSystemCertificate, out var fsFailures, $"{prefix}:{nameof(FileSystemCertificate)}")) { failures.AddRange(fsFailures); } break; case CertificateProviderMode.AzureBlob: - if (!AzureBlobCertificateOptions.Validator.TryValidate(options.AzureBlobCertificate, out var blobFailures, nameof(AzureBlobCertificate))) + if (!AzureBlobCertificateOptions.Validator.TryValidate(options.AzureBlobCertificate, out var blobFailures, $"{prefix}:{nameof(AzureBlobCertificate)}")) { failures.AddRange(blobFailures); } @@ -99,7 +100,7 @@ public ValidateOptionsResult Validate(string? name, DataProtectionOptions option if (options.Mode is DataProtectionMode.Azure || options.CertificateProvider is CertificateProviderMode.AzureBlob) { - if (!AzureCredentialOptions.Validator.TryValidate(options.AzureCredential, out var credentialFailures, nameof(AzureCredential))) + if (!AzureCredentialOptions.Validator.TryValidate(options.AzureCredential, out var credentialFailures, $"{prefix}:{nameof(AzureCredential)}")) { failures.AddRange(credentialFailures); } @@ -107,11 +108,11 @@ public ValidateOptionsResult Validate(string? name, DataProtectionOptions option switch (options.Mode) { - case DataProtectionMode.Redis when !RedisOptions.Validator.TryValidate(options.Redis, out var redisFailures, nameof(Redis)): + case DataProtectionMode.Redis when !RedisOptions.Validator.TryValidate(options.Redis, out var redisFailures, $"{prefix}:{nameof(Redis)}"): failures.AddRange(redisFailures); break; - case DataProtectionMode.Azure when !AzureOptions.Validator.TryValidate(options.Azure, out var azureFailures, nameof(Azure)): + case DataProtectionMode.Azure when !AzureOptions.Validator.TryValidate(options.Azure, out var azureFailures, $"{prefix}:{nameof(Azure)}"): failures.AddRange(azureFailures); break; } diff --git a/src/GroundControl.AppHost/GroundControl.AppHost.csproj b/src/GroundControl.AppHost/GroundControl.AppHost.csproj index d3dd7beb..cfa691bd 100644 --- a/src/GroundControl.AppHost/GroundControl.AppHost.csproj +++ b/src/GroundControl.AppHost/GroundControl.AppHost.csproj @@ -9,13 +9,13 @@ - - + + - - + + diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/AzureBlobCertificateProviderTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/AzureBlobCertificateProviderTests.cs new file mode 100644 index 00000000..156c183a --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/AzureBlobCertificateProviderTests.cs @@ -0,0 +1,217 @@ +using System.Diagnostics.CodeAnalysis; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Azure; +using Azure.Core; +using Azure.Storage.Blobs; +using Azure.Storage.Blobs.Models; +using GroundControl.Api.Core.DataProtection.Certificate; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using NSubstitute; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection; + +/// +/// Verifies downloads PFX bytes via the injected +/// BlobClient factory and loads them as X.509 certificates. We mock BlobClient +/// directly: the production network/auth path is the Azure SDK's responsibility, and Azurite's +/// API version trails the SDK package the project pins, so an integration container would be +/// flaky. The provider's own logic — choose blob → download → decode PFX — is fully covered here. +/// +public sealed class AzureBlobCertificateProviderTests +{ + private readonly TokenCredential _credential = Substitute.For(); + private readonly ILogger _logger = NullLogger.Instance; + + [Fact] + public void GetCurrentCertificate_DownloadsAndLoadsBlob_NoPassword() + { + // Arrange + var pfxBytes = CreateSelfSignedPfxBytes(password: null); + var blobUri = new Uri("https://account.blob.core.windows.net/dp-certs/current.pfx"); + var provider = CreateProvider(blobUri, password: null, downloadResponse: BuildDownloadResponse(pfxBytes)); + + // Act + using var certificate = provider.GetCurrentCertificate(); + + // Assert + certificate.HasPrivateKey.ShouldBeTrue(); + certificate.Thumbprint.ShouldNotBeNullOrEmpty(); + } + + [Fact] + public void GetCurrentCertificate_DownloadsAndLoadsBlob_WithPassword() + { + // Arrange + const string Password = "blob-pfx-password"; + var pfxBytes = CreateSelfSignedPfxBytes(Password); + var blobUri = new Uri("https://account.blob.core.windows.net/dp-certs/current.pfx"); + var provider = CreateProvider(blobUri, password: Password, downloadResponse: BuildDownloadResponse(pfxBytes)); + + // Act + using var certificate = provider.GetCurrentCertificate(); + + // Assert + certificate.HasPrivateKey.ShouldBeTrue(); + } + + [Fact] + public void GetCurrentCertificate_WrongPassword_ThrowsCryptographicException() + { + // Arrange + var pfxBytes = CreateSelfSignedPfxBytes(password: "correct-password"); + var blobUri = new Uri("https://account.blob.core.windows.net/dp-certs/current.pfx"); + var provider = CreateProvider(blobUri, password: "wrong-password", downloadResponse: BuildDownloadResponse(pfxBytes)); + + // Act + Assert + Should.Throw(() => provider.GetCurrentCertificate()); + } + + [Fact] + public void GetCurrentCertificate_BlobClientThrows_PropagatesException() + { + // Arrange — A 404-style failure surfacing through the real BlobClient implementation. + var blobUri = new Uri("https://account.blob.core.windows.net/dp-certs/missing.pfx"); + var blobClient = MockBlobClientThrowing(new RequestFailedException(404, "Not Found")); + + var options = new AzureBlobCertificateOptions + { + BlobUri = blobUri, + PreviousBlobUris = [] + }; + var provider = new AzureBlobCertificateProvider( + Options.Create(options), + _credential, + _logger, + (_, _) => blobClient); + + // Act + Assert + Should.Throw(() => provider.GetCurrentCertificate()); + } + + [Fact] + public void GetPreviousCertificates_NoneConfigured_ReturnsEmpty() + { + // Arrange + var options = new AzureBlobCertificateOptions + { + BlobUri = new Uri("https://account.blob.core.windows.net/dp-certs/current.pfx"), + PreviousBlobUris = [] + }; + var provider = new AzureBlobCertificateProvider( + Options.Create(options), + _credential, + _logger, + (_, _) => Substitute.For()); + + // Act + var result = provider.GetPreviousCertificates(); + + // Assert + result.ShouldBeEmpty(); + } + + [Fact] + public void GetPreviousCertificates_LoadsAllConfiguredBlobs() + { + // Arrange + var firstUri = new Uri("https://account.blob.core.windows.net/dp-certs/prev-1.pfx"); + var secondUri = new Uri("https://account.blob.core.windows.net/dp-certs/prev-2.pfx"); + var firstClient = BuildBlobClient(BuildDownloadResponse(CreateSelfSignedPfxBytes(password: null))); + var secondClient = BuildBlobClient(BuildDownloadResponse(CreateSelfSignedPfxBytes(password: null))); + + var clientsByUri = new Dictionary + { + [firstUri] = firstClient, + [secondUri] = secondClient + }; + + var options = new AzureBlobCertificateOptions + { + BlobUri = firstUri, + PreviousBlobUris = [firstUri, secondUri] + }; + var provider = new AzureBlobCertificateProvider( + Options.Create(options), + _credential, + _logger, + (uri, _) => clientsByUri[uri]); + + // Act + var certificates = provider.GetPreviousCertificates(); + + // Assert + certificates.Count.ShouldBe(2); + certificates.ShouldAllBe(c => c.HasPrivateKey); + + foreach (var cert in certificates) + { + cert.Dispose(); + } + } + + private AzureBlobCertificateProvider CreateProvider(Uri blobUri, string? password, Response downloadResponse) + { + var options = new AzureBlobCertificateOptions + { + BlobUri = blobUri, + Password = password, + PreviousBlobUris = [] + }; + + return new AzureBlobCertificateProvider( + Options.Create(options), + _credential, + _logger, + (_, _) => BuildBlobClient(downloadResponse)); + } + + [SuppressMessage( + "Usage", + "xUnit1051:Calls to methods which accept CancellationToken should use TestContext.Current.CancellationToken", + Justification = "The mocked overload must match the parameterless DownloadContent the production code invokes.")] + private static BlobClient BuildBlobClient(Response downloadResponse) + { + var client = Substitute.For(); + client.DownloadContent().Returns(downloadResponse); + return client; + } + + [SuppressMessage( + "Usage", + "xUnit1051:Calls to methods which accept CancellationToken should use TestContext.Current.CancellationToken", + Justification = "The mocked overload must match the parameterless DownloadContent the production code invokes.")] + private static BlobClient MockBlobClientThrowing(Exception exception) + { + var client = Substitute.For(); + client.DownloadContent().Returns>(_ => throw exception); + return client; + } + + private static Response BuildDownloadResponse(byte[] pfxBytes) + { + var result = BlobsModelFactory.BlobDownloadResult(BinaryData.FromBytes(pfxBytes)); + var response = Substitute.For(); + return Response.FromValue(result, response); + } + + private static byte[] CreateSelfSignedPfxBytes(string? password) + { + using var rsa = RSA.Create(2048); + var request = new CertificateRequest( + "CN=GroundControl Test", + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + using var certificate = request.CreateSelfSigned( + DateTimeOffset.UtcNow.AddMinutes(-1), + DateTimeOffset.UtcNow.AddYears(1)); + + return certificate.Export(X509ContentType.Pfx, password); + } +} \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/AzureCredentialFactoryTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/AzureCredentialFactoryTests.cs new file mode 100644 index 00000000..c66a7586 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/AzureCredentialFactoryTests.cs @@ -0,0 +1,209 @@ +using System.Reflection; +using Azure.Identity; +using GroundControl.Api.Core.DataProtection; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection; + +/// +/// Verifies that returns the correct concrete +/// TokenCredential for each and threads the configured +/// fields through to the credential. +/// +public sealed class AzureCredentialFactoryTests +{ + [Fact] + public void Create_Default_ReturnsDefaultAzureCredential() + { + // Arrange + var options = new AzureCredentialOptions { Mode = AzureCredentialType.Default }; + + // Act + var credential = AzureCredentialFactory.Create(options); + + // Assert + credential.ShouldBeOfType(); + } + + [Fact] + public void Create_ManagedIdentity_NoClientId_ReturnsManagedIdentityCredential() + { + // Arrange + var options = new AzureCredentialOptions { Mode = AzureCredentialType.ManagedIdentity }; + + // Act + var credential = AzureCredentialFactory.Create(options); + + // Assert + credential.ShouldBeOfType(); + } + + [Fact] + public void Create_ManagedIdentity_WithClientId_ReturnsManagedIdentityCredential() + { + // Arrange + var options = new AzureCredentialOptions + { + Mode = AzureCredentialType.ManagedIdentity, + ClientId = "11111111-1111-1111-1111-111111111111" + }; + + // Act + var credential = AzureCredentialFactory.Create(options); + + // Assert + credential.ShouldBeOfType(); + } + + [Fact] + public void Create_WorkloadIdentity_ReturnsWorkloadIdentityCredential() + { + // Arrange + var options = new AzureCredentialOptions + { + Mode = AzureCredentialType.WorkloadIdentity, + TenantId = "tenant-id", + ClientId = "client-id", + TokenFilePath = "/var/run/secrets/tokens/azure-identity-token" + }; + + // Act + var credential = AzureCredentialFactory.Create(options); + + // Assert + credential.ShouldBeOfType(); + } + + [Fact] + public void Create_ClientSecret_ReturnsClientSecretCredential() + { + // Arrange + var options = new AzureCredentialOptions + { + Mode = AzureCredentialType.ClientSecret, + TenantId = "tenant-id", + ClientId = "client-id", + ClientSecret = "secret" + }; + + // Act + var credential = AzureCredentialFactory.Create(options); + + // Assert — Azure.Identity does not expose TenantId/ClientId as public properties on + // ClientSecretCredential, so this test pins only the credential type. Field propagation + // is verified separately via reflection in the AuthorityHost tests. + credential.ShouldBeOfType(); + } + + [Fact] + public void Create_AzureCli_ReturnsAzureCliCredential() + { + // Arrange + var options = new AzureCredentialOptions { Mode = AzureCredentialType.AzureCli }; + + // Act + var credential = AzureCredentialFactory.Create(options); + + // Assert + credential.ShouldBeOfType(); + } + + [Fact] + public void Create_Environment_ReturnsEnvironmentCredential() + { + // Arrange + var options = new AzureCredentialOptions { Mode = AzureCredentialType.Environment }; + + // Act + var credential = AzureCredentialFactory.Create(options); + + // Assert + credential.ShouldBeOfType(); + } + + [Fact] + public void Create_UnknownMode_ThrowsInvalidOperationException() + { + // Arrange — Cast to invalid enum value to exercise the default switch arm. + var options = new AzureCredentialOptions { Mode = (AzureCredentialType)999 }; + + // Act + var ex = Should.Throw(() => AzureCredentialFactory.Create(options)); + + // Assert + ex.Message.ShouldContain(nameof(AzureCredentialOptions.Mode)); + ex.Message.ShouldContain(nameof(AzureCredentialType.Default)); + } + + [Fact] + public void Create_PropagatesAuthorityHost_ToClientSecretCredential() + { + // Arrange + var sovereignAuthority = new Uri("https://login.microsoftonline.us/"); + var options = new AzureCredentialOptions + { + Mode = AzureCredentialType.ClientSecret, + TenantId = "tenant-id", + ClientId = "client-id", + ClientSecret = "secret", + AuthorityHost = sovereignAuthority + }; + + // Act + var credential = AzureCredentialFactory.Create(options); + + // Assert + credential.ShouldBeOfType(); + ReadAuthorityHost(credential).ShouldBe(sovereignAuthority); + } + + [Fact] + public void Create_WorkloadIdentity_AcceptsAuthorityHostWithoutThrowing() + { + // Arrange — WorkloadIdentityCredential nests its options behind a private inner + // ClientAssertionCredential, so the AuthorityHost reflection used elsewhere does not + // reach it. Pin the contract that the factory accepts and forwards the field; deeper + // verification would couple the test to Azure.Identity internals. + var options = new AzureCredentialOptions + { + Mode = AzureCredentialType.WorkloadIdentity, + TenantId = "tenant-id", + ClientId = "client-id", + AuthorityHost = new Uri("https://login.microsoftonline.us/") + }; + + // Act + var credential = AzureCredentialFactory.Create(options); + + // Assert + credential.ShouldBeOfType(); + } + + /// + /// Walks the credential's private fields looking for any nested options object whose + /// AuthorityHost property has been set. This is intentionally tolerant of small SDK + /// shape changes — Azure.Identity wraps each credential's options on a privately held field. + /// + private static Uri? ReadAuthorityHost(object credential) + { + const BindingFlags Flags = BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public; + + foreach (var field in credential.GetType().GetFields(Flags)) + { + var value = field.GetValue(credential); + if (value is null) + { + continue; + } + + var authorityHostProperty = value.GetType().GetProperty(nameof(TokenCredentialOptions.AuthorityHost), Flags); + if (authorityHostProperty?.GetValue(value) is Uri uri) + { + return uri; + } + } + + return null; + } +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/AzureCredentialOptionsValidatorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/AzureCredentialOptionsValidatorTests.cs new file mode 100644 index 00000000..33326dc4 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/AzureCredentialOptionsValidatorTests.cs @@ -0,0 +1,169 @@ +using GroundControl.Api.Core.DataProtection; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection; + +/// +/// Verifies enforces the per-mode field requirements +/// for each and honours the supplied member-name prefix. +/// +public sealed class AzureCredentialOptionsValidatorTests +{ + [Fact] + public void Validate_ReturnsSuccess_ForDefaultMode() => AssertSuccess(AzureCredentialType.Default); + + [Fact] + public void Validate_ReturnsSuccess_ForAzureCliMode() => AssertSuccess(AzureCredentialType.AzureCli); + + [Fact] + public void Validate_ReturnsSuccess_ForEnvironmentMode() => AssertSuccess(AzureCredentialType.Environment); + + private static void AssertSuccess(AzureCredentialType mode) + { + // Arrange + var validator = new AzureCredentialOptions.Validator(); + var options = new AzureCredentialOptions { Mode = mode }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + [Fact] + public void Validate_ManagedIdentity_AcceptsNoClientId_ForSystemAssigned() + { + // Arrange + var validator = new AzureCredentialOptions.Validator(); + var options = new AzureCredentialOptions { Mode = AzureCredentialType.ManagedIdentity }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + [Fact] + public void Validate_ManagedIdentity_AcceptsClientId_ForUserAssigned() + { + // Arrange + var validator = new AzureCredentialOptions.Validator(); + var options = new AzureCredentialOptions + { + Mode = AzureCredentialType.ManagedIdentity, + ClientId = "11111111-1111-1111-1111-111111111111" + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + [Fact] + public void Validate_WorkloadIdentity_RequiresTenantIdAndClientId() + { + // Arrange + var validator = new AzureCredentialOptions.Validator(); + var options = new AzureCredentialOptions { Mode = AzureCredentialType.WorkloadIdentity }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(AzureCredentialOptions.TenantId))); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(AzureCredentialOptions.ClientId))); + } + + [Fact] + public void Validate_WorkloadIdentity_PassesWhenRequiredFieldsPresent() + { + // Arrange + var validator = new AzureCredentialOptions.Validator(); + var options = new AzureCredentialOptions + { + Mode = AzureCredentialType.WorkloadIdentity, + TenantId = "tenant", + ClientId = "client" + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + [Fact] + public void Validate_ClientSecret_RequiresAllThreeFields() + { + // Arrange + var validator = new AzureCredentialOptions.Validator(); + var options = new AzureCredentialOptions { Mode = AzureCredentialType.ClientSecret }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(f => f.Contains(nameof(AzureCredentialOptions.TenantId))); + result.Failures!.ShouldContain(f => f.Contains(nameof(AzureCredentialOptions.ClientId))); + result.Failures!.ShouldContain(f => f.Contains(nameof(AzureCredentialOptions.ClientSecret))); + } + + [Fact] + public void Validate_ClientSecret_PassesWhenAllRequiredFieldsPresent() + { + // Arrange + var validator = new AzureCredentialOptions.Validator(); + var options = new AzureCredentialOptions + { + Mode = AzureCredentialType.ClientSecret, + TenantId = "tenant", + ClientId = "client", + ClientSecret = "shhh" + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + [Fact] + public void Validate_HonoursSuppliedMemberNamePrefix() + { + // Arrange — When the parent options validator passes a name (e.g. "AzureCredential"), + // failure messages should be prefixed with that name rather than the type name. + var validator = new AzureCredentialOptions.Validator(); + var options = new AzureCredentialOptions { Mode = AzureCredentialType.ClientSecret }; + + // Act + var result = validator.Validate(name: "AzureCredential", options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldAllBe(f => f.StartsWith("AzureCredential:", StringComparison.Ordinal)); + } + + [Fact] + public void Validate_DefaultsToTypeName_WhenNoNamePassed() + { + // Arrange + var validator = new AzureCredentialOptions.Validator(); + var options = new AzureCredentialOptions { Mode = AzureCredentialType.ClientSecret }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldAllBe(f => f.StartsWith($"{nameof(AzureCredentialOptions)}:", StringComparison.Ordinal)); + } +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/AzureKeyRingConfiguratorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/AzureKeyRingConfiguratorTests.cs index ccd39c68..c8dc72b9 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/AzureKeyRingConfiguratorTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/AzureKeyRingConfiguratorTests.cs @@ -2,6 +2,7 @@ using GroundControl.Api.Core.DataProtection; using GroundControl.Api.Core.DataProtection.KeyRing; using Microsoft.AspNetCore.DataProtection; +using Microsoft.AspNetCore.DataProtection.KeyManagement; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Shouldly; @@ -57,4 +58,40 @@ public void Configure_OptionsValidationException_WhenKeyVaultKeyIdNotConfigured( exception.Message.ShouldContain("KeyVaultKeyId: The AzureOptions.KeyVaultKeyId field is required."); } + + [Fact] + public void Configure_HappyPath_WiresUpAzureBlobRepositoryAndAzureKeyVaultEncryptor() + { + // Arrange — End-to-end Azure mode requires real Azure (no Key Vault emulator), so this + // test verifies registration only: after Configure runs, KeyManagementOptions points at + // the Azure-flavoured XmlRepository and XmlEncryptor types. No network call is made + // because the underlying clients are constructed lazily. + var options = new DataProtectionOptions + { + Mode = DataProtectionMode.Azure, + Azure = new AzureOptions + { + BlobUri = new Uri("https://account.blob.core.windows.net/keys/key.xml"), + KeyVaultKeyId = new Uri("https://kv.vault.azure.net/keys/dp/abc") + } + }; + + var services = new ServiceCollection(); + var dpBuilder = services.AddDataProtection().SetApplicationName("GroundControl.Tests"); + var configurator = new AzureKeyRingConfigurator(new DefaultAzureCredential()); + + // Act + configurator.Configure(dpBuilder, options); + + var keyManagementOptions = services.BuildServiceProvider() + .GetRequiredService>().Value; + + // Assert — Type names assert the integration with the Azure DataProtection packages + // without taking a hard reference to internal types in those packages. + keyManagementOptions.XmlRepository.ShouldNotBeNull(); + keyManagementOptions.XmlRepository.GetType().Name.ShouldContain("AzureBlob"); + + keyManagementOptions.XmlEncryptor.ShouldNotBeNull(); + keyManagementOptions.XmlEncryptor.GetType().Name.ShouldContain("KeyVault"); + } } \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionModuleTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionModuleTests.cs new file mode 100644 index 00000000..2ba4188e --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionModuleTests.cs @@ -0,0 +1,240 @@ +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Azure.Core; +using GroundControl.Api.Core.DataProtection; +using GroundControl.Api.Core.DataProtection.Certificate; +using GroundControl.Api.Shared.Security.Protection; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.DataProtection.KeyManagement; +using Microsoft.Extensions.Options; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection; + +/// +/// Verifies registers the right services for each +/// / combination, and +/// surfaces validation and unknown-enum failures fast. +/// +public sealed class DataProtectionModuleTests : IDisposable +{ + private readonly string _tempDir = Path.Combine(Path.GetTempPath(), $"gc-module-{Guid.NewGuid():N}"); + + public DataProtectionModuleTests() + { + Directory.CreateDirectory(_tempDir); + } + + [Fact] + public void OnServiceConfiguration_FileSystem_DoesNotRegisterCertificateOrAzureServices() + { + // Arrange + var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions()); + var module = new DataProtectionModule(new DataProtectionOptions + { + Mode = DataProtectionMode.FileSystem, + KeyStorePath = _tempDir + }); + + // Act + module.OnServiceConfiguration(builder); + + // Assert + builder.Services.ShouldContain(d => d.ServiceType == typeof(IValueProtector) + && d.ImplementationType == typeof(DataProtectionValueProtector)); + builder.Services.ShouldNotContain(d => d.ServiceType == typeof(TokenCredential)); + builder.Services.ShouldNotContain(d => d.ServiceType == typeof(IDataProtectionCertificateProvider)); + builder.Services.ShouldNotContain(d => d.ServiceType == typeof(GroundControlCertificateXmlEncryptor)); + builder.Services.ShouldNotContain(d => d.ServiceType == typeof(IConfigureOptions) + && d.ImplementationType == typeof(CertificateKeyEncryptionConfigurator)); + } + + [Fact] + public void OnServiceConfiguration_Certificate_RegistersCertificateProviderAndEncryptorPipeline() + { + // Arrange + var pfxPath = CreateSelfSignedPfx(password: null); + var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions()); + var module = new DataProtectionModule(new DataProtectionOptions + { + Mode = DataProtectionMode.Certificate, + KeyStorePath = _tempDir, + CertificateProvider = CertificateProviderMode.FileSystem, + FileSystemCertificate = new FileSystemCertificateOptions { Path = pfxPath } + }); + + // Act + module.OnServiceConfiguration(builder); + + // Assert + builder.Services.ShouldContain(d => d.ServiceType == typeof(IDataProtectionCertificateProvider) + && d.ImplementationType == typeof(FileSystemCertificateProvider)); + builder.Services.ShouldContain(d => d.ServiceType == typeof(GroundControlCertificateXmlEncryptor)); + builder.Services.ShouldContain(d => d.ServiceType == typeof(IConfigureOptions) + && d.ImplementationType == typeof(CertificateKeyEncryptionConfigurator)); + builder.Services.ShouldContain(d => d.ServiceType == typeof(IValueProtector) + && d.ImplementationType == typeof(DataProtectionValueProtector)); + } + + [Fact] + public void OnServiceConfiguration_AzureBlobCertificateProvider_RegistersTokenCredentialAndAzureBlobProvider() + { + // Arrange + var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions()); + var module = new DataProtectionModule(new DataProtectionOptions + { + Mode = DataProtectionMode.Certificate, + KeyStorePath = _tempDir, + CertificateProvider = CertificateProviderMode.AzureBlob, + AzureBlobCertificate = new AzureBlobCertificateOptions + { + BlobUri = new Uri("https://account.blob.core.windows.net/certs/dp.pfx") + }, + AzureCredential = new AzureCredentialOptions { Mode = AzureCredentialType.Default } + }); + + // Act + module.OnServiceConfiguration(builder); + + // Assert + builder.Services.Count(d => d.ServiceType == typeof(TokenCredential)).ShouldBe(1); + builder.Services.ShouldContain(d => d.ServiceType == typeof(IDataProtectionCertificateProvider) + && d.ImplementationType == typeof(AzureBlobCertificateProvider)); + } + + [Fact] + public void OnServiceConfiguration_AzureMode_RegistersTokenCredentialAndNoCertificateProvider() + { + // Arrange + var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions()); + var module = new DataProtectionModule(new DataProtectionOptions + { + Mode = DataProtectionMode.Azure, + KeyStorePath = _tempDir, + Azure = new AzureOptions + { + BlobUri = new Uri("https://account.blob.core.windows.net/keys/key.xml"), + KeyVaultKeyId = new Uri("https://kv.vault.azure.net/keys/dp/abc") + }, + AzureCredential = new AzureCredentialOptions { Mode = AzureCredentialType.Default } + }); + + // Act + module.OnServiceConfiguration(builder); + + // Assert + builder.Services.ShouldContain(d => d.ServiceType == typeof(TokenCredential)); + builder.Services.ShouldNotContain(d => d.ServiceType == typeof(IDataProtectionCertificateProvider)); + builder.Services.ShouldNotContain(d => d.ServiceType == typeof(GroundControlCertificateXmlEncryptor)); + } + + [Fact] + public void OnServiceConfiguration_AzureMode_AndAzureBlobCertificateProvider_SharesSingleTokenCredential() + { + // Arrange + var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions()); + var module = new DataProtectionModule(new DataProtectionOptions + { + Mode = DataProtectionMode.Azure, + KeyStorePath = _tempDir, + Azure = new AzureOptions + { + BlobUri = new Uri("https://account.blob.core.windows.net/keys/key.xml"), + KeyVaultKeyId = new Uri("https://kv.vault.azure.net/keys/dp/abc") + }, + CertificateProvider = CertificateProviderMode.AzureBlob, + AzureBlobCertificate = new AzureBlobCertificateOptions + { + BlobUri = new Uri("https://account.blob.core.windows.net/certs/dp.pfx") + }, + AzureCredential = new AzureCredentialOptions { Mode = AzureCredentialType.Default } + }); + + // Act + module.OnServiceConfiguration(builder); + + // Assert — both consumers (Azure key ring + AzureBlob certificate provider) share the + // single registered TokenCredential singleton. + builder.Services.Count(d => d.ServiceType == typeof(TokenCredential)).ShouldBe(1); + } + + [Fact] + public void OnServiceConfiguration_AlwaysRegistersIValueProtector() + { + // Arrange + var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions()); + var module = new DataProtectionModule(new DataProtectionOptions + { + Mode = DataProtectionMode.FileSystem, + KeyStorePath = _tempDir + }); + + // Act + module.OnServiceConfiguration(builder); + + // Assert + builder.Services.ShouldContain(d => d.ServiceType == typeof(IValueProtector) + && d.ImplementationType == typeof(DataProtectionValueProtector)); + } + + [Fact] + public void OnServiceConfiguration_ThrowsOptionsValidationException_WhenOptionsInvalid() + { + // Arrange + var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions()); + var module = new DataProtectionModule(new DataProtectionOptions + { + Mode = DataProtectionMode.FileSystem, + KeyStorePath = string.Empty + }); + + // Act & Assert — fails fast before any DI registration. + Should.Throw(() => module.OnServiceConfiguration(builder)); + } + + [Fact] + public void OnServiceConfiguration_ThrowsInvalidOperationException_ForUnknownMode() + { + // Arrange — Cast to invalid enum value to exercise the configurator factory's default arm. + // We must bypass the validator to reach the factory; the validator only catches *known* + // invalid combinations. + var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions()); + var module = new DataProtectionModule(new DataProtectionOptions + { + Mode = (DataProtectionMode)999, + KeyStorePath = _tempDir + }); + + // Act & Assert + var ex = Should.Throw(() => module.OnServiceConfiguration(builder)); + ex.Message.ShouldContain(nameof(DataProtectionOptions.Mode)); + } + + public void Dispose() + { + if (Directory.Exists(_tempDir)) + { + try + { + Directory.Delete(_tempDir, recursive: true); + } + catch (IOException) + { + // Disk locks (AV scans, lingering file handles) should not fail an otherwise green test. + } + } + } + + private string CreateSelfSignedPfx(string? password) + { + using var rsa = RSA.Create(2048); + var request = new CertificateRequest("CN=GroundControl Test", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + using var certificate = request.CreateSelfSigned(DateTimeOffset.UtcNow.AddMinutes(-1), DateTimeOffset.UtcNow.AddYears(1)); + + var pfxBytes = certificate.Export(X509ContentType.Pfx, password); + var pfxPath = Path.Combine(_tempDir, $"{Guid.NewGuid():N}.pfx"); + File.WriteAllBytes(pfxPath, pfxBytes); + return pfxPath; + } +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionOptionsBindingTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionOptionsBindingTests.cs new file mode 100644 index 00000000..fe536dd4 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionOptionsBindingTests.cs @@ -0,0 +1,214 @@ +using GroundControl.Api.Core.DataProtection; +using Microsoft.Extensions.Configuration; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection; + +/// +/// Verifies that the JSON / colon-separated configuration shape documented in +/// docs/guide/server/configuration.md binds correctly into , +/// including the array-shaped previous-paths/URIs and the AzureCredential URI. +/// +public sealed class DataProtectionOptionsBindingTests +{ + [Fact] + public void Bind_MinimalFileSystemConfig_PopulatesScalarFields() + { + // Arrange + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["DataProtection:Mode"] = "FileSystem", + ["DataProtection:KeyStorePath"] = "/keys", + ["DataProtection:UseDpapi"] = "true" + }) + .Build(); + + // Act + var options = configuration.GetSection("DataProtection").Get(); + + // Assert + options.ShouldNotBeNull(); + options.Mode.ShouldBe(DataProtectionMode.FileSystem); + options.KeyStorePath.ShouldBe("/keys"); + options.UseDpapi.ShouldBeTrue(); + } + + [Fact] + public void Bind_FileSystemCertificate_BindsPreviousPathsArray() + { + // Arrange + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["DataProtection:Mode"] = "Certificate", + ["DataProtection:CertificateProvider"] = "FileSystem", + ["DataProtection:FileSystemCertificate:Path"] = "/certs/dp-2026.pfx", + ["DataProtection:FileSystemCertificate:Password"] = "secret", + ["DataProtection:FileSystemCertificate:PreviousPaths:0"] = "/certs/dp-2024.pfx", + ["DataProtection:FileSystemCertificate:PreviousPaths:1"] = "/certs/dp-2025.pfx" + }) + .Build(); + + // Act + var options = configuration.GetSection("DataProtection").Get(); + + // Assert + options.ShouldNotBeNull(); + options.Mode.ShouldBe(DataProtectionMode.Certificate); + options.CertificateProvider.ShouldBe(CertificateProviderMode.FileSystem); + options.FileSystemCertificate.Path.ShouldBe("/certs/dp-2026.pfx"); + options.FileSystemCertificate.Password.ShouldBe("secret"); + options.FileSystemCertificate.PreviousPaths.ShouldBe(["/certs/dp-2024.pfx", "/certs/dp-2025.pfx"]); + } + + [Fact] + public void Bind_AzureBlobCertificate_BindsPreviousBlobUriArray() + { + // Arrange + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["DataProtection:Mode"] = "Certificate", + ["DataProtection:CertificateProvider"] = "AzureBlob", + ["DataProtection:AzureBlobCertificate:BlobUri"] = "https://account.blob.core.windows.net/certs/dp-2026.pfx", + ["DataProtection:AzureBlobCertificate:Password"] = "secret", + ["DataProtection:AzureBlobCertificate:PreviousBlobUris:0"] = "https://account.blob.core.windows.net/certs/dp-2024.pfx", + ["DataProtection:AzureBlobCertificate:PreviousBlobUris:1"] = "https://account.blob.core.windows.net/certs/dp-2025.pfx" + }) + .Build(); + + // Act + var options = configuration.GetSection("DataProtection").Get(); + + // Assert + options.ShouldNotBeNull(); + options.CertificateProvider.ShouldBe(CertificateProviderMode.AzureBlob); + options.AzureBlobCertificate.BlobUri.ShouldBe(new Uri("https://account.blob.core.windows.net/certs/dp-2026.pfx")); + options.AzureBlobCertificate.PreviousBlobUris.Length.ShouldBe(2); + options.AzureBlobCertificate.PreviousBlobUris[0].ShouldBe(new Uri("https://account.blob.core.windows.net/certs/dp-2024.pfx")); + options.AzureBlobCertificate.PreviousBlobUris[1].ShouldBe(new Uri("https://account.blob.core.windows.net/certs/dp-2025.pfx")); + } + + [Fact] + public void Bind_AzureCredential_BindsAuthorityHostUri() + { + // Arrange + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["DataProtection:Mode"] = "Azure", + ["DataProtection:AzureCredential:Mode"] = "ClientSecret", + ["DataProtection:AzureCredential:TenantId"] = "tenant", + ["DataProtection:AzureCredential:ClientId"] = "client", + ["DataProtection:AzureCredential:ClientSecret"] = "secret", + ["DataProtection:AzureCredential:AuthorityHost"] = "https://login.microsoftonline.us/" + }) + .Build(); + + // Act + var options = configuration.GetSection("DataProtection").Get(); + + // Assert + options.ShouldNotBeNull(); + options.AzureCredential.Mode.ShouldBe(AzureCredentialType.ClientSecret); + options.AzureCredential.AuthorityHost.ShouldBe(new Uri("https://login.microsoftonline.us/")); + options.AzureCredential.TenantId.ShouldBe("tenant"); + options.AzureCredential.ClientId.ShouldBe("client"); + options.AzureCredential.ClientSecret.ShouldBe("secret"); + } + + [Fact] + public void Bind_RedisOptions_BindsAllScalarFields() + { + // Arrange + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["DataProtection:Mode"] = "Redis", + ["DataProtection:Redis:ConnectionString"] = "redis-host:6379", + ["DataProtection:Redis:KeyName"] = "groundcontrol-keys", + ["DataProtection:Redis:ConnectTimeoutMs"] = "1500" + }) + .Build(); + + // Act + var options = configuration.GetSection("DataProtection").Get(); + + // Assert + options.ShouldNotBeNull(); + options.Mode.ShouldBe(DataProtectionMode.Redis); + options.Redis.ConnectionString.ShouldBe("redis-host:6379"); + options.Redis.KeyName.ShouldBe("groundcontrol-keys"); + options.Redis.ConnectTimeoutMs.ShouldBe(1500); + } + + [Fact] + public void Bind_AzureMode_BindsBlobUriAndKeyVaultKeyId() + { + // Arrange + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["DataProtection:Mode"] = "Azure", + ["DataProtection:Azure:BlobUri"] = "https://account.blob.core.windows.net/keys/key.xml", + ["DataProtection:Azure:KeyVaultKeyId"] = "https://kv.vault.azure.net/keys/dp/abc" + }) + .Build(); + + // Act + var options = configuration.GetSection("DataProtection").Get(); + + // Assert + options.ShouldNotBeNull(); + options.Azure.BlobUri.ShouldBe(new Uri("https://account.blob.core.windows.net/keys/key.xml")); + options.Azure.KeyVaultKeyId.ShouldBe(new Uri("https://kv.vault.azure.net/keys/dp/abc")); + } + + [Fact] + public void Bind_FromEnvironmentVariables_ProducesSameShapeAsJson() + { + // Arrange — Set process env vars with the documented `__` delimiter, then bind through + // ConfigurationBuilder.AddEnvironmentVariables(). Cleaned up in Dispose. + var prefix = $"GC_DP_BINDING_{Guid.NewGuid():N}_"; + var keys = new Dictionary + { + [$"{prefix}DataProtection__Mode"] = "Redis", + [$"{prefix}DataProtection__CertificateProvider"] = "FileSystem", + [$"{prefix}DataProtection__FileSystemCertificate__Path"] = "/certs/dp.pfx", + [$"{prefix}DataProtection__Redis__ConnectionString"] = "redis-host:6379", + [$"{prefix}DataProtection__Redis__KeyName"] = "test-keys" + }; + + try + { + foreach (var (key, value) in keys) + { + Environment.SetEnvironmentVariable(key, value); + } + + var configuration = new ConfigurationBuilder() + .AddEnvironmentVariables(prefix) + .Build(); + + // Act + var options = configuration.GetSection("DataProtection").Get(); + + // Assert + options.ShouldNotBeNull(); + options.Mode.ShouldBe(DataProtectionMode.Redis); + options.CertificateProvider.ShouldBe(CertificateProviderMode.FileSystem); + options.FileSystemCertificate.Path.ShouldBe("/certs/dp.pfx"); + options.Redis.ConnectionString.ShouldBe("redis-host:6379"); + options.Redis.KeyName.ShouldBe("test-keys"); + } + finally + { + foreach (var key in keys.Keys) + { + Environment.SetEnvironmentVariable(key, value: null); + } + } + } +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionOptionsValidatorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionOptionsValidatorTests.cs new file mode 100644 index 00000000..bd1a82d3 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionOptionsValidatorTests.cs @@ -0,0 +1,331 @@ +using GroundControl.Api.Core.DataProtection; +using GroundControl.Api.Core.DataProtection.Certificate; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection; + +/// +/// Verifies enforces mode/provider consistency +/// and dispatches to the right sub-options validators for the active mode. +/// +public sealed class DataProtectionOptionsValidatorTests +{ + [Fact] + public void Validate_ReturnsFail_WhenKeyStorePathIsEmpty() + { + // Arrange + var validator = new DataProtectionOptions.Validator(); + var options = ValidFileSystemOptions(); + options.KeyStorePath = string.Empty; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(DataProtectionOptions.KeyStorePath))); + } + + [Theory] + [InlineData(" ")] + [InlineData("\t")] + public void Validate_ReturnsFail_WhenKeyStorePathIsWhitespace(string keyStorePath) + { + // Arrange + var validator = new DataProtectionOptions.Validator(); + var options = ValidFileSystemOptions(); + options.KeyStorePath = keyStorePath; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(DataProtectionOptions.KeyStorePath))); + } + + [Fact] + public void Validate_ReturnsFail_WhenCertificateProviderMissingForCertificateMode() + => AssertCertificateProviderRequired(DataProtectionMode.Certificate); + + [Fact] + public void Validate_ReturnsFail_WhenCertificateProviderMissingForRedisMode() + => AssertCertificateProviderRequired(DataProtectionMode.Redis); + + [Fact] + public void Validate_DoesNotRequireCertificateProvider_ForFileSystemMode() + { + // Arrange + var validator = new DataProtectionOptions.Validator(); + + // Act + var result = validator.Validate(name: null, ValidFileSystemOptions()); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + [Fact] + public void Validate_DoesNotRequireCertificateProvider_ForAzureMode() + { + // Arrange + var validator = new DataProtectionOptions.Validator(); + + // Act + var result = validator.Validate(name: null, ValidAzureOptions()); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + private static void AssertCertificateProviderRequired(DataProtectionMode mode) + { + // Arrange + var validator = new DataProtectionOptions.Validator(); + var options = new DataProtectionOptions + { + Mode = mode, + CertificateProvider = null, + Redis = new RedisOptions { ConnectionString = "localhost:6379" } + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(DataProtectionOptions.CertificateProvider))); + } + + [Fact] + public void Validate_DispatchesToFileSystemCertificateValidator_WhenCertificateProviderIsFileSystem() + { + // Arrange — Mode=Certificate + Provider=FileSystem with empty Path should surface + // the FileSystemCertificateOptions failure prefixed with the property name. + var validator = new DataProtectionOptions.Validator(); + var options = new DataProtectionOptions + { + Mode = DataProtectionMode.Certificate, + CertificateProvider = CertificateProviderMode.FileSystem, + FileSystemCertificate = new FileSystemCertificateOptions { Path = string.Empty } + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(DataProtectionOptions.FileSystemCertificate)) + && failure.Contains(nameof(FileSystemCertificateOptions.Path))); + } + + [Fact] + public void Validate_DispatchesToAzureBlobCertificateValidator_WhenCertificateProviderIsAzureBlob() + { + // Arrange — Provider=AzureBlob without BlobUri should surface a failure pointing at AzureBlobCertificate. + var validator = new DataProtectionOptions.Validator(); + var options = new DataProtectionOptions + { + Mode = DataProtectionMode.Certificate, + CertificateProvider = CertificateProviderMode.AzureBlob, + AzureBlobCertificate = new AzureBlobCertificateOptions { BlobUri = null } + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(DataProtectionOptions.AzureBlobCertificate)) + && failure.Contains(nameof(AzureBlobCertificateOptions.BlobUri))); + } + + [Fact] + public void Validate_DispatchesToAzureCredentialValidator_WhenModeIsAzure() + { + // Arrange — Mode=Azure + ClientSecret credential mode missing required fields should surface failures. + var validator = new DataProtectionOptions.Validator(); + var options = ValidAzureOptions(); + options.AzureCredential = new AzureCredentialOptions { Mode = AzureCredentialType.ClientSecret }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(DataProtectionOptions.AzureCredential)) + && failure.Contains(nameof(AzureCredentialOptions.TenantId))); + } + + [Fact] + public void Validate_DispatchesToAzureCredentialValidator_WhenCertificateProviderIsAzureBlob() + { + // Arrange — Even with Mode=Certificate, AzureBlob provider triggers credential validation. + var validator = new DataProtectionOptions.Validator(); + var options = new DataProtectionOptions + { + Mode = DataProtectionMode.Certificate, + CertificateProvider = CertificateProviderMode.AzureBlob, + AzureBlobCertificate = new AzureBlobCertificateOptions { BlobUri = new Uri("https://account.blob.core.windows.net/certs/dp.pfx") }, + AzureCredential = new AzureCredentialOptions { Mode = AzureCredentialType.ClientSecret } + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(DataProtectionOptions.AzureCredential))); + } + + [Fact] + public void Validate_DispatchesToRedisValidator_WhenModeIsRedis() + { + // Arrange — Mode=Redis + empty ConnectionString surfaces a failure pointing at Redis. + var validator = new DataProtectionOptions.Validator(); + var options = new DataProtectionOptions + { + Mode = DataProtectionMode.Redis, + CertificateProvider = CertificateProviderMode.FileSystem, + FileSystemCertificate = new FileSystemCertificateOptions { Path = "/certs/dp.pfx" }, + Redis = new RedisOptions { ConnectionString = string.Empty } + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(DataProtectionOptions.Redis)) + && failure.Contains(nameof(RedisOptions.ConnectionString))); + } + + [Fact] + public void Validate_DispatchesToAzureValidator_WhenModeIsAzure() + { + // Arrange — Mode=Azure with missing Azure.BlobUri should surface a failure prefixed with Azure. + var validator = new DataProtectionOptions.Validator(); + var options = new DataProtectionOptions + { + Mode = DataProtectionMode.Azure, + Azure = new AzureOptions { BlobUri = null, KeyVaultKeyId = new Uri("https://kv.vault.azure.net/keys/k/v") } + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(failure => failure.Contains(nameof(DataProtectionOptions.Azure)) + && failure.Contains(nameof(AzureOptions.BlobUri))); + } + + [Fact] + public void Validate_AggregatesMultipleFailures() + { + // Arrange — Empty KeyStorePath AND missing CertificateProvider AND empty Redis connection string. + var validator = new DataProtectionOptions.Validator(); + var options = new DataProtectionOptions + { + Mode = DataProtectionMode.Redis, + CertificateProvider = null, + KeyStorePath = string.Empty, + Redis = new RedisOptions { ConnectionString = string.Empty } + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldNotBeNull(); + result.Failures.Count().ShouldBeGreaterThanOrEqualTo(2); + result.Failures.ShouldContain(f => f.Contains(nameof(DataProtectionOptions.KeyStorePath))); + result.Failures.ShouldContain(f => f.Contains(nameof(DataProtectionOptions.CertificateProvider))); + } + + [Fact] + public void Validate_ReturnsSuccess_ForValidFileSystemConfiguration() + { + // Arrange + var validator = new DataProtectionOptions.Validator(); + var options = ValidFileSystemOptions(); + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + [Fact] + public void Validate_ReturnsSuccess_ForValidCertificateConfiguration() + { + // Arrange + var validator = new DataProtectionOptions.Validator(); + var options = new DataProtectionOptions + { + Mode = DataProtectionMode.Certificate, + CertificateProvider = CertificateProviderMode.FileSystem, + FileSystemCertificate = new FileSystemCertificateOptions { Path = "/certs/dp.pfx" } + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + [Fact] + public void Validate_ReturnsSuccess_ForValidRedisConfiguration() + { + // Arrange + var validator = new DataProtectionOptions.Validator(); + var options = new DataProtectionOptions + { + Mode = DataProtectionMode.Redis, + CertificateProvider = CertificateProviderMode.FileSystem, + FileSystemCertificate = new FileSystemCertificateOptions { Path = "/certs/dp.pfx" }, + Redis = new RedisOptions { ConnectionString = "localhost:6379" } + }; + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + [Fact] + public void Validate_ReturnsSuccess_ForValidAzureConfiguration() + { + // Arrange + var validator = new DataProtectionOptions.Validator(); + var options = ValidAzureOptions(); + + // Act + var result = validator.Validate(name: null, options); + + // Assert + result.Succeeded.ShouldBeTrue(result.FailureMessage); + } + + private static DataProtectionOptions ValidFileSystemOptions() => new() + { + Mode = DataProtectionMode.FileSystem, + KeyStorePath = "./keys" + }; + + private static DataProtectionOptions ValidAzureOptions() => new() + { + Mode = DataProtectionMode.Azure, + Azure = new AzureOptions + { + BlobUri = new Uri("https://account.blob.core.windows.net/keys/key.xml"), + KeyVaultKeyId = new Uri("https://kv.vault.azure.net/keys/dp/abc") + }, + AzureCredential = new AzureCredentialOptions { Mode = AzureCredentialType.Default } + }; +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionValueProtectorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionValueProtectorTests.cs index 9f2986b3..bf4c9e16 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionValueProtectorTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/DataProtectionValueProtectorTests.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Security.Cryptography; using GroundControl.Api.Core.DataProtection; using Microsoft.AspNetCore.DataProtection; @@ -19,6 +20,13 @@ private static DataProtectionValueProtector CreateProtector(string? applicationN return new DataProtectionValueProtector(provider.GetRequiredService()); } + private static IDataProtectionProvider CreateSharedProvider(string applicationName) + { + var services = new ServiceCollection(); + services.AddDataProtection().SetApplicationName(applicationName); + return services.BuildServiceProvider().GetRequiredService(); + } + [Fact] public void Protect_ReturnsNonEmptyStringDifferentFromInput() { @@ -87,4 +95,108 @@ public void Unprotect_WithDifferentApplicationKey_ThrowsCryptographicException() // Act & Assert Should.Throw(() => protector2.Unprotect(encrypted)); } + + [Fact] + public void ProtectAndUnprotect_EmptyString_RoundTripsWithoutThrowing() + { + // Arrange + var protector = CreateProtector(); + + // Act + var encrypted = protector.Protect(string.Empty); + var decrypted = protector.Unprotect(encrypted); + + // Assert — DP encrypts the empty string (it has length and an MAC envelope), so the + // ciphertext is non-empty even though the plaintext is. + encrypted.ShouldNotBeNullOrEmpty(); + decrypted.ShouldBe(string.Empty); + } + + [Theory] + [InlineData("héllo wörld")] + [InlineData("日本語のテスト")] + [InlineData("emoji-secret-🔐🚀")] + [InlineData("\0binary-ish")] + public void ProtectAndUnprotect_UnicodeAndControlCharacters_RoundTrip(string plainText) + { + // Arrange + var protector = CreateProtector(); + + // Act + var decrypted = protector.Unprotect(protector.Protect(plainText)); + + // Assert + decrypted.ShouldBe(plainText); + } + + [Fact] + public void ProtectAndUnprotect_LargeString_RoundTrips() + { + // Arrange — 1 MiB of data exceeds typical envelope buffering thresholds. + var protector = CreateProtector(); + var plainText = new string('x', 1024 * 1024); + + // Act + var decrypted = protector.Unprotect(protector.Protect(plainText)); + + // Assert + decrypted.Length.ShouldBe(plainText.Length); + decrypted.ShouldBe(plainText); + } + + [Fact] + public void Unprotect_NonBase64Input_ThrowsKnownException() + { + // Arrange — pin the contract: malformed input surfaces as an exception, not silent + // success or a corrupted plaintext. The framework currently throws either + // FormatException (during base64 decode) or CryptographicException (during MAC check). + var protector = CreateProtector(); + + // Act + var thrown = Record.Exception(() => protector.Unprotect("not-base64-!@#$%")); + + // Assert + thrown.ShouldNotBeNull("malformed input must not be silently accepted"); + thrown.ShouldBeAssignableTo(); + (thrown is FormatException or CryptographicException).ShouldBeTrue( + $"Expected FormatException or CryptographicException, got {thrown.GetType().Name}: {thrown.Message}"); + } + + [Fact] + public void TwoProtectorsBackedBySameProvider_Interoperate() + { + // Arrange — Two DataProtectionValueProtector instances over the same shared + // IDataProtectionProvider must be able to read each other's output. + var sharedProvider = CreateSharedProvider("GroundControl.Tests"); + var protectorA = new DataProtectionValueProtector(sharedProvider); + var protectorB = new DataProtectionValueProtector(sharedProvider); + + // Act + var encrypted = protectorA.Protect("interop-value"); + var decrypted = protectorB.Unprotect(encrypted); + + // Assert + decrypted.ShouldBe("interop-value"); + } + + [Fact] + public async Task Protect_ConcurrentCallsFromManyThreads_AllSucceedAndRoundTrip() + { + // Arrange + var protector = CreateProtector(); + var inputs = Enumerable.Range(0, 64).Select(i => $"value-{i}").ToArray(); + var roundTripped = new ConcurrentBag(); + + // Act — race many threads against the same protector instance. + await Parallel.ForEachAsync(inputs, TestContext.Current.CancellationToken, (value, _) => + { + var ciphertext = protector.Protect(value); + roundTripped.Add(protector.Unprotect(ciphertext)); + return ValueTask.CompletedTask; + }); + + // Assert + roundTripped.OrderBy(v => v, StringComparer.Ordinal) + .ShouldBe(inputs.OrderBy(v => v, StringComparer.Ordinal)); + } } \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemKeyRingConfiguratorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemKeyRingConfiguratorTests.cs index dd413ca8..74d34d46 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemKeyRingConfiguratorTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/FileSystemKeyRingConfiguratorTests.cs @@ -2,7 +2,9 @@ using GroundControl.Api.Core.DataProtection.KeyRing; using GroundControl.Api.Shared.Security.Protection; using Microsoft.AspNetCore.DataProtection; +using Microsoft.AspNetCore.DataProtection.KeyManagement; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Shouldly; using Xunit; using DataProtectionOptions = GroundControl.Api.Core.DataProtection.DataProtectionOptions; @@ -58,6 +60,87 @@ public void Configure_UsesDefaultPath_WhenNotConfigured() Should.NotThrow(() => configurator.Configure(dpBuilder, options)); } + [Fact] + public void Configure_UseDpapiTrue_OnWindows_RegistersDpapiXmlEncryptor() + { + Assert.SkipUnless(OperatingSystem.IsWindows(), "DPAPI is Windows-only."); + + // Arrange + var options = new DataProtectionOptions + { + KeyStorePath = _tempDir, + UseDpapi = true + }; + + var services = new ServiceCollection(); + var dpBuilder = services.AddDataProtection() + .SetApplicationName("GroundControl.Tests"); + + var configurator = new FileSystemKeyRingConfigurator(); + + // Act + configurator.Configure(dpBuilder, options); + var keyManagementOptions = services.BuildServiceProvider().GetRequiredService>().Value; + + // Assert — DPAPI wires up an XmlEncryptor (the framework's DpapiXmlEncryptor type is + // internal, so we assert by name to avoid an internal type reference). + keyManagementOptions.XmlEncryptor.ShouldNotBeNull("ProtectKeysWithDpapi should configure an XmlEncryptor"); + keyManagementOptions.XmlEncryptor.GetType().Name.ShouldContain("Dpapi"); + } + + [Fact] + public void Configure_UseDpapiFalse_OnWindows_DoesNotRegisterAnyXmlEncryptor() + { + Assert.SkipUnless(OperatingSystem.IsWindows(), "DPAPI is Windows-only."); + + // Arrange + var options = new DataProtectionOptions + { + KeyStorePath = _tempDir, + UseDpapi = false + }; + + var services = new ServiceCollection(); + var dpBuilder = services.AddDataProtection() + .SetApplicationName("GroundControl.Tests"); + + var configurator = new FileSystemKeyRingConfigurator(); + + // Act + configurator.Configure(dpBuilder, options); + var keyManagementOptions = services.BuildServiceProvider().GetRequiredService>().Value; + + // Assert — Without DPAPI no XmlEncryptor is configured (keys persist as plaintext XML; + // appropriate only for development). + keyManagementOptions.XmlEncryptor.ShouldBeNull(); + } + + [Fact] + public void Configure_UseDpapiTrue_OnNonWindows_IsNoOp() + { + Assert.SkipWhen(OperatingSystem.IsWindows(), "Verifies the non-Windows branch."); + + // Arrange + var options = new DataProtectionOptions + { + KeyStorePath = _tempDir, + UseDpapi = true + }; + + var services = new ServiceCollection(); + var dpBuilder = services.AddDataProtection() + .SetApplicationName("GroundControl.Tests"); + + var configurator = new FileSystemKeyRingConfigurator(); + + // Act + Assert — On Linux/macOS the DPAPI request is silently ignored rather than + // throwing PlatformNotSupportedException. + Should.NotThrow(() => configurator.Configure(dpBuilder, options)); + + var keyManagementOptions = services.BuildServiceProvider().GetRequiredService>().Value; + keyManagementOptions.XmlEncryptor.ShouldBeNull(); + } + public void Dispose() { if (Directory.Exists(_tempDir)) diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlDecryptorActivationTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlDecryptorActivationTests.cs new file mode 100644 index 00000000..978b1327 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlDecryptorActivationTests.cs @@ -0,0 +1,102 @@ +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using System.Xml.Linq; +using GroundControl.Api.Core.DataProtection.Certificate; +using Microsoft.AspNetCore.DataProtection.Internal; +using Microsoft.AspNetCore.DataProtection.XmlEncryption; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection; + +/// +/// Pins the contract that can be constructed +/// through ASP.NET Data Protection's — the path the framework actually +/// takes when it sees the GroundControlCertificateXmlEncryptor's pinned decryptor type +/// in persisted key XML. Every other test bypasses this via the internal test-only constructor; +/// without this test, breaking the public IServiceProvider ctor would only fail in +/// production. +/// +public sealed class GroundControlCertificateXmlDecryptorActivationTests : IDisposable +{ + private readonly List _certificates = []; + + [Fact] + public void Activator_BuildsDecryptor_AndDecryptsAPayloadProducedByTheEncryptor() + { + // Arrange — a real DI container with the same shape DataProtectionModule sets up at + // startup, then the framework's IActivator constructs our decryptor via the public + // IServiceProvider ctor. + var certificate = CreateSelfSignedCertificate(); + var provider = Substitute.For(); + provider.GetCurrentCertificate().Returns(certificate); + provider.GetPreviousCertificates().Returns([]); + + var encryptor = new GroundControlCertificateXmlEncryptor(provider, NullLoggerFactory.Instance); + var encrypted = encryptor.Encrypt(new XElement("payload", "via-activator")); + + var services = new ServiceCollection(); + services.AddDataProtection(); + services.AddSingleton(provider); + services.AddLogging(); + var serviceProvider = services.BuildServiceProvider(); + + var activator = serviceProvider.GetRequiredService(); + + // Act + var decryptor = (IXmlDecryptor)activator.CreateInstance( + typeof(IXmlDecryptor), + typeof(GroundControlCertificateXmlDecryptor).AssemblyQualifiedName!); + var decrypted = decryptor.Decrypt(encrypted.EncryptedElement); + + // Assert + decrypted.Value.ShouldBe("via-activator"); + } + + [Fact] + public void DirectActivator_CreateInstance_WithPublicServiceProviderConstructor() + { + // Arrange — Belt-and-braces: confirm System.Activator can instantiate the decryptor + // through the public single-IServiceProvider ctor. SimpleActivator falls back to this + // when no IActivator-compatible service is registered. + var provider = Substitute.For(); + var serviceProvider = new ServiceCollection() + .AddSingleton(provider) + .AddLogging() + .BuildServiceProvider(); + + // Act + var instance = Activator.CreateInstance(typeof(GroundControlCertificateXmlDecryptor), serviceProvider); + + // Assert + instance.ShouldBeOfType(); + } + + public void Dispose() + { + foreach (var cert in _certificates) + { + cert.Dispose(); + } + } + + private X509Certificate2 CreateSelfSignedCertificate() + { + using var rsa = RSA.Create(2048); + var request = new CertificateRequest( + "CN=GroundControl Test Certificate", + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + var certificate = request.CreateSelfSigned( + DateTimeOffset.UtcNow, + DateTimeOffset.UtcNow.AddYears(1)); + + _certificates.Add(certificate); + return certificate; + } +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlDecryptorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlDecryptorTests.cs new file mode 100644 index 00000000..522d668f --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlDecryptorTests.cs @@ -0,0 +1,109 @@ +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using System.Xml.Linq; +using GroundControl.Api.Core.DataProtection.Certificate; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection; + +/// +/// Edge-case coverage for beyond the happy +/// paths covered by . +/// +public sealed class GroundControlCertificateXmlDecryptorTests : IDisposable +{ + private readonly List _certificates = []; + + [Fact] + public void ServiceProviderConstructor_NullServices_Throws() + { + // Arrange + Act + Assert + Should.Throw(() => new GroundControlCertificateXmlDecryptor(services: null!)); + } + + [Fact] + public void ServiceProviderConstructor_ResolvesProviderAndLoggerFromContainer() + { + // Arrange — A real DI container with the two dependencies registered. This mirrors the + // path ASP.NET Data Protection's SimpleActivator takes when it constructs the decryptor. + var provider = Substitute.For(); + var services = new ServiceCollection() + .AddSingleton(provider) + .AddLogging() + .BuildServiceProvider(); + + // Act + var decryptor = new GroundControlCertificateXmlDecryptor(services); + + // Assert — Construction itself proves both dependencies were resolved. + decryptor.ShouldNotBeNull(); + } + + [Fact] + public void Decrypt_NullEncryptedElement_Throws() + { + // Arrange + var provider = Substitute.For(); + var decryptor = new GroundControlCertificateXmlDecryptor(provider, NullLogger.Instance); + + // Act + Assert + Should.Throw(() => decryptor.Decrypt(encryptedElement: null!)); + } + + [Fact] + public void Decrypt_WhenMatchingCertificateLacksPrivateKey_FailsWithCryptographicException() + { + // Arrange — Encrypt under a full (private+public) cert. Then build a provider that + // returns the same cert but with the private key stripped. FindCertificateWithPrivateKey + // returns null in that case, so decryption falls through to the base EncryptedXml which + // can't find the cert in the OS store either. + var fullCertificate = CreateSelfSignedCertificate(); + var encryptionProvider = Substitute.For(); + encryptionProvider.GetCurrentCertificate().Returns(fullCertificate); + + var encryptor = new GroundControlCertificateXmlEncryptor(encryptionProvider, NullLoggerFactory.Instance); + var encrypted = encryptor.Encrypt(new XElement("payload", "no-private-key-on-decrypt")); + + var publicOnly = X509CertificateLoader.LoadCertificate(fullCertificate.Export(X509ContentType.Cert)); + _certificates.Add(publicOnly); + var publicOnlyProvider = Substitute.For(); + publicOnlyProvider.GetCurrentCertificate().Returns(publicOnly); + publicOnlyProvider.GetPreviousCertificates().Returns([]); + + var decryptor = new GroundControlCertificateXmlDecryptor( + publicOnlyProvider, + NullLogger.Instance); + + // Act + Assert + Should.Throw(() => decryptor.Decrypt(encrypted.EncryptedElement)); + } + + public void Dispose() + { + foreach (var cert in _certificates) + { + cert.Dispose(); + } + } + + private X509Certificate2 CreateSelfSignedCertificate() + { + using var rsa = RSA.Create(2048); + var request = new CertificateRequest( + "CN=GroundControl Test Certificate", + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + var certificate = request.CreateSelfSigned( + DateTimeOffset.UtcNow, + DateTimeOffset.UtcNow.AddYears(1)); + + _certificates.Add(certificate); + return certificate; + } +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlEncryptorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlEncryptorTests.cs new file mode 100644 index 00000000..209b5742 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/GroundControlCertificateXmlEncryptorTests.cs @@ -0,0 +1,144 @@ +using System.Collections.Concurrent; +using System.Reflection; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using System.Xml.Linq; +using GroundControl.Api.Core.DataProtection.Certificate; +using Microsoft.AspNetCore.DataProtection.XmlEncryption; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection; + +/// +/// Verifies that caches the inner +/// CertificateXmlEncryptor per certificate thumbprint, rebuilds it after rotation, +/// and tolerates concurrent calls. +/// +public sealed class GroundControlCertificateXmlEncryptorTests : IDisposable +{ + private readonly List _certificates = []; + + [Fact] + public void Encrypt_TwiceWithSameCertificate_ReusesInnerEncryptor() + { + // Arrange + var certificate = CreateSelfSignedCertificate(); + var provider = Substitute.For(); + provider.GetCurrentCertificate().Returns(certificate); + var encryptor = new GroundControlCertificateXmlEncryptor(provider, NullLoggerFactory.Instance); + + // Act + encryptor.Encrypt(new XElement("first")); + var innerAfterFirst = ReadInnerEncryptor(encryptor); + encryptor.Encrypt(new XElement("second")); + var innerAfterSecond = ReadInnerEncryptor(encryptor); + + // Assert — same certificate => exact same inner instance reused, no re-allocation. + innerAfterFirst.ShouldNotBeNull(); + innerAfterSecond.ShouldBeSameAs(innerAfterFirst); + } + + [Fact] + public void Encrypt_AfterCertificateRotation_RebuildsInnerEncryptor() + { + // Arrange — Provider returns cert A first, then cert B (simulating a rotation). + var certA = CreateSelfSignedCertificate(); + var certB = CreateSelfSignedCertificate(); + var provider = Substitute.For(); + provider.GetCurrentCertificate().Returns(certA, certB); + + var encryptor = new GroundControlCertificateXmlEncryptor(provider, NullLoggerFactory.Instance); + + // Act + encryptor.Encrypt(new XElement("under-a")); + var innerForA = ReadInnerEncryptor(encryptor); + encryptor.Encrypt(new XElement("under-b")); + var innerForB = ReadInnerEncryptor(encryptor); + + // Assert — different thumbprint => a fresh inner encryptor. + innerForA.ShouldNotBeNull(); + innerForB.ShouldNotBeNull(); + innerForB.ShouldNotBeSameAs(innerForA); + } + + [Fact] + public async Task Encrypt_UnderConcurrency_AllCallsProduceDecryptableOutput() + { + // Arrange + var certificate = CreateSelfSignedCertificate(); + var provider = Substitute.For(); + provider.GetCurrentCertificate().Returns(certificate); + provider.GetPreviousCertificates().Returns([]); + + var encryptor = new GroundControlCertificateXmlEncryptor(provider, NullLoggerFactory.Instance); + var decryptor = new GroundControlCertificateXmlDecryptor(provider, NullLogger.Instance); + + var results = new ConcurrentBag(); + + // Act — 32 concurrent encrypts; the cache field is unsynchronised, so this pins the + // current "benign race" behaviour: parallel callers may briefly construct extra inner + // encryptors but every result is still valid and decryptable. + await Parallel.ForAsync(0, 32, TestContext.Current.CancellationToken, (i, _) => + { + results.Add(encryptor.Encrypt(new XElement("payload", new XAttribute("i", i)))); + return ValueTask.CompletedTask; + }); + + // Assert + results.Count.ShouldBe(32); + foreach (var encrypted in results) + { + var decrypted = decryptor.Decrypt(encrypted.EncryptedElement); + decrypted.Name.LocalName.ShouldBe("payload"); + } + } + + public void Dispose() + { + foreach (var cert in _certificates) + { + cert.Dispose(); + } + } + + /// + /// Reads the inner CertificateXmlEncryptor reference from the encryptor's private + /// cache field via reflection so the tests can verify caching identity without changing the + /// production surface. + /// + private static object? ReadInnerEncryptor(GroundControlCertificateXmlEncryptor encryptor) + { + const BindingFlags Flags = BindingFlags.Instance | BindingFlags.NonPublic; + var cacheField = typeof(GroundControlCertificateXmlEncryptor).GetField("_cache", Flags); + cacheField.ShouldNotBeNull("the encryptor must have an inner cache field for these tests to be meaningful"); + + var cache = cacheField.GetValue(encryptor); + if (cache is null) + { + return null; + } + + var encryptorProperty = cache.GetType().GetProperty("Encryptor", Flags | BindingFlags.Public); + return encryptorProperty?.GetValue(cache); + } + + private X509Certificate2 CreateSelfSignedCertificate() + { + using var rsa = RSA.Create(2048); + var request = new CertificateRequest( + "CN=GroundControl Test Certificate", + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + var certificate = request.CreateSelfSigned( + DateTimeOffset.UtcNow, + DateTimeOffset.UtcNow.AddYears(1)); + + _certificates.Add(certificate); + return certificate; + } +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs index 30833d54..ee163d35 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/CertificateRotationTests.cs @@ -73,6 +73,37 @@ public async Task PostRotationEntry_RoundTrips_UnderNewCertificateOnly() body.Values.ShouldHaveSingleItem().Value.ShouldBe("after-cert-rotation"); } + [Fact] + public async Task PreRotationEntry_FailsToDecrypt_WhenOldCertificateNotInPreviousPaths() + { + // Arrange — C1 protects the key ring while Factory A writes a sensitive entry. Then + // Factory B comes up with C2 ONLY (no previous list). Without the original certificate, + // the key XML written under C1 cannot be unwrapped, and the protector cannot reconstruct + // the AES key needed to decrypt the stored ciphertext. This guards against the "premature + // old cert removal" risk listed in Security-Model.md. + var c1Path = SelfSignedCertificate.CreatePfxFile(_certificateDir, "c1.pfx"); + Guid preRotationId; + + await using (var factoryA = CreateLifecycleFactory(currentCertificatePath: c1Path, previousCertificatePaths: [])) + using (var clientA = factoryA.CreateClient()) + { + var created = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "stranded-by-rotation"); + preRotationId = created.Id; + } + + var c2Path = SelfSignedCertificate.CreatePfxFile(_certificateDir, "c2.pfx"); + + // Act — Factory B drops C1 entirely; the pre-rotation entry should now be undecryptable. + await using var factoryB = CreateLifecycleFactory(currentCertificatePath: c2Path, previousCertificatePaths: []); + using var clientB = factoryB.CreateClient(); + + var response = await clientB.GetAsync($"/api/config-entries/{preRotationId}?decrypt=true", TestCancellationToken); + + // Assert — surface as a server error rather than silent corruption / an empty response. + response.IsSuccessStatusCode.ShouldBeFalse( + $"Expected decrypt to fail without the original certificate, but server returned {response.StatusCode}."); + } + private GroundControlApiFactory CreateLifecycleFactory(string currentCertificatePath, IReadOnlyList previousCertificatePaths) { var config = new Dictionary diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemScaledOutTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemScaledOutTests.cs new file mode 100644 index 00000000..887a179c --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/FileSystemScaledOutTests.cs @@ -0,0 +1,77 @@ +using GroundControl.Api.Features.ConfigEntries.Contracts; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle; + +/// +/// Verifies that two API hosts running concurrently against the same FileSystem-backed key +/// store can decrypt each other's sensitive values without restart. This is the multi-instance +/// deployment scenario (e.g. a horizontally-scaled service): the per-restart tests prove a host +/// can read state written by its predecessor; this test proves a peer can read it live. +/// +public sealed class FileSystemScaledOutTests : DataProtectionLifecycleTestBase +{ + private readonly string _keyStorePath; + + public FileSystemScaledOutTests(MongoFixture mongoFixture) + : base(mongoFixture) + { + _keyStorePath = AllocateTempDirectory("gc-keys"); + } + + [Fact] + public async Task SensitiveValue_DecryptableByLivePeer_SharingKeyStoreAndDatabase() + { + // Arrange — two factories alive at the same time, sharing the same key store + DB. + await using var factoryA = CreateLifecycleFactory(); + await using var factoryB = CreateLifecycleFactory(); + + using var clientA = factoryA.CreateClient(); + using var clientB = factoryB.CreateClient(); + + // Act — A protects, then B reads back without anyone restarting. + var created = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "live-peer-secret"); + var response = await clientB.GetAsync($"/api/config-entries/{created.Id}?decrypt=true", TestCancellationToken); + + // Assert + response.IsSuccessStatusCode.ShouldBeTrue(); + var body = await ReadRequiredJsonAsync(response, TestCancellationToken); + body.IsSensitive.ShouldBeTrue(); + body.Values.ShouldHaveSingleItem().Value.ShouldBe("live-peer-secret"); + } + + [Fact] + public async Task SensitiveValue_DecryptableInBothDirections_AcrossLiveHosts() + { + // Arrange + await using var factoryA = CreateLifecycleFactory(); + await using var factoryB = CreateLifecycleFactory(); + + using var clientA = factoryA.CreateClient(); + using var clientB = factoryB.CreateClient(); + + // Act — Each host writes a value; each host then reads back the other's. + var fromA = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "secret-from-a"); + var fromB = await CreateSensitiveConfigEntryAsync(clientB, "api.token", "secret-from-b"); + + var bReadsA = await clientB.GetAsync($"/api/config-entries/{fromA.Id}?decrypt=true", TestCancellationToken); + var aReadsB = await clientA.GetAsync($"/api/config-entries/{fromB.Id}?decrypt=true", TestCancellationToken); + + // Assert + bReadsA.IsSuccessStatusCode.ShouldBeTrue(); + var bBody = await ReadRequiredJsonAsync(bReadsA, TestCancellationToken); + bBody.Values.ShouldHaveSingleItem().Value.ShouldBe("secret-from-a"); + + aReadsB.IsSuccessStatusCode.ShouldBeTrue(); + var aBody = await ReadRequiredJsonAsync(aReadsB, TestCancellationToken); + aBody.Values.ShouldHaveSingleItem().Value.ShouldBe("secret-from-b"); + } + + private GroundControlApiFactory CreateLifecycleFactory() => CreateFactory(new Dictionary + { + ["Persistence:MongoDb:DatabaseName"] = DatabaseName, + ["DataProtection:Mode"] = "FileSystem", + ["DataProtection:KeyStorePath"] = _keyStorePath + }); +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs index 570857c9..fd78be85 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisCertificateRotationTests.cs @@ -23,6 +23,56 @@ public RedisCertificateRotationTests(MongoFixture mongoFixture, RedisFixture red _certificateDir = AllocateTempDirectory("gc-certs"); } + [Fact] + public async Task PostRotationEntry_RoundTrips_UnderNewCertificateOnly() + { + // Arrange — Boot Redis-backed host directly with C2 only; nothing was ever encrypted + // under C1. Round-trips a freshly written value to prove Mode=Redis works in steady state + // when only the current cert is configured. + var c2Path = SelfSignedCertificate.CreatePfxFile(_certificateDir, "c2-only.pfx"); + + // Act + await using var factory = CreateLifecycleFactory(currentCertificatePath: c2Path, previousCertificatePaths: []); + using var client = factory.CreateClient(); + + var created = await CreateSensitiveConfigEntryAsync(client, "api.token", "redis-after-rotation"); + var response = await client.GetAsync($"/api/config-entries/{created.Id}?decrypt=true", TestCancellationToken); + + // Assert + response.IsSuccessStatusCode.ShouldBeTrue(); + var body = await ReadRequiredJsonAsync(response, TestCancellationToken); + body.Values.ShouldHaveSingleItem().Value.ShouldBe("redis-after-rotation"); + } + + [Fact] + public async Task PreRotationEntry_FailsToDecrypt_WhenOldCertificateNotInPreviousList() + { + // Arrange — C1 protects the Redis key ring while Factory A writes a value. Factory B + // boots with C2 ONLY; the key XML in Redis is encrypted under C1 and unrecoverable. + var c1Path = SelfSignedCertificate.CreatePfxFile(_certificateDir, "c1.pfx"); + Guid preRotationId; + + await using (var factoryA = CreateLifecycleFactory(currentCertificatePath: c1Path, previousCertificatePaths: [])) + using (var clientA = factoryA.CreateClient()) + { + var created = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "redis-stranded"); + preRotationId = created.Id; + } + + var c2Path = SelfSignedCertificate.CreatePfxFile(_certificateDir, "c2.pfx"); + + // Act + await using var factoryB = CreateLifecycleFactory(currentCertificatePath: c2Path, previousCertificatePaths: []); + using var clientB = factoryB.CreateClient(); + + var response = await clientB.GetAsync($"/api/config-entries/{preRotationId}?decrypt=true", TestCancellationToken); + + // Assert — must surface a server-side failure rather than silently masking or returning + // empty plaintext. + response.IsSuccessStatusCode.ShouldBeFalse( + $"Expected decrypt to fail without the original certificate, but server returned {response.StatusCode}."); + } + [Fact] public async Task PreRotationEntry_RemainsDecryptable_AfterCertificateSwap() { diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisScaledOutTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisScaledOutTests.cs new file mode 100644 index 00000000..1a87afb5 --- /dev/null +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/Lifecycle/Redis/RedisScaledOutTests.cs @@ -0,0 +1,55 @@ +using GroundControl.Api.Features.ConfigEntries.Contracts; +using Shouldly; +using Xunit; + +namespace GroundControl.Api.Tests.Core.DataProtection.Lifecycle.Redis; + +/// +/// Redis equivalent of : two API hosts running concurrently +/// against the same Redis-backed Data Protection key ring must be able to decrypt each other's +/// sensitive values without restart. This is the canonical multi-instance scenario for Redis mode. +/// +public sealed class RedisScaledOutTests : DataProtectionLifecycleTestBase +{ + private readonly RedisFixture _redisFixture; + private readonly string _redisKeyName = $"groundcontrol-test-keys-{Guid.NewGuid():N}"; + private readonly string _certificatePath; + + public RedisScaledOutTests(MongoFixture mongoFixture, RedisFixture redisFixture) + : base(mongoFixture) + { + _redisFixture = redisFixture; + _certificatePath = SelfSignedCertificate.CreatePfxFile(AllocateTempDirectory("gc-certs"), "dp.pfx", password: null); + } + + [Fact] + public async Task SensitiveValue_DecryptableByLivePeer_SharingRedisKeyRing() + { + // Arrange + await using var factoryA = CreateLifecycleFactory(); + await using var factoryB = CreateLifecycleFactory(); + + using var clientA = factoryA.CreateClient(); + using var clientB = factoryB.CreateClient(); + + // Act — A protects, B reads back without restart. + var created = await CreateSensitiveConfigEntryAsync(clientA, "db.password", "redis-live-peer"); + var response = await clientB.GetAsync($"/api/config-entries/{created.Id}?decrypt=true", TestCancellationToken); + + // Assert + response.IsSuccessStatusCode.ShouldBeTrue(); + var body = await ReadRequiredJsonAsync(response, TestCancellationToken); + body.IsSensitive.ShouldBeTrue(); + body.Values.ShouldHaveSingleItem().Value.ShouldBe("redis-live-peer"); + } + + private GroundControlApiFactory CreateLifecycleFactory() => CreateFactory(new Dictionary + { + ["Persistence:MongoDb:DatabaseName"] = DatabaseName, + ["DataProtection:Mode"] = "Redis", + ["DataProtection:CertificateProvider"] = "FileSystem", + ["DataProtection:FileSystemCertificate:Path"] = _certificatePath, + ["DataProtection:Redis:ConnectionString"] = _redisFixture.ConnectionString, + ["DataProtection:Redis:KeyName"] = _redisKeyName + }); +} diff --git a/tests/GroundControl.Api.Tests/Core/DataProtection/RedisKeyRingConfiguratorTests.cs b/tests/GroundControl.Api.Tests/Core/DataProtection/RedisKeyRingConfiguratorTests.cs index a8c367ca..49026cb5 100644 --- a/tests/GroundControl.Api.Tests/Core/DataProtection/RedisKeyRingConfiguratorTests.cs +++ b/tests/GroundControl.Api.Tests/Core/DataProtection/RedisKeyRingConfiguratorTests.cs @@ -32,4 +32,35 @@ public void Configure_OptionsValidationException_WhenConnectionStringNotConfigur exception.Message.ShouldContain("ConnectionString: The RedisOptions.ConnectionString field is required."); } + + [Fact] + public void Configure_WrapsRedisConnectionFailure_InInvalidOperationException() + { + // Arrange — Point at a TCP port that nothing should be listening on, with a tiny timeout + // so the test fails fast and surfaces the wrapping exception path rather than hanging. + const string UnreachableEndpoint = "127.0.0.1:1"; + var options = new DataProtectionOptions + { + Mode = DataProtectionMode.Redis, + Redis = new RedisOptions + { + ConnectionString = UnreachableEndpoint, + ConnectTimeoutMs = 200 + } + }; + + var services = new ServiceCollection(); + var builder = services.AddDataProtection() + .SetApplicationName("GroundControl.Tests"); + + var configurator = new RedisKeyRingConfigurator(); + + // Act + var exception = Should.Throw(() => configurator.Configure(builder, options)); + + // Assert — The wrapper preserves the inner exception and surfaces the connection string + // so operators can spot misconfiguration in the logs. + exception.Message.ShouldContain(UnreachableEndpoint); + exception.InnerException.ShouldNotBeNull(); + } } \ No newline at end of file diff --git a/tests/GroundControl.Api.Tests/GroundControl.Api.Tests.csproj b/tests/GroundControl.Api.Tests/GroundControl.Api.Tests.csproj index 2f7bb4d3..44c6ecf0 100644 --- a/tests/GroundControl.Api.Tests/GroundControl.Api.Tests.csproj +++ b/tests/GroundControl.Api.Tests/GroundControl.Api.Tests.csproj @@ -5,6 +5,7 @@ +