From 0c80bf622d47f31d7f11b355e68573cb2cd5edbe Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 24 Sep 2026 07:10:32 +0200 Subject: [PATCH] Improve team invitation handling --- app/Livewire/Team/Invitations.php | 3 +- app/Livewire/Team/InviteLink.php | 2 +- app/Models/User.php | 2 +- tests/Feature/TeamInvitationHandlingTest.php | 170 +++++++++++++++++++ 4 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 tests/Feature/TeamInvitationHandlingTest.php diff --git a/app/Livewire/Team/Invitations.php b/app/Livewire/Team/Invitations.php index b66c49ac9e..8550afd9e3 100644 --- a/app/Livewire/Team/Invitations.php +++ b/app/Livewire/Team/Invitations.php @@ -25,12 +25,11 @@ class Invitations extends Component $invitationEmail = $invitation->email; $invitationUuid = $invitation->uuid; DB::transaction(function () use ($invitation): void { + $invitation->delete(); $user = User::whereEmail($invitation->email)->first(); if (filled($user)) { $user->deleteIfNotVerifiedAndForcePasswordReset(); } - - $invitation->delete(); }); auditLog('ui.team_invitation.revoked', [ 'team_id' => currentTeam()->id, diff --git a/app/Livewire/Team/InviteLink.php b/app/Livewire/Team/InviteLink.php index d6ea836075..61efff39a9 100644 --- a/app/Livewire/Team/InviteLink.php +++ b/app/Livewire/Team/InviteLink.php @@ -85,7 +85,7 @@ class InviteLink extends Component $token = Crypt::encryptString("{$user->email}@@@{$uuid}@@@{$password}"); $link = $this->invitationUrl('auth.link', ['token' => $token]); } - $invitation = TeamInvitation::whereEmail($this->email)->first(); + $invitation = TeamInvitation::ownedByCurrentTeam()->whereEmail($this->email)->first(); if (! is_null($invitation)) { $invitationValid = $invitation->isValid(); if ($invitationValid) { diff --git a/app/Models/User.php b/app/Models/User.php index 9f037bb917..4177eff973 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -212,7 +212,7 @@ class User extends Authenticatable implements SendsEmail */ public function deleteIfNotVerifiedAndForcePasswordReset() { - if ($this->hasVerifiedEmail() === false && $this->force_password_reset === true) { + if ($this->hasVerifiedEmail() === false && $this->force_password_reset === true && ! TeamInvitation::whereEmail($this->email)->exists()) { $this->delete(); } } diff --git a/tests/Feature/TeamInvitationHandlingTest.php b/tests/Feature/TeamInvitationHandlingTest.php new file mode 100644 index 0000000000..36ee59235d --- /dev/null +++ b/tests/Feature/TeamInvitationHandlingTest.php @@ -0,0 +1,170 @@ + InstanceSettings::query()->updateOrCreate(['id' => 0], [ + 'fqdn' => null, + 'smtp_enabled' => true, + 'smtp_from_address' => 'admin@example.com', + 'smtp_host' => 'coolify-mail', + 'smtp_port' => 1025, + ])); + Once::flush(); + Mail::fake(); + + $this->teamA = Team::factory()->create(); + $this->teamB = Team::factory()->create(); + $this->ownerA = User::factory()->create(); + $this->ownerB = User::factory()->create(); + $this->teamA->members()->attach($this->ownerA, ['role' => 'owner']); + $this->teamB->members()->attach($this->ownerB, ['role' => 'owner']); + + $this->pendingUser = User::factory()->create([ + 'email' => 'shared-invitee@example.com', + 'email_verified_at' => null, + 'force_password_reset' => true, + ]); + $this->invitationA = TeamInvitation::create([ + 'team_id' => $this->teamA->id, + 'uuid' => 'team-a-shared-invitation', + 'email' => $this->pendingUser->email, + 'role' => 'member', + 'link' => 'https://example.test/invite/team-a-shared-invitation', + 'via' => 'link', + ]); + + $this->actingAs($this->ownerB); + session(['currentTeam' => $this->teamB]); +}); + +test('allows separate teams to invite the same email', function (string $method, string $via) { + Livewire::test(InviteLink::class) + ->set('email', $this->pendingUser->email) + ->set('role', 'member') + ->call($method) + ->assertDispatched('success') + ->assertNotDispatched('error'); + + $this->assertDatabaseHas('team_invitations', ['id' => $this->invitationA->id, 'team_id' => $this->teamA->id]); + $this->assertDatabaseHas('team_invitations', ['team_id' => $this->teamB->id, 'email' => $this->pendingUser->email, 'via' => $via]); + $this->assertDatabaseHas('users', ['id' => $this->pendingUser->id]); +})->with(['via link' => ['viaLink', 'link'], 'via email' => ['viaEmail', 'email']]); + +test('keeps existing invitations and pending users when another team invites', function (string $method, string $via) { + $this->invitationA->forceFill(['created_at' => now()->subDays(10)])->save(); + + Livewire::test(InviteLink::class) + ->set('email', $this->pendingUser->email) + ->set('role', 'member') + ->call($method) + ->assertDispatched('success') + ->assertNotDispatched('error'); + + $this->assertDatabaseHas('team_invitations', ['id' => $this->invitationA->id, 'team_id' => $this->teamA->id]); + $this->assertDatabaseHas('team_invitations', ['team_id' => $this->teamB->id, 'email' => $this->pendingUser->email, 'via' => $via]); + $this->assertDatabaseHas('users', ['id' => $this->pendingUser->id]); +})->with(['via link' => ['viaLink', 'link'], 'via email' => ['viaEmail', 'email']]); + +test('keeps same-team duplicate handling', function (string $method) { + $this->actingAs($this->ownerA); + session(['currentTeam' => $this->teamA]); + + Livewire::test(InviteLink::class) + ->set('email', $this->pendingUser->email) + ->set('role', 'member') + ->call($method) + ->assertDispatched('error') + ->assertNotDispatched('success'); + + expect(TeamInvitation::whereEmail($this->pendingUser->email)->count())->toBe(1); + $this->assertDatabaseHas('team_invitations', ['id' => $this->invitationA->id]); +})->with(['viaLink', 'viaEmail']); + +test('shows and changes only current-team invitations', function () { + Livewire::test(Invitations::class, ['invitations' => TeamInvitation::ownedByCurrentTeam()->get()]) + ->assertDontSee($this->pendingUser->email) + ->call('deleteInvitation', $this->invitationA->id) + ->assertDispatched('error'); + + $this->assertDatabaseHas('team_invitations', ['id' => $this->invitationA->id]); + $this->assertDatabaseHas('users', ['id' => $this->pendingUser->id]); +}); + +test('requires invitation management access for both actions', function (string $method) { + $memberB = User::factory()->create(); + $this->teamB->members()->attach($memberB, ['role' => 'member']); + $this->actingAs($memberB); + session(['currentTeam' => $this->teamB]); + + Livewire::test(InviteLink::class) + ->set('email', $this->pendingUser->email) + ->set('role', 'member') + ->call($method) + ->assertDispatched('error') + ->assertNotDispatched('success'); + + $this->assertDatabaseHas('team_invitations', ['id' => $this->invitationA->id]); + $this->assertDatabaseMissing('team_invitations', ['team_id' => $this->teamB->id, 'email' => $this->pendingUser->email]); + $this->assertDatabaseHas('users', ['id' => $this->pendingUser->id]); +})->with(['viaLink', 'viaEmail']); + +test('keeps a pending user with another invitation after expiration', function () { + $invitationB = TeamInvitation::create([ + 'team_id' => $this->teamB->id, + 'uuid' => 'team-b-shared-invitation', + 'email' => $this->pendingUser->email, + 'role' => 'member', + 'link' => 'https://example.test/invite/team-b-shared-invitation', + 'via' => 'link', + ]); + $this->invitationA->forceFill(['created_at' => now()->subDays(10)])->save(); + + expect($this->invitationA->isValid())->toBeFalse(); + + $this->assertDatabaseMissing('team_invitations', ['id' => $this->invitationA->id]); + $this->assertDatabaseHas('team_invitations', ['id' => $invitationB->id]); + $this->assertDatabaseHas('users', ['id' => $this->pendingUser->id]); +}); + +test('keeps a pending user with another invitation after revocation', function () { + $invitationB = TeamInvitation::create([ + 'team_id' => $this->teamB->id, + 'uuid' => 'team-b-shared-invitation', + 'email' => $this->pendingUser->email, + 'role' => 'member', + 'link' => 'https://example.test/invite/team-b-shared-invitation', + 'via' => 'link', + ]); + + Livewire::test(Invitations::class, ['invitations' => TeamInvitation::ownedByCurrentTeam()->get()]) + ->call('deleteInvitation', $invitationB->id) + ->assertDispatched('success'); + + $this->assertDatabaseMissing('team_invitations', ['id' => $invitationB->id]); + $this->assertDatabaseHas('team_invitations', ['id' => $this->invitationA->id]); + $this->assertDatabaseHas('users', ['id' => $this->pendingUser->id]); +}); + +test('removes an unused pending user after the last revocation', function () { + $this->actingAs($this->ownerA); + session(['currentTeam' => $this->teamA]); + + Livewire::test(Invitations::class, ['invitations' => TeamInvitation::ownedByCurrentTeam()->get()]) + ->call('deleteInvitation', $this->invitationA->id) + ->assertDispatched('success'); + + $this->assertDatabaseMissing('team_invitations', ['id' => $this->invitationA->id]); + $this->assertDatabaseMissing('users', ['id' => $this->pendingUser->id]); +});