Skip to content

Commit

Permalink
Disable ReadFormOnPost by default (#68)
Browse files Browse the repository at this point in the history
  • Loading branch information
Shane32 authored Aug 19, 2024
1 parent 9f8548f commit f1b6afe
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 55 deletions.
105 changes: 53 additions & 52 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,27 @@ http://localhost:5000/graphql?query={hero}
{"data":{"hero":"Luke Skywalker"}}
```

### Basic options

By default, the middleware will be installed with these configurable options:
- GET, POST, and WebSocket requests are all enabled
- Form content types are disabled, and cross-site request forgery (CSRF)
protection is enabled
- There are no authentication or authorization requirements
- The default response content type is `application/graphql-response+json`
- The middleware will use the default schema instance

To configure these options, pass a confiuguration delegate to the `UseGraphQL`
method as demonstrated below:

```csharp
app.UseGraphQL("/graphql", opts => {
opts.ReadFormOnPost = true;
});
```

Configuration of these options and more are further described below in this document.

### Configuration with endpoint routing

To use endpoint routing, call `MapGraphQL` from inside the endpoint configuration
Expand Down Expand Up @@ -496,7 +517,7 @@ will reject requests that do not meet one of the following criteria:
`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`.
To disable this behavior, set the `CsrfProtectionEnabled` option to `false`.

```csharp
app.UseGraphQL("/graphql", config =>
Expand Down Expand Up @@ -599,7 +620,7 @@ endpoint.
| `MaximumFileSize` | Sets the maximum file size allowed for GraphQL multipart requests. | unlimited |
| `MaximumFileCount` | Sets the maximum number of files allowed for GraphQL multipart requests. | unlimited |
| `ReadExtensionsFromQueryString` | Enables reading extensions from the query string. | True |
| `ReadFormOnPost` | Enables parsing of form data for POST requests (may have security implications). | True |
| `ReadFormOnPost` | Enables parsing of form data for POST requests (may have security implications). | False |
| `ReadQueryStringOnPost` | Enables parsing the query string on POST requests. | True |
| `ReadVariablesFromQueryString` | Enables reading variables from the query string. | True |
| `ValidationErrorsReturnBadRequest` | When enabled, GraphQL requests with validation errors have the HTTP status code set to 400 Bad Request. | True |
Expand Down Expand Up @@ -821,14 +842,29 @@ even when the CORS policy prohibits it, regardless of whether the sender has acc
This situation exposes the system to security vulnerabilities, which should be carefully evaluated and
mitigated to ensure the safe handling of GraphQL requests and maintain the integrity of the data.

This functionality is activated by default to maintain backward compatibility, but it can be turned off by
setting the `ReadFormOnPost` value to `false`. The next major version of GraphQL.NET Server will have this
feature disabled by default, enhancing security measures.
To mitigate this potential security vulnerability, CSRF protection is enabled by default, requiring a
`GraphQL-Require-Preflight` header to be sent with form data requests, which will trigger a CORS preflight
request. In addition, form data requests are disabled by default, as they are not recommended for typical
use.

Keep in mind that CORS pre-flight requests are also not executed for GET requests, potentially presenting a
security risk. However, GraphQL query operations usually do not alter data, and mutations are refused.
Additionally, the response is not expected to be readable in the browser (unless CORS checks are successful),
which helps alleviate this concern.
To enable form data for POST request, set the `ReadFormOnPost` setting to `true`. GraphQL.NET Server supports
two formats of `application/x-www-form-urlencoded` or `multipart/form-data` requests:

1. The following keys are read from the form data and used to populate the GraphQL request:
- `query`: The GraphQL query string.
- `operationName`: The name of the operation to execute.
- `variables`: A JSON-encoded object containing the variables for the operation.
- `extensions`: A JSON-encoded object containing the extensions for the operation.

2. The following keys are read from the form data and used to populate the GraphQL request:
- `operations`: A JSON-encoded object containing the GraphQL request, in the same format as typical
requests sent via `application/json`. This can be a single object or an array of objects if batching
is enabled.
- `map`: An optional JSON-encoded map of file keys to file objects. This is used to map attached files
into the GraphQL request's variables property. See the section below titled 'File uploading/downloading' and the
[GraphQL multipart request specification](https://github.com/jaydenseric/graphql-multipart-request-spec)
for additional details. Since `application/x-www-form-urlencoded` cannot transmit files, this feature
is only available for `multipart/form-data` requests.

### Excessive `OperationCanceledException`s

Expand All @@ -850,54 +886,17 @@ app.UseWebSockets();
app.UseGraphQL();
```

### Handling form data for POST requests

The GraphQL over HTTP specification does not outline a procedure for transmitting GraphQL requests via
HTTP POST connections using a `Content-Type` of `application/x-www-form-urlencoded` or `multipart/form-data`.
Allowing the processing of such requests could be advantageous in avoiding CORS preflight requests when
sending GraphQL queries from a web browser. Nevertheless, enabling this feature may give rise to security
risks when utilizing cookie authentication, since transmitting cookies with these requests does not trigger
a pre-flight CORS check. As a consequence, GraphQL.NET might execute a request and potentially modify data
even when the CORS policy prohibits it, regardless of whether the sender has access to the response.
This situation exposes the system to security vulnerabilities, which should be carefully evaluated and
mitigated to ensure the safe handling of GraphQL requests and maintain the integrity of the data.

This functionality is activated by default to maintain backward compatibility, but it can be turned off by
setting the `ReadFormOnPost` value to `false`. The next major version of GraphQL.NET Server will have this
feature disabled by default, enhancing security measures.

Keep in mind that CORS pre-flight requests are also not executed for GET requests, potentially presenting a
security risk. However, GraphQL query operations usually do not alter data, and mutations are refused.
Additionally, the response is not expected to be readable in the browser (unless CORS checks are successful),
which helps alleviate this concern.

GraphQL.NET Server supports two formats of `application/x-www-form-urlencoded` or `multipart/form-data` requests:

1. The following keys are read from the form data and used to populate the GraphQL request:
- `query`: The GraphQL query string.
- `operationName`: The name of the operation to execute.
- `variables`: A JSON-encoded object containing the variables for the operation.
- `extensions`: A JSON-encoded object containing the extensions for the operation.

2. The following keys are read from the form data and used to populate the GraphQL request:
- `operations`: A JSON-encoded object containing the GraphQL request, in the same format as typical
requests sent via `application/json`. This can be a single object or an array of objects if batching
is enabled.
- `map`: An optional JSON-encoded map of file keys to file objects. This is used to map attached files
into the GraphQL request's variables property. See the section below titled 'File uploading/downloading' and the
[GraphQL multipart request specification](https://github.com/jaydenseric/graphql-multipart-request-spec)
for additional details. Since `application/x-www-form-urlencoded` cannot transmit files, this feature
is only available for `multipart/form-data` requests.

### File uploading/downloading

A common question is how to upload or download files attached to GraphQL data.
For instance, storing and retrieving photographs attached to product data.

One common technique is to encode the data as Base64 and transmitting as a custom
GraphQL scalar (encoded as a string value within the JSON transport).
This may not be ideal, but works well for smaller files. It can also couple with
response compression (details listed above) to reduce the impact of the Base64
encoding.

Another technique is to get or store the data out-of-band. For responses, this can
be as simple as a Uri pointing to a location to retrieve the data, especially if
the data are photographs used in a SPA client application. This may have additional
Expand All @@ -924,10 +923,12 @@ services.AddGraphQL(b => b
.AddSystemTextJson());
```

Please see the 'Upload' sample for a demonstration of this technique. Note that
using the `FormFileGraphType` scalar requires that the uploaded files be sent only
via the `multipart/form-data` content type as attached files. If you wish to also
allow clients to send files as base-64 encoded strings, you can write a custom scalar
Please see the 'Upload' sample for a demonstration of this technique, which also
demonstrates the use of the `MediaTypeAttribute` to restrict the allowable media
types that will be accepted. Note that using the `FormFileGraphType` scalar requires
that the uploaded files be sent only via the `multipart/form-data` content type as
attached files, with the `ReadFormOnPost` option enabled. If you also wish to allow
clients to send files as base-64 encoded strings, you can write a custom scalar
better suited to your needs.

## Samples
Expand Down
6 changes: 4 additions & 2 deletions src/GraphQL.AspNetCore3/GraphQLHttpMiddlewareOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,11 @@ public class GraphQLHttpMiddlewareOptions : IAuthorizationOptions
/// alongside the request does not initiate a pre-flight CORS request.
/// As a result, GraphQL.NET carries out the request and potentially modifies data,
/// even if the CORS policy forbids it, irrespective of the sender's ability to access
/// the response.
/// the response. With <see cref="CsrfProtectionEnabled"/> enabled, these requests
/// are blocked the request contains a non-empty header from the
/// <see cref="CsrfProtectionHeaders"/> list, providing a measure of protection.
/// </remarks>
public bool ReadFormOnPost { get; set; } = true;
public bool ReadFormOnPost { get; set; }

/// <summary>
/// Enables cross-site request forgery (CSRF) protection for both GET and POST requests.
Expand Down
8 changes: 7 additions & 1 deletion src/Tests/Middleware/PostTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ public async Task AltCharset_Invalid()
[InlineData(false, false)]
public async Task FormMultipart_Legacy(bool requireCsrf, bool supplyCsrf)
{
_options2.ReadFormOnPost = true;
if (!requireCsrf)
_options2.CsrfProtectionEnabled = false;
var client = _server.CreateClient();
Expand Down Expand Up @@ -183,6 +184,7 @@ public async Task FormMultipart_Legacy(bool requireCsrf, bool supplyCsrf)
[InlineData(false, false)]
public async Task FormMultipart_Upload(bool requireCsrf, bool supplyCsrf)
{
_options2.ReadFormOnPost = true;
if (!requireCsrf)
_options2.CsrfProtectionEnabled = false;
var client = _server.CreateClient();
Expand Down Expand Up @@ -353,6 +355,7 @@ public async Task FormMultipart_Upload(bool requireCsrf, bool supplyCsrf)
[Theory]
public async Task FormMultipart_Upload_Matrix(int testIndex, string? operations, string? map, bool file0, bool file1, int expectedStatusCode, string expectedResponse)
{
_options2.ReadFormOnPost = true;
_ = testIndex;
operations ??= "{\"query\":\"query($arg:FormFile){file(file:$arg){content}}\",\"variables\":{\"arg\":null}}";
var client = _server.CreateClient();
Expand Down Expand Up @@ -381,6 +384,7 @@ public async Task FormMultipart_Upload_Validation(int? maxFileCount, int? maxFil
var client = _server.CreateClient();
_options2.MaximumFileCount = maxFileCount;
_options2.MaximumFileSize = maxFileLength;
_options2.ReadFormOnPost = true;
using var content = new MultipartFormDataContent
{
{ new StringContent(operations, Encoding.UTF8, "application/json"), "operations" },
Expand All @@ -401,6 +405,7 @@ public async Task FormMultipart_Upload_Validation(int? maxFileCount, int? maxFil
[InlineData(false, false)]
public async Task FormUrlEncoded(bool requireCsrf, bool supplyCsrf)
{
_options2.ReadFormOnPost = true;
if (!requireCsrf)
_options2.CsrfProtectionEnabled = false;
var client = _server.CreateClient();
Expand All @@ -425,7 +430,8 @@ public async Task FormUrlEncoded(bool requireCsrf, bool supplyCsrf)
[InlineData(true)]
public async Task FormUrlEncoded_DeserializationError(bool badRequest)
{
_options.ValidationErrorsReturnBadRequest = badRequest;
_options2.ValidationErrorsReturnBadRequest = badRequest;
_options2.ReadFormOnPost = true;
var client = _server.CreateClient();
var content = new FormUrlEncodedContent(new[] {
new KeyValuePair<string?, string?>("query", @"{ext}"),
Expand Down

0 comments on commit f1b6afe

Please sign in to comment.