BUG Fix cors breaking if referer header is present

BUG Prevent un-extendable config by shifting defaults into PHP BUG Remove dependency on Doctrine module breaking with --prefer-dist BUG Fix tests not checking cors port ENHANCEMENT Clean up Controller::index() method and make lovely ENHANCEMENT Optimise all imports Fixes silverstripe#118
open-sausages · Jan 18, 2018 · dabde7e · dabde7e
1 parent 4e75efe
commit dabde7e
Show file tree

Hide file tree

Showing 52 changed files with 459 additions and 294 deletions.
diff --git a/_config/config.yml b/_config/config.yml
@@ -1,12 +1,6 @@
 ---
 Name: graphqlconfig
 ---
-# Minimum fields that any type will expose. Useful for implicitly
-# created types, e.g. exposing a has_one.
-SilverStripe\GraphQL\Scaffolding\Scaffolders\DataObjectScaffolder:
-  default_fields:
-    ID: ID
-
 # Define the type parsers
 SilverStripe\Core\Injector\Injector:
   SilverStripe\GraphQL\Scaffolding\Interfaces\TypeParserInterface.string:
@@ -30,11 +24,3 @@ SilverStripe\ORM\FieldType\DBPrimaryKey:
 SilverStripe\ORM\FieldType\DBForeignKey:
   graphql_type: ID
 
-## CORS default config
-SilverStripe\GraphQL\Controller:
-  cors:
-    Enabled: false # Off by default
-    Allow-Origin: # Deny all by default
-    Allow-Headers: 'Authorization, Content-Type'
-    Allow-Methods:  'GET, POST, OPTIONS'
-    Max-Age:  86400  # 86,400 seconds = 1 day.
diff --git a/examples/code/MemberTypeCreator.php b/examples/code/MemberTypeCreator.php
@@ -3,8 +3,8 @@
 namespace MyProject\GraphQL;
 
 use GraphQL\Type\Definition\Type;
-use SilverStripe\GraphQL\TypeCreator;
 use SilverStripe\GraphQL\Pagination\Connection;
