fix(security): correlate SAML responses to the request that solicited them

src/dotnet/Modgud.Api.Tests/Security/SamlReplayProtectionTests.cs

@@ -0,0 +1,187 @@
+using Marten;
+using Modgud.Api.Tests.Infrastructure;
+using Modgud.Authentication.Domain.Saml;
+using Modgud.Authentication.Identity.LoginProviders.Saml;
+
+namespace Modgud.Api.Tests.Security;
+
+/// <summary>
+/// Pins SAML request correlation and single-use (P0-2).
+///
+/// <para>
+/// The ACS validated status and signatures but never checked that the Response
+/// answered a request this SP actually sent: <c>StartLoginAsync</c> discarded the
+/// AuthnRequest ID and <c>InResponseTo</c> appeared nowhere in the SAML code. A
+/// captured, genuinely-signed Response could therefore be presented repeatedly
+/// within its <c>NotOnOrAfter</c> window — replay, login-CSRF, session swapping.
+/// </para>
+///
+/// <para>
+/// These tests drive <see cref="MartenSamlAuthnRequestStore"/> against real
+/// Postgres, which is where the guarantee lives. The full HTTP ACS path is not
+/// exercised end to end because the repo has no rig for minting signed SAML
+/// Responses; the flow's wiring of this store is covered by review, not by test.
+/// </para>
+/// </summary>
+[Collection(IntegrationTestCollection.Name)]
+public class SamlReplayProtectionTests : IntegrationTestBase
+{
+    public SamlReplayProtectionTests(SharedPostgresFixture fixture) : base(fixture) { }
+
+    private static string NewRequestId() => $"_id{Guid.NewGuid():N}";
+
+    [Fact]
+    public async Task A_response_can_answer_its_request_exactly_once()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        var requestId = NewRequestId();
+        var providerId = Guid.NewGuid();
+
+        await using (var s = GetTenantedDocumentSession())
+            await new MartenSamlAuthnRequestStore(s).RecordAsync(requestId, providerId, ct);
+
+        await using (var s = GetTenantedDocumentSession())
+        {
+            var first = await new MartenSamlAuthnRequestStore(s)
+                .TryConsumeAsync(requestId, providerId, ct);
+            Assert.Equal(SamlAuthnRequestConsumeResult.Consumed, first);
+        }
+
+        // The replay: the very same captured Response presented again.
+        await using (var s = GetTenantedDocumentSession())
+        {
+            var second = await new MartenSamlAuthnRequestStore(s)
+                .TryConsumeAsync(requestId, providerId, ct);
+            Assert.Equal(SamlAuthnRequestConsumeResult.AlreadyConsumed, second);
+        }
+    }
+
+    [Fact]
+    public async Task Concurrent_presentations_of_one_response_cannot_both_win()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        var requestId = NewRequestId();
+        var providerId = Guid.NewGuid();
+
+        await using (var s = GetTenantedDocumentSession())
+            await new MartenSamlAuthnRequestStore(s).RecordAsync(requestId, providerId, ct);
+
+        // Separate sessions, both loading before either commits — the shape a
+        // load-then-delete consume would lose, since Marten does not version-check
+        // deletes.
+        await using var sA = GetTenantedDocumentSession();
+        await using var sB = GetTenantedDocumentSession();
+        var storeA = new MartenSamlAuthnRequestStore(sA);
+        var storeB = new MartenSamlAuthnRequestStore(sB);
+
+        var results = await Task.WhenAll(
+            storeA.TryConsumeAsync(requestId, providerId, ct),
+            storeB.TryConsumeAsync(requestId, providerId, ct));
+
+        Assert.Equal(1, results.Count(r => r == SamlAuthnRequestConsumeResult.Consumed));
+        Assert.Equal(1, results.Count(r => r == SamlAuthnRequestConsumeResult.AlreadyConsumed));
+    }
+
+    [Fact]
+    public async Task An_unsolicited_response_is_refused()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        await using var s = GetTenantedDocumentSession();
+        var store = new MartenSamlAuthnRequestStore(s);
+
+        // IdP-initiated SSO: no InResponseTo at all. Only SP-initiated login
+        // exists in this codebase, so an unsolicited Response is never legitimate.
+        Assert.Equal(SamlAuthnRequestConsumeResult.Unsolicited,
+            await store.TryConsumeAsync(null, Guid.NewGuid(), ct));
+        Assert.Equal(SamlAuthnRequestConsumeResult.Unsolicited,
+            await store.TryConsumeAsync("   ", Guid.NewGuid(), ct));
+    }
+
+    [Fact]
+    public async Task A_response_referencing_an_unknown_request_is_refused()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        await using var s = GetTenantedDocumentSession();
+
+        Assert.Equal(SamlAuthnRequestConsumeResult.Unknown,
+            await new MartenSamlAuthnRequestStore(s)
+                .TryConsumeAsync(NewRequestId(), Guid.NewGuid(), ct));
+    }
+
+    [Fact]
+    public async Task A_response_cannot_be_presented_at_a_different_providers_acs()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        var requestId = NewRequestId();
+        var solicitedFrom = Guid.NewGuid();
+        var presentedAt = Guid.NewGuid();
+
+        await using (var s = GetTenantedDocumentSession())
+            await new MartenSamlAuthnRequestStore(s).RecordAsync(requestId, solicitedFrom, ct);
+
+        await using (var s = GetTenantedDocumentSession())
+        {
+            Assert.Equal(SamlAuthnRequestConsumeResult.ProviderMismatch,
+                await new MartenSamlAuthnRequestStore(s)
+                    .TryConsumeAsync(requestId, presentedAt, ct));
+        }
+
+        // And the mismatch must not have spent it for the rightful provider.
+        await using (var s = GetTenantedDocumentSession())
+        {
+            Assert.Equal(SamlAuthnRequestConsumeResult.Consumed,
+                await new MartenSamlAuthnRequestStore(s)
+                    .TryConsumeAsync(requestId, solicitedFrom, ct));
+        }
+    }
+
+    [Fact]
+    public async Task A_stale_request_is_refused()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        var requestId = NewRequestId();
+        var providerId = Guid.NewGuid();
+
+        await using (var seed = GetTenantedDocumentSession())
+        {
+            seed.Insert(new SamlPendingAuthnRequest
+            {
+                Id = requestId,
+                LoginProviderId = providerId,
+                CreatedAt = DateTimeOffset.UtcNow.AddHours(-2),
+                ExpiresAt = DateTimeOffset.UtcNow.AddHours(-1),
+            });
+            await seed.SaveChangesAsync(ct);
+        }
+
+        await using var s = GetTenantedDocumentSession();
+        Assert.Equal(SamlAuthnRequestConsumeResult.Expired,
+            await new MartenSamlAuthnRequestStore(s).TryConsumeAsync(requestId, providerId, ct));
+    }
+
+    [Fact]
+    public async Task Recording_a_request_prunes_ones_that_can_no_longer_be_answered()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        var stale = NewRequestId();
+        var providerId = Guid.NewGuid();
+
+        await using (var seed = GetTenantedDocumentSession())
+        {
+            seed.Insert(new SamlPendingAuthnRequest
+            {
+                Id = stale,
+                LoginProviderId = providerId,
+                CreatedAt = DateTimeOffset.UtcNow.AddHours(-2),
+                ExpiresAt = DateTimeOffset.UtcNow.AddHours(-1),
+            });
+            await seed.SaveChangesAsync(ct);
+        }
+
+        await using (var s = GetTenantedDocumentSession())
+            await new MartenSamlAuthnRequestStore(s).RecordAsync(NewRequestId(), providerId, ct);
+
+        await using var check = GetTenantedDocumentSession();
+        Assert.Null(await check.LoadAsync<SamlPendingAuthnRequest>(stale, ct));
+    }
+}

