IpMiddleware::defaults() - Code Metrics - Inspection of "Cleanup CacheConfig, CacheMiddleware, IpMiddleware" - locomotivemtl/charcoal-app - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#5)

by Chauncey

created 2017-09-19 16:01 UTC

IpMiddleware::defaults() A

↳ Parent: IpMiddleware

Complexity

Conditions	1
Paths	1

Size

Total Lines	12
Code Lines	7

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	12
rs	9.4285
c	0
b	0
f	0
cc	1
eloc	7
nc	1
nop	0

<?php

namespace Charcoal\App\Middleware;

// From PSR-7
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;

/**
 * The IP middleware can restrict access to certain routes to certain IP.
 */
class IpMiddleware
{
    /**
     * @var array|null
     */
    private $blacklist;

    /**
     * @var array|null
     */
    private $whitelist;

    /**
     * @var string|null
     */
    private $blacklistedRedirect;

    /**
     * @var string|null
     */
    private $notWhitelistedRedirect;

    /**
     * @var boolean
     */
    private $failOnInvalidIp;

    /**
     * @param array $data Constructor dependencies and options.
     */
    public function __construct(array $data)
    {
        $data = array_merge($this->defaults(), $data);

        $this->blacklist = $data['blacklist'];
        $this->whitelist = $data['whitelist'];

        $this->blacklistedRedirect    = $data['blacklisted_redirect'];
        $this->notWhitelistedRedirect = $data['not_whitelisted_redirect'];

        $this->failOnInvalidIp = $data['fail_on_invalid_ip'];
    }

    /**
     * Default middleware options.
     *
     * @return array
     */
    public function defaults()
    {
        return [
            'blacklist' => null,
            'whitelist' => null,

            'blacklisted_redirect'     => null,
            'not_whitelisted_redirect' => null,

            'fail_on_invalid_ip' => false
        ];
    }

    /**
     * Load a route content from path's cache.
     *
     * This method is as dumb / simple as possible.
     * It does not rely on any sort of settings / configuration.
     * Simply: if the cache for the route exists, it will be used to display the page.
     * The `$next` callback will not be called, therefore stopping the middleware stack.
     *
     * To generate the cache used in this middleware, @see \Charcoal\App\Middleware\CacheGeneratorMiddleware.
     *
     * @param RequestInterface  $request  The PSR-7 HTTP request.
     * @param ResponseInterface $response The PSR-7 HTTP response.
     * @param callable          $next     The next middleware callable in the stack.
     * @return ResponseInterface
     */
    public function __invoke(RequestInterface $request, ResponseInterface $response, callable $next)
    {
        $ip = $this->getClientIp($request);
        if (!$ip) {
            if ((!empty($this->blacklist) || !empty($this->whitelist)) && $this->failOnInvalidIp === true) {
                return $response->withStatus(403);
            } else {
                return $next($request, $response);
            }
        }

        // Check blacklist.
        if ($this->isIpBlacklisted($ip) === true) {

            if ($this->blacklistedRedirect) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
                return $response
                    ->withStatus(302)
                    ->withHeader('Location', $this->blacklistedRedirect);
            } else {
                // IP explicitely blacklisted: forbidden
                return $response->withStatus(403);
            }
        }

