From 26dab849fb7acf495bd0b957bd6910c8eead2cb0 Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Sat, 26 Sep 2026 15:31:52 +0100 Subject: [PATCH] Fix GitHub App setup race and tidy integrations cards GitHub sends the installation webhook at the same moment it redirects the user to the setup URL. The webhook could save the installation while the setup request was still checking it with GitHub, so the setup's create() hit the unique key on installation_id. Setup now uses createOrFirst() and checks who owns the installation afterwards. The cards on the integrations page now share the Flux card radius and none has a shadow. The App Installations card is a Flux card like the GitHub Account card above it. Co-Authored-By: Claude Opus 5.5 --- .../GitHubIntegrationController.php | 22 ++++--- .../github-migration-banner.blade.php | 2 +- .../claude-plugins-access-banner.blade.php | 2 +- .../livewire/discord-access-banner.blade.php | 2 +- .../livewire/git-hub-access-banner.blade.php | 2 +- .../livewire/git-hub-app-status.blade.php | 23 +++----- tests/Feature/GitHubAppSetupTest.php | 59 +++++++++++++++++++ 7 files changed, 84 insertions(+), 28 deletions(-) diff --git a/app/Http/Controllers/GitHubIntegrationController.php b/app/Http/Controllers/GitHubIntegrationController.php index 7891c5fee..205c2e58c 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 d41279bb9..29283a3c1 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 948239c2c..43435d9bf 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 58ec9f8cc..d3e9016fa 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 159fe0554..2793f1d01 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 161bf1fbf..37ec3182d 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 bcd8540bb..5df77ba01 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']);