diff --git a/app/Livewire/Source/Github/Change.php b/app/Livewire/Source/Github/Change.php index 1876b8c49..07755ec05 100644 --- a/app/Livewire/Source/Github/Change.php +++ b/app/Livewire/Source/Github/Change.php @@ -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) { diff --git a/app/Livewire/Source/Gitlab/Change.php b/app/Livewire/Source/Gitlab/Change.php index f22f1845d..dd0284582 100644 --- a/app/Livewire/Source/Gitlab/Change.php +++ b/app/Livewire/Source/Gitlab/Change.php @@ -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) { diff --git a/app/Policies/GithubAppPolicy.php b/app/Policies/GithubAppPolicy.php index 79dd79838..d152ab1d1 100644 --- a/app/Policies/GithubAppPolicy.php +++ b/app/Policies/GithubAppPolicy.php @@ -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); } diff --git a/app/Policies/GitlabAppPolicy.php b/app/Policies/GitlabAppPolicy.php index 56861e89b..8a9d773d6 100644 --- a/app/Policies/GitlabAppPolicy.php +++ b/app/Policies/GitlabAppPolicy.php @@ -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); } diff --git a/resources/views/livewire/source/github/change.blade.php b/resources/views/livewire/source/github/change.blade.php index 3b0ac9388..795c1b1fc 100644 --- a/resources/views/livewire/source/github/change.blade.php +++ b/resources/views/livewire/source/github/change.blade.php @@ -15,22 +15,24 @@ Test Connection @endif - @can('delete', $github_app) - @if ($applications->count() > 0) - - @else - - @endif - @endcan + @if ($github_app) + @can('delete', $github_app) + @if ($applications->count() > 0) + + @else + + @endif + @endcan + @endif
Your Private GitHub App for private repositories.
@@ -232,13 +234,15 @@

GitHub App

- @can('delete', $github_app) - - @endcan + @if ($github_app) + @can('delete', $github_app) + + @endcan + @endif
diff --git a/resources/views/livewire/source/gitlab/change.blade.php b/resources/views/livewire/source/gitlab/change.blade.php index b743fa0d7..011c342b3 100644 --- a/resources/views/livewire/source/gitlab/change.blade.php +++ b/resources/views/livewire/source/gitlab/change.blade.php @@ -9,14 +9,16 @@
Save Test Connection - @can('delete', $gitlab_app) - - @endcan + @if ($gitlab_app) + @can('delete', $gitlab_app) + + @endcan + @endif
Your GitLab App for private repositories.
@@ -120,14 +122,16 @@

GitLab App

- @can('delete', $gitlab_app) - - @endcan + @if ($gitlab_app) + @can('delete', $gitlab_app) + + @endcan + @endif
Connect your GitLab instance to deploy private repositories.
diff --git a/tests/Feature/GitlabAppAuthorizationTest.php b/tests/Feature/GitlabAppAuthorizationTest.php index 4ff591120..860ef41c0 100644 --- a/tests/Feature/GitlabAppAuthorizationTest.php +++ b/tests/Feature/GitlabAppAuthorizationTest.php @@ -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(); + }); + }); diff --git a/tests/Unit/Policies/GithubAppPolicyTest.php b/tests/Unit/Policies/GithubAppPolicyTest.php index 5aedbf7b8..3e6eb173c 100644 --- a/tests/Unit/Policies/GithubAppPolicyTest.php +++ b/tests/Unit/Policies/GithubAppPolicyTest.php @@ -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; diff --git a/tests/Unit/Policies/GitlabAppPolicyTest.php b/tests/Unit/Policies/GitlabAppPolicyTest.php index a2cd2a091..f0a67718a 100644 --- a/tests/Unit/Policies/GitlabAppPolicyTest.php +++ b/tests/Unit/Policies/GitlabAppPolicyTest.php @@ -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;