        // Check whitelist.
        if ($this->isIpWhitelisted($ip) === false) {

            if ($this->notWhitelistedRedirect) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
                return $response
                    ->withStatus(302)
                    ->withHeader('Location', $this->blacklistedRedirect);
            } else {
                // IP not whistelisted: forbidden
                return $response->withStatus(403);
            }
        }

        // If here, not blacklisted or not-whitelisted; continue as normal.
        return $next($request, $response);
    }

    /**
     * Check wether a certain IP is explicitely blacklisted.
     *
     * If the blacklist is null or empty, then nothing is ever blacklisted (return false).
     *
     * Note: this method only performs an exact string match on IP address, no IP masking / range features.
     *
     * @param string $ip The IP address to check against the blacklist.
     * @return boolean
     */
    private function isIpBlacklisted($ip)
    {
        if (empty($this->blacklist)) {
            return false;
        }
        return $this->isIpInRange($ip, $this->blacklist);
    }

    /**
     * Check wether a certain IP is explicitely whitelisted.
     *
     * If the whitelist is null or empty, then all IPs are whitelisted (return true).
     *
     * Note; This method only performs an exact string match on IP address, no IP masking / range features.
     *
     * @param string $ip The IP address to check against the whitelist.
     * @return boolean
     */
    private function isIpWhitelisted($ip)
    {
        if (empty($this->whitelist)) {
            return true;
        }
        return $this->isIpInRange($ip, $this->whitelist);
    }


    /**
     * @param string   $ip    The IP to check.
     * @param string[] $cidrs The array of IPs/CIDRs to validate against. "/32" netmask is assumed.
     * @return boolean
     */
    private function isIpInRange($ip, array $cidrs)
    {
        foreach ($cidrs as $range) {
            if (strpos($range, '/') === false) {
                $range .= '/32';
            }
            // $range is in IP/CIDR format eg 127.0.0.1/24
            list($subnet, $netmask) = explode('/', $range, 2);
            $netmask = ~(pow(2, (32 - $netmask)) - 1);
            if ((ip2long($ip) & $netmask) == (ip2long($subnet) & $netmask)) {
                return true;
            }
        }
        // Nothing matched
        return false;
    }

    /**
     * @param RequestInterface $request The PSR-7 HTTP request.
     * @return string
     */
    private function getClientIp(RequestInterface $request)
// 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 (isset($_SERVER['REMOTE_ADDR'])) {
            return filter_var($_SERVER['REMOTE_ADDR'], FILTER_VALIDATE_IP);
        }
        return '';
    }
}


