diff --git a/src/Bifrost.Web/Features/ConceptVotes/CastRequestGuard.cs b/src/Bifrost.Web/Features/ConceptVotes/CastRequestGuard.cs new file mode 100644 index 0000000..6af4a6b --- /dev/null +++ b/src/Bifrost.Web/Features/ConceptVotes/CastRequestGuard.cs @@ -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); + } +} diff --git a/src/Bifrost.Web/Pages/Mill/Vote.cshtml b/src/Bifrost.Web/Pages/Mill/Vote.cshtml index daa82b1..fe75338 100644 --- a/src/Bifrost.Web/Pages/Mill/Vote.cshtml +++ b/src/Bifrost.Web/Pages/Mill/Vote.cshtml @@ -28,15 +28,15 @@

-
+
-
+
@if (stance is not null) { -
+
} diff --git a/src/Bifrost.Web/Pages/Mill/Vote.cshtml.cs b/src/Bifrost.Web/Pages/Mill/Vote.cshtml.cs index 2182f65..b25eec9 100644 --- a/src/Bifrost.Web/Pages/Mill/Vote.cshtml.cs +++ b/src/Bifrost.Web/Pages/Mill/Vote.cshtml.cs @@ -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 options) : PageModel + IOptions options, + IOptions 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 OnGetAsync(string collection, string key, CancellationToken cancellationToken) { var actor = await actorAccessor.GetAsync(cancellationToken); diff --git a/tests/Bifrost.Web.Tests/ConceptVotePageTests.cs b/tests/Bifrost.Web.Tests/ConceptVotePageTests.cs index 2e081c8..bf719b7 100644 --- a/tests/Bifrost.Web.Tests/ConceptVotePageTests.cs +++ b/tests/Bifrost.Web.Tests/ConceptVotePageTests.cs @@ -118,21 +118,144 @@ public sealed partial class ConceptVotePageTests : IClassFixture 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(.*?)", 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 Jar { get; } = new(StringComparer.Ordinal); @@ -447,54 +559,36 @@ public sealed partial class ConceptVotePageTests : IClassFixture 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 Post( + // A cookieless POST as a browser form would send it: Origin the site's own by default, null to omit it. + public Task 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>(); - 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 Send(HttpRequestMessage request) + public async Task Send(HttpRequestMessage request) { if (anonymous) {