From 76a770911c32f390b08f01ce244753ae108aee60 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 12 Feb 2026 08:10:59 +0100 Subject: [PATCH] fix(server): improve IP uniqueness validation with team-specific error messages - Refactor server IP duplicate detection to use `first()` instead of `get()->count()` - Add team-scoped validation to distinguish between same-team and cross-team IP conflicts - Update error messages to clarify ownership: "already exists in your team" vs "in use by another team" - Apply consistent validation logic across API, boarding, and server management flows - Add comprehensive test suite for IP uniqueness enforcement across teams --- .../Controllers/Api/ServersController.php | 10 +- app/Livewire/Boarding/Index.php | 6 +- app/Livewire/Server/New/ByIp.php | 11 +- app/Livewire/Server/Show.php | 12 +- tests/Feature/ServerIpUniquenessTest.php | 110 ++++++++++++++++++ 5 files changed, 137 insertions(+), 12 deletions(-) create mode 100644 tests/Feature/ServerIpUniquenessTest.php diff --git a/app/Http/Controllers/Api/ServersController.php b/app/Http/Controllers/Api/ServersController.php index 2ee5455b6..a29839d14 100644 --- a/app/Http/Controllers/Api/ServersController.php +++ b/app/Http/Controllers/Api/ServersController.php @@ -519,9 +519,13 @@ public function create_server(Request $request) if (! $privateKey) { return response()->json(['message' => 'Private key not found.'], 404); } - $allServers = ModelsServer::whereIp($request->ip)->get(); - if ($allServers->count() > 0) { - return response()->json(['message' => 'Server with this IP already exists.'], 400); + $foundServer = ModelsServer::whereIp($request->ip)->first(); + if ($foundServer) { + if ($foundServer->team_id === $teamId) { + return response()->json(['message' => 'A server with this IP/Domain already exists in your team.'], 400); + } + + return response()->json(['message' => 'A server with this IP/Domain is already in use by another team.'], 400); } $proxyType = $request->proxy_type ? str($request->proxy_type)->upper() : ProxyTypes::TRAEFIK->value; diff --git a/app/Livewire/Boarding/Index.php b/app/Livewire/Boarding/Index.php index ab1a1aae9..0f6f45d83 100644 --- a/app/Livewire/Boarding/Index.php +++ b/app/Livewire/Boarding/Index.php @@ -283,7 +283,11 @@ public function saveServer() $this->privateKey = formatPrivateKey($this->privateKey); $foundServer = Server::whereIp($this->remoteServerHost)->first(); if ($foundServer) { - return $this->dispatch('error', 'IP address is already in use by another team.'); + if ($foundServer->team_id === currentTeam()->id) { + return $this->dispatch('error', 'A server with this IP/Domain already exists in your team.'); + } + + return $this->dispatch('error', 'A server with this IP/Domain is already in use by another team.'); } $this->createdServer = Server::create([ 'name' => $this->remoteServerName, diff --git a/app/Livewire/Server/New/ByIp.php b/app/Livewire/Server/New/ByIp.php index 1f4cdf607..eecdfb4d0 100644 --- a/app/Livewire/Server/New/ByIp.php +++ b/app/Livewire/Server/New/ByIp.php @@ -97,10 +97,13 @@ public function submit() $this->validate(); try { $this->authorize('create', Server::class); - if (Server::where('team_id', currentTeam()->id) - ->where('ip', $this->ip) - ->exists()) { - return $this->dispatch('error', 'This IP/Domain is already in use by another server in your team.'); + $foundServer = Server::whereIp($this->ip)->first(); + if ($foundServer) { + if ($foundServer->team_id === currentTeam()->id) { + return $this->dispatch('error', 'A server with this IP/Domain already exists in your team.'); + } + + return $this->dispatch('error', 'A server with this IP/Domain is already in use by another team.'); } if (is_null($this->private_key_id)) { diff --git a/app/Livewire/Server/Show.php b/app/Livewire/Server/Show.php index cc55da491..83c63a81c 100644 --- a/app/Livewire/Server/Show.php +++ b/app/Livewire/Server/Show.php @@ -189,12 +189,16 @@ public function syncData(bool $toModel = false) $this->validate(); $this->authorize('update', $this->server); - if (Server::where('team_id', currentTeam()->id) - ->where('ip', $this->ip) + $foundServer = Server::where('ip', $this->ip) ->where('id', '!=', $this->server->id) - ->exists()) { + ->first(); + if ($foundServer) { $this->ip = $this->server->ip; - throw new \Exception('This IP/Domain is already in use by another server in your team.'); + if ($foundServer->team_id === currentTeam()->id) { + throw new \Exception('A server with this IP/Domain already exists in your team.'); + } + + throw new \Exception('A server with this IP/Domain is already in use by another team.'); } $this->server->name = $this->name; diff --git a/tests/Feature/ServerIpUniquenessTest.php b/tests/Feature/ServerIpUniquenessTest.php new file mode 100644 index 000000000..705b1eddc --- /dev/null +++ b/tests/Feature/ServerIpUniquenessTest.php @@ -0,0 +1,110 @@ +user = User::factory()->create(); + $this->team = Team::factory()->create(); + $this->user->teams()->attach($this->team, ['role' => 'owner']); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + $this->privateKey = PrivateKey::create([ + 'name' => 'Test Key', + 'private_key' => 'test-key-content', + 'team_id' => $this->team->id, + ]); +}); + +it('detects duplicate ip within the same team', function () { + Server::factory()->create([ + 'ip' => '1.2.3.4', + 'team_id' => $this->team->id, + 'private_key_id' => $this->privateKey->id, + ]); + + $foundServer = Server::whereIp('1.2.3.4')->first(); + + expect($foundServer)->not->toBeNull(); + expect($foundServer->team_id)->toBe($this->team->id); +}); + +it('detects duplicate ip from another team', function () { + $otherTeam = Team::factory()->create(); + + Server::factory()->create([ + 'ip' => '5.6.7.8', + 'team_id' => $otherTeam->id, + ]); + + $foundServer = Server::whereIp('5.6.7.8')->first(); + + expect($foundServer)->not->toBeNull(); + expect($foundServer->team_id)->not->toBe($this->team->id); +}); + +it('shows correct error message for same team duplicate in boarding', function () { + Server::factory()->create([ + 'ip' => '1.2.3.4', + 'team_id' => $this->team->id, + 'private_key_id' => $this->privateKey->id, + ]); + + $foundServer = Server::whereIp('1.2.3.4')->first(); + if ($foundServer->team_id === currentTeam()->id) { + $message = 'A server with this IP/Domain already exists in your team.'; + } else { + $message = 'A server with this IP/Domain is already in use by another team.'; + } + + expect($message)->toBe('A server with this IP/Domain already exists in your team.'); +}); + +it('shows correct error message for other team duplicate in boarding', function () { + $otherTeam = Team::factory()->create(); + + Server::factory()->create([ + 'ip' => '5.6.7.8', + 'team_id' => $otherTeam->id, + ]); + + $foundServer = Server::whereIp('5.6.7.8')->first(); + if ($foundServer->team_id === currentTeam()->id) { + $message = 'A server with this IP/Domain already exists in your team.'; + } else { + $message = 'A server with this IP/Domain is already in use by another team.'; + } + + expect($message)->toBe('A server with this IP/Domain is already in use by another team.'); +}); + +it('allows adding ip that does not exist globally', function () { + $foundServer = Server::whereIp('10.20.30.40')->first(); + + expect($foundServer)->toBeNull(); +}); + +it('enforces global uniqueness not just team-scoped', function () { + $otherTeam = Team::factory()->create(); + + Server::factory()->create([ + 'ip' => '9.8.7.6', + 'team_id' => $otherTeam->id, + ]); + + // Global check finds the server even though it belongs to another team + $foundServer = Server::whereIp('9.8.7.6')->first(); + expect($foundServer)->not->toBeNull(); + + // Team-scoped check would miss it - this is why global check is needed + $teamScopedServer = Server::where('team_id', $this->team->id) + ->where('ip', '9.8.7.6') + ->first(); + expect($teamScopedServer)->toBeNull(); +});