diff --git a/app/Http/Controllers/Api/GithubController.php b/app/Http/Controllers/Api/GithubController.php index 85238e799b..3847c56021 100644 --- a/app/Http/Controllers/Api/GithubController.php +++ b/app/Http/Controllers/Api/GithubController.php @@ -247,6 +247,10 @@ class GithubController extends Controller ], 422); } + if (! isCloud() && $request->boolean('is_system_wide')) { + $this->authorize('makeSystemWide', GithubApp::class); + } + try { // Verify the private key belongs to the team $privateKey = PrivateKey::where('uuid', $request->input('private_key_uuid')) @@ -663,6 +667,10 @@ class GithubController extends Controller $payload['api_url'] = githubApiUrlFromHtmlUrl($payload['html_url']); } + if (! isCloud() && filter_var($payload['is_system_wide'] ?? false, FILTER_VALIDATE_BOOLEAN) && ! $githubApp->is_system_wide) { + $this->authorize('makeSystemWide', GithubApp::class); + } + // Handle private_key_uuid -> private_key_id conversion if (isset($payload['private_key_uuid'])) { $privateKey = PrivateKey::where('team_id', $teamId) diff --git a/app/Http/Controllers/Api/GitlabController.php b/app/Http/Controllers/Api/GitlabController.php index 959a3067aa..f33e5d8392 100644 --- a/app/Http/Controllers/Api/GitlabController.php +++ b/app/Http/Controllers/Api/GitlabController.php @@ -247,6 +247,10 @@ class GitlabController extends Controller ], 422); } + if (! isCloud() && $request->boolean('is_system_wide')) { + $this->authorize('makeSystemWide', GitlabApp::class); + } + try { $htmlUrl = rtrim((string) $request->input('html_url'), '/'); $apiUrl = filled($request->input('api_url')) @@ -420,6 +424,10 @@ class GitlabController extends Controller ], 422); } + if (! isCloud() && filter_var($payload['is_system_wide'] ?? false, FILTER_VALIDATE_BOOLEAN) && ! $gitlabApp->is_system_wide) { + $this->authorize('makeSystemWide', GitlabApp::class); + } + if (isset($payload['html_url'])) { $payload['html_url'] = rtrim((string) $payload['html_url'], '/'); if (! filled($payload['api_url'] ?? null)) { diff --git a/app/Livewire/Source/Github/Change.php b/app/Livewire/Source/Github/Change.php index feba6f3bea..7e0753ad69 100644 --- a/app/Livewire/Source/Github/Change.php +++ b/app/Livewire/Source/Github/Change.php @@ -416,10 +416,18 @@ class Change extends Component } } + private function authorizeSystemWideChange(): void + { + if ($this->isSystemWide && ! $this->github_app->is_system_wide) { + $this->authorize('makeSystemWide', GithubApp::class); + } + } + public function submit() { try { $this->authorize('update', $this->github_app); + $this->authorizeSystemWideChange(); $this->organization = normalizeGithubOrganization($this->organization); $this->apiUrl = filled($this->apiUrl) @@ -455,6 +463,7 @@ class Change extends Component { try { $this->authorize('update', $this->github_app); + $this->authorizeSystemWideChange(); $this->validateOnly('privateKeyId'); diff --git a/app/Livewire/Source/Github/Create.php b/app/Livewire/Source/Github/Create.php index 88eb296e5d..329528aaa4 100644 --- a/app/Livewire/Source/Github/Create.php +++ b/app/Livewire/Source/Github/Create.php @@ -83,6 +83,9 @@ class Create extends Component 'is_system_wide' => $this->is_system_wide, 'team_id' => currentTeam()->id, ]; + if ($this->is_system_wide) { + $this->authorize('makeSystemWide', GithubApp::class); + } $github_app = GithubApp::create($payload); if (session('from')) { session(['from' => session('from') + ['source_id' => $github_app->id]]); diff --git a/app/Livewire/Source/Gitlab/Change.php b/app/Livewire/Source/Gitlab/Change.php index 128374efc1..67bcea322b 100644 --- a/app/Livewire/Source/Gitlab/Change.php +++ b/app/Livewire/Source/Gitlab/Change.php @@ -264,10 +264,18 @@ class Change extends Component } } + private function authorizeSystemWideChange(): void + { + if ($this->isSystemWide && ! $this->gitlab_app->is_system_wide) { + $this->authorize('makeSystemWide', GitlabApp::class); + } + } + public function submit() { try { $this->authorize('update', $this->gitlab_app); + $this->authorizeSystemWideChange(); $this->validate(); @@ -283,6 +291,7 @@ class Change extends Component { try { $this->authorize('update', $this->gitlab_app); + $this->authorizeSystemWideChange(); $this->validateOnly('isSystemWide'); diff --git a/app/Livewire/Source/Gitlab/Create.php b/app/Livewire/Source/Gitlab/Create.php index 2846d20b1b..5156047faf 100644 --- a/app/Livewire/Source/Gitlab/Create.php +++ b/app/Livewire/Source/Gitlab/Create.php @@ -67,6 +67,10 @@ class Create extends Component 'group_name' => 'nullable|string', ]); + if ($this->is_system_wide) { + $this->authorize('makeSystemWide', GitlabApp::class); + } + $gitlab_app = GitlabApp::create([ 'name' => $this->name, 'api_url' => $this->api_url, diff --git a/app/Policies/GithubAppPolicy.php b/app/Policies/GithubAppPolicy.php index d152ab1d1a..0558e716b7 100644 --- a/app/Policies/GithubAppPolicy.php +++ b/app/Policies/GithubAppPolicy.php @@ -35,6 +35,14 @@ class GithubAppPolicy return $user->isAdmin(); } + /** + * Determine whether the user can make a Github App available to every team on the instance. + */ + public function makeSystemWide(User $user): bool + { + return $user->canAccessSystemResources(); + } + /** * Determine whether the user can update the model. */ diff --git a/app/Policies/GitlabAppPolicy.php b/app/Policies/GitlabAppPolicy.php index 8a9d773d6d..e7e7620b62 100644 --- a/app/Policies/GitlabAppPolicy.php +++ b/app/Policies/GitlabAppPolicy.php @@ -35,6 +35,14 @@ class GitlabAppPolicy return $user->isAdmin(); } + /** + * Determine whether the user can make a Gitlab App available to every team on the instance. + */ + public function makeSystemWide(User $user): bool + { + return $user->canAccessSystemResources(); + } + /** * Determine whether the user can update the model. */ diff --git a/resources/views/livewire/source/github/change.blade.php b/resources/views/livewire/source/github/change.blade.php index 252a2d4406..9567a3f7fa 100644 --- a/resources/views/livewire/source/github/change.blade.php +++ b/resources/views/livewire/source/github/change.blade.php @@ -138,6 +138,7 @@ label="Organization" placeholder="Personal account when empty" /> @if (!isCloud()) + @can('makeSystemWide', \App\Models\GithubApp::class)
+ @endcan @if ($isSystemWide)
diff --git a/resources/views/livewire/source/github/create.blade.php b/resources/views/livewire/source/github/create.blade.php index d60517d4c7..05f9ff5d4e 100644 --- a/resources/views/livewire/source/github/create.blade.php +++ b/resources/views/livewire/source/github/create.blade.php @@ -13,7 +13,7 @@ :placeholder="$use_for_github_runners ? 'GitHub organization, for example coollabsio' : 'Personal account when empty'" />
- @if (! isCloud()) + @if (! isCloud() && auth()->user()->can('makeSystemWide', \App\Models\GithubApp::class))
@if (! isCloud()) -
- -
+ @can('makeSystemWide', \App\Models\GitlabApp::class) +
+ +
+ @endcan @if ($isSystemWide)
@@ -283,7 +285,7 @@ helper="Usually your GitLab URL with /api/v4 appended." /> - @if (! isCloud()) + @if (! isCloud() && auth()->user()->can('makeSystemWide', \App\Models\GitlabApp::class))
diff --git a/resources/views/livewire/source/gitlab/create.blade.php b/resources/views/livewire/source/gitlab/create.blade.php index bac16db248..2a1482cc9c 100644 --- a/resources/views/livewire/source/gitlab/create.blade.php +++ b/resources/views/livewire/source/gitlab/create.blade.php @@ -11,7 +11,7 @@ placeholder="All accessible repositories when empty" />
- @if (! isCloud()) + @if (! isCloud() && auth()->user()->can('makeSystemWide', \App\Models\GitlabApp::class))
team_id)->toBe($this->team->id); }); - test('creates github app with system wide enabled', function () { + test('instance admin creates github app with system wide enabled', function () { + Team::factory()->create(['id' => 0])->members()->attach($this->user->id, ['role' => 'owner']); + Livewire::test(Create::class) ->assertSuccessful() ->set('name', 'system-wide-app') diff --git a/tests/Feature/GitlabAppAuthorizationTest.php b/tests/Feature/GitlabAppAuthorizationTest.php index 4e1f2c2df1..5b96dde0a7 100644 --- a/tests/Feature/GitlabAppAuthorizationTest.php +++ b/tests/Feature/GitlabAppAuthorizationTest.php @@ -104,7 +104,8 @@ describe('GitLab App authorization', function () { expect($this->gitlabApp->refresh()->is_system_wide)->toBeFalse(); }); - test('team owner can update a gitlab app via instantSave', function () { + test('instance admin team owner can update a gitlab app via instantSave', function () { + Team::factory()->create(['id' => 0])->members()->attach($this->owner->id, ['role' => 'owner']); $this->actingAs($this->owner); session(['currentTeam' => $this->team]); @@ -154,6 +155,7 @@ describe('GitLab App authorization', function () { }); test('instantSave rejects unsafe GitLab URLs', function (string $url) { + Team::factory()->create(['id' => 0])->members()->attach($this->owner->id, ['role' => 'owner']); $this->actingAs($this->owner); session(['currentTeam' => $this->team]); diff --git a/tests/Feature/SystemWideGitSourceAuthorizationTest.php b/tests/Feature/SystemWideGitSourceAuthorizationTest.php new file mode 100644 index 0000000000..28d851e438 --- /dev/null +++ b/tests/Feature/SystemWideGitSourceAuthorizationTest.php @@ -0,0 +1,310 @@ + 2048, + 'private_key_type' => OPENSSL_KEYTYPE_RSA, + ]); + + openssl_pkey_export($key, $privateKey); + + return $privateKey; +} + +/** + * @return array{team: Team, user: User, token: string, privateKey: PrivateKey} + */ +function systemWideSourceTestTeam(array $teamAttributes = []): array +{ + $team = Team::factory()->create($teamAttributes); + $user = User::factory()->create(); + $team->members()->attach($user->id, ['role' => 'admin']); + session(['currentTeam' => $team]); + $token = $user->createToken('test-token', ['*'])->plainTextToken; + + $privateKey = PrivateKey::create([ + 'name' => 'Test Key', + 'private_key' => systemWideSourceTestPrivateKey(), + 'team_id' => $team->id, + ]); + + return compact('team', 'user', 'token', 'privateKey'); +} + +function systemWideSourceGithubPayload(PrivateKey $privateKey, array $overrides = []): array +{ + return array_merge([ + 'name' => 'Team GitHub App', + 'html_url' => 'https://github.com', + 'app_id' => 12345, + 'installation_id' => 67890, + 'client_id' => 'client-id', + 'client_secret' => 'client-secret', + 'webhook_secret' => 'webhook-secret', + 'private_key_uuid' => $privateKey->uuid, + ], $overrides); +} + +function systemWideSourceGithubApp(Team $team, PrivateKey $privateKey, bool $systemWide = false): GithubApp +{ + return GithubApp::create([ + 'name' => 'Existing GitHub App', + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'custom_user' => 'git', + 'custom_port' => 22, + 'private_key_id' => $privateKey->id, + 'team_id' => $team->id, + 'is_system_wide' => $systemWide, + 'is_public' => false, + ]); +} + +function systemWideSourceGitlabApp(Team $team, bool $systemWide = false): GitlabApp +{ + return GitlabApp::create([ + 'name' => 'Existing GitLab App', + 'api_url' => 'https://gitlab.com/api/v4', + 'html_url' => 'https://gitlab.com', + 'custom_user' => 'git', + 'custom_port' => 22, + 'team_id' => $team->id, + 'is_system_wide' => $systemWide, + 'is_public' => false, + ]); +} + +beforeEach(function () { + config()->set('app.maintenance.driver', 'file'); + config()->set('cache.default', 'array'); + + InstanceSettings::forceCreate(['id' => 0, 'is_api_enabled' => true]); +}); + +describe('normal team admin', function () { + beforeEach(function () { + // Root team exists, but this admin is not a member of it. + Team::factory()->create(['id' => 0]); + [ + 'team' => $this->team, + 'user' => $this->user, + 'token' => $this->token, + 'privateKey' => $this->privateKey, + ] = systemWideSourceTestTeam(); + }); + + test('cannot create a system-wide github app via api', function () { + $this->withToken($this->token) + ->postJson('/api/v1/github-apps', systemWideSourceGithubPayload($this->privateKey, ['is_system_wide' => true])) + ->assertForbidden(); + + expect(GithubApp::count())->toBe(0); + }); + + test('cannot create a system-wide gitlab app via api', function () { + $this->withToken($this->token) + ->postJson('/api/v1/gitlab-apps', [ + 'name' => 'Team GitLab App', + 'html_url' => 'https://gitlab.com', + 'is_system_wide' => true, + ]) + ->assertForbidden(); + + expect(GitlabApp::count())->toBe(0); + }); + + test('can still create team-only sources via api', function () { + $this->withToken($this->token) + ->postJson('/api/v1/github-apps', systemWideSourceGithubPayload($this->privateKey, ['is_system_wide' => false])) + ->assertCreated(); + $this->withToken($this->token) + ->postJson('/api/v1/gitlab-apps', ['name' => 'Team GitLab App', 'html_url' => 'https://gitlab.com']) + ->assertCreated(); + + expect(GithubApp::sole()->is_system_wide)->toBeFalse() + ->and(GitlabApp::sole()->is_system_wide)->toBeFalse(); + }); + + test('cannot make an own github app system-wide via api', function () { + $githubApp = systemWideSourceGithubApp($this->team, $this->privateKey); + + $this->withToken($this->token) + ->patchJson("/api/v1/github-apps/{$githubApp->id}", ['is_system_wide' => true]) + ->assertForbidden(); + + expect($githubApp->refresh()->is_system_wide)->toBeFalse(); + }); + + test('cannot make an own gitlab app system-wide via api', function () { + $gitlabApp = systemWideSourceGitlabApp($this->team); + + $this->withToken($this->token) + ->patchJson("/api/v1/gitlab-apps/{$gitlabApp->id}", ['is_system_wide' => true]) + ->assertForbidden(); + + expect($gitlabApp->refresh()->is_system_wide)->toBeFalse(); + }); + + test('can still update team-only sources via api', function () { + $githubApp = systemWideSourceGithubApp($this->team, $this->privateKey); + $gitlabApp = systemWideSourceGitlabApp($this->team); + + $this->withToken($this->token) + ->patchJson("/api/v1/github-apps/{$githubApp->id}", ['name' => 'Renamed', 'is_system_wide' => false]) + ->assertSuccessful(); + $this->withToken($this->token) + ->patchJson("/api/v1/gitlab-apps/{$gitlabApp->id}", ['name' => 'Renamed', 'is_system_wide' => false]) + ->assertSuccessful(); + + expect($githubApp->refresh()->name)->toBe('Renamed') + ->and($githubApp->is_system_wide)->toBeFalse() + ->and($gitlabApp->refresh()->name)->toBe('Renamed') + ->and($gitlabApp->is_system_wide)->toBeFalse(); + }); + + test('cannot create system-wide sources via livewire', function () { + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + Livewire::test(GithubCreate::class) + ->set('name', 'shared-github') + ->set('is_system_wide', true) + ->call('createGitHubApp') + ->assertDispatched('error'); + Livewire::test(GitlabCreate::class) + ->set('name', 'shared-gitlab') + ->set('is_system_wide', true) + ->call('createGitLabApp') + ->assertDispatched('error'); + + expect(GithubApp::count())->toBe(0) + ->and(GitlabApp::count())->toBe(0); + }); + + test('cannot make a github app system-wide via livewire', function (string $method) { + $githubApp = systemWideSourceGithubApp($this->team, $this->privateKey); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + Livewire::withQueryParams(['github_app_uuid' => $githubApp->uuid]) + ->test(GithubChange::class) + ->set('isSystemWide', true) + ->call($method) + ->assertDispatched('error'); + + expect($githubApp->refresh()->is_system_wide)->toBeFalse(); + })->with(['submit', 'instantSave']); + + test('cannot make a gitlab app system-wide via livewire', function (string $method) { + $gitlabApp = systemWideSourceGitlabApp($this->team); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + Livewire::withQueryParams(['gitlab_app_uuid' => $gitlabApp->uuid]) + ->test(GitlabChange::class) + ->set('isSystemWide', true) + ->call($method) + ->assertDispatched('error'); + + expect($gitlabApp->refresh()->is_system_wide)->toBeFalse(); + })->with(['submit', 'instantSave']); + + test('does not show the system-wide toggle', function () { + $gitlabApp = systemWideSourceGitlabApp($this->team); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + Livewire::test(GithubCreate::class)->assertDontSee('System wide'); + Livewire::test(GitlabCreate::class)->assertDontSee('System wide'); + Livewire::withQueryParams(['gitlab_app_uuid' => $gitlabApp->uuid]) + ->test(GitlabChange::class) + ->assertDontSee('System wide'); + }); +}); + +describe('root team admin', function () { + beforeEach(function () { + [ + 'team' => $this->team, + 'user' => $this->user, + 'token' => $this->token, + 'privateKey' => $this->privateKey, + ] = systemWideSourceTestTeam(['id' => 0]); + }); + + test('can create system-wide sources via api', function () { + $this->withToken($this->token) + ->postJson('/api/v1/github-apps', systemWideSourceGithubPayload($this->privateKey, ['is_system_wide' => true])) + ->assertCreated(); + $this->withToken($this->token) + ->postJson('/api/v1/gitlab-apps', [ + 'name' => 'Shared GitLab App', + 'html_url' => 'https://gitlab.com', + 'is_system_wide' => true, + ]) + ->assertCreated(); + + expect(GithubApp::sole()->is_system_wide)->toBeTrue() + ->and(GitlabApp::sole()->is_system_wide)->toBeTrue(); + }); + + test('can make sources system-wide via api', function () { + $githubApp = systemWideSourceGithubApp($this->team, $this->privateKey); + $gitlabApp = systemWideSourceGitlabApp($this->team); + + $this->withToken($this->token) + ->patchJson("/api/v1/github-apps/{$githubApp->id}", ['is_system_wide' => true]) + ->assertSuccessful(); + $this->withToken($this->token) + ->patchJson("/api/v1/gitlab-apps/{$gitlabApp->id}", ['is_system_wide' => true]) + ->assertSuccessful(); + + expect($githubApp->refresh()->is_system_wide)->toBeTrue() + ->and($gitlabApp->refresh()->is_system_wide)->toBeTrue(); + }); + + test('can make a gitlab app system-wide via livewire', function () { + $gitlabApp = systemWideSourceGitlabApp($this->team); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + Livewire::withQueryParams(['gitlab_app_uuid' => $gitlabApp->uuid]) + ->test(GitlabChange::class) + ->assertSee('System wide') + ->set('isSystemWide', true) + ->call('instantSave') + ->assertDispatched('success'); + + expect($gitlabApp->refresh()->is_system_wide)->toBeTrue(); + }); + + test('can create a system-wide github app via livewire', function () { + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + Livewire::test(GithubCreate::class) + ->assertSee('System wide') + ->set('name', 'shared-github') + ->set('is_system_wide', true) + ->call('createGitHubApp') + ->assertRedirect(); + + expect(GithubApp::sole()->is_system_wide)->toBeTrue(); + }); +});