+use SilverStripe\GraphQL\TypeCreator;
 
 class MemberTypeCreator extends TypeCreator
 {

diff --git a/examples/code/PaginatedReadMembersQueryCreator.php b/examples/code/PaginatedReadMembersQueryCreator.php
@@ -2,9 +2,9 @@
 namespace MyProject\GraphQL;
 
 use GraphQL\Type\Definition\Type;
-use SilverStripe\Security\Member;
 use SilverStripe\GraphQL\Pagination\Connection;
 use SilverStripe\GraphQL\Pagination\PaginatedQueryCreator;
+use SilverStripe\Security\Member;
 
 class PaginatedReadMembersQueryCreator extends PaginatedQueryCreator
 {

diff --git a/examples/code/Post.php b/examples/code/Post.php
@@ -2,11 +2,11 @@
 
 namespace MyProject;
 
-use SilverStripe\ORM\DataObject;
+use SilverStripe\Assets\File;
 use SilverStripe\GraphQL\Scaffolding\Interfaces\ScaffoldingProvider;
 use SilverStripe\GraphQL\Scaffolding\Scaffolders\SchemaScaffolder;
+use SilverStripe\ORM\DataObject;
 use SilverStripe\Security\Member;
-use SilverStripe\Assets\File;
 
 class Post extends DataObject implements ScaffoldingProvider
 {

diff --git a/examples/code/ReadMembersQueryCreator.php b/examples/code/ReadMembersQueryCreator.php
@@ -4,9 +4,9 @@
 
 use GraphQL\Type\Definition\ResolveInfo;
 use GraphQL\Type\Definition\Type;
-use SilverStripe\Security\Member;
 use SilverStripe\GraphQL\OperationResolver;
 use SilverStripe\GraphQL\QueryCreator;
+use SilverStripe\Security\Member;
 
 class ReadMembersQueryCreator extends QueryCreator implements OperationResolver
 {

diff --git a/src/Controller.php b/src/Controller.php
@@ -2,15 +2,16 @@
 
 namespace SilverStripe\GraphQL;
 
+use Exception;
 use SilverStripe\Control\Controller as BaseController;
+use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Core\Config\Config;
-use SilverStripe\Control\Director;
 use SilverStripe\GraphQL\Auth\Handler;
-use SilverStripe\Versioned\Versioned;
-use Exception;
+use SilverStripe\Security\Member;
 use SilverStripe\Security\Permission;
+use SilverStripe\Versioned\Versioned;
 
 /**
  * Top level controller for handling graphql requests.
@@ -19,6 +20,20 @@
  */
 class Controller extends BaseController
 {
+    /**
+     * Cors default config
+     *
+     * @config
+     * @var array
+     */
+    private static $cors = [
+        'Enabled' => false, // Off by default
+        'Allow-Origin' => [], // List of all allowed origins; Deny by default
+        'Allow-Headers' => 'Authorization, Content-Type',
+        'Allow-Methods' => 'GET, POST, OPTIONS',
+        'Max-Age' => 86400, // 86,400 seconds = 1 day.
+    ];
+
     /**
      * @var Manager
      */
@@ -39,56 +54,22 @@ public function index(HTTPRequest $request)
 
         // Check for a possible CORS preflight request and handle if necessary
         // Refer issue 66:  https://github.com/silverstripe/silverstripe-graphql/issues/66
-        $corsConfig = Config::inst()->get(self::class, 'cors');
-        $corsEnabled = true; // Default to have CORS turned on.
-
-        if ($corsConfig && isset($corsConfig['Enabled']) && !$corsConfig['Enabled']) {
-            // Dev has turned off CORS
-            $corsEnabled = false;
-        }
-        if ($corsEnabled && $request->httpMethod() == 'OPTIONS') {
-            // CORS config is enabled and the request is an OPTIONS pre-flight.
-            // Process the CORS config and add appropriate headers.
-            $response = new HTTPResponse();
-            return $this->addCorsHeaders($request, $response);
-        } elseif (!$corsEnabled && $request->httpMethod() == 'OPTIONS') {
-            // CORS is disabled but we have received an OPTIONS request.  This is not a valid request method in this
-            // situation.  Return a 405 Method Not Allowed response.
-            return $this->httpError(405, "Method Not Allowed");
-        }
-
-        $contentType = $request->getHeader('Content-Type') ?: $request->getHeader('content-type');
-        $isJson = preg_match('#^application/json\b#', $contentType);
-        if ($isJson) {
-            $rawBody = $request->getBody();
-            $data = json_decode($rawBody ?: '', true);
-            $query = isset($data['query']) ? $data['query'] : null;
-            $variables = isset($data['variables']) ? (array) $data['variables'] : null;
-        } else {
-            $query = $request->requestVar('query');
-            $variables = json_decode($request->requestVar('variables'), true);
+        if ($request->httpMethod() === 'OPTIONS') {
+            return $this->handleOptions($request);
         }
 
-        $this->setManager($manager = $this->getManager());
-
+        // Main query handling
         try {
-            // Check authentication
-            $member = $this->getAuthHandler()->requireAuthentication($request);
+            $manager = $this->getManager();
+
+            // Check and validate user for this request
+            $member = $this->getRequestUser($request);
             if ($member) {
                 $manager->setMember($member);
             }
 
-            // Check authorisation
-            $permissions = $request->param('Permissions');
-            if ($permissions) {
-                if (!$member) {
-                    throw new Exception("Authentication required");
-                }
-                $allowed = Permission::checkMember($member, $permissions);
-                if (!$allowed) {
-                    throw new Exception("Not authorised");
-                }
-            }
+            // Parse input
+            list($query, $variables) = $this->getRequestQueryVariables($request);
 
             // Run query
             $result = $manager->query($query, $variables);
@@ -123,16 +104,18 @@ public function getManager()
         // Get a service rather than an instance (to allow procedural configuration)
         $config = Config::inst()->get(static::class, 'schema');
         $manager = Manager::createFromConfig($config);
-
+        $this->setManager($manager);
         return $manager;
     }
 
     /**
      * @param Manager $manager
+     * @return $this
      */
     public function setManager($manager)
     {
         $this->manager = $manager;
+        return $this;
     }
 
     /**
@@ -155,45 +138,157 @@ public function getAuthHandler()
     public function addCorsHeaders(HTTPRequest $request, HTTPResponse $response)
     {
         $corsConfig = Config::inst()->get(static::class, 'cors');
+
+        // If CORS is disabled don't add the extra headers. Simply return the response untouched.
         if (empty($corsConfig['Enabled'])) {
-            // If CORS is disabled don't add the extra headers. Simply return the response untouched.
             return $response;
         }
 
-        // Allow Origins header.
-        if (is_string($corsConfig['Allow-Origin'])) {
-            $allowedOrigins = [$corsConfig['Allow-Origin']];
-        } else {
-            $allowedOrigins = $corsConfig['Allow-Origin'];
-        }
-        if (!empty($allowedOrigins)) {
-            $origin = $request->getHeader('Origin');
-            if ($origin) {
-                $originAuthorised = false;
-                foreach ($allowedOrigins as $allowedOrigin) {
-                    if ($allowedOrigin == $origin || $allowedOrigin === '*') {
-                        $response->addHeader("Access-Control-Allow-Origin", $origin);
-                        $originAuthorised = true;
-                        break;
-                    }
-                }
-
-                if (!$originAuthorised) {
-                    return $this->httpError(403, "Access Forbidden");
-                }
-            } else {
-                // No Origin header present in Request.
-                return $this->httpError(403, "Access Forbidden");
-            }
-        } else {
-            // No allowed origins, ergo all origins forbidden.
-            return $this->httpError(403, "Access Forbidden");
+        // Calculate origin
+        $origin = $this->getRequestOrigin($request);
+
+        // Check if valid
+        $allowedOrigins = (array)$corsConfig['Allow-Origin'];
+        $originAuthorised = $this->validateOrigin($origin, $allowedOrigins);
+        if (!$originAuthorised) {
+            $this->httpError(403, "Access Forbidden");
         }
 
+        $response->addHeader('Access-Control-Allow-Origin', $origin);
         $response->addHeader('Access-Control-Allow-Headers', $corsConfig['Allow-Headers']);
         $response->addHeader('Access-Control-Allow-Methods', $corsConfig['Allow-Methods']);
         $response->addHeader('Access-Control-Max-Age', $corsConfig['Max-Age']);
 
         return $response;
     }
+
+    /**
+     * Validate an origin matches a set of allowed origins
+     *
+     * @param string $origin Origin string
+     * @param array $allowedOrigins List of allowed origins
+     * @return bool
+     */
+    protected function validateOrigin($origin, $allowedOrigins)
+    {
+        if (empty($allowedOrigins) || empty($origin)) {
+            return false;
+        }
+        foreach ($allowedOrigins as $allowedOrigin) {
+            if ($allowedOrigin === '*') {
+                return true;
+            }
+            if (strcasecmp($allowedOrigin, $origin) === 0) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Get (or infer) value of Origin header
+     *
+     * @param HTTPRequest $request
+     * @return string|null
+     */
+    protected function getRequestOrigin(HTTPRequest $request)
+    {
+        // Prefer Origin header
+        $origin = $request->getHeader('Origin');
+        if ($origin) {
+            return $origin;
+        }
+
+        // Check referer
+        $referer = $request->getHeader('Referer');
+        if ($referer) {
+            // Extract protocol, hostname, and port
+            $refererParts = parse_url($referer);
+            if (!$refererParts) {
+                return null;
+            }
+            // Rebuild
+            $origin = $refererParts['scheme'] . '://' . $refererParts['host'];
+            if (isset($refererParts['port'])) {
+                $origin .= ':' . $refererParts['port'];
+            }
+            return $origin;
+        }
+
+        return null;
+    }
+
+    /**
+     * Response for HTTP OPTIONS request
+     *
+     * @param HTTPRequest $request
+     * @return HTTPResponse
+     */
+    protected function handleOptions(HTTPRequest $request)
+    {
+        $response = HTTPResponse::create();
+        $corsConfig = Config::inst()->get(self::class, 'cors');
+        if ($corsConfig['Enabled']) {
+            // CORS config is enabled and the request is an OPTIONS pre-flight.
+            // Process the CORS config and add appropriate headers.
+            $this->addCorsHeaders($request, $response);
+        } else {
+            // CORS is disabled but we have received an OPTIONS request.  This is not a valid request method in this
+            // situation.  Return a 405 Method Not Allowed response.
+            $this->httpError(405, "Method Not Allowed");
+        }
+        return $response;
+    }
+
+    /**
+     * Parse query and variables from the given request
+     *
+     * @param HTTPRequest $request
+     * @return array Array containing query and variables as a pair
+     */
+    protected function getRequestQueryVariables(HTTPRequest $request)
+    {
+        $contentType = $request->getHeader('content-type');
+        $isJson = preg_match('#^application/json\b#', $contentType);
+        if ($isJson) {
+            $rawBody = $request->getBody();
+            $data = json_decode($rawBody ?: '', true);
+            $query = isset($data['query']) ? $data['query'] : null;
+            $variables = isset($data['variables']) ? (array)$data['variables'] : null;
+        } else {
+            $query = $request->requestVar('query');
+            $variables = json_decode($request->requestVar('variables'), true);
+        }
+        return [$query, $variables];
+    }
+
+    /**
+     * Get user and validate for this request
+     *
+     * @param HTTPRequest $request
+     * @return Member
+     */
+    protected function getRequestUser(HTTPRequest $request)
+    {
+        // Check authentication
+        $member = $this->getAuthHandler()->requireAuthentication($request);
+
+        // Check authorisation
+        $permissions = $request->param('Permissions');
+        if (!$permissions) {
+            return $member;
+        }
+
+        // If permissions requested require authentication
+        if (!$member) {
+            throw new Exception("Authentication required");
+        }
+
+        // Check authorisation for this member
+        $allowed = Permission::checkMember($member, $permissions);
+        if (!$allowed) {
+            throw new Exception("Not authorised");
+        }
+        return $member;
+    }
 }
diff --git a/src/FieldCreator.php b/src/FieldCreator.php
@@ -2,8 +2,8 @@
 
 namespace SilverStripe\GraphQL;
 
-use SilverStripe\Core\Injector\Injectable;
 use GraphQL\Type\Definition\Type;
+use SilverStripe\Core\Injector\Injectable;
 
 /**
  * Base type for query types within graphql. I.e. mutations or queries