diff --git a/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs b/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs index 7749402..0d39c2f 100644 --- a/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs +++ b/src/Bifrost.Web/Features/Heimdall/HeimdallAuthService.cs @@ -470,8 +470,9 @@ public static class HeimdallAttemptCookie }; /// - /// The options a deletion carries: only the attempt cookie's Path, which a cookie is matched on. It carries no - /// Max-Age, which would take precedence over the expiry Delete writes and leave the emptied cookie alive. + /// The options a deletion carries: the attempt cookie's Path, which a cookie is matched on, and Secure, without + /// which a browser ignores a Set-Cookie for a __Host- name. It carries no Max-Age, which would take precedence + /// over the expiry Delete writes and leave the emptied cookie alive. /// - public static CookieOptions DeleteOptions() => new() { Path = BifrostCookies.Path }; + public static CookieOptions DeleteOptions() => new() { Path = BifrostCookies.Path, Secure = true }; } diff --git a/tests/Bifrost.Web.Tests/CookieHardeningTests.cs b/tests/Bifrost.Web.Tests/CookieHardeningTests.cs index 3a43ebc..287e8ac 100644 --- a/tests/Bifrost.Web.Tests/CookieHardeningTests.cs +++ b/tests/Bifrost.Web.Tests/CookieHardeningTests.cs @@ -84,11 +84,10 @@ public sealed class CookieHardeningTests(CookieHardeningTests.OAuthConfiguredFac Assert.DoesNotContain(attributes, x => x.StartsWith("domain", StringComparison.Ordinal)); } + // A browser ignores a __Host- Set-Cookie that lacks Secure, a deletion included. + Assert.All(headers, header => Assert.Contains("secure", header!.Split(';').Select(x => x.Trim().ToLowerInvariant()))); Assert.All(headers.Take(2), header => - { - Assert.Contains("secure", header!.Split(';').Select(x => x.Trim().ToLowerInvariant())); - Assert.Contains("httponly", header!.Split(';').Select(x => x.Trim().ToLowerInvariant())); - }); + Assert.Contains("httponly", header!.Split(';').Select(x => x.Trim().ToLowerInvariant()))); } private static void AssertHostOnly(string? name, CookieSecurePolicy secure, string? path, string? domain) diff --git a/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs b/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs index 68f0a94..532f669 100644 --- a/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs +++ b/tests/Bifrost.Web.Tests/HeimdallCompletionTests.cs @@ -49,6 +49,29 @@ public sealed class HeimdallCompletionTests private static void AssertTokenRefused(HeimdallHost host, string rule) => AssertLogged(host, $"Heimdall token refused: {rule}."); + /// A __Host- cookie is only deleted by a Set-Cookie that is itself Secure at Path=/; a browser ignores any other. + private static void AssertAttemptCookieCleared(string? header) + { + Assert.NotNull(header); + var attributes = header!.Split(';').Select(x => x.Trim().ToLowerInvariant()).ToArray(); + Assert.Contains(attributes, x => x.StartsWith("expires=", StringComparison.Ordinal) && x.Contains("1970")); + Assert.Contains("secure", attributes); + Assert.Contains("path=/", attributes); + Assert.DoesNotContain(attributes, x => x.StartsWith("domain", StringComparison.Ordinal)); + } + + [Fact] + public async Task A_refused_completion_clears_the_attempt_cookie_with_a_set_cookie_the_browser_honours() + { + using var host = new HeimdallHost(); + var attempt = await host.StartAsync(); + + var refused = await host.CompleteAsync(attempt.Nonce, "unknown", attempt.Cookie); + + AssertRefusedRedirect(refused); + AssertAttemptCookieCleared(HeimdallHost.SetCookieHeader(refused.Response, attempt.CookieName!)); + } + private static void AssertRefusedRedirect(CompletionResult result) { Assert.Equal(HttpStatusCode.Redirect, result.Response.StatusCode); @@ -81,8 +104,7 @@ public sealed class HeimdallCompletionTests Assert.Equal(MembershipStatus.Authenticated, account.Membership!.Status); Assert.Empty(account.Membership.RoleAssignments); var cleared = HeimdallHost.SetCookieHeader(done.Response, attempt.CookieName!); - Assert.NotNull(cleared); - Assert.Contains("1970", cleared); + AssertAttemptCookieCleared(cleared); var again = await host.CompleteAsync(attempt, code);