From 9a4469a42d472588cd159a1b3059dd099fd75936 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:10:10 +0200 Subject: [PATCH] fix(team): keep users of other teams when an invitation is revoked or expires A user who accepted another team's invitation but had not set a password yet was deleted, with their team data. Now only users in no team except their own personal team are deleted. --- app/Models/User.php | 32 ++++- .../TeamInvitationRevokeUserCleanupTest.php | 124 ++++++++++++++++++ 2 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 tests/Feature/Team/TeamInvitationRevokeUserCleanupTest.php diff --git a/app/Models/User.php b/app/Models/User.php index a66147d941..e8429ac41a 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -210,14 +210,38 @@ class User extends Authenticatable implements SendsEmail } /** - * Delete the user if they are not verified and have a force password reset. + * Delete the user if they are a provisional invitee that never joined any team. * This is used to clean up users that have been invited, did not accept the invitation (and did not verify their email and have a force password reset). + * Users that already belong to a team other than their own personal team, or still have pending invitations, are kept. */ - public function deleteIfNotVerifiedAndForcePasswordReset() + public function deleteIfNotVerifiedAndForcePasswordReset(): void { - if ($this->hasVerifiedEmail() === false && $this->force_password_reset === true && ! TeamInvitation::whereEmail($this->email)->exists()) { - $this->delete(); + if ($this->hasVerifiedEmail() || $this->force_password_reset !== true) { + return; } + + if (TeamInvitation::whereEmail($this->email)->exists()) { + return; + } + + if ($this->belongsToNonPersonalTeam()) { + return; + } + + $this->delete(); + } + + /** + * Whether the user is a member of any team other than a personal team where they are the only member. + */ + private function belongsToNonPersonalTeam(): bool + { + return $this->teams() + ->where(function ($query) { + $query->where('personal_team', false) + ->orWhereHas('members', fn ($members) => $members->where('users.id', '!=', $this->id)); + }) + ->exists(); } public function recreate_personal_team() diff --git a/tests/Feature/Team/TeamInvitationRevokeUserCleanupTest.php b/tests/Feature/Team/TeamInvitationRevokeUserCleanupTest.php new file mode 100644 index 0000000000..a5b694012d --- /dev/null +++ b/tests/Feature/Team/TeamInvitationRevokeUserCleanupTest.php @@ -0,0 +1,124 @@ + InstanceSettings::query()->updateOrCreate(['id' => 0], ['fqdn' => null])); + + $this->teamA = Team::factory()->create(); + $this->teamB = Team::factory()->create(); + $this->ownerB = User::factory()->create(); + $this->teamB->members()->attach($this->ownerB->id, ['role' => 'owner']); + + $this->actingAs($this->ownerB); + session(['currentTeam' => $this->teamB]); +}); + +/** + * Mirrors InviteLink: a provisional user is created (with a personal team via the created hook). + */ +function revokeCleanupProvisionalInvitee(string $email): User +{ + return User::create([ + 'name' => str($email)->before('@'), + 'email' => $email, + 'password' => bcrypt('secret-password'), + 'force_password_reset' => true, + ]); +} + +function revokeCleanupInvitation(Team $team, string $email, string $uuid): TeamInvitation +{ + return TeamInvitation::create([ + 'team_id' => $team->id, + 'uuid' => $uuid, + 'email' => $email, + 'role' => 'member', + 'link' => "http://example.test/invitations/{$uuid}", + 'via' => 'link', + ]); +} + +it('does not delete an invitee who already joined another team when revoking an invitation', function () { + $bob = revokeCleanupProvisionalInvitee('bob@example.com'); + $bob->teams()->attach($this->teamA->id, ['role' => 'member']); + $invitation = revokeCleanupInvitation($this->teamB, $bob->email, 'team-b-invitation'); + + Livewire::test(Invitations::class, ['invitations' => collect([$invitation])]) + ->call('deleteInvitation', $invitation->id) + ->assertDispatched('success'); + + $this->assertDatabaseMissing('team_invitations', ['id' => $invitation->id]); + $this->assertDatabaseHas('users', ['id' => $bob->id]); + $this->assertDatabaseHas('team_user', ['team_id' => $this->teamA->id, 'user_id' => $bob->id]); + $this->assertDatabaseHas('teams', ['id' => $this->teamA->id]); +}); + +it('does not delete an invitee who is the sole member of another shared team when revoking', function () { + $bob = revokeCleanupProvisionalInvitee('bob@example.com'); + $bob->teams()->attach($this->teamA->id, ['role' => 'owner']); + $invitation = revokeCleanupInvitation($this->teamB, $bob->email, 'team-b-invitation'); + + Livewire::test(Invitations::class, ['invitations' => collect([$invitation])]) + ->call('deleteInvitation', $invitation->id) + ->assertDispatched('success'); + + $this->assertDatabaseHas('users', ['id' => $bob->id]); + $this->assertDatabaseHas('teams', ['id' => $this->teamA->id]); +}); + +it('still deletes a provisional invitee who never accepted any invitation when revoking', function () { + $invitee = revokeCleanupProvisionalInvitee('never-accepted@example.com'); + $personalTeamId = $invitee->teams()->value('teams.id'); + $invitation = revokeCleanupInvitation($this->teamB, $invitee->email, 'never-accepted-invitation'); + + Livewire::test(Invitations::class, ['invitations' => collect([$invitation])]) + ->call('deleteInvitation', $invitation->id) + ->assertDispatched('success'); + + $this->assertDatabaseMissing('users', ['id' => $invitee->id]); + $this->assertDatabaseMissing('teams', ['id' => $personalTeamId]); +}); + +it('keeps a provisional invitee who still has another pending invitation when revoking', function () { + $invitee = revokeCleanupProvisionalInvitee('two-invites@example.com'); + revokeCleanupInvitation($this->teamA, $invitee->email, 'team-a-pending'); + $invitation = revokeCleanupInvitation($this->teamB, $invitee->email, 'team-b-pending'); + + Livewire::test(Invitations::class, ['invitations' => collect([$invitation])]) + ->call('deleteInvitation', $invitation->id) + ->assertDispatched('success'); + + $this->assertDatabaseHas('users', ['id' => $invitee->id]); +}); + +it('does not delete an invitee who already joined another team when an invitation expires', function () { + $bob = revokeCleanupProvisionalInvitee('bob@example.com'); + $bob->teams()->attach($this->teamA->id, ['role' => 'member']); + $invitation = revokeCleanupInvitation($this->teamB, $bob->email, 'team-b-expired'); + $invitation->forceFill(['created_at' => now()->subDays(config('constants.invitation.link.expiration_days') + 2)])->save(); + + expect($invitation->fresh()->isValid())->toBeFalse(); + + $this->assertDatabaseMissing('team_invitations', ['id' => $invitation->id]); + $this->assertDatabaseHas('users', ['id' => $bob->id]); + $this->assertDatabaseHas('team_user', ['team_id' => $this->teamA->id, 'user_id' => $bob->id]); +}); + +it('still deletes a provisional invitee who never accepted when their only invitation expires', function () { + $invitee = revokeCleanupProvisionalInvitee('expired@example.com'); + $invitation = revokeCleanupInvitation($this->teamB, $invitee->email, 'expired-invitation'); + $invitation->forceFill(['created_at' => now()->subDays(config('constants.invitation.link.expiration_days') + 2)])->save(); + + expect($invitation->fresh()->isValid())->toBeFalse(); + + $this->assertDatabaseMissing('users', ['id' => $invitee->id]); +});