diff --git a/src/BuildingBlocks/Web/Extensions.cs b/src/BuildingBlocks/Web/Extensions.cs index 50c6568fda..5ee1dfe320 100644 --- a/src/BuildingBlocks/Web/Extensions.cs +++ b/src/BuildingBlocks/Web/Extensions.cs @@ -20,8 +20,10 @@ using FSH.Framework.Web.RateLimiting; using FSH.Framework.Web.Realtime; using FSH.Framework.Web.Security; +using FSH.Framework.Web.TrustedProxy; using FSH.Framework.Web.Versioning; using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.HttpOverrides; using Microsoft.AspNetCore.ResponseCompression; using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.Configuration; @@ -29,6 +31,7 @@ using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Hosting; using Mediator; +using System.Net; namespace FSH.Framework.Web; @@ -63,6 +66,39 @@ public static IHostApplicationBuilder AddHeroPlatform(this IHostApplicationBuild } builder.Services.AddHttpContextAccessor(); + + // The app runs behind a reverse proxy (e.g. cloudflared → Caddy → app), so the real client IP + // and scheme arrive via X-Forwarded-*. Without this, RemoteIpAddress is the proxy's container + // IP, which collapses the rate-limit partition into one bucket and records useless audit IPs. + // Trust is bound to the configured ingress CIDRs/proxies (see TrustedProxyOptions): forwarded + // headers from any other source are ignored, so a client reaching the app directly cannot forge + // its IP/scheme. With nothing configured, the framework default (loopback only) stands. + var trustedProxy = builder.Configuration + .GetSection(nameof(TrustedProxyOptions)).Get() ?? new TrustedProxyOptions(); + builder.Services.Configure(forwarded => + { + forwarded.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; + forwarded.ForwardLimit = trustedProxy.ForwardLimit; + + if (trustedProxy.KnownProxies.Length == 0 && trustedProxy.KnownNetworks.Length == 0) + { + return; + } + + forwarded.KnownProxies.Clear(); + forwarded.KnownIPNetworks.Clear(); + + foreach (var proxy in trustedProxy.KnownProxies) + { + forwarded.KnownProxies.Add(IPAddress.Parse(proxy)); + } + + foreach (var network in trustedProxy.KnownNetworks) + { + forwarded.KnownIPNetworks.Add(System.Net.IPNetwork.Parse(network)); + } + }); + builder.Services.AddHeroDatabaseOptions(builder.Configuration); builder.Services.AddHeroRateLimiting(builder.Configuration); @@ -150,6 +186,11 @@ public static WebApplication UseHeroPlatform(this WebApplication app, Action +/// Trusted reverse-proxy configuration for X-Forwarded-* processing. Behind an ingress +/// (e.g. cloudflared → Caddy → app) the real client IP and scheme arrive via forwarded headers; +/// these settings bound which upstream sources are trusted so a client reaching the app from +/// outside the proxy network cannot forge its own IP/scheme. When no proxies or networks are +/// configured, the framework default (loopback only) stands and forwarded headers from any other +/// source are ignored. +/// +public sealed class TrustedProxyOptions +{ + /// Individual upstream proxy IP addresses whose X-Forwarded-* headers are trusted. + public string[] KnownProxies { get; init; } = []; + + /// Trusted upstream networks in CIDR notation (e.g. "10.0.0.0/8", "172.16.0.0/12"). + public string[] KnownNetworks { get; init; } = []; + + /// + /// Number of proxy hops to unwind from X-Forwarded-For. Must match the real ingress hop count + /// (cloudflared → Caddy → app is 2). The framework default of 1 reads only the rightmost hop, + /// which yields the nearest proxy's IP (or an attacker-injected value) in a multi-hop topology. + /// + public int ForwardLimit { get; init; } = 1; +} diff --git a/src/Host/FSH.Starter.Api/appsettings.Production.json b/src/Host/FSH.Starter.Api/appsettings.Production.json index 332724534b..8cf660327d 100644 --- a/src/Host/FSH.Starter.Api/appsettings.Production.json +++ b/src/Host/FSH.Starter.Api/appsettings.Production.json @@ -91,6 +91,11 @@ "Ip": { "PermitLimit": 300, "WindowSeconds": 60, "QueueLimit": 0 }, "Auth": { "PermitLimit": 10, "WindowSeconds": 60, "QueueLimit": 0 } }, + "TrustedProxyOptions": { + "KnownProxies": [], + "KnownNetworks": [], + "ForwardLimit": 1 + }, "Storage": { "Provider": "local" } diff --git a/src/Host/FSH.Starter.Api/appsettings.json b/src/Host/FSH.Starter.Api/appsettings.json index 293fdfebb6..6a0a968ba2 100644 --- a/src/Host/FSH.Starter.Api/appsettings.json +++ b/src/Host/FSH.Starter.Api/appsettings.json @@ -156,6 +156,11 @@ "QueueLimit": 0 } }, + "TrustedProxyOptions": { + "KnownProxies": [], + "KnownNetworks": [], + "ForwardLimit": 1 + }, "Storage": { "Provider": "local" }, diff --git a/src/Tests/Integration.Tests/Infrastructure/FshWebApplicationFactory.cs b/src/Tests/Integration.Tests/Infrastructure/FshWebApplicationFactory.cs index ab8cfe3c65..99b621799c 100644 --- a/src/Tests/Integration.Tests/Infrastructure/FshWebApplicationFactory.cs +++ b/src/Tests/Integration.Tests/Infrastructure/FshWebApplicationFactory.cs @@ -153,6 +153,22 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) builder.ConfigureServices(services => { + // Stamp the connection IP from a test header so forwarded-headers trust checks are testable. + services.AddSingleton(); + + // The production TrustedProxyOptions read happens eagerly, before the test config overlay + // applies (same quirk as storage below), so bind the trusted upstream here instead. This + // exercises the real UseForwardedHeaders trust boundary against TestConstants.TrustedProxyIp. + services.PostConfigure(forwarded => + { + forwarded.ForwardedHeaders = Microsoft.AspNetCore.HttpOverrides.ForwardedHeaders.XForwardedFor + | Microsoft.AspNetCore.HttpOverrides.ForwardedHeaders.XForwardedProto; + forwarded.ForwardLimit = 1; + forwarded.KnownProxies.Clear(); + forwarded.KnownIPNetworks.Clear(); + forwarded.KnownProxies.Add(System.Net.IPAddress.Parse(TestConstants.TrustedProxyIp)); + }); + // Remove hosted services that need unavailable infra or race migrations (RolePermissionSync, // Hangfire server + stale-lock cleanup, OutboxDispatcher); we register our own InMemory server below. var hostedServicesToRemove = services diff --git a/src/Tests/Integration.Tests/Infrastructure/TestConstants.cs b/src/Tests/Integration.Tests/Infrastructure/TestConstants.cs index d5a2a6e49c..2b49c3398a 100644 --- a/src/Tests/Integration.Tests/Infrastructure/TestConstants.cs +++ b/src/Tests/Integration.Tests/Infrastructure/TestConstants.cs @@ -6,6 +6,10 @@ public static class TestConstants public const string RootAdminEmail = "admin@root.com"; public const string DefaultPassword = "123Pa$$word!"; + // Documentation IP ranges (RFC 5737) so the trusted-proxy fixture never collides with a real host. + public const string TrustedProxyIp = "192.0.2.10"; + public const string UntrustedSourceIp = "198.51.100.9"; + public const string JwtIssuer = "fsh.local"; public const string JwtAudience = "fsh.clients"; public const string JwtSigningKey = "integration-test-signing-key-that-is-at-least-32-chars-long!!"; diff --git a/src/Tests/Integration.Tests/Infrastructure/TestRemoteIpStartupFilter.cs b/src/Tests/Integration.Tests/Infrastructure/TestRemoteIpStartupFilter.cs new file mode 100644 index 0000000000..118c96a91b --- /dev/null +++ b/src/Tests/Integration.Tests/Infrastructure/TestRemoteIpStartupFilter.cs @@ -0,0 +1,35 @@ +using System.Net; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; + +namespace Integration.Tests.Infrastructure; + +/// +/// TestServer has no real socket, so Connection.RemoteIpAddress is null and the +/// forwarded-headers trust check (known proxies/networks) can't be exercised. This filter runs +/// before the app pipeline (hence before UseForwardedHeaders) and stamps the connection IP from the +/// X-Test-Remote-Ip header so a test can present itself as a trusted or untrusted upstream. +/// Inert for requests that don't carry the header. +/// +public sealed class TestRemoteIpStartupFilter : IStartupFilter +{ + public const string RemoteIpHeader = "X-Test-Remote-Ip"; + + public Action Configure(Action next) => + app => + { + app.Use(async (context, nextMiddleware) => + { + var header = context.Request.Headers[RemoteIpHeader].FirstOrDefault(); + if (!string.IsNullOrEmpty(header) && IPAddress.TryParse(header, out var ip)) + { + context.Connection.RemoteIpAddress = ip; + } + + await nextMiddleware(); + }); + + next(app); + }; +} diff --git a/src/Tests/Integration.Tests/Tests/Security/ForwardedHeadersIpTests.cs b/src/Tests/Integration.Tests/Tests/Security/ForwardedHeadersIpTests.cs new file mode 100644 index 0000000000..01e7c8dd9c --- /dev/null +++ b/src/Tests/Integration.Tests/Tests/Security/ForwardedHeadersIpTests.cs @@ -0,0 +1,94 @@ +using Finbuckle.MultiTenant; +using Finbuckle.MultiTenant.Abstractions; +using FSH.Framework.Shared.Multitenancy; +using FSH.Modules.Identity.Data; +using Integration.Tests.Infrastructure; + +namespace Integration.Tests.Tests.Security; + +/// +/// Runtime repro for audit finding API-02 (no UseForwardedHeaders → proxy IP collapses the real +/// client IP) plus its security boundary. Token issuance persists a UserSession whose IpAddress comes +/// from RequestContextService.IpAddress => Connection.RemoteIpAddress. The forwarded-headers config +/// trusts only the configured upstream (TestConstants.TrustedProxyIp), so: +/// - a request arriving from the trusted proxy has its X-Forwarded-For honored (real client IP), and +/// - a request arriving from any other source has X-Forwarded-For ignored (spoofing is blocked). +/// TestServer has no socket, so the connection IP is stamped via the X-Test-Remote-Ip header (see +/// TestRemoteIpStartupFilter). +/// +[Collection(FshCollectionDefinition.Name)] +public sealed class ForwardedHeadersIpTests +{ + private const string ForwardedClientIp = "203.0.113.7"; + + private readonly FshWebApplicationFactory _factory; + + public ForwardedHeadersIpTests(FshWebApplicationFactory factory) + { + _factory = factory; + } + + [Fact] + public async Task TokenIssue_Should_RecordForwardedClientIp_When_RequestArrivesFromTrustedProxy() + { + var recordedIp = await IssueTokenAndReadSessionIpAsync( + connectionIp: TestConstants.TrustedProxyIp, + forwardedFor: ForwardedClientIp); + + recordedIp.ShouldBe( + ForwardedClientIp, + "behind a trusted proxy the persisted session IP should be the real client IP from " + + "X-Forwarded-For."); + } + + [Fact] + public async Task TokenIssue_Should_IgnoreForwardedClientIp_When_RequestArrivesFromUntrustedSource() + { + var recordedIp = await IssueTokenAndReadSessionIpAsync( + connectionIp: TestConstants.UntrustedSourceIp, + forwardedFor: ForwardedClientIp); + + recordedIp.ShouldBe( + TestConstants.UntrustedSourceIp, + "X-Forwarded-For from a source outside the trusted-proxy set must be ignored; the persisted " + + "IP should be the connection IP, never the attacker-supplied forwarded value."); + recordedIp.ShouldNotBe(ForwardedClientIp); + } + + private async Task IssueTokenAndReadSessionIpAsync(string connectionIp, string forwardedFor) + { + using var client = _factory.CreateClient(); + using var request = new HttpRequestMessage(HttpMethod.Post, $"{TestConstants.IdentityBasePath}/token/issue"); + request.Headers.Add("tenant", TestConstants.RootTenantId); + request.Headers.Add(TestRemoteIpStartupFilter.RemoteIpHeader, connectionIp); + request.Headers.Add("X-Forwarded-For", forwardedFor); + request.Content = JsonContent.Create(new + { + email = TestConstants.RootAdminEmail, + password = TestConstants.DefaultPassword, + }); + + using var response = await client.SendAsync(request); + response.StatusCode.ShouldBe(HttpStatusCode.OK); + + return await GetNewestSessionIpAsync(); + } + + private async Task GetNewestSessionIpAsync() + { + using var scope = _factory.Services.CreateScope(); + + var tenantStore = scope.ServiceProvider.GetRequiredService>(); + var tenant = await tenantStore.GetAsync(TestConstants.RootTenantId); + scope.ServiceProvider.GetRequiredService().MultiTenantContext = + new MultiTenantContext(tenant); + + var db = scope.ServiceProvider.GetRequiredService(); + var session = await db.UserSessions + .AsNoTracking() + .OrderByDescending(s => s.CreatedAt) + .FirstOrDefaultAsync(); + + return session?.IpAddress; + } +}