mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(api): use the Cloudflare client IP for allowed IPs and audit logs
Closes #12301 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
0f0a1e920d
commit
80ce0efe7e
@@ -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,
|
||||
],
|
||||
|
||||
@@ -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
|
||||
{
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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.');
|
||||
|
||||
|
||||
@@ -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,
|
||||
];
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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'))),
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user