mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(keys): check duplicate private keys in the key's own team
The duplicate check used the signed-in user's current team, or all teams when there was none. Check the team the key belongs to. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
e755f4c9e0
commit
75976d9882
@@ -225,7 +225,7 @@ class SecurityController extends Controller
|
||||
], 422);
|
||||
}
|
||||
$fingerPrint = PrivateKey::generateFingerprint($request->private_key);
|
||||
$isFingerPrintExists = PrivateKey::fingerprintExists($fingerPrint);
|
||||
$isFingerPrintExists = PrivateKey::fingerprintExists($fingerPrint, teamId: $teamId);
|
||||
if ($isFingerPrintExists) {
|
||||
return response()->json([
|
||||
'message' => 'Private key already exists.',
|
||||
|
||||
@@ -65,7 +65,7 @@ class PrivateKey extends BaseModel
|
||||
}
|
||||
|
||||
$key->fingerprint = self::generateFingerprint($key->private_key);
|
||||
if (self::fingerprintExists($key->fingerprint, $key->id)) {
|
||||
if (self::fingerprintExists($key->fingerprint, $key->id, $key->team_id)) {
|
||||
throw ValidationException::withMessages([
|
||||
'private_key' => ['This private key already exists.'],
|
||||
]);
|
||||
@@ -365,17 +365,24 @@ class PrivateKey extends BaseModel
|
||||
}
|
||||
}
|
||||
|
||||
public static function fingerprintExists($fingerprint, $excludeId = null)
|
||||
/**
|
||||
* Whether another key with this fingerprint exists in the given team.
|
||||
*
|
||||
* The check is always limited to $teamId, never to the signed in user's
|
||||
* team or to all teams. Without a team id nothing can match, because every
|
||||
* key belongs to a team.
|
||||
*/
|
||||
public static function fingerprintExists(?string $fingerprint, ?int $excludeId = null, ?int $teamId = null): bool
|
||||
{
|
||||
$query = self::query()
|
||||
->where('fingerprint', $fingerprint)
|
||||
->where('id', '!=', $excludeId);
|
||||
|
||||
if (currentTeam()) {
|
||||
$query->where('team_id', currentTeam()->id);
|
||||
if (is_null($fingerprint) || is_null($teamId)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return $query->exists();
|
||||
return self::query()
|
||||
->where('team_id', $teamId)
|
||||
->where('fingerprint', $fingerprint)
|
||||
->when(! is_null($excludeId), fn ($query) => $query->whereKeyNot($excludeId))
|
||||
->exists();
|
||||
}
|
||||
|
||||
public static function cleanupUnusedKeys()
|
||||
|
||||
@@ -0,0 +1,135 @@
|
||||
<?php
|
||||
|
||||
use App\Models\InstanceSettings;
|
||||
use App\Models\PrivateKey;
|
||||
use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Validation\ValidationException;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
config()->set('app.maintenance.driver', 'file');
|
||||
config()->set('cache.default', 'array');
|
||||
|
||||
InstanceSettings::forceCreate(['id' => 0, 'is_api_enabled' => true]);
|
||||
|
||||
$this->keyMaterial = generateSSHKey('ed25519')['private'];
|
||||
$this->teamA = Team::factory()->create();
|
||||
$this->teamB = Team::factory()->create();
|
||||
});
|
||||
|
||||
function createTeamScopedPrivateKey(Team $team, string $material, string $name = 'key'): PrivateKey
|
||||
{
|
||||
return PrivateKey::create([
|
||||
'name' => $name,
|
||||
'private_key' => $material,
|
||||
'team_id' => $team->id,
|
||||
]);
|
||||
}
|
||||
|
||||
test('the same key can be saved in two different teams', function () {
|
||||
$first = createTeamScopedPrivateKey($this->teamA, $this->keyMaterial);
|
||||
$second = createTeamScopedPrivateKey($this->teamB, $this->keyMaterial);
|
||||
|
||||
expect($first->fingerprint)->toBe($second->fingerprint)
|
||||
->and(PrivateKey::query()->where('fingerprint', $first->fingerprint)->count())->toBe(2);
|
||||
});
|
||||
|
||||
test('a duplicate key in the same team is rejected', function () {
|
||||
createTeamScopedPrivateKey($this->teamA, $this->keyMaterial);
|
||||
|
||||
expect(fn () => createTeamScopedPrivateKey($this->teamA, $this->keyMaterial, 'duplicate'))
|
||||
->toThrow(ValidationException::class, 'This private key already exists.');
|
||||
});
|
||||
|
||||
test('updating a key does not collide with itself', function () {
|
||||
$key = createTeamScopedPrivateKey($this->teamA, $this->keyMaterial);
|
||||
|
||||
$key->update(['name' => 'renamed']);
|
||||
|
||||
expect($key->fresh()->name)->toBe('renamed');
|
||||
});
|
||||
|
||||
test('the root team (id 0) is scoped like any other team', function () {
|
||||
$rootTeam = Team::factory()->create(['id' => 0]);
|
||||
createTeamScopedPrivateKey($rootTeam, $this->keyMaterial);
|
||||
$fingerprint = PrivateKey::generateFingerprint($this->keyMaterial);
|
||||
|
||||
expect(PrivateKey::fingerprintExists($fingerprint, teamId: 0))->toBeTrue()
|
||||
->and(PrivateKey::fingerprintExists($fingerprint, teamId: $this->teamA->id))->toBeFalse()
|
||||
->and(fn () => createTeamScopedPrivateKey($rootTeam, $this->keyMaterial, 'duplicate'))
|
||||
->toThrow(ValidationException::class, 'This private key already exists.');
|
||||
|
||||
createTeamScopedPrivateKey($this->teamA, $this->keyMaterial);
|
||||
});
|
||||
|
||||
test('the saving hook checks the team of the key, not the team of the signed in user', function () {
|
||||
$user = User::factory()->create();
|
||||
$this->teamA->members()->attach($user->id, ['role' => 'owner']);
|
||||
$this->teamB->members()->attach($user->id, ['role' => 'owner']);
|
||||
$this->actingAs($user);
|
||||
session(['currentTeam' => $this->teamA]);
|
||||
|
||||
createTeamScopedPrivateKey($this->teamA, $this->keyMaterial);
|
||||
|
||||
// Another team may hold the same key while team A is the current team.
|
||||
createTeamScopedPrivateKey($this->teamB, $this->keyMaterial);
|
||||
|
||||
// A duplicate in team B is rejected even though team A is the current team.
|
||||
expect(fn () => createTeamScopedPrivateKey($this->teamB, $this->keyMaterial, 'duplicate'))
|
||||
->toThrow(ValidationException::class, 'This private key already exists.');
|
||||
});
|
||||
|
||||
test('without a signed in user the check does not see keys of other teams', function () {
|
||||
expect(auth()->check())->toBeFalse();
|
||||
$existing = createTeamScopedPrivateKey($this->teamA, $this->keyMaterial);
|
||||
|
||||
expect(PrivateKey::fingerprintExists($existing->fingerprint))->toBeFalse()
|
||||
->and(PrivateKey::fingerprintExists($existing->fingerprint, teamId: $this->teamB->id))->toBeFalse()
|
||||
->and(PrivateKey::fingerprintExists($existing->fingerprint, teamId: $this->teamA->id))->toBeTrue()
|
||||
->and(PrivateKey::fingerprintExists($existing->fingerprint, $existing->id, $this->teamA->id))->toBeFalse();
|
||||
|
||||
$copy = createTeamScopedPrivateKey($this->teamB, $this->keyMaterial);
|
||||
|
||||
expect($copy->exists)->toBeTrue();
|
||||
});
|
||||
|
||||
describe('POST /api/v1/security/keys', function () {
|
||||
beforeEach(function () {
|
||||
$this->user = User::factory()->create();
|
||||
$this->teamA->members()->attach($this->user->id, ['role' => 'owner']);
|
||||
$this->teamB->members()->attach($this->user->id, ['role' => 'owner']);
|
||||
|
||||
session(['currentTeam' => $this->teamA]);
|
||||
$token = $this->user->createToken('write-token', ['write']);
|
||||
DB::table('personal_access_tokens')->where('id', $token->accessToken->id)->update(['team_id' => $this->teamA->id]);
|
||||
$this->bearerToken = $token->plainTextToken;
|
||||
});
|
||||
|
||||
test('a key that exists in another team is created in the token team', function () {
|
||||
createTeamScopedPrivateKey($this->teamB, $this->keyMaterial);
|
||||
// The browser session points to team B; the API must use the token team.
|
||||
session(['currentTeam' => $this->teamB]);
|
||||
|
||||
$response = $this->withHeader('Authorization', 'Bearer '.$this->bearerToken)
|
||||
->postJson('/api/v1/security/keys', ['private_key' => $this->keyMaterial]);
|
||||
|
||||
$response->assertCreated();
|
||||
expect(PrivateKey::query()->where('uuid', $response->json('uuid'))->value('team_id'))->toBe($this->teamA->id);
|
||||
});
|
||||
|
||||
test('a key that exists in the token team is rejected', function () {
|
||||
createTeamScopedPrivateKey($this->teamA, $this->keyMaterial);
|
||||
session(['currentTeam' => $this->teamB]);
|
||||
|
||||
$this->withHeader('Authorization', 'Bearer '.$this->bearerToken)
|
||||
->postJson('/api/v1/security/keys', ['private_key' => $this->keyMaterial])
|
||||
->assertStatus(422)
|
||||
->assertJson(['message' => 'Private key already exists.']);
|
||||
|
||||
expect(PrivateKey::query()->where('team_id', $this->teamA->id)->count())->toBe(1);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user