diff --git a/README.md b/README.md index c9ba436e5..a9e62c726 100644 --- a/README.md +++ b/README.md @@ -1918,6 +1918,9 @@ resources are accessed. The `MemberAuthenticator` class is configured as the default option for authentication, and will attempt to use the current CMS `Member` session for authentication context. +**If you are using the default session-based authentication, please be sure that you have +the [CSRF Middleware](#csrf-tokens-required-for-mutations) enabled. (It is by default).** + ### HTTP basic authentication Silverstripe has built in support for [HTTP basic authentication](https://en.wikipedia.org/wiki/Basic_access_authentication). @@ -1927,6 +1930,9 @@ authenticator because GraphQL needs to use the successfully authenticated member for CMS permission filtering, whereas the global `BasicAuth` does not log the member in or use it for model security. +When using HTTP basic authentication, you can feel free to remove the [CSRF Middleware](#csrf-tokens-required-for-mutations), +as it just adds unnecessary overhead to the request. + #### In GraphiQL If you want to add basic authentication support to your GraphQL requests you can @@ -1993,12 +1999,19 @@ the `SecurityToken` API, using `SecurityToken::inst()->getValue()`. Queries do not require CSRF tokens. +### Disabling CSRF protection (for token-based authentication only) + +If you are using HTTP basic authentication or a token-based system like OAuth or [JWT](https://github.com/Firesphere/silverstripe-graphql-jwt), +you will want to remove the CSRF protection, as it just adds unnecessary overhead. You can do this by setting +the middleware to `false`. + ```yaml SilverStripe\GraphQL\Manager: properties: Middlewares: CSRFMiddleware: false ``` + ## Cross-Origin Resource Sharing (CORS) By default [CORS](https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS) is disabled in the GraphQL Server. This can be easily enabled via YAML: diff --git a/src/Middleware/CSRFMiddleware.php b/src/Middleware/CSRFMiddleware.php index 335bed436..ecf72daa3 100644 --- a/src/Middleware/CSRFMiddleware.php +++ b/src/Middleware/CSRFMiddleware.php @@ -2,7 +2,11 @@ namespace SilverStripe\GraphQL\Middleware; +use GraphQL\Language\Parser; +use GraphQL\Language\Source; use GraphQL\Schema; +use GraphQL\Language\AST\NodeKind; +use SilverStripe\GraphQL\Manager; use SilverStripe\GraphQL\Middleware\QueryMiddleware; use SilverStripe\Security\SecurityToken; use Exception; @@ -11,7 +15,7 @@ class CSRFMiddleware implements QueryMiddleware { public function process(Schema $schema, $query, $context, $params, callable $next) { - if (preg_match('/^\s*mutation/', $query)) { + if ($this->isMutation($query)) { if (empty($context['token'])) { throw new Exception('Mutations must provide a CSRF token in the X-CSRF-TOKEN header'); } @@ -24,4 +28,39 @@ public function process(Schema $schema, $query, $context, $params, callable $nex return $next($schema, $query, $context, $params); } + + /** + * @param string $query + * @return bool + */ + protected function isMutation($query) + { + // Simple string matching as a first check to prevent unnecessary static analysis + if (stristr($query, Manager::MUTATION_ROOT) === false) { + return false; + } + + // If "mutation" is the first expression in the query, then it's a mutation. + if (preg_match('/^\s*' . preg_quote(Manager::MUTATION_ROOT, '/') . '/', $query)) { + return true; + } + + // Otherwise, bring in the big guns. + $document = Parser::parse(new Source($query ?: 'GraphQL')); + $defs = $document->definitions; + foreach ($defs as $statement) { + $options = [ + NodeKind::OPERATION_DEFINITION, + NodeKind::OPERATION_TYPE_DEFINITION + ]; + if (!in_array($statement->kind, $options, true)) { + continue; + } + if ($statement->operation === Manager::MUTATION_ROOT) { + return true; + } + } + + return false; + } } diff --git a/tests/Middleware/CSRFMiddlewareTest.php b/tests/Middleware/CSRFMiddlewareTest.php index ac8ddefb2..c851bb494 100644 --- a/tests/Middleware/CSRFMiddlewareTest.php +++ b/tests/Middleware/CSRFMiddlewareTest.php @@ -30,7 +30,42 @@ public function testItThrowsIfNoTokenIsProvided() ' mutation someMutation { tester }' ); $this->assertNotEquals('resolved', $result); + $result = $this->simulateMiddlewareProcess( + new CSRFMiddleware(), + ' mutation someMutation { tester }' + ); + $this->assertNotEquals('resolved', $result); + $graphql = <<simulateMiddlewareProcess( + new CSRFMiddleware(), + $graphql + ); + $this->assertNotEquals('resolved', $result); + $graphql = <<simulateMiddlewareProcess( + new CSRFMiddleware(), + $graphql + ); + $this->assertNotEquals('resolved', $result); + } + public function testItThrowsIfTokenIsInvalid() {