src/dotnet/Modgud.Authentication/Api/ExternalAuth/Saml/SamlLoginFlow.cs

@@ -33,7 +33,8 @@
     SamlContextBuilder contextBuilder,
     SamlSpCertificateService spCertService,
     ExternalLoginProcessor processor,
     SignInManager<ApplicationUser> signInManager,
+    ISamlAuthnRequestStore authnRequestStore,
     ISessionService sessionService,
     ISecurityAuditLog securityAudit,
     ILogger<SamlLoginFlow> logger)
@@ -107,6 +108,14 @@
 
         binding.Bind(authnRequest);
 
+        // Remember the request BEFORE handing the browser to the IdP. This record
+        // is what makes the Response's InResponseTo checkable at the ACS — without
+        // it, any correctly-signed Response is acceptable at any time, which is
+        // precisely the precondition for replay. Recorded after Bind so we persist
+        // the exact ID that went onto the wire.
+        await authnRequestStore.RecordAsync(
+            authnRequest.Id.Value, provider.LoginProviderId, ct);
+
         logger.LogInformation(
             "SAML AuthnRequest built for provider {Id} → IdP {IdpEntity}",
             provider.LoginProviderId, provider.IdpMetadata.EntityId);
@@ -220,6 +229,48 @@
             return Results.Redirect($"/login?error=saml-{Uri.EscapeDataString(sigError)}");
         }
 
