diff --git a/backend/Api/Options/FrontendCorsOptions.cs b/backend/Api/Options/FrontendCorsOptions.cs new file mode 100644 index 00000000..401483d0 --- /dev/null +++ b/backend/Api/Options/FrontendCorsOptions.cs @@ -0,0 +1,29 @@ +namespace TrueMain.Options; + +/// +/// Cross-origin policy for the public API, bound from Cors:*. The single +/// list drives the FrontendCors policy: the browser +/// receives Access-Control-Allow-Origin only for the hosts listed here. +/// Named FrontendCorsOptions (not CorsOptions) to avoid colliding +/// with ASP.NET Core's own Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions. +/// +/// +/// An empty list is a silent-failure trap — the policy still builds, but with no +/// allowed origins the frontend's cross-origin requests are rejected by the +/// browser even though the API itself answers. It reads as "works locally, +/// broken in prod" because Development ships real origins while +/// appsettings.json ships an empty array. Startup therefore validates the +/// list: empty fails the boot in every non-Development environment (see +/// Program.cs) and only logs a warning under Development. +/// +public sealed class FrontendCorsOptions +{ + public const string SectionName = "Cors"; + + /// + /// Exact origins (scheme + host + port) allowed to make cross-origin browser + /// requests, e.g. https://truemain.app. Must be non-empty outside + /// Development. + /// + public string[] Origins { get; set; } = []; +} diff --git a/backend/Api/Program.cs b/backend/Api/Program.cs index bf0c1b22..01193710 100644 --- a/backend/Api/Program.cs +++ b/backend/Api/Program.cs @@ -12,6 +12,7 @@ using TrueMain.Services.Champions; using TrueMain.Services.Ops; using TrueMain.Services.Truemains; +using AspNetCorsOptions = Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions; var builder = WebApplication.CreateBuilder(args); const string frontendCorsPolicy = "FrontendCors"; @@ -40,18 +41,37 @@ tags: ["ready"]); } -var corsOrigins = builder.Configuration.GetSection("Cors:Origins").Get() ?? []; -builder.Services.AddCors(options => -{ - options.AddPolicy(frontendCorsPolicy, policy => - { - var builderPolicy = policy.AllowAnyHeader().AllowAnyMethod(); - if (corsOrigins.Length > 0) +// CORS origins must be present outside Development: an empty list still builds a +// valid (but no-op) policy, so without this guard production silently ships a +// CORS policy that allows no cross-origin browser request — the frontend appears +// to work locally (Development ships real origins) but breaks in prod, where +// appsettings.json ships an empty array. Fail the boot when empty in any +// non-Development environment; only warn under Development (handled after build). +var isDevelopment = builder.Environment.IsDevelopment(); +builder.Services.AddOptions() + .Bind(builder.Configuration.GetSection(FrontendCorsOptions.SectionName)) + .Validate( + options => isDevelopment || options.Origins.Length > 0, + "Cors:Origins must contain at least one origin outside the Development environment; " + + "an empty list ships a no-op CORS policy that silently rejects the frontend.") + .ValidateOnStart(); +builder.Services.AddCors(); +// Build the FrontendCors policy from the bound FrontendCorsOptions (single +// source — no separate config read) so the validated origins are the ones the +// policy uses. +builder.Services.AddOptions() + .Configure>((corsPolicies, appCors) => + corsPolicies.AddPolicy(frontendCorsPolicy, policy => { - builderPolicy.WithOrigins(corsOrigins); - } - }); -}); + var builderPolicy = policy.AllowAnyHeader().AllowAnyMethod(); + // Origins is guaranteed non-empty outside Development by + // ValidateOnStart; this guard only matters under Development, where an + // empty list is tolerated (and the policy then allows no origin). + if (appCors.Value.Origins.Length > 0) + { + builderPolicy.WithOrigins(appCors.Value.Origins); + } + })); builder.Services.AddOptions() .Bind(builder.Configuration.GetSection("MainAnalysis")) @@ -183,6 +203,18 @@ // the Ingestor's. builder.Services.AddMongoLogging(builder.Configuration, processName: "Api"); var app = builder.Build(); + +// Non-Development boots already fail in ValidateOnStart when Origins is empty; +// this only fires under Development, where an empty list is tolerated but still +// worth flagging so a missing local override doesn't read as a working CORS setup. +if (app.Environment.IsDevelopment() + && app.Services.GetRequiredService>().Value.Origins.Length == 0) +{ + app.Logger.LogWarning( + "Cors:Origins is empty; the {Policy} policy allows no cross-origin browser request. Set Cors:Origins in configuration to let the frontend reach the API.", + frontendCorsPolicy); +} + await DatabaseMigrator.ApplyPendingMigrationsAsync(app.Services); // Wrap unhandled exceptions in RFC 7807 ProblemDetails so clients diff --git a/backend/tests/TrueMain.IntegrationTests/CorsStartupIntegrationTests.cs b/backend/tests/TrueMain.IntegrationTests/CorsStartupIntegrationTests.cs new file mode 100644 index 00000000..94391e76 --- /dev/null +++ b/backend/tests/TrueMain.IntegrationTests/CorsStartupIntegrationTests.cs @@ -0,0 +1,107 @@ +using System.Net; +using AwesomeAssertions; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Options; + +namespace TrueMain.IntegrationTests; + +/// +/// Covers the startup CORS guard added for issue #209: outside Development an +/// empty Cors:Origins must fail the boot loudly (it would otherwise ship a +/// no-op CORS policy that silently rejects the frontend in production), while a +/// populated list boots and actually allows the configured origin. +/// +[Collection(IntegrationCollection.Name)] +public sealed class CorsStartupIntegrationTests +{ + private readonly PostgresFixture _fixture; + + public CorsStartupIntegrationTests(PostgresFixture fixture) + { + _fixture = fixture; + } + + [Fact] + public async Task Startup_InNonDevelopment_FailsWhenCorsOriginsEmpty() + { + // appsettings.json ships "Cors:Origins": [], so withholding the override + // leaves the list empty. Testing is non-Development, so ValidateOnStart + // must reject the boot rather than silently run with a no-op policy. + // (Testing, not Production: the readiness-health-check guard fails fast on + // a missing connection string only under Production, and the test host + // injects that string after Program reads it.) + await using var factory = new CorsStartupFactory(_fixture, environment: "Testing", origin: null); + + var startup = () => factory.CreateClient(); + + startup.Should().Throw( + "an empty Cors:Origins outside Development must fail the boot, not run a no-op policy") + .WithMessage("*Cors:Origins*"); + } + + [Fact] + public async Task Startup_InNonDevelopment_AllowsConfiguredOriginWhenPresent() + { + const string allowedOrigin = "https://app.truemain.test"; + await using var factory = new CorsStartupFactory(_fixture, environment: "Testing", origin: allowedOrigin); + using var client = factory.CreateClient(new WebApplicationFactoryClientOptions + { + BaseAddress = new Uri("https://localhost") + }); + + // A CORS preflight exercises the policy directly through the CORS + // middleware, so the assertion stays on the header that matters + // (Access-Control-Allow-Origin) without coupling to any endpoint's + // business logic or status code. + using var preflight = new HttpRequestMessage(HttpMethod.Options, "/champions"); + preflight.Headers.Add("Origin", allowedOrigin); + preflight.Headers.Add("Access-Control-Request-Method", "GET"); + + var response = await client.SendAsync(preflight); + + response.StatusCode.Should().Be(HttpStatusCode.NoContent, + "a handled CORS preflight is short-circuited by the middleware with 204 No Content"); + response.Headers.GetValues("Access-Control-Allow-Origin").Should().ContainSingle() + .Which.Should().Be(allowedOrigin, + "the configured origin must be echoed back so the browser accepts the cross-origin response"); + } + + /// + /// A that — unlike + /// — does not inject a + /// default Cors:Origins, so a test can drive the host with the origin + /// list either empty (a null origin argument) or populated, and pick the + /// environment that decides whether the guard fails or warns. + /// + private sealed class CorsStartupFactory : WebApplicationFactory + { + private readonly string _environment; + private readonly List> _configuration; + + public CorsStartupFactory(PostgresFixture fixture, string environment, string? origin) + { + _environment = environment; + _configuration = + [ + new KeyValuePair("ConnectionStrings:TrueMain", fixture.ConnectionString), + new KeyValuePair( + "Ops:ApiKey", + TrueMainWebApplicationFactory.DefaultOpsApiKey) + ]; + + if (origin is not null) + { + _configuration.Add(new KeyValuePair("Cors:Origins:0", origin)); + } + } + + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + builder.UseEnvironment(_environment); + builder.ConfigureAppConfiguration((_, configurationBuilder) => + configurationBuilder.AddInMemoryCollection(_configuration)); + } + } +} diff --git a/backend/tests/TrueMain.TestKit/TrueMainWebApplicationFactory.cs b/backend/tests/TrueMain.TestKit/TrueMainWebApplicationFactory.cs index 895439ec..6fad1eb9 100644 --- a/backend/tests/TrueMain.TestKit/TrueMainWebApplicationFactory.cs +++ b/backend/tests/TrueMain.TestKit/TrueMainWebApplicationFactory.cs @@ -9,7 +9,9 @@ namespace TrueMain.TestKit; /// against a : sets the Testing /// environment, injects ConnectionStrings:TrueMain, a test /// Ops:ApiKey that satisfies the [MinLength(32)] validation, -/// plus any additional overrides the test wants to add. +/// a default Cors:Origins entry (Testing is non-Development, so the +/// startup CORS guard fails the boot when the list is empty), plus any +/// additional overrides the test wants to add. /// public class TrueMainWebApplicationFactory( PostgresFixture fixture, @@ -24,6 +26,13 @@ public class TrueMainWebApplicationFactory( /// public const string DefaultOpsApiKey = "test-kit-ops-key-0123456789-abcdefghijklmnop"; + /// + /// A single allowed origin so the startup CORS guard (which fails the boot + /// outside Development when Cors:Origins is empty) is satisfied for + /// the Testing environment tests run under. + /// + public const string DefaultCorsOrigin = "https://frontend.test.truemain.local"; + protected override void ConfigureWebHost(IWebHostBuilder builder) { builder.UseEnvironment("Testing"); @@ -32,7 +41,8 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) var baseline = new List> { new("ConnectionStrings:TrueMain", fixture.ConnectionString), - new("Ops:ApiKey", DefaultOpsApiKey) + new("Ops:ApiKey", DefaultOpsApiKey), + new("Cors:Origins:0", DefaultCorsOrigin) }; if (extraConfiguration is { Count: > 0 })