IpMiddleware::__construct() - Code Metrics - Inspection of "Include middlewares from config. Fix cache middlew..." - locomotivemtl/charcoal-app - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 700921...f4d17a )

by Mathieu

created 2017-06-20 19:43 UTC

IpMiddleware::__construct() A

↳ Parent: IpMiddleware

Complexity

Conditions	1
Paths	1

Size

Total Lines	21
Code Lines	13

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	21
rs	9.3142
c	0
b	0
f	0
cc	1
eloc	13
nc	1
nop	1

<?php

namespace Charcoal\App\Middleware;

// Dependencies from 'PSR-7' (HTTP Messaging)
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)
    {
        $defaults = [
            'blacklist' => null,
            'whitelist' => null,

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

            'fail_on_invalid_ip' => false
        ];
        $data = array_merge($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'];
    }

    /**
     * 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		// Dependencies from 'PSR-7' (HTTP Messaging)
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		$defaults = [
45		'blacklist' => null,
46		'whitelist' => null,
47
48		'blacklisted_redirect' => null,
49		'not_whitelisted_redirect' => null,
50
51		'fail_on_invalid_ip' => false
52		];
53		$data = array_merge($defaults, $data);
54
55		$this->blacklist = $data['blacklist'];
56		$this->whitelist = $data['whitelist'];
57
58		$this->blacklistedRedirect = $data['blacklisted_redirect'];
59		$this->notWhitelistedRedirect = $data['not_whitelisted_redirect'];
60
61		$this->failOnInvalidIp = $data['fail_on_invalid_ip'];
62		}
63
64		/**
65		* Load a route content from path's cache.
66		*
67		* This method is as dumb / simple as possible.
68		* It does not rely on any sort of settings / configuration.
69		* Simply: if the cache for the route exists, it will be used to display the page.
70		* The `$next` callback will not be called, therefore stopping the middleware stack.
71		*
72		* To generate the cache used in this middleware, @see \Charcoal\App\Middleware\CacheGeneratorMiddleware.
73		*
74		* @param RequestInterface $request The PSR-7 HTTP request.
75		* @param ResponseInterface $response The PSR-7 HTTP response.
76		* @param callable $next The next middleware callable in the stack.
77		* @return ResponseInterface
78		*/
79		public function __invoke(RequestInterface $request, ResponseInterface $response, callable $next)
80		{
81		$ip = $this->getClientIp($request);
82		if (!$ip) {
83		if ((!empty($this->blacklist) \|\| !empty($this->whitelist)) && $this->failOnInvalidIp === true) {
84		return $response->withStatus(403);
85		} else {
86		return $next($request, $response);
87		}
88		}
89
90		// Check blacklist.
91	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...
92		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...
93		return $response
94		->withStatus(302)
95		->withHeader('Location', $this->blacklistedRedirect);
96		} else {
97		// IP explicitely blacklisted: forbidden
98		return $response->withStatus(403);
99		}
100		}
101
102		// Check whitelist.
103	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...
104		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...
105		return $response
106		->withStatus(302)
107		->withHeader('Location', $this->blacklistedRedirect);
108		} else {
109		// IP not whistelisted: forbidden
110		return $response->withStatus(403);
111		}
112		}
113
114		// If here, not blacklisted or not-whitelisted; continue as normal.
115		return $next($request, $response);
116		}
117
118		/**
119		* Check wether a certain IP is explicitely blacklisted.
120		*
121		* If the blacklist is null or empty, then nothing is ever blacklisted (return false).
122		*
123		* Note: this method only performs an exact string match on IP address, no IP masking / range features.
124		*
125		* @param string $ip The IP address to check against the blacklist.
126		* @return boolean
127		*/
128		private function isIpBlacklisted($ip)
129		{
130		if (empty($this->blacklist)) {
131		return false;
132		}
133		return $this->isIpInRange($ip, $this->blacklist);
134		}
135
136		/**
137		* Check wether a certain IP is explicitely whitelisted.
138		*
139		* If the whitelist is null or empty, then all IPs are whitelisted (return true).
140		*
141		* Note; This method only performs an exact string match on IP address, no IP masking / range features.
142		*
143		* @param string $ip The IP address to check against the whitelist.
144		* @return boolean
145		*/
146		private function isIpWhitelisted($ip)
147		{
148		if (empty($this->whitelist)) {
149		return true;
150		}
151		return $this->isIpInRange($ip, $this->whitelist);
152		}
153
154
155		/**
156		* @param string $ip The IP to check.
157		* @param string[] $cidrs The array of IPs/CIDRs to validate against. "/32" netmask is assumed.
158		* @return boolean
159		*/
160		private function isIpInRange($ip, array $cidrs)
161		{
162		foreach ($cidrs as $range) {
163		if (strpos($range, '/') === false) {
164		$range .= '/32';
165		}
166		// $range is in IP/CIDR format eg 127.0.0.1/24
167		list($subnet, $netmask) = explode('/', $range, 2);
168		$netmask = ~(pow(2, (32 - $netmask)) - 1);
169		if ((ip2long($ip) & $netmask) == (ip2long($subnet) & $netmask)) {
170		return true;
171		}
172		}
173		// Nothing matched
174		return false;
175		}
176
177		/**
178		* @param RequestInterface $request The PSR-7 HTTP request.
179		* @return string
180		*/
181		private function getClientIp(RequestInterface $request)
		0 ignored issues – show Unused Code introduced 2017-06-20 19:53 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-06-20 19:53 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...
182		{
183		if (isset($_SERVER['REMOTE_ADDR'])) {
184		return filter_var($_SERVER['REMOTE_ADDR'], FILTER_VALIDATE_IP);
185		}
186		return '';
187		}
188		}
189

locomotivemtl / charcoal-app

Push — master ( 700921...f4d17a )

IpMiddleware::__construct() A

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like