Merge pull request #432 from acelaya/feature/extended-ip-addresses

Feature/extended ip addresses

CHANGELOG.md

@@ -33,6 +33,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
     Sadly, this feature is not enabled when serving shlink via apache/nginx, where you should still rely on cronjobs.
 
+* [#384](https://github.com/shlinkio/shlink/issues/384) Improved how remote IP addresses are detected.
+
+    This new set of headers is now also inspected looking for the IP address:
+
+    * CF-Connecting-IP
+    * True-Client-IP
+    * X-Real-IP
+
 #### Changed
 
 * *Nothing*
@@ -49,6 +57,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 * [#416](https://github.com/shlinkio/shlink/issues/416) Fixed error thrown when trying to locate visits after the GeoLite2 DB is downloaded for the first time.
 * [#424](https://github.com/shlinkio/shlink/issues/424) Update wkhtmltoimage to version 0.12.5
+* [#427](https://github.com/shlinkio/shlink/issues/427) Fixed shlink being unusable after a database error on swoole contexts.
 
 
 ## 1.17.0 - 2019-05-13

config/autoload/ip-address.global.php

@@ -0,0 +1,19 @@
+<?php
+declare(strict_types=1);
+
+return [
+
+    'ip_address_resolution' => [
+        'headers_to_inspect' => [
+            'CF-Connecting-IP',
+            'True-Client-IP',
+            'X-Real-IP',
+            'Forwarded',
+            'X-Forwarded-For',
+            'X-Forwarded',
+            'X-Cluster-Client-Ip',
+            'Client-Ip',
+        ],
+    ],
+
+];

module/Common/src/Middleware/IpAddressMiddlewareFactory.php

@@ -23,6 +23,8 @@ class IpAddressMiddlewareFactory implements FactoryInterface
      */
     public function __invoke(ContainerInterface $container, $requestedName, array $options = null): IpAddress
     {
-        return new IpAddress(true, [], Visitor::REMOTE_ADDRESS_ATTR);
+        $config = $container->get('config');
+        $headersToInspect = $config['ip_address_resolution']['headers_to_inspect'] ?? [];
+        return new IpAddress(true, [], Visitor::REMOTE_ADDRESS_ATTR, $headersToInspect);
     }
 }

module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php

@@ -18,10 +18,15 @@ class IpAddressMiddlewareFactoryTest extends TestCase
         $this->factory = new IpAddressMiddlewareFactory();
     }
 
-    /** @test */
+    /**
-    public function returnedInstanceIsProperlyConfigured()
+     * @test
+     * @dataProvider provideConfigs
+     */
+    public function returnedInstanceIsProperlyConfigured(array $config, array $expectedHeadersToInspect): void
     {
-        $instance = $this->factory->__invoke(new ServiceManager(), '');
+        $instance = ($this->factory)(new ServiceManager(['services' => [
+            'config' => $config,
+        ]]), '');
 
         $ref = new ReflectionObject($instance);
         $checkProxyHeaders = $ref->getProperty('checkProxyHeaders');
@@ -30,9 +35,52 @@ class IpAddressMiddlewareFactoryTest extends TestCase
         $trustedProxies->setAccessible(true);
         $attributeName = $ref->getProperty('attributeName');
         $attributeName->setAccessible(true);
+        $headersToInspect = $ref->getProperty('headersToInspect');
+        $headersToInspect->setAccessible(true);
 
         $this->assertTrue($checkProxyHeaders->getValue($instance));
         $this->assertEquals([], $trustedProxies->getValue($instance));
         $this->assertEquals(Visitor::REMOTE_ADDRESS_ATTR, $attributeName->getValue($instance));
+        $this->assertEquals($expectedHeadersToInspect, $headersToInspect->getValue($instance));
+    }
+
+    public function provideConfigs(): iterable
+    {
+        $defaultHeadersToInspect = [
+            'Forwarded',
+            'X-Forwarded-For',
+            'X-Forwarded',
+            'X-Cluster-Client-Ip',
+            'Client-Ip',
+        ];
+
+        yield 'no ip_address_resolution config' => [[], $defaultHeadersToInspect];
+        yield 'no headers_to_inspect config' => [['ip_address_resolution' => []], $defaultHeadersToInspect];
+        yield 'null headers_to_inspect' => [['ip_address_resolution' => [
+            'headers_to_inspect' => null,
+        ]], $defaultHeadersToInspect];
+        yield 'empty headers_to_inspect' => [['ip_address_resolution' => [
+            'headers_to_inspect' => [],
+        ]], $defaultHeadersToInspect];
+        yield 'some headers_to_inspect' => [['ip_address_resolution' => [
+            'headers_to_inspect' => [
+                'foo',
+                'bar',
+                'baz',
+            ],
+        ]], [
+            'foo',
+            'bar',
+            'baz',
+        ]];
+        yield 'some other headers_to_inspect' => [['ip_address_resolution' => [
+            'headers_to_inspect' => [
+                'something',
+                'something_else',
+            ],
+        ]], [
+            'something',
+            'something_else',
+        ]];
     }
 }