From 25d424c743d5134d4a005a6d8f754bb3235b632c Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 26 Mar 2026 14:30:27 +0100 Subject: [PATCH] refactor: split invitation endpoint into GET (show) and POST (accept) Refactor the invitation acceptance flow to use a landing page pattern: - GET shows invitation details (team name, role, confirmation button) - POST processes the acceptance with proper form submission - Remove unused revoke GET route (handled by Livewire component) - Add Blade view for the invitation landing page - Add feature tests for the new invitation flow Co-Authored-By: Claude Opus 4.6 --- app/Http/Controllers/Controller.php | 62 ++++---- resources/views/invitation/accept.blade.php | 43 +++++ routes/web.php | 9 +- .../TeamInvitationCsrfProtectionTest.php | 147 ++++++++++++++++++ 4 files changed, 226 insertions(+), 35 deletions(-) create mode 100644 resources/views/invitation/accept.blade.php create mode 100644 tests/Feature/TeamInvitationCsrfProtectionTest.php diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 09007ad96..17d14296b 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -108,9 +108,31 @@ public function link() return redirect()->route('login')->with('error', 'Invalid credentials.'); } + public function showInvitation() + { + $invitationUuid = request()->route('uuid'); + $invitation = TeamInvitation::whereUuid($invitationUuid)->firstOrFail(); + $user = User::whereEmail($invitation->email)->firstOrFail(); + + if (Auth::id() !== $user->id) { + abort(400, 'You are not allowed to accept this invitation.'); + } + + if (! $invitation->isValid()) { + abort(400, 'Invitation expired.'); + } + + $alreadyMember = $user->teams()->where('team_id', $invitation->team->id)->exists(); + + return view('invitation.accept', [ + 'invitation' => $invitation, + 'team' => $invitation->team, + 'alreadyMember' => $alreadyMember, + ]); + } + public function acceptInvitation() { - $resetPassword = request()->query('reset-password'); $invitationUuid = request()->route('uuid'); $invitation = TeamInvitation::whereUuid($invitationUuid)->firstOrFail(); @@ -119,43 +141,21 @@ public function acceptInvitation() if (Auth::id() !== $user->id) { abort(400, 'You are not allowed to accept this invitation.'); } - $invitationValid = $invitation->isValid(); - if ($invitationValid) { - if ($resetPassword) { - $user->update([ - 'password' => Hash::make($invitationUuid), - 'force_password_reset' => true, - ]); - } - if ($user->teams()->where('team_id', $invitation->team->id)->exists()) { - $invitation->delete(); - - return redirect()->route('team.index'); - } - $user->teams()->attach($invitation->team->id, ['role' => $invitation->role]); - $invitation->delete(); - - refreshSession($invitation->team); - - return redirect()->route('team.index'); - } else { + if (! $invitation->isValid()) { abort(400, 'Invitation expired.'); } - } - public function revokeInvitation() - { - $invitation = TeamInvitation::whereUuid(request()->route('uuid'))->firstOrFail(); - $user = User::whereEmail($invitation->email)->firstOrFail(); - if (is_null(Auth::user())) { - return redirect()->route('login'); - } - if (Auth::id() !== $user->id) { - abort(401); + if ($user->teams()->where('team_id', $invitation->team->id)->exists()) { + $invitation->delete(); + + return redirect()->route('team.index'); } + $user->teams()->attach($invitation->team->id, ['role' => $invitation->role]); $invitation->delete(); + refreshSession($invitation->team); + return redirect()->route('team.index'); } } diff --git a/resources/views/invitation/accept.blade.php b/resources/views/invitation/accept.blade.php new file mode 100644 index 000000000..7e4773866 --- /dev/null +++ b/resources/views/invitation/accept.blade.php @@ -0,0 +1,43 @@ + +
+
+
+
+

+ Coolify +

+
+ +
+
+

Team Invitation

+ +

+ You have been invited to join: +

+

+ {{ $team->name }} +

+ +

+ Role: {{ ucfirst($invitation->role) }} +

+ + @if ($alreadyMember) +
+

You are already a member of this team.

+
+ @endif + +
+ @csrf + + {{ $alreadyMember ? 'Dismiss Invitation' : 'Accept Invitation' }} + +
+
+
+
+
+
+ diff --git a/routes/web.php b/routes/web.php index 4154fefab..dfb44324c 100644 --- a/routes/web.php +++ b/routes/web.php @@ -84,6 +84,7 @@ use App\Livewire\Team\Member\Index as TeamMemberIndex; use App\Livewire\Terminal\Index as TerminalIndex; use App\Models\ScheduledDatabaseBackupExecution; +use App\Models\ServiceDatabase; use App\Providers\RouteServiceProvider; use Illuminate\Support\Facades\Route; use Illuminate\Support\Facades\Storage; @@ -192,8 +193,8 @@ })->name('terminal.auth.ips')->middleware('can.access.terminal'); Route::prefix('invitations')->group(function () { - Route::get('/{uuid}', [Controller::class, 'acceptInvitation'])->name('team.invitation.accept'); - Route::get('/{uuid}/revoke', [Controller::class, 'revokeInvitation'])->name('team.invitation.revoke'); + Route::get('/{uuid}', [Controller::class, 'showInvitation'])->name('team.invitation.show'); + Route::post('/{uuid}', [Controller::class, 'acceptInvitation'])->name('team.invitation.accept'); }); Route::get('/projects', ProjectIndex::class)->name('project.index'); @@ -344,7 +345,7 @@ } } $filename = data_get($execution, 'filename'); - if ($execution->scheduledDatabaseBackup->database->getMorphClass() === \App\Models\ServiceDatabase::class) { + if ($execution->scheduledDatabaseBackup->database->getMorphClass() === ServiceDatabase::class) { $server = $execution->scheduledDatabaseBackup->database->service->destination->server; } else { $server = $execution->scheduledDatabaseBackup->database->destination->server; @@ -385,7 +386,7 @@ 'Content-Type' => 'application/octet-stream', 'Content-Disposition' => 'attachment; filename="'.basename($filename).'"', ]); - } catch (\Throwable $e) { + } catch (Throwable $e) { return response()->json(['message' => $e->getMessage()], 500); } })->name('download.backup'); diff --git a/tests/Feature/TeamInvitationCsrfProtectionTest.php b/tests/Feature/TeamInvitationCsrfProtectionTest.php new file mode 100644 index 000000000..1e911ed86 --- /dev/null +++ b/tests/Feature/TeamInvitationCsrfProtectionTest.php @@ -0,0 +1,147 @@ +team = Team::factory()->create(); + $this->user = User::factory()->create(['email' => 'invited@example.com']); + + $this->invitation = TeamInvitation::create([ + 'team_id' => $this->team->id, + 'uuid' => 'test-invitation-uuid', + 'email' => 'invited@example.com', + 'role' => 'member', + 'link' => url('/invitations/test-invitation-uuid'), + 'via' => 'link', + ]); +}); + +test('GET invitation shows landing page without accepting', function () { + $this->actingAs($this->user); + + $response = $this->get('/invitations/test-invitation-uuid'); + + $response->assertStatus(200); + $response->assertViewIs('invitation.accept'); + $response->assertSee($this->team->name); + $response->assertSee('Accept Invitation'); + + // Invitation should NOT be deleted (not accepted yet) + $this->assertDatabaseHas('team_invitations', [ + 'uuid' => 'test-invitation-uuid', + ]); + + // User should NOT be added to the team + expect($this->user->teams()->where('team_id', $this->team->id)->exists())->toBeFalse(); +}); + +test('GET invitation with reset-password query param does not reset password', function () { + $this->actingAs($this->user); + $originalPassword = $this->user->password; + + $response = $this->get('/invitations/test-invitation-uuid?reset-password=1'); + + $response->assertStatus(200); + + // Password should NOT be changed + $this->user->refresh(); + expect($this->user->password)->toBe($originalPassword); + + // Invitation should NOT be accepted + $this->assertDatabaseHas('team_invitations', [ + 'uuid' => 'test-invitation-uuid', + ]); +}); + +test('POST invitation accepts and adds user to team', function () { + $this->actingAs($this->user); + + $response = $this->post('/invitations/test-invitation-uuid'); + + $response->assertRedirect(route('team.index')); + + // Invitation should be deleted + $this->assertDatabaseMissing('team_invitations', [ + 'uuid' => 'test-invitation-uuid', + ]); + + // User should be added to the team + expect($this->user->teams()->where('team_id', $this->team->id)->exists())->toBeTrue(); +}); + +test('POST invitation without CSRF token is rejected', function () { + $this->actingAs($this->user); + + $response = $this->withoutMiddleware(EncryptCookies::class) + ->post('/invitations/test-invitation-uuid', [], [ + 'X-CSRF-TOKEN' => 'invalid-token', + ]); + + // Should be rejected with 419 (CSRF token mismatch) + $response->assertStatus(419); + + // Invitation should NOT be accepted + $this->assertDatabaseHas('team_invitations', [ + 'uuid' => 'test-invitation-uuid', + ]); +}); + +test('unauthenticated user cannot view invitation', function () { + $response = $this->get('/invitations/test-invitation-uuid'); + + $response->assertRedirect(); +}); + +test('wrong user cannot view invitation', function () { + $otherUser = User::factory()->create(['email' => 'other@example.com']); + $this->actingAs($otherUser); + + $response = $this->get('/invitations/test-invitation-uuid'); + + $response->assertStatus(400); +}); + +test('wrong user cannot accept invitation via POST', function () { + $otherUser = User::factory()->create(['email' => 'other@example.com']); + $this->actingAs($otherUser); + + $response = $this->post('/invitations/test-invitation-uuid'); + + $response->assertStatus(400); + + // Invitation should still exist + $this->assertDatabaseHas('team_invitations', [ + 'uuid' => 'test-invitation-uuid', + ]); +}); + +test('GET revoke route no longer exists', function () { + $this->actingAs($this->user); + + $response = $this->get('/invitations/test-invitation-uuid/revoke'); + + $response->assertStatus(404); +}); + +test('POST invitation for already-member user deletes invitation without duplicating', function () { + $this->user->teams()->attach($this->team->id, ['role' => 'member']); + $this->actingAs($this->user); + + $response = $this->post('/invitations/test-invitation-uuid'); + + $response->assertRedirect(route('team.index')); + + // Invitation should be deleted + $this->assertDatabaseMissing('team_invitations', [ + 'uuid' => 'test-invitation-uuid', + ]); + + // User should still have exactly one membership in this team + expect($this->user->teams()->where('team_id', $this->team->id)->count())->toBe(1); +});