diff --git a/composer.json b/composer.json index a6fe63d..71c159c 100644 --- a/composer.json +++ b/composer.json @@ -29,6 +29,7 @@ "php": "~8.5.0", "ext-curl": "*", "ext-dom": "*", + "akrabat/ip-address-middleware": "^2.6", "doctrine/data-fixtures": "^2.2", "doctrine/doctrine-fixtures-bundle": "^4.3", "dotkernel/dot-cache": "^4.0", diff --git a/config/autoload/ip-address.global.php b/config/autoload/ip-address.global.php new file mode 100644 index 0000000..d0c0f2c --- /dev/null +++ b/config/autoload/ip-address.global.php @@ -0,0 +1,31 @@ + [ + 'ip_address' => [ + // Inspect proxy headers below instead of using REMOTE_ADDR only. + 'check_proxy_headers' => true, + // Proxy IPs/CIDRs/wildcards trusted to set the headers below. + 'trusted_proxies' => [], + // Request attribute the resolved IP is stored under. + 'attribute_name' => 'ipAddress', + // Headers checked, in order, for the client IP. + 'headers_to_inspect' => [ + // nginx specific + 'X-Real-IP', + 'X-Forwarded-For', + // cloudflare specific + 'CF-Connecting-IP', + 'True-Client-IP', + // generic + 'Forwarded', + 'X-Forwarded', + 'X-Cluster-Client-Ip', + 'Client-Ip', + ], + ], + ], +]; diff --git a/config/config.php b/config/config.php index 147edc1..63abf42 100644 --- a/config/config.php +++ b/config/config.php @@ -23,6 +23,7 @@ \Mezzio\Router\ConfigProvider::class, \Mezzio\Router\FastRouteRouter\ConfigProvider::class, \Mezzio\Twig\ConfigProvider::class, + \RKA\Middleware\Mezzio\ConfigProvider::class, // Dotkernel packages \Dot\DependencyInjection\ConfigProvider::class, diff --git a/src/App/src/Handler/PostCreateContactHandler.php b/src/App/src/Handler/PostCreateContactHandler.php index 04e974c..a7fb829 100644 --- a/src/App/src/Handler/PostCreateContactHandler.php +++ b/src/App/src/Handler/PostCreateContactHandler.php @@ -18,17 +18,30 @@ use function is_array; use function is_string; +/** + * @phpstan-type IpAddressConfigType array{ + * check_proxy_headers: bool, + * trusted_proxies: array, + * attribute_name: non-empty-string, + * headers_to_inspect: array, + * } + */ class PostCreateContactHandler implements RequestHandlerInterface { + /** + * @param IpAddressConfigType $ipAddressConfig + */ #[Inject( TemplateRendererInterface::class, UrlHelper::class, ContactService::class, + 'config.rka.ip_address', )] public function __construct( protected TemplateRendererInterface $template, protected UrlHelper $urlHelper, protected ContactService $contactService, + protected array $ipAddressConfig, ) { } @@ -42,13 +55,13 @@ public function handle(ServerRequestInterface $request): ResponseInterface $token = is_array($rawBody) && is_string($rawBody['cf-turnstile-response'] ?? null) ? $rawBody['cf-turnstile-response'] : ''; - $ip = $request->getServerParams()['REMOTE_ADDR'] ?? null; + $ip = (string) $request->getAttribute($this->ipAddressConfig['attribute_name']); - if (! $this->contactService->verifyTurnstile($token, is_string($ip) ? $ip : null)) { + if (! $this->contactService->verifyTurnstile($token, $ip)) { $errors['captcha'] = "We couldn't verify you're human. Please try again."; } - if ($errors === [] && ! $this->contactService->send($data)) { + if ($errors === [] && ! $this->contactService->send($data, $ip)) { $mailFailed = true; } diff --git a/src/App/src/RoutesDelegator.php b/src/App/src/RoutesDelegator.php index be6e8c3..fb4cce6 100644 --- a/src/App/src/RoutesDelegator.php +++ b/src/App/src/RoutesDelegator.php @@ -13,6 +13,7 @@ use Light\App\Handler\PostCreateContactHandler; use Mezzio\Application; use Psr\Container\ContainerInterface; +use RKA\Middleware\IpAddress; use function assert; @@ -28,7 +29,7 @@ public function __invoke(ContainerInterface $container, string $serviceName, cal $app->get('/{categorySlug}/{slug}.md', [GetMarkdownArticleHandler::class], 'app::markdown-article'); $app->get('/contact/', [GetCreateContactFormHandler::class], 'app::create-contact-form'); - $app->post('/contact/', [PostCreateContactHandler::class], 'app::create-contact'); + $app->post('/contact/', [IpAddress::class, PostCreateContactHandler::class], 'app::create-contact'); // Route name kept as `page::…` because `@layout/default.html.twig` links it by name. // The matching entry must stay out of `routes.page` in local.php to avoid a duplicate. diff --git a/src/App/src/Service/ContactService.php b/src/App/src/Service/ContactService.php index 83ded42..9f43fba 100644 --- a/src/App/src/Service/ContactService.php +++ b/src/App/src/Service/ContactService.php @@ -79,7 +79,7 @@ public function turnstileSiteKey(): string return $this->turnstile['secret_key'] !== '' ? $this->turnstile['site_key'] : ''; } - public function verifyTurnstile(string $token, ?string $remoteIp): bool + public function verifyTurnstile(string $token, string $remoteIp): bool { if ($this->turnstileSiteKey() === '') { return true; @@ -93,7 +93,7 @@ public function verifyTurnstile(string $token, ?string $remoteIp): bool 'secret' => $this->turnstile['secret_key'], 'response' => $token, ]; - if ($remoteIp) { + if ($remoteIp !== '') { $data['remoteip'] = $remoteIp; } @@ -176,11 +176,11 @@ public function validate(array $data): array /** * @param ContactData $data */ - public function send(array $data): bool + public function send(array $data, string $ipAddress = ''): bool { $this->mailService->getMessage()->setReplyTo($data['email'], $data['name']); $this->mailService->setSubject('New contact form submission on dotkernel.com'); - $this->mailService->setBody($this->buildBody($data)); + $this->mailService->setBody($this->buildBody($data, $ipAddress)); try { $result = $this->mailService->send(); @@ -237,7 +237,7 @@ private function stringValue(array $raw, string $key): string /** * @param ContactData $data */ - private function buildBody(array $data): string + private function buildBody(array $data, string $ipAddress): string { $rows = [ 'Topic' => ContactTopicEnum::tryFrom($data['topic'])?->label() ?? '', @@ -247,6 +247,7 @@ private function buildBody(array $data): string 'Current stack' => $data['stack'], 'Message' => $data['message'], 'Query params' => $data['query'], + 'IP address' => $ipAddress, ]; $html = ''; diff --git a/test/Unit/App/Handler/PostCreateContactHandlerTest.php b/test/Unit/App/Handler/PostCreateContactHandlerTest.php index 8581bed..9166de6 100644 --- a/test/Unit/App/Handler/PostCreateContactHandlerTest.php +++ b/test/Unit/App/Handler/PostCreateContactHandlerTest.php @@ -26,6 +26,24 @@ class PostCreateContactHandlerTest extends TestCase { + /** + * @return array{ + * check_proxy_headers: bool, + * trusted_proxies: array, + * attribute_name: non-empty-string, + * headers_to_inspect: array, + * } + */ + private function ipAddressConfig(): array + { + return [ + 'check_proxy_headers' => false, + 'trusted_proxies' => [], + 'attribute_name' => 'ipAddress', + 'headers_to_inspect' => [], + ]; + } + /** * @return array */ @@ -74,7 +92,12 @@ public function testValidSubmissionRedirectsOnSuccess(): void ->with('app::create-contact-form', [], ['contact' => 'sent'], 'contact-form') ->willReturn('/contact/?contact=sent#contact-form'); - $handler = new PostCreateContactHandler($template, $urlHelper, $this->service(new MailResult(true))); + $handler = new PostCreateContactHandler( + $template, + $urlHelper, + $this->service(new MailResult(true)), + $this->ipAddressConfig() + ); $response = $handler->handle($request); $this->assertInstanceOf(RedirectResponse::class, $response); @@ -113,7 +136,8 @@ public function testMissingRequiredFieldsRerendersWithErrors(): void $handler = new PostCreateContactHandler( $template, $this->createStub(UrlHelper::class), - $this->service(new MailResult(true)) + $this->service(new MailResult(true)), + $this->ipAddressConfig() ); $response = $handler->handle($request); @@ -150,7 +174,8 @@ public function testRerenderKeepsAnyExtraQueryParamsAsDynamicHiddenFields(): voi $handler = new PostCreateContactHandler( $template, $this->createStub(UrlHelper::class), - $this->service(new MailResult(true)) + $this->service(new MailResult(true)), + $this->ipAddressConfig() ); $handler->handle($request); } @@ -179,7 +204,8 @@ public function testMailTransportFailureRerendersWithMailFailedFlag(): void $handler = new PostCreateContactHandler( $template, $this->createStub(UrlHelper::class), - $this->service(new RuntimeException('SMTP connection refused')) + $this->service(new RuntimeException('SMTP connection refused')), + $this->ipAddressConfig() ); $response = $handler->handle($request); @@ -197,7 +223,7 @@ public function testFailedTurnstileRerendersWithCaptchaErrorAndSendsNothing(): v $request = $this->createStub(ServerRequestInterface::class); $request->method('getParsedBody')->willReturn($data); - $request->method('getServerParams')->willReturn(['REMOTE_ADDR' => '203.0.113.5']); + $request->method('getAttribute')->willReturnMap([['ipAddress', null, '203.0.113.5']]); $contactService = $this->createMock(ContactService::class); $contactService->method('normalize')->willReturn([ @@ -232,10 +258,55 @@ public function testFailedTurnstileRerendersWithCaptchaErrorAndSendsNothing(): v ) ->willReturn(''); - $handler = new PostCreateContactHandler($template, $this->createStub(UrlHelper::class), $contactService); + $handler = new PostCreateContactHandler( + $template, + $this->createStub(UrlHelper::class), + $contactService, + $this->ipAddressConfig() + ); $response = $handler->handle($request); $this->assertInstanceOf(HtmlResponse::class, $response); $this->assertSame(200, $response->getStatusCode()); } + + /** + * @throws Exception + */ + public function testValidSubmissionSendsTheIpAddressFromTheRequestAttribute(): void + { + $request = $this->createStub(ServerRequestInterface::class); + $request->method('getParsedBody')->willReturn($this->validData()); + $request->method('getAttribute')->willReturnMap([['ipAddress', null, '203.0.113.5']]); + + $contactService = $this->createMock(ContactService::class); + $contactService->method('normalize')->willReturn([ + 'topic' => 'migration', + 'name' => 'Jane Doe', + 'email' => 'jane@example.com', + 'company' => 'Acme Inc', + 'stack' => 'ZF3, PHP 7.4', + 'message' => 'We would like to talk about migrating our platform.', + 'query' => '', + ]); + $contactService->method('validate')->willReturn([]); + $contactService->method('verifyTurnstile')->willReturn(true); + $contactService + ->expects($this->once()) + ->method('send') + ->with($this->anything(), '203.0.113.5') + ->willReturn(true); + + $urlHelper = $this->createStub(UrlHelper::class); + $urlHelper->method('generate')->willReturn('/contact/?contact=sent#contact-form'); + + $response = (new PostCreateContactHandler( + $this->createStub(TemplateRendererInterface::class), + $urlHelper, + $contactService, + $this->ipAddressConfig() + ))->handle($request); + + $this->assertInstanceOf(RedirectResponse::class, $response); + } } diff --git a/test/Unit/App/RoutesDelegatorTest.php b/test/Unit/App/RoutesDelegatorTest.php index 51b96ba..15c207e 100644 --- a/test/Unit/App/RoutesDelegatorTest.php +++ b/test/Unit/App/RoutesDelegatorTest.php @@ -19,6 +19,7 @@ use Psr\Container\ContainerExceptionInterface; use Psr\Container\ContainerInterface; use Psr\Container\NotFoundExceptionInterface; +use RKA\Middleware\IpAddress; class RoutesDelegatorTest extends UnitTest { @@ -77,7 +78,10 @@ public function testWillRegisterAllRoutes(): void $this->assertSame([GetCreateContactFormHandler::class], $this->registeredRoutes['/contact/']['handler']); $this->assertSame('app::create-contact-form', $this->registeredRoutes['/contact/']['name']); - $this->assertSame([PostCreateContactHandler::class], $this->registeredRoutes['POST /contact/']['handler']); + $this->assertSame( + [IpAddress::class, PostCreateContactHandler::class], + $this->registeredRoutes['POST /contact/']['handler'] + ); $this->assertSame('app::create-contact', $this->registeredRoutes['POST /contact/']['name']); } } diff --git a/test/Unit/App/Service/ContactServiceTest.php b/test/Unit/App/Service/ContactServiceTest.php index a9d58ac..209c52b 100644 --- a/test/Unit/App/Service/ContactServiceTest.php +++ b/test/Unit/App/Service/ContactServiceTest.php @@ -280,6 +280,38 @@ public function testSendReturnsFalseWhenResultIsInvalid(): void $this->assertFalse($service->send($this->validData())); } + public function testSendIncludesTheIpAddressInBody(): void + { + $message = $this->createStub(Email::class); + + $mailService = $this->createMock(MailServiceInterface::class); + $mailService->method('getMessage')->willReturn($message); + $mailService + ->expects($this->once()) + ->method('setBody') + ->with($this->callback( + fn (string $body): bool => str_contains($body, '

IP address:
203.0.113.5

') + )); + $mailService->method('send')->willReturn(new MailResult(true)); + + (new ContactService($mailService, new NullLogger()))->send($this->validData(), '203.0.113.5'); + } + + public function testSendOmitsTheIpAddressRowWhenUnknown(): void + { + $message = $this->createStub(Email::class); + + $mailService = $this->createMock(MailServiceInterface::class); + $mailService->method('getMessage')->willReturn($message); + $mailService + ->expects($this->once()) + ->method('setBody') + ->with($this->callback(fn (string $body): bool => ! str_contains($body, 'IP address'))); + $mailService->method('send')->willReturn(new MailResult(true)); + + (new ContactService($mailService, new NullLogger()))->send($this->validData()); + } + public function testSendIncludesQueryParamsInBodyWhenPresent(): void { $message = $this->createStub(Email::class); @@ -359,7 +391,7 @@ public function testTurnstileIsOffWithEmptyKeys(): void ); $this->assertSame('', $service->turnstileSiteKey()); - $this->assertTrue($service->verifyTurnstile('', null)); + $this->assertTrue($service->verifyTurnstile('', '')); $this->assertSame([], $calls); } @@ -373,14 +405,14 @@ public function testTurnstileIsOffWithoutASecretKey(): void ); $this->assertSame('', $service->turnstileSiteKey()); - $this->assertTrue($service->verifyTurnstile('', null)); + $this->assertTrue($service->verifyTurnstile('', '')); $this->assertSame([], $calls); } public function testTurnstileIsOffByDefault(): void { $this->assertSame('', $this->service()->turnstileSiteKey()); - $this->assertTrue($this->service()->verifyTurnstile('', null)); + $this->assertTrue($this->service()->verifyTurnstile('', '')); } public function testTurnstileSiteKeyIsExposedWhenBothKeysAreSet(): void @@ -417,7 +449,7 @@ public function testRemoteIpIsOmittedWhenNull(): void $calls = []; $service = $this->turnstileService(self::TURNSTILE, self::SITEVERIFY_SUCCESS, $calls); - $this->assertTrue($service->verifyTurnstile('token', null)); + $this->assertTrue($service->verifyTurnstile('token', '')); $this->assertSame([['secret' => 'secret-key', 'response' => 'token']], $calls); } @@ -444,7 +476,7 @@ public static function rejectedSiteverifyProvider(): array #[DataProvider('rejectedSiteverifyProvider')] public function testRejectedSiteverifyFails(array $turnstile, ?string $response): void { - $this->assertFalse($this->turnstileService($turnstile, $response)->verifyTurnstile('token', null)); + $this->assertFalse($this->turnstileService($turnstile, $response)->verifyTurnstile('token', '')); } public function testMatchingHostnamePasses(): void @@ -454,7 +486,7 @@ public function testMatchingHostnamePasses(): void self::SITEVERIFY_SUCCESS ); - $this->assertTrue($service->verifyTurnstile('token', null)); + $this->assertTrue($service->verifyTurnstile('token', '')); } public function testTestingKeyResponsePassesEvenWithAHostnameConfigured(): void @@ -464,6 +496,6 @@ public function testTestingKeyResponsePassesEvenWithAHostnameConfigured(): void '{"success":true,"hostname":"example.com","metadata":{"result_with_testing_key":true}}' ); - $this->assertTrue($service->verifyTurnstile('token', null)); + $this->assertTrue($service->verifyTurnstile('token', '')); } }