From 9cb316bdfac962f1cf5217162c5bf00a6b3feef4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 1 Aug 2019 18:27:43 +0200 Subject: [PATCH] Added more headers to inspect while looking for the remote IP address --- config/autoload/ip-address.global.php | 19 +++++++ .../Middleware/IpAddressMiddlewareFactory.php | 4 +- .../IpAddressMiddlewareFactoryTest.php | 54 +++++++++++++++++-- 3 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 config/autoload/ip-address.global.php diff --git a/config/autoload/ip-address.global.php b/config/autoload/ip-address.global.php new file mode 100644 index 00000000..2def62fd --- /dev/null +++ b/config/autoload/ip-address.global.php @@ -0,0 +1,19 @@ + [ + 'headers_to_inspect' => [ + 'Forwarded', + 'X-Forwarded-For', + 'X-Forwarded', + 'X-Cluster-Client-Ip', + 'Client-Ip', + 'X-Real-IP', + 'CF-Connecting-IP', + 'True-Client-IP', + ], + ], + +]; diff --git a/module/Common/src/Middleware/IpAddressMiddlewareFactory.php b/module/Common/src/Middleware/IpAddressMiddlewareFactory.php index b9cd3879..2116d231 100644 --- a/module/Common/src/Middleware/IpAddressMiddlewareFactory.php +++ b/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); } } diff --git a/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php b/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php index 41fbafbb..4a3361bd 100644 --- a/module/Common/test/Middleware/IpAddressMiddlewareFactoryTest.php +++ b/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', + ]]; } }