CrossOriginRequestPlugin - Code Metrics - Inspection of "WIP: Add support for an input stream to HttpReques..." - Vectorface/SnappyRouter - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — dev (#11)

unknown

created 2016-06-24 18:49 UTC

CrossOriginRequestPlugin A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	176
Duplicated Lines	0 %

Coupling/Cohesion

Components	1
Dependencies	5

Test Coverage

Coverage

100%

Importance

Changes	8
Bugs	0	Features	2

Metric	Value
wmc	24
c	8
b	0
f	2
lcom	1
cbo	5
dl	0
loc	176
ccs	66
cts	66
cp	1
rs	10

5 Methods

Rating	Name	Size	Complexity
B	afterHandlerSelected()	29	6
A	processRequestsForAccessDenial()	14	3
B	addHeadersForOptionsRequests()	23	4
C	isServiceEnabledForCrossOrigin()	31	8
A	isRequestCrossOrigin()	12	3

<?php

namespace Vectorface\SnappyRouter\Plugin\AccessControl;

use Vectorface\SnappyRouter\Exception\AccessDeniedException;
use Vectorface\SnappyRouter\Exception\InternalErrorException;
use Vectorface\SnappyRouter\Handler\AbstractHandler;
use Vectorface\SnappyRouter\Handler\AbstractRequestHandler;
use Vectorface\SnappyRouter\Handler\BatchRequestHandlerInterface;
use Vectorface\SnappyRouter\Plugin\AbstractPlugin;

/**
 * A plugin that adds appropriate content headers for http requests based on the response.
 * @copyright Copyright (c) 2014, VectorFace, Inc.
 * @author Dan Bruce <[email protected]>
 */
class CrossOriginRequestPlugin extends AbstractPlugin
{
    /** the PHP constant for the Origin HTTP request header */
    const HEADER_CLIENT_ORIGIN = 'HTTP_ORIGIN';

    /** The HTTP header for allowing cross origin requests */
    const HEADER_ALLOW_ORIGIN      = 'Access-Control-Allow-Origin';
    /** The HTTP header for allowing a list of headers */
    const HEADER_ALLOW_HEADERS     = 'Access-Control-Allow-Headers';
    /** The HTTP header for allowing a list of methods */
    const HEADER_ALLOW_METHODS     = 'Access-Control-Allow-Methods';
    /** The HTTP header for allowing credentials */
    const HEADER_ALLOW_CREDENTIALS = 'Access-Control-Allow-Credentials';
    /** The HTTP header for the max age to cache access control */
    const HEADER_MAX_AGE           = 'Access-Control-Max-Age';

    /** the config key for the whitelist of allowed services */
    const CONFIG_SERVICE_WHITELIST = 'whitelist';
    /** the config key for the whitelist of allowed origin domains */
    const CONFIG_ORIGIN_WHITELIST = 'ignoreOrigins';
    /** the magic config option to allow all methods on a service */
    const CONFIG_ALL_METHODS   = 'all';

    /** A constant indicating how long (in seconds) a user agent should cache
       cross origin preflight response headers */
    const MAX_AGE = 86400; // 1 day

    // the array of allowed headers the user agent can send in a cross origin request
    private static $allowedHeaders = array(
        'accept', 'content-type'
    );

    // the array of allowed HTTP verbs that can be used to perform cross origin requests
    private static $allowedMethods = array(
        'GET', 'POST', 'OPTIONS'
    );

    /**
     * Invoked directly after the router decides which handler will be used.
     * @param AbstractHandler $handler The handler selected by the router.
     */
    public function afterHandlerSelected(AbstractHandler $handler)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        parent::afterHandlerSelected($handler);
        if (false === $this->isRequestCrossOrigin() || !($handler instanceof AbstractRequestHandler)) {
            // since the request isn't cross origin we don't need this plugin to
            // do any processing at all
            return;
        }

        $request = $handler->getRequest();
        if (null === $request) {
            // the CORS plugin only supports handlers with standard requests
            return;
        }
        $requests = array($request);
        if ($handler instanceof BatchRequestHandlerInterface) {
            $requests = $handler->getRequests();
        }
        $this->processRequestsForAccessDenial($requests);

        // let the browser know this domain can make cross origin requests
        @header(self::HEADER_ALLOW_ORIGIN.': '.$_SERVER[self::HEADER_CLIENT_ORIGIN]);
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
        // do not explicitly block requests that pass a cookie
        @header(self::HEADER_ALLOW_CREDENTIALS.': true');
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}

        if ('OPTIONS' === strtoupper($request->getVerb())) {
            $this->addHeadersForOptionsRequests();
        }
    }

    /**
     * Processes the list of requests to check if any should be blocked due
     * to CORS policy.
     * @param array $requests The array of requests.
     */
    private function processRequestsForAccessDenial(array $requests)
    {
        foreach ($requests as $request) {
            $controller = $request->getController();
            $action = $request->getAction();
            if (false === $this->isServiceEnabledForCrossOrigin($controller, $action)) {
                // we have a cross origin request for a controller that's not enabled
                // so throw an exception instead of processing the request
                throw new AccessDeniedException(
                    'Cross origin access denied to '.$controller.' and action '.$action
                );
            }
        }
    }

    /**
     * Adds additional headers for the OPTIONS http verb.
     */
    private function addHeadersForOptionsRequests()
    {
        // header for preflight cache expiry
        $maxAge = self::MAX_AGE;
        if (isset($this->options[self::HEADER_MAX_AGE])) {
            $maxAge = intval($this->options[self::HEADER_MAX_AGE]);
        }
        @header(self::HEADER_MAX_AGE.': '.$maxAge);
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}

        // header for allowed request headers in cross origin requests
        $allowedHeaders = self::$allowedHeaders;
        if (isset($this->options[self::HEADER_ALLOW_HEADERS])) {
            $allowedHeaders = (array)$this->options[self::HEADER_ALLOW_HEADERS];
        }
        @header(self::HEADER_ALLOW_HEADERS.':'.implode(',', $allowedHeaders));
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}

        // header for allowed HTTP methods in cross orgin requests
        $allowedMethods = self::$allowedMethods;
        if (isset($this->options[self::HEADER_ALLOW_METHODS])) {
            $allowedMethods = (array)$this->options[self::HEADER_ALLOW_METHODS];
        }
        @header(self::HEADER_ALLOW_METHODS.':'.implode(',', $allowedMethods));
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
    }

    /**
     * Returns whether or not the current service/method combination is enabled
     * for cross origin requests.
     * @param string $service The service requested.
     * @param string|null $method The method requested.
     * @return boolean Returns true if the service/method pair is in the whitelist and
     *         false otherwise.
     */
    protected function isServiceEnabledForCrossOrigin($service, $method)
    {
        // ensure the plugin has a valid whitelist
        if (!isset($this->options[self::CONFIG_SERVICE_WHITELIST])) {
            throw new InternalErrorException(
                'Cross origin request plugin missing whitelist.'
            );
        }

        $whitelist = $this->options[self::CONFIG_SERVICE_WHITELIST];
        // check if the whitelist is the string "all"
        if (self::CONFIG_ALL_METHODS === $whitelist) {
            return true;
        }

        // ensure the whitelist is an array and the service is listed within
        // the whitelist
        if (!is_array($whitelist) || !isset($whitelist[$service])) {
            return false;
        }

        // if the service is listed and set to the string 'all' this means all
        // methods are enabled for cross origin so we're good!
        if (self::CONFIG_ALL_METHODS === $whitelist[$service] || null === $method) {
            return true;
        }

        $whitelistedServices = (is_array($whitelist[$service])) ? $whitelist[$service] : array();
        // ensure the method is listed in the list of services
        return in_array($method, $whitelistedServices);
    }

    /**
     * Returns true if the current requests is a cross origin request (i.e. does
     * the Origin HTTP header exist in the request) and false otherwise.
     * @return boolean Returns true if the request is cross origin and false otherwise.
     */
    protected function isRequestCrossOrigin()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        if (empty($_SERVER[self::HEADER_CLIENT_ORIGIN])) {
            return false;
        }

        $ignoredDomains = array();
        if (isset($this->options[self::CONFIG_ORIGIN_WHITELIST])) {
            $ignoredDomains = (array)$this->options[self::CONFIG_ORIGIN_WHITELIST];
        }
        return !in_array($_SERVER[self::HEADER_CLIENT_ORIGIN], $ignoredDomains);
    }
}


