Skip to content
Open
63 changes: 63 additions & 0 deletions src/BuildingBlocks/Web/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,18 @@
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;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using Microsoft.Extensions.Hosting;
using Mediator;
using System.Net;

namespace FSH.Framework.Web;

Expand Down Expand Up @@ -63,6 +66,61 @@ 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<TrustedProxyOptions>() ?? new TrustedProxyOptions();
builder.Services.Configure<ForwardedHeadersOptions>(forwarded =>
{
// A hop count below 1 is never what an operator means, and neither bad value announces itself:
// 0 truncates the unwind loop to zero iterations, so forwarded headers stop being processed with
// no error, while a negative value overflows the middleware's buffer allocation and 500s every
// request - including requests carrying no forwarded headers at all. Fail the boot instead.
if (trustedProxy.ForwardLimit < 1)
{
throw new InvalidOperationException(
$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.ForwardLimit)} is {trustedProxy.ForwardLimit}, which is not a valid proxy hop count: it must be at least 1 (one hop per proxy in front of the app).");
}

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)
{
if (!IPAddress.TryParse(proxy, out var address))
{
throw new InvalidOperationException(
$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.KnownProxies)} contains \"{proxy}\", which is not a valid IP address (for example \"10.0.0.5\").");
}

forwarded.KnownProxies.Add(address);
}

foreach (var network in trustedProxy.KnownNetworks)
{
if (!System.Net.IPNetwork.TryParse(network, out var parsedNetwork))
{
throw new InvalidOperationException(
$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.KnownNetworks)} contains \"{network}\", which is not a valid CIDR network (for example \"10.0.0.0/8\").");
}

forwarded.KnownIPNetworks.Add(parsedNetwork);
}
});

builder.Services.AddHeroDatabaseOptions(builder.Configuration);
builder.Services.AddHeroRateLimiting(builder.Configuration);

Expand Down Expand Up @@ -150,6 +208,11 @@ public static WebApplication UseHeroPlatform(this WebApplication app, Action<Fsh
var openApiEnabled = options.UseOpenApi && IsOpenApiEnabled(app.Configuration);

app.UseExceptionHandler();

// Apply forwarded headers before anything reads the client IP or scheme (HTTPS redirect,
// rate limiting, auth, audit) so they all see the real client, not the reverse proxy.
app.UseForwardedHeaders();

app.UseResponseCompression();

// CORS MUST run before UseHttpsRedirection: preflight OPTIONS can't follow an HTTP→HTTPS redirect, so
Expand Down
34 changes: 34 additions & 0 deletions src/BuildingBlocks/Web/TrustedProxy/TrustedProxyOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
namespace FSH.Framework.Web.TrustedProxy;

/// <summary>
/// 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.
/// <para>
/// Only X-Forwarded-For and X-Forwarded-Proto are honoured. X-Forwarded-Host is deliberately left
/// out: rewriting Request.Host from a header is a host-header injection primitive, and the endpoints
/// that build a public URL from the request (user registration and confirmation e-mails) would then
/// send links pointing wherever the header said. The trade-off is that Request.Host keeps the
/// internal host behind a proxy, and those links carry it.
/// </para>
/// </summary>
public sealed class TrustedProxyOptions
{
/// <summary>Individual upstream proxy IP addresses whose X-Forwarded-* headers are trusted.</summary>
public string[] KnownProxies { get; init; } = [];

/// <summary>Trusted upstream networks in CIDR notation (e.g. "10.0.0.0/8", "172.16.0.0/12").</summary>
public string[] KnownNetworks { get; init; } = [];

/// <summary>
/// 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.
/// Must be at least 1: anything lower is rejected at startup, since 0 would silently stop
/// forwarded-header processing and a negative value would fail every request.
/// </summary>
public int ForwardLimit { get; init; } = 1;
}
7 changes: 7 additions & 0 deletions src/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -143,5 +143,12 @@
AccessViolation). Transitive pinning is enabled, so this entry alone bumps it.
Remove once the SignalR backplane package depends on a patched version itself. -->
<PackageVersion Include="MessagePack" Version="2.5.301" />
<!-- Pulled transitively by the Testcontainers packages; versions up to 2025.1.0 fail
NuGet audit (NU1903, GHSA-q939-rpr3-3284 / CVE-2026-48798: ScpClient recursive
download writes outside the target directory), which breaks restore for the whole
solution under TreatWarningsAsErrors. Testcontainers 4.11.0 and 4.13.0 both depend
on 2025.1.0, so bumping Testcontainers does not help; 2026.0.0 is the first patched
release. Remove once Testcontainers depends on a patched version itself. -->
<PackageVersion Include="SSH.NET" Version="2026.0.0" />
</ItemGroup>
</Project>
5 changes: 5 additions & 0 deletions src/Host/FSH.Starter.Api/appsettings.Production.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
5 changes: 5 additions & 0 deletions src/Host/FSH.Starter.Api/appsettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,11 @@
"QueueLimit": 0
}
},
"TrustedProxyOptions": {
"KnownProxies": [],
"KnownNetworks": [],
"ForwardLimit": 1
},
"Storage": {
"Provider": "local"
},
Expand Down
113 changes: 113 additions & 0 deletions src/Tests/Framework.Tests/Web/TrustedProxyOptionsBindingTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
using FSH.Framework.Web;
using FSH.Framework.Web.TrustedProxy;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Options;

