diff --git a/app/Models/Team.php b/app/Models/Team.php index 479bf4e00..d5d564444 100644 --- a/app/Models/Team.php +++ b/app/Models/Team.php @@ -71,27 +71,31 @@ protected static function booted() } }); - static::deleting(function ($team) { - $keys = $team->privateKeys; - foreach ($keys as $key) { + static::deleting(function (Team $team) { + foreach ($team->privateKeys as $key) { $key->delete(); } - // Only delete sources owned by this team, not system-wide ones from other teams + + // Transfer instance-wide sources to root team so they remain available + GithubApp::where('team_id', $team->id)->where('is_system_wide', true)->update(['team_id' => 0]); + GitlabApp::where('team_id', $team->id)->where('is_system_wide', true)->update(['team_id' => 0]); + + // Delete non-instance-wide sources owned by this team $teamSources = GithubApp::where('team_id', $team->id)->get() ->merge(GitlabApp::where('team_id', $team->id)->get()); foreach ($teamSources as $source) { $source->delete(); } - $tags = Tag::whereTeamId($team->id)->get(); - foreach ($tags as $tag) { + + foreach (Tag::whereTeamId($team->id)->get() as $tag) { $tag->delete(); } - $shared_variables = $team->environment_variables(); - foreach ($shared_variables as $shared_variable) { - $shared_variable->delete(); + + foreach ($team->environment_variables()->get() as $sharedVariable) { + $sharedVariable->delete(); } - $s3s = $team->s3s; - foreach ($s3s as $s3) { + + foreach ($team->s3s as $s3) { $s3->delete(); } }); @@ -340,4 +344,5 @@ public function webhookNotificationSettings() { return $this->hasOne(WebhookNotificationSettings::class); } + } diff --git a/app/Models/User.php b/app/Models/User.php index 7a9600ec4..3199d2024 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -176,23 +176,6 @@ private static function finalizeTeamDeletion(User $user, Team $team) $project->forceDelete(); } - // Detach applications from other teams that reference this team's sources, - // so the GithubApp/GitlabApp deleting guard doesn't block team deletion - $githubAppIds = GithubApp::where('team_id', $team->id)->pluck('id'); - $gitlabAppIds = GitlabApp::where('team_id', $team->id)->pluck('id'); - - if ($githubAppIds->isNotEmpty()) { - Application::where('source_type', GithubApp::class) - ->whereIn('source_id', $githubAppIds) - ->update(['source_id' => null, 'source_type' => null]); - } - - if ($gitlabAppIds->isNotEmpty()) { - Application::where('source_type', GitlabApp::class) - ->whereIn('source_id', $gitlabAppIds) - ->update(['source_id' => null, 'source_type' => null]); - } - $team->members()->detach($user->id); $team->delete(); } diff --git a/tests/Feature/UserDeletionWithGithubAppTest.php b/tests/Feature/UserDeletionWithGithubAppTest.php index d8986e378..b2ccbccb0 100644 --- a/tests/Feature/UserDeletionWithGithubAppTest.php +++ b/tests/Feature/UserDeletionWithGithubAppTest.php @@ -118,49 +118,101 @@ expect(GithubApp::find($systemGithubApp->id))->not->toBeNull(); }); -it('nullifies source references on other teams apps when deleting a user', function () { - // Create the user to be deleted with their own team +it('transfers instance-wide github app to root team when owning user is deleted', function () { + // Create a user whose team owns an instance-wide GitHub App $targetUser = User::factory()->create(); $targetTeam = $targetUser->teams()->first(); - // Create a GitHub App owned by the target team $targetPrivateKey = PrivateKey::factory()->create(['team_id' => $targetTeam->id]); - $githubApp = GithubApp::create([ - 'name' => 'Target GitHub App', + $instanceWideApp = GithubApp::create([ + 'name' => 'Instance-Wide GitHub App', 'team_id' => $targetTeam->id, 'private_key_id' => $targetPrivateKey->id, 'api_url' => 'https://api.github.com', 'html_url' => 'https://github.com', 'is_public' => false, + 'is_system_wide' => true, ]); - // Create an application on the ADMIN's team that uses the target team's GitHub App - $adminPrivateKey = PrivateKey::factory()->create(['team_id' => $this->rootTeam->id]); - $adminServer = Server::factory()->create([ + // Create an application on the ROOT team that uses this instance-wide GitHub App + $rootPrivateKey = PrivateKey::factory()->create(['team_id' => $this->rootTeam->id]); + $rootServer = Server::factory()->create([ 'team_id' => $this->rootTeam->id, - 'private_key_id' => $adminPrivateKey->id, + 'private_key_id' => $rootPrivateKey->id, ]); - $adminDestination = StandaloneDocker::factory()->create(['server_id' => $adminServer->id]); - $adminProject = Project::factory()->create(['team_id' => $this->rootTeam->id]); - $adminEnvironment = Environment::factory()->create(['project_id' => $adminProject->id]); + $rootDestination = StandaloneDocker::factory()->create(['server_id' => $rootServer->id]); + $rootProject = Project::factory()->create(['team_id' => $this->rootTeam->id]); + $rootEnvironment = Environment::factory()->create(['project_id' => $rootProject->id]); $otherTeamApp = Application::factory()->create([ - 'environment_id' => $adminEnvironment->id, - 'destination_id' => $adminDestination->id, + 'environment_id' => $rootEnvironment->id, + 'destination_id' => $rootDestination->id, 'destination_type' => StandaloneDocker::class, - 'source_id' => $githubApp->id, + 'source_id' => $instanceWideApp->id, 'source_type' => GithubApp::class, ]); - // Delete the target user — should succeed, nullifying the source reference + // Delete the user — should succeed and transfer the instance-wide app to root team $targetUser->delete(); // Assert user is deleted expect(User::find($targetUser->id))->toBeNull(); - // Assert the other team's application still exists but source is nullified + // Assert the instance-wide GitHub App is preserved and transferred to root team + $instanceWideApp->refresh(); + expect($instanceWideApp)->not->toBeNull(); + expect($instanceWideApp->team_id)->toBe($this->rootTeam->id); + + // Assert the other team's application still has its source intact $otherTeamApp->refresh(); - expect($otherTeamApp)->not->toBeNull(); - expect($otherTeamApp->source_id)->toBeNull(); - expect($otherTeamApp->source_type)->toBeNull(); + expect($otherTeamApp->source_id)->toBe($instanceWideApp->id); + expect($otherTeamApp->source_type)->toBe(GithubApp::class); +}); + +it('transfers instance-wide github app to root team when team is deleted directly', function () { + // Create a team that owns an instance-wide GitHub App + $targetUser = User::factory()->create(); + $targetTeam = $targetUser->teams()->first(); + + $targetPrivateKey = PrivateKey::factory()->create(['team_id' => $targetTeam->id]); + $instanceWideApp = GithubApp::create([ + 'name' => 'Instance-Wide GitHub App', + 'team_id' => $targetTeam->id, + 'private_key_id' => $targetPrivateKey->id, + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'is_public' => false, + 'is_system_wide' => true, + ]); + + // Create an application on the ROOT team that uses this instance-wide GitHub App + $rootPrivateKey = PrivateKey::factory()->create(['team_id' => $this->rootTeam->id]); + $rootServer = Server::factory()->create([ + 'team_id' => $this->rootTeam->id, + 'private_key_id' => $rootPrivateKey->id, + ]); + $rootDestination = StandaloneDocker::factory()->create(['server_id' => $rootServer->id]); + $rootProject = Project::factory()->create(['team_id' => $this->rootTeam->id]); + $rootEnvironment = Environment::factory()->create(['project_id' => $rootProject->id]); + + $otherTeamApp = Application::factory()->create([ + 'environment_id' => $rootEnvironment->id, + 'destination_id' => $rootDestination->id, + 'destination_type' => StandaloneDocker::class, + 'source_id' => $instanceWideApp->id, + 'source_type' => GithubApp::class, + ]); + + // Delete the team directly — should transfer instance-wide app to root team + $targetTeam->delete(); + + // Assert the instance-wide GitHub App is preserved and transferred to root team + $instanceWideApp->refresh(); + expect($instanceWideApp)->not->toBeNull(); + expect($instanceWideApp->team_id)->toBe($this->rootTeam->id); + + // Assert the other team's application still has its source intact + $otherTeamApp->refresh(); + expect($otherTeamApp->source_id)->toBe($instanceWideApp->id); + expect($otherTeamApp->source_type)->toBe(GithubApp::class); });