1		<?php
2
3		namespace Vectorface\SnappyRouter\Plugin\AccessControl;
4
5		use Vectorface\SnappyRouter\Exception\AccessDeniedException;
6		use Vectorface\SnappyRouter\Exception\InternalErrorException;
7		use Vectorface\SnappyRouter\Handler\AbstractHandler;
8		use Vectorface\SnappyRouter\Handler\AbstractRequestHandler;
9		use Vectorface\SnappyRouter\Handler\BatchRequestHandlerInterface;
10		use Vectorface\SnappyRouter\Plugin\AbstractPlugin;
11
12		/**
13		* A plugin that adds appropriate content headers for http requests based on the response.
14		* @copyright Copyright (c) 2014, VectorFace, Inc.
15		* @author Dan Bruce <[email protected]>
16		*/
17		class CrossOriginRequestPlugin extends AbstractPlugin
18		{
19		/** the PHP constant for the Origin HTTP request header */
20		const HEADER_CLIENT_ORIGIN = 'HTTP_ORIGIN';
21
22		/** The HTTP header for allowing cross origin requests */
23		const HEADER_ALLOW_ORIGIN = 'Access-Control-Allow-Origin';
24		/** The HTTP header for allowing a list of headers */
25		const HEADER_ALLOW_HEADERS = 'Access-Control-Allow-Headers';
26		/** The HTTP header for allowing a list of methods */
27		const HEADER_ALLOW_METHODS = 'Access-Control-Allow-Methods';
28		/** The HTTP header for allowing credentials */
29		const HEADER_ALLOW_CREDENTIALS = 'Access-Control-Allow-Credentials';
30		/** The HTTP header for the max age to cache access control */
31		const HEADER_MAX_AGE = 'Access-Control-Max-Age';
32
33		/** the config key for the whitelist of allowed services */
34		const CONFIG_SERVICE_WHITELIST = 'whitelist';
35		/** the config key for the whitelist of allowed origin domains */
36		const CONFIG_ORIGIN_WHITELIST = 'ignoreOrigins';
37		/** the magic config option to allow all methods on a service */
38		const CONFIG_ALL_METHODS = 'all';
39
40		/** A constant indicating how long (in seconds) a user agent should cache
41		cross origin preflight response headers */
42		const MAX_AGE = 86400; // 1 day
43
44		// the array of allowed headers the user agent can send in a cross origin request
45		private static $allowedHeaders = array(
46		'accept', 'content-type'
47		);
48
49		// the array of allowed HTTP verbs that can be used to perform cross origin requests
50		private static $allowedMethods = array(
51		'GET', 'POST', 'OPTIONS'
52		);
53
54		/**
55		* Invoked directly after the router decides which handler will be used.
56		* @param AbstractHandler $handler The handler selected by the router.
57		*/
58	8	public function afterHandlerSelected(AbstractHandler $handler)
		0 ignored issues – show Coding Style introduced 2016-04-26 14:02 UTC by Report Bug Copy Issue Report `afterHandlerSelected` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
59		{
60	8	parent::afterHandlerSelected($handler);
61	8	if (false === $this->isRequestCrossOrigin() \|\| !($handler instanceof AbstractRequestHandler)) {
62		// since the request isn't cross origin we don't need this plugin to
63		// do any processing at all
64	1	return;
65		}
66
67	7	$request = $handler->getRequest();
68	7	if (null === $request) {
69		// the CORS plugin only supports handlers with standard requests
70	1	return;
71		}
72	6	$requests = array($request);
73	6	if ($handler instanceof BatchRequestHandlerInterface) {
74	1	$requests = $handler->getRequests();
75	1	}
76	6	$this->processRequestsForAccessDenial($requests);
77
78		// let the browser know this domain can make cross origin requests
79	3	@header(self::HEADER_ALLOW_ORIGIN.': '.$_SERVER[self::HEADER_CLIENT_ORIGIN]);
		0 ignored issues – show Security Best Practice introduced 2016-04-26 14:02 UTC by Report Bug Copy Issue Report It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
80		// do not explicitly block requests that pass a cookie
81	3	@header(self::HEADER_ALLOW_CREDENTIALS.': true');
		0 ignored issues – show Security Best Practice introduced 2016-04-26 14:02 UTC by Report Bug Copy Issue Report It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
82
83	3	if ('OPTIONS' === strtoupper($request->getVerb())) {
84	1	$this->addHeadersForOptionsRequests();
85	1	}
86	3	}
87
88		/**
89		* Processes the list of requests to check if any should be blocked due
90		* to CORS policy.
91		* @param array $requests The array of requests.
92		*/
93	6	private function processRequestsForAccessDenial(array $requests)
94		{
95	6	foreach ($requests as $request) {
96	6	$controller = $request->getController();
97	6	$action = $request->getAction();
98	6	if (false === $this->isServiceEnabledForCrossOrigin($controller, $action)) {
99		// we have a cross origin request for a controller that's not enabled
100		// so throw an exception instead of processing the request
101	2	throw new AccessDeniedException(
102	2	'Cross origin access denied to '.$controller.' and action '.$action
103	2	);
104		}
105	3	}
106	3	}
107
108		/**
109		* Adds additional headers for the OPTIONS http verb.
110		*/
111	1	private function addHeadersForOptionsRequests()
112		{
113		// header for preflight cache expiry
114	1	$maxAge = self::MAX_AGE;
115	1	if (isset($this->options[self::HEADER_MAX_AGE])) {
116	1	$maxAge = intval($this->options[self::HEADER_MAX_AGE]);
117	1	}
118	1	@header(self::HEADER_MAX_AGE.': '.$maxAge);
		0 ignored issues – show Security Best Practice introduced 2016-04-26 14:02 UTC by Report Bug Copy Issue Report It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
119
120		// header for allowed request headers in cross origin requests
121	1	$allowedHeaders = self::$allowedHeaders;
122	1	if (isset($this->options[self::HEADER_ALLOW_HEADERS])) {
123	1	$allowedHeaders = (array)$this->options[self::HEADER_ALLOW_HEADERS];
124	1	}
125	1	@header(self::HEADER_ALLOW_HEADERS.':'.implode(',', $allowedHeaders));
		0 ignored issues – show Security Best Practice introduced 2016-04-26 14:02 UTC by Report Bug Copy Issue Report It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
126
127		// header for allowed HTTP methods in cross orgin requests
128	1	$allowedMethods = self::$allowedMethods;
129	1	if (isset($this->options[self::HEADER_ALLOW_METHODS])) {
130	1	$allowedMethods = (array)$this->options[self::HEADER_ALLOW_METHODS];
131	1	}
132	1	@header(self::HEADER_ALLOW_METHODS.':'.implode(',', $allowedMethods));
		0 ignored issues – show Security Best Practice introduced 2016-04-26 14:02 UTC by Report Bug Copy Issue Report It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
133	1	}
134
135		/**
136		* Returns whether or not the current service/method combination is enabled
137		* for cross origin requests.
138		* @param string $service The service requested.
139		* @param string\|null $method The method requested.
140		* @return boolean Returns true if the service/method pair is in the whitelist and
141		* false otherwise.
142		*/
143	6	protected function isServiceEnabledForCrossOrigin($service, $method)
144		{
145		// ensure the plugin has a valid whitelist
146	6	if (!isset($this->options[self::CONFIG_SERVICE_WHITELIST])) {
147	1	throw new InternalErrorException(
148		'Cross origin request plugin missing whitelist.'
149	1	);
150		}
151
152	5	$whitelist = $this->options[self::CONFIG_SERVICE_WHITELIST];
153		// check if the whitelist is the string "all"
154	5	if (self::CONFIG_ALL_METHODS === $whitelist) {
155	1	return true;
156		}
157
158		// ensure the whitelist is an array and the service is listed within
159		// the whitelist
160	4	if (!is_array($whitelist) \|\| !isset($whitelist[$service])) {
161	1	return false;
162		}
163
164		// if the service is listed and set to the string 'all' this means all
165		// methods are enabled for cross origin so we're good!
166	3	if (self::CONFIG_ALL_METHODS === $whitelist[$service] \|\| null === $method) {
167	1	return true;
168		}
169
170	2	$whitelistedServices = (is_array($whitelist[$service])) ? $whitelist[$service] : array();
171		// ensure the method is listed in the list of services
172	2	return in_array($method, $whitelistedServices);
173		}
174
175		/**
176		* Returns true if the current requests is a cross origin request (i.e. does
177		* the Origin HTTP header exist in the request) and false otherwise.
178		* @return boolean Returns true if the request is cross origin and false otherwise.
179		*/
180	8	protected function isRequestCrossOrigin()
		0 ignored issues – show Coding Style introduced 2016-04-26 14:02 UTC by Report Bug Copy Issue Report `isRequestCrossOrigin` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
181		{
182	8	if (empty($_SERVER[self::HEADER_CLIENT_ORIGIN])) {
183	1	return false;
184		}
185
186	8	$ignoredDomains = array();
187	8	if (isset($this->options[self::CONFIG_ORIGIN_WHITELIST])) {
188	2	$ignoredDomains = (array)$this->options[self::CONFIG_ORIGIN_WHITELIST];
189	2	}
190	8	return !in_array($_SERVER[self::HEADER_CLIENT_ORIGIN], $ignoredDomains);
191		}
192		}
193

Vectorface / SnappyRouter

Pull Request — dev (#11)

CrossOriginRequestPlugin A

Complexity

Size/Duplication

Coupling/Cohesion

Test Coverage

Importance

5 Methods

Duplication Side-by-Side

Filter issues like