From 00028214ebe6520e67ba1888fbf5fde5fd772488 Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Fri, 14 Aug 2026 20:16:23 +0800 Subject: [PATCH] Answer 404 for a path this server does not serve The global FallbackPolicy requires an authenticated user for any endpoint without an explicit [Authorize] -- deliberately, so a forgotten attribute cannot mean anonymous access. It also applied to requests that matched NO endpoint, and there the 401 it produced is a lie: the request was refused for want of a route, not a session, and adding one would not have helped. That cost a real diagnosis. An invite link built from a misconfigured App:PublicBaseUrl pointed at the API host rather than the SPA, so opening it asked this server for /invite/{token} -- a path only the SPA has. The 401 sent the investigation hunting a broken authorization rule on an endpoint that was already [AllowAnonymous] and working. Implemented as a fallback ENDPOINT rather than middleware before UseAuthorization, because only the endpoint form composes with branch middleware. An adversarial review caught the first attempt: Hangfire mounts its dashboard with app.Map, which registers no endpoint, so a middleware keyed on "GetEndpoint() == null" swallowed /hangfire whole and made UseCodeSpaceHangfire dead code on the API role. Routing reaches a fallback only after every real endpoint and every earlier branch has declined, so the special case disappears rather than needing an allowlist. The boot banner also names where invite and reset links will point, read from the raw configuration key rather than through PublicBaseUrlSetting: that type throws outside Development when the key is unset and is resolved lazily today, so constructing it at startup would move the failure to boot for both roles -- and Main catches it into a Fatal log and returns 0, so the container would exit cleanly and every restart would look like a healthy stop. --- backend/src/CodeSpace.Api/Program.cs | 16 +++ backend/src/CodeSpace.Api/Startup.cs | 26 ++++ .../Auth/UnroutedRequestE2ETests.cs | 131 ++++++++++++++++++ .../Infrastructure/TaskLaunchApiFactory.cs | 3 + 4 files changed, 176 insertions(+) create mode 100644 backend/tests/CodeSpace.E2ETests/Auth/UnroutedRequestE2ETests.cs diff --git a/backend/src/CodeSpace.Api/Program.cs b/backend/src/CodeSpace.Api/Program.cs index 1ca52137d..cdc7047a2 100644 --- a/backend/src/CodeSpace.Api/Program.cs +++ b/backend/src/CodeSpace.Api/Program.cs @@ -52,6 +52,22 @@ public static void Main(string[] args) environment, string.IsNullOrWhiteSpace(seqDestination) ? "none — Seq is off, set " + SerilogServerUrlSetting.ConfigurationKey + " to enable it" : seqDestination); + // Where the links this server MINTS will point. It is the SPA's origin, not this API's, + // and the two are usually different hosts -- an invite pointed at the API opens a path + // only the SPA has, which is a 404 from a server the invitee has no account on. Printing + // it costs a line and turns that into something an operator sees before sending one. + // + // The RAW key, deliberately, not PublicBaseUrlSetting: that type throws outside + // Development when the key is unset, and it is resolved lazily today -- only by the two + // request-scoped services that mint links. Constructing it here would move that failure + // to boot, for both roles, and Main catches it into a Fatal log and returns 0, so the + // container would exit CLEANLY and every restart would look like a healthy stop. A line + // that reports configuration must not be able to change what configuration does. + Log.Information("Invite and password-reset links will point at {PublicBaseUrl} (this must be the web app, not this API)", + configuration[PublicBaseUrlSetting.ConfigurationKey] is { Length: > 0 } configured + ? configured + : $"(unset — set {PublicBaseUrlSetting.ConfigurationKey}; outside Development, minting a link will fail)"); + Log.Information("Configuring {Application} host...", application); new DbUpRunner(new CodeSpaceConnectionString(configuration).Value).Run(); diff --git a/backend/src/CodeSpace.Api/Startup.cs b/backend/src/CodeSpace.Api/Startup.cs index d358c84de..0b8de4926 100644 --- a/backend/src/CodeSpace.Api/Startup.cs +++ b/backend/src/CodeSpace.Api/Startup.cs @@ -1,5 +1,6 @@ using System.Text.Json.Serialization; using CodeSpace.Api.Extensions; +using CodeSpace.Messages.Failures; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Services.Workflows.Llm; using CodeSpace.Api.Filters; @@ -128,6 +129,31 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) endpoints.MapControllers(); if (env.IsDevelopment()) endpoints.MapOpenApi(); + + // A path this server does not serve answers 404, not 401. The global FallbackPolicy + // requires an authenticated user for any endpoint without an explicit [Authorize] -- + // deliberately -- and it applied to requests that matched NOTHING too, where the answer + // is a lie: the request was refused for want of a route, not a session, and adding one + // would not have helped. That cost a real diagnosis, an invite link built from a + // misconfigured App:PublicBaseUrl pointed at this API instead of the SPA, so opening it + // asked here for /invite/{token} -- a path only the SPA has -- and the 401 sent everyone + // hunting a broken authorization rule on an endpoint that was already anonymous. + // + // A fallback ENDPOINT rather than middleware before UseAuthorization, because only the + // endpoint form composes with branch middleware. Hangfire's dashboard is mounted by + // app.Map (above, line ~124), which never registers an endpoint, so a middleware keyed + // on "GetEndpoint() == null" would have swallowed /hangfire whole. Routing reaches this + // only after every real endpoint AND every earlier branch has declined. + endpoints.MapFallback(async context => + { + context.Response.StatusCode = StatusCodes.Status404NotFound; + + await context.Response.WriteAsJsonAsync(new Dictionary + { + ["code"] = FailureCodes.NotFound, + ["message"] = $"This server has no {context.Request.Path.Value}. Check the host — the API and the web app are different origins." + }); + }).AllowAnonymous(); }); } } diff --git a/backend/tests/CodeSpace.E2ETests/Auth/UnroutedRequestE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Auth/UnroutedRequestE2ETests.cs new file mode 100644 index 000000000..331db1ace --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Auth/UnroutedRequestE2ETests.cs @@ -0,0 +1,131 @@ +using System.Net; +using System.Net.Http.Json; +using CodeSpace.E2ETests.Infrastructure; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.Configuration; +using Shouldly; + +namespace CodeSpace.E2ETests.Auth; + +/// +/// A path this server does not serve says so, and every path it DOES serve still requires a session. +/// +/// The second half is the one that must not regress. The global FallbackPolicy exists so +/// that an endpoint someone forgot to mark [Authorize] is refused rather than silently +/// anonymous; answering 404 for unmatched routes must not become a way to reach a matched one. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class UnroutedRequestE2ETests : IClassFixture +{ + private readonly TaskLaunchApiFactory _factory; + + public UnroutedRequestE2ETests(TaskLaunchApiFactory factory) { _factory = factory; } + + /// + /// The exact shape that cost a production diagnosis: an invite link built from a misconfigured + /// public base URL opens the API host, and /invite/{token} is a route only the SPA has. + /// + [Fact] + public async Task A_web_app_route_requested_from_the_api_says_there_is_no_such_page() + { + var response = await _factory.CreateClient().GetAsync("/invite/some-token"); + + response.StatusCode.ShouldBe(HttpStatusCode.NotFound, + customMessage: "An unrouted path answered 401, which reads as 'you need to sign in' when the truth is " + + "'this server has never had this page'. That sent a real investigation after an " + + "authorization rule on an endpoint that was already anonymous and working."); + + var body = await response.Content.ReadFromJsonAsync>(); + + body!["message"].ShouldContain("no /invite/some-token"); + body["message"].ShouldContain("different origins", + customMessage: "The message has to name the likely cause. A bare 404 is honest but still leaves the reader guessing."); + } + + [Fact] + public async Task An_unknown_path_is_not_found_rather_than_unauthorized() + { + var response = await _factory.CreateClient().GetAsync("/nothing-here"); + + response.StatusCode.ShouldBe(HttpStatusCode.NotFound); + } + + /// + /// The guard. A real endpoint with no explicit attribute still meets the fallback policy — this + /// is what stops the change above from turning into anonymous access. + /// + [Fact] + public async Task A_real_endpoint_still_refuses_an_unauthenticated_caller() + { + var response = await _factory.CreateClient().GetAsync("/api/repositories"); + + response.StatusCode.ShouldBe(HttpStatusCode.Unauthorized, + customMessage: "A matched endpoint must still be challenged. If this is 404, the middleware is running " + + "for endpoints that DID match and the FallbackPolicy has been bypassed."); + } + + /// An anonymous endpoint that exists must still run, not be swallowed as unrouted. + [Fact] + public async Task An_anonymous_endpoint_still_answers() + { + var response = await _factory.CreateClient().GetAsync("/api/invitations/not-a-real-token"); + + response.StatusCode.ShouldBe(HttpStatusCode.NotFound); + + var body = await response.Content.ReadFromJsonAsync>(); + + body!["code"].ShouldBe("invitation_not_usable", + customMessage: "This is the invitation endpoint's OWN 404 about a token, not the middleware's about a route. " + + "If the code is 'not_found', the middleware swallowed a matched endpoint."); + } + + /// + /// The regression an adversarial review caught before this shipped. The first attempt answered + /// 404 from a middleware placed before UseAuthorization, keyed on + /// GetEndpoint() == null — and Hangfire's dashboard is mounted by app.Map, classic + /// branch middleware that never registers an endpoint. The middleware therefore swallowed + /// /hangfire whole and UseCodeSpaceHangfire became dead code on the API role. + /// + /// A fallback ENDPOINT cannot do that: routing reaches it only after every earlier branch + /// has declined. This asserts the property that makes it safe, so a future move back to + /// middleware fails here. + /// + [Fact] + public async Task The_not_found_answer_does_not_swallow_branch_mounted_paths() + { + await using var apiRole = new HangfireApiRoleFactory(_factory); + + var response = await apiRole.CreateClient().GetAsync("/hangfire"); + + var body = response.Content.Headers.ContentType?.MediaType == "application/json" + ? await response.Content.ReadFromJsonAsync>() + : null; + + body?.GetValueOrDefault("code").ShouldNotBe("not_found", + customMessage: "/hangfire was answered by the catch-all rather than by Hangfire's own branch. The " + + "dashboard is mounted with app.Map and has no endpoint, so anything keyed on " + + "\"no endpoint matched\" unmounts it."); + } + + /// Runs the same app with the Hangfire role that actually mounts the dashboard; the shared factory runs Worker, which mounts none. + private sealed class HangfireApiRoleFactory : WebApplicationFactory + { + private readonly TaskLaunchApiFactory _inner; + + public HangfireApiRoleFactory(TaskLaunchApiFactory inner) { _inner = inner; } + + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + builder.UseEnvironment("Development"); + builder.ConfigureAppConfiguration((_, cfg) => cfg.AddInMemoryCollection(new Dictionary + { + ["CodeSpaceStore:ConnectionString"] = _inner.ConnectionString, + ["Authentication:Jwt:SymmetricKey"] = TaskLaunchApiFactory.JwtKey, + ["OAuth:CallbackUrl"] = "http://localhost/api/credentials/oauth/callback", + ["HangfireHosting"] = "Api", + })); + } + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Infrastructure/TaskLaunchApiFactory.cs b/backend/tests/CodeSpace.E2ETests/Infrastructure/TaskLaunchApiFactory.cs index e1e0efb97..db44b3d5d 100644 --- a/backend/tests/CodeSpace.E2ETests/Infrastructure/TaskLaunchApiFactory.cs +++ b/backend/tests/CodeSpace.E2ETests/Infrastructure/TaskLaunchApiFactory.cs @@ -72,6 +72,9 @@ async Task IAsyncLifetime.DisposeAsync() await drop.ExecuteNonQueryAsync(); } + /// The per-run test database, so a sibling factory can host the same app in a different role against the same data. + public string ConnectionString => _testConnectionString; + protected override void ConfigureWebHost(IWebHostBuilder builder) { builder.UseEnvironment("Development");