Skip to content

Commit

Permalink
Merge pull request #2277 from acelaya-forks/feature/ip-address-factory
Browse files Browse the repository at this point in the history
Use `IpAddressFactory` from akrabat/ip-address-middleware
  • Loading branch information
acelaya authored Nov 22, 2024
2 parents b2bfe97 + 2946b63 commit 8ee9058
Show file tree
Hide file tree
Showing 10 changed files with 62 additions and 50 deletions.
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
20 changes: 0 additions & 20 deletions config/autoload/client-detection.global.php

This file was deleted.

37 changes: 37 additions & 0 deletions config/autoload/ip-address.global.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

use RKA\Middleware\IpAddress;
use RKA\Middleware\Mezzio\IpAddressFactory;

use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE;

return [

// Configuration for RKA\Middleware\IpAddress
'rka' => [
'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,
],
],

];
1 change: 1 addition & 0 deletions config/constants.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@
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';
5 changes: 3 additions & 2 deletions module/Core/functions/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
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;
Expand All @@ -38,6 +37,8 @@
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;
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions module/Core/test-api/Action/RedirectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ public static function provideRequestOptions(): iterable
'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'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
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;
Expand All @@ -24,6 +23,8 @@
use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
use Throwable;

use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE;

class IpGeolocationMiddlewareTest extends TestCase
{
private MockObject & IpLocationResolverInterface $ipLocationResolver;
Expand Down Expand Up @@ -76,10 +77,7 @@ public function emptyLocationIsReturnedIfIpAddressIsNotLocatable(string|null $ip
$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);
Expand All @@ -104,10 +102,7 @@ public function locationIsResolvedFromIpAddress(): void
);
$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);
Expand Down Expand Up @@ -147,10 +142,7 @@ public function warningIsPrintedIfAnErrorOccurs(
->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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
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;
Expand Down Expand Up @@ -88,7 +88,7 @@ public function matchesRemoteIpAddress(string|null $remoteIp, string $ipToMatch,
{
$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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
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;
Expand All @@ -18,6 +17,7 @@
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;
Expand Down Expand Up @@ -90,22 +90,22 @@ public static function provideData(): iterable
'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',
];
Expand Down
11 changes: 6 additions & 5 deletions module/Core/test/Visit/RequestTrackerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@
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;
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';
Expand Down Expand Up @@ -67,15 +68,15 @@ public static function provideNonTrackingRequests(): iterable
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',
)];
}
Expand All @@ -102,7 +103,7 @@ public function trackingHappensOverShortUrlsWhenRemoteAddressIsInvalid(): void
);

$this->requestTracker->trackIfApplicable($shortUrl, ServerRequestFactory::fromGlobals()->withAttribute(
IpAddressMiddlewareFactory::REQUEST_ATTR,
IP_ADDRESS_REQUEST_ATTRIBUTE,
'invalid',
));
}
Expand Down

0 comments on commit 8ee9058

Please sign in to comment.