Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CSRF protection #1138

Merged
merged 11 commits into from
Aug 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 |
Expand Down
7 changes: 7 additions & 0 deletions docs/migration/migration8.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 16 additions & 0 deletions src/Transports.AspNetCore/Errors/CsrfProtectionError.cs
Original file line number Diff line number Diff line change
@@ -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}'"));
}
55 changes: 51 additions & 4 deletions src/Transports.AspNetCore/GraphQLHttpMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Dismissed Show dismissed Hide dismissed
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>
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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>
Expand Down
16 changes: 16 additions & 0 deletions src/Transports.AspNetCore/GraphQLHttpMiddlewareOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) { }
Expand All @@ -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) { }
Expand All @@ -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; }
Expand Down Expand Up @@ -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() { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) { }
Expand All @@ -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) { }
Expand All @@ -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; }
Expand Down Expand Up @@ -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() { }
Expand Down
Loading
Loading