diff --git a/Applications/Pgan.PoracleWebNet.Api/Configuration/SecurityHeaders.cs b/Applications/Pgan.PoracleWebNet.Api/Configuration/SecurityHeaders.cs new file mode 100644 index 00000000..490ddabf --- /dev/null +++ b/Applications/Pgan.PoracleWebNet.Api/Configuration/SecurityHeaders.cs @@ -0,0 +1,47 @@ +namespace Pgan.PoracleWebNet.Api.Configuration; + +/// +/// Response security headers applied to every request by the middleware in Program.cs. +/// Extracted from an inline lambda so the policy values are assertable in unit tests +/// without booting the whole app (issue #383). +/// +public static class SecurityHeaders +{ + /// + /// Sends the full referrer on same-origin requests and nothing at all cross-origin. + /// + /// + /// Cross-origin suppression is the point: without it, every remote image host the SPA + /// touches (uicons on raw.githubusercontent.com, Discord avatars on cdn.discordapp.com, + /// Google Fonts) learns the origin of the PoracleWeb instance the user is browsing, + /// which for a private instance is the thing worth not disclosing. Issue #383. + /// + /// Do NOT tighten this to no-referrer. AuthController reads the Referer + /// header on the login and logout entry points (DiscordLogin, the OIDC login path, and + /// OIDC RP-initiated logout) to recover which frontend origin the user came from, + /// validate it against the configured CORS origins, and redirect back there after the + /// provider callback. Those reads are same-origin when the SPA is served by this host, + /// so same-origin keeps them working; no-referrer would blank them and + /// silently bounce users to this host's own origin after login instead. + /// + public const string ReferrerPolicy = "same-origin"; + + public const string ContentSecurityPolicy = + "default-src 'self'; script-src 'self' 'unsafe-hashes' 'sha256-MhtPZXr7+LpJUY5qtMutB+qWfQtMaPccfe7QXtCcEYc=' https://telegram.org; " + + "style-src 'self' 'unsafe-inline'; font-src 'self' https://fonts.gstatic.com; img-src 'self' data: https:; " + + "connect-src 'self' https://raw.githubusercontent.com; frame-src https://oauth.telegram.org"; + + /// + /// Stamps the security headers onto an outgoing response. + /// + public static void Apply(IHeaderDictionary headers) + { + ArgumentNullException.ThrowIfNull(headers); + + headers.XContentTypeOptions = "nosniff"; + headers.XFrameOptions = "DENY"; + headers.XXSSProtection = "0"; + headers["Referrer-Policy"] = ReferrerPolicy; + headers.ContentSecurityPolicy = ContentSecurityPolicy; + } +} diff --git a/Applications/Pgan.PoracleWebNet.Api/Program.cs b/Applications/Pgan.PoracleWebNet.Api/Program.cs index f887c8fd..9d2f712c 100644 --- a/Applications/Pgan.PoracleWebNet.Api/Program.cs +++ b/Applications/Pgan.PoracleWebNet.Api/Program.cs @@ -370,17 +370,12 @@ await context.Response.WriteAsync( forwardedHeadersOptions.KnownProxies.Clear(); app.UseForwardedHeaders(forwardedHeadersOptions); -// Security headers +// Security headers -- values live in SecurityHeaders so they can be unit-tested app.Use(async (context, next) => { context.Response.OnStarting(() => { - var headers = context.Response.Headers; - headers.XContentTypeOptions = "nosniff"; - headers.XFrameOptions = "DENY"; - headers.XXSSProtection = "0"; - headers["Referrer-Policy"] = "strict-origin-when-cross-origin"; - headers.ContentSecurityPolicy = "default-src 'self'; script-src 'self' 'unsafe-hashes' 'sha256-MhtPZXr7+LpJUY5qtMutB+qWfQtMaPccfe7QXtCcEYc=' https://telegram.org; style-src 'self' 'unsafe-inline'; font-src 'self' https://fonts.gstatic.com; img-src 'self' data: https:; connect-src 'self' https://raw.githubusercontent.com; frame-src https://oauth.telegram.org"; + SecurityHeaders.Apply(context.Response.Headers); return Task.CompletedTask; }); await next(); diff --git a/CHANGELOG.md b/CHANGELOG.md index 7078897b..f9ad97f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security - **Gym-picker images no longer send a `Referer` header to third-party hosts** ([#242](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/242)): the gym picker renders two kinds of remote image — the scanner DB's `gym.url` photo (a Niantic CDN URL in stock Golbat/RDM deployments, though an operator can rewrite the column to point at a self-hosted mirror) and the team-icon fallback from `raw.githubusercontent.com`. Neither carried a referrer policy, so every image request told the remote host which PoracleWeb instance the user was browsing. All four `` tags in `gym-picker.component.html` now set `referrerpolicy="no-referrer"`. Modern browsers already default to `strict-origin-when-cross-origin`, so the pre-existing leak was the origin rather than the full URL — this closes the remainder. Presentation-only: no API, model, or scanner-query change, and `GymSearchResult.Url` still carries the raw scanner URL as before. The photo-proxy endpoint floated in the original issue was **not** implemented: server-side fetching of a URL supplied by a database PoracleWeb does not own would turn a passive disclosure into an authenticated outbound-request primitive from a host that can reach Poracle, Koji, Golbat, and both MySQL servers. A host allowlist applied at projection remains the cheaper option if a deployment ever needs the mirror case handled. +- **App-wide `Referrer-Policy` tightened to `same-origin`, so no remote host learns the instance origin** ([#383](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/383)): the per-element fix above covered the gym picker, but the same leak existed everywhere else the SPA loads a remote resource — uicons from `raw.githubusercontent.com` (`icon.service.ts`, operator-overridable, so possibly a self-hosted mirror) across the Pokémon/raid/egg/lure/invasion/gym/quick-pick lists and dialogs, Discord avatars from `cdn.discordapp.com`, and the Google Fonts stylesheets in `index.html`. Each request disclosed the origin of the PoracleWeb instance being browsed, which for a private or invite-only deployment is the part worth withholding. The security-headers middleware previously sent `strict-origin-when-cross-origin` (the browser default, which sends the origin cross-origin); it now sends `same-origin` — full referrer within the site, nothing at all to third parties — fixing every case in one place rather than annotating tags individually. `no-referrer` was considered and rejected: `AuthController` reads the `Referer` header on `DiscordLogin`, the OIDC login path, and OIDC RP-initiated logout to recover which frontend origin the user came from, validate it against the configured CORS origins, and redirect back there after the provider callback — blanking the same-origin referrer would degrade all three to this host's own origin and bounce users to the wrong place. The header values moved out of the inline lambda in `Program.cs` into a `SecurityHeaders` class so they're assertable without booting the app; the CSP is carried over byte-identical (a test pins it against the original literal). Tests cover the policy value, a guard that it never becomes `no-referrer` or any of the origin-leaking values, and the previously untested `AuthController` origin recovery it depends on (allowed referer honored, disallowed and non-absolute referers rejected, absent referer falling back to self). The per-element `referrerpolicy` attributes from #242 are left in place as defence-in-depth. ## [2.11.1] - 2026-06-05 diff --git a/Tests/Pgan.PoracleWebNet.Tests/Configuration/SecurityHeadersTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Configuration/SecurityHeadersTests.cs new file mode 100644 index 00000000..ed2900e5 --- /dev/null +++ b/Tests/Pgan.PoracleWebNet.Tests/Configuration/SecurityHeadersTests.cs @@ -0,0 +1,99 @@ +using Microsoft.AspNetCore.Http; +using Pgan.PoracleWebNet.Api.Configuration; + +namespace Pgan.PoracleWebNet.Tests.Configuration; + +/// +/// Tests for the response security headers (issue #383). +/// The Referrer-Policy assertions are the point of this file: the value has to stay +/// cross-origin-suppressing (so remote image hosts don't learn the instance origin) while +/// still sending a same-origin referrer, which AuthController's login/logout redirects +/// depend on. +/// +public class SecurityHeadersTests +{ + /// + /// The CSP exactly as it was written inline in Program.cs before being extracted into + /// . Guards the extraction against a typo in the + /// concatenated string. + /// + private const string OriginalCsp = + "default-src 'self'; script-src 'self' 'unsafe-hashes' 'sha256-MhtPZXr7+LpJUY5qtMutB+qWfQtMaPccfe7QXtCcEYc=' https://telegram.org; style-src 'self' 'unsafe-inline'; font-src 'self' https://fonts.gstatic.com; img-src 'self' data: https:; connect-src 'self' https://raw.githubusercontent.com; frame-src https://oauth.telegram.org"; + + [Fact] + public void Apply_SetsReferrerPolicy_ToSameOrigin() + { + IHeaderDictionary headers = new HeaderDictionary(); + + SecurityHeaders.Apply(headers); + + Assert.Equal("same-origin", headers["Referrer-Policy"]); + } + + /// + /// A referrer policy of no-referrer blanks the Referer header on same-origin requests + /// too, which breaks the origin recovery in AuthController's DiscordLogin, OIDC login, + /// and OIDC logout handlers -- they would silently fall back to this host's own origin + /// and redirect users to the wrong place after a provider callback. If this assertion + /// fails, read the remarks on SecurityHeaders.ReferrerPolicy before changing it. + /// + [Fact] + public void ReferrerPolicy_IsNotNoReferrer_SoAuthRedirectsKeepWorking() + { + Assert.NotEqual("no-referrer", SecurityHeaders.ReferrerPolicy); + } + + /// + /// The whole reason for #383: the policy must not send anything cross-origin. The two + /// values that leak the origin are the browser default and the explicit unsafe opt-outs. + /// + [Theory] + [InlineData("strict-origin-when-cross-origin")] + [InlineData("no-referrer-when-downgrade")] + [InlineData("origin")] + [InlineData("origin-when-cross-origin")] + [InlineData("unsafe-url")] + public void ReferrerPolicy_DoesNotLeakOriginCrossOrigin(string leakyPolicy) + { + Assert.NotEqual(leakyPolicy, SecurityHeaders.ReferrerPolicy); + } + + [Fact] + public void Apply_SetsContentSecurityPolicy_UnchangedFromTheInlineVersion() + { + IHeaderDictionary headers = new HeaderDictionary(); + + SecurityHeaders.Apply(headers); + + Assert.Equal(OriginalCsp, headers.ContentSecurityPolicy); + } + + [Fact] + public void Apply_SetsTheRemainingHardeningHeaders() + { + IHeaderDictionary headers = new HeaderDictionary(); + + SecurityHeaders.Apply(headers); + + Assert.Equal("nosniff", headers.XContentTypeOptions); + Assert.Equal("DENY", headers.XFrameOptions); + Assert.Equal("0", headers.XXSSProtection); + } + + [Fact] + public void Apply_OverwritesAnyValuePresetByAnUpstreamProxyOrHost() + { + IHeaderDictionary headers = new HeaderDictionary + { + ["Referrer-Policy"] = "unsafe-url" + }; + + SecurityHeaders.Apply(headers); + + Assert.Equal("same-origin", headers["Referrer-Policy"]); + } + + [Fact] + public void Apply_Throws_WhenHeadersAreNull() => + Assert.Throws(() => SecurityHeaders.Apply(null!)); +} diff --git a/Tests/Pgan.PoracleWebNet.Tests/Controllers/AuthControllerLoginOriginTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Controllers/AuthControllerLoginOriginTests.cs new file mode 100644 index 00000000..22f46319 --- /dev/null +++ b/Tests/Pgan.PoracleWebNet.Tests/Controllers/AuthControllerLoginOriginTests.cs @@ -0,0 +1,149 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Moq; +using Pgan.PoracleWebNet.Api.Configuration; +using Pgan.PoracleWebNet.Api.Controllers; +using Pgan.PoracleWebNet.Api.Services.Oidc; +using Pgan.PoracleWebNet.Core.Abstractions.Services; + +namespace Pgan.PoracleWebNet.Tests.Controllers; + +/// +/// Tests for the frontend-origin recovery in AuthController.DiscordLogin — the Referer read +/// that decides which origin the user is sent back to after the provider callback, stashed in +/// the oauth_origin cookie. +/// +/// This behavior is why the app's Referrer-Policy is "same-origin" and not "no-referrer" +/// (issue #383): a policy that blanks the same-origin Referer would make every case below +/// fall back to the API's own origin. See SecurityHeaders.ReferrerPolicy. +/// +public class AuthControllerLoginOriginTests +{ + private const string SelfOrigin = "https://alerts.example.net"; + + [Fact] + public async Task DiscordLoginUsesRefererOriginWhenItIsAnAllowedCorsOrigin() + { + var controller = CreateController( + allowedOrigins: ["https://app.example.net", SelfOrigin], + referer: "https://app.example.net/auth/login"); + + await controller.DiscordLogin(); + + Assert.Equal("https://app.example.net", ReadOriginCookie(controller)); + } + + /// + /// The shared-host production topology: the SPA is served by this same host, so the + /// referrer is same-origin. "same-origin" policy sends the full URL here, so the header + /// is present and the recovered origin matches self. + /// + [Fact] + public async Task DiscordLoginUsesRefererOriginWhenItMatchesSelfAndNoCorsOriginsConfigured() + { + var controller = CreateController( + allowedOrigins: [], + referer: $"{SelfOrigin}/auth/login"); + + await controller.DiscordLogin(); + + Assert.Equal(SelfOrigin, ReadOriginCookie(controller)); + } + + /// + /// The no-referrer scenario, asserted explicitly: with no Referer to read, the origin + /// silently degrades to this host. Harmless when the SPA shares the host, wrong when it + /// doesn't — which is the regression a "no-referrer" policy would introduce everywhere. + /// + [Fact] + public async Task DiscordLoginFallsBackToSelfOriginWhenRefererIsAbsent() + { + var controller = CreateController(allowedOrigins: ["https://app.example.net"], referer: null); + + await controller.DiscordLogin(); + + Assert.Equal(SelfOrigin, ReadOriginCookie(controller)); + } + + [Fact] + public async Task DiscordLoginIgnoresRefererOriginThatIsNotAllowed() + { + var controller = CreateController( + allowedOrigins: ["https://app.example.net"], + referer: "https://evil.example.com/auth/login"); + + await controller.DiscordLogin(); + + Assert.Equal(SelfOrigin, ReadOriginCookie(controller)); + } + + [Fact] + public async Task DiscordLoginIgnoresRefererThatIsNotAnAbsoluteUri() + { + var controller = CreateController(allowedOrigins: [], referer: "/auth/login"); + + await controller.DiscordLogin(); + + Assert.Equal(SelfOrigin, ReadOriginCookie(controller)); + } + + private static AuthController CreateController(string[] allowedOrigins, string? referer) + { + var settings = new Dictionary(); + for (var i = 0; i < allowedOrigins.Length; i++) + { + settings[$"Cors:AllowedOrigins:{i}"] = allowedOrigins[i]; + } + + var controller = new AuthController( + new Mock().Object, + new Mock().Object, + new Mock().Object, + new Mock().Object, + new Mock().Object, + new Mock().Object, + new Mock().Object, + new Mock().Object, + Options.Create(new DiscordSettings { ClientId = "test-id", ClientSecret = "test-secret" }), + Options.Create(new TelegramSettings()), + Options.Create(new OidcSettings()), + Options.Create(new PoracleSettings()), + new ConfigurationBuilder().AddInMemoryCollection(settings).Build(), + new Mock>().Object); + + var httpContext = new DefaultHttpContext(); + httpContext.Request.Scheme = "https"; + httpContext.Request.Host = new HostString("alerts.example.net"); + if (referer != null) + { + httpContext.Request.Headers.Referer = referer; + } + + controller.ControllerContext = new ControllerContext { HttpContext = httpContext }; + return controller; + } + + private static string? ReadOriginCookie(ControllerBase controller) + { + const string Name = "oauth_origin="; + + var cookie = controller.Response.Headers.SetCookie + .FirstOrDefault(c => c != null && c.StartsWith(Name, StringComparison.Ordinal)); + if (cookie == null) + { + return null; + } + + var value = cookie[Name.Length..]; + var end = value.IndexOf(';', StringComparison.Ordinal); + if (end >= 0) + { + value = value[..end]; + } + + return Uri.UnescapeDataString(value); + } +}