From 8d90661d0a7e0b5d7bb91e1b48a477500151ca53 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Jul 2024 10:12:05 +0200 Subject: [PATCH 1/9] Extract logic to match IP address against list of groups --- .../Exception/InvalidIpFormatException.php | 15 +++++ module/Core/src/Util/IpAddressUtils.php | 65 +++++++++++++++++++ module/Core/src/Visit/RequestTracker.php | 51 +++------------ module/Core/test/Visit/RequestTrackerTest.php | 15 +++++ 4 files changed, 103 insertions(+), 43 deletions(-) create mode 100644 module/Core/src/Exception/InvalidIpFormatException.php create mode 100644 module/Core/src/Util/IpAddressUtils.php diff --git a/module/Core/src/Exception/InvalidIpFormatException.php b/module/Core/src/Exception/InvalidIpFormatException.php new file mode 100644 index 00000000..df9d207b --- /dev/null +++ b/module/Core/src/Exception/InvalidIpFormatException.php @@ -0,0 +1,15 @@ + strict equality with provided IP address. + * * CIDR block -> provided IP address is part of that block. + * * Wildcard -> static parts match the corresponding ones in provided IP address. + * + * @param string[] $groups + * @throws InvalidIpFormatException + */ + public static function ipAddressMatchesGroups(string $ipAddress, array $groups): bool + { + $ip = IPv4::parseString($ipAddress); + if ($ip === null) { + throw InvalidIpFormatException::fromInvalidIp($ipAddress); + } + + $ipAddressParts = explode('.', $ipAddress); + + return some($groups, function (string $value) use ($ip, $ipAddressParts): bool { + $range = str_contains($value, '*') + ? self::parseValueWithWildcards($value, $ipAddressParts) + : Factory::parseRangeString($value); + + return $range !== null && $ip->matches($range); + }); + } + + private static function parseValueWithWildcards(string $value, array $ipAddressParts): ?RangeInterface + { + $octets = explode('.', $value); + $keys = array_keys($octets); + + // Replace wildcard parts with the corresponding ones from the remote address + return Factory::parseRangeString( + implode('.', array_map( + fn (string $part, int $index) => $part === '*' ? $ipAddressParts[$index] : $part, + $octets, + $keys, + )), + ); + } +} diff --git a/module/Core/src/Visit/RequestTracker.php b/module/Core/src/Visit/RequestTracker.php index 1a6b04f9..ecc3d94f 100644 --- a/module/Core/src/Visit/RequestTracker.php +++ b/module/Core/src/Visit/RequestTracker.php @@ -5,30 +5,20 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit; use Fig\Http\Message\RequestMethodInterface; -use IPLib\Address\IPv4; -use IPLib\Factory; -use IPLib\Range\RangeInterface; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; +use Shlinkio\Shlink\Core\Exception\InvalidIpFormatException; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Util\IpAddressUtils; use Shlinkio\Shlink\Core\Visit\Model\Visitor; -use function array_keys; -use function array_map; -use function explode; -use function implode; -use function Shlinkio\Shlink\Core\ArrayUtils\some; -use function str_contains; - -class RequestTracker implements RequestTrackerInterface, RequestMethodInterface +readonly class RequestTracker implements RequestTrackerInterface, RequestMethodInterface { - public function __construct( - private readonly VisitsTrackerInterface $visitsTracker, - private readonly TrackingOptions $trackingOptions, - ) { + public function __construct(private VisitsTrackerInterface $visitsTracker, private TrackingOptions $trackingOptions) + { } public function trackIfApplicable(ShortUrl $shortUrl, ServerRequestInterface $request): void @@ -78,35 +68,10 @@ class RequestTracker implements RequestTrackerInterface, RequestMethodInterface return false; } - $ip = IPv4::parseString($remoteAddr); - if ($ip === null) { + try { + return IpAddressUtils::ipAddressMatchesGroups($remoteAddr, $this->trackingOptions->disableTrackingFrom); + } catch (InvalidIpFormatException) { return false; } - - $remoteAddrParts = explode('.', $remoteAddr); - $disableTrackingFrom = $this->trackingOptions->disableTrackingFrom; - - return some($disableTrackingFrom, function (string $value) use ($ip, $remoteAddrParts): bool { - $range = str_contains($value, '*') - ? $this->parseValueWithWildcards($value, $remoteAddrParts) - : Factory::parseRangeString($value); - - return $range !== null && $ip->matches($range); - }); - } - - private function parseValueWithWildcards(string $value, array $remoteAddrParts): ?RangeInterface - { - $octets = explode('.', $value); - $keys = array_keys($octets); - - // Replace wildcard parts with the corresponding ones from the remote address - return Factory::parseRangeString( - implode('.', array_map( - fn (string $part, int $index) => $part === '*' ? $remoteAddrParts[$index] : $part, - $octets, - $keys, - )), - ); } } diff --git a/module/Core/test/Visit/RequestTrackerTest.php b/module/Core/test/Visit/RequestTrackerTest.php index fdf7e493..c8746f91 100644 --- a/module/Core/test/Visit/RequestTrackerTest.php +++ b/module/Core/test/Visit/RequestTrackerTest.php @@ -92,6 +92,21 @@ class RequestTrackerTest extends TestCase $this->requestTracker->trackIfApplicable($shortUrl, $this->request); } + #[Test] + public function trackingHappensOverShortUrlsWhenRemoteAddressIsInvalid(): void + { + $shortUrl = ShortUrl::withLongUrl(self::LONG_URL); + $this->visitsTracker->expects($this->once())->method('track')->with( + $shortUrl, + $this->isInstanceOf(Visitor::class), + ); + + $this->requestTracker->trackIfApplicable($shortUrl, ServerRequestFactory::fromGlobals()->withAttribute( + IpAddressMiddlewareFactory::REQUEST_ATTR, + 'invalid', + )); + } + #[Test] public function baseUrlErrorIsTracked(): void { From 1312ea61f4ecc260e48a0fb8b3f0cc4a2df3901c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Jul 2024 10:35:33 +0200 Subject: [PATCH 2/9] Add new IP address redirect condition --- .../RedirectRule/Entity/RedirectCondition.php | 19 +++++++++++++++++++ .../Model/RedirectConditionType.php | 1 + 2 files changed, 20 insertions(+) diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index 5e6df494..3032eb1a 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -5,10 +5,12 @@ namespace Shlinkio\Shlink\Core\RedirectRule\Entity; use JsonSerializable; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Entity\AbstractEntity; +use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; +use Shlinkio\Shlink\Core\Util\IpAddressUtils; use function Shlinkio\Shlink\Core\acceptLanguageToLocales; use function Shlinkio\Shlink\Core\ArrayUtils\some; use function Shlinkio\Shlink\Core\normalizeLocale; @@ -41,6 +43,15 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable return new self(RedirectConditionType::DEVICE, $device->value); } + /** + * @param string $ipAddressPattern - A static IP address (100.200.80.40), CIDR block (192.168.10.0/24) or wildcard + * pattern (11.22.*.*) + */ + public static function forIpAddress(string $ipAddressPattern): self + { + return new self(RedirectConditionType::IP_ADDRESS, $ipAddressPattern); + } + public static function fromRawData(array $rawData): self { $type = RedirectConditionType::from($rawData[RedirectRulesInputFilter::CONDITION_TYPE]); @@ -59,6 +70,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable RedirectConditionType::QUERY_PARAM => $this->matchesQueryParam($request), RedirectConditionType::LANGUAGE => $this->matchesLanguage($request), RedirectConditionType::DEVICE => $this->matchesDevice($request), + RedirectConditionType::IP_ADDRESS => $this->matchesRemoteIpAddress($request), }; } @@ -100,6 +112,12 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable return $device !== null && $device->value === strtolower($this->matchValue); } + private function matchesRemoteIpAddress(ServerRequestInterface $request): bool + { + $remoteAddress = $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR); + return $remoteAddress !== null && IpAddressUtils::ipAddressMatchesGroups($remoteAddress, [$this->matchValue]); + } + public function jsonSerialize(): array { return [ @@ -119,6 +137,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable $this->matchKey, $this->matchValue, ), + RedirectConditionType::IP_ADDRESS => sprintf('IP address matches %s', $this->matchValue), }; } } diff --git a/module/Core/src/RedirectRule/Model/RedirectConditionType.php b/module/Core/src/RedirectRule/Model/RedirectConditionType.php index c00cca7f..891a8ccc 100644 --- a/module/Core/src/RedirectRule/Model/RedirectConditionType.php +++ b/module/Core/src/RedirectRule/Model/RedirectConditionType.php @@ -7,4 +7,5 @@ enum RedirectConditionType: string case DEVICE = 'device'; case LANGUAGE = 'language'; case QUERY_PARAM = 'query-param'; + case IP_ADDRESS = 'ip-address'; } From f49d98f2ea7bde1b3020d137222d65b82c911d8b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 Jul 2024 19:51:13 +0200 Subject: [PATCH 3/9] Add logic for IP-based dynamic redirects --- .../src/RedirectRule/RedirectRuleHandler.php | 3 ++ module/Core/functions/functions.php | 7 +++ .../RedirectRule/Entity/RedirectCondition.php | 6 +-- module/Core/src/Util/IpAddressUtils.php | 3 +- module/Core/src/Visit/Model/Visitor.php | 4 +- module/Core/src/Visit/RequestTracker.php | 5 +- .../Entity/RedirectConditionTest.php | 47 ++++++++++++++----- .../Entity/ShortUrlRedirectRuleTest.php | 26 ++++++++-- .../ShortUrlRedirectionResolverTest.php | 26 ++++++++++ 9 files changed, 102 insertions(+), 25 deletions(-) diff --git a/module/CLI/src/RedirectRule/RedirectRuleHandler.php b/module/CLI/src/RedirectRule/RedirectRuleHandler.php index 068cdc74..cb1d3faf 100644 --- a/module/CLI/src/RedirectRule/RedirectRuleHandler.php +++ b/module/CLI/src/RedirectRule/RedirectRuleHandler.php @@ -108,6 +108,9 @@ class RedirectRuleHandler implements RedirectRuleHandlerInterface $this->askMandatory('Query param name?', $io), $this->askOptional('Query param value?', $io), ), + RedirectConditionType::IP_ADDRESS => RedirectCondition::forIpAddress( + $this->askMandatory('IP address, CIDR block or wildcard-pattern (1.2.*.*)', $io), + ), }; $continue = $io->confirm('Do you want to add another condition?'); diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 5900e8e1..720f27c1 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -14,6 +14,8 @@ use Jaybizzle\CrawlerDetect\CrawlerDetect; 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; @@ -273,3 +275,8 @@ function splitByComma(?string $value): array return array_map(trim(...), explode(',', $value)); } + +function ipAddressFromRequest(ServerRequestInterface $request): ?string +{ + return $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR); +} diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index 3032eb1a..59c2798b 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -5,14 +5,14 @@ namespace Shlinkio\Shlink\Core\RedirectRule\Entity; use JsonSerializable; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Entity\AbstractEntity; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; - use Shlinkio\Shlink\Core\Util\IpAddressUtils; + use function Shlinkio\Shlink\Core\acceptLanguageToLocales; use function Shlinkio\Shlink\Core\ArrayUtils\some; +use function Shlinkio\Shlink\Core\ipAddressFromRequest; use function Shlinkio\Shlink\Core\normalizeLocale; use function Shlinkio\Shlink\Core\splitLocale; use function sprintf; @@ -114,7 +114,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable private function matchesRemoteIpAddress(ServerRequestInterface $request): bool { - $remoteAddress = $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR); + $remoteAddress = ipAddressFromRequest($request); return $remoteAddress !== null && IpAddressUtils::ipAddressMatchesGroups($remoteAddress, [$this->matchValue]); } diff --git a/module/Core/src/Util/IpAddressUtils.php b/module/Core/src/Util/IpAddressUtils.php index 6f8f566b..79e7f839 100644 --- a/module/Core/src/Util/IpAddressUtils.php +++ b/module/Core/src/Util/IpAddressUtils.php @@ -14,8 +14,9 @@ use function array_map; use function explode; use function implode; use function Shlinkio\Shlink\Core\ArrayUtils\some; +use function str_contains; -class IpAddressUtils +final class IpAddressUtils { /** * Checks if an IP address matches any of provided groups. diff --git a/module/Core/src/Visit/Model/Visitor.php b/module/Core/src/Visit/Model/Visitor.php index 8e3b7f0c..9cf524bc 100644 --- a/module/Core/src/Visit/Model/Visitor.php +++ b/module/Core/src/Visit/Model/Visitor.php @@ -5,9 +5,9 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit\Model; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Options\TrackingOptions; +use function Shlinkio\Shlink\Core\ipAddressFromRequest; use function Shlinkio\Shlink\Core\isCrawler; use function substr; @@ -46,7 +46,7 @@ final class Visitor return new self( $request->getHeaderLine('User-Agent'), $request->getHeaderLine('Referer'), - $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR), + ipAddressFromRequest($request), $request->getUri()->__toString(), ); } diff --git a/module/Core/src/Visit/RequestTracker.php b/module/Core/src/Visit/RequestTracker.php index ecc3d94f..824e7c24 100644 --- a/module/Core/src/Visit/RequestTracker.php +++ b/module/Core/src/Visit/RequestTracker.php @@ -7,7 +7,6 @@ namespace Shlinkio\Shlink\Core\Visit; use Fig\Http\Message\RequestMethodInterface; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Exception\InvalidIpFormatException; use Shlinkio\Shlink\Core\Options\TrackingOptions; @@ -15,6 +14,8 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Util\IpAddressUtils; use Shlinkio\Shlink\Core\Visit\Model\Visitor; +use function Shlinkio\Shlink\Core\ipAddressFromRequest; + readonly class RequestTracker implements RequestTrackerInterface, RequestMethodInterface { public function __construct(private VisitsTrackerInterface $visitsTracker, private TrackingOptions $trackingOptions) @@ -53,7 +54,7 @@ readonly class RequestTracker implements RequestTrackerInterface, RequestMethodI return false; } - $remoteAddr = $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR); + $remoteAddr = ipAddressFromRequest($request); if ($this->shouldDisableTrackingFromAddress($remoteAddr)) { return false; } diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index a8ab2a16..3cd44ef0 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -6,6 +6,7 @@ use Laminas\Diactoros\ServerRequestFactory; 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; @@ -28,19 +29,19 @@ class RedirectConditionTest extends TestCase } #[Test] - #[TestWith([null, '', false])] // no accept language - #[TestWith(['', '', false])] // empty accept language - #[TestWith(['*', '', false])] // wildcard accept language - #[TestWith(['en', 'en', true])] // single language match - #[TestWith(['es, en,fr', 'en', true])] // multiple languages match - #[TestWith(['es, en-US,fr', 'EN', true])] // multiple locales match - #[TestWith(['es_ES', 'es-ES', true])] // single locale match - #[TestWith(['en-US,es-ES;q=0.6', 'es-ES', false])] // too low quality - #[TestWith(['en-US,es-ES;q=0.9', 'es-ES', true])] // quality high enough - #[TestWith(['en-UK', 'en-uk', true])] // different casing match - #[TestWith(['en-UK', 'en', true])] // only lang - #[TestWith(['es-AR', 'en', false])] // different only lang - #[TestWith(['fr', 'fr-FR', false])] // less restrictive matching locale + #[TestWith([null, '', false], 'no accept language')] + #[TestWith(['', '', false], 'empty accept language')] + #[TestWith(['*', '', false], 'wildcard accept language')] + #[TestWith(['en', 'en', true], 'single language match')] + #[TestWith(['es, en,fr', 'en', true], 'multiple languages match')] + #[TestWith(['es, en-US,fr', 'EN', true], 'multiple locales match')] + #[TestWith(['es_ES', 'es-ES', true], 'single locale match')] + #[TestWith(['en-US,es-ES;q=0.6', 'es-ES', false], 'too low quality')] + #[TestWith(['en-US,es-ES;q=0.9', 'es-ES', true], 'quality high enough')] + #[TestWith(['en-UK', 'en-uk', true], 'different casing match')] + #[TestWith(['en-UK', 'en', true], 'only lang')] + #[TestWith(['es-AR', 'en', false], 'different only lang')] + #[TestWith(['fr', 'fr-FR', false], 'less restrictive matching locale')] public function matchesLanguage(?string $acceptLanguage, string $value, bool $expected): void { $request = ServerRequestFactory::fromGlobals(); @@ -72,4 +73,24 @@ class RedirectConditionTest extends TestCase self::assertEquals($expected, $result); } + + #[Test] + #[TestWith([null, '1.2.3.4', false], 'no remote IP address')] + #[TestWith(['1.2.3.4', '1.2.3.4', true], 'static IP address match')] + #[TestWith(['4.3.2.1', '1.2.3.4', false], 'no static IP address match')] + #[TestWith(['192.168.1.35', '192.168.1.0/24', true], 'CIDR block match')] + #[TestWith(['1.2.3.4', '192.168.1.0/24', false], 'no CIDR block match')] + #[TestWith(['192.168.1.35', '192.168.1.*', true], 'wildcard pattern match')] + #[TestWith(['1.2.3.4', '192.168.1.*', false], 'no wildcard pattern match')] + public function matchesRemoteIpAddress(?string $remoteIp, string $ipToMatch, bool $expected): void + { + $request = ServerRequestFactory::fromGlobals(); + if ($remoteIp !== null) { + $request = $request->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, $remoteIp); + } + + $result = RedirectCondition::forIpAddress($ipToMatch)->matchesRequest($request); + + self::assertEquals($expected, $result); + } } diff --git a/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php b/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php index d61bc6fa..dc19dcd3 100644 --- a/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php +++ b/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php @@ -1,17 +1,20 @@ createRule($conditions); $result = $rule->mapConditions($callback); @@ -78,10 +84,22 @@ class ShortUrlRedirectRuleTest extends TestCase 'matchKey' => 'foo', 'matchValue' => 'bar', ], + [ + 'type' => RedirectConditionType::DEVICE->value, + 'matchKey' => null, + 'matchValue' => DeviceType::ANDROID->value, + ], + [ + 'type' => RedirectConditionType::IP_ADDRESS->value, + 'matchKey' => null, + 'matchValue' => '1.2.3.*', + ], ]]; yield 'human-friendly conditions' => [fn (RedirectCondition $cond) => $cond->toHumanFriendly(), [ 'en-UK language is accepted', 'query string contains foo=bar', + sprintf('device is %s', DeviceType::ANDROID->value), + 'IP address matches 1.2.3.*', ]]; } diff --git a/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php b/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php index 5bf435b2..3bf23863 100644 --- a/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php +++ b/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php @@ -9,6 +9,7 @@ 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; @@ -88,5 +89,30 @@ class ShortUrlRedirectionResolverTest extends TestCase RedirectCondition::forQueryParam('foo', 'bar'), 'https://example.com/from-rule', ]; + yield 'matching static IP address' => [ + $request()->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, '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'), + 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'), + RedirectCondition::forIpAddress('1.2.*.*'), + 'https://example.com/from-rule', + ]; + yield 'non-matching IP address' => [ + $request()->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, '4.3.2.1'), + RedirectCondition::forIpAddress('1.2.3.4'), + 'https://example.com/foo/bar', + ]; + yield 'missing remote IP address' => [ + $request(), + RedirectCondition::forIpAddress('1.2.3.4'), + 'https://example.com/foo/bar', + ]; } } From bab6a3951ef91bb63f23a05e822b030ddae33ea9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 Jul 2024 19:56:53 +0200 Subject: [PATCH 4/9] Add missing unit test --- module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php | 2 ++ module/Core/src/Util/IpAddressUtils.php | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php index 0c0b7d12..edd1eae3 100644 --- a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php +++ b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php @@ -116,6 +116,7 @@ class RedirectRuleHandlerTest extends TestCase 'Language to match?' => 'en-US', 'Query param name?' => 'foo', 'Query param value?' => 'bar', + 'IP address, CIDR block or wildcard-pattern (1.2.*.*)' => '1.2.3.4', default => '', }, ); @@ -163,6 +164,7 @@ class RedirectRuleHandlerTest extends TestCase [RedirectCondition::forQueryParam('foo', 'bar'), RedirectCondition::forQueryParam('foo', 'bar')], true, ]; + yield 'IP address' => [RedirectConditionType::IP_ADDRESS, [RedirectCondition::forIpAddress('1.2.3.4')]]; } #[Test] diff --git a/module/Core/src/Util/IpAddressUtils.php b/module/Core/src/Util/IpAddressUtils.php index 79e7f839..f5e1b052 100644 --- a/module/Core/src/Util/IpAddressUtils.php +++ b/module/Core/src/Util/IpAddressUtils.php @@ -26,7 +26,7 @@ final class IpAddressUtils * Matching will happen as follows: * * Static IP address -> strict equality with provided IP address. * * CIDR block -> provided IP address is part of that block. - * * Wildcard -> static parts match the corresponding ones in provided IP address. + * * Wildcard pattern -> static parts match the corresponding ones in provided IP address. * * @param string[] $groups * @throws InvalidIpFormatException From f4a7712ded59cb08b0bb04d6b01777a4ad74ee9b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 Jul 2024 19:59:13 +0200 Subject: [PATCH 5/9] Add InvalidIpFormatExceptionTest --- .../InvalidIpFormatExceptionTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 module/Core/test/Exception/InvalidIpFormatExceptionTest.php diff --git a/module/Core/test/Exception/InvalidIpFormatExceptionTest.php b/module/Core/test/Exception/InvalidIpFormatExceptionTest.php new file mode 100644 index 00000000..6f0c3804 --- /dev/null +++ b/module/Core/test/Exception/InvalidIpFormatExceptionTest.php @@ -0,0 +1,19 @@ +getMessage()); + } +} From 626caa4afa997750d83015e0f2c62bea50f12dfb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 Jul 2024 20:12:56 +0200 Subject: [PATCH 6/9] Add API test for dynamic IP-based redirects --- module/Core/test-api/Action/RedirectTest.php | 12 ++++++++++++ .../Rest/test-api/Action/ListRedirectRulesTest.php | 11 +++++++++++ .../Fixtures/ShortUrlRedirectRulesFixture.php | 8 ++++++++ 3 files changed, 31 insertions(+) diff --git a/module/Core/test-api/Action/RedirectTest.php b/module/Core/test-api/Action/RedirectTest.php index 2c54e8a7..dc6ca174 100644 --- a/module/Core/test-api/Action/RedirectTest.php +++ b/module/Core/test-api/Action/RedirectTest.php @@ -10,6 +10,8 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\TestWith; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use function sprintf; + use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT; use const ShlinkioTest\Shlink\IOS_USER_AGENT; @@ -86,6 +88,16 @@ 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) { + yield sprintf('rule: IP address in "%s" header', $header) => [ + [ + RequestOptions::HEADERS => [$header => '1.2.3.4'], + ], + 'https://example.com/static-ip-address', + ]; + } } /** diff --git a/module/Rest/test-api/Action/ListRedirectRulesTest.php b/module/Rest/test-api/Action/ListRedirectRulesTest.php index 8f4efdda..494a6564 100644 --- a/module/Rest/test-api/Action/ListRedirectRulesTest.php +++ b/module/Rest/test-api/Action/ListRedirectRulesTest.php @@ -87,6 +87,17 @@ class ListRedirectRulesTest extends ApiTestCase ], ], ], + [ + 'longUrl' => 'https://example.com/static-ip-address', + 'priority' => 6, + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => '1.2.3.4', + ], + ], + ], ]])] public function returnsListOfRulesForShortUrl(string $shortCode, array $expectedRules): void { diff --git a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php index b21f86e1..3aa81315 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php @@ -70,6 +70,14 @@ class ShortUrlRedirectRulesFixture extends AbstractFixture implements DependentF ); $manager->persist($iosRule); + $ipAddressRule = new ShortUrlRedirectRule( + shortUrl: $defShortUrl, + priority: 6, + longUrl: 'https://example.com/static-ip-address', + conditions: new ArrayCollection([RedirectCondition::forIpAddress('1.2.3.4')]), + ); + $manager->persist($ipAddressRule); + $manager->flush(); } } From ce2ed237c75d4f0b293d92db69458cc9d2b83597 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 Jul 2024 20:23:58 +0200 Subject: [PATCH 7/9] Add ip-address condition type to redirect rules API spec docs --- docs/swagger/definitions/SetShortUrlRedirectRule.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/swagger/definitions/SetShortUrlRedirectRule.json b/docs/swagger/definitions/SetShortUrlRedirectRule.json index d17bedb3..00074acf 100644 --- a/docs/swagger/definitions/SetShortUrlRedirectRule.json +++ b/docs/swagger/definitions/SetShortUrlRedirectRule.json @@ -15,8 +15,8 @@ "properties": { "type": { "type": "string", - "enum": ["device", "language", "query-param"], - "description": "The type of the condition, which will condition the logic used to match it" + "enum": ["device", "language", "query-param", "ip-address"], + "description": "The type of the condition, which will determine the logic used to match it" }, "matchKey": { "type": ["string", "null"] From 7e2f755dfd26e6cc115a9763a915e877496cdd9f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 Jul 2024 21:23:48 +0200 Subject: [PATCH 8/9] Validate IP address patterns when creating ip-address redirect conditions --- .../Validation/RedirectRulesInputFilter.php | 16 +++-- module/Core/src/Util/IpAddressUtils.php | 31 +++++++-- .../Model/RedirectRulesDataTest.php | 67 +++++++++++++++++++ module/Core/test/Util/IpAddressUtilsTest.php | 26 +++++++ .../test-api/Action/SetRedirectRulesTest.php | 65 ++++++++++++++++-- 5 files changed, 185 insertions(+), 20 deletions(-) create mode 100644 module/Core/test/Util/IpAddressUtilsTest.php diff --git a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php index 5decaf4c..892b93e4 100644 --- a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php +++ b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Common\Validation\InputFactory; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; +use Shlinkio\Shlink\Core\Util\IpAddressUtils; use function Shlinkio\Shlink\Core\ArrayUtils\contains; use function Shlinkio\Shlink\Core\enumValues; @@ -71,13 +72,14 @@ class RedirectRulesInputFilter extends InputFilter $redirectConditionInputFilter->add($type); $value = InputFactory::basic(self::CONDITION_MATCH_VALUE, required: true); - $value->getValidatorChain()->attach(new Callback(function (string $value, array $context) { - if ($context[self::CONDITION_TYPE] === RedirectConditionType::DEVICE->value) { - return contains($value, enumValues(DeviceType::class)); - } - - return true; - })); + $value->getValidatorChain()->attach(new Callback( + fn (string $value, array $context) => match ($context[self::CONDITION_TYPE]) { + RedirectConditionType::DEVICE->value => contains($value, enumValues(DeviceType::class)), + RedirectConditionType::IP_ADDRESS->value => IpAddressUtils::isStaticIpCidrOrWildcard($value), + // RedirectConditionType::LANGUAGE->value => TODO, + default => true, + }, + )); $redirectConditionInputFilter->add($value); $redirectConditionInputFilter->add( diff --git a/module/Core/src/Util/IpAddressUtils.php b/module/Core/src/Util/IpAddressUtils.php index f5e1b052..66354c37 100644 --- a/module/Core/src/Util/IpAddressUtils.php +++ b/module/Core/src/Util/IpAddressUtils.php @@ -18,6 +18,11 @@ use function str_contains; final class IpAddressUtils { + public static function isStaticIpCidrOrWildcard(string $candidate): bool + { + return self::candidateToRange($candidate, ['0', '0', '0', '0']) !== null; + } + /** * Checks if an IP address matches any of provided groups. * Every group can be a static IP address (100.200.80.40), a CIDR block (192.168.10.0/24) or a wildcard pattern @@ -40,15 +45,29 @@ final class IpAddressUtils $ipAddressParts = explode('.', $ipAddress); - return some($groups, function (string $value) use ($ip, $ipAddressParts): bool { - $range = str_contains($value, '*') - ? self::parseValueWithWildcards($value, $ipAddressParts) - : Factory::parseRangeString($value); - - return $range !== null && $ip->matches($range); + return some($groups, function (string $group) use ($ip, $ipAddressParts): bool { + $range = self::candidateToRange($group, $ipAddressParts); + return $range !== null && $range->contains($ip); }); } + /** + * Convert a static IP, CIDR block or wildcard pattern into a Range object + * + * @param string[] $ipAddressParts + */ + private static function candidateToRange(string $candidate, array $ipAddressParts): ?RangeInterface + { + return str_contains($candidate, '*') + ? self::parseValueWithWildcards($candidate, $ipAddressParts) + : Factory::parseRangeString($candidate); + } + + /** + * Try to generate an IP range from a wildcard pattern. + * Factory::parseRangeString can usually do this automatically, but only if wildcards are at the end. This also + * covers cases where wildcards are in between. + */ private static function parseValueWithWildcards(string $value, array $ipAddressParts): ?RangeInterface { $octets = explode('.', $value); diff --git a/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php b/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php index f0ded32b..e71140cb 100644 --- a/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php +++ b/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php @@ -51,9 +51,76 @@ class RedirectRulesDataTest extends TestCase ], ], ]]])] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => 'not an IP address', + ], + ], + ], + ]]])] public function throwsWhenProvidedDataIsInvalid(array $invalidData): void { $this->expectException(ValidationException::class); RedirectRulesData::fromRawData($invalidData); } + + #[Test] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => '1.2.3.4', + ], + ], + ], + ]]], 'static IP')] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => '1.2.3.0/24', + ], + ], + ], + ]]], 'CIDR block')] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => '1.2.3.*', + ], + ], + ], + ]]], 'IP wildcard pattern')] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => '1.2.*.4', + ], + ], + ], + ]]], 'in-between IP wildcard pattern')] + public function allowsValidDataToBeSet(array $validData): void + { + $result = RedirectRulesData::fromRawData($validData); + self::assertEquals($result->rules, $validData['redirectRules']); + } } diff --git a/module/Core/test/Util/IpAddressUtilsTest.php b/module/Core/test/Util/IpAddressUtilsTest.php new file mode 100644 index 00000000..c1045e68 --- /dev/null +++ b/module/Core/test/Util/IpAddressUtilsTest.php @@ -0,0 +1,26 @@ +callApiWithKey(self::METHOD_POST, '/short-urls/invalid/redirect-rules'); $payload = $this->getJsonResponsePayload($response); @@ -39,16 +39,67 @@ class SetRedirectRulesTest extends ApiTestCase } #[Test] - public function errorIsReturnedWhenInvalidDataProvided(): void - { - $response = $this->callApiWithKey(self::METHOD_POST, '/short-urls/abc123/redirect-rules', [ - RequestOptions::JSON => [ - 'redirectRules' => [ + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'invalid', + ], + ], + ]], 'invalid long URL')] + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => 'foo', + ], + ], + ]], 'non-array conditions')] + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ [ - 'longUrl' => 'invalid', + 'type' => 'invalid', + 'matchKey' => null, + 'matchValue' => 'foo', ], ], ], + ], + ]], 'invalid condition type')] + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'device', + 'matchValue' => 'invalid-device', + 'matchKey' => null, + ], + ], + ], + ], + ]], 'invalid device type')] + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => 'not an IP address', + ], + ], + ], + ], + ]], 'invalid IP address')] + public function errorIsReturnedWhenInvalidDataIsProvided(array $bodyPayload): void + { + $response = $this->callApiWithKey(self::METHOD_POST, '/short-urls/abc123/redirect-rules', [ + RequestOptions::JSON => $bodyPayload, ]); $payload = $this->getJsonResponsePayload($response); From 9e6cdcb838e6bda4b15f35ed898f8f566f2d64cd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 Jul 2024 21:26:28 +0200 Subject: [PATCH 9/9] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ff88fcf..ec85e5cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added +* [#2120](https://github.com/shlinkio/shlink/issues/2120) Add new IP address condition for the dynamic rules redirections system. + + The conditions allow you to define IP addresses to match as static IP (1.2.3.4), CIDR block (192.168.1.0/24) or wildcard pattern (1.2.\*.\*). + * [#2018](https://github.com/shlinkio/shlink/issues/2018) Add option to allow all short URLs to be unconditionally crawlable in robots.txt, via `ROBOTS_ALLOW_ALL_SHORT_URLS=true` env var, or config option. * [#2109](https://github.com/shlinkio/shlink/issues/2109) Add option to customize user agents robots.txt, via `ROBOTS_USER_AGENTS=foo,bar,baz` env var, or config option.