+        // Request correlation + single use. Deliberately AFTER the signature
+        // checks: consuming first would let an unauthenticated attacker burn a
+        // victim's pending request by POSTing a garbage Response with a guessed
+        // InResponseTo, turning a replay defense into a login-denial lever. The
+        // signature is what proves the Response actually came from the IdP; only
+        // then is it worth spending the pending request on it.
+        //
+        // This is the replay defense proper: signature validation alone accepts a
+        // GENUINE, correctly-signed Response any number of times within its
+        // NotOnOrAfter window (login-CSRF, session swapping). Claiming the
+        // AuthnRequest we issued makes each Response answerable exactly once, and
+        // only at the provider it was solicited from.
+        var correlation = await authnRequestStore.TryConsumeAsync(
+            saml2Response.InResponseToAsString, provider.LoginProviderId, ct);
+        if (correlation != SamlAuthnRequestConsumeResult.Consumed)
+        {
+            var reason = correlation switch
+            {
+                SamlAuthnRequestConsumeResult.Unsolicited => "unsolicited",
+                SamlAuthnRequestConsumeResult.AlreadyConsumed => "replayed",
+                SamlAuthnRequestConsumeResult.Expired => "expired",
+                SamlAuthnRequestConsumeResult.ProviderMismatch => "provider-mismatch",
+                _ => "unknown-request",
+            };
+
+            logger.LogWarning(
+                "SAML response rejected ({Reason}) for provider {Id} — InResponseTo={InResponseTo}",
+                reason, provider.LoginProviderId, saml2Response.InResponseToAsString);
+
+            securityAudit.Record(new SecurityAuditRecord
+            {
+                EventType = AuditEvents.ExternalLoginRejected,
+                Level = "Warning",
+                Ip = ip,
+                Status = "rejected",
+                Reason = $"SAML: request correlation failed ({reason}) for provider {provider.LoginProviderId}",
+                Message = $"SAML response refused for provider {provider.Slug} — request correlation failed ({reason})",
+            });
+            ModgudMeters.RecordLogin(ModgudMeters.LoginMethod.External, ModgudMeters.LoginOutcome.Failure);
+            return Results.Redirect($"/login?error=saml-{reason}");
+        }
+
         // ITfoxtec's response builds its own ClaimsIdentity from the
         // assertion. We translate that into the OIDC-shaped principal
         // ExternalLoginProcessor expects (iss/sub + attribute claims under

src/dotnet/Modgud.Authentication/Domain/Saml/SamlPendingAuthnRequest.cs

@@ -0,0 +1,53 @@
+namespace Modgud.Authentication.Domain.Saml;
+
+/// <summary>
+/// A SAML AuthnRequest this SP issued and is still willing to accept a Response
+/// for. Persisting it is what makes <c>InResponseTo</c> checkable: without a
+/// record of the requests we sent, any correctly-signed Response is acceptable
+/// at any time, which is exactly what replay needs.
+///
+/// <para>
+/// Stored per-realm (the tenant-scoped Marten session routes it to the calling
+/// realm's database), so a Response solicited in one realm can't be presented in
+/// another. <see cref="Id"/> is the AuthnRequest's own ID — the natural key, and
+/// the value the IdP echoes back in <c>InResponseTo</c>.
+/// </para>
+///
+/// <para>
+/// The consume is a version-checked <c>Store</c> of <see cref="ConsumedAt"/>,
+/// NOT a delete: Marten does not version-check deletes, so two concurrent
+/// redemptions of one captured Response would both pass a load-then-delete
+/// check. Keeping the consumed row until it expires also gives the ACS a
+/// truthful "already used" signal instead of an indistinguishable "unknown id".
+/// </para>
+/// </summary>
+public sealed class SamlPendingAuthnRequest
+{
+    /// <summary>
+    /// How long we accept a Response for a request we sent. Generous enough for a
+    /// slow interactive IdP login (MFA prompts, password resets at the IdP), short
+    /// enough to bound the window in which a captured Response is worth anything.
+    /// </summary>
+    public const int ExpirationMinutes = 15;
+
+    /// <summary>The AuthnRequest ID we generated — echoed back as <c>InResponseTo</c>.</summary>
+    public string Id { get; set; } = default!;
+
+    /// <summary>
+    /// The provider this request went to. Checked at the ACS so a Response
+    /// solicited from one IdP cannot be presented at another provider's ACS
+    /// within the same realm.
+    /// </summary>
+    public Guid LoginProviderId { get; set; }
+
+    public DateTimeOffset CreatedAt { get; set; }
+
+    public DateTimeOffset ExpiresAt { get; set; }
+
+    /// <summary>Set when a Response for this request was accepted. Single-use marker.</summary>
+    public DateTimeOffset? ConsumedAt { get; set; }
+
+    public bool IsConsumed => ConsumedAt.HasValue;
+
+    public bool IsExpired => DateTimeOffset.UtcNow > ExpiresAt;
+}