mirror of
https://github.com/tiennm99/coolify.git
synced 2026-08-20 08:23:25 +00:00
fix(sources): prevent 500 when deleting GitLab/GitHub apps (#11105)
This commit is contained in:
@@ -478,6 +478,9 @@ class Change extends Component
|
||||
return;
|
||||
}
|
||||
$this->github_app->delete();
|
||||
// Clear so post-delete Livewire re-render / modal $refresh does not re-run
|
||||
// @can and canGate checks against a deleted model (null team_id TypeError).
|
||||
$this->github_app = null;
|
||||
|
||||
return redirect()->route('source.all');
|
||||
} catch (\Throwable $e) {
|
||||
|
||||
@@ -334,6 +334,9 @@ class Change extends Component
|
||||
return;
|
||||
}
|
||||
$this->gitlab_app->delete();
|
||||
// Clear so post-delete Livewire re-render / modal $refresh does not re-run
|
||||
// @can and canGate checks against a deleted model (null team_id TypeError).
|
||||
$this->gitlab_app = null;
|
||||
|
||||
return redirect()->route('source.all');
|
||||
} catch (\Throwable $e) {
|
||||
|
||||
@@ -44,6 +44,11 @@ class GithubAppPolicy
|
||||
return $user->canAccessSystemResources();
|
||||
}
|
||||
|
||||
// Guard null team_id (e.g. post-delete Livewire re-render of @can checks).
|
||||
if ($githubApp->team_id === null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return $user->isAdminOfTeam($githubApp->team_id);
|
||||
}
|
||||
|
||||
@@ -56,6 +61,11 @@ class GithubAppPolicy
|
||||
return $user->canAccessSystemResources();
|
||||
}
|
||||
|
||||
// Guard null team_id (e.g. post-delete Livewire re-render of @can checks).
|
||||
if ($githubApp->team_id === null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return $user->isAdminOfTeam($githubApp->team_id);
|
||||
}
|
||||
|
||||
|
||||
@@ -44,6 +44,11 @@ class GitlabAppPolicy
|
||||
return $user->canAccessSystemResources();
|
||||
}
|
||||
|
||||
// Guard null team_id (e.g. post-delete Livewire re-render of @can checks).
|
||||
if ($gitlabApp->team_id === null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return $user->isAdminOfTeam($gitlabApp->team_id);
|
||||
}
|
||||
|
||||
@@ -56,6 +61,11 @@ class GitlabAppPolicy
|
||||
return $user->canAccessSystemResources();
|
||||
}
|
||||
|
||||
// Guard null team_id (e.g. post-delete Livewire re-render of @can checks).
|
||||
if ($gitlabApp->team_id === null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return $user->isAdminOfTeam($gitlabApp->team_id);
|
||||
}
|
||||
|
||||
|
||||
@@ -15,22 +15,24 @@
|
||||
<x-forms.button canGate="view" :canResource="$github_app"
|
||||
wire:click.prevent="testConnection">Test Connection</x-forms.button>
|
||||
@endif
|
||||
@can('delete', $github_app)
|
||||
@if ($applications->count() > 0)
|
||||
<x-modal-confirmation title="Confirm GitHub App Deletion?" isErrorButton buttonTitle="Delete"
|
||||
submitAction="delete" :actions="['The selected GitHub App will be permanently deleted.']" confirmationText="{{ data_get($github_app, 'name') }}"
|
||||
confirmationLabel="Please confirm the execution of the actions by entering the GitHub App Name below"
|
||||
shortConfirmationLabel="GitHub App Name" :confirmWithPassword="false"
|
||||
step2ButtonText="Permanently Delete" />
|
||||
@else
|
||||
<x-modal-confirmation title="Confirm GitHub App Deletion?" isErrorButton buttonTitle="Delete"
|
||||
submitAction="delete" :actions="['The selected GitHub App will be permanently deleted.']"
|
||||
confirmationLabel="Please confirm the execution of the actions by entering the GitHub App Name below"
|
||||
shortConfirmationLabel="GitHub App Name"
|
||||
confirmationText="{{ data_get($github_app, 'name') }}" :confirmWithPassword="false"
|
||||
step2ButtonText="Permanently Delete" />
|
||||
@endif
|
||||
@endcan
|
||||
@if ($github_app)
|
||||
@can('delete', $github_app)
|
||||
@if ($applications->count() > 0)
|
||||
<x-modal-confirmation title="Confirm GitHub App Deletion?" isErrorButton buttonTitle="Delete"
|
||||
submitAction="delete" :actions="['The selected GitHub App will be permanently deleted.']" confirmationText="{{ data_get($github_app, 'name') }}"
|
||||
confirmationLabel="Please confirm the execution of the actions by entering the GitHub App Name below"
|
||||
shortConfirmationLabel="GitHub App Name" :confirmWithPassword="false"
|
||||
step2ButtonText="Permanently Delete" />
|
||||
@else
|
||||
<x-modal-confirmation title="Confirm GitHub App Deletion?" isErrorButton buttonTitle="Delete"
|
||||
submitAction="delete" :actions="['The selected GitHub App will be permanently deleted.']"
|
||||
confirmationLabel="Please confirm the execution of the actions by entering the GitHub App Name below"
|
||||
shortConfirmationLabel="GitHub App Name"
|
||||
confirmationText="{{ data_get($github_app, 'name') }}" :confirmWithPassword="false"
|
||||
step2ButtonText="Permanently Delete" />
|
||||
@endif
|
||||
@endcan
|
||||
@endif
|
||||
</div>
|
||||
</div>
|
||||
<div class="subtitle">Your Private GitHub App for private repositories.</div>
|
||||
@@ -232,13 +234,15 @@
|
||||
<div class="flex flex-col sm:flex-row sm:items-center gap-2 pb-4">
|
||||
<h1>GitHub App</h1>
|
||||
<div class="flex gap-2">
|
||||
@can('delete', $github_app)
|
||||
<x-modal-confirmation title="Confirm GitHub App Deletion?" isErrorButton buttonTitle="Delete"
|
||||
submitAction="delete" :actions="['The selected GitHub App will be permanently deleted.']" confirmationText="{{ data_get($github_app, 'name') }}"
|
||||
confirmationLabel="Please confirm the execution of the actions by entering the GitHub App Name below"
|
||||
shortConfirmationLabel="GitHub App Name" :confirmWithPassword="false"
|
||||
step2ButtonText="Permanently Delete" />
|
||||
@endcan
|
||||
@if ($github_app)
|
||||
@can('delete', $github_app)
|
||||
<x-modal-confirmation title="Confirm GitHub App Deletion?" isErrorButton buttonTitle="Delete"
|
||||
submitAction="delete" :actions="['The selected GitHub App will be permanently deleted.']" confirmationText="{{ data_get($github_app, 'name') }}"
|
||||
confirmationLabel="Please confirm the execution of the actions by entering the GitHub App Name below"
|
||||
shortConfirmationLabel="GitHub App Name" :confirmWithPassword="false"
|
||||
step2ButtonText="Permanently Delete" />
|
||||
@endcan
|
||||
@endif
|
||||
</div>
|
||||
</div>
|
||||
<div class="flex items-center justify-center min-h-[calc(100vh-12rem)]">
|
||||
|
||||
@@ -9,14 +9,16 @@
|
||||
<div class="flex gap-2">
|
||||
<x-forms.button canGate="update" :canResource="$gitlab_app" type="submit">Save</x-forms.button>
|
||||
<x-forms.button wire:click.prevent="testConnection">Test Connection</x-forms.button>
|
||||
@can('delete', $gitlab_app)
|
||||
<x-modal-confirmation title="Confirm GitLab App Deletion?" isErrorButton buttonTitle="Delete"
|
||||
submitAction="delete" :actions="['The selected GitLab App will be permanently deleted.']"
|
||||
confirmationText="{{ data_get($gitlab_app, 'name') }}"
|
||||
confirmationLabel="Please confirm by entering the GitLab App Name below"
|
||||
shortConfirmationLabel="GitLab App Name" :confirmWithPassword="false"
|
||||
step2ButtonText="Permanently Delete" />
|
||||
@endcan
|
||||
@if ($gitlab_app)
|
||||
@can('delete', $gitlab_app)
|
||||
<x-modal-confirmation title="Confirm GitLab App Deletion?" isErrorButton buttonTitle="Delete"
|
||||
submitAction="delete" :actions="['The selected GitLab App will be permanently deleted.']"
|
||||
confirmationText="{{ data_get($gitlab_app, 'name') }}"
|
||||
confirmationLabel="Please confirm by entering the GitLab App Name below"
|
||||
shortConfirmationLabel="GitLab App Name" :confirmWithPassword="false"
|
||||
step2ButtonText="Permanently Delete" />
|
||||
@endcan
|
||||
@endif
|
||||
</div>
|
||||
</div>
|
||||
<div class="subtitle">Your GitLab App for private repositories.</div>
|
||||
@@ -120,14 +122,16 @@
|
||||
<div class="flex flex-col sm:flex-row sm:items-center gap-2">
|
||||
<h1>GitLab App</h1>
|
||||
<div class="flex gap-2">
|
||||
@can('delete', $gitlab_app)
|
||||
<x-modal-confirmation title="Confirm GitLab App Deletion?" isErrorButton buttonTitle="Delete"
|
||||
submitAction="delete" :actions="['The selected GitLab App will be permanently deleted.']"
|
||||
confirmationText="{{ data_get($gitlab_app, 'name') }}"
|
||||
confirmationLabel="Please confirm by entering the GitLab App Name below"
|
||||
shortConfirmationLabel="GitLab App Name" :confirmWithPassword="false"
|
||||
step2ButtonText="Permanently Delete" />
|
||||
@endcan
|
||||
@if ($gitlab_app)
|
||||
@can('delete', $gitlab_app)
|
||||
<x-modal-confirmation title="Confirm GitLab App Deletion?" isErrorButton buttonTitle="Delete"
|
||||
submitAction="delete" :actions="['The selected GitLab App will be permanently deleted.']"
|
||||
confirmationText="{{ data_get($gitlab_app, 'name') }}"
|
||||
confirmationLabel="Please confirm by entering the GitLab App Name below"
|
||||
shortConfirmationLabel="GitLab App Name" :confirmWithPassword="false"
|
||||
step2ButtonText="Permanently Delete" />
|
||||
@endcan
|
||||
@endif
|
||||
</div>
|
||||
</div>
|
||||
<div class="subtitle">Connect your GitLab instance to deploy private repositories.</div>
|
||||
|
||||
@@ -144,4 +144,39 @@ describe('GitLab App authorization', function () {
|
||||
|
||||
expect(Application::count())->toBe($applicationsBefore);
|
||||
});
|
||||
|
||||
test('team owner can delete a gitlab app without type error', function () {
|
||||
$this->actingAs($this->owner);
|
||||
session(['currentTeam' => $this->team]);
|
||||
|
||||
$gitlabAppId = $this->gitlabApp->id;
|
||||
|
||||
Livewire::withQueryParams(['gitlab_app_uuid' => $this->gitlabApp->uuid])
|
||||
->test(Change::class)
|
||||
->call('delete')
|
||||
->assertRedirect(route('source.all'));
|
||||
|
||||
expect(GitlabApp::find($gitlabAppId))->toBeNull();
|
||||
});
|
||||
|
||||
test('delete policy is safe when team_id becomes null after model is removed', function () {
|
||||
$this->actingAs($this->owner);
|
||||
session(['currentTeam' => $this->team]);
|
||||
|
||||
// Reproduce the post-delete Livewire re-render path: @can('delete') runs while
|
||||
// the in-memory model may have a null team_id (TypeError in isAdminOfTeam).
|
||||
$orphaned = new GitlabApp([
|
||||
'name' => 'Orphaned',
|
||||
'api_url' => 'https://gitlab.example.com/api/v4',
|
||||
'html_url' => 'https://gitlab.example.com',
|
||||
'is_system_wide' => false,
|
||||
'team_id' => null,
|
||||
]);
|
||||
|
||||
expect(fn () => $this->owner->can('delete', $orphaned))->not->toThrow(TypeError::class);
|
||||
expect($this->owner->can('delete', $orphaned))->toBeFalse();
|
||||
expect(fn () => $this->owner->can('update', $orphaned))->not->toThrow(TypeError::class);
|
||||
expect($this->owner->can('update', $orphaned))->toBeFalse();
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
@@ -144,6 +144,26 @@ it('denies team member to delete non-system-wide github app', function () {
|
||||
expect($policy->delete($user, $model))->toBeFalse();
|
||||
});
|
||||
|
||||
it('denies update when team_id is null without type error', function () {
|
||||
$user = Mockery::mock(User::class)->makePartial();
|
||||
$user->shouldNotReceive('isAdminOfTeam');
|
||||
|
||||
$model = mockGithubApp(teamId: null, isSystemWide: false);
|
||||
|
||||
$policy = new GithubAppPolicy;
|
||||
expect($policy->update($user, $model))->toBeFalse();
|
||||
});
|
||||
|
||||
it('denies delete when team_id is null without type error', function () {
|
||||
$user = Mockery::mock(User::class)->makePartial();
|
||||
$user->shouldNotReceive('isAdminOfTeam');
|
||||
|
||||
$model = mockGithubApp(teamId: null, isSystemWide: false);
|
||||
|
||||
$policy = new GithubAppPolicy;
|
||||
expect($policy->delete($user, $model))->toBeFalse();
|
||||
});
|
||||
|
||||
it('denies restore of github app', function () {
|
||||
$user = Mockery::mock(User::class)->makePartial();
|
||||
|
||||
@@ -162,7 +182,7 @@ it('denies force delete of github app', function () {
|
||||
expect($policy->forceDelete($user, $model))->toBeFalse();
|
||||
});
|
||||
|
||||
function mockGithubApp(int $teamId, bool $isSystemWide): GithubApp
|
||||
function mockGithubApp(?int $teamId, bool $isSystemWide): GithubApp
|
||||
{
|
||||
$githubApp = Mockery::mock(GithubApp::class)->makePartial();
|
||||
$githubApp->team_id = $teamId;
|
||||
|
||||
@@ -144,6 +144,26 @@ it('denies team member to delete non-system-wide gitlab app', function () {
|
||||
expect($policy->delete($user, $model))->toBeFalse();
|
||||
});
|
||||
|
||||
it('denies update when team_id is null without type error', function () {
|
||||
$user = Mockery::mock(User::class)->makePartial();
|
||||
$user->shouldNotReceive('isAdminOfTeam');
|
||||
|
||||
$model = mockGitlabApp(teamId: null, isSystemWide: false);
|
||||
|
||||
$policy = new GitlabAppPolicy;
|
||||
expect($policy->update($user, $model))->toBeFalse();
|
||||
});
|
||||
|
||||
it('denies delete when team_id is null without type error', function () {
|
||||
$user = Mockery::mock(User::class)->makePartial();
|
||||
$user->shouldNotReceive('isAdminOfTeam');
|
||||
|
||||
$model = mockGitlabApp(teamId: null, isSystemWide: false);
|
||||
|
||||
$policy = new GitlabAppPolicy;
|
||||
expect($policy->delete($user, $model))->toBeFalse();
|
||||
});
|
||||
|
||||
it('denies restore of gitlab app', function () {
|
||||
$user = Mockery::mock(User::class)->makePartial();
|
||||
|
||||
@@ -162,7 +182,7 @@ it('denies force delete of gitlab app', function () {
|
||||
expect($policy->forceDelete($user, $model))->toBeFalse();
|
||||
});
|
||||
|
||||
function mockGitlabApp(int $teamId, bool $isSystemWide): GitlabApp
|
||||
function mockGitlabApp(?int $teamId, bool $isSystemWide): GitlabApp
|
||||
{
|
||||
$gitlabApp = Mockery::mock(GitlabApp::class)->makePartial();
|
||||
$gitlabApp->team_id = $teamId;
|
||||
|
||||
Reference in New Issue
Block a user