Make the attempt cookie's deletion Secure so a browser honours it
A __Host- cookie is only removed by a Set-Cookie that is itself Secure. The hardening test now checks Secure on every header, deletion included, and the completion tests pin that success and refusal both clear the attempt cookie with Secure, Path=/ and no Domain. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
c757acf525
commit
58fa80bc5b
3 changed files with 31 additions and 9 deletions
|
|
@ -470,8 +470,9 @@ public static class HeimdallAttemptCookie
|
|||
};
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
public static CookieOptions DeleteOptions() => new() { Path = BifrostCookies.Path };
|
||||
public static CookieOptions DeleteOptions() => new() { Path = BifrostCookies.Path, Secure = true };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -49,6 +49,29 @@ public sealed class HeimdallCompletionTests
|
|||
private static void AssertTokenRefused(HeimdallHost host, string rule) =>
|
||||
AssertLogged(host, $"Heimdall token refused: {rule}.");
|
||||
|
||||
/// <summary>A __Host- cookie is only deleted by a Set-Cookie that is itself Secure at Path=/; a browser ignores any other.</summary>
|
||||
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);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue