From f9d0c24f6660c7cfe7a0a1d5a2c4d3eb90b841e8 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Thu, 12 Nov 2026 16:26:25 +0000 Subject: [PATCH 1/2] Take the link's cookie slot from the started attempt; drop unused IsSignInProvider The await step passed replaceSlot to StartAsync but named the slot from its own cookieName, so replaceSlot decided nothing. The step now returns the slot the started attempt chose, one owner. Co-Authored-By: Claude Sonnet 5.5 --- src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs b/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs index b24b4ab..8ab2add 100644 --- a/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs +++ b/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs @@ -41,9 +41,6 @@ public sealed class HeimdallAuthService( "twitch" }; - /// Whether a provider may sign a person in (and so be asked for at /auth/heimdall/{provider}). Google is identity only: it can be linked, never signed in with. - public static bool IsSignInProvider(string provider) => AllowedProviders.Contains(provider); - /// The headers HttpClient logging must never write: the app secret and any bearer. public static bool RedactsHeader(string header) => string.Equals(header, AppSecretHeader, StringComparison.OrdinalIgnoreCase) @@ -343,7 +340,7 @@ public sealed class HeimdallAuthService( attempt.ExpectedAccountId, pendingLinkId, replaceSlot: cookieName); - return new HeimdallLinkStep(next.AuthorizationUrl, cookieName, next.CookieValue, null); + return new HeimdallLinkStep(next.AuthorizationUrl, next.CookieName, next.CookieValue, null); } case HeimdallAttemptPurpose.LinkConfirm: -- 2.47.3 From 1298e7db1e9b89dc38373c4813ac6dfc7cf21071 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Thu, 12 Nov 2026 16:26:25 +0000 Subject: [PATCH 2/2] Pin the Google link guards: own slot beside a parallel attempt, undefined purpose, empty pending link id, non-linked 201 Co-Authored-By: Claude Sonnet 5.5 --- .../HeimdallGoogleLinkTests.cs | 111 ++++++++++++++++++ .../Support/HeimdallTestKit.cs | 16 ++- 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/tests/Bifrost.Web.Tests/HeimdallGoogleLinkTests.cs b/tests/Bifrost.Web.Tests/HeimdallGoogleLinkTests.cs index deaf751..5de3eec 100644 --- a/tests/Bifrost.Web.Tests/HeimdallGoogleLinkTests.cs +++ b/tests/Bifrost.Web.Tests/HeimdallGoogleLinkTests.cs @@ -490,4 +490,115 @@ public sealed class HeimdallGoogleLinkTests Assert.Equal(Status + "?link=linked", Location(d.Response)); Assert.Equal(Status + "?link=expired", Location(replayed.Response)); } + + // ---- 10. pins for guards the first pass left unproven ---- + + [Fact] + public async Task A_link_continues_in_its_own_slot_and_leaves_a_parallel_sign_in_attempt_alive() + { + using var host = new HeimdallHost(realSessions: true); + var account = HeimdallShapes.NewAccountId(); + var session = await host.SignInSessionAsync(account); + var parallel = await host.StartAsync(); + var parallelCookie = $"{parallel.CookieName}={parallel.Cookie}"; + var start = await host.StartLinkAsync(session, extraCookies: parallelCookie); + // The parallel attempt holds slot 0, so the link lives in the other slot. + Assert.Equal(HeimdallAttemptCookie.SlotNames[0], parallel.CookieName); + Assert.Equal(HeimdallAttemptCookie.SlotNames[1], start.CookieName); + + var b = await StepB(host, session, start, account); + var awaiting = new Step(b.Response, b.Next, b.Token); + var pending = host.Heimdall.RegisterPendingLink(); + var c = await StepC(host, session, awaiting.Next, pending); + var d = await StepD(host, session, c.Next, account); + + // Every continuation wrote the link's own slot and none touched the parallel attempt's slot. + foreach (var response in new[] { b.Response, c.Response }) + { + Assert.NotNull(HeimdallHost.SetCookieHeader(response, start.CookieName!)); + Assert.Null(HeimdallHost.SetCookieHeader(response, parallel.CookieName!)); + } + + Assert.Equal(start.CookieName, b.Next.CookieName); + Assert.Equal(start.CookieName, c.Next.CookieName); + Assert.Equal(Status + "?link=linked", Location(d.Response)); + Assert.Null(HeimdallHost.SetCookieHeader(d.Response, parallel.CookieName!)); + + var signIn = await host.CompleteWithHeaderAsync( + parallel.Nonce, host.CodeFor(parallel, HeimdallShapes.NewAccountId()), parallelCookie); + Assert.True(signIn.SignedIn); + } + + [Theory] + [InlineData("9")] + [InlineData("-1")] + [InlineData("4")] + public async Task An_attempt_cookie_with_an_undefined_purpose_is_no_attempt_and_asks_Heimdall_for_nothing(string purpose) + { + using var host = new HeimdallHost(realSessions: true); + var account = HeimdallShapes.NewAccountId(); + var session = await host.SignInSessionAsync(account); + var nonce = "crafted-" + Guid.NewGuid().ToString("N"); + var protector = host.Services.GetRequiredService().CreateProtector("Bifrost.Heimdall.Attempt.v1"); + var crafted = protector.Protect( + $$"""{"Nonce":"{{nonce}}","Provider":"discord","RequiresMemberAccess":false,"ReturnPath":"/App/Mill","ExpiresAt":"{{host.Clock.Now.AddMinutes(5):O}}","Purpose":{{purpose}},"ExpectedAccountId":"{{account}}","PendingLinkId":"p"}"""); + var attempt = new StartedAttempt( + nonce, HeimdallAttemptCookie.SlotNames[0], crafted, $"https://localhost/auth/heimdall/complete?attempt={nonce}", null!); + var starts = host.Heimdall.StartBodies.Count; + var redeems = host.Heimdall.RedeemCalls; + + var response = await host.PostCompleteAsync( + new Dictionary + { + ["attempt"] = nonce, ["code"] = host.CodeFor(attempt, account), ["pendingLinkId"] = "p" + }, + Jar(session, attempt)); + + Assert.Equal(ErrorRedirect, Location(response)); + Assert.Equal(starts, host.Heimdall.StartBodies.Count); + Assert.Equal(redeems, host.Heimdall.RedeemCalls); + Assert.Empty(host.Heimdall.ConfirmBodies); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + public async Task An_await_step_without_a_pending_link_id_is_refused_before_Heimdall_hears_of_it(string pendingLinkId) + { + using var host = new HeimdallHost(realSessions: true); + var account = HeimdallShapes.NewAccountId(); + var session = await host.SignInSessionAsync(account); + var start = await host.StartLinkAsync(session); + var b = await StepB(host, session, start, account); + var starts = host.Heimdall.StartBodies.Count; + var redeems = host.Heimdall.RedeemCalls; + + var c = await StepC(host, session, b.Next, pendingLinkId); + + Assert.Equal(Status + "?link=error", Location(c.Response)); + Assert.Equal(starts, host.Heimdall.StartBodies.Count); + Assert.Equal(redeems, host.Heimdall.RedeemCalls); + Assert.Empty(host.Heimdall.ConfirmBodies); + AssertAttemptCleared(c.Response, b.Next); + } + + [Theory] + [InlineData("")] + [InlineData("{}")] + [InlineData("""{"status":"pending"}""")] + [InlineData("""{"status":"Linked"}""")] + [InlineData("not json")] + public async Task A_201_confirm_that_does_not_say_linked_is_not_shown_as_linked(string body) + { + using var host = new HeimdallHost(realSessions: true); + var journey = await ThroughC(host); + host.Heimdall.ConfirmStatus = HttpStatusCode.Created; + host.Heimdall.ConfirmBody = body; + + var d = await StepD(host, journey.Session, journey.C.Next, journey.Account); + + Assert.Equal(Status + "?link=error", Location(d.Response)); + Assert.Single(host.Heimdall.ConfirmBodies); + AssertAttemptCleared(d.Response, journey.C.Next); + } } diff --git a/tests/Bifrost.Web.Tests/Support/HeimdallTestKit.cs b/tests/Bifrost.Web.Tests/Support/HeimdallTestKit.cs index a051db0..a074ede 100644 --- a/tests/Bifrost.Web.Tests/Support/HeimdallTestKit.cs +++ b/tests/Bifrost.Web.Tests/Support/HeimdallTestKit.cs @@ -215,6 +215,9 @@ public sealed class FakeHeimdall : HttpMessageHandler /// When set, every links/confirm answers this status with Heimdall's error code for it. public HttpStatusCode? ConfirmStatus { get; set; } + /// When set with , the body of that answer verbatim (empty string included). + public string? ConfirmBody { get; set; } + private readonly HashSet _pendingLinks = new(StringComparer.Ordinal); /// Registers a pending link Heimdall will confirm once for any bearer. @@ -330,6 +333,11 @@ public sealed class FakeHeimdall : HttpMessageHandler if (ConfirmStatus is { } forced) { + if (ConfirmBody is not null) + { + return Json(forced, ConfirmBody); + } + return forced switch { HttpStatusCode.Conflict => Json(forced, """{"error":"identity_linked_elsewhere"}"""), @@ -621,7 +629,8 @@ public sealed class HeimdallHost : TestWebApplicationFactory /// The account page's link button as a browser posts it: the page is fetched for its antiforgery token and cookie /// (when ), then the form is posted with the session. Without a session nothing is fetched. /// - public async Task StartLinkAsync(string? session, bool antiforgery = true, HttpMethod? method = null) + public async Task StartLinkAsync( + string? session, bool antiforgery = true, HttpMethod? method = null, string? extraCookies = null) { using var client = NewClient(); var cookies = new List(); @@ -643,6 +652,11 @@ public sealed class HeimdallHost : TestWebApplicationFactory cookies.Add($"{BifrostCookies.Antiforgery}={CookieValue(page, BifrostCookies.Antiforgery)}"); } + if (extraCookies is not null) + { + cookies.Add(extraCookies); + } + var before = Heimdall.StartBodies.Count; using var request = new HttpRequestMessage(method ?? HttpMethod.Post, LinkPostUrl); if (request.Method == HttpMethod.Post) -- 2.47.3