From 7cdbbb2fa7c2fa11aad8be9c887c095e4a7fcda7 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 22 Jul 2026 15:25:21 +0200 Subject: [PATCH 1/2] feat(openiddict): start on ephemeral keys with a warning when certs are missing A real deployment (a plain `docker run`, or a PaaS like Dokploy where the image inherits Production) previously refused to start until OpenIddict signing/encryption certificates were configured. The app now starts on ephemeral keys and logs a prominent warning instead: those keys regenerate on every restart, invalidating all issued tokens and signing everyone out on each redeploy, so certificates remain the documented requirement for anything beyond a throwaway deployment. The ROPC password-grant guard still fails startup. Also fixes docs/site/guide/identity.md, which documented wrong config key names (SigningCertPath/EncryptionCertPath/CertPassword instead of SigningCertificatePath/EncryptionCertificatePath/CertificatePassword), and adds openssl cert-generation instructions to the README. --- README.md | 20 +++++- docker-compose.yml | 4 +- docs/site/guide/identity.md | 13 ++-- .../OpenIddictModule.cs | 8 ++- .../Services/OpenIddictProductionGuard.cs | 49 +++++++++----- .../Unit/OpenIddictProductionGuardTests.cs | 65 ++++++++++++++----- 6 files changed, 115 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 70af1485..4e725d97 100644 --- a/README.md +++ b/README.md @@ -73,9 +73,23 @@ account page instead. > want it. The admin account is unaffected — it already exists, and existing accounts are never > re-seeded or rotated. -Note that `Production` additionally requires OpenIddict signing and encryption certificates -(`OpenIddict__SigningCertificatePath` / `OpenIddict__EncryptionCertificatePath`, PKCS#12), and -refuses to start without them. +Note that `Production` expects OpenIddict signing and encryption certificates +(`OpenIddict__SigningCertificatePath` / `OpenIddict__EncryptionCertificatePath`, PKCS#12, with an +optional shared `OpenIddict__CertificatePassword`). Without them the app still starts — so a plain +`docker run` or a PaaS deploy (Dokploy, Coolify, …) works out of the box — but it falls back to +ephemeral token keys and logs a warning: every restart or redeploy then regenerates the keys, +invalidating all issued tokens and signing everyone out. Configure real certificates for anything +beyond a throwaway deployment: + +```bash +openssl req -x509 -newkey rsa:2048 -nodes -days 3650 \ + -subj "/CN=simplemodule-oidc-signing" -keyout signing.key -out signing.crt +openssl pkcs12 -export -inkey signing.key -in signing.crt \ + -out signing.pfx -passout pass:YOUR_CERT_PASSWORD +# repeat with -subj "/CN=simplemodule-oidc-encryption" for encryption.pfx +``` + +Mount the two `.pfx` files into the container and point the environment variables at them. ### Development diff --git a/docker-compose.yml b/docker-compose.yml index e4906fc8..9e97db31 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -6,8 +6,8 @@ services: environment: # Development enables auto-migration and ephemeral signing keys for a # local quickstart. Do NOT expose this stack to the internet as-is: set - # ASPNETCORE_ENVIRONMENT=Production (which additionally enforces signing - # certificates and refuses the password grant), apply migrations + # ASPNETCORE_ENVIRONMENT=Production (which refuses the password grant and + # warns when signing certificates are missing), apply migrations # externally, and trust your reverse proxy via a comma-separated # ForwardedHeaders__KnownProxies=10.0.0.5,10.0.0.6 (or # ForwardedHeaders__KnownNetworks=10.0.0.0/8). Without it X-Forwarded-For diff --git a/docs/site/guide/identity.md b/docs/site/guide/identity.md index 6b97c47f..cf4b99e0 100644 --- a/docs/site/guide/identity.md +++ b/docs/site/guide/identity.md @@ -97,19 +97,22 @@ Self-hosted OAuth2/OIDC server. No external dependencies. ### Certificate Management -Production requires signing and encryption certificates: +Production expects signing and encryption certificates (PKCS#12, one shared password): ```json { "OpenIddict": { - "SigningCertPath": "/certs/signing.pfx", - "EncryptionCertPath": "/certs/encryption.pfx", - "CertPassword": "your-cert-password" + "SigningCertificatePath": "/certs/signing.pfx", + "EncryptionCertificatePath": "/certs/encryption.pfx", + "CertificatePassword": "your-cert-password" } } ``` -Development uses ephemeral keys automatically. +Development uses ephemeral keys automatically. A real deployment without certificates still +starts, but falls back to ephemeral keys and logs a warning — every restart then regenerates +the keys, invalidating all issued tokens and signing everyone out. Configure certificates for +anything beyond a throwaway deployment. ### OpenIddict Session Management diff --git a/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs b/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs index 776d4e9f..325fe71b 100644 --- a/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs +++ b/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs @@ -112,9 +112,11 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config } else { - // Development/Testing: use ephemeral keys (avoids macOS keychain - // issues). OpenIddictProductionGuard fails host startup if - // Production runs without certificates. + // No certificates configured: use ephemeral keys (avoids macOS + // keychain issues locally). OpenIddictProductionGuard logs a + // prominent warning when a real deployment runs this way — + // ephemeral keys regenerate on every restart, invalidating all + // issued tokens. options.AddEphemeralEncryptionKey().AddEphemeralSigningKey(); } diff --git a/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs b/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs index d84c7461..0ebd94a0 100644 --- a/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs +++ b/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs @@ -1,24 +1,29 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; using SimpleModule.Core.Hosting; using SimpleModule.OpenIddict.Contracts; namespace SimpleModule.OpenIddict.Services; /// -/// Fails host startup when the OpenIddict configuration is unsafe for a real -/// deployment (anything but Development/Testing): the ROPC password grant must -/// stay off (it lets anyone exchange leaked or default credentials for a -/// fully-privileged token in a single request), and token signing/encryption -/// must use real certificates — ephemeral keys are regenerated on every restart, -/// invalidating all issued tokens, and signal a copy-pasted Development config. -/// Shares with +/// Guards the OpenIddict configuration in real deployments (anything but +/// Development/Testing). The ROPC password grant fails host startup — it lets +/// anyone exchange leaked or default credentials for a fully-privileged token +/// in a single request. Missing token signing/encryption certificates only log +/// a prominent warning: the app starts on ephemeral keys so a plain +/// docker run of the image works out of the box, but those keys are +/// regenerated on every restart, invalidating all issued tokens and signing +/// everyone out on each redeploy. Configure real certificates for anything +/// beyond a throwaway deployment. Shares +/// with /// UserSeedService so the two guards never disagree about whether an /// environment is a real deployment. /// -public sealed class OpenIddictProductionGuard( +public sealed partial class OpenIddictProductionGuard( IConfiguration configuration, - IHostEnvironment environment + IHostEnvironment environment, + ILogger logger ) : IHostedService { public Task StartAsync(CancellationToken cancellationToken) @@ -42,12 +47,11 @@ public Task StartAsync(CancellationToken cancellationToken) if (string.IsNullOrEmpty(encryptionCertPath) || string.IsNullOrEmpty(signingCertPath)) { - throw new InvalidOperationException( - $"'{ConfigKeys.OpenIddictSigningCertPath}' and " - + $"'{ConfigKeys.OpenIddictEncryptionCertPath}' must be configured in " - + "Production. Without certificates OpenIddict falls back to ephemeral " - + "keys that are regenerated on every restart, invalidating all issued " - + "tokens." + LogEphemeralKeys( + logger, + environment.EnvironmentName, + ConfigKeys.OpenIddictSigningCertPath, + ConfigKeys.OpenIddictEncryptionCertPath ); } @@ -55,4 +59,19 @@ public Task StartAsync(CancellationToken cancellationToken) } public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + [LoggerMessage( + Level = LogLevel.Warning, + Message = "No OpenIddict certificates configured in the '{EnvironmentName}' " + + "environment — falling back to ephemeral signing/encryption keys. These keys " + + "are regenerated on every restart, which invalidates all issued tokens and " + + "signs everyone out on each redeploy. Configure '{SigningCertKey}' and " + + "'{EncryptionCertKey}' (PKCS#12) for stable keys." + )] + private static partial void LogEphemeralKeys( + ILogger logger, + string environmentName, + string signingCertKey, + string encryptionCertKey + ); } diff --git a/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs b/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs index 359d1a3e..217f012e 100644 --- a/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs +++ b/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs @@ -2,6 +2,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; using SimpleModule.OpenIddict.Contracts; using SimpleModule.OpenIddict.Services; @@ -16,11 +17,15 @@ public class OpenIddictProductionGuardTests [InlineData("Testing")] public async Task StartAsync_LocalOrTest_DoesNotThrow_EvenWithUnsafeConfig(string environment) { - // Password grant on + no certs would be refused in a real deployment, - // but is tolerated locally. - var guard = CreateGuard(environment, (ConfigKeys.OpenIddictAllowPasswordGrant, "true")); + // Password grant on + no certs is tolerated locally, without warnings — + // ephemeral keys are the expected local setup. + var (guard, logger) = CreateGuard( + environment, + (ConfigKeys.OpenIddictAllowPasswordGrant, "true") + ); await guard.Invoking(g => g.StartAsync(default)).Should().NotThrowAsync(); + logger.Entries.Should().BeEmpty(); } [Theory] @@ -29,7 +34,7 @@ public async Task StartAsync_LocalOrTest_DoesNotThrow_EvenWithUnsafeConfig(strin [InlineData("QA")] public async Task StartAsync_RealDeployment_PasswordGrantEnabled_Throws(string environment) { - var guard = CreateGuard( + var (guard, _) = CreateGuard( environment, (ConfigKeys.OpenIddictAllowPasswordGrant, "true"), (ConfigKeys.OpenIddictEncryptionCertPath, CertPath), @@ -47,31 +52,35 @@ await guard [Theory] [InlineData("Production")] [InlineData("Staging")] - public async Task StartAsync_RealDeployment_MissingCertificates_Throws(string environment) + public async Task StartAsync_RealDeployment_MissingCertificates_StartsAndLogsWarning( + string environment + ) { - var guard = CreateGuard(environment); // no cert paths configured + var (guard, logger) = CreateGuard(environment); // no cert paths configured - ( - await guard - .Invoking(g => g.StartAsync(default)) - .Should() - .ThrowAsync() - ).WithMessage("*certificate*"); + await guard.Invoking(g => g.StartAsync(default)).Should().NotThrowAsync(); + + var warning = logger.Entries.Should().ContainSingle().Subject; + warning.Level.Should().Be(LogLevel.Warning); + warning.Message.Should().Contain(ConfigKeys.OpenIddictSigningCertPath); + warning.Message.Should().Contain(ConfigKeys.OpenIddictEncryptionCertPath); + warning.Message.Should().Contain("restart"); } [Fact] - public async Task StartAsync_RealDeployment_FullyConfigured_DoesNotThrow() + public async Task StartAsync_RealDeployment_FullyConfigured_DoesNotThrowOrWarn() { - var guard = CreateGuard( + var (guard, logger) = CreateGuard( "Production", (ConfigKeys.OpenIddictEncryptionCertPath, CertPath), (ConfigKeys.OpenIddictSigningCertPath, CertPath) ); await guard.Invoking(g => g.StartAsync(default)).Should().NotThrowAsync(); + logger.Entries.Should().BeEmpty(); } - private static OpenIddictProductionGuard CreateGuard( + private static (OpenIddictProductionGuard Guard, CollectingLogger Logger) CreateGuard( string environment, params (string Key, string Value)[] settings ) @@ -80,7 +89,13 @@ private static OpenIddictProductionGuard CreateGuard( .AddInMemoryCollection(settings.ToDictionary(s => s.Key, s => (string?)s.Value)) .Build(); - return new OpenIddictProductionGuard(configuration, new FakeHostEnvironment(environment)); + var logger = new CollectingLogger(); + var guard = new OpenIddictProductionGuard( + configuration, + new FakeHostEnvironment(environment), + logger + ); + return (guard, logger); } private sealed class FakeHostEnvironment(string environmentName) : IHostEnvironment @@ -90,4 +105,22 @@ private sealed class FakeHostEnvironment(string environmentName) : IHostEnvironm public string ContentRootPath { get; set; } = AppContext.BaseDirectory; public IFileProvider ContentRootFileProvider { get; set; } = new NullFileProvider(); } + + private sealed class CollectingLogger : ILogger + { + public List<(LogLevel Level, string Message)> Entries { get; } = []; + + public IDisposable? BeginScope(TState state) + where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter + ) => Entries.Add((logLevel, formatter(state, exception))); + } } From 56c18cfd5d11e1811d93b424561208addd7be050 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 22 Jul 2026 16:13:17 +0200 Subject: [PATCH 2/2] fix: address code review findings (round 1, pass 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Half-configured certificate pair (exactly one path set) now fails startup naming the missing key — the module ignores a lone certificate, so partial config is always an operator mistake. Warning-fallback remains for the fully-unconfigured case. Also fixes a stale guard-registration comment. --- README.md | 5 +-- docs/site/guide/identity.md | 3 +- .../OpenIddictModule.cs | 3 +- .../Services/OpenIddictProductionGuard.cs | 35 ++++++++++++++----- .../Unit/OpenIddictProductionGuardTests.cs | 26 ++++++++++++++ 5 files changed, 60 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 4e725d97..1648f734 100644 --- a/README.md +++ b/README.md @@ -78,8 +78,9 @@ Note that `Production` expects OpenIddict signing and encryption certificates optional shared `OpenIddict__CertificatePassword`). Without them the app still starts — so a plain `docker run` or a PaaS deploy (Dokploy, Coolify, …) works out of the box — but it falls back to ephemeral token keys and logs a warning: every restart or redeploy then regenerates the keys, -invalidating all issued tokens and signing everyone out. Configure real certificates for anything -beyond a throwaway deployment: +invalidating all issued tokens and signing everyone out. Setting only one of the two paths fails +startup — the certificates only work as a pair, so configure both or neither. Configure real +certificates for anything beyond a throwaway deployment: ```bash openssl req -x509 -newkey rsa:2048 -nodes -days 3650 \ diff --git a/docs/site/guide/identity.md b/docs/site/guide/identity.md index cf4b99e0..3fe0960f 100644 --- a/docs/site/guide/identity.md +++ b/docs/site/guide/identity.md @@ -111,7 +111,8 @@ Production expects signing and encryption certificates (PKCS#12, one shared pass Development uses ephemeral keys automatically. A real deployment without certificates still starts, but falls back to ephemeral keys and logs a warning — every restart then regenerates -the keys, invalidating all issued tokens and signing everyone out. Configure certificates for +the keys, invalidating all issued tokens and signing everyone out. Configuring only one of the +two certificate paths fails startup (the pair only works together). Configure certificates for anything beyond a throwaway deployment. ### OpenIddict Session Management diff --git a/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs b/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs index 325fe71b..13d879a3 100644 --- a/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs +++ b/modules/OpenIddict/src/SimpleModule.OpenIddict/OpenIddictModule.cs @@ -140,7 +140,8 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config options.UseAspNetCore(); }); - // Refuses unsafe Production configurations (password grant, ephemeral keys) + // Fails host startup on unsafe config (password grant, half-configured + // certificates); missing certificates only log a warning (ephemeral keys) services.AddHostedService(); // Seed service diff --git a/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs b/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs index 0ebd94a0..e33a1a16 100644 --- a/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs +++ b/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs @@ -10,13 +10,16 @@ namespace SimpleModule.OpenIddict.Services; /// Guards the OpenIddict configuration in real deployments (anything but /// Development/Testing). The ROPC password grant fails host startup — it lets /// anyone exchange leaked or default credentials for a fully-privileged token -/// in a single request. Missing token signing/encryption certificates only log -/// a prominent warning: the app starts on ephemeral keys so a plain -/// docker run of the image works out of the box, but those keys are -/// regenerated on every restart, invalidating all issued tokens and signing -/// everyone out on each redeploy. Configure real certificates for anything -/// beyond a throwaway deployment. Shares -/// with +/// in a single request. A half-configured certificate pair (exactly one of the +/// signing/encryption paths set) also fails startup: the module only uses real +/// certificates when both are present, so the configured one would be silently +/// ignored — that is always an operator mistake, not a deliberate choice. +/// Only the fully-unconfigured case logs a prominent warning instead: the app +/// starts on ephemeral keys so a plain docker run of the image works +/// out of the box, but those keys are regenerated on every restart, +/// invalidating all issued tokens and signing everyone out on each redeploy. +/// Configure real certificates for anything beyond a throwaway deployment. +/// Shares with /// UserSeedService so the two guards never disagree about whether an /// environment is a real deployment. /// @@ -44,8 +47,24 @@ public Task StartAsync(CancellationToken cancellationToken) var encryptionCertPath = configuration[ConfigKeys.OpenIddictEncryptionCertPath]; var signingCertPath = configuration[ConfigKeys.OpenIddictSigningCertPath]; + var hasEncryptionCert = !string.IsNullOrEmpty(encryptionCertPath); + var hasSigningCert = !string.IsNullOrEmpty(signingCertPath); - if (string.IsNullOrEmpty(encryptionCertPath) || string.IsNullOrEmpty(signingCertPath)) + if (hasEncryptionCert != hasSigningCert) + { + var (missingKey, presentKey) = hasSigningCert + ? (ConfigKeys.OpenIddictEncryptionCertPath, ConfigKeys.OpenIddictSigningCertPath) + : (ConfigKeys.OpenIddictSigningCertPath, ConfigKeys.OpenIddictEncryptionCertPath); + + throw new InvalidOperationException( + $"'{missingKey}' is not configured but '{presentKey}' is. Both certificates " + + "are required together — with only one configured it would be ignored " + + "and both token keys would fall back to ephemeral. Configure both, or " + + "neither to explicitly run on ephemeral keys." + ); + } + + if (!hasEncryptionCert) { LogEphemeralKeys( logger, diff --git a/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs b/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs index 217f012e..055b0260 100644 --- a/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs +++ b/modules/OpenIddict/tests/SimpleModule.OpenIddict.Tests/Unit/OpenIddictProductionGuardTests.cs @@ -67,6 +67,32 @@ string environment warning.Message.Should().Contain("restart"); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task StartAsync_RealDeployment_HalfConfigured_ThrowsNamingMissingKey( + bool signingConfigured + ) + { + // Exactly one path set: the module would ignore the configured cert and + // run fully ephemeral — always an operator mistake, so fail loudly. + var configuredKey = signingConfigured + ? ConfigKeys.OpenIddictSigningCertPath + : ConfigKeys.OpenIddictEncryptionCertPath; + var missingKey = signingConfigured + ? ConfigKeys.OpenIddictEncryptionCertPath + : ConfigKeys.OpenIddictSigningCertPath; + var (guard, logger) = CreateGuard("Production", (configuredKey, CertPath)); + + ( + await guard + .Invoking(g => g.StartAsync(default)) + .Should() + .ThrowAsync() + ).WithMessage($"*'{missingKey}' is not configured but '{configuredKey}' is*"); + logger.Entries.Should().BeEmpty(); + } + [Fact] public async Task StartAsync_RealDeployment_FullyConfigured_DoesNotThrowOrWarn() {