diff --git a/app/Http/Controllers/Webhook/Github.php b/app/Http/Controllers/Webhook/Github.php
index 009b11f299..ba270cf688 100644
--- a/app/Http/Controllers/Webhook/Github.php
+++ b/app/Http/Controllers/Webhook/Github.php
@@ -26,6 +26,7 @@ use Illuminate\Http\Request;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Http;
+use Illuminate\Support\Facades\Log;
use Illuminate\Support\Str;
class Github extends Controller
@@ -747,11 +748,21 @@ class Github extends Controller
action: 'install',
);
- abort_unless(
- $this->githubInstallationBelongsToApp($github_app, $installation_id),
- 403,
- 'GitHub App installation could not be verified.'
- );
+ $verification_error = $this->githubInstallationVerificationError($github_app, $installation_id);
+ if ($verification_error) {
+ Log::warning('GitHub App installation could not be verified.', [
+ 'github_app_id' => $github_app->id,
+ 'installation_id' => $installation_id,
+ 'reason' => $verification_error,
+ ]);
+
+ $message = "GitHub App installation could not be verified: {$verification_error}";
+ abort_if($request->expectsJson(), 403, $message);
+
+ return redirect()
+ ->route('source.github.show', ['github_app_uuid' => $github_app->uuid])
+ ->with('error', $message);
+ }
$github_app->installation_id = $installation_id;
$github_app->save();
@@ -778,11 +789,13 @@ class Github extends Controller
* The installation id arrives as an untrusted query parameter on an
* unauthenticated-reachable GET callback, so it must be confirmed against
* the GitHub API using the App's own credentials before it is persisted.
+ *
+ * @return string|null The reason the verification failed, or null when it passed.
*/
- private function githubInstallationBelongsToApp(GithubApp $github_app, string $installation_id): bool
+ private function githubInstallationVerificationError(GithubApp $github_app, string $installation_id): ?string
{
- if (blank($github_app->app_id) || blank($github_app->privateKey?->private_key)) {
- return false;
+ if (blank($github_app->app_id) || blank(githubAppPrivateKey($github_app)?->private_key)) {
+ return 'The GitHub App ID or private key is missing.';
}
try {
@@ -794,12 +807,19 @@ class Github extends Controller
->timeout(10)
->connectTimeout(5)
->get("{$github_app->api_url}/app/installations/{$installation_id}");
-
- return $response->successful()
- && (string) data_get($response->json(), 'app_id') === (string) $github_app->app_id;
- } catch (\Throwable) {
- return false;
+ } catch (\Throwable $e) {
+ return strip_tags(str_replace('
', ' ', $e->getMessage()));
}
+
+ if (! $response->successful()) {
+ return "GitHub API returned HTTP {$response->status()}: ".data_get($response->json(), 'message', 'no error message found');
+ }
+
+ if ((string) data_get($response->json(), 'app_id') !== (string) $github_app->app_id) {
+ return 'The installation belongs to a different GitHub App.';
+ }
+
+ return null;
}
private function consumeGithubAppSetupState(Request $request, string $state, string $action): GithubApp
@@ -813,12 +833,20 @@ class Github extends Controller
$this->rejectInvalidGithubAppSetupState($request);
}
- $team_id = $request->user()?->currentTeam()?->id;
- abort_unless(! is_null($team_id) && (int) data_get($payload, 'team_id') === $team_id, 403);
+ // The state is bound to the team that started the setup. The user may have switched
+ // teams while on GitHub, so check membership of that team, not the session team.
+ $team_id = (int) data_get($payload, 'team_id');
+ abort_unless((bool) $request->user()?->teams->contains('id', $team_id), 403);
- return GithubApp::whereKey(data_get($payload, 'github_app_id'))
- ->where('team_id', data_get($payload, 'team_id'))
+ $github_app = GithubApp::whereKey(data_get($payload, 'github_app_id'))
+ ->where('team_id', $team_id)
->firstOrFail();
+
+ if ($request->user()->currentTeam()?->id !== $team_id) {
+ refreshSession($github_app->team);
+ }
+
+ return $github_app;
}
private function rejectInvalidGithubAppSetupState(Request $request): never
diff --git a/resources/views/livewire/source/github/change.blade.php b/resources/views/livewire/source/github/change.blade.php
index dcebcfac14..252a2d4406 100644
--- a/resources/views/livewire/source/github/change.blade.php
+++ b/resources/views/livewire/source/github/change.blade.php
@@ -3,6 +3,10 @@
{{ $github_app->name ?: 'GitHub App' }} | Sources | Coolify
+ @if (session('error'))
+
+ @endif
+
@if (data_get($github_app, 'app_id'))
@php
$githubAppRouteParameters = ['github_app_uuid' => $github_app->uuid];
diff --git a/tests/Feature/Security/GithubAppSetupCallbackTest.php b/tests/Feature/Security/GithubAppSetupCallbackTest.php
index 69671f2c24..93ba0e533f 100644
--- a/tests/Feature/Security/GithubAppSetupCallbackTest.php
+++ b/tests/Feature/Security/GithubAppSetupCallbackTest.php
@@ -400,3 +400,54 @@ it('allows reinstalling an already configured github app installation id', funct
$this->githubApp->refresh();
expect($this->githubApp->installation_id)->toBe(222222);
});
+
+it('redirects to the github app with the reason when github does not confirm the installation', function () {
+ authenticateGithubSetupCallbackTest($this);
+ configureGithubAppCredentials($this->githubApp);
+ fakeGithubInstallationVerificationFailure();
+ cacheGithubAppSetupState('valid-install-state', 'install', $this->githubApp);
+
+ $this->get('/webhooks/source/github/install?state=valid-install-state&setup_action=install&installation_id=999999')
+ ->assertRedirect(route('source.github.show', ['github_app_uuid' => $this->githubApp->uuid]))
+ ->assertSessionHas('error', fn (string $error) => str_contains($error, 'HTTP 404: Not Found'));
+
+ $this->githubApp->refresh();
+ expect($this->githubApp->installation_id)->toBeNull();
+});
+
+it('redirects to the github app with the reason when the server clock is out of sync with github', function () {
+ authenticateGithubSetupCallbackTest($this);
+ configureGithubAppCredentials($this->githubApp);
+ Http::preventStrayRequests();
+ Http::fake([
+ 'https://api.github.com/zen' => Http::response('Keep it logically awesome.', 200, [
+ 'Date' => now()->addMinutes(5)->toRfc7231String(),
+ ]),
+ ]);
+ cacheGithubAppSetupState('valid-install-state', 'install', $this->githubApp);
+
+ $this->get('/webhooks/source/github/install?state=valid-install-state&setup_action=install&installation_id=123456')
+ ->assertRedirect(route('source.github.show', ['github_app_uuid' => $this->githubApp->uuid]))
+ ->assertSessionHas('error', fn (string $error) => str_contains($error, 'System time is out of sync with GitHub API time'));
+
+ $this->githubApp->refresh();
+ expect($this->githubApp->installation_id)->toBeNull();
+});
+
+it('completes the github app installation after the user switched to another of their teams', function () {
+ authenticateGithubSetupCallbackTest($this);
+ configureGithubAppCredentials($this->githubApp);
+ fakeGithubInstallationVerification($this->githubApp->app_id);
+ cacheGithubAppSetupState('valid-install-state', 'install', $this->githubApp);
+
+ $otherTeam = Team::factory()->create();
+ $otherTeam->members()->attach($this->user->id, ['role' => 'owner']);
+ session(['currentTeam' => $otherTeam]);
+
+ $this->get('/webhooks/source/github/install?state=valid-install-state&setup_action=install&installation_id=123456')
+ ->assertRedirect(route('source.github.show', ['github_app_uuid' => $this->githubApp->uuid]));
+
+ $this->githubApp->refresh();
+ expect($this->githubApp->installation_id)->toBe(123456)
+ ->and(data_get(session('currentTeam'), 'id'))->toBe($this->team->id);
+});