Add CSRF protection (#1138)

graphql-dotnet · Aug 3, 2024 · 23b5829 · 23b5829
1 parent c421048
commit 23b5829
Show file tree

Hide file tree

Showing 21 changed files with 288 additions and 29 deletions.
diff --git a/README.md b/README.md
@@ -569,6 +569,31 @@ app.UseEndpoints(endpoints =>
 await app.RunAsync();
 ```
 
+In order to ensure that all requests trigger CORS preflight requests, by default the server
+will reject requests that do not meet one of the following criteria:
+
+- The request is a POST request that includes a Content-Type header that is not
+  `application/x-www-form-urlencoded`, `multipart/form-data`, or `text/plain`.
+- The request includes a non-empty `GraphQL-Require-Preflight` header.
+
+To disable this behavior, set the `CsrfProtectionEnabled` option to `false` in the `GraphQLServerOptions`.
+
+```csharp
+app.UseGraphQL("/graphql", config =>
+{
+    config.CsrfProtectionEnabled = false;
+});
+```
+
+You may also change the allowed headers by modifying the `CsrfProtectionHeaders` option.
+
+```csharp
+app.UseGraphQL("/graphql", config =>
+{
+    config.CsrfProtectionHeaders = ["MyCustomHeader"];
+});
+```
+
 ### Response compression
 
 ASP.NET Core supports response compression independently of GraphQL, with brotli and gzip
@@ -660,6 +685,8 @@ methods allowing for different options for each configured endpoint.
 | `AuthorizationRequired`            | Requires `HttpContext.User` to represent an authenticated user. | False |
 | `AuthorizedPolicy`                 | If set, requires `HttpContext.User` to pass authorization of the specified policy. | |
 | `AuthorizedRoles`                  | If set, requires `HttpContext.User` to be a member of any one of a list of roles. | |
+| `CsrfProtectionEnabled`            | Enables cross-site request forgery (CSRF) protection for both GET and POST requests. | True |
+| `CsrfProtectionHeaders`            | Sets the headers used for CSRF protection when necessary. | `GraphQL-Require-Preflight` |
 | `DefaultResponseContentType`       | Sets the default response content type used within responses. | `application/graphql-response+json; charset=utf-8` |
 | `EnableBatchedRequests`            | Enables handling of batched GraphQL requests for POST requests when formatted as JSON. | True |
 | `ExecuteBatchedRequestsInParallel` | Enables parallel execution of batched GraphQL requests. | True |

diff --git a/docs/migration/migration8.md b/docs/migration/migration8.md
@@ -4,13 +4,20 @@
 
 - When using `FormFileGraphType` with type-first schemas, you may specify the allowed media
   types for the file by using the new `[MediaType]` attribute on the argument or input object field.
+- Cross-site request forgery (CSRF) protection has been added for both GET and POST requests,
+  enabled by default.
 
 ## Breaking changes
 
 - The validation rules' signatures have changed slightly due to the underlying changes to the
   GraphQL.NET library.  Please see the GraphQL.NET v8 migration document for more information.
 - The obsolete (v6 and prior) authorization validation rule has been removed.  See the v7 migration
   document for more information on how to migrate to the v7/v8 authorization validation rule.
+- Cross-site request forgery (CSRF) protection has been enabled for all requests by default.
+  This will require that the `GraphQL-Require-Preflight` header be sent with all GET requests and
+  all form-POST requests.  To disable this feature, set the `CsrfProtectionEnabled` property on the
+  `GraphQLMiddlewareOptions` class to `false`.  You may also configure the headers list by modifying
+  the `CsrfProtectionHeaders` property on the same class.  See the readme for more details.
 
 ## Other changes
 

diff --git a/src/Transports.AspNetCore/Errors/CsrfProtectionError.cs b/src/Transports.AspNetCore/Errors/CsrfProtectionError.cs
@@ -0,0 +1,16 @@
+namespace GraphQL.Server.Transports.AspNetCore.Errors;
+
+/// <summary>
+/// Represents an error indicating that the request may not have triggered a CORS preflight request.
+/// </summary>
+public class CsrfProtectionError : RequestError
+{
+    /// <inheritdoc cref="CsrfProtectionError"/>
+    public CsrfProtectionError(IEnumerable<string> headersRequired) : base($"This request requires a non-empty header from the following list: {FormatHeaders(headersRequired)}.") { }
+
+    /// <inheritdoc cref="CsrfProtectionError"/>
+    public CsrfProtectionError(IEnumerable<string> headersRequired, Exception innerException) : base($"This request requires a non-empty header from the following list: {FormatHeaders(headersRequired)}. {innerException.Message}") { }
+
+    private static string FormatHeaders(IEnumerable<string> headersRequired)
+        => string.Join(", ", headersRequired.Select(x => $"'{x}'"));
+}
diff --git a/src/Transports.AspNetCore/GraphQLHttpMiddleware.cs b/src/Transports.AspNetCore/GraphQLHttpMiddleware.cs
@@ -142,6 +142,10 @@ protected virtual async Task InvokeAsync(HttpContext context, RequestDelegate ne
             return;
         }
 
+        // Perform CSRF protection if necessary
+        if (await HandleCsrfProtectionAsync(context, next))
+            return;
+
         // Authenticate request if necessary
         if (await HandleAuthorizeAsync(context, next))
             return;
@@ -484,7 +488,36 @@ static void ApplyFileToRequest(IFormFile file, string target, GraphQLRequest? re
     }
 
     /// <summary>
-    /// Perform authentication, if required, and return <see langword="true"/> if the
+    /// Performs CSRF protection, if required, and returns <see langword="true"/> if the
+    /// request was handled (typically by returning an error message).  If <see langword="false"/>
+    /// is returned, the request is processed normally.
+    /// </summary>
+    protected virtual async ValueTask<bool> HandleCsrfProtectionAsync(HttpContext context, RequestDelegate next)
+    {
+        if (!_options.CsrfProtectionEnabled)
+            return false;
+        if (context.Request.Headers.TryGetValue("Content-Type", out var contentTypes) && contentTypes.Count > 0 && contentTypes[0] != null)
+        {
+            var contentType = contentTypes[0]!;
+            if (contentType.IndexOf(';') > 0)
+            {
+                contentType = contentType.Substring(0, contentType.IndexOf(';'));
+            }
+            contentType = contentType.Trim().ToLowerInvariant();
+            if (!(contentType == "text/plain" || contentType == "application/x-www-form-urlencoded" || contentType == "multipart/form-data"))
+                return false;
+        }
+        foreach (var header in _options.CsrfProtectionHeaders)
+        {
+            if (context.Request.Headers.TryGetValue(header, out var values) && values.Count > 0 && values[0]?.Length > 0)
+                return false;
+        }
+        await HandleCsrfProtectionErrorAsync(context, next);
+        return true;
+    }
+
+    /// <summary>
+    /// Perform authentication, if required, and returns <see langword="true"/> if the
     /// request was handled (typically by returning an error message).  If <see langword="false"/>
     /// is returned, the request is processed normally.
     /// </summary>
@@ -1034,21 +1067,29 @@ protected virtual Task HandleNotAuthorizedPolicyAsync(HttpContext context, Reque
     /// </summary>
     protected virtual async ValueTask<bool> HandleDeserializationErrorAsync(HttpContext context, RequestDelegate next, Exception exception)
     {
-        await WriteErrorResponseAsync(context, HttpStatusCode.BadRequest, new JsonInvalidError(exception));
+        await WriteErrorResponseAsync(context, new JsonInvalidError(exception));
         return true;
     }
 
+    /// <summary>
+    /// Writes a '.' message to the output.
+    /// </summary>
+    protected virtual async Task HandleCsrfProtectionErrorAsync(HttpContext context, RequestDelegate next)
+    {
+        await WriteErrorResponseAsync(context, new CsrfProtectionError(_options.CsrfProtectionHeaders));
+    }
+
     /// <summary>
     /// Writes a '400 Batched requests are not supported.' message to the output.
     /// </summary>
     protected virtual Task HandleBatchedRequestsNotSupportedAsync(HttpContext context, RequestDelegate next)
-        => WriteErrorResponseAsync(context, HttpStatusCode.BadRequest, new BatchedRequestsNotSupportedError());
+        => WriteErrorResponseAsync(context, new BatchedRequestsNotSupportedError());
 
     /// <summary>
     /// Writes a '400 Invalid requested WebSocket sub-protocol(s).' message to the output.
     /// </summary>
     protected virtual Task HandleWebSocketSubProtocolNotSupportedAsync(HttpContext context, RequestDelegate next)
-        => WriteErrorResponseAsync(context, HttpStatusCode.BadRequest, new WebSocketSubProtocolNotSupportedError(context.WebSockets.WebSocketRequestedProtocols));
+        => WriteErrorResponseAsync(context, new WebSocketSubProtocolNotSupportedError(context.WebSockets.WebSocketRequestedProtocols));
 
     /// <summary>
     /// Writes a '415 Invalid Content-Type header: could not be parsed.' message to the output.
@@ -1079,6 +1120,12 @@ protected virtual Task HandleInvalidHttpMethodErrorAsync(HttpContext context, Re
         return next(context);
     }
 
+    /// <summary>
+    /// Writes the specified error as a JSON-formatted GraphQL response.
+    /// </summary>
+    protected virtual Task WriteErrorResponseAsync(HttpContext context, ExecutionError executionError)
+        => WriteErrorResponseAsync(context, executionError is IHasPreferredStatusCode withCode ? withCode.PreferredStatusCode : HttpStatusCode.BadRequest, executionError);
+
     /// <summary>
     /// Writes the specified error message as a JSON-formatted GraphQL response, with the specified HTTP status code.
     /// </summary>

diff --git a/src/Transports.AspNetCore/GraphQLHttpMiddlewareOptions.cs b/src/Transports.AspNetCore/GraphQLHttpMiddlewareOptions.cs
@@ -70,6 +70,22 @@ public class GraphQLHttpMiddlewareOptions : IAuthorizationOptions
     /// </remarks>
     public bool ReadFormOnPost { get; set; } = true; // TODO: change to false for v9
 
+    /// <summary>
+    /// Enables cross-site request forgery (CSRF) protection for both GET and POST requests.
+    /// Requires a non-empty header from the <see cref="CsrfProtectionHeaders"/> list to be
+    /// present, or a POST request with a Content-Type header that is not <c>text/plain</c>,
+    /// <c>application/x-www-form-urlencoded</c>, or <c>multipart/form-data</c>.
+    /// </summary>
+    public bool CsrfProtectionEnabled { get; set; } = true;
+
+    /// <summary>
+    /// When <see cref="CsrfProtectionEnabled"/> is enabled, requests require a non-empty
+    /// header from this list or a POST request with a Content-Type header that is not
+    /// <c>text/plain</c>, <c>application/x-www-form-urlencoded</c>, or <c>multipart/form-data</c>.
+    /// Defaults to <c>GraphQL-Require-Preflight</c>.
+    /// </summary>
+    public List<string> CsrfProtectionHeaders { get; set; } = ["GraphQL-Require-Preflight"]; // see https://github.com/graphql/graphql-over-http/pull/303
+
     /// <summary>
     /// Enables reading variables from the query string.
     /// Variables are interpreted as JSON and deserialized before being

diff --git a/tests/ApiApprovalTests/net50+net60+net80/GraphQL.Server.Transports.AspNetCore.approved.txt b/tests/ApiApprovalTests/net50+net60+net80/GraphQL.Server.Transports.AspNetCore.approved.txt
@@ -99,6 +99,8 @@ namespace GraphQL.Server.Transports.AspNetCore
         protected virtual System.Threading.Tasks.Task HandleBatchRequestAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next, System.Collections.Generic.IList<GraphQL.Transport.GraphQLRequest?> gqlRequests) { }
         protected virtual System.Threading.Tasks.Task HandleBatchedRequestsNotSupportedAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
         protected virtual System.Threading.Tasks.Task HandleContentTypeCouldNotBeParsedErrorAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
+        protected virtual System.Threading.Tasks.ValueTask<bool> HandleCsrfProtectionAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
+        protected virtual System.Threading.Tasks.Task HandleCsrfProtectionErrorAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
         protected virtual System.Threading.Tasks.ValueTask<bool> HandleDeserializationErrorAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next, System.Exception exception) { }
         protected virtual System.Threading.Tasks.Task HandleInvalidContentTypeErrorAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
         protected virtual System.Threading.Tasks.Task HandleInvalidHttpMethodErrorAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
@@ -115,6 +117,7 @@ namespace GraphQL.Server.Transports.AspNetCore
                 "BatchRequest"})]
         protected virtual System.Threading.Tasks.Task<System.ValueTuple<GraphQL.Transport.GraphQLRequest?, System.Collections.Generic.IList<GraphQL.Transport.GraphQLRequest?>?>?> ReadPostContentAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next, string? mediaType, System.Text.Encoding? sourceEncoding) { }
         protected virtual string SelectResponseContentType(Microsoft.AspNetCore.Http.HttpContext context) { }
+        protected virtual System.Threading.Tasks.Task WriteErrorResponseAsync(Microsoft.AspNetCore.Http.HttpContext context, GraphQL.ExecutionError executionError) { }
         protected virtual System.Threading.Tasks.Task WriteErrorResponseAsync(Microsoft.AspNetCore.Http.HttpContext context, System.Net.HttpStatusCode httpStatusCode, GraphQL.ExecutionError executionError) { }
         protected virtual System.Threading.Tasks.Task WriteErrorResponseAsync(Microsoft.AspNetCore.Http.HttpContext context, System.Net.HttpStatusCode httpStatusCode, string errorMessage) { }
         protected virtual System.Threading.Tasks.Task WriteJsonResponseAsync<TResult>(Microsoft.AspNetCore.Http.HttpContext context, System.Net.HttpStatusCode httpStatusCode, TResult result) { }
@@ -126,6 +129,8 @@ namespace GraphQL.Server.Transports.AspNetCore
         public bool AuthorizationRequired { get; set; }
         public string? AuthorizedPolicy { get; set; }
         public System.Collections.Generic.List<string> AuthorizedRoles { get; set; }
+        public bool CsrfProtectionEnabled { get; set; }
+        public System.Collections.Generic.List<string> CsrfProtectionHeaders { get; set; }
         public Microsoft.Net.Http.Headers.MediaTypeHeaderValue DefaultResponseContentType { get; set; }
         public bool EnableBatchedRequests { get; set; }
         public bool ExecuteBatchedRequestsInParallel { get; set; }
@@ -199,6 +204,11 @@ namespace GraphQL.Server.Transports.AspNetCore.Errors
     {
         public BatchedRequestsNotSupportedError() { }
     }
+    public class CsrfProtectionError : GraphQL.Execution.RequestError
+    {
+        public CsrfProtectionError(System.Collections.Generic.IEnumerable<string> headersRequired) { }
+        public CsrfProtectionError(System.Collections.Generic.IEnumerable<string> headersRequired, System.Exception innerException) { }
+    }
     public class FileCountExceededError : GraphQL.Execution.RequestError, GraphQL.Server.Transports.AspNetCore.Errors.IHasPreferredStatusCode
     {
         public FileCountExceededError() { }

diff --git a/...pprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt b/...pprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt
@@ -106,6 +106,8 @@ namespace GraphQL.Server.Transports.AspNetCore
         protected virtual System.Threading.Tasks.Task HandleBatchRequestAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next, System.Collections.Generic.IList<GraphQL.Transport.GraphQLRequest?> gqlRequests) { }
         protected virtual System.Threading.Tasks.Task HandleBatchedRequestsNotSupportedAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
         protected virtual System.Threading.Tasks.Task HandleContentTypeCouldNotBeParsedErrorAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
+        protected virtual System.Threading.Tasks.ValueTask<bool> HandleCsrfProtectionAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
+        protected virtual System.Threading.Tasks.Task HandleCsrfProtectionErrorAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
         protected virtual System.Threading.Tasks.ValueTask<bool> HandleDeserializationErrorAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next, System.Exception exception) { }
         protected virtual System.Threading.Tasks.Task HandleInvalidContentTypeErrorAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
         protected virtual System.Threading.Tasks.Task HandleInvalidHttpMethodErrorAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next) { }
@@ -122,6 +124,7 @@ namespace GraphQL.Server.Transports.AspNetCore
                 "BatchRequest"})]
         protected virtual System.Threading.Tasks.Task<System.ValueTuple<GraphQL.Transport.GraphQLRequest?, System.Collections.Generic.IList<GraphQL.Transport.GraphQLRequest?>?>?> ReadPostContentAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.RequestDelegate next, string? mediaType, System.Text.Encoding? sourceEncoding) { }
         protected virtual string SelectResponseContentType(Microsoft.AspNetCore.Http.HttpContext context) { }
+        protected virtual System.Threading.Tasks.Task WriteErrorResponseAsync(Microsoft.AspNetCore.Http.HttpContext context, GraphQL.ExecutionError executionError) { }
         protected virtual System.Threading.Tasks.Task WriteErrorResponseAsync(Microsoft.AspNetCore.Http.HttpContext context, System.Net.HttpStatusCode httpStatusCode, GraphQL.ExecutionError executionError) { }
         protected virtual System.Threading.Tasks.Task WriteErrorResponseAsync(Microsoft.AspNetCore.Http.HttpContext context, System.Net.HttpStatusCode httpStatusCode, string errorMessage) { }
         protected virtual System.Threading.Tasks.Task WriteJsonResponseAsync<TResult>(Microsoft.AspNetCore.Http.HttpContext context, System.Net.HttpStatusCode httpStatusCode, TResult result) { }
@@ -133,6 +136,8 @@ namespace GraphQL.Server.Transports.AspNetCore
         public bool AuthorizationRequired { get; set; }
         public string? AuthorizedPolicy { get; set; }
         public System.Collections.Generic.List<string> AuthorizedRoles { get; set; }
+        public bool CsrfProtectionEnabled { get; set; }
+        public System.Collections.Generic.List<string> CsrfProtectionHeaders { get; set; }
         public Microsoft.Net.Http.Headers.MediaTypeHeaderValue DefaultResponseContentType { get; set; }
         public bool EnableBatchedRequests { get; set; }
         public bool ExecuteBatchedRequestsInParallel { get; set; }
@@ -217,6 +222,11 @@ namespace GraphQL.Server.Transports.AspNetCore.Errors
     {
         public BatchedRequestsNotSupportedError() { }
     }
+    public class CsrfProtectionError : GraphQL.Execution.RequestError
+    {
+        public CsrfProtectionError(System.Collections.Generic.IEnumerable<string> headersRequired) { }
+        public CsrfProtectionError(System.Collections.Generic.IEnumerable<string> headersRequired, System.Exception innerException) { }
+    }
     public class FileCountExceededError : GraphQL.Execution.RequestError, GraphQL.Server.Transports.AspNetCore.Errors.IHasPreferredStatusCode
     {
         public FileCountExceededError() { }