Vote page: cookieless CSRF (Origin / Sec-Fetch-Site) and a defined error for handler-less requests
Drops antiforgery from the anonymous vote flow: no antiforgery cookie, no hidden token. CastRequestGuard is the one decider; VoteModel.OnPageHandlerSelected is the one gate and answers 4xx when no handler is selected (was a NullReferenceException 500). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
0c6f9216b6
commit
e0b3b1a38a
4 changed files with 207 additions and 60 deletions
29
src/Bifrost.Web/Features/ConceptVotes/CastRequestGuard.cs
Normal file
29
src/Bifrost.Web/Features/ConceptVotes/CastRequestGuard.cs
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
using Bifrost.Web.Configuration;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
|
||||
namespace Bifrost.Web.Features.ConceptVotes;
|
||||
|
||||
// The one place that decides whether a state-changing request to the vote page came from the site itself. The anonymous
|
||||
// vote flow carries no antiforgery token and no antiforgery cookie, so a cross-site form post is refused here instead
|
||||
// (OWASP Fetch Metadata): the Origin header must equal the configured site origin byte for byte, or, when the browser
|
||||
// sent no Origin, Sec-Fetch-Site must be exactly same-origin. Same-site, cross-site, none, absent and a repeated header
|
||||
// are all refused. Header values are compared, never echoed.
|
||||
public static class CastRequestGuard
|
||||
{
|
||||
public static bool Allows(HttpRequest request, BifrostHostOptions host)
|
||||
{
|
||||
if (!Uri.TryCreate(host.PublicBaseUrl, UriKind.Absolute, out var site))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
var origin = request.Headers.Origin;
|
||||
if (origin.Count > 0)
|
||||
{
|
||||
return origin.Count == 1 && string.Equals(origin[0], site.GetLeftPart(UriPartial.Authority), StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
var fetchSite = request.Headers["Sec-Fetch-Site"];
|
||||
return fetchSite.Count == 1 && string.Equals(fetchSite[0], "same-origin", StringComparison.Ordinal);
|
||||
}
|
||||
}
|
||||
|
|
@ -28,15 +28,15 @@
|
|||
</p>
|
||||
|
||||
<div class="mill-actions">
|
||||
<form method="post" asp-page-handler="Up" hx-boost="false">
|
||||
<form method="post" asp-page-handler="Up" asp-antiforgery="false" hx-boost="false">
|
||||
<button class="button" type="submit">Up</button>
|
||||
</form>
|
||||
<form method="post" asp-page-handler="Down" hx-boost="false">
|
||||
<form method="post" asp-page-handler="Down" asp-antiforgery="false" hx-boost="false">
|
||||
<button class="button" type="submit">Down</button>
|
||||
</form>
|
||||
@if (stance is not null)
|
||||
{
|
||||
<form method="post" asp-page-handler="Retract" hx-boost="false">
|
||||
<form method="post" asp-page-handler="Retract" asp-antiforgery="false" hx-boost="false">
|
||||
<button class="button ghost" type="submit">Retract</button>
|
||||
</form>
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ using Bifrost.Web.Domain;
|
|||
using Bifrost.Web.Features.ConceptVotes;
|
||||
using Bifrost.Web.Features.Shared;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
using Microsoft.AspNetCore.Mvc.Filters;
|
||||
using Microsoft.AspNetCore.Mvc.RazorPages;
|
||||
using Microsoft.Extensions.Options;
|
||||
|
||||
|
|
@ -10,11 +11,13 @@ namespace Bifrost.Web.Pages.Mill;
|
|||
|
||||
// The anonymous vote page. Counts and the visitor's own stance come from ConceptAppraisalService on every GET; the
|
||||
// voter key is decided by VoterIdentity.For from the actor and the device id VoterKeyResolver reads.
|
||||
[IgnoreAntiforgeryToken]
|
||||
public sealed class VoteModel(
|
||||
ICurrentBifrostActorAccessor actorAccessor,
|
||||
ConceptAppraisalService appraisals,
|
||||
VoterKeyResolver deviceCookie,
|
||||
IOptions<ConceptVoteOptions> options) : PageModel
|
||||
IOptions<ConceptVoteOptions> options,
|
||||
IOptions<BifrostHostOptions> host) : PageModel
|
||||
{
|
||||
public ConceptSubjectView Subject { get; private set; } = null!;
|
||||
|
||||
|
|
@ -22,6 +25,27 @@ public sealed class VoteModel(
|
|||
|
||||
public string MillPageLink => options.Value.MillPageUrl + "#" + Uri.EscapeDataString(Subject.Key);
|
||||
|
||||
// The single gate before any handler runs. A request that selects no handler (a POST with no or an unknown handler,
|
||||
// any other method) is a client error, never a view over a null subject. A cast is accepted only from the site's own
|
||||
// origin (CastRequestGuard); no antiforgery cookie or token exists on this flow.
|
||||
public override void OnPageHandlerSelected(PageHandlerSelectedContext context)
|
||||
{
|
||||
var method = context.HttpContext.Request.Method;
|
||||
if (context.HandlerMethod is null)
|
||||
{
|
||||
context.Result = new StatusCodeResult(
|
||||
HttpMethods.IsGet(method) || HttpMethods.IsHead(method) || HttpMethods.IsPost(method)
|
||||
? StatusCodes.Status400BadRequest
|
||||
: StatusCodes.Status405MethodNotAllowed);
|
||||
return;
|
||||
}
|
||||
|
||||
if (HttpMethods.IsPost(method) && !CastRequestGuard.Allows(context.HttpContext.Request, host.Value))
|
||||
{
|
||||
context.Result = new StatusCodeResult(StatusCodes.Status403Forbidden);
|
||||
}
|
||||
}
|
||||
|
||||
public async Task<IActionResult> OnGetAsync(string collection, string key, CancellationToken cancellationToken)
|
||||
{
|
||||
var actor = await actorAccessor.GetAsync(cancellationToken);
|
||||
|
|
|
|||
|
|
@ -118,21 +118,144 @@ public sealed partial class ConceptVotePageTests : IClassFixture<TestWebApplicat
|
|||
Assert.Null(SetCookieHeader(page, CookieName));
|
||||
}
|
||||
|
||||
private const string SiteOrigin = "https://localhost";
|
||||
|
||||
[Fact]
|
||||
public async Task Cast_without_antiforgery_token_is_refused()
|
||||
public async Task Cast_from_the_sites_own_origin_is_accepted_and_writes_one_row()
|
||||
{
|
||||
var collection = NewCollection();
|
||||
await AddSubject(collection, "alpha");
|
||||
var visitor = NewVisitor(anonymous: true);
|
||||
|
||||
using var noToken = await visitor.Post(collection, "alpha", "Up", token: Token.Omitted);
|
||||
using var noCookie = await visitor.Post(collection, "alpha", "Up", token: Token.WithoutAntiforgeryCookie);
|
||||
using var garbage = await visitor.Post(collection, "alpha", "Up", token: Token.Garbage);
|
||||
using var byOrigin = await visitor.Post(collection, "alpha", "Up", origin: SiteOrigin);
|
||||
Assert.Equal(HttpStatusCode.Redirect, byOrigin.StatusCode);
|
||||
Assert.Single(await Rows(collection, "alpha"));
|
||||
|
||||
using var byFetchSite = await visitor.Post(collection, "alpha", "Down", origin: null, fetchSite: "same-origin");
|
||||
Assert.Equal(HttpStatusCode.Redirect, byFetchSite.StatusCode);
|
||||
Assert.Equal(ConceptStance.Down, Assert.Single(await Rows(collection, "alpha")).Stance);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("https://localhost.evil.example")]
|
||||
[InlineData("https://evil-localhost")]
|
||||
[InlineData("https://sub.localhost")]
|
||||
[InlineData("https://LOCALHOST")]
|
||||
[InlineData("HTTPS://localhost")]
|
||||
[InlineData("https://localhost:8443")]
|
||||
[InlineData("http://localhost")]
|
||||
[InlineData("null")]
|
||||
[InlineData("")]
|
||||
[InlineData("https://localhost/")]
|
||||
[InlineData("https://evil.example")]
|
||||
public async Task Cast_from_any_other_origin_is_refused_even_when_fetch_site_says_same_origin(string origin)
|
||||
{
|
||||
var collection = NewCollection();
|
||||
await AddSubject(collection, "alpha");
|
||||
var visitor = NewVisitor(anonymous: true);
|
||||
|
||||
foreach (var handler in new[] { "Up", "Down", "Retract" })
|
||||
{
|
||||
using var response = await visitor.Post(collection, "alpha", handler, origin: origin, fetchSite: "same-origin");
|
||||
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
|
||||
Assert.Null(SetCookieHeader(response, CookieName));
|
||||
Assert.Empty(response.Headers.Where(h => h.Key == "Set-Cookie"));
|
||||
Assert.Equal(string.Empty, await response.Content.ReadAsStringAsync());
|
||||
}
|
||||
|
||||
Assert.Empty(await Rows(collection, "alpha"));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("same-origin", true)]
|
||||
[InlineData("same-site", false)]
|
||||
[InlineData("cross-site", false)]
|
||||
[InlineData("none", false)]
|
||||
[InlineData("Same-Origin", false)]
|
||||
[InlineData("same-origin, cross-site", false)]
|
||||
[InlineData(null, false)]
|
||||
public async Task Cast_without_an_origin_header_needs_fetch_site_same_origin(string? fetchSite, bool accepted)
|
||||
{
|
||||
var collection = NewCollection();
|
||||
await AddSubject(collection, "alpha");
|
||||
var visitor = NewVisitor(anonymous: true);
|
||||
|
||||
using var response = await visitor.Post(collection, "alpha", "Up", origin: null, fetchSite: fetchSite);
|
||||
|
||||
Assert.Equal(accepted ? HttpStatusCode.Redirect : HttpStatusCode.Forbidden, response.StatusCode);
|
||||
Assert.Equal(accepted ? 1 : 0, (await Rows(collection, "alpha")).Count);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Refusal_never_echoes_the_origin_or_fetch_site_values()
|
||||
{
|
||||
const string canary = "https://CANARY-origin-5e1c.example";
|
||||
var logs = new CapturingLoggerProvider();
|
||||
var factory = _factory.WithWebHostBuilder(b => b.ConfigureServices(s => s.AddLogging(l => l.AddProvider(logs))));
|
||||
var collection = NewCollection();
|
||||
await AddSubject(collection, "alpha");
|
||||
var visitor = NewVisitor(anonymous: true, factory);
|
||||
|
||||
using var byOrigin = await visitor.Post(collection, "alpha", "Up", origin: canary);
|
||||
using var byFetchSite = await visitor.Post(collection, "alpha", "Up", origin: null, fetchSite: "CANARY-site-5e1c");
|
||||
|
||||
foreach (var response in new[] { byOrigin, byFetchSite })
|
||||
{
|
||||
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
|
||||
var seen = await response.Content.ReadAsStringAsync()
|
||||
+ string.Join("\n", response.Headers.Select(h => h.Key + ":" + string.Join(",", h.Value)));
|
||||
Assert.DoesNotContain("CANARY", seen, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
Assert.DoesNotContain(logs.Lines, line => line.Contains("CANARY", StringComparison.OrdinalIgnoreCase));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task A_page_view_sets_no_cookie_at_all_and_a_cast_sets_only_the_device_cookie()
|
||||
{
|
||||
var collection = NewCollection();
|
||||
await AddSubject(collection, "alpha");
|
||||
var visitor = NewVisitor(anonymous: true);
|
||||
|
||||
using (var view = await visitor.Get(collection, "alpha"))
|
||||
{
|
||||
Assert.Equal(HttpStatusCode.OK, view.StatusCode);
|
||||
Assert.False(view.Headers.Contains("Set-Cookie"));
|
||||
var html = await view.Content.ReadAsStringAsync();
|
||||
Assert.DoesNotContain("__RequestVerificationToken", html, StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
using var cast = await visitor.Post(collection, "alpha", "Up");
|
||||
var cookies = cast.Headers.GetValues("Set-Cookie").ToArray();
|
||||
Assert.Single(cookies);
|
||||
Assert.StartsWith(CookieName + "=", cookies[0], StringComparison.Ordinal);
|
||||
|
||||
using var later = await visitor.Get(collection, "alpha");
|
||||
Assert.False(later.Headers.Contains("Set-Cookie"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task A_request_that_selects_no_handler_is_a_client_error_and_never_a_500()
|
||||
{
|
||||
var collection = NewCollection();
|
||||
await AddSubject(collection, "alpha");
|
||||
var visitor = NewVisitor(anonymous: true);
|
||||
|
||||
using var noHandler = await visitor.Post(collection, "alpha", handler: null);
|
||||
using var bogus = await visitor.Post(collection, "alpha", "CANARYQ9");
|
||||
using var put = await visitor.Post(collection, "alpha", "Up", method: HttpMethod.Put);
|
||||
using var delete = await visitor.Post(collection, "alpha", null, method: HttpMethod.Delete);
|
||||
using var getBogus = await visitor.Send(new HttpRequestMessage(HttpMethod.Get, $"/mill/{collection}/alpha?handler=CANARYQ9"));
|
||||
using var getUp = await visitor.Send(new HttpRequestMessage(HttpMethod.Get, $"/mill/{collection}/alpha?handler=Up"));
|
||||
|
||||
foreach (var response in new[] { noHandler, bogus, put, delete, getBogus, getUp })
|
||||
{
|
||||
var status = (int)response.StatusCode;
|
||||
Assert.InRange(status, 400, 499);
|
||||
Assert.DoesNotContain("CANARYQ9", await response.Content.ReadAsStringAsync(), StringComparison.Ordinal);
|
||||
Assert.False(response.Headers.Contains("Set-Cookie"));
|
||||
}
|
||||
|
||||
Assert.Equal(HttpStatusCode.BadRequest, noToken.StatusCode);
|
||||
Assert.Equal(HttpStatusCode.BadRequest, noCookie.StatusCode);
|
||||
Assert.Equal(HttpStatusCode.BadRequest, garbage.StatusCode);
|
||||
Assert.Null(SetCookieHeader(noToken, CookieName));
|
||||
Assert.Empty(await Rows(collection, "alpha"));
|
||||
}
|
||||
|
||||
|
|
@ -228,7 +351,7 @@ public sealed partial class ConceptVotePageTests : IClassFixture<TestWebApplicat
|
|||
Assert.Equal(HttpStatusCode.NotFound, get.StatusCode);
|
||||
foreach (var handler in new[] { "Up", "Down", "Retract" })
|
||||
{
|
||||
using var post = await visitor.Post(c, k, handler, tokenFrom: (collection, "alpha"));
|
||||
using var post = await visitor.Post(c, k, handler);
|
||||
Assert.Equal(HttpStatusCode.NotFound, post.StatusCode);
|
||||
Assert.Null(SetCookieHeader(post, CookieName));
|
||||
}
|
||||
|
|
@ -371,9 +494,6 @@ public sealed partial class ConceptVotePageTests : IClassFixture<TestWebApplicat
|
|||
[GeneratedRegex("<dd>(.*?)</dd>", RegexOptions.Singleline)]
|
||||
private static partial Regex DefinitionCell();
|
||||
|
||||
[GeneratedRegex("name=\"__RequestVerificationToken\"[^>]*value=\"([^\"]+)\"")]
|
||||
private static partial Regex RequestToken();
|
||||
|
||||
private async Task AddSubject(string collection, string key)
|
||||
{
|
||||
using var scope = _factory.Services.CreateScope();
|
||||
|
|
@ -432,14 +552,6 @@ public sealed partial class ConceptVotePageTests : IClassFixture<TestWebApplicat
|
|||
.ToListAsync();
|
||||
}
|
||||
|
||||
public enum Token
|
||||
{
|
||||
Valid,
|
||||
Omitted,
|
||||
WithoutAntiforgeryCookie,
|
||||
Garbage
|
||||
}
|
||||
|
||||
private sealed class Visitor(HttpClient client, bool anonymous)
|
||||
{
|
||||
public Dictionary<string, string> Jar { get; } = new(StringComparer.Ordinal);
|
||||
|
|
@ -447,54 +559,36 @@ public sealed partial class ConceptVotePageTests : IClassFixture<TestWebApplicat
|
|||
public Task<HttpResponseMessage> Get(string collection, string key) =>
|
||||
Send(new HttpRequestMessage(HttpMethod.Get, $"/mill/{collection}/{key}"));
|
||||
|
||||
// Reads a token from the page of tokenFrom (default: the page being posted to), then posts the handler.
|
||||
public async Task<HttpResponseMessage> Post(
|
||||
// A cookieless POST as a browser form would send it: Origin the site's own by default, null to omit it.
|
||||
public Task<HttpResponseMessage> Post(
|
||||
string collection,
|
||||
string key,
|
||||
string handler,
|
||||
Token token = Token.Valid,
|
||||
(string Collection, string Key)? tokenFrom = null)
|
||||
string? handler,
|
||||
string? origin = SiteOrigin,
|
||||
string? fetchSite = null,
|
||||
HttpMethod? method = null)
|
||||
{
|
||||
var from = tokenFrom ?? (collection, key);
|
||||
string value;
|
||||
using (var page = await Get(from.Collection, from.Key))
|
||||
var request = new HttpRequestMessage(
|
||||
method ?? HttpMethod.Post,
|
||||
$"/mill/{collection}/{key}" + (handler is null ? string.Empty : "?handler=" + handler))
|
||||
{
|
||||
var match = RequestToken().Match(await page.Content.ReadAsStringAsync());
|
||||
Assert.True(match.Success);
|
||||
value = match.Groups[1].Value;
|
||||
}
|
||||
|
||||
var antiforgery = Jar
|
||||
.Where(x => x.Key.StartsWith(".AspNetCore.Antiforgery", StringComparison.Ordinal))
|
||||
.ToList();
|
||||
if (token is Token.WithoutAntiforgeryCookie)
|
||||
{
|
||||
foreach (var cookie in antiforgery)
|
||||
{
|
||||
Jar.Remove(cookie.Key);
|
||||
}
|
||||
}
|
||||
|
||||
var form = new List<KeyValuePair<string, string>>();
|
||||
if (token is not Token.Omitted)
|
||||
{
|
||||
form.Add(new("__RequestVerificationToken", token is Token.Garbage ? "garbage" : value));
|
||||
}
|
||||
|
||||
var request = new HttpRequestMessage(HttpMethod.Post, $"/mill/{collection}/{key}?handler={handler}")
|
||||
{
|
||||
Content = new FormUrlEncodedContent(form)
|
||||
Content = new FormUrlEncodedContent([])
|
||||
};
|
||||
var response = await Send(request);
|
||||
foreach (var cookie in antiforgery)
|
||||
if (origin is not null)
|
||||
{
|
||||
Jar.TryAdd(cookie.Key, cookie.Value);
|
||||
request.Headers.TryAddWithoutValidation("Origin", origin);
|
||||
}
|
||||
|
||||
return response;
|
||||
if (fetchSite is not null)
|
||||
{
|
||||
request.Headers.TryAddWithoutValidation("Sec-Fetch-Site", fetchSite);
|
||||
}
|
||||
|
||||
return Send(request);
|
||||
}
|
||||
}
|
||||
|
||||
private async Task<HttpResponseMessage> Send(HttpRequestMessage request)
|
||||
public async Task<HttpResponseMessage> Send(HttpRequestMessage request)
|
||||
{
|
||||
if (anonymous)
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue