From 2946b630c55d6003fe5172c426690bffef587ad1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 22 Nov 2024 08:59:42 +0100 Subject: [PATCH] Use IpAddressFactory from akrabat/ip-address-middleware --- composer.json | 2 +- config/autoload/client-detection.global.php | 20 ---------- config/autoload/ip-address.global.php | 37 +++++++++++++++++++ config/constants.php | 1 + module/Core/functions/functions.php | 5 ++- module/Core/test-api/Action/RedirectTest.php | 4 +- .../IpGeolocationMiddlewareTest.php | 18 +++------ .../Entity/RedirectConditionTest.php | 4 +- .../ShortUrlRedirectionResolverTest.php | 10 ++--- module/Core/test/Visit/RequestTrackerTest.php | 11 +++--- 10 files changed, 62 insertions(+), 50 deletions(-) delete mode 100644 config/autoload/client-detection.global.php create mode 100644 config/autoload/ip-address.global.php diff --git a/composer.json b/composer.json index c880ac36..6cc93738 100644 --- a/composer.json +++ b/composer.json @@ -43,7 +43,7 @@ "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.1.1", - "shlinkio/shlink-common": "dev-main#698f580 as 6.6", + "shlinkio/shlink-common": "dev-main#abdad29 as 6.6", "shlinkio/shlink-config": "dev-main#e7dbed3 as 3.4", "shlinkio/shlink-event-dispatcher": "^4.1", "shlinkio/shlink-importer": "^5.3.2", diff --git a/config/autoload/client-detection.global.php b/config/autoload/client-detection.global.php deleted file mode 100644 index a49b3d93..00000000 --- a/config/autoload/client-detection.global.php +++ /dev/null @@ -1,20 +0,0 @@ - [ - 'headers_to_inspect' => [ - 'CF-Connecting-IP', - 'X-Forwarded-For', - 'X-Forwarded', - 'Forwarded', - 'True-Client-IP', - 'X-Real-IP', - 'X-Cluster-Client-Ip', - 'Client-Ip', - ], - ], - -]; diff --git a/config/autoload/ip-address.global.php b/config/autoload/ip-address.global.php new file mode 100644 index 00000000..9d531040 --- /dev/null +++ b/config/autoload/ip-address.global.php @@ -0,0 +1,37 @@ + [ + 'ip_address' => [ + 'attribute_name' => IP_ADDRESS_REQUEST_ATTRIBUTE, + 'check_proxy_headers' => true, + 'trusted_proxies' => [], + 'headers_to_inspect' => [ + 'CF-Connecting-IP', + 'X-Forwarded-For', + 'X-Forwarded', + 'Forwarded', + 'True-Client-IP', + 'X-Real-IP', + 'X-Cluster-Client-Ip', + 'Client-Ip', + ], + ], + ], + + 'dependencies' => [ + 'factories' => [ + IpAddress::class => IpAddressFactory::class, + ], + ], + +]; diff --git a/config/constants.php b/config/constants.php index 20c64f19..d6bb9621 100644 --- a/config/constants.php +++ b/config/constants.php @@ -21,3 +21,4 @@ const DEFAULT_QR_CODE_ROUND_BLOCK_SIZE = true; const DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS = true; const DEFAULT_QR_CODE_COLOR = '#000000'; // Black const DEFAULT_QR_CODE_BG_COLOR = '#ffffff'; // White +const IP_ADDRESS_REQUEST_ATTRIBUTE = 'remote_address'; diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 6ccc42e2..513e885d 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -15,7 +15,6 @@ use Laminas\Filter\Word\CamelCaseToSeparator; use Laminas\Filter\Word\CamelCaseToUnderscore; use Laminas\InputFilter\InputFilter; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; use Shlinkio\Shlink\IpGeolocation\Model\Location; @@ -38,6 +37,8 @@ use function strtolower; use function trim; use function ucfirst; +use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; + function generateRandomShortCode(int $length, ShortUrlMode $mode = ShortUrlMode::STRICT): string { static $nanoIdClient; @@ -288,7 +289,7 @@ function splitByComma(string|null $value): array function ipAddressFromRequest(ServerRequestInterface $request): string|null { - return $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR); + return $request->getAttribute(IP_ADDRESS_REQUEST_ATTRIBUTE); } function geolocationFromRequest(ServerRequestInterface $request): Location|null diff --git a/module/Core/test-api/Action/RedirectTest.php b/module/Core/test-api/Action/RedirectTest.php index 36031da8..79a13fbf 100644 --- a/module/Core/test-api/Action/RedirectTest.php +++ b/module/Core/test-api/Action/RedirectTest.php @@ -89,8 +89,8 @@ class RedirectTest extends ApiTestCase 'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/', ]; - $clientDetection = require __DIR__ . '/../../../../config/autoload/client-detection.global.php'; - foreach ($clientDetection['ip_address_resolution']['headers_to_inspect'] as $header) { + $ipAddressConfig = require __DIR__ . '/../../../../config/autoload/ip-address.global.php'; + foreach ($ipAddressConfig['rka']['ip_address']['headers_to_inspect'] as $header) { yield sprintf('rule: IP address in "%s" header', $header) => [ [ RequestOptions::HEADERS => [$header => '1.2.3.4'], diff --git a/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php b/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php index 80768f5b..210fb46f 100644 --- a/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php +++ b/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php @@ -14,7 +14,6 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use RuntimeException; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Config\Options\TrackingOptions; use Shlinkio\Shlink\Core\Geolocation\Middleware\IpGeolocationMiddleware; @@ -24,6 +23,8 @@ use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Throwable; +use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; + class IpGeolocationMiddlewareTest extends TestCase { private MockObject & IpLocationResolverInterface $ipLocationResolver; @@ -76,10 +77,7 @@ class IpGeolocationMiddlewareTest extends TestCase $this->ipLocationResolver->expects($this->never())->method('resolveIpLocation'); $this->logger->expects($this->never())->method('warning'); - $request = ServerRequestFactory::fromGlobals()->withAttribute( - IpAddressMiddlewareFactory::REQUEST_ATTR, - $ipAddress, - ); + $request = ServerRequestFactory::fromGlobals()->withAttribute(IP_ADDRESS_REQUEST_ATTRIBUTE, $ipAddress); $this->handler->expects($this->once())->method('handle')->with($this->callback( function (ServerRequestInterface $req): bool { $location = $req->getAttribute(Location::class); @@ -104,10 +102,7 @@ class IpGeolocationMiddlewareTest extends TestCase ); $this->logger->expects($this->never())->method('warning'); - $request = ServerRequestFactory::fromGlobals()->withAttribute( - IpAddressMiddlewareFactory::REQUEST_ATTR, - '1.2.3.4', - ); + $request = ServerRequestFactory::fromGlobals()->withAttribute(IP_ADDRESS_REQUEST_ATTRIBUTE, '1.2.3.4'); $this->handler->expects($this->once())->method('handle')->with($this->callback( function (ServerRequestInterface $req): bool { $location = $req->getAttribute(Location::class); @@ -147,10 +142,7 @@ class IpGeolocationMiddlewareTest extends TestCase ->willThrowException($exception); $this->logger->expects($this->once())->method($loggerMethod)->with($expectedLoggedMessage, ['e' => $exception]); - $request = ServerRequestFactory::fromGlobals()->withAttribute( - IpAddressMiddlewareFactory::REQUEST_ATTR, - '1.2.3.4', - ); + $request = ServerRequestFactory::fromGlobals()->withAttribute(IP_ADDRESS_REQUEST_ATTRIBUTE, '1.2.3.4'); $this->handler->expects($this->once())->method('handle')->with($this->callback( function (ServerRequestInterface $req): bool { $location = $req->getAttribute(Location::class); diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index 2ae5df18..5a4a2e2b 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -7,11 +7,11 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\TestCase; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition; use Shlinkio\Shlink\IpGeolocation\Model\Location; +use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT; use const ShlinkioTest\Shlink\IOS_USER_AGENT; @@ -88,7 +88,7 @@ class RedirectConditionTest extends TestCase { $request = ServerRequestFactory::fromGlobals(); if ($remoteIp !== null) { - $request = $request->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, $remoteIp); + $request = $request->withAttribute(IP_ADDRESS_REQUEST_ATTRIBUTE, $remoteIp); } $result = RedirectCondition::forIpAddress($ipToMatch)->matchesRequest($request); diff --git a/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php b/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php index f26627c6..470ff95e 100644 --- a/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php +++ b/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php @@ -9,7 +9,6 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition; use Shlinkio\Shlink\Core\RedirectRule\Entity\ShortUrlRedirectRule; @@ -18,6 +17,7 @@ use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT; use const ShlinkioTest\Shlink\IOS_USER_AGENT; @@ -90,22 +90,22 @@ class ShortUrlRedirectionResolverTest extends TestCase 'https://example.com/from-rule', ]; yield 'matching static IP address' => [ - $request()->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, '1.2.3.4'), + $request()->withAttribute(IP_ADDRESS_REQUEST_ATTRIBUTE, '1.2.3.4'), RedirectCondition::forIpAddress('1.2.3.4'), 'https://example.com/from-rule', ]; yield 'matching CIDR block' => [ - $request()->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, '192.168.1.35'), + $request()->withAttribute(IP_ADDRESS_REQUEST_ATTRIBUTE, '192.168.1.35'), RedirectCondition::forIpAddress('192.168.1.0/24'), 'https://example.com/from-rule', ]; yield 'matching wildcard IP address' => [ - $request()->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, '1.2.5.5'), + $request()->withAttribute(IP_ADDRESS_REQUEST_ATTRIBUTE, '1.2.5.5'), RedirectCondition::forIpAddress('1.2.*.*'), 'https://example.com/from-rule', ]; yield 'non-matching IP address' => [ - $request()->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, '4.3.2.1'), + $request()->withAttribute(IP_ADDRESS_REQUEST_ATTRIBUTE, '4.3.2.1'), RedirectCondition::forIpAddress('1.2.3.4'), 'https://example.com/foo/bar', ]; diff --git a/module/Core/test/Visit/RequestTrackerTest.php b/module/Core/test/Visit/RequestTrackerTest.php index ae0a74c4..f9357c6a 100644 --- a/module/Core/test/Visit/RequestTrackerTest.php +++ b/module/Core/test/Visit/RequestTrackerTest.php @@ -12,7 +12,6 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Config\Options\TrackingOptions; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -20,6 +19,8 @@ use Shlinkio\Shlink\Core\Visit\Model\Visitor; use Shlinkio\Shlink\Core\Visit\RequestTracker; use Shlinkio\Shlink\Core\Visit\VisitsTrackerInterface; +use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; + class RequestTrackerTest extends TestCase { private const LONG_URL = 'https://domain.com/foo/bar?some=thing'; @@ -67,15 +68,15 @@ class RequestTrackerTest extends TestCase ServerRequestFactory::fromGlobals()->withQueryParams(['foobar' => null]), ]; yield 'exact remote address' => [ServerRequestFactory::fromGlobals()->withAttribute( - IpAddressMiddlewareFactory::REQUEST_ATTR, + IP_ADDRESS_REQUEST_ATTRIBUTE, '80.90.100.110', )]; yield 'matching wildcard remote address' => [ServerRequestFactory::fromGlobals()->withAttribute( - IpAddressMiddlewareFactory::REQUEST_ATTR, + IP_ADDRESS_REQUEST_ATTRIBUTE, '1.2.3.4', )]; yield 'matching CIDR block remote address' => [ServerRequestFactory::fromGlobals()->withAttribute( - IpAddressMiddlewareFactory::REQUEST_ATTR, + IP_ADDRESS_REQUEST_ATTRIBUTE, '192.168.10.100', )]; } @@ -102,7 +103,7 @@ class RequestTrackerTest extends TestCase ); $this->requestTracker->trackIfApplicable($shortUrl, ServerRequestFactory::fromGlobals()->withAttribute( - IpAddressMiddlewareFactory::REQUEST_ATTR, + IP_ADDRESS_REQUEST_ATTRIBUTE, 'invalid', )); }