From f84451db2b717ea8cbf7fe77996117467e1a7cca Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Wed, 11 Aug 2021 16:07:36 +1200 Subject: [PATCH 1/2] NEW: Disable schema introspection in dev --- README.md | 83 +++++++++++++++--------- src/Controller.php | 3 +- src/Extensions/IntrospectionProvider.php | 3 + src/Manager.php | 32 +++++++++ 4 files changed, 90 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index b9e82e205..3985286d7 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ composer require silverstripe/graphql - [CSRF tokens (required for mutations)](#csrf-tokens-required-for-mutations) - [Cross-Origin Resource Sharing (CORS)](#cross-origin-resource-sharing-cors) - [Sample Custom CORS Config](#sample-custom-cors-config) - - [Persisting Queries](#persisting-queries) + - [Persisting Queries](#persisting-queries) - [Schema introspection](#schema-introspection) - [Setting up a new GraphQL schema](#setting-up-a-new-graphql-schema) - [Strict HTTP Method Checking](#strict-http-method-checking) @@ -165,7 +165,7 @@ through YAML configuration: SilverStripe\GraphQL\Manager: schemas: default: - types: + types: member: 'MyProject\GraphQL\MemberTypeCreator' ``` @@ -547,7 +547,7 @@ in your constructor. ```php $this->queryFilter = DataObjectQueryFilter::create(MyDataObject::class); -``` +``` You can then add filters to fields of the dataobject. @@ -557,7 +557,7 @@ $this->queryFilter ->addFieldFilterByIdentifier('CommentCount', 'gt') ->addFieldFilterByIdentifier('Categories__Title', 'in') ->addFieldFilterByIdentifier('Hidden', 'eq'); -``` +``` Don't worry about the filter keys (`contains`, `gt`, `eq`, etc) for now. That will be explained [further down](#the-filter-registry). @@ -584,11 +584,11 @@ public function resolve($obj, $args = [], $context = [], ResolveInfo $info) { $list = MyDataObject::get(); $list = $this->queryFilter->applyArgsToList($list, $args); - + return $list; } ``` - + #### Shortcuts (for the 80% case) All `SilverStripe\ORM\DBField` instances are configured to have a set of "default" filters (see `filters.yml`). @@ -666,7 +666,7 @@ class MyCustomFieldFilter implements FieldFilterInterface { return 'eq'; } - + public function applyInclusion(DataList $list, $fieldName, $value) { return $list->addWhere([ @@ -987,7 +987,7 @@ You can add all default filters for every field on your dataobject with `filters ```yaml read: filters: '*' -``` +``` > Note: "every field" means every field exposed by `searchable_fields` on the dataobject -- not just those exposed on its GraphQL type. To be more granular, break it up into a list of specific fields. @@ -2239,7 +2239,7 @@ session. In the absence of a token-based authentication system, like OAuth, the best countermeasure to this is the use of a CSRF token for any requests that destroy or mutate data. -By default, this module comes with a `CSRFMiddleware` implementation that forces all mutations to check +By default, this module comes with a `CSRFMiddleware` implementation that forces all mutations to check for the presence of a CSRF token in the request. That token must be applied to a header named` X-CSRF-TOKEN`. In SilverStripe, CSRF tokens are most commonly stored in the session as `SecurityID`, or accessed through @@ -2347,27 +2347,27 @@ Once you have enabled CORS you can then control four new headers in the HTTP Res ```yaml Max-Age: 600 ``` - + 5. **Access-Control-Allow-Credentials.** - + When a request's credentials mode (Request.credentials) is "include", browsers - will only expose the response to frontend JavaScript code if the + will only expose the response to frontend JavaScript code if the Access-Control-Allow-Credentials value is true. - + The Access-Control-Allow-Credentials header works in conjunction with the XMLHttpRequest.withCredentials property or with the credentials option in the - Request() constructor of the Fetch API. For a CORS request with credentials, + Request() constructor of the Fetch API. For a CORS request with credentials, in order for browsers to expose the response to frontend JavaScript code, both - the server (using the Access-Control-Allow-Credentials header) and the client - (by setting the credentials mode for the XHR, Fetch, or Ajax request) must + the server (using the Access-Control-Allow-Credentials header) and the client + (by setting the credentials mode for the XHR, Fetch, or Ajax request) must indicate that they’re opting in to including credentials. - + This is set to empty by default but can be changed in YAML as in this example: ```yaml Allow-Credentials: 'true' ``` - + ### Apply a CORS config to all GraphQL endpoints ```yaml @@ -2380,7 +2380,7 @@ SilverStripe\GraphQL\Controller: Allow-Methods: 'GET, POST, OPTIONS' Allow-Credentials: 'true' Max-Age: 600 # 600 seconds = 10 minutes. -``` +``` ### Apply a CORS config to a single GraphQL endpoint @@ -2391,7 +2391,7 @@ SilverStripe\Core\Injector\Injector: properties: corsConfig: Enabled: false -``` +``` ## Persisting queries @@ -2411,7 +2411,7 @@ which cover most use cases: ### Configuring query mapping providers -All of these implementations can be configured through `Injector`. Note that each schema gets its +All of these implementations can be configured through `Injector`. Note that each schema gets its own set of persisted queries. In these examples, we're using the `default`schema. #### FileProvider @@ -2470,18 +2470,18 @@ To access a persisted query, simply pass an `id` parameter in the request in lie Note that if you pass `query` along with `id`, an exception will be thrown. ## Schema introspection -Some GraphQL clients such as [Apollo](http://apollographql.com) require some level of introspection -into the schema. While introspection is [part of the GraphQL spec](http://graphql.org/learn/introspection/), +Some GraphQL clients such as [Apollo](http://apollographql.com) require some level of introspection +into the schema. While introspection is [part of the GraphQL spec](http://graphql.org/learn/introspection/), this module provides a limited API for fetching it via non-graphql endpoints. By default, the `graphql/` controller provides a `types` action that will return the type schema (serialised as JSON) dynamically. *GET http://example.com/graphql/types* ```js -{ - "data":{ - "__schema":{ - "types":[ - { +{ + "data":{ + "__schema":{ + "types":[ + { "kind":"OBJECT", "name":"Query", "possibleTypes":null @@ -2493,7 +2493,7 @@ controller provides a `types` action that will return the type schema (serialise ``` -As your schema grows, introspecting it dynamically may have a performance hit. Alternatively, +As your schema grows, introspecting it dynamically may have a performance hit. Alternatively, if you have the `silverstripe/assets` module installed (as it is in the default SilverStripe installation), GraphQL can cache your schema as a flat file in the `assets/` directory. To enable this, simply set the `cache_types_in_filesystem` setting to `true` on `SilverStripe\GraphQL\Controller`. Once enabled, @@ -2502,6 +2502,7 @@ a `types.graphql` file will be written to your `assets/` directory on `flush`. When `cache_types_in_filesystem` is enabled, it is recommended that you remove the extension that provides the dynamic introspection endpoint. + ```php use SilverStripe\GraphQL\Controller; use SilverStripe\GraphQL\Extensions\IntrospectionProvider; @@ -2509,6 +2510,28 @@ use SilverStripe\GraphQL\Extensions\IntrospectionProvider; Controller::remove_extension(IntrospectionProvider::class); ``` +### Disabling Schema introspection + +By default, schema introspection is only enabled in development environments. While schema intospection provides no +information that can be explicitly used for malicious purposes, it is best practice to disable it in production. This +is strongly encouraged by [OWASP guidelines](https://cheatsheetseries.owasp.org/cheatsheets/GraphQL_Cheat_Sheet.html) +in the spirit of "security through obscurity." + +In development environments, however, schema introspection is critical to a good developer experience, particularly in +the [graphql IDE](https://github.com/silverstripe/graphql-devtools). + +If you would like to enable schema introspection outside of your dev environment, you can set the `allowIntrospection` +property on the `Manager` instance. + +```html +SilverStripe\Core\Injector\Injector: + SilverStripe\GraphQL\Manager.default: + properties: + AllowIntrospection: true +``` + + + ## Setting up a new GraphQL schema In addition to the default `/graphql` endpoint provided by this module by default, @@ -2526,7 +2549,7 @@ SilverStripe\Core\Injector\Injector: schemaKey: mySchema ``` -The `schemaKey` setting is a bit of meta-configuration used to tell the Manager where to +The `schemaKey` setting is a bit of meta-configuration used to tell the Manager where to look in the `SilverStripe\GraphQL\Manager.schemas` config for the schema information. Now let's setup a new controller to handle the requests. It will use our custom Manager instance. diff --git a/src/Controller.php b/src/Controller.php index 7abf0d25e..bdb508846 100644 --- a/src/Controller.php +++ b/src/Controller.php @@ -109,7 +109,7 @@ public function index(HTTPRequest $request) Versioned::set_stage($stage); } } - + // Check for a possible CORS preflight request and handle if necessary // Refer issue 66: https://github.com/silverstripe/silverstripe-graphql/issues/66 if ($request->httpMethod() === 'OPTIONS') { @@ -460,6 +460,7 @@ public function writeSchemaToFilesystem() $request = new NullHTTPRequest(); } $manager = $this->getManager($request); + $manager->setAllowIntrospection(true); try { $types = StaticSchema::inst()->introspectTypes($manager); } catch (Exception $e) { diff --git a/src/Extensions/IntrospectionProvider.php b/src/Extensions/IntrospectionProvider.php index def18bfa8..1ff51bb5a 100644 --- a/src/Extensions/IntrospectionProvider.php +++ b/src/Extensions/IntrospectionProvider.php @@ -5,6 +5,7 @@ use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Core\Extension; +use SilverStripe\GraphQL\Manager; use SilverStripe\GraphQL\Scaffolding\StaticSchema; /** @@ -22,7 +23,9 @@ class IntrospectionProvider extends Extension */ public function types(HTTPRequest $request) { + /* @var Manager $manager */ $manager = $this->owner->getManager(); + $manager->setAllowIntrospection(true); $fragments = StaticSchema::inst()->introspectTypes($manager); return (new HTTPResponse(json_encode($fragments), 200)) diff --git a/src/Manager.php b/src/Manager.php index 3cbf645e6..ece306f0a 100644 --- a/src/Manager.php +++ b/src/Manager.php @@ -2,11 +2,14 @@ namespace SilverStripe\GraphQL; +use GraphQL\Validator\DocumentValidator; +use GraphQL\Validator\Rules\DisableIntrospection; use InvalidArgumentException; use GraphQL\Executor\ExecutionResult; use GraphQL\Language\SourceLocation; use GraphQL\Type\Schema; use GraphQL\GraphQL; +use SilverStripe\Control\Director; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Extensible; use SilverStripe\Core\Injector\Injector; @@ -89,6 +92,11 @@ class Manager implements ConfigurationApplier */ protected $extraContext = []; + /** + * @var bool + */ + protected $allowIntrospection = false; + /** * @return QueryMiddleware[] */ @@ -157,6 +165,7 @@ public function __construct($schemaKey = null) if ($schemaKey) { $this->setSchemaKey($schemaKey); } + $this->setAllowIntrospection(!Director::isDev()); } /** @@ -335,6 +344,10 @@ public function schema() ]); } + if (!$this->getAllowIntrospection()) { + DocumentValidator::addRule(new DisableIntrospection()); + } + return new Schema($schema); } @@ -566,6 +579,25 @@ public function getQueryFromPersistedID($id) return $provider->getByID($id); } + /** + * @return bool + */ + public function getAllowIntrospection(): bool + { + return $this->allowIntrospection; + } + + /** + * @param bool $allowIntrospection + * @return $this + */ + public function setAllowIntrospection(bool $allowIntrospection): self + { + $this->allowIntrospection = $allowIntrospection; + + return $this; + } + /** * Get global context to pass to $context for all queries * From f5d7b81a1f9eab2bb4b6cdaa53c1b0e243583c07 Mon Sep 17 00:00:00 2001 From: Matt Peel <893117+madmatt@users.noreply.github.com> Date: Thu, 3 Mar 2022 13:22:16 +1300 Subject: [PATCH 2/2] Update README.md Co-authored-by: Michal Kleiner --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3985286d7..91052968c 100644 --- a/README.md +++ b/README.md @@ -2527,7 +2527,7 @@ property on the `Manager` instance. SilverStripe\Core\Injector\Injector: SilverStripe\GraphQL\Manager.default: properties: - AllowIntrospection: true + allowIntrospection: true ```