diff --git a/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs b/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs index 05e1ccf..dc5e302 100644 --- a/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs +++ b/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs @@ -109,6 +109,53 @@ public sealed class HeimdallAuthService( new Uri(payload.AuthorizationUrl, UriKind.Absolute), HeimdallAttemptCookie.NameFor(nonce)!, Protect(attempt)); } + /// + /// The attempt cookies Start must delete so the Cookie header the browser sends to /auth/heimdall stays under + /// the proxy's header limit: every request cookie that is no live attempt, then the live attempts that expire + /// first until the new attempt and the survivors fit . + /// The new attempt is always kept. + /// + public IReadOnlyList SupersededAttemptCookies( + IEnumerable> requestCookies, + HeimdallAuthStartResult start) + { + var used = HeimdallAttemptCookie.EntrySize(start.CookieName, start.CookieValue); + var superseded = new List(); + var live = new List<(string Name, int Size, DateTimeOffset ExpiresAt)>(); + foreach (var (name, value) in requestCookies) + { + if (!name.StartsWith(HeimdallAttemptCookie.Prefix, StringComparison.Ordinal) || + string.Equals(name, start.CookieName, StringComparison.Ordinal)) + { + continue; + } + + var attempt = Unprotect(value, name[HeimdallAttemptCookie.Prefix.Length..]); + if (attempt is null) + { + superseded.Add(name); + } + else + { + live.Add((name, HeimdallAttemptCookie.EntrySize(name, value), attempt.ExpiresAt)); + } + } + + foreach (var cookie in live.OrderByDescending(x => x.ExpiresAt)) + { + if (used + cookie.Size <= HeimdallAttemptCookie.MaxAggregateBytes) + { + used += cookie.Size; + } + else + { + superseded.Add(cookie.Name); + } + } + + return superseded; + } + public async Task CompleteAsync( string callbackBaseUrl, string? nonce, @@ -405,6 +452,15 @@ public static class HeimdallAttemptCookie public static readonly TimeSpan Lifetime = TimeSpan.FromMinutes(10); + /// + /// The most bytes of attempt cookies a browser may hold at once. One worst-case cookie is about 2.4 KB, and + /// the Cookie header must stay under nginx's 8 KB with room for the session and antiforgery cookies. + /// + public const int MaxAggregateBytes = 5120; + + /// A cookie's bytes in a Cookie header: name, '=', value and the '; ' separator. + public static int EntrySize(string name, string value) => name.Length + 1 + value.Length + 2; + public static string? NameFor(string? nonce) => nonce is { Length: NonceLength } && nonce.All(c => char.IsAsciiLetterOrDigit(c) || c is '-' or '_') ? Prefix + nonce diff --git a/src/Bifrost.Web/Program.cs b/src/Bifrost.Web/Program.cs index 1c96ea0..5c1e2fa 100644 --- a/src/Bifrost.Web/Program.cs +++ b/src/Bifrost.Web/Program.cs @@ -380,6 +380,11 @@ app.MapGet("/auth/heimdall/{provider}", async ( requireMemberAccess, cancellationToken); + foreach (var superseded in heimdallAuthService.SupersededAttemptCookies(httpContext.Request.Cookies, start)) + { + httpContext.Response.Cookies.Delete(superseded, HeimdallAttemptCookie.DeleteOptions()); + } + httpContext.Response.Cookies.Append(start.CookieName, start.CookieValue, HeimdallAttemptCookie.Options()); return Results.Redirect(start.AuthorizationUrl.ToString()); } diff --git a/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs b/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs index 2e9ed0c..647852b 100644 --- a/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs +++ b/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs @@ -1,5 +1,6 @@ using System.Net; using System.Text; +using System.Text.RegularExpressions; using System.Text.Json; using Bifrost.Web.Domain; using Bifrost.Web.Features.Heimdall; @@ -1234,6 +1235,131 @@ public sealed class HeimdallCompletionTests Assert.InRange(header.Split(';')[0].Length, 1, 3072); } + /// The browser's cookie jar for /auth/heimdall: Set-Cookie headers applied, a deletion removing the cookie. + private static void ApplySetCookies(Dictionary jar, HttpResponseMessage response) + { + if (!response.Headers.TryGetValues("Set-Cookie", out var headers)) + { + return; + } + + foreach (var header in headers) + { + var parts = header.Split(';').Select(x => x.Trim()).ToArray(); + var pair = parts[0].Split('=', 2); + if (parts.Any(x => x.StartsWith("expires=", StringComparison.OrdinalIgnoreCase) && x.Contains("1970"))) + { + jar.Remove(pair[0]); + } + else + { + jar[pair[0]] = pair[1]; + } + } + } + + private static string CookieHeader(Dictionary jar) => + string.Join("; ", jar.Select(x => x.Key + "=" + x.Value)); + + /// The deletions a response carries; each must match the cookie by Path and carry no Max-Age. + private static int Deletions(HttpResponseMessage response) + { + var deletions = response.Headers.TryGetValues("Set-Cookie", out var headers) + ? headers.Where(x => x.Contains("1970")).ToArray() + : []; + foreach (var header in deletions) + { + var attributes = header.Split(';').Skip(1).Select(x => x.Trim().ToLowerInvariant()).ToArray(); + Assert.Contains("path=/auth/heimdall", attributes); + Assert.DoesNotContain(attributes, x => x.StartsWith("max-age", StringComparison.Ordinal)); + } + + return deletions.Length; + } + + [Fact] + public async Task Attempts_opened_past_the_cookie_bound_keep_the_header_small_and_the_newest_completes() + { + using var host = new HeimdallHost(); + var worstCase = "/" + new string('<', LocalReturnPath.MaxLength - 1); + var jar = new Dictionary(); + var started = new List(); + var deletions = 0; + + for (var i = 0; i < 12; i++) + { + host.Clock.Advance(TimeSpan.FromSeconds(1)); + var attempt = await host.StartAsync(returnUrl: worstCase, cookieHeader: jar.Count == 0 ? null : CookieHeader(jar)); + Assert.NotNull(attempt.Cookie); + deletions += Deletions(attempt.Response); + ApplySetCookies(jar, attempt.Response); + started.Add(attempt); + Assert.True(CookieHeader(jar).Length <= 6144, $"Cookie header is {CookieHeader(jar).Length} bytes after start {i}"); + Assert.Contains(HeimdallAttemptCookie.Prefix + attempt.Nonce, jar.Keys); + } + + Assert.True(deletions > 0, "the bound was never reached"); + Assert.True(jar.Count < started.Count); + + // The oldest attempts were the ones given up: the newest completes, the first has no cookie left. + Assert.True((await Genuine(host, started[^1], HeimdallShapes.NewAccountId())).SignedIn); + Assert.DoesNotContain(HeimdallAttemptCookie.Prefix + started[0].Nonce, jar.Keys); + Assert.Contains(HeimdallAttemptCookie.Prefix + started[^2].Nonce, jar.Keys); + } + + [Fact] + public async Task Small_attempts_all_survive_under_the_cookie_bound_and_dead_cookies_are_swept() + { + using var host = new HeimdallHost(); + var jar = new Dictionary(); + var started = new List(); + for (var i = 0; i < 5; i++) + { + host.Clock.Advance(TimeSpan.FromSeconds(1)); + var attempt = await host.StartAsync(returnUrl: "/App", cookieHeader: jar.Count == 0 ? null : CookieHeader(jar)); + Assert.Equal(0, Deletions(attempt.Response)); + ApplySetCookies(jar, attempt.Response); + started.Add(attempt); + } + + Assert.Equal(5, jar.Count); + foreach (var attempt in started) + { + Assert.True((await Genuine(host, attempt, HeimdallShapes.NewAccountId())).SignedIn); + } + + var deadName = HeimdallAttemptCookie.Prefix + new string('A', HeimdallAttemptCookie.NonceLength); + var next = await host.StartAsync(cookieHeader: deadName + "=not-protected"); + Assert.Contains("1970", HeimdallHost.SetCookieHeader(next.Response, deadName)); + } + + [Fact] + public async Task The_complete_page_form_has_the_fields_and_action_the_server_reads() + { + using var host = new HeimdallHost(); + using var client = host.NewClient(); + var page = await client.GetStringAsync("/auth/heimdall/complete"); + + var form = Regex.Match(page, @"]*)>(.*?)", RegexOptions.Singleline); + Assert.True(form.Success); + Assert.Matches(@"\bmethod=""post""", form.Groups[1].Value); + var action = Regex.Match(form.Groups[1].Value, @"\baction=""([^""]*)""").Groups[1].Value; + Assert.Equal("/auth/heimdall/complete", action); + var inputs = Regex.Matches(form.Groups[2].Value, @"]*)>"); + Assert.All(inputs, x => Assert.Contains("type=\"hidden\"", x.Groups[1].Value)); + var names = inputs.Select(x => Regex.Match(x.Groups[1].Value, @"\bname=""([^""]*)""").Groups[1].Value).ToArray(); + Assert.Equal(new[] { "attempt", "code" }, names); + + // The page's own field names and action, filled as its script fills them, complete a sign-in. + var attempt = await host.StartAsync(); + var code = host.CodeFor(attempt, HeimdallShapes.NewAccountId()); + var fields = new Dictionary { [names[0]] = attempt.Nonce, [names[1]] = code }; + using var post = new HttpRequestMessage(HttpMethod.Post, action) { Content = new FormUrlEncodedContent(fields) }; + post.Headers.Add("Cookie", $"{HeimdallAttemptCookie.Prefix + attempt.Nonce}={attempt.Cookie}"); + using var response = await client.SendAsync(post); + Assert.False(string.IsNullOrEmpty(HeimdallHost.CookieValue(response, ".AspNetCore.Cookies"))); + } + [Fact] public async Task A_blank_app_slug_is_refused_before_Heimdall_is_called() { diff --git a/tests/Bifrost.Web.Tests/Support/HeimdallTestKit.cs b/tests/Bifrost.Web.Tests/Support/HeimdallTestKit.cs index 40e024b..e0719e9 100644 --- a/tests/Bifrost.Web.Tests/Support/HeimdallTestKit.cs +++ b/tests/Bifrost.Web.Tests/Support/HeimdallTestKit.cs @@ -418,15 +418,23 @@ public sealed class HeimdallHost : TestWebApplicationFactory Action>? mutate = null, HeimdallKey? signer = null) => (signer ?? Key).Sign(HeimdallShapes.AccessClaims(accountId, Clock.Now, capabilities, displayName, mutate)); - public Task StartAsync(bool member = false, string provider = "discord", string returnUrl = "/App/Mill") => + public Task StartAsync( + bool member = false, string provider = "discord", string returnUrl = "/App/Mill", string? cookieHeader = null) => StartAtAsync( - $"/auth/heimdall/{provider}?returnUrl={Uri.EscapeDataString(returnUrl)}" + (member ? "&access=member" : string.Empty)); + $"/auth/heimdall/{provider}?returnUrl={Uri.EscapeDataString(returnUrl)}" + (member ? "&access=member" : string.Empty), + cookieHeader); - public async Task StartAtAsync(string url) + public async Task StartAtAsync(string url, string? cookieHeader = null) { using var client = NewClient(); var before = Heimdall.StartBodies.Count; - var response = await client.GetAsync(url); + using var request = new HttpRequestMessage(HttpMethod.Get, url); + if (cookieHeader is not null) + { + request.Headers.Add("Cookie", cookieHeader); + } + + var response = await client.SendAsync(request); if (Heimdall.StartBodies.Count == before) { return new StartedAttempt(string.Empty, null, string.Empty, response);