From 781c083c9fb461bf7359a381a841bf0b8f9be992 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 12 Nov 2024 08:37:22 +0100 Subject: [PATCH 1/7] Add new geolocatio-country-code redirect condition type --- composer.json | 2 +- .../src/RedirectRule/RedirectRuleHandler.php | 3 +++ .../RedirectRule/Entity/RedirectCondition.php | 22 +++++++++++++++++-- .../Model/RedirectConditionType.php | 1 + .../Entity/RedirectConditionTest.php | 18 +++++++++++++++ 5 files changed, 43 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 88e94946..76d94a07 100644 --- a/composer.json +++ b/composer.json @@ -49,7 +49,7 @@ "shlinkio/shlink-event-dispatcher": "^4.1", "shlinkio/shlink-importer": "^5.3.2", "shlinkio/shlink-installer": "^9.2", - "shlinkio/shlink-ip-geolocation": "^4.1", + "shlinkio/shlink-ip-geolocation": "dev-main#fadae5d as 4.2", "shlinkio/shlink-json": "^1.1", "spiral/roadrunner": "^2024.1", "spiral/roadrunner-cli": "^2.6", diff --git a/module/CLI/src/RedirectRule/RedirectRuleHandler.php b/module/CLI/src/RedirectRule/RedirectRuleHandler.php index 924876fc..f72d1ed0 100644 --- a/module/CLI/src/RedirectRule/RedirectRuleHandler.php +++ b/module/CLI/src/RedirectRule/RedirectRuleHandler.php @@ -111,6 +111,9 @@ class RedirectRuleHandler implements RedirectRuleHandlerInterface RedirectConditionType::IP_ADDRESS => RedirectCondition::forIpAddress( $this->askMandatory('IP address, CIDR block or wildcard-pattern (1.2.*.*)', $io), ), + RedirectConditionType::GEOLOCATION_COUNTRY_CODE => RedirectCondition::forGeolocationCountryCode( + $this->askMandatory('Country code to match?', $io), + ) }; $continue = $io->confirm('Do you want to add another condition?'); diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index 99f5fb9c..affa994a 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -9,6 +9,7 @@ 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 Shlinkio\Shlink\IpGeolocation\Model\Location; use function Shlinkio\Shlink\Core\acceptLanguageToLocales; use function Shlinkio\Shlink\Core\ArrayUtils\some; @@ -16,7 +17,7 @@ use function Shlinkio\Shlink\Core\ipAddressFromRequest; use function Shlinkio\Shlink\Core\normalizeLocale; use function Shlinkio\Shlink\Core\splitLocale; use function sprintf; -use function strtolower; +use function strcasecmp; use function trim; class RedirectCondition extends AbstractEntity implements JsonSerializable @@ -52,6 +53,11 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable return new self(RedirectConditionType::IP_ADDRESS, $ipAddressPattern); } + public static function forGeolocationCountryCode(string $countryCode): self + { + return new self(RedirectConditionType::GEOLOCATION_COUNTRY_CODE, $countryCode); + } + public static function fromRawData(array $rawData): self { $type = RedirectConditionType::from($rawData[RedirectRulesInputFilter::CONDITION_TYPE]); @@ -71,6 +77,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable RedirectConditionType::LANGUAGE => $this->matchesLanguage($request), RedirectConditionType::DEVICE => $this->matchesDevice($request), RedirectConditionType::IP_ADDRESS => $this->matchesRemoteIpAddress($request), + RedirectConditionType::GEOLOCATION_COUNTRY_CODE => $this->matchesGeolocationCountryCode($request), }; } @@ -109,7 +116,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable private function matchesDevice(ServerRequestInterface $request): bool { $device = DeviceType::matchFromUserAgent($request->getHeaderLine('User-Agent')); - return $device !== null && $device->value === strtolower($this->matchValue); + return $device !== null && $device->value === $this->matchValue; } private function matchesRemoteIpAddress(ServerRequestInterface $request): bool @@ -118,6 +125,16 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable return $remoteAddress !== null && IpAddressUtils::ipAddressMatchesGroups($remoteAddress, [$this->matchValue]); } + private function matchesGeolocationCountryCode(ServerRequestInterface $request): bool + { + $geolocation = $request->getAttribute(Location::class); + if (!($geolocation instanceof Location)) { + return false; + } + + return strcasecmp($geolocation->countryCode, $this->matchValue) === 0; + } + public function jsonSerialize(): array { return [ @@ -138,6 +155,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable $this->matchValue, ), RedirectConditionType::IP_ADDRESS => sprintf('IP address matches %s', $this->matchValue), + RedirectConditionType::GEOLOCATION_COUNTRY_CODE => sprintf('country code is %s', $this->matchValue), }; } } diff --git a/module/Core/src/RedirectRule/Model/RedirectConditionType.php b/module/Core/src/RedirectRule/Model/RedirectConditionType.php index 891a8ccc..ed587ffa 100644 --- a/module/Core/src/RedirectRule/Model/RedirectConditionType.php +++ b/module/Core/src/RedirectRule/Model/RedirectConditionType.php @@ -8,4 +8,5 @@ enum RedirectConditionType: string case LANGUAGE = 'language'; case QUERY_PARAM = 'query-param'; case IP_ADDRESS = 'ip-address'; + case GEOLOCATION_COUNTRY_CODE = 'geolocation-country-code'; } diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index b31d1fd3..81b69fe5 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -3,12 +3,14 @@ namespace ShlinkioTest\Shlink\Core\RedirectRule\Entity; use Laminas\Diactoros\ServerRequestFactory; +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 ShlinkioTest\Shlink\ANDROID_USER_AGENT; use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT; @@ -93,4 +95,20 @@ class RedirectConditionTest extends TestCase self::assertEquals($expected, $result); } + + #[Test, DataProvider('provideVisits')] + public function matchesGeolocationCountryCode(Location|null $location, string $countryCodeToMatch, bool $expected): void + { + $request = ServerRequestFactory::fromGlobals()->withAttribute(Location::class, $location); + $result = RedirectCondition::forGeolocationCountryCode($countryCodeToMatch)->matchesRequest($request); + + self::assertEquals($expected, $result); + } + public static function provideVisits(): iterable + { + yield 'no location' => [null, 'US', false]; + yield 'non-matching location' => [new Location(countryCode: 'ES'), 'US', false]; + yield 'matching location' => [new Location(countryCode: 'US'), 'US', true]; + yield 'matching case-insensitive' => [new Location(countryCode: 'US'), 'us', true]; + } } From b5b5f92eda064923ec999e25b33f25137f3ab957 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 12 Nov 2024 09:52:09 +0100 Subject: [PATCH 2/7] Add validation for country-code redirect conditions --- .../definitions/SetShortUrlRedirectRule.json | 2 +- .../Model/RedirectConditionType.php | 37 +++++++++++++++++++ .../Validation/RedirectRulesInputFilter.php | 11 ++---- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/docs/swagger/definitions/SetShortUrlRedirectRule.json b/docs/swagger/definitions/SetShortUrlRedirectRule.json index 00074acf..5ff6371c 100644 --- a/docs/swagger/definitions/SetShortUrlRedirectRule.json +++ b/docs/swagger/definitions/SetShortUrlRedirectRule.json @@ -15,7 +15,7 @@ "properties": { "type": { "type": "string", - "enum": ["device", "language", "query-param", "ip-address"], + "enum": ["device", "language", "query-param", "ip-address", "geolocation-country-code"], "description": "The type of the condition, which will determine the logic used to match it" }, "matchKey": { diff --git a/module/Core/src/RedirectRule/Model/RedirectConditionType.php b/module/Core/src/RedirectRule/Model/RedirectConditionType.php index ed587ffa..73f11a27 100644 --- a/module/Core/src/RedirectRule/Model/RedirectConditionType.php +++ b/module/Core/src/RedirectRule/Model/RedirectConditionType.php @@ -2,6 +2,12 @@ namespace Shlinkio\Shlink\Core\RedirectRule\Model; +use Shlinkio\Shlink\Core\Model\DeviceType; +use Shlinkio\Shlink\Core\Util\IpAddressUtils; + +use function Shlinkio\Shlink\Core\ArrayUtils\contains; +use function Shlinkio\Shlink\Core\enumValues; + enum RedirectConditionType: string { case DEVICE = 'device'; @@ -9,4 +15,35 @@ enum RedirectConditionType: string case QUERY_PARAM = 'query-param'; case IP_ADDRESS = 'ip-address'; case GEOLOCATION_COUNTRY_CODE = 'geolocation-country-code'; + + /** + * Tells if a value is valid for the condition type + */ + public function isValid(string $value): bool + { + return match ($this) { + RedirectConditionType::DEVICE => contains($value, enumValues(DeviceType::class)), + // RedirectConditionType::LANGUAGE => TODO Validate at least format, + RedirectConditionType::IP_ADDRESS => IpAddressUtils::isStaticIpCidrOrWildcard($value), + RedirectConditionType::GEOLOCATION_COUNTRY_CODE => contains($value, [ + 'AF', 'AX', 'AL', 'DZ', 'AS', 'AD', 'AO', 'AI', 'AQ', 'AG', 'AR', 'AM', 'AW', 'AU', 'AT', 'AZ', + 'BS', 'BH', 'BD', 'BB', 'BY', 'BE', 'BZ', 'BJ', 'BM', 'BT', 'BO', 'BQ', 'BA', 'BW', 'BV', 'BR', + 'IO', 'BN', 'BG', 'BF', 'BI', 'CV', 'KH', 'CM', 'CA', 'KY', 'CF', 'TD', 'CL', 'CN', 'CX', 'CC', + 'CO', 'KM', 'CG', 'CD', 'CK', 'CR', 'CI', 'HR', 'CU', 'CW', 'CY', 'CZ', 'DK', 'DJ', 'DM', 'DO', + 'EC', 'EG', 'SV', 'GQ', 'ER', 'EE', 'SZ', 'ET', 'FK', 'FO', 'FJ', 'FI', 'FR', 'GF', 'PF', 'TF', + 'GA', 'GM', 'GE', 'DE', 'GH', 'GI', 'GR', 'GL', 'GD', 'GP', 'GU', 'GT', 'GG', 'GN', 'GW', 'GY', + 'HT', 'HM', 'VA', 'HN', 'HK', 'HU', 'IS', 'IN', 'ID', 'IR', 'IQ', 'IE', 'IM', 'IL', 'IT', 'JM', + 'JP', 'JE', 'JO', 'KZ', 'KE', 'KI', 'KP', 'KR', 'KW', 'KG', 'LA', 'LV', 'LB', 'LS', 'LR', 'LY', + 'LI', 'LT', 'LU', 'MO', 'MG', 'MW', 'MY', 'MV', 'ML', 'MT', 'MH', 'MQ', 'MR', 'MU', 'YT', 'MX', + 'FM', 'MD', 'MC', 'MN', 'ME', 'MS', 'MA', 'MZ', 'MM', 'NA', 'NR', 'NP', 'NL', 'NC', 'NZ', 'NI', + 'NE', 'NG', 'NU', 'NF', 'MK', 'MP', 'NO', 'OM', 'PK', 'PW', 'PS', 'PA', 'PG', 'PY', 'PE', 'PH', + 'PN', 'PL', 'PT', 'PR', 'QA', 'RE', 'RO', 'RU', 'RW', 'BL', 'SH', 'KN', 'LC', 'MF', 'PM', 'VC', + 'WS', 'SM', 'ST', 'SA', 'SN', 'RS', 'SC', 'SL', 'SG', 'SX', 'SK', 'SI', 'SB', 'SO', 'ZA', 'GS', + 'SS', 'ES', 'LK', 'SD', 'SR', 'SJ', 'SE', 'CH', 'SY', 'TW', 'TJ', 'TZ', 'TH', 'TL', 'TG', 'TK', + 'TO', 'TT', 'TN', 'TR', 'TM', 'TC', 'TV', 'UG', 'UA', 'AE', 'GB', 'US', 'UM', 'UY', 'UZ', 'VU', + 'VE', 'VN', 'VG', 'VI', 'WF', 'EH', 'YE', 'ZM', 'ZW', + ]), + default => true, + }; + } } diff --git a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php index c2fee661..42520a97 100644 --- a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php +++ b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php @@ -9,12 +9,9 @@ use Laminas\InputFilter\InputFilter; use Laminas\Validator\Callback; use Laminas\Validator\InArray; 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; /** @extends InputFilter */ @@ -80,11 +77,9 @@ class RedirectRulesInputFilter extends InputFilter $value = InputFactory::basic(self::CONDITION_MATCH_VALUE, required: 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, + function (string $value, array $context): bool { + $conditionType = RedirectConditionType::tryFrom($context[self::CONDITION_TYPE]); + return $conditionType === null || $conditionType->isValid($value); }, )); $redirectConditionInputFilter->add($value); From f2371b612424e191e75bf97f81ece65956804806 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 13 Nov 2024 10:00:36 +0100 Subject: [PATCH 3/7] Update RedirectRuleHandlerTest --- module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php | 5 +++++ .../test/RedirectRule/Entity/RedirectConditionTest.php | 7 +++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php index 18713e00..99a2167b 100644 --- a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php +++ b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php @@ -117,6 +117,7 @@ class RedirectRuleHandlerTest extends TestCase 'Query param name?' => 'foo', 'Query param value?' => 'bar', 'IP address, CIDR block or wildcard-pattern (1.2.*.*)' => '1.2.3.4', + 'Country code to match?' => 'FR', default => '', }, ); @@ -165,6 +166,10 @@ class RedirectRuleHandlerTest extends TestCase true, ]; yield 'IP address' => [RedirectConditionType::IP_ADDRESS, [RedirectCondition::forIpAddress('1.2.3.4')]]; + yield 'Geolocation country code' => [ + RedirectConditionType::GEOLOCATION_COUNTRY_CODE, + [RedirectCondition::forGeolocationCountryCode('FR')], + ]; } #[Test] diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index 81b69fe5..895b8236 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -97,8 +97,11 @@ class RedirectConditionTest extends TestCase } #[Test, DataProvider('provideVisits')] - public function matchesGeolocationCountryCode(Location|null $location, string $countryCodeToMatch, bool $expected): void - { + public function matchesGeolocationCountryCode( + Location|null $location, + string $countryCodeToMatch, + bool $expected, + ): void { $request = ServerRequestFactory::fromGlobals()->withAttribute(Location::class, $location); $result = RedirectCondition::forGeolocationCountryCode($countryCodeToMatch)->matchesRequest($request); From 4619ebd0145e508ff8ba75a08d05601eef4317df Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 14 Nov 2024 08:20:20 +0100 Subject: [PATCH 4/7] After tracking a visit, set its location in the request as attribute --- module/Core/src/Action/AbstractTrackingAction.php | 8 ++++++-- module/Core/src/Action/RedirectAction.php | 3 +-- .../src/RedirectRule/Entity/RedirectCondition.php | 4 +++- .../Middleware/ExtraPathRedirectMiddleware.php | 9 +++++++-- .../RedirectRule/Entity/RedirectConditionTest.php | 13 ++++++++++++- 5 files changed, 29 insertions(+), 8 deletions(-) diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 78eebc05..b7ddb69a 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -15,6 +15,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use Shlinkio\Shlink\IpGeolocation\Model\Location; abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface { @@ -30,9 +31,12 @@ abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMet try { $shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier); - $this->requestTracker->trackIfApplicable($shortUrl, $request); + $visit = $this->requestTracker->trackIfApplicable($shortUrl, $request); - return $this->createSuccessResp($shortUrl, $request); + return $this->createSuccessResp( + $shortUrl, + $request->withAttribute(Location::class, $visit?->getVisitLocation()), + ); } catch (ShortUrlNotFoundException) { return $this->createErrorResp($request, $handler); } diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 942cf550..a929f290 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Action; -use Fig\Http\Message\StatusCodeInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -13,7 +12,7 @@ use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; -class RedirectAction extends AbstractTrackingAction implements StatusCodeInterface +class RedirectAction extends AbstractTrackingAction { public function __construct( ShortUrlResolverInterface $urlResolver, diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index affa994a..3782f0ef 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -9,6 +9,7 @@ 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 Shlinkio\Shlink\Core\Visit\Entity\VisitLocation; use Shlinkio\Shlink\IpGeolocation\Model\Location; use function Shlinkio\Shlink\Core\acceptLanguageToLocales; @@ -128,7 +129,8 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable private function matchesGeolocationCountryCode(ServerRequestInterface $request): bool { $geolocation = $request->getAttribute(Location::class); - if (!($geolocation instanceof Location)) { + // TODO We should eventually rely on `Location` type only + if (! ($geolocation instanceof Location) && ! ($geolocation instanceof VisitLocation)) { return false; } diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 4a02f6e9..2b4ac7cc 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -17,6 +17,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use Shlinkio\Shlink\IpGeolocation\Model\Location; use function array_slice; use function count; @@ -73,9 +74,13 @@ readonly class ExtraPathRedirectMiddleware implements MiddlewareInterface try { $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); - $this->requestTracker->trackIfApplicable($shortUrl, $request); + $visit = $this->requestTracker->trackIfApplicable($shortUrl, $request); - $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath); + $longUrl = $this->redirectionBuilder->buildShortUrlRedirect( + $shortUrl, + $request->withAttribute(Location::class, $visit?->getVisitLocation()), + $extraPath, + ); return $this->redirectResponseHelper->buildRedirectResponse($longUrl); } catch (ShortUrlNotFoundException) { if ($extraPath === null || ! $this->urlShortenerOptions->multiSegmentSlugsEnabled) { diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index 895b8236..179d35e9 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -10,6 +10,7 @@ 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\Core\Visit\Entity\VisitLocation; use Shlinkio\Shlink\IpGeolocation\Model\Location; use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; @@ -98,7 +99,7 @@ class RedirectConditionTest extends TestCase #[Test, DataProvider('provideVisits')] public function matchesGeolocationCountryCode( - Location|null $location, + Location|VisitLocation|null $location, string $countryCodeToMatch, bool $expected, ): void { @@ -113,5 +114,15 @@ class RedirectConditionTest extends TestCase yield 'non-matching location' => [new Location(countryCode: 'ES'), 'US', false]; yield 'matching location' => [new Location(countryCode: 'US'), 'US', true]; yield 'matching case-insensitive' => [new Location(countryCode: 'US'), 'us', true]; + yield 'matching visit location' => [ + VisitLocation::fromGeolocation(new Location(countryCode: 'US')), + 'US', + true, + ]; + yield 'matching visit case-insensitive' => [ + VisitLocation::fromGeolocation(new Location(countryCode: 'es')), + 'ES', + true, + ]; } } From 51d838870d77011f761006f83cb0cee2e3c1d2a2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 14 Nov 2024 09:14:17 +0100 Subject: [PATCH 5/7] Add reference to ISO 3166-1 alpha-2 country codes wikipedia page --- module/Core/src/RedirectRule/Model/RedirectConditionType.php | 1 + 1 file changed, 1 insertion(+) diff --git a/module/Core/src/RedirectRule/Model/RedirectConditionType.php b/module/Core/src/RedirectRule/Model/RedirectConditionType.php index 73f11a27..6e685709 100644 --- a/module/Core/src/RedirectRule/Model/RedirectConditionType.php +++ b/module/Core/src/RedirectRule/Model/RedirectConditionType.php @@ -26,6 +26,7 @@ enum RedirectConditionType: string // RedirectConditionType::LANGUAGE => TODO Validate at least format, RedirectConditionType::IP_ADDRESS => IpAddressUtils::isStaticIpCidrOrWildcard($value), RedirectConditionType::GEOLOCATION_COUNTRY_CODE => contains($value, [ + // List of ISO 3166-1 alpha-2 two-letter country codes https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2 'AF', 'AX', 'AL', 'DZ', 'AS', 'AD', 'AO', 'AI', 'AQ', 'AG', 'AR', 'AM', 'AW', 'AU', 'AT', 'AZ', 'BS', 'BH', 'BD', 'BB', 'BY', 'BE', 'BZ', 'BJ', 'BM', 'BT', 'BO', 'BQ', 'BA', 'BW', 'BV', 'BR', 'IO', 'BN', 'BG', 'BF', 'BI', 'CV', 'KH', 'CM', 'CA', 'KY', 'CF', 'TD', 'CL', 'CN', 'CX', 'CC', From fd34332e69e6f778fff939844fece0a6a9943eae Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 14 Nov 2024 09:17:41 +0100 Subject: [PATCH 6/7] Improve ExtraPathRedirectMiddlewareTest --- CHANGELOG.md | 4 ++++ .../Middleware/ExtraPathRedirectMiddlewareTest.php | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 032be60e..c0fee24b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this This change applies both to the `GET /short-urls` endpoint, via the `domain` query parameter, and the `short-url:list` console command, via the `--domain`|`-d` flag. +* [#1774](https://github.com/shlinkio/shlink/issues/1774) Add new geolocation redirect rules for the dynamic redirects system. + + * `geolocation-country-code`: Allows to perform redirections based on the ISO 3166-1 alpha-2 two-letter country code resolved while geolocating the visitor. + ### Changed * [#2193](https://github.com/shlinkio/shlink/issues/2193) API keys are now hashed using SHA256, instead of being saved in plain text. diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index 85168020..2e35c1a4 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -9,6 +9,7 @@ use Laminas\Diactoros\ServerRequestFactory; use Laminas\Diactoros\Uri; use Mezzio\Router\Route; use Mezzio\Router\RouteResult; +use PHPUnit\Framework\Assert; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; @@ -26,6 +27,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use Shlinkio\Shlink\IpGeolocation\Model\Location; use function Laminas\Stratigility\middleware; use function str_starts_with; @@ -153,7 +155,10 @@ class ExtraPathRedirectMiddlewareTest extends TestCase ); $this->redirectionBuilder->expects($this->once())->method('buildShortUrlRedirect')->with( $shortUrl, - $this->isInstanceOf(ServerRequestInterface::class), + $this->callback(function (ServerRequestInterface $req) { + Assert::assertArrayHasKey(Location::class, $req->getAttributes()); + return true; + }), $expectedExtraPath, )->willReturn('the_built_long_url'); $this->redirectResponseHelper->expects($this->once())->method('buildRedirectResponse')->with( From 7ddb3e7a70c901618f190809201eb413a5f177e8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 14 Nov 2024 09:40:10 +0100 Subject: [PATCH 7/7] Add tests covering country code validation --- .../Model/RedirectRulesDataTest.php | 24 +++++++++++++++++++ .../test-api/Action/SetRedirectRulesTest.php | 14 +++++++++++ 2 files changed, 38 insertions(+) diff --git a/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php b/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php index e71140cb..d0186faa 100644 --- a/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php +++ b/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php @@ -63,6 +63,18 @@ class RedirectRulesDataTest extends TestCase ], ], ]]])] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'geolocation-country-code', + 'matchKey' => null, + 'matchValue' => 'not an country code', + ], + ], + ], + ]]])] public function throwsWhenProvidedDataIsInvalid(array $invalidData): void { $this->expectException(ValidationException::class); @@ -118,6 +130,18 @@ class RedirectRulesDataTest extends TestCase ], ], ]]], 'in-between IP wildcard pattern')] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'geolocation-country-code', + 'matchKey' => null, + 'matchValue' => 'US', + ], + ], + ], + ]]], 'country code')] public function allowsValidDataToBeSet(array $validData): void { $result = RedirectRulesData::fromRawData($validData); diff --git a/module/Rest/test-api/Action/SetRedirectRulesTest.php b/module/Rest/test-api/Action/SetRedirectRulesTest.php index f096e411..6501ef13 100644 --- a/module/Rest/test-api/Action/SetRedirectRulesTest.php +++ b/module/Rest/test-api/Action/SetRedirectRulesTest.php @@ -96,6 +96,20 @@ class SetRedirectRulesTest extends ApiTestCase ], ], ]], 'invalid IP address')] + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'geolocation-country-code', + 'matchKey' => null, + 'matchValue' => 'not a country code', + ], + ], + ], + ], + ]], 'invalid country code')] public function errorIsReturnedWhenInvalidDataIsProvided(array $bodyPayload): void { $response = $this->callApiWithKey(self::METHOD_POST, '/short-urls/abc123/redirect-rules', [