Skip to content

Commit 2376520

Browse files
klausiklausi
committed
fix(routing): Fix handling of POST requests
1 parent f55a29d commit 2376520

5 files changed

Lines changed: 283 additions & 5 deletions

File tree

‎graphql.services.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ services:
8787
# Upcasting for graphql query request parameters.
8888
graphql.route_enhancer.query:
8989
class: Drupal\graphql\Routing\QueryRouteEnhancer
90-
arguments: ['@request_stack']
90+
arguments: ['%cors.config%']
9191
tags:
9292
- { name: route_enhancer }
9393

‎src/Routing/QueryRouteEnhancer.php‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,35 @@
22

33
namespace Drupal\graphql\Routing;
44

5+
use Asm89\Stack\CorsService;
56
use Drupal\Component\Utility\NestedArray;
67
use Drupal\Core\Routing\EnhancerInterface;
78
use Drupal\Core\Routing\RouteObjectInterface;
89
use Drupal\graphql\GraphQL\Utility\JsonHelper;
910
use GraphQL\Server\Helper;
1011
use Symfony\Component\HttpFoundation\Request;
12+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
1113
use Symfony\Component\Routing\Route;
1214

1315
/**
1416
* Adds GraphQL operation information to the Symfony route being resolved.
1517
*/
1618
class QueryRouteEnhancer implements EnhancerInterface {
1719

20+
/**
21+
* The CORS options for Origin header checking.
22+
*
23+
* @var array
24+
*/
25+
protected $corsOptions;
26+
27+
/**
28+
* Constructor.
29+
*/
30+
public function __construct(array $corsOptions) {
31+
$this->corsOptions = $corsOptions;
32+
}
33+
1834
/**
1935
* Returns whether the enhancer runs on the current route.
2036
*
@@ -38,6 +54,10 @@ public function enhance(array $defaults, Request $request) {
3854
return $defaults;
3955
}
4056

57+
if ($request->getMethod() === "POST") {
58+
$this->assertValidPostRequestHeaders($request);
59+
}
60+
4161
$helper = new Helper();
4262
$method = $request->getMethod();
4363
$body = $this->extractBody($request);
@@ -47,6 +67,83 @@ public function enhance(array $defaults, Request $request) {
4767
return $defaults + ['operations' => $operations];
4868
}
4969

70+
/**
71+
* Ensures that the headers for a POST request have triggered a preflight.
72+
*
73+
* POST requests must be submitted with content-type headers that properly
74+
* trigger a cross-origin preflight request. In case content-headers are used
75+
* that would trigger a "simple" request then custom headers must be provided.
76+
*
77+
* @param \Symfony\Component\HttpFoundation\Request $request
78+
* The request to check.
79+
*
80+
* @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
81+
* In case the headers indicated a preflight was not performed.
82+
*/
83+
protected function assertValidPostRequestHeaders(Request $request) : void {
84+
$content_type = $request->headers->get('content-type');
85+
if ($content_type === NULL) {
86+
throw new BadRequestHttpException("GraphQL requests must specify a valid content type header.");
87+
}
88+
89+
// application/graphql is a non-standard header that's supported by our
90+
// server implementation and triggers CORS.
91+
if ($content_type === "application/graphql") {
92+
return;
93+
}
94+
95+
/** @phpstan-ignore-next-line */
96+
$content_format = method_exists($request, 'getContentTypeFormat') ? $request->getContentTypeFormat() : $request->getContentType();
97+
if ($content_format === NULL) {
98+
// Symfony before 5.4 does not detect "multipart/form-data", check for it
99+
// manually.
100+
if (stripos($content_type, 'multipart/form-data') === 0) {
101+
$content_format = 'form';
102+
}
103+
else {
104+
throw new BadRequestHttpException("The content type '$content_type' is not supported.");
105+
}
106+
}
107+
108+
// JSON requests provide a non-standard header that trigger CORS.
109+
if ($content_format === "json") {
110+
return;
111+
}
112+
113+
// The form content types are considered simple requests and don't trigger
114+
// CORS pre-flight checks, so these require a separate header to prevent
115+
// CSRF. We need to support "form" for file uploads.
116+
if ($content_format === "form") {
117+
// If the client set a custom header then we can be sure CORS was
118+
// respected.
119+
$custom_headers = ['Apollo-Require-Preflight', 'X-Apollo-Operation-Name', 'x-graphql-yoga-csrf'];
120+
foreach ($custom_headers as $custom_header) {
121+
if ($request->headers->has($custom_header)) {
122+
return;
123+
}
124+
}
125+
// 1. Allow requests that have set no Origin header at all, for example
126+
// server-to-server requests.
127+
// 2. Allow requests where the Origin matches the site's domain name.
128+
$origin = $request->headers->get('Origin');
129+
if ($origin === NULL || $request->getSchemeAndHttpHost() === $origin) {
130+
return;
131+
}
132+
// Allow other origins as configured in the CORS policy.
133+
if (!empty($this->corsOptions['enabled'])) {
134+
$cors_service = new CorsService($this->corsOptions);
135+
// Drupal 9 compatibility, method name has changed in Drupal 10.
136+
/** @phpstan-ignore-next-line */
137+
if ($cors_service->isActualRequestAllowed($request)) {
138+
return;
139+
}
140+
}
141+
throw new BadRequestHttpException("Form requests must include a Apollo-Require-Preflight HTTP header or the Origin HTTP header value needs to be in the allowedOrigins in the CORS settings.");
142+
}
143+
144+
throw new BadRequestHttpException("The content type '$content_type' is not supported.");
145+
}
146+
50147
/**
51148
* Extracts the query parameters from a request.
52149
*

‎tests/src/Kernel/Framework/AutomaticPersistedQueriesTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public function testAutomaticPersistedQueries(): void {
7272

7373
// Post query to endpoint with a not matching hash.
7474
$content = json_encode(['query' => $query] + $parameters);
75-
$request = Request::create($endpoint, 'POST', [], [], [], [], $content);
75+
$request = Request::create($endpoint, 'POST', [], [], [], ['CONTENT_TYPE' => 'application/json'], $content);
7676
$result = $this->container->get('http_kernel')->handle($request);
7777
$this->assertSame(200, $result->getStatusCode());
7878
$this->assertSame([
@@ -88,7 +88,7 @@ public function testAutomaticPersistedQueries(): void {
8888
$parameters['extensions']['persistedQuery']['sha256Hash'] = hash('sha256', $query);
8989

9090
$content = json_encode(['query' => $query] + $parameters);
91-
$request = Request::create($endpoint, 'POST', [], [], [], [], $content);
91+
$request = Request::create($endpoint, 'POST', [], [], [], ['CONTENT_TYPE' => 'application/json'], $content);
9292
$result = $this->container->get('http_kernel')->handle($request);
9393
$this->assertSame(200, $result->getStatusCode());
9494
$this->assertSame(['data' => ['field_one' => 'this is the field one']], json_decode($result->getContent(), TRUE));

‎tests/src/Kernel/Framework/CsrfTest.php‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
<?php
2+
3+
namespace Drupal\Tests\graphql\Kernel\Framework;
4+
5+
use Drupal\graphql\Routing\QueryRouteEnhancer;
6+
use Drupal\Tests\graphql\Kernel\GraphQLTestBase;
7+
use Symfony\Component\HttpFoundation\Request;
8+
9+
/**
10+
* Test CSRF protection on mutations.
11+
*
12+
* @group graphql
13+
*/
14+
class CsrfTest extends GraphQLTestBase {
15+
16+
/**
17+
* Helper state variable that will be flipped when the test mutation executes.
18+
*
19+
* @var bool
20+
*/
21+
protected $mutationTriggered = FALSE;
22+
23+
/**
24+
* {@inheritdoc}
25+
*/
26+
protected function setUp(): void {
27+
parent::setUp();
28+
29+
$schema = <<<GQL
30+
schema {
31+
mutation: Mutation
32+
}
33+
34+
type Mutation {
35+
write: Boolean
36+
}
37+
GQL;
38+
$this->setUpSchema($schema);
39+
$this->mockResolver('Mutation', 'write',
40+
function () {
41+
$this->mutationTriggered = TRUE;
42+
return TRUE;
43+
}
44+
);
45+
}
46+
47+
/**
48+
* Tests that a simple request from an evil origin is not executed.
49+
*
50+
* @dataProvider provideSimpleContentTypes
51+
*/
52+
public function testEvilOrigin(string $content_type): void {
53+
$request = Request::create('https://example.com/graphql/test', 'POST', [], [], [], [
54+
'CONTENT_TYPE' => $content_type,
55+
'HTTP_ORIGIN' => 'https://evil.example.com',
56+
], '{ "query": "mutation { write }" }');
57+
58+
/** @var \Symfony\Component\HttpFoundation\Response $response */
59+
$response = $this->container->get('http_kernel')->handle($request);
60+
$this->assertFalse($this->mutationTriggered, 'Mutation was triggered');
61+
$this->assertSame(400, $response->getStatusCode());
62+
}
63+
64+
/**
65+
* Data provider for testContentTypeCsrf().
66+
*/
67+
public function provideSimpleContentTypes(): array {
68+
// Three content types that can be sent with simple no-cors POST requests.
69+
return [
70+
['text/plain'],
71+
['application/x-www-form-urlencoded'],
72+
['multipart/form-data'],
73+
];
74+
}
75+
76+
/**
77+
* Tests that a simple multipart form data no-cors request is not executed.
78+
*/
79+
public function testMultipartFormDataCsrf(): void {
80+
$request = Request::create('https://example.com/graphql/test', 'POST',
81+
[
82+
'operations' => '[{ "query": "mutation { write }" }]',
83+
],
84+
[],
85+
[],
86+
[
87+
'CONTENT_TYPE' => 'multipart/form-data',
88+
'HTTP_ORIGIN' => 'https://evil.example.com',
89+
]
90+
);
91+
92+
/** @var \Symfony\Component\HttpFoundation\Response $response */
93+
$response = $this->container->get('http_kernel')->handle($request);
94+
$this->assertFalse($this->mutationTriggered, 'Mutation was triggered');
95+
$this->assertSame(400, $response->getStatusCode());
96+
$result = json_decode($response->getContent());
97+
$this->assertSame("Form requests must include a Apollo-Require-Preflight HTTP header or the Origin HTTP header value needs to be in the allowedOrigins in the CORS settings.", $result->message);
98+
}
99+
100+
/**
101+
* Test that the JSON content types always work, cannot be forged with CSRF.
102+
*
103+
* @dataProvider provideAllowedJsonHeaders
104+
*/
105+
public function testAllowedJsonRequests(array $headers): void {
106+
$request = Request::create('https://example.com/graphql/test', 'POST', [], [], [],
107+
$headers, '{ "query": "mutation { write }" }');
108+
109+
/** @var \Symfony\Component\HttpFoundation\Response $response */
110+
$response = $this->container->get('http_kernel')->handle($request);
111+
$this->assertTrue($this->mutationTriggered, 'Mutation was triggered');
112+
$this->assertSame(200, $response->getStatusCode());
113+
}
114+
115+
/**
116+
* Data provider for testAllowedJsonRequests().
117+
*/
118+
public function provideAllowedJsonHeaders(): array {
119+
return [
120+
[['CONTENT_TYPE' => 'application/json']],
121+
[['CONTENT_TYPE' => 'application/graphql']],
122+
];
123+
}
124+
125+
/**
126+
* Test that a form request with the correct headers against CSRF are allowed.
127+
*
128+
* @dataProvider provideAllowedFormRequests
129+
*/
130+
public function testAllowedFormRequests(array $headers, array $allowedDomains = []): void {
131+
$request = Request::create('https://example.com/graphql/test', 'POST',
132+
[
133+
'operations' => '[{ "query": "mutation { write }" }]',
134+
], [], [], $headers);
135+
136+
if (!empty($allowedDomains)) {
137+
// Replace the QueryRouteEnhancer to inject CORS config we want to test.
138+
$this->container->set('graphql.route_enhancer.query', new QueryRouteEnhancer([
139+
'enabled' => TRUE,
140+
'allowedOrigins' => $allowedDomains,
141+
]));
142+
}
143+
/** @var \Symfony\Component\HttpFoundation\Response $response */
144+
$response = $this->container->get('http_kernel')->handle($request);
145+
$this->assertTrue($this->mutationTriggered, 'Mutation was triggered');
146+
$this->assertSame(200, $response->getStatusCode());
147+
}
148+
149+
/**
150+
* Data provider for testAllowedFormRequests().
151+
*/
152+
public function provideAllowedFormRequests(): array {
153+
return [
154+
// Omitting the Origin and Apollo-Require-Preflight is allowed.
155+
[['CONTENT_TYPE' => 'multipart/form-data']],
156+
// The custom Apollo-Require-Preflight header overrules any evil Origin
157+
// header.
158+
[[
159+
'CONTENT_TYPE' => 'multipart/form-data',
160+
'HTTP_APOLLO_REQUIRE_PREFLIGHT' => 'test',
161+
'HTTP_ORIGIN' => 'https://evil.example.com',
162+
]],
163+
// The Origin header alone with the correct domain is allowed.
164+
[[
165+
'CONTENT_TYPE' => 'multipart/form-data',
166+
'HTTP_ORIGIN' => 'https://example.com',
167+
]],
168+
// The Origin header with an allowed domain.
169+
[[
170+
'CONTENT_TYPE' => 'multipart/form-data',
171+
'HTTP_ORIGIN' => 'https://allowed.example.com',
172+
], ['https://allowed.example.com']],
173+
// The Origin header with any allowed domain.
174+
[[
175+
'CONTENT_TYPE' => 'multipart/form-data',
176+
'HTTP_ORIGIN' => 'https://allowed.example.com',
177+
], ['*']],
178+
];
179+
}
180+
181+
}

‎tests/src/Traits/HttpRequestTrait.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ protected function query($query, $server = NULL, array $variables = [], array $e
5656
$request = Request::create($endpoint, $method, $data);
5757
}
5858
else {
59-
$request = Request::create($endpoint, $method, [], [], [], [], json_encode($data));
59+
$request = Request::create($endpoint, $method, [], [], [], ['CONTENT_TYPE' => 'application/json'], json_encode($data));
6060
}
6161

6262
return $this->container->get('http_kernel')->handle($request);
@@ -81,7 +81,7 @@ protected function batchedQueries(array $queries, ServerInterface $server = NULL
8181

8282
$queries = json_encode($queries);
8383
$endpoint = $this->server->get('endpoint');
84-
$request = Request::create($endpoint, 'POST', [], [], [], [], $queries);
84+
$request = Request::create($endpoint, 'POST', [], [], [], ['CONTENT_TYPE' => 'application/json'], $queries);
8585
return $this->container->get('http_kernel')->handle($request);
8686
}
8787

0 commit comments

Comments
 (0)