starred/coolify
1
0
Fork 0
mirror of https://github.com/coollabsio/coolify.git synced 2026-08-24 02:24:11 -05:00
Code Issues Packages Projects Releases Wiki Activity

fix(teams): restrict deletion to owners and backfill team ownership

Browse Source
This commit is contained in:
Andras Bacsai
2026-08-20 09:32:32 +02:00
parent 14d40c73a2
commit 680071dca8
14 changed files with 364 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Actions/Team/DeleteTeam.php
+65
View File
@@ -0,0 +1,65 @@
<?php
namespace App\Actions\Team;
use App\Models\Application;
use App\Models\Team;
use App\Models\User;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\DB;
use RuntimeException;
class DeleteTeam
{
public function handle(Team $team, User $user): ?Team
{
$newTeam = DB::transaction(function () use ($team, $user): ?Team {
$team = Team::query()->lockForUpdate()->findOrFail($team->id);
$role = DB::table('team_user')
->where('team_id', $team->id)
->where('user_id', $user->id)
->lockForUpdate()
->value('role');
if ($role !== 'owner') {
throw new AuthorizationException('Only team owners can delete a team.');
}
$hasRunningApplications = Application::query()
->whereHas('environment.project', fn ($query) => $query->where('team_id', $team->id))
->lockForUpdate()
->get(['id', 'status'])
->contains(fn (Application $application): bool => $application->isRunning());
if ($hasRunningApplications) {
throw new RuntimeException('Stop all running applications before deleting this team.');
}
if ($team->servers()->lockForUpdate()->get(['servers.id'])->isNotEmpty()) {
throw new RuntimeException('Delete all team servers before deleting this team.');
}
if (! $team->isEmpty()) {
throw new RuntimeException('Delete all team resources before deleting this team.');
}
$team->members()
->where('users.id', '!=', $user->id)
->get()
->each(function (User $member) use ($team): void {
$member->teams()->detach($team);
DB::table('sessions')->where('user_id', $member->id)->delete();
});
$team->delete();
return $user->teams()->first();
});
Cache::forget("user:{$user->id}:team:{$team->id}");
return $newTeam;
}
}
app/Livewire/NavbarDeleteTeam.php
+2 -19
View File
@@ -2,10 +2,8 @@
namespace App\Livewire;
use App\Actions\Team\DeleteTeam;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\DB;
use Livewire\Component;
class NavbarDeleteTeam extends Component
@@ -28,22 +26,7 @@ class NavbarDeleteTeam extends Component
$currentTeam = currentTeam();
$this->authorize('delete', $currentTeam);
$currentTeam->members->each(function ($user) use ($currentTeam) {
if ($user->id === Auth::id()) {
return;
}
$user->teams()->detach($currentTeam);
$session = DB::table('sessions')->where('user_id', $user->id)->first();
if ($session) {
DB::table('sessions')->where('id', $session->id)->delete();
}
});
Cache::forget('user:'.Auth::id().':team:'.$currentTeam->id);
$currentTeam->delete();
$newTeam = Auth::user()->teams()->first();
$newTeam = app(DeleteTeam::class)->handle($currentTeam, auth()->user());
refreshSession($newTeam);
return redirect()->route('team.index');
app/Livewire/Team/Create.php
+1 -1
View File
@@ -35,7 +35,7 @@ class Create extends Component
'personal_team' => false,
'is_mcp_server_enabled' => true,
]);
auth()->user()->teams()->attach($team, ['role' => 'admin']);
auth()->user()->teams()->attach($team, ['role' => 'owner']);
refreshSession($team);
return redirectRoute($this, 'team.index');
app/Livewire/Team/DangerZone.php
+8 -19
View File
@@ -2,11 +2,9 @@
namespace App\Livewire\Team;
use App\Actions\Team\DeleteTeam;
use App\Models\Team;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\DB;
use Livewire\Component;
class DangerZone extends Component
@@ -25,22 +23,7 @@ class DangerZone extends Component
try {
$currentTeam = currentTeam();
$this->authorize('delete', $currentTeam);
$currentTeam->members->each(function ($user) use ($currentTeam): void {
if ($user->id === Auth::id()) {
return;
}
$user->teams()->detach($currentTeam);
$session = DB::table('sessions')->where('user_id', $user->id)->first();
if ($session) {
DB::table('sessions')->where('id', $session->id)->delete();
}
});
Cache::forget('user:'.Auth::id().':team:'.$currentTeam->id);
$currentTeam->delete();
$newTeam = Auth::user()->teams()->first();
$newTeam = app(DeleteTeam::class)->handle($currentTeam, auth()->user());
refreshSession($newTeam);
return redirect()->route('team.index');
@@ -49,6 +32,12 @@ class DangerZone extends Component
}
}
public function refreshResources(): void
{
$this->team = Team::query()->findOrFail($this->team->id);
refreshSession($this->team);
}
public function render(): mixed
{
return view('livewire.team.danger-zone');
app/Policies/TeamPolicy.php
+1 -1
View File
@@ -53,7 +53,7 @@ class TeamPolicy
return false;
}
return $user->isAdminOfTeam($team->id);
return $user->roleInTeam($team->id) === 'owner';
}
/**
database/migrations/2026_08_20_070831_promote_first_team_member_when_team_has_no_owner.php
+52
View File
@@ -0,0 +1,52 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Support\Facades\DB;
return new class extends Migration
{
/**
* Run the migrations.
*/
public function up(): void
{
DB::table('teams')
->select('teams.id')
->whereNotExists(function ($query): void {
$query->selectRaw('1')
->from('team_user as owners')
->whereColumn('owners.team_id', 'teams.id')
->where('owners.role', 'owner');
})
->orderBy('teams.id')
->chunkById(100, function ($teams): void {
foreach ($teams as $team) {
$firstMember = DB::table('team_user')
->where('team_id', $team->id)
->orderByRaw("CASE WHEN role = 'admin' THEN 0 ELSE 1 END")
->orderBy('created_at')
->orderBy('id')
->first();
if ($firstMember === null) {
continue;
}
DB::table('team_user')
->where('id', $firstMember->id)
->update([
'role' => 'owner',
'updated_at' => now(),
]);
}
}, 'teams.id', 'id');
}
/**
* Reverse the migrations.
*/
public function down(): void
{
// This data migration cannot identify which owners were promoted safely.
}
};
resources/views/livewire/navbar-delete-team.blade.php
+2
View File
@@ -1,4 +1,5 @@
<div class="w-full min-w-0">
@if (auth()->user()->roleInTeam(currentTeam()->id) === 'owner')
<x-modal-confirmation title="Confirm Team Deletion?" buttonFullWidth isErrorButton submitAction="delete"
:actions="['The current Team will be permanently deleted.']" confirmationText="{{ $team }}"
confirmationLabel="Please confirm the execution of the actions by entering the Team Name below"
@@ -11,4 +12,5 @@
</button>
</x-slot:trigger>
</x-modal-confirmation>
@endif
</div>
resources/views/livewire/team/danger-zone.blade.php
+51 -22
View File
@@ -16,7 +16,11 @@
<x-status-badge status="Permanent" type="error" />
</div>
@if (session('currentTeam.id') === 0)
@if (auth()->user()->roleInTeam(currentTeam()->id) !== 'owner')
<p class="mt-2 text-[13px] leading-5 text-neutral-600 dark:text-fg-dim">
Only team owners can delete this team.
</p>
@elseif (session('currentTeam.id') === 0)
<p class="mt-2 text-[13px] leading-5 text-neutral-600 dark:text-fg-dim">
The default team cannot be deleted.
</p>
@@ -49,6 +53,7 @@
<div class="shrink-0">
@if (
session('currentTeam.id') !== 0 &&
auth()->user()->roleInTeam(currentTeam()->id) === 'owner' &&
auth()->user()->teams()->count() > 1 &&
!auth()->user()->currentTeam()->personal_team &&
!currentTeam()->subscription &&
@@ -69,27 +74,51 @@
</div>
</div>
@if (session('currentTeam.id') !== 0 && !currentTeam()->subscription && !currentTeam()->isEmpty())
<div class="mt-4 grid gap-3 sm:grid-cols-2 lg:grid-cols-4">
@foreach ([
'Projects' => currentTeam()->projects,
'Servers' => currentTeam()->servers,
'Private keys' => currentTeam()->privateKeys,
'Sources' => currentTeam()->sources(),
] as $label => $resources)
@if ($resources->isNotEmpty())
<div class="rounded-lg border border-neutral-200 p-3 dark:border-white/[0.08]">
<p class="text-[11px] font-semibold uppercase tracking-wide text-neutral-500 dark:text-fg-faint">
{{ $label }}
</p>
<ul class="mt-2 space-y-1 text-[12px] text-neutral-600 dark:text-fg-dim">
@foreach ($resources as $resource)
<li class="truncate">{{ $resource->name }}</li>
@endforeach
</ul>
</div>
@endif
@endforeach
@if (session('currentTeam.id') !== 0 && !currentTeam()->subscription && (currentTeam()->projects->isNotEmpty() || currentTeam()->servers->isNotEmpty()))
<div class="mt-4 overflow-hidden rounded-lg border border-neutral-200 dark:border-white/[0.08]">
<div class="flex items-center justify-between gap-3 border-b border-neutral-200 px-3 py-2 dark:border-white/[0.08]">
<h5 class="text-sm font-medium text-black dark:text-fg">Resources</h5>
<x-forms.button type="button" wire:click="refreshResources">
<x-reicon name="refresh" class="size-3.5" />
Refresh
</x-forms.button>
</div>
<table class="w-full text-left text-sm">
<thead class="bg-neutral-50 text-[11px] uppercase tracking-wide text-neutral-500 dark:bg-coolgray-100 dark:text-fg-dim">
<tr>
<th class="px-3 py-2 font-medium">Resource</th>
<th class="px-3 py-2 font-medium">Name</th>
</tr>
</thead>
<tbody class="divide-y divide-neutral-200 dark:divide-white/[0.08]">
@foreach (currentTeam()->projects as $project)
<tr class="text-[13px] text-neutral-600 hover:bg-neutral-50 dark:text-fg-dim dark:hover:bg-white/[0.03]">
<td>
<a class="block px-3 py-2.5" href="{{ route('project.show', ['project_uuid' => $project->uuid]) }}"
target="_blank" rel="noopener noreferrer">Project</a>
</td>
<td>
<a class="block px-3 py-2.5 font-medium text-black dark:text-fg"
href="{{ route('project.show', ['project_uuid' => $project->uuid]) }}"
target="_blank" rel="noopener noreferrer">{{ $project->name }}</a>
</td>
</tr>
@endforeach
@foreach (currentTeam()->servers as $server)
<tr class="text-[13px] text-neutral-600 hover:bg-neutral-50 dark:text-fg-dim dark:hover:bg-white/[0.03]">
<td>
<a class="block px-3 py-2.5" href="{{ route('server.show', ['server_uuid' => $server->uuid]) }}"
target="_blank" rel="noopener noreferrer">Server</a>
</td>
<td>
<a class="block px-3 py-2.5 font-medium text-black dark:text-fg"
href="{{ route('server.show', ['server_uuid' => $server->uuid]) }}"
target="_blank" rel="noopener noreferrer">{{ $server->name }}</a>
</td>
</tr>
@endforeach
</tbody>
</table>
</div>
@endif
</x-application.settings-section>
tests/Feature/Authorization/TeamAuthorizationTest.php
+9 -8
View File
@@ -1,5 +1,6 @@
<?php
use App\Livewire\Team\DangerZone;
use App\Livewire\Team\Index as TeamIndex;
use App\Livewire\Team\Member as TeamMember;
use App\Models\InstanceSettings;
@@ -60,11 +61,11 @@ test('owner can delete team', function () {
expect(auth()->user()->can('delete', $this->team))->toBeTrue();
});
test('admin can delete team', function () {
test('admin cannot delete team', function () {
$this->actingAs($this->admin);
session(['currentTeam' => $this->team]);
expect(auth()->user()->can('delete', $this->team))->toBeTrue();
expect(auth()->user()->can('delete', $this->team))->toBeFalse();
});
test('member cannot delete team', function () {
@@ -156,24 +157,24 @@ test('team index mounts when is_mcp_server_enabled is null on the session team',
->assertSet('is_mcp_server_enabled', true);
});
// --- Team Index Livewire: delete ---
// --- Team Danger Zone Livewire: delete ---
test('member cannot delete team via index', function () {
test('member cannot delete team via danger zone', function () {
$this->actingAs($this->member);
session(['currentTeam' => $this->team]);
Livewire::test(TeamIndex::class)
->call('delete', 'password')
Livewire::test(DangerZone::class)
->call('delete')
->assertDispatched('error');
expect(Team::find($this->team->id))->not->toBeNull();
});
test('admin can delete team via policy', function () {
test('admin cannot delete team via policy', function () {
$this->actingAs($this->admin);
session(['currentTeam' => $this->team]);
expect(auth()->user()->can('delete', $this->team))->toBeTrue();
expect(auth()->user()->can('delete', $this->team))->toBeFalse();
});
// --- Team Member Livewire: role changes ---
tests/Feature/Team/TeamDeletionTest.php
+108
View File
@@ -1,11 +1,19 @@
<?php
use App\Actions\Team\DeleteTeam;
use App\Livewire\Team\DangerZone;
use App\Models\Application;
use App\Models\Environment;
use App\Models\GithubApp;
use App\Models\GitlabApp;
use App\Models\InstanceSettings;
use App\Models\PrivateKey;
use App\Models\Project;
use App\Models\Server;
use App\Models\StandaloneDocker;
use App\Models\Team;
use App\Models\User;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Livewire\Livewire;
@@ -42,6 +50,89 @@ test('deleting a team switches session to another team without error', function
->and($sessionTeam->id)->toBe($this->personalTeam->id);
});
test('the danger zone resource list can be refreshed', function () {
$this->actingAs($this->owner);
session(['currentTeam' => $this->teamToDelete]);
Livewire::test(DangerZone::class)
->call('refreshResources')
->assertSuccessful();
});
test('a team with a running application cannot be deleted', function () {
$server = Server::factory()->create(['team_id' => $this->teamToDelete->id]);
$destination = StandaloneDocker::query()->where('server_id', $server->id)->firstOrFail();
$project = Project::factory()->create(['team_id' => $this->teamToDelete->id]);
$environment = Environment::factory()->create(['project_id' => $project->id]);
Application::factory()->create([
'environment_id' => $environment->id,
'destination_id' => $destination->id,
'destination_type' => $destination->getMorphClass(),
'status' => 'running:healthy',
]);
$this->actingAs($this->owner);
session(['currentTeam' => $this->teamToDelete]);
Livewire::test(DangerZone::class)
->call('delete')
->assertDispatched('error');
expect(Team::find($this->teamToDelete->id))->not->toBeNull();
});
test('a team with a server cannot be deleted', function () {
Server::factory()->create(['team_id' => $this->teamToDelete->id]);
$this->actingAs($this->owner);
session(['currentTeam' => $this->teamToDelete]);
Livewire::test(DangerZone::class)
->call('delete')
->assertDispatched('error', fn (string $event, array $params): bool => $params[0] === 'Delete all team servers before deleting this team.');
expect(Team::find($this->teamToDelete->id))->not->toBeNull();
});
test('a team with a project but no servers cannot be deleted', function () {
Project::factory()->create(['team_id' => $this->teamToDelete->id]);
$member = User::factory()->create();
$this->teamToDelete->members()->attach($member->id, ['role' => 'member']);
$privateKey = PrivateKey::factory()->create(['team_id' => $this->teamToDelete->id]);
$this->actingAs($this->owner);
session(['currentTeam' => $this->teamToDelete]);
Livewire::test(DangerZone::class)
->call('delete')
->assertDispatched('error', fn (string $event, array $params): bool => $params[0] === 'Delete all team resources before deleting this team.');
expect(Team::find($this->teamToDelete->id))->not->toBeNull()
->and(PrivateKey::find($privateKey->id))->not->toBeNull()
->and($this->teamToDelete->members()->whereKey($member->id)->exists())->toBeTrue();
});
test('an admin cannot delete a team through the deletion action', function () {
$admin = User::factory()->create();
$this->teamToDelete->members()->attach($admin->id, ['role' => 'admin']);
expect(fn () => app(DeleteTeam::class)->handle($this->teamToDelete, $admin))
->toThrow(AuthorizationException::class);
expect(Team::find($this->teamToDelete->id))->not->toBeNull();
});
test('a stale owner relationship cannot authorize team deletion', function () {
$this->owner->teams;
$this->owner->teams()->updateExistingPivot($this->teamToDelete->id, ['role' => 'admin']);
expect(fn () => app(DeleteTeam::class)->handle($this->teamToDelete, $this->owner))
->toThrow(AuthorizationException::class);
expect(Team::find($this->teamToDelete->id))->not->toBeNull();
});
test('refreshSession clears session when no team exists', function () {
$user = User::factory()->create();
// Detach all teams so user has none
@@ -78,3 +169,20 @@ test('deleting a team deletes github and gitlab sources with the same primary ke
expect(GithubApp::find($githubApp->id))->toBeNull()
->and(GitlabApp::find($gitlabApp->id))->toBeNull();
});
test('team deletion rolls back all database changes when an operation fails', function () {
$member = User::factory()->create();
$this->teamToDelete->members()->attach($member->id, ['role' => 'member']);
Team::deleting(function (Team $deletingTeam): void {
if ($deletingTeam->id === $this->teamToDelete->id) {
throw new RuntimeException('Simulated deletion failure.');
}
});
expect(fn () => app(DeleteTeam::class)->handle($this->teamToDelete, $this->owner))
->toThrow(RuntimeException::class, 'Simulated deletion failure.');
expect(Team::find($this->teamToDelete->id))->not->toBeNull()
->and($this->teamToDelete->members()->whereKey($member->id)->exists())->toBeTrue();
});
tests/Feature/TeamCreateSetsMcpDefaultTest.php
+2 -1
View File
@@ -33,5 +33,6 @@ test('creating a team sets is_mcp_server_enabled to true on the model', function
$created = Team::query()->where('name', 'MCP Safe Team')->first();
expect($created)->not->toBeNull()
->and($created->is_mcp_server_enabled)->toBeTrue();
->and($created->is_mcp_server_enabled)->toBeTrue()
->and($this->user->fresh()->roleInTeam($created->id))->toBe('owner');
});
tests/Feature/TeamOwnerBackfillMigrationTest.php
+42
View File
@@ -0,0 +1,42 @@
<?php
use App\Models\Team;
use App\Models\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
uses(RefreshDatabase::class);
test('it promotes the first member only when a team has no owner', function () {
$teamWithoutOwner = Team::factory()->create(['personal_team' => false]);
$firstMember = User::factory()->create();
$firstAdmin = User::factory()->create();
$teamWithoutOwner->members()->attach($firstMember, [
'role' => 'member',
'created_at' => now()->subMinute(),
'updated_at' => now()->subMinute(),
]);
$teamWithoutOwner->members()->attach($firstAdmin, [
'role' => 'admin',
'created_at' => now(),
'updated_at' => now(),
]);
$teamWithoutAdmin = Team::factory()->create(['personal_team' => false]);
$fallbackMember = User::factory()->create();
$teamWithoutAdmin->members()->attach($fallbackMember, ['role' => 'member']);
$teamWithOwner = Team::factory()->create(['personal_team' => false]);
$existingOwner = User::factory()->create();
$existingAdmin = User::factory()->create();
$teamWithOwner->members()->attach($existingOwner, ['role' => 'owner']);
$teamWithOwner->members()->attach($existingAdmin, ['role' => 'admin']);
$migration = require database_path('migrations/2026_08_20_070831_promote_first_team_member_when_team_has_no_owner.php');
$migration->up();
expect($teamWithoutOwner->members()->find($firstMember->id)->pivot->role)->toBe('member')
->and($teamWithoutOwner->members()->find($firstAdmin->id)->pivot->role)->toBe('owner')
->and($teamWithoutAdmin->members()->find($fallbackMember->id)->pivot->role)->toBe('owner')
->and($teamWithOwner->members()->find($existingOwner->id)->pivot->role)->toBe('owner')
->and($teamWithOwner->members()->find($existingAdmin->id)->pivot->role)->toBe('admin');
});
tests/Feature/TeamSettingsNavigationTest.php
+9 -2
View File
@@ -29,8 +29,15 @@ it('uses shared sidebar navigation for every team settings page', function () {
->toContain('Delete team')
->toContain('status="Permanent"')
->toContain('border-red-300')
->toContain("'Sources' => currentTeam()->sources()")
->not->toContain("'Sources' => currentTeam()->sources,");
->toContain('<table class="w-full text-left text-sm">')
->toContain('wire:click="refreshResources"')
->not->toContain('wire:loading.class="animate-spin"')
->toContain("route('project.show', ['project_uuid' => \$project->uuid])")
->toContain("route('server.show', ['server_uuid' => \$server->uuid])")
->toContain('target="_blank" rel="noopener noreferrer"')
->toContain('Delete every server owned by this team before deleting it.')
->toContain('currentTeam()->servers->isEmpty()')
->not->toContain('currentTeam()->isEmpty()');
expect(file_get_contents(resource_path('views/livewire/switch-team.blade.php')))
->toContain('New team')
->toContain('team-switcher-create-expanded')
tests/Unit/Policies/TeamPolicyTest.php
+12 -2
View File
@@ -56,7 +56,6 @@ it('allows target team admins to perform privileged team actions', function (str
expect((new TeamPolicy)->{$ability}($user, $team))->toBeTrue();
})->with([
'update',
'delete',
'manageMembers',
'viewAdmin',
'manageInvitations',
@@ -72,7 +71,6 @@ it('denies target team members even when their current session role is admin els
expect((new TeamPolicy)->{$ability}($user, $team))->toBeFalse();
})->with([
'update',
'delete',
'manageMembers',
'viewAdmin',
'manageInvitations',
@@ -90,3 +88,15 @@ it('denies non-members from privileged team actions', function (string $ability)
'viewAdmin',
'manageInvitations',
]);
it('only allows target team owners to delete the team', function (string $role, bool $allowed) {
$user = teamPolicyUserWithTeams([1]);
$user->shouldReceive('roleInTeam')->with(1)->andReturn($role);
$team = teamPolicyTeam(1);
expect((new TeamPolicy)->delete($user, $team))->toBe($allowed);
})->with([
'owner' => ['owner', true],
'admin' => ['admin', false],
'member' => ['member', false],
]);