fix(user-deletion): handle GitHub app sources across team cleanup
Limit team cleanup to apps owned by the deleted team and nullify cross-team application source references before deleting team-owned sources. Adds feature tests covering user deletion with GitHub app-backed applications, preserving system-wide apps, and nullifying external source links.
This commit is contained in:
parent
968508583d
commit
3f564f9b2e
3 changed files with 187 additions and 2 deletions
|
|
@ -76,8 +76,10 @@ protected static function booted()
|
|||
foreach ($keys as $key) {
|
||||
$key->delete();
|
||||
}
|
||||
$sources = $team->sources();
|
||||
foreach ($sources as $source) {
|
||||
// Only delete sources owned by this team, not system-wide ones from other teams
|
||||
$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();
|
||||
|
|
|
|||
|
|
@ -176,6 +176,23 @@ 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();
|
||||
}
|
||||
|
|
|
|||
166
tests/Feature/UserDeletionWithGithubAppTest.php
Normal file
166
tests/Feature/UserDeletionWithGithubAppTest.php
Normal file
|
|
@ -0,0 +1,166 @@
|
|||
<?php
|
||||
|
||||
use App\Models\Application;
|
||||
use App\Models\Environment;
|
||||
use App\Models\GithubApp;
|
||||
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\Foundation\Testing\RefreshDatabase;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
// Create root team and admin user (instance admin)
|
||||
$this->rootTeam = Team::factory()->create(['id' => 0, 'name' => 'Root Team']);
|
||||
$this->adminUser = User::factory()->create();
|
||||
$this->rootTeam->members()->attach($this->adminUser->id, ['role' => 'owner']);
|
||||
$this->actingAs($this->adminUser);
|
||||
session(['currentTeam' => $this->rootTeam]);
|
||||
});
|
||||
|
||||
it('deletes a user whose team has a github app with applications', function () {
|
||||
// Create the user to be deleted with their own team
|
||||
$targetUser = User::factory()->create();
|
||||
$targetTeam = $targetUser->teams()->first(); // created by User::created event
|
||||
|
||||
// Create a private key for the team
|
||||
$privateKey = PrivateKey::factory()->create(['team_id' => $targetTeam->id]);
|
||||
|
||||
// Create a server and destination for the team
|
||||
$server = Server::factory()->create([
|
||||
'team_id' => $targetTeam->id,
|
||||
'private_key_id' => $privateKey->id,
|
||||
]);
|
||||
$destination = StandaloneDocker::factory()->create(['server_id' => $server->id]);
|
||||
|
||||
// Create a project and environment
|
||||
$project = Project::factory()->create(['team_id' => $targetTeam->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
// Create a GitHub App owned by the target team
|
||||
$githubApp = GithubApp::create([
|
||||
'name' => 'Test GitHub App',
|
||||
'team_id' => $targetTeam->id,
|
||||
'private_key_id' => $privateKey->id,
|
||||
'api_url' => 'https://api.github.com',
|
||||
'html_url' => 'https://github.com',
|
||||
'is_public' => false,
|
||||
]);
|
||||
|
||||
// Create an application that uses the GitHub App as its source
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'destination_id' => $destination->id,
|
||||
'destination_type' => StandaloneDocker::class,
|
||||
'source_id' => $githubApp->id,
|
||||
'source_type' => GithubApp::class,
|
||||
]);
|
||||
|
||||
// Delete the user — this should NOT throw a GithubApp exception
|
||||
$targetUser->delete();
|
||||
|
||||
// Assert user is deleted
|
||||
expect(User::find($targetUser->id))->toBeNull();
|
||||
|
||||
// Assert the GitHub App is deleted
|
||||
expect(GithubApp::find($githubApp->id))->toBeNull();
|
||||
|
||||
// Assert the application is deleted
|
||||
expect(Application::find($application->id))->toBeNull();
|
||||
});
|
||||
|
||||
it('does not delete system-wide github apps when deleting a different team', function () {
|
||||
// Create a system-wide GitHub App owned by the root team
|
||||
$rootPrivateKey = PrivateKey::factory()->create(['team_id' => $this->rootTeam->id]);
|
||||
$systemGithubApp = GithubApp::create([
|
||||
'name' => 'System GitHub App',
|
||||
'team_id' => $this->rootTeam->id,
|
||||
'private_key_id' => $rootPrivateKey->id,
|
||||
'api_url' => 'https://api.github.com',
|
||||
'html_url' => 'https://github.com',
|
||||
'is_public' => false,
|
||||
'is_system_wide' => true,
|
||||
]);
|
||||
|
||||
// Create a target user with their own team
|
||||
$targetUser = User::factory()->create();
|
||||
$targetTeam = $targetUser->teams()->first();
|
||||
|
||||
// Create an application on the target team that uses the system-wide GitHub App
|
||||
$privateKey = PrivateKey::factory()->create(['team_id' => $targetTeam->id]);
|
||||
$server = Server::factory()->create([
|
||||
'team_id' => $targetTeam->id,
|
||||
'private_key_id' => $privateKey->id,
|
||||
]);
|
||||
$destination = StandaloneDocker::factory()->create(['server_id' => $server->id]);
|
||||
$project = Project::factory()->create(['team_id' => $targetTeam->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'destination_id' => $destination->id,
|
||||
'destination_type' => StandaloneDocker::class,
|
||||
'source_id' => $systemGithubApp->id,
|
||||
'source_type' => GithubApp::class,
|
||||
]);
|
||||
|
||||
// Delete the target user — should NOT throw or delete the system-wide GitHub App
|
||||
$targetUser->delete();
|
||||
|
||||
// Assert user is deleted
|
||||
expect(User::find($targetUser->id))->toBeNull();
|
||||
|
||||
// Assert the system-wide GitHub App still exists
|
||||
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
|
||||
$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',
|
||||
'team_id' => $targetTeam->id,
|
||||
'private_key_id' => $targetPrivateKey->id,
|
||||
'api_url' => 'https://api.github.com',
|
||||
'html_url' => 'https://github.com',
|
||||
'is_public' => false,
|
||||
]);
|
||||
|
||||
// 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([
|
||||
'team_id' => $this->rootTeam->id,
|
||||
'private_key_id' => $adminPrivateKey->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]);
|
||||
|
||||
$otherTeamApp = Application::factory()->create([
|
||||
'environment_id' => $adminEnvironment->id,
|
||||
'destination_id' => $adminDestination->id,
|
||||
'destination_type' => StandaloneDocker::class,
|
||||
'source_id' => $githubApp->id,
|
||||
'source_type' => GithubApp::class,
|
||||
]);
|
||||
|
||||
// Delete the target user — should succeed, nullifying the source reference
|
||||
$targetUser->delete();
|
||||
|
||||
// Assert user is deleted
|
||||
expect(User::find($targetUser->id))->toBeNull();
|
||||
|
||||
// Assert the other team's application still exists but source is nullified
|
||||
$otherTeamApp->refresh();
|
||||
expect($otherTeamApp)->not->toBeNull();
|
||||
expect($otherTeamApp->source_id)->toBeNull();
|
||||
expect($otherTeamApp->source_type)->toBeNull();
|
||||
});
|
||||
Loading…
Reference in a new issue