namespace Framework.Tests.Web;

/// <summary>
/// Pins the TrustedProxyOptions -> ForwardedHeadersOptions binding that AddHeroPlatform registers: which
/// upstreams end up trusted, that an unconfigured section keeps the framework's loopback-only default, and
/// that a malformed entry surfaces a message naming the offending setting rather than a bare FormatException.
/// </summary>
public sealed class TrustedProxyOptionsBindingTests
{
private const string ProxyIp = "192.0.2.10";

private static ForwardedHeadersOptions Resolve(Dictionary<string, string?> settings)
{
// DisableDefaults keeps the host's environment-variable and appsettings providers out, so an ambient
// TrustedProxyOptions__* on the machine or CI runner can't change what "nothing configured" resolves to.
var builder = Host.CreateApplicationBuilder(new HostApplicationBuilderSettings
{
DisableDefaults = true,
});
builder.Configuration.AddInMemoryCollection(settings);
builder.AddHeroPlatform();

using var provider = builder.Services.BuildServiceProvider();
return provider.GetRequiredService<IOptions<ForwardedHeadersOptions>>().Value;
}

#region Trust boundary

[Fact]
public void ForwardedHeaders_Should_KeepFrameworkLoopbackDefault_When_NothingConfigured()
{
// Act
var options = Resolve([]);

// Assert - clearing the framework default here would make every caller a trusted proxy.
options.KnownProxies.ShouldNotBeEmpty();
options.KnownIPNetworks.ShouldNotBeEmpty();
}

[Fact]
public void ForwardedHeaders_Should_TrustOnlyConfiguredProxy_When_KnownProxiesSet()
{
// Act
var options = Resolve(new Dictionary<string, string?>
{
[$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.KnownProxies)}:0"] = ProxyIp,
[$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.ForwardLimit)}"] = "2",
});

// Assert
options.KnownProxies.ShouldBe([System.Net.IPAddress.Parse(ProxyIp)]);
options.KnownIPNetworks.ShouldBeEmpty();
options.ForwardLimit.ShouldBe(2);
}

#endregion

#region Malformed configuration

[Fact]
public void ForwardedHeaders_Should_NameTheSetting_When_KnownProxyMalformed()
{
// Act
var exception = Should.Throw<InvalidOperationException>(() => Resolve(new Dictionary<string, string?>
{
[$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.KnownProxies)}:0"] = "not-an-ip",
}));

// Assert
exception.Message.ShouldContain("TrustedProxyOptions:KnownProxies");
exception.Message.ShouldContain("not-an-ip");
}

[Fact]
public void ForwardedHeaders_Should_NameTheSetting_When_KnownNetworkMalformed()
{
// Act
var exception = Should.Throw<InvalidOperationException>(() => Resolve(new Dictionary<string, string?>
{
[$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.KnownNetworks)}:0"] = "10.0.0.0/999",
}));

// Assert
exception.Message.ShouldContain("TrustedProxyOptions:KnownNetworks");
exception.Message.ShouldContain("10.0.0.0/999");
}

[Theory]
[InlineData("-1")] // negative — overflows the middleware's buffer allocation, 500s every request
[InlineData("0")] // zero — truncates the unwind loop, forwarded headers silently stop being read
public void ForwardedHeaders_Should_NameTheSetting_When_ForwardLimitBelowOne(string forwardLimit)
{
// Act - no proxies or networks configured, so this has to be rejected before the trust-boundary block.
var exception = Should.Throw<InvalidOperationException>(() => Resolve(new Dictionary<string, string?>
{
[$"{nameof(TrustedProxyOptions)}:{nameof(TrustedProxyOptions.ForwardLimit)}"] = forwardLimit,
}));

// Assert
exception.Message.ShouldContain("TrustedProxyOptions:ForwardLimit");
exception.Message.ShouldContain($"is {forwardLimit}");
}

#endregion
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<IStartupFilter, TestRemoteIpStartupFilter>();

// 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<Microsoft.AspNetCore.Builder.ForwardedHeadersOptions>(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
Expand Down
4 changes: 4 additions & 0 deletions src/Tests/Integration.Tests/Infrastructure/TestConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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!!";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
using System.Net;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;

namespace Integration.Tests.Infrastructure;

/// <summary>
/// TestServer has no real socket, so <c>Connection.RemoteIpAddress</c> 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
/// <c>X-Test-Remote-Ip</c> header so a test can present itself as a trusted or untrusted upstream.
/// Inert for requests that don't carry the header.
/// </summary>
public sealed class TestRemoteIpStartupFilter : IStartupFilter
{
public const string RemoteIpHeader = "X-Test-Remote-Ip";

public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> 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);
};
}
Loading
Loading