diff --git a/app/Http/Controllers/Api/ServerTransferController.php b/app/Http/Controllers/Api/ServerTransferController.php index 6179cb350a..6cbdde8d1b 100644 --- a/app/Http/Controllers/Api/ServerTransferController.php +++ b/app/Http/Controllers/Api/ServerTransferController.php @@ -6,13 +6,16 @@ use App\Http\Controllers\Controller; use App\Models\GithubApp; use App\Models\GitlabApp; use App\Models\Server; +use App\Rules\SafeWebhookUrl; use App\Rules\ValidServerIp; use App\Services\ServerTransfer\ServerTransferBundle; use App\Services\ServerTransfer\ServerTransferClaimer; use App\Services\ServerTransfer\ServerTransferImporter; use App\Services\ServerTransfer\ServerTransferMigrator; +use Illuminate\Http\Exceptions\ThrottleRequestsException; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; +use Illuminate\Support\Facades\Validator; use Illuminate\Validation\ValidationException; use OpenApi\Attributes as OA; use Throwable; @@ -217,7 +220,12 @@ class ServerTransferController extends Controller // The target connects to the server over SSH when it takes over management, so an unreachable // server would leave an imported server here that this instance cannot manage. - $connection = @fsockopen($ip, $port, timeout: 5); + // On Cloud the probe is rate limited and cannot reach private, loopback, or link-local addresses, + // so users cannot scan ports of Cloud's own network. + Server::throttleValidation($teamId, $ip); + $probeHost = filter_var(Server::normalizeIp($ip), FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) ? '['.Server::normalizeIp($ip).']' : $ip; + $probeAllowed = ! isCloud() || Validator::make(['ip' => "http://{$probeHost}:{$port}"], ['ip' => [new SafeWebhookUrl]])->passes(); + $connection = $probeAllowed ? @fsockopen($ip, $port, timeout: 5) : false; if ($connection !== false) { fclose($connection); } @@ -235,8 +243,11 @@ class ServerTransferController extends Controller [ 'label' => "the target can reach the SSH port of the server ({$ip}:{$port})", 'ok' => $connection !== false, - 'message' => $connection !== false ? null - : "This instance cannot reach the server at {$ip}:{$port}. Allow SSH from this instance to the server, then transfer again.", + 'message' => match (true) { + $connection !== false => null, + ! $probeAllowed => "This instance cannot connect to a private, loopback, or link-local address ({$ip}). Use the public IP address of the server.", + default => "This instance cannot reach the server at {$ip}:{$port}. Allow SSH from this instance to the server, then transfer again.", + }, ], ]; @@ -319,6 +330,8 @@ class ServerTransferController extends Controller preserveUuids: $request->boolean('preserve_uuids', true), adoptMode: $request->boolean('adopt_mode', true), ); + } catch (ThrottleRequestsException $e) { + throw $e; } catch (Throwable $e) { $payload = ['message' => $e->getMessage()]; if ($e instanceof ValidationException) { diff --git a/app/Jobs/ServerLimitCheckJob.php b/app/Jobs/ServerLimitCheckJob.php index ff4b7531c3..82ad38d6c8 100644 --- a/app/Jobs/ServerLimitCheckJob.php +++ b/app/Jobs/ServerLimitCheckJob.php @@ -31,7 +31,7 @@ class ServerLimitCheckJob implements ShouldBeEncrypted, ShouldQueue public function handle() { try { - $servers = $this->team->servers; + $servers = $this->team->managedServers; $servers_count = $servers->count(); $number_of_servers_to_disable = $servers_count - $this->team->limits; if ($number_of_servers_to_disable > 0) { diff --git a/app/Models/Server.php b/app/Models/Server.php index 6e82272a1c..b8b56389ec 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -1333,6 +1333,16 @@ $siteAddress { return $this->proxy->modelScope(); } + /** + * Servers that this instance manages. A transferred server is only an archive and does not use a server slot. + */ + public function scopeNotTransferredAway(Builder $query): Builder + { + return $query->where(fn (Builder $query) => $query + ->whereNull('server_metadata->transfer->status') + ->orWhere('server_metadata->transfer->status', '!=', 'transferred')); + } + public function scopeWhereProxyType(Builder $query, string $proxyType): Builder { return $query->where('proxy->type', $proxyType); diff --git a/app/Models/Team.php b/app/Models/Team.php index 05c9287491..9d02d229a5 100644 --- a/app/Models/Team.php +++ b/app/Models/Team.php @@ -158,7 +158,7 @@ class Team extends Model implements SendsDiscord, SendsEmail, SendsPushover, Sen return true; } $serverLimit = Team::serverLimit($team); - $servers = $team->servers()->count(); + $servers = $team->managedServers()->count(); return $servers >= $serverLimit; } @@ -192,7 +192,7 @@ class Team extends Model implements SendsDiscord, SendsEmail, SendsPushover, Sen public function serverOverflow() { - if (Team::serverLimit($this) < $this->servers->count()) { + if (Team::serverLimit($this) < $this->managedServers()->count()) { return true; } @@ -392,6 +392,14 @@ class Team extends Model implements SendsDiscord, SendsEmail, SendsPushover, Sen return $this->hasMany(Server::class); } + /** + * Servers that use a server slot. Archives of transferred servers are left out. + */ + public function managedServers() + { + return $this->servers()->notTransferredAway(); + } + public function usesSwarm(): bool { return $this->servers() diff --git a/app/Services/ServerTransfer/ServerTransferImporter.php b/app/Services/ServerTransfer/ServerTransferImporter.php index 707f7fdc02..3dd43f8d7c 100644 --- a/app/Services/ServerTransfer/ServerTransferImporter.php +++ b/app/Services/ServerTransfer/ServerTransferImporter.php @@ -206,6 +206,8 @@ class ServerTransferImporter if (! $dryRun) { $this->assertCanImport($serverUuid, $serverIp, $teamId, $preserveUuids); + // The import validates the server over SSH, so it uses the same Cloud limit as a new server. + Server::throttleValidation($teamId, $serverIp); } if ($uuidConflict) { @@ -509,7 +511,7 @@ class ServerTransferImporter $uuidMessage = match (true) { $existingServer === null => null, $existingServer->trashed() => "The server with UUID {$serverUuid} is still being deleted on this instance. Wait a minute, then transfer again.", - $existingServer->isTransferredAway() => "Another team of this instance has the archived copy of server {$existingServer->name}. Transfer the server into that team, or delete the archive there first.", + $existingServer->isTransferredAway() => 'Another team of this instance has the archived copy of this server. Transfer the server into that team, or delete the archive there first.', default => "A server with UUID {$serverUuid} already exists on this instance. Delete/transfer it first, or import with preserve_uuids=false.", }; diff --git a/resources/views/livewire/subscription/actions.blade.php b/resources/views/livewire/subscription/actions.blade.php index 75ea746245..1b0c391963 100644 --- a/resources/views/livewire/subscription/actions.blade.php +++ b/resources/views/livewire/subscription/actions.blade.php @@ -1,7 +1,7 @@
assertStatus(422); + } + + $this->withHeaders(transferHeaders($this->importToken)) + ->getJson('/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=10.0.0.7') + ->assertStatus(429); + }); + test('rejects a token without the write permission', function () { $this->withHeaders(transferHeaders($this->readToken)) ->getJson('/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=10.66.0.99') diff --git a/tests/Feature/ServerLimitCheckJobTest.php b/tests/Feature/ServerLimitCheckJobTest.php index 6b2c074be1..30b993a526 100644 --- a/tests/Feature/ServerLimitCheckJobTest.php +++ b/tests/Feature/ServerLimitCheckJobTest.php @@ -81,3 +81,23 @@ it('does not change servers when under limit and none are force-disabled', funct expect($server1->fresh()->settings->force_disabled)->toBeFalsy(); expect($server2->fresh()->settings->force_disabled)->toBeFalsy(); }); + +it('does not count archives of transferred servers against the limit', function () { + $this->team->update(['custom_server_limit' => 1]); + + $live = createServerForTeam($this->team); + sleep(1); + $archive = createServerForTeam($this->team); + $archive->forceFill(['server_metadata' => ['transfer' => ['status' => 'transferred']]])->save(); + + ServerLimitCheckJob::dispatchSync($this->team); + + expect($live->fresh()->settings->force_disabled)->toBeFalsy() + ->and(Team::serverLimitReached($this->team))->toBeTrue() + ->and($this->team->serverOverflow())->toBeFalse(); + Notification::assertNothingSent(); + + $live->forceFill(['server_metadata' => ['transfer' => ['status' => 'transferred']]])->save(); + + expect(Team::serverLimitReached($this->team))->toBeFalse(); +}); diff --git a/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php b/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php index 0e656a35e7..b322aa3ff6 100644 --- a/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php +++ b/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php @@ -525,7 +525,10 @@ test('the archive of another team is not replaced', function () { $otherTeam = Team::factory()->create(); expect(fn () => $this->importer->import($bundle, teamId: $otherTeam->id)) - ->toThrow(ValidationException::class, 'Another team of this instance has the archived copy'); + ->toThrow(ValidationException::class, 'Another team of this instance has the archived copy of this server.'); + + $uuidCheck = collect($this->importer->importChecks($this->server->uuid, '203.0.113.50', $otherTeam->id))->firstWhere('field', 'server.uuid'); + expect($uuidCheck['message'])->not->toContain($this->server->name); }); test('import rolls back all created rows when a later resource fails', function () {