diff --git a/app/Actions/Fortify/CreateNewUser.php b/app/Actions/Fortify/CreateNewUser.php index f24b9699f2..1a8d3332a1 100644 --- a/app/Actions/Fortify/CreateNewUser.php +++ b/app/Actions/Fortify/CreateNewUser.php @@ -93,7 +93,7 @@ class CreateNewUser implements CreatesNewUsers { $keys = [ [ - 'key' => 'registration:ip:'.sha1(auth_rate_limit_ip(request())), + 'key' => 'registration:ip:'.sha1(client_ip(request())), 'max' => self::REGISTRATION_IP_MAX_ATTEMPTS, 'decay' => self::REGISTRATION_IP_DECAY_SECONDS, ], diff --git a/app/Http/Controllers/Webhook/Concerns/ThrottlesManualWebhookFailures.php b/app/Http/Controllers/Webhook/Concerns/ThrottlesManualWebhookFailures.php index 7e48d1a637..f02e0af13e 100644 --- a/app/Http/Controllers/Webhook/Concerns/ThrottlesManualWebhookFailures.php +++ b/app/Http/Controllers/Webhook/Concerns/ThrottlesManualWebhookFailures.php @@ -51,7 +51,7 @@ trait ThrottlesManualWebhookFailures /** * The connection address of the client. X-Forwarded-For is ignored, since * any client can set it and rotate it to get a new bucket. On cloud, a - * valid CF-Connecting-IP is used, as in auth_rate_limit_ip(). + * valid CF-Connecting-IP is used. */ protected function manualWebhookFailureClientAddress(Request $request): string { diff --git a/app/Http/Middleware/ApiIpAllowed.php b/app/Http/Middleware/ApiIpAllowed.php index ede32567d0..72aaa4b7c8 100644 --- a/app/Http/Middleware/ApiIpAllowed.php +++ b/app/Http/Middleware/ApiIpAllowed.php @@ -22,7 +22,7 @@ class ApiIpAllowed $allowedIps = array_filter(array_map('trim', explode(',', $allowedIps))); - if (! empty($allowedIps) && ! checkIPAgainstAllowlist($request->ip(), $allowedIps)) { + if (! empty($allowedIps) && ! checkIPAgainstAllowlist(client_ip($request), $allowedIps)) { return response()->json(['success' => true, 'message' => 'You are not allowed to access the API.'], 403); } diff --git a/app/Livewire/Profile/Index.php b/app/Livewire/Profile/Index.php index e782de176a..825d6a27c5 100644 --- a/app/Livewire/Profile/Index.php +++ b/app/Livewire/Profile/Index.php @@ -151,7 +151,7 @@ class Index extends Component } // Additional rate limit by IP address (5 requests per hour) - $ipKey = 'email-change:ip:'.request()->ip(); + $ipKey = 'email-change:ip:'.client_ip(request()); if (! RateLimiter::attempt($ipKey, 5, function () {}, 3600)) { $this->dispatch('error', 'Too many requests from your IP address. Please try again later.'); diff --git a/app/Models/AuditEvent.php b/app/Models/AuditEvent.php index 2742a3f667..3020119781 100644 --- a/app/Models/AuditEvent.php +++ b/app/Models/AuditEvent.php @@ -200,7 +200,7 @@ class AuditEvent extends Model ?? trim(($resourceName ?? Str::headline((string) $resourceType)).' '.Str::headline($action)), 'metadata' => self::redact(Arr::except($context, ['audit_changes'])), 'changes' => $changes, - 'ip_address' => app()->bound('request') ? request()->ip() : null, + 'ip_address' => app()->bound('request') ? (client_ip(request()) ?: null) : null, 'user_agent' => app()->bound('request') ? Str::limit((string) request()->userAgent(), 200, '') : null, ]; } diff --git a/app/Providers/RouteServiceProvider.php b/app/Providers/RouteServiceProvider.php index dedac3a095..a4932d9ce1 100644 --- a/app/Providers/RouteServiceProvider.php +++ b/app/Providers/RouteServiceProvider.php @@ -46,23 +46,23 @@ class RouteServiceProvider extends ServiceProvider { RateLimiter::for('api', function (Request $request) { if ($request->path() === 'api/health') { - return Limit::perMinute(1000)->by($request->user()?->id ?: auth_rate_limit_ip($request)); + return Limit::perMinute(1000)->by($request->user()?->id ?: client_ip($request)); } - return Limit::perMinute((int) config('api.rate_limit'))->by($request->user()?->id ?: auth_rate_limit_ip($request)); + return Limit::perMinute((int) config('api.rate_limit'))->by($request->user()?->id ?: client_ip($request)); }); RateLimiter::for('5', function (Request $request) { - return Limit::perMinute(5)->by($request->user()?->id ?: auth_rate_limit_ip($request)); + return Limit::perMinute(5)->by($request->user()?->id ?: client_ip($request)); }); RateLimiter::for('feedback', function (Request $request) { - return Limit::perMinute(3)->by($request->user()?->id ?: auth_rate_limit_ip($request)); + return Limit::perMinute(3)->by($request->user()?->id ?: client_ip($request)); }); RateLimiter::for('login', function (Request $request) { $email = $request->input('email'); $emailIdentity = normalize_email_identity(is_string($email) ? $email : null); - $limits = [Limit::perMinute(5)->by((is_string($email) ? $email : '').'|'.auth_rate_limit_ip($request))]; + $limits = [Limit::perMinute(5)->by((is_string($email) ? $email : '').'|'.client_ip($request))]; if ($emailIdentity !== null) { $limits[] = Limit::perMinute(5)->by('login:email-identity:'.sha1($emailIdentity)); } @@ -75,12 +75,12 @@ class RouteServiceProvider extends ServiceProvider }); RateLimiter::for('passkeys', function (Request $request) { - return Limit::perMinute(30)->by('passkeys:'.auth_rate_limit_ip($request)); + return Limit::perMinute(30)->by('passkeys:'.client_ip($request)); }); RateLimiter::for('forgot-password', function (Request $request) { $limits = [ - Limit::perMinutes(10, 3)->by('forgot-password:ip:'.sha1(auth_rate_limit_ip($request))), + Limit::perMinutes(10, 3)->by('forgot-password:ip:'.sha1(client_ip($request))), ]; $emailIdentity = normalize_email_identity($request->input('email')); @@ -92,7 +92,7 @@ class RouteServiceProvider extends ServiceProvider }); RateLimiter::for('magic-link', function (Request $request) { - return Limit::perMinute(5)->by(hash('sha256', (string) $request->input('token').'|'.auth_rate_limit_ip($request))); + return Limit::perMinute(5)->by(hash('sha256', (string) $request->input('token').'|'.client_ip($request))); }); RateLimiter::for('force-password-reset', function (Request $request) { diff --git a/bootstrap/helpers/audit.php b/bootstrap/helpers/audit.php index aea549a5be..46ec4ba2b4 100644 --- a/bootstrap/helpers/audit.php +++ b/bootstrap/helpers/audit.php @@ -28,7 +28,7 @@ if (! function_exists('auditLog')) { $token = $user?->currentAccessToken(); $payload = AuditEvent::redactContext(array_merge([ 'event' => $event, - 'ip' => $request?->ip(), + 'ip' => $request ? (client_ip($request) ?: null) : null, 'ua' => substr((string) $request?->userAgent(), 0, 200), 'user_id' => $user?->id, 'user_email' => $user?->email, @@ -102,7 +102,7 @@ if (! function_exists('shouldRecordAuditEvent')) { $request = app()->bound('request') ? request() : null; $fingerprint = hash('sha256', json_encode([ $event, - $request?->ip(), + $request ? client_ip($request) : null, data_get($context, 'reason'), data_get($context, 'server_uuid'), Str::lower(trim((string) data_get($context, 'attempted_email'))), diff --git a/bootstrap/helpers/auth.php b/bootstrap/helpers/auth.php index 7580fc1e06..564ae31062 100644 --- a/bootstrap/helpers/auth.php +++ b/bootstrap/helpers/auth.php @@ -2,13 +2,20 @@ use Illuminate\Http\Request; -function auth_rate_limit_ip(Request $request): string +/** + * The client IP for rate limits, allow-lists, and audit logs. Behind Cloudflare, $request->ip() + * is the Cloudflare edge, so a valid CF-Connecting-IP is used: on cloud, and on self-hosted + * instances when the request comes from a Cloudflare IP. A client that connects directly + * cannot set the client IP with the header. + */ +function client_ip(Request $request): string { + $ip = (string) $request->ip(); $cloudflareIp = $request->header('CF-Connecting-IP'); - if (isCloud() && is_string($cloudflareIp) && filter_var($cloudflareIp, FILTER_VALIDATE_IP) !== false) { + if (is_string($cloudflareIp) && filter_var($cloudflareIp, FILTER_VALIDATE_IP) !== false && (isCloud() || isCloudflareIp($ip))) { return $cloudflareIp; } - return (string) $request->ip(); + return $ip; } diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 7ce429bfd1..2ed69ed773 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -2448,7 +2448,7 @@ function isCloudflareIp(string $ip): bool $cloudflareIps = [ '173.245.48.0/20', '103.21.244.0/22', '103.22.200.0/22', '103.31.4.0/22', '141.101.64.0/18', '108.162.192.0/18', '190.93.240.0/20', '188.114.96.0/20', - '197.234.240.0/22', '198.41.128.0/17', '162.158.0.0/15', '104.16.0.0/13', + '197.234.240.0/22', '198.41.128.0/17', '162.158.0.0/15', '104.16.0.0/13', '104.24.0.0/14', '172.64.0.0/13', '131.0.72.0/22', '2400:cb00::/32', '2606:4700::/32', '2803:f800::/32', '2405:b500::/32', '2405:8100::/32', '2a06:98c0::/29', '2c0f:f248::/32', diff --git a/tests/Feature/AuditEventsTest.php b/tests/Feature/AuditEventsTest.php index f10de08adc..0e45f2f839 100644 --- a/tests/Feature/AuditEventsTest.php +++ b/tests/Feature/AuditEventsTest.php @@ -35,6 +35,7 @@ use Illuminate\Contracts\Database\Eloquent\CastsAttributes; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\QueryException; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Schema; @@ -170,6 +171,17 @@ test('audit log persists a structured event for the current team', function () { ->and($event->metadata['changed'])->toBe(['name']); }); +test('audit log records the Cloudflare client ip when the request comes from Cloudflare', function () { + app()->instance('request', Request::create('/', server: [ + 'REMOTE_ADDR' => '162.158.79.140', + 'HTTP_CF_CONNECTING_IP' => '160.79.104.10', + ])); + + auditLog('api.application.updated', ['application_uuid' => 'app-123']); + + expect(AuditEvent::query()->sole()->ip_address)->toBe('160.79.104.10'); +}); + test('auditable models record authenticated create update and delete actions', function () { $project = Project::factory()->create([ 'team_id' => $this->team->id, diff --git a/tests/Feature/AuthRateLimitClientIpTest.php b/tests/Feature/AuthRateLimitClientIpTest.php index ff32dab6a9..cd150a107a 100644 --- a/tests/Feature/AuthRateLimitClientIpTest.php +++ b/tests/Feature/AuthRateLimitClientIpTest.php @@ -10,7 +10,7 @@ it('uses the Cloudflare client ip on cloud', function () { 'HTTP_CF_CONNECTING_IP' => '2001:db8::10', ]); - expect(auth_rate_limit_ip($request))->toBe('2001:db8::10'); + expect(client_ip($request))->toBe('2001:db8::10'); }); it('falls back to the resolved request ip when the Cloudflare header is invalid', function () { @@ -21,10 +21,21 @@ it('falls back to the resolved request ip when the Cloudflare header is invalid' 'HTTP_CF_CONNECTING_IP' => 'invalid', ]); - expect(auth_rate_limit_ip($request))->toBe('203.0.113.10'); + expect(client_ip($request))->toBe('203.0.113.10'); }); -it('ignores the Cloudflare header on self-hosted instances', function () { +it('uses the Cloudflare client ip on self-hosted instances when the request comes from Cloudflare', function (string $cloudflareEdge) { + config()->set('constants.coolify.self_hosted', true); + + $request = Request::create('/', server: [ + 'REMOTE_ADDR' => $cloudflareEdge, + 'HTTP_CF_CONNECTING_IP' => '2001:db8::20', + ]); + + expect(client_ip($request))->toBe('2001:db8::20'); +})->with(['162.158.79.140', '104.24.0.1', '2a06:98c0::1']); + +it('ignores the Cloudflare header on self-hosted instances when the request does not come from Cloudflare', function () { config()->set('constants.coolify.self_hosted', true); $request = Request::create('/', server: [ @@ -32,5 +43,5 @@ it('ignores the Cloudflare header on self-hosted instances', function () { 'HTTP_CF_CONNECTING_IP' => '2001:db8::20', ]); - expect(auth_rate_limit_ip($request))->toBe('203.0.113.20'); + expect(client_ip($request))->toBe('203.0.113.20'); }); diff --git a/tests/Feature/Mcp/McpEndpointTest.php b/tests/Feature/Mcp/McpEndpointTest.php index b1376df7cf..6d48447051 100644 --- a/tests/Feature/Mcp/McpEndpointTest.php +++ b/tests/Feature/Mcp/McpEndpointTest.php @@ -143,6 +143,30 @@ test('MCP endpoint accepts a token from an IP inside the allow-list', function ( mcpListTools($token)->assertOk(); }); +test('MCP endpoint checks the allow-list against the client IP behind Cloudflare', function () { + InstanceSettings::query()->where('id', 0)->update(['allowed_ips' => '160.79.104.0/21']); + Once::flush(); + $token = $this->user->createToken('mcp-read', ['read'])->plainTextToken; + + test()->withServerVariables(['REMOTE_ADDR' => '10.0.1.5']) + ->withHeaders(['X-Forwarded-For' => '172.71.194.182', 'CF-Connecting-IP' => '160.79.104.10']); + + mcpListTools($token)->assertOk(); +}); + +test('MCP endpoint ignores a CF-Connecting-IP header that does not come from Cloudflare', function () { + InstanceSettings::query()->where('id', 0)->update(['allowed_ips' => '160.79.104.0/21']); + Once::flush(); + $token = $this->user->createToken('mcp-read', ['read'])->plainTextToken; + + test()->withServerVariables(['REMOTE_ADDR' => '10.0.1.5']) + ->withHeaders(['X-Forwarded-For' => '198.51.100.7', 'CF-Connecting-IP' => '160.79.104.10']); + + mcpListTools($token) + ->assertForbidden() + ->assertJson(['message' => 'You are not allowed to access the API.']); +}); + test('MCP endpoint lists tools for an authenticated token', function () { $token = $this->user->createToken('mcp-read', ['read'])->plainTextToken;