1		<?php
2
3		namespace Charcoal\App\Middleware;
4
5		// From PSR-7
6		use Psr\Http\Message\RequestInterface;
7		use Psr\Http\Message\ResponseInterface;
8
9		/**
10		* The IP middleware can restrict access to certain routes to certain IP.
11		*/
12		class IpMiddleware
13		{
14		/**
15		* @var array\|null
16		*/
17		private $blacklist;
18
19		/**
20		* @var array\|null
21		*/
22		private $whitelist;
23
24		/**
25		* @var string\|null
26		*/
27		private $blacklistedRedirect;
28
29		/**
30		* @var string\|null
31		*/
32		private $notWhitelistedRedirect;
33
34		/**
35		* @var boolean
36		*/
37		private $failOnInvalidIp;
38
39		/**
40		* @param array $data Constructor dependencies and options.
41		*/
42		public function __construct(array $data)
43		{
44		$data = array_merge($this->defaults(), $data);
45
46		$this->blacklist = $data['blacklist'];
47		$this->whitelist = $data['whitelist'];
48
49		$this->blacklistedRedirect = $data['blacklisted_redirect'];
50		$this->notWhitelistedRedirect = $data['not_whitelisted_redirect'];
51
52		$this->failOnInvalidIp = $data['fail_on_invalid_ip'];
53		}
54
55		/**
56		* Default middleware options.
57		*
58		* @return array
59		*/
60		public function defaults()
61		{
62		return [
63		'blacklist' => null,
64		'whitelist' => null,
65
66		'blacklisted_redirect' => null,
67		'not_whitelisted_redirect' => null,
68
69		'fail_on_invalid_ip' => false
70		];
71		}
72
73		/**
74		* Load a route content from path's cache.
75		*
76		* This method is as dumb / simple as possible.
77		* It does not rely on any sort of settings / configuration.
78		* Simply: if the cache for the route exists, it will be used to display the page.
79		* The `$next` callback will not be called, therefore stopping the middleware stack.
80		*
81		* To generate the cache used in this middleware, @see \Charcoal\App\Middleware\CacheGeneratorMiddleware.
82		*
83		* @param RequestInterface $request The PSR-7 HTTP request.
84		* @param ResponseInterface $response The PSR-7 HTTP response.
85		* @param callable $next The next middleware callable in the stack.
86		* @return ResponseInterface
87		*/
88		public function __invoke(RequestInterface $request, ResponseInterface $response, callable $next)
89		{
90		$ip = $this->getClientIp($request);
91		if (!$ip) {
92		if ((!empty($this->blacklist) \|\| !empty($this->whitelist)) && $this->failOnInvalidIp === true) {
93		return $response->withStatus(403);
94		} else {
95		return $next($request, $response);
96		}
97		}
98
99		// Check blacklist.
100	View Code Duplication	if ($this->isIpBlacklisted($ip) === true) {
		0 ignored issues – show Duplication introduced 2017-06-20 19:53 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
101		if ($this->blacklistedRedirect) {
		0 ignored issues – show Bug Best Practice introduced 2017-06-20 19:53 UTC by Report Bug Copy Issue Report The expression `$this->blacklistedRedirect` of type `string\|null` is loosely compared to `true`; this is ambiguous if the string can be empty. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
102		return $response
103		->withStatus(302)
104		->withHeader('Location', $this->blacklistedRedirect);
105		} else {
106		// IP explicitely blacklisted: forbidden
107		return $response->withStatus(403);
108		}
109		}
110
111		// Check whitelist.
112	View Code Duplication	if ($this->isIpWhitelisted($ip) === false) {
		0 ignored issues – show Duplication introduced 2017-06-20 19:53 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
113		if ($this->notWhitelistedRedirect) {
		0 ignored issues – show Bug Best Practice introduced 2017-06-20 19:53 UTC by Report Bug Copy Issue Report The expression `$this->notWhitelistedRedirect` of type `string\|null` is loosely compared to `true`; this is ambiguous if the string can be empty. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
114		return $response
115		->withStatus(302)
116		->withHeader('Location', $this->blacklistedRedirect);
117		} else {
118		// IP not whistelisted: forbidden
119		return $response->withStatus(403);
120		}
121		}
122
123		// If here, not blacklisted or not-whitelisted; continue as normal.
124		return $next($request, $response);
125		}
126
127		/**
128		* Check wether a certain IP is explicitely blacklisted.
129		*
130		* If the blacklist is null or empty, then nothing is ever blacklisted (return false).
131		*
132		* Note: this method only performs an exact string match on IP address, no IP masking / range features.
133		*
134		* @param string $ip The IP address to check against the blacklist.
135		* @return boolean
136		*/
137		private function isIpBlacklisted($ip)
138		{
139		if (empty($this->blacklist)) {
140		return false;
141		}
142		return $this->isIpInRange($ip, $this->blacklist);
143		}
144
145		/**
146		* Check wether a certain IP is explicitely whitelisted.
147		*
148		* If the whitelist is null or empty, then all IPs are whitelisted (return true).
149		*
150		* Note; This method only performs an exact string match on IP address, no IP masking / range features.
151		*
152		* @param string $ip The IP address to check against the whitelist.
153		* @return boolean
154		*/
155		private function isIpWhitelisted($ip)
156		{
157		if (empty($this->whitelist)) {
158		return true;
159		}
160		return $this->isIpInRange($ip, $this->whitelist);
161		}
162
163
164		/**
165		* @param string $ip The IP to check.
166		* @param string[] $cidrs The array of IPs/CIDRs to validate against. "/32" netmask is assumed.
167		* @return boolean
168		*/
169		private function isIpInRange($ip, array $cidrs)
170		{
171		foreach ($cidrs as $range) {
172		if (strpos($range, '/') === false) {
173		$range .= '/32';
174		}
175		// $range is in IP/CIDR format eg 127.0.0.1/24
176		list($subnet, $netmask) = explode('/', $range, 2);
177		$netmask = ~(pow(2, (32 - $netmask)) - 1);
178		if ((ip2long($ip) & $netmask) == (ip2long($subnet) & $netmask)) {
179		return true;
180		}
181		}
182		// Nothing matched
183		return false;
184		}
185
186		/**
187		* @param RequestInterface $request The PSR-7 HTTP request.
188		* @return string
189		*/
190		private function getClientIp(RequestInterface $request)
		0 ignored issues – show Unused Code introduced 2017-08-10 20:11 UTC by Report Bug Copy Issue Report The parameter `$request` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history... Coding Style introduced 2017-08-10 20:11 UTC by Report Bug Copy Issue Report `getClientIp` 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...
191		{
192		if (isset($_SERVER['REMOTE_ADDR'])) {
193		return filter_var($_SERVER['REMOTE_ADDR'], FILTER_VALIDATE_IP);
194		}
195		return '';
196		}
197		}
198

locomotivemtl / charcoal-app

Pull Request — master (#5)

IpMiddleware::defaults() A

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like