From 43137d65b0988b2e8f4cac7bbc9afd42b10cdf6b Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 26 Oct 2022 17:52:48 +0200 Subject: [PATCH 1/4] IBX-4046: Allow custom header name to be used in reverse proxy env --- .../TrustedHeaderClientIpEventSubscriber.php | 67 +++++++ .../Resources/config/default_settings.yml | 2 + src/bundle/Core/Resources/config/services.yml | 6 + ...ustedHeaderClientIpEventSubscriberTest.php | 173 ++++++++++++++++++ 4 files changed, 248 insertions(+) create mode 100644 src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php create mode 100644 tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php diff --git a/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php b/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php new file mode 100644 index 0000000000..e7bc5240d9 --- /dev/null +++ b/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php @@ -0,0 +1,67 @@ +trustedHeaderName = $trustedHeaderName; + } + + public static function getSubscribedEvents(): array + { + return [ + KernelEvents::REQUEST => ['onKernelRequest', PHP_INT_MAX], + ]; + } + + public function onKernelRequest(RequestEvent $event): void + { + $request = $event->getRequest(); + + $trustedProxies = Request::getTrustedProxies(); + $trustedHeaderSet = Request::getTrustedHeaderSet(); + + $trustedHeaderName = $this->trustedHeaderName; + if (null === $trustedHeaderName && $this->isPlatformShProxy($request)) { + $trustedHeaderName = self::PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP; + } + + if (null === $trustedHeaderName) { + return; + } + + $trustedClientIp = $request->headers->get($trustedHeaderName); + + if (null !== $trustedClientIp) { + if ($trustedHeaderSet !== -1) { + $trustedHeaderSet |= Request::HEADER_X_FORWARDED_FOR; + } + $request->headers->set('X_FORWARDED_FOR', $trustedClientIp); + } + + Request::setTrustedProxies($trustedProxies, $trustedHeaderSet); + } + + private function isPlatformShProxy(Request $request): bool + { + return null !== $request->server->get('PLATFORM_RELATIONSHIPS'); + } +} diff --git a/src/bundle/Core/Resources/config/default_settings.yml b/src/bundle/Core/Resources/config/default_settings.yml index 4ad50ca8db..c97f1a1fa3 100644 --- a/src/bundle/Core/Resources/config/default_settings.yml +++ b/src/bundle/Core/Resources/config/default_settings.yml @@ -2,6 +2,8 @@ parameters: # Kernel related params webroot_dir: "%kernel.project_dir%/public" + trusted_header_client_ip_name: ~ + ### # ibexa.site_access.config namespace, default scope ### diff --git a/src/bundle/Core/Resources/config/services.yml b/src/bundle/Core/Resources/config/services.yml index edb5e9f974..938ff3ca3b 100644 --- a/src/bundle/Core/Resources/config/services.yml +++ b/src/bundle/Core/Resources/config/services.yml @@ -294,6 +294,12 @@ services: tags: - {name: kernel.event_subscriber} + Ibexa\Bundle\Core\EventSubscriber\TrustedHeaderClientIpEventSubscriber: + arguments: + $trustedHeaderName: '%trusted_header_client_ip_name%' + tags: + - {name: kernel.event_subscriber} + Ibexa\Bundle\Core\Command\DeleteContentTranslationCommand: class: Ibexa\Bundle\Core\Command\DeleteContentTranslationCommand arguments: diff --git a/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php b/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php new file mode 100644 index 0000000000..56a7e44d17 --- /dev/null +++ b/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php @@ -0,0 +1,173 @@ +originalRemoteAddr = $_SERVER['REMOTE_ADDR'] ?? null; + } + + protected function setUp(): void + { + $_SERVER['REMOTE_ADDR'] = null; + Request::setTrustedProxies([], -1); + } + + protected function tearDown(): void + { + $_SERVER['REMOTE_ADDR'] = $this->originalRemoteAddr; + } + + public function getTrustedHeaderEventSubscriberTestData(): array + { + return [ + 'default behaviour' => [ + self::REAL_CLIENT_IP, + self::REAL_CLIENT_IP, + ], + 'use custom header name with valid value' => [ + self::REAL_CLIENT_IP, + self::PROXY_IP, + 'X-Custom-Header', + ['X-Custom-Header' => self::REAL_CLIENT_IP], + ], + 'use custom header name without valid value' => [ + self::PROXY_IP, + self::PROXY_IP, + 'X-Custom-Header', + ], + 'use custom header value without custom header name' => [ + self::PROXY_IP, + self::PROXY_IP, + null, + ['X-Custom-Header' => self::REAL_CLIENT_IP], + ], + 'default platform.sh behaviour' => [ + self::REAL_CLIENT_IP, + self::PROXY_IP, + null, + ['X-Client-IP' => self::REAL_CLIENT_IP], + ['PLATFORM_RELATIONSHIPS' => true], + ], + 'use custom header name without valid value on platform.sh' => [ + self::PROXY_IP, + self::PROXY_IP, + 'X-Custom-Header', + [TrustedHeaderClientIpEventSubscriber::PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP => self::REAL_CLIENT_IP], + ['PLATFORM_RELATIONSHIPS' => true], + ], + 'use custom header with valid value on platform.sh' => [ + self::CUSTOM_CLIENT_IP, + self::PROXY_IP, + 'X-Custom-Header', + [ + TrustedHeaderClientIpEventSubscriber::PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP => self::REAL_CLIENT_IP, + 'X-Custom-Header' => self::CUSTOM_CLIENT_IP, + ], + ['PLATFORM_RELATIONSHIPS' => true], + ], + 'use valid value without custom header name on platform.sh' => [ + self::REAL_CLIENT_IP, + self::PROXY_IP, + null, + [ + TrustedHeaderClientIpEventSubscriber::PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP => self::REAL_CLIENT_IP, + 'X-Custom-Header' => self::CUSTOM_CLIENT_IP, + ], + ['PLATFORM_RELATIONSHIPS' => true], + ], + ]; + } + + public function testTrustedHeaderEventSubscriberWithoutTrustedProxy(): void + { + $_SERVER['REMOTE_ADDR'] = self::PROXY_IP; + + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addSubscriber( + new TrustedHeaderClientIpEventSubscriber('X-Custom-Header') + ); + + $request = Request::create('/', 'GET', [], [], [], array_merge( + $_SERVER, + ['PLATFORM_RELATIONSHIPS' => true], + )); + $request->headers->add([ + 'X-Custom-Header' => self::REAL_CLIENT_IP, + ]); + + $event = $eventDispatcher->dispatch(new RequestEvent( + self::createMock(KernelInterface::class), + $request, + HttpKernelInterface::MAIN_REQUEST + ), KernelEvents::REQUEST); + + /** @var \Symfony\Component\HttpFoundation\Request $request */ + $request = $event->getRequest(); + + self::assertEquals(self::PROXY_IP, $request->getClientIp()); + } + + /** + * @dataProvider getTrustedHeaderEventSubscriberTestData + */ + public function testTrustedHeaderEventSubscriberWithTrustedProxy( + string $expectedIp, + string $remoteAddrIp, + ?string $trustedHeaderName = null, + array $headers = [], + array $server = [] + ): void { + $_SERVER['REMOTE_ADDR'] = $remoteAddrIp; + Request::setTrustedProxies(['REMOTE_ADDR'], Request::getTrustedHeaderSet()); + + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addSubscriber( + new TrustedHeaderClientIpEventSubscriber($trustedHeaderName) + ); + + $request = Request::create('/', 'GET', [], [], [], array_merge( + $server, + ['REMOTE_ADDR' => $remoteAddrIp], + )); + $request->headers->add($headers); + + $event = $eventDispatcher->dispatch(new RequestEvent( + self::createMock(KernelInterface::class), + $request, + HttpKernelInterface::MAIN_REQUEST + ), KernelEvents::REQUEST); + + /** @var \Symfony\Component\HttpFoundation\Request $request */ + $request = $event->getRequest(); + + self::assertEquals($expectedIp, $request->getClientIp()); + } +} From 006fbf4b30800f3e76f466a8f4115f96ebf37dfe Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 27 Oct 2022 08:23:38 +0200 Subject: [PATCH 2/4] fix: Code review --- src/bundle/Core/Resources/config/default_settings.yml | 2 +- src/bundle/Core/Resources/config/services.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bundle/Core/Resources/config/default_settings.yml b/src/bundle/Core/Resources/config/default_settings.yml index c97f1a1fa3..cf1718dbe7 100644 --- a/src/bundle/Core/Resources/config/default_settings.yml +++ b/src/bundle/Core/Resources/config/default_settings.yml @@ -2,7 +2,7 @@ parameters: # Kernel related params webroot_dir: "%kernel.project_dir%/public" - trusted_header_client_ip_name: ~ + ibexa.trusted_header_client_ip_name: ~ ### # ibexa.site_access.config namespace, default scope diff --git a/src/bundle/Core/Resources/config/services.yml b/src/bundle/Core/Resources/config/services.yml index 938ff3ca3b..bba5a01dc8 100644 --- a/src/bundle/Core/Resources/config/services.yml +++ b/src/bundle/Core/Resources/config/services.yml @@ -296,7 +296,7 @@ services: Ibexa\Bundle\Core\EventSubscriber\TrustedHeaderClientIpEventSubscriber: arguments: - $trustedHeaderName: '%trusted_header_client_ip_name%' + $trustedHeaderName: '%ibexa.trusted_header_client_ip_name%' tags: - {name: kernel.event_subscriber} From f3c339bef304007de36427dd8d20db2a74b12889 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 27 Oct 2022 08:25:25 +0200 Subject: [PATCH 3/4] fix: Code review --- .../EventSubscriber/TrustedHeaderClientIpEventSubscriber.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php b/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php index e7bc5240d9..33d3e80e6e 100644 --- a/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php +++ b/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php @@ -13,7 +13,7 @@ use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\KernelEvents; -class TrustedHeaderClientIpEventSubscriber implements EventSubscriberInterface +final class TrustedHeaderClientIpEventSubscriber implements EventSubscriberInterface { public const PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP = 'X-Client-IP'; From 293dee5f770d80a761680f753a8bfcd339acb086 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 27 Oct 2022 14:17:46 +0200 Subject: [PATCH 4/4] fix: Code Review --- .../TrustedHeaderClientIpEventSubscriber.php | 2 +- .../TrustedHeaderClientIpEventSubscriberTest.php | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php b/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php index 33d3e80e6e..cff112d523 100644 --- a/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php +++ b/src/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriber.php @@ -15,7 +15,7 @@ final class TrustedHeaderClientIpEventSubscriber implements EventSubscriberInterface { - public const PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP = 'X-Client-IP'; + private const PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP = 'X-Client-IP'; private ?string $trustedHeaderName; diff --git a/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php b/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php index 56a7e44d17..db84d93207 100644 --- a/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php +++ b/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php @@ -19,6 +19,8 @@ final class TrustedHeaderClientIpEventSubscriberTest extends TestCase { + private const PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP = 'X-Client-IP'; + private ?string $originalRemoteAddr; private const PROXY_IP = '127.100.100.1'; @@ -80,7 +82,7 @@ public function getTrustedHeaderEventSubscriberTestData(): array self::PROXY_IP, self::PROXY_IP, 'X-Custom-Header', - [TrustedHeaderClientIpEventSubscriber::PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP => self::REAL_CLIENT_IP], + [self::PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP => self::REAL_CLIENT_IP], ['PLATFORM_RELATIONSHIPS' => true], ], 'use custom header with valid value on platform.sh' => [ @@ -88,7 +90,7 @@ public function getTrustedHeaderEventSubscriberTestData(): array self::PROXY_IP, 'X-Custom-Header', [ - TrustedHeaderClientIpEventSubscriber::PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP => self::REAL_CLIENT_IP, + self::PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP => self::REAL_CLIENT_IP, 'X-Custom-Header' => self::CUSTOM_CLIENT_IP, ], ['PLATFORM_RELATIONSHIPS' => true], @@ -98,7 +100,7 @@ public function getTrustedHeaderEventSubscriberTestData(): array self::PROXY_IP, null, [ - TrustedHeaderClientIpEventSubscriber::PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP => self::REAL_CLIENT_IP, + self::PLATFORM_SH_TRUSTED_HEADER_CLIENT_IP => self::REAL_CLIENT_IP, 'X-Custom-Header' => self::CUSTOM_CLIENT_IP, ], ['PLATFORM_RELATIONSHIPS' => true],