Merge pull request #6 from comicrelief/customise-forwarded-order

Add 'forwarded_ip_index' option

README.md

@@ -48,6 +48,7 @@ php composer.phar require los/los-rate-limit
         'X-Forwarded',
         'X-Forwarded-For',
     ],
+    'forwarded_ip_index' => null,
 ]
 ```
 
@@ -57,8 +58,9 @@ php composer.phar require los/los-rate-limit
 * `ip_reset_time` After how many seconds the counter will be reset (using remote IP Key)
 * `api_header` Header name to get the api key from.
 * `trust_forwarded` If the X-Forwarded (and similar) headers and be trusted. If not, only $_SERVER['REMOTE_ADDR'] will be used.
-* `prefer_forwarded` Whether forwarded headers should be used in preference to the remote address, e.g. if all requests are forwarded through a routing component which adds these headers.
+* `prefer_forwarded` Whether forwarded headers should be used in preference to the remote address, e.g. if all requests are forwarded through a routing component or reverse proxy which adds these headers predictably. This is a bad idea unless your app can **only** be reached this way.
 * `forwarded_headers_allowed` An array of strings which are headers you trust to contain source IP addresses.
+* `forwarded_ip_index` If null (default), the first plausible IP in an XFF header (reading left to right) is used. If numeric, only a specific index of IP is used. Use `-2` to get the penultimate IP from the list, which could make sense if the header always ends `...<client_ip>, <router_ip>`. Or use `0` to use only the first IP (stopping if it's not valid). Like `prefer_forwarded`, this only makes sense if your app's always reached through a predictable hop that controls the header - remember these are easily spoofed on the initial request.
 
 The values above indicate that the user can trigger 100 requests per hour.
 

src/RateLimit.php

@@ -50,6 +50,7 @@ public function __construct(StorageInterface $storage, $config)
                 'X-Forwarded',
                 'X-Forwarded-For',
             ],
+            'forwarded_ip_index' => null,
         ], $config);
 
         if ($this->options['prefer_forwarded'] && !$this->options['trust_forwarded']) {
@@ -60,7 +61,7 @@ public function __construct(StorageInterface $storage, $config)
     private function getClientIp(ServerRequestInterface $request)
     {
         $server = $request->getServerParams();
-        if (!empty($server['REMOTE_ADDR']) && filter_var($server['REMOTE_ADDR'], FILTER_VALIDATE_IP)) {
+        if (!empty($server['REMOTE_ADDR']) && $this->isIp($server['REMOTE_ADDR'])) {
             $realIp = $server['REMOTE_ADDR'];
 
             if (!$this->options['prefer_forwarded']) {
@@ -70,22 +71,33 @@ private function getClientIp(ServerRequestInterface $request)
         }
 
         if ($this->options['trust_forwarded']) {
+            // At this point, we either couldn't find a real IP or prefer_forwarded ones.
             foreach ($this->options['forwarded_headers_allowed'] as $name) {
                 $header = $request->getHeaderLine($name);
                 if (!empty($header)) {
-                    foreach (array_map('trim', explode(',', $header)) as $ip) {
-                        if (filter_var($ip, FILTER_VALIDATE_IP)) {
-                            // If we got this far, we always favour the first forwarded match. And either we're
-                            // preferring forwarded IPs or didn't have a real one.
-                            return $ip;
+                    /** @var string[] $ips Possible IPs, verbatim from the forwarded header */
+                    $ips = array_map('trim', explode(',', $header));
+
+                    if ($this->options['forwarded_ip_index'] === null) {
+                        // Permit any IP in this header regardless of position, as long as it's a plausible format.
+                        foreach ($ips as $ip) {
+                            if ($this->isIp($ip)) {
+                                return $ip;
+                            }
                         }
+                    } else {
+                        // Permit only an IP at the configured index / position.
+                        $ip = array_slice($ips, (int) $this->options['forwarded_ip_index'], 1)[0];
+                        if ($this->isIp($ip)) {
+                            return $ip;
+                        } // else there may be other permitted header keys to check
                     }
                 }
             }
         }
 
         if (isset($realIp)) {
-            // We waited in order to 'prefer_forwarded', but only a direct IP was set.
+            // We waited in order to 'prefer_forwarded', but no acceptable forwarded option was found.
             return $realIp;
         }
 
@@ -159,4 +171,13 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res
 
         return $response;
     }
+
+    /**
+     * @param string $possibleIp
+     * @return bool Whether the given string is in correct format for an IP address
+     */
+    private function isIp($possibleIp)
+    {
+        return (filter_var($possibleIp, FILTER_VALIDATE_IP) !== false);
+    }
 }

test/RateLimitTest.php

@@ -36,6 +36,7 @@ protected function setUp()
                     'X-Forwarded',
                     'X-Forwarded-For',
                 ],
+                'forwarded_ip_index' => null,
             ],
         ]);
         //$factory = new RateLimitFactory();