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
This commit is contained in:
parent
6dea1ab0f3
commit
76a770911c
5 changed files with 137 additions and 12 deletions
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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)) {
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
110
tests/Feature/ServerIpUniquenessTest.php
Normal file
110
tests/Feature/ServerIpUniquenessTest.php
Normal file
|
|
@ -0,0 +1,110 @@
|
|||
<?php
|
||||
|
||||
use App\Models\PrivateKey;
|
||||
use App\Models\Server;
|
||||
use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
$this->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();
|
||||
});
|
||||
Loading…
Reference in a new issue