Skip to content

Commit

Permalink
Merge pull request #2263 from acelaya-forks/feature/geolocation-city-…
Browse files Browse the repository at this point in the history
…name-redirects

Add support for city name dynamic redirects
  • Loading branch information
acelaya authored Nov 14, 2024
2 parents dbef32f + a6e0916 commit 1fee745
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
* [#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.
* `geolocation-city-name`: Allows to perform redirections based on the city name 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.
Expand Down
9 changes: 8 additions & 1 deletion docs/swagger/definitions/SetShortUrlRedirectRule.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@
"properties": {
"type": {
"type": "string",
"enum": ["device", "language", "query-param", "ip-address", "geolocation-country-code"],
"enum": [
"device",
"language",
"query-param",
"ip-address",
"geolocation-country-code",
"geolocation-city-name"
],
"description": "The type of the condition, which will determine the logic used to match it"
},
"matchKey": {
Expand Down
3 changes: 3 additions & 0 deletions module/CLI/src/RedirectRule/RedirectRuleHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ private function addRule(ShortUrl $shortUrl, StyleInterface $io, array $currentR
),
RedirectConditionType::GEOLOCATION_COUNTRY_CODE => RedirectCondition::forGeolocationCountryCode(
$this->askMandatory('Country code to match?', $io),
),
RedirectConditionType::GEOLOCATION_CITY_NAME => RedirectCondition::forGeolocationCityName(
$this->askMandatory('City name to match?', $io),
)
};

Expand Down
5 changes: 5 additions & 0 deletions module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public function newRulesCanBeAdded(
'Query param value?' => 'bar',
'IP address, CIDR block or wildcard-pattern (1.2.*.*)' => '1.2.3.4',
'Country code to match?' => 'FR',
'City name to match?' => 'Los angeles',
default => '',
},
);
Expand Down Expand Up @@ -170,6 +171,10 @@ public static function provideDeviceConditions(): iterable
RedirectConditionType::GEOLOCATION_COUNTRY_CODE,
[RedirectCondition::forGeolocationCountryCode('FR')],
];
yield 'Geolocation city name' => [
RedirectConditionType::GEOLOCATION_CITY_NAME,
[RedirectCondition::forGeolocationCityName('Los angeles')],
];
}

#[Test]
Expand Down
19 changes: 19 additions & 0 deletions module/Core/src/RedirectRule/Entity/RedirectCondition.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ public static function forGeolocationCountryCode(string $countryCode): self
return new self(RedirectConditionType::GEOLOCATION_COUNTRY_CODE, $countryCode);
}

public static function forGeolocationCityName(string $cityName): self
{
return new self(RedirectConditionType::GEOLOCATION_CITY_NAME, $cityName);
}

public static function fromRawData(array $rawData): self
{
$type = RedirectConditionType::from($rawData[RedirectRulesInputFilter::CONDITION_TYPE]);
Expand All @@ -79,6 +84,7 @@ public function matchesRequest(ServerRequestInterface $request): bool
RedirectConditionType::DEVICE => $this->matchesDevice($request),
RedirectConditionType::IP_ADDRESS => $this->matchesRemoteIpAddress($request),
RedirectConditionType::GEOLOCATION_COUNTRY_CODE => $this->matchesGeolocationCountryCode($request),
RedirectConditionType::GEOLOCATION_CITY_NAME => $this->matchesGeolocationCityName($request),
};
}

Expand Down Expand Up @@ -137,6 +143,18 @@ private function matchesGeolocationCountryCode(ServerRequestInterface $request):
return strcasecmp($geolocation->countryCode, $this->matchValue) === 0;
}

private function matchesGeolocationCityName(ServerRequestInterface $request): bool
{
$geolocation = $request->getAttribute(Location::class);
// TODO We should eventually rely on `Location` type only
if (! ($geolocation instanceof Location) && ! ($geolocation instanceof VisitLocation)) {
return false;
}

$cityName = $geolocation instanceof Location ? $geolocation->city : $geolocation->cityName;
return strcasecmp($cityName, $this->matchValue) === 0;
}

public function jsonSerialize(): array
{
return [
Expand All @@ -158,6 +176,7 @@ public function toHumanFriendly(): string
),
RedirectConditionType::IP_ADDRESS => sprintf('IP address matches %s', $this->matchValue),
RedirectConditionType::GEOLOCATION_COUNTRY_CODE => sprintf('country code is %s', $this->matchValue),
RedirectConditionType::GEOLOCATION_CITY_NAME => sprintf('city name is %s', $this->matchValue),
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ enum RedirectConditionType: string
case QUERY_PARAM = 'query-param';
case IP_ADDRESS = 'ip-address';
case GEOLOCATION_COUNTRY_CODE = 'geolocation-country-code';
case GEOLOCATION_CITY_NAME = 'geolocation-city-name';

/**
* Tells if a value is valid for the condition type
Expand Down
33 changes: 31 additions & 2 deletions module/Core/test/RedirectRule/Entity/RedirectConditionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public function matchesRemoteIpAddress(string|null $remoteIp, string $ipToMatch,
self::assertEquals($expected, $result);
}

#[Test, DataProvider('provideVisits')]
#[Test, DataProvider('provideVisitsWithCountry')]
public function matchesGeolocationCountryCode(
Location|VisitLocation|null $location,
string $countryCodeToMatch,
Expand All @@ -108,7 +108,7 @@ public function matchesGeolocationCountryCode(

self::assertEquals($expected, $result);
}
public static function provideVisits(): iterable
public static function provideVisitsWithCountry(): iterable
{
yield 'no location' => [null, 'US', false];
yield 'non-matching location' => [new Location(countryCode: 'ES'), 'US', false];
Expand All @@ -125,4 +125,33 @@ public static function provideVisits(): iterable
true,
];
}

#[Test, DataProvider('provideVisitsWithCity')]
public function matchesGeolocationCityName(
Location|VisitLocation|null $location,
string $cityNameToMatch,
bool $expected,
): void {
$request = ServerRequestFactory::fromGlobals()->withAttribute(Location::class, $location);
$result = RedirectCondition::forGeolocationCityName($cityNameToMatch)->matchesRequest($request);

self::assertEquals($expected, $result);
}
public static function provideVisitsWithCity(): iterable
{
yield 'no location' => [null, 'New York', false];
yield 'non-matching location' => [new Location(city: 'Los Angeles'), 'New York', false];
yield 'matching location' => [new Location(city: 'Madrid'), 'Madrid', true];
yield 'matching case-insensitive' => [new Location(city: 'Los Angeles'), 'los angeles', true];
yield 'matching visit location' => [
VisitLocation::fromGeolocation(new Location(city: 'New York')),
'New York',
true,
];
yield 'matching visit case-insensitive' => [
VisitLocation::fromGeolocation(new Location(city: 'barcelona')),
'Barcelona',
true,
];
}
}

0 comments on commit 1fee745

Please sign in to comment.