diff --git a/README.md b/README.md index 70af1485..1648f734 100644 --- a/README.md +++ b/README.md @@ -73,9 +73,24 @@ 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. 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 \ + -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..3fe0960f 100644 --- a/docs/site/guide/identity.md +++ b/docs/site/guide/identity.md @@ -97,19 +97,23 @@ 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. 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 776d4e9f..13d879a3 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(); } @@ -138,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 d84c7461..e33a1a16 100644 --- a/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs +++ b/modules/OpenIddict/src/SimpleModule.OpenIddict/Services/OpenIddictProductionGuard.cs @@ -1,24 +1,32 @@ 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. +/// 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. 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. /// -public sealed class OpenIddictProductionGuard( +public sealed partial class OpenIddictProductionGuard( IConfiguration configuration, - IHostEnvironment environment + IHostEnvironment environment, + ILogger logger ) : IHostedService { public Task StartAsync(CancellationToken cancellationToken) @@ -39,15 +47,30 @@ 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( - $"'{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." + $"'{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, + environment.EnvironmentName, + ConfigKeys.OpenIddictSigningCertPath, + ConfigKeys.OpenIddictEncryptionCertPath ); } @@ -55,4 +78,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..055b0260 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,61 @@ 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, logger) = CreateGuard(environment); // no cert paths configured + + 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"); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task StartAsync_RealDeployment_HalfConfigured_ThrowsNamingMissingKey( + bool signingConfigured + ) { - var guard = CreateGuard(environment); // no cert paths configured + // 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("*certificate*"); + ).WithMessage($"*'{missingKey}' is not configured but '{configuredKey}' is*"); + logger.Entries.Should().BeEmpty(); } [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 +115,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 +131,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))); + } }