diff --git a/app/Http/Controllers/GitHubIntegrationController.php b/app/Http/Controllers/GitHubIntegrationController.php index 7891c5fe..205c2e58 100644 --- a/app/Http/Controllers/GitHubIntegrationController.php +++ b/app/Http/Controllers/GitHubIntegrationController.php @@ -200,6 +200,10 @@ protected function redirectAfterLogin(User $user, GitHubAuthType $authType, stri ->with('success', $message); } + /** + * GitHub redirects here after the app is installed and sends the installation webhook at the same + * time, so the webhook can record the installation while this request is still checking it. + */ public function handleSetup(Request $request): RedirectResponse { $installationId = (int) $request->query('installation_id'); @@ -213,21 +217,21 @@ public function handleSetup(Request $request): RedirectResponse $appService = app(GitHubAppService::class); $installation = GitHubInstallation::where('installation_id', $installationId)->first(); - if ($installation && $installation->user_id !== $user->id) { - return to_route('customer.integrations') - ->with('error', 'That GitHub App installation is already linked to another NativePHP account.'); - } - if (! $installation) { if (! $user->isUsingGitHubApp() || ! $appService->userCanAccessInstallation($user, $installationId)) { return to_route('customer.integrations') ->with('error', "We couldn't confirm that GitHub App installation belongs to your GitHub account. Please connect GitHub and try again."); } - $installation = $user->githubInstallations()->create([ - 'installation_id' => $installationId, - 'account_login' => $user->github_username ?? 'unknown', - ]); + $installation = GitHubInstallation::createOrFirst( + ['installation_id' => $installationId], + ['user_id' => $user->id, 'account_login' => $user->github_username ?? 'unknown'], + ); + } + + if ($installation->user_id !== $user->id) { + return to_route('customer.integrations') + ->with('error', 'That GitHub App installation is already linked to another NativePHP account.'); } if (! $appService->syncInstallation($installation) && ! $installation->exists) { diff --git a/resources/views/components/github-migration-banner.blade.php b/resources/views/components/github-migration-banner.blade.php index d41279bb..29283a3c 100644 --- a/resources/views/components/github-migration-banner.blade.php +++ b/resources/views/components/github-migration-banner.blade.php @@ -21,7 +21,7 @@ @if($needsMigration || $missingAccess->isNotEmpty())
$urgent, 'border-blue-200 bg-blue-50 dark:border-blue-900/50 dark:bg-blue-900/20' => ! $urgent, ])> diff --git a/resources/views/livewire/claude-plugins-access-banner.blade.php b/resources/views/livewire/claude-plugins-access-banner.blade.php index 948239c2..43435d9b 100644 --- a/resources/views/livewire/claude-plugins-access-banner.blade.php +++ b/resources/views/livewire/claude-plugins-access-banner.blade.php @@ -7,7 +7,7 @@ @if($hasLicense)
!$inline])> -
+
diff --git a/resources/views/livewire/discord-access-banner.blade.php b/resources/views/livewire/discord-access-banner.blade.php index 58ec9f8c..d3e9016f 100644 --- a/resources/views/livewire/discord-access-banner.blade.php +++ b/resources/views/livewire/discord-access-banner.blade.php @@ -1,6 +1,6 @@
!$inline])> -
+
diff --git a/resources/views/livewire/git-hub-access-banner.blade.php b/resources/views/livewire/git-hub-access-banner.blade.php index 159fe055..2793f1d0 100644 --- a/resources/views/livewire/git-hub-access-banner.blade.php +++ b/resources/views/livewire/git-hub-access-banner.blade.php @@ -1,7 +1,7 @@
@if(auth()->user()->hasMobileRepoAccess())
!$inline])> -
+
diff --git a/resources/views/livewire/git-hub-app-status.blade.php b/resources/views/livewire/git-hub-app-status.blade.php index 161bf1fb..37ec3182 100644 --- a/resources/views/livewire/git-hub-app-status.blade.php +++ b/resources/views/livewire/git-hub-app-status.blade.php @@ -1,24 +1,17 @@
-
-
+ +
-

GitHub App Installations

-

- Manage which accounts and repositories the NativePHP app can access. -

+ GitHub App Installations + Manage which accounts and repositories the NativePHP app can access.
@if($installUrl) - - Add Account - - - - + Add Account @endif
@if($installations->isEmpty()) -
+

No GitHub App installations found. Install the app on your GitHub account to grant repository access.

@@ -34,7 +27,7 @@ @else
@foreach($installations as $installation) -
+
@if($installation->account_type === 'Organization') @@ -91,5 +84,5 @@
@endif -
+
diff --git a/tests/Feature/GitHubAppSetupTest.php b/tests/Feature/GitHubAppSetupTest.php index bcd8540b..5df77ba0 100644 --- a/tests/Feature/GitHubAppSetupTest.php +++ b/tests/Feature/GitHubAppSetupTest.php @@ -51,6 +51,30 @@ private function fakeGitHub(array $userInstallationIds, string $selection = 'sel ]); } + /** + * Fake GitHub so the installation webhook records installation 555 for the given user while + * the setup request is still asking GitHub whether the installation is theirs. + */ + private function fakeGitHubWhileTheWebhookRecordsTheInstallationFor(User $webhookUser): void + { + Http::fake([ + 'api.github.com/user/installations*' => function () use ($webhookUser) { + GitHubInstallation::factory()->create([ + 'user_id' => $webhookUser->id, + 'installation_id' => 555, + ]); + + return Http::response(['installations' => [['id' => 555]]]); + }, + 'api.github.com/app/installations/555' => Http::response([ + 'id' => 555, + 'account' => ['login' => 'acme', 'type' => 'Organization', 'id' => 99], + 'repository_selection' => 'all', + 'suspended_at' => null, + ]), + ]); + } + public function test_setup_records_an_installation_the_user_can_see_on_github(): void { $this->fakeGitHub([555], 'selected', ['acme/one', 'acme/two']); @@ -153,6 +177,41 @@ public function test_setup_refreshes_an_installation_the_webhook_already_recorde $this->assertSame(['acme/new-repo'], $installation->fresh()->repository_selection); } + public function test_setup_uses_the_installation_the_webhook_records_while_it_checks_github(): void + { + $user = User::factory()->withGitHubApp()->create(); + $this->fakeGitHubWhileTheWebhookRecordsTheInstallationFor($user); + + $this->actingAs($user) + ->get(route('github.setup', ['installation_id' => 555])) + ->assertRedirect(route('customer.integrations')) + ->assertSessionHas('success'); + + $installation = $user->githubInstallations()->sole(); + + $this->assertSame('acme', $installation->account_login); + $this->assertSame('Organization', $installation->account_type); + } + + public function test_setup_will_not_hand_over_an_installation_the_webhook_links_to_someone_else_while_it_checks_github(): void + { + $owner = User::factory()->withGitHubApp()->create(); + $this->fakeGitHubWhileTheWebhookRecordsTheInstallationFor($owner); + + $otherUser = User::factory()->withGitHubApp()->create(); + + $this->actingAs($otherUser) + ->get(route('github.setup', ['installation_id' => 555])) + ->assertRedirect(route('customer.integrations')) + ->assertSessionHas('error', 'That GitHub App installation is already linked to another NativePHP account.'); + + $this->assertDatabaseCount('github_installations', 1); + $this->assertDatabaseHas('github_installations', [ + 'installation_id' => 555, + 'user_id' => $owner->id, + ]); + } + public function test_sync_command_updates_installations_and_removes_ones_github_no_longer_has(): void { $this->fakeGitHub([], 'selected', ['acme/one']);