Repository navigation
Added cross-site request forgery prevention to the ASP.NET Core handler - #630
Open
xperiandri wants to merge 5 commits into
Open
xperiandri wants to merge 5 commits into
xperiandri wants to merge 5 commits into
Conversation
A browser sends a GET, HEAD or POST request to another site without a CORS preflight when it has no custom headers and its Content-Type is missing or application/x-www-form-urlencoded, multipart/form-data or text/plain. Such a request carries the cookies of the site it goes to, so a page of any other site could run an operation on behalf of a signed-in user. - Modeled on Apollo Server's csrfPrevention and enabled by default (breaking change): such a request is rejected with 400 and a GraphQL error unless it carries a non-empty GraphQL-Preflight, Apollo-Require-Preflight or X-Apollo-Operation-Name header. Requests with another Content-Type, such as application/json, and WebSocket connections are not affected. - GraphQLOptions.CsrfPrevention configures the accepted headers or, set to ValueNone, turns the check off; GraphQLRequestHandler.CheckCsrfPrevention lets handlers overriding HandleAsync run the same check. - The GraphQL client provider sends GraphQL-Preflight: 1 with every HTTP request, multipart file uploads and introspection included, unless the caller sets that header itself. - The star-wars-api sample allows the header in CORS, and the README of FSharp.Data.GraphQL.Server.AspNetCore documents the protection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The check no longer looks at the method of a request, as Apollo Server does not: a middleware overriding the method from a form field, such as UseHttpMethodOverride, turned a form posted from another site without a preflight into a PUT that skipped the check and executed a mutation. Only a CORS preflight, an OPTIONS request with Access-Control-Request-Method, which a form cannot set, always passes. - The error of a blocked request carries extensions.code BAD_REQUEST, as Apollo Server's does. - A blocked request is logged at the debug level: a page of another site decides how many such requests browsers send. - The client provider sends GraphQL-Preflight only with the requests a server's CSRF prevention would otherwise reject, GET requests and multipart uploads: a browser-hosted client would otherwise need every server's CORS policy to allow the header on its JSON requests too. - The README says that WebSocket connections are not checked, so a server authenticating them through cookies must check their Origin itself. - Fixed a doc comment mixing plain text and a tag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The client builds a multipart upload as MultipartContent of the form-data subtype, not as MultipartFormDataContent, so the header was only added for the latter and every upload was rejected by the server's CSRF prevention. The integration tests of uploads caught it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Introspection GET requests overwrite rather than preserve a caller-supplied preflight header value.
2 open findings
What changed in this PR
Adds default CSRF protection to the ASP.NET Core GraphQL handler and updates clients to satisfy it.
Changes:
- Rejects potentially unsafe requests unless preflight evidence is present.
- Adds client preflight headers and configuration options.
- Adds comprehensive unit/integration coverage and documentation.
| File | Description |
|---|---|
tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj |
Registers CSRF unit tests. |
tests/FSharp.Data.GraphQL.Tests/AspNetCore/RequestHandlerExtensibilityTests.fs |
Adapts dispatch tests for CSRF protection. |
tests/FSharp.Data.GraphQL.Tests/AspNetCore/CsrfPreventionTests.fs |
Tests handler CSRF behavior and configuration. |
tests/FSharp.Data.GraphQL.IntegrationTests/IntrospectionUpdateTests.fs |
Adds preflight headers to introspection requests. |
tests/FSharp.Data.GraphQL.IntegrationTests/FSharp.Data.GraphQL.IntegrationTests.fsproj |
Registers CSRF integration tests. |
tests/FSharp.Data.GraphQL.IntegrationTests/CsrfPreventionTests.fs |
Tests client headers and multipart handling. |
src/FSharp.Data.GraphQL.Server.AspNetCore/ServiceCollectionExtensions.fs |
Enables CSRF protection by default. |
src/FSharp.Data.GraphQL.Server.AspNetCore/README.md |
Documents protection and configuration. |
src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLRequestHandler.fs |
Implements request classification and rejection. |
src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLOptions.fs |
Defines CSRF options and headers. |
src/FSharp.Data.GraphQL.Client/GraphQLClient.fs |
Adds preflight headers to GETs and uploads. |
samples/star-wars-api/Startup.fs |
Allows the preflight header through CORS. |
RELEASE_NOTES.md |
Documents the breaking security change. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| let private getAsync ct (invoker : HttpMessageInvoker) (serverUrl : string) = task { | ||
| use requestMessage = new HttpRequestMessage (HttpMethod.Get, serverUrl) | ||
| addPreflightHeader requestMessage |
| let ctx = DefaultHttpContext (RequestServices = serviceProvider) | ||
| ctx.Request.Method <- method | ||
| ctx.Request.Path <- PathString "/graphql" | ||
| // These tests are about dispatch, so a GET must not be stopped by the CSRF prevention CsrfPreventionTests covers |
Test Results 9 files 9 suites 14m 41s ⏱️ Results for commit 6aff919. ♻️ This comment has been updated with latest results. |
The default set is created once and compares header names case-insensitively. The names in the error of a blocked request are sorted, as a hash set enumerates in no particular order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
xperiandri
commented
Oct 10, 2026
| /// An HTTP client of the integration server whose GET requests pass its CSRF prevention | ||
| let createPreflightedHttpClient () = | ||
| let httpClient = TestHosts.createIntegrationHttpClient () | ||
| httpClient.DefaultRequestHeaders.Add ("GraphQL-Preflight", "1") |
Collaborator
Author
There was a problem hiding this comment.
Why not const from our public module?
- Media types, header names and methods come from `MediaTypeNames`, `HeaderNames` and `HttpMethods` instead of being spelled out. - The header names of CSRF prevention moved to `CsrfPreventionHeaders` in Shared, so that the client that sends one, the server that checks for it and the tests share a single definition. - The media types and header names in the error of a blocked request are sorted, as both come from hash sets, which enumerate in no particular order. - The introspection GET of the client carries the headers of the caller, so that a preflight header among them is kept. - The tests assert with `Assert.Equal` and `Assert.Empty` instead of `Assert.True` on a comparison. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A browser sends some requests to another site without a CORS preflight, and with the cookies of that site. These are requests with no custom headers whose
Content-Typeis missing or one ofapplication/x-www-form-urlencoded,multipart/form-dataandtext/plain.The HTTP handler executed such requests, since it reads the JSON whatever
Content-Typea request declares. So a page of any other site could run a mutation on behalf of a user signed in to an API that authenticates through cookies.Changes
CSRF prevention (breaking: on by default). It is modeled on Apollo Server's
csrfPrevention. Such a request is rejected with 400 and a GraphQL error whoseextensions.codeisBAD_REQUEST, unless it carries a non-empty value in one of these headers:GraphQL-Preflight;Apollo-Require-PreflightorX-Apollo-Operation-Name, Apollo's names, so that clients written for Apollo Server work unchanged.A browser never sends a custom header to another site without a preflight, which the CORS policy then allows or refuses.
Any method. As in Apollo Server, the method is not checked. A middleware overriding the method from a form field, such as
UseHttpMethodOverride, would otherwise turn a form posted from another site into a PUT that skips the check. Only a CORS preflight (OPTIONSwithAccess-Control-Request-Method, which a form cannot set) always passes.Not affected:
Content-Type, such asapplication/json;Originitself.Configuration:
GraphQLOptions.CsrfPreventionaccepts other headers or, set toValueNone, turns the check off.Custom handlers:
GraphQLRequestHandler.CheckCsrfPreventionruns the check for handlers that overrideHandleAsync.Client: the client provider sends
GraphQL-Preflight: 1with its GET requests and multipart uploads, unless the caller sets it. JSON requests go without it, so that a browser-hosted client does not need every server's CORS policy to allow the header.Sample: the
star-wars-apisample allows the header in CORS.Tests
The unit tests are xUnit, in
AspNetCore/CsrfPreventionTests.fs:Content-Typespelling, blocked without the header and executed with it;Content-Type, GET, an empty header value, each default header and their case;The integration tests check:
An independent review compared the
Content-Typereading with whatwg-mimetype, Chromium and Apollo Server on 207 811 generated values: none of the 410 a browser sends without a preflight got through, and none differed from Apollo. It found the method-override bypass, now fixed and tested.Verification:
🤖 Generated with Claude Code