diff --git a/src/Http/Ip.php b/src/Http/Ip.php index 59d2fba3..dd55f054 100644 --- a/src/Http/Ip.php +++ b/src/Http/Ip.php @@ -61,27 +61,33 @@ public function forwardingAddress(): string|null public function hash(): string { - return hash('sha256', $this->host); + [$host] = $this->parseAddress($this->forwardingAddress ?? $this->host); + + return hash('sha256', $host); } protected function parse(): void { - $address = trim($this->address); + [$this->host, $this->port] = $this->parseAddress($this->address); + } - if (preg_match('/^\[(?[^\]]+)\](?::(?\d+))?$/', $address, $m) === 1) { - $this->host = $m['addr']; - $this->port = isset($m['port']) ? (int) $m['port'] : null; + /** + * @return array{0: string, 1: int|null} + */ + protected function parseAddress(string $address): array + { + $address = trim($address); - return; + if (preg_match('/^\[(?[^\]]+)\](?::(?\d+))?$/', $address, $m) === 1) { + return [$m['addr'], isset($m['port']) ? (int) $m['port'] : null]; } if (filter_var($address, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { - $this->host = $address; - $this->port = null; - - return; + return [$address, null]; } + $result = [$address, null]; + if (str_contains($address, ':')) { [$maybeHost, $maybePort] = explode(':', $address, 2); @@ -89,14 +95,10 @@ protected function parse(): void filter_var($maybeHost, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) || filter_var($maybeHost, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME) ) { - $this->host = $maybeHost; - $this->port = is_numeric($maybePort) ? (int) $maybePort : null; - - return; + $result = [$maybeHost, is_numeric($maybePort) ? (int) $maybePort : null]; } } - $this->host = $address; - $this->port = null; + return $result; } } diff --git a/src/Testing/Concerns/InteractWithResponses.php b/src/Testing/Concerns/InteractWithResponses.php index 5be6a3ae..c51e9fbb 100644 --- a/src/Testing/Concerns/InteractWithResponses.php +++ b/src/Testing/Concerns/InteractWithResponses.php @@ -24,6 +24,15 @@ trait InteractWithResponses { + protected int $redirectsFollowed = 0; + + protected function followingRedirects(): static + { + $this->redirectsFollowed = 10; + + return $this; + } + public function call( HttpMethod $method, string $path, @@ -61,6 +70,7 @@ public function connect( }; $client = (new HttpClientBuilder()) + ->followRedirects($this->redirectsFollowed) ->usingPool(new UnlimitedConnectionPool(new DefaultConnectionFactory($connector))) ->build(); diff --git a/tests/Feature/AuthenticationTest.php b/tests/Feature/AuthenticationTest.php index ffcc74e9..d93a36c1 100644 --- a/tests/Feature/AuthenticationTest.php +++ b/tests/Feature/AuthenticationTest.php @@ -12,6 +12,7 @@ use Phenix\Auth\Middlewares\Guest; use Phenix\Auth\PersonalAccessToken; use Phenix\Auth\User; +use Phenix\Constants\AppMode; use Phenix\Database\Constants\Connection; use Phenix\Facades\Config; use Phenix\Facades\Crypto; @@ -389,6 +390,42 @@ ])->assertStatusCode(HttpStatus::TOO_MANY_REQUESTS)->assertHeaders(['Retry-After' => '300']); }); +it('tracks failed token validation attempts by forwarded client ip behind a trusted proxy', function (): void { + Config::set('app.app_mode', AppMode::PROXIED->value); + Config::set('app.trusted_proxies', ['127.0.0.1/32', '127.0.0.1']); + + $connection = $this->getMockBuilder(MysqlConnectionPool::class)->getMock(); + $connection->expects($this->any()) + ->method('prepare') + ->willReturn( + new Statement(new Result()), + ); + + $this->app->swap(Connection::default(), $connection); + + Route::get('/limited-proxy', fn (): Response => response()->plain('Never reached')) + ->middleware(Authenticated::class); + + $this->app->run(); + + for ($i = 0; $i < 5; $i++) { + $this->get('/limited-proxy', headers: [ + 'Authorization' => 'Bearer invalid-token', + 'X-Forwarded-For' => '203.0.113.10', + ])->assertUnauthorized(); + } + + $this->get('/limited-proxy', headers: [ + 'Authorization' => 'Bearer invalid-token', + 'X-Forwarded-For' => '203.0.113.10', + ])->assertStatusCode(HttpStatus::TOO_MANY_REQUESTS)->assertHeaders(['Retry-After' => '300']); + + $this->get('/limited-proxy', headers: [ + 'Authorization' => 'Bearer invalid-token', + 'X-Forwarded-For' => '203.0.113.11', + ])->assertUnauthorized(); +}); + it('resets rate limit counter on successful authentication', function (): void { $user = new User(); $user->id = 1; diff --git a/tests/Feature/Cache/LocalRateLimitTest.php b/tests/Feature/Cache/LocalRateLimitTest.php index 04441f67..b082ffd5 100644 --- a/tests/Feature/Cache/LocalRateLimitTest.php +++ b/tests/Feature/Cache/LocalRateLimitTest.php @@ -2,6 +2,7 @@ declare(strict_types=1); +use Phenix\Constants\AppMode; use Phenix\Facades\Config; use Phenix\Facades\Crypto; use Phenix\Facades\Route; @@ -65,3 +66,25 @@ $this->get(path: '/') ->assertOk(); }); + +it('uses forwarded client ip for rate limiting behind a trusted proxy', function (): void { + Config::set('app.app_mode', AppMode::PROXIED->value); + Config::set('app.trusted_proxies', ['127.0.0.1/32', '127.0.0.1']); + Config::set('cache.rate_limit.per_minute', 1); + + Route::get('/', fn (): Response => response()->plain('Ok')); + + $this->app->run(); + + $this->get(path: '/', headers: [ + 'X-Forwarded-For' => '203.0.113.10', + ])->assertOk(); + + $this->get(path: '/', headers: [ + 'X-Forwarded-For' => '203.0.113.10', + ])->assertStatusCode(HttpStatus::TOO_MANY_REQUESTS); + + $this->get(path: '/', headers: [ + 'X-Forwarded-For' => '203.0.113.11', + ])->assertOk(); +}); diff --git a/tests/Feature/RequestTest.php b/tests/Feature/RequestTest.php index 67f84c9a..125fef15 100644 --- a/tests/Feature/RequestTest.php +++ b/tests/Feature/RequestTest.php @@ -509,3 +509,21 @@ 'Cross-Origin-Opener-Policy', ]); }); + +it('can follow redirects when enabled', function (): void { + Route::get('/redirect-follow', fn (): Response => response()->redirect('/redirect-target')); + Route::get('/redirect-target', fn (): Response => response()->json([ + 'message' => 'Redirect followed', + 'redirected' => true, + ])); + + $this->app->run(); + + $this->followingRedirects() + ->get('/redirect-follow') + ->assertOk() + ->assertIsJson() + ->assertJsonPath('message', 'Redirect followed') + ->assertJsonPath('redirected', true) + ->assertHeaderIsMissing('Location'); +}); diff --git a/tests/Unit/Http/IpAddressTest.php b/tests/Unit/Http/IpAddressTest.php index 15c6fa4a..ed1cc2df 100644 --- a/tests/Unit/Http/IpAddressTest.php +++ b/tests/Unit/Http/IpAddressTest.php @@ -269,4 +269,50 @@ public function __toString(): string expect($ip->isForwarded())->toBeTrue(); expect($ip->forwardingAddress())->toBe('203.0.113.1:4711'); + expect($ip->hash())->toBe(hash('sha256', '203.0.113.1')); +}); + +it('uses forwarded ipv6 host for hash when request is behind a trusted proxy', function (): void { + $client = $this->createMock(Client::class); + $client->method('getRemoteAddress')->willReturn( + new class ('10.0.0.1:1234') implements SocketAddress { + public function __construct(private string $address) + { + } + + public function toString(): string + { + return $this->address; + } + + public function getType(): SocketAddressType + { + return SocketAddressType::Internet; + } + + public function __toString(): string + { + return $this->address; + } + } + ); + + $request = new ServerRequest($client, HttpMethod::GET->value, Http::new(Url::to('/'))); + $request->setHeader('X-Forwarded-For', '2001:db8::10'); + $request->setAttribute( + Forwarded::class, + new Forwarded( + new InternetAddress('2001:db8::10', 4711), + [ + 'for' => '[2001:db8::10]:4711', + ] + ) + ); + + $ip = Ip::make($request); + + expect($ip->isForwarded())->toBeTrue(); + expect($ip->forwardingAddress())->toBe('[2001:db8::10]:4711'); + expect($ip->host())->toBe('10.0.0.1'); + expect($ip->hash())->toBe(hash('sha256', '2001:db8::10')); });