From 25761e29c4ae8b81bf4e5129f4a5135022b0104e Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 27 Oct 2026 14:37:38 +0100 Subject: [PATCH] fix(team): keep member API tokens on promotion Only revoke a member's team tokens when the role goes down. Promotions and ownership transfers on account deletion keep the tokens. Closes #12342 Co-Authored-By: Claude Opus 5.5 --- app/Actions/User/DeleteUserTeams.php | 1 - app/Livewire/Team/Member.php | 32 +++++-------- app/Models/User.php | 1 - .../ApiTokenTeamLifecycleSecurityTest.php | 48 +++++++++++++++++++ 4 files changed, 61 insertions(+), 21 deletions(-) diff --git a/app/Actions/User/DeleteUserTeams.php b/app/Actions/User/DeleteUserTeams.php index b2b06e7ba3..6c38942257 100644 --- a/app/Actions/User/DeleteUserTeams.php +++ b/app/Actions/User/DeleteUserTeams.php @@ -137,7 +137,6 @@ class DeleteUserTeams // Update the new owner's role to owner $team->members()->updateExistingPivot($newOwner->id, ['role' => 'owner']); - RevokeUserTeamTokens::forUserTeam($newOwner, $team->id); // Remove the current user from the team $team->members()->detach($this->user->id); diff --git a/app/Livewire/Team/Member.php b/app/Livewire/Team/Member.php index f28087056f..aeaf525505 100644 --- a/app/Livewire/Team/Member.php +++ b/app/Livewire/Team/Member.php @@ -25,12 +25,7 @@ class Member extends Component || Role::from($this->getMemberRole())->gt(auth()->user()->role())) { throw new \Exception('You are not authorized to perform this action.'); } - $teamId = currentTeam()->id; - DB::transaction(function () use ($teamId): void { - $this->member->teams()->updateExistingPivot($teamId, ['role' => Role::ADMIN->value]); - RevokeUserTeamTokens::forUserTeam($this->member, $teamId); - }); - $this->auditRoleUpdate($teamId, Role::ADMIN); + $this->updateRole(Role::ADMIN); $this->dispatch('reloadWindow'); } catch (\Exception $e) { $this->dispatch('error', $e->getMessage()); @@ -46,12 +41,7 @@ class Member extends Component || Role::from($this->getMemberRole())->gt(auth()->user()->role())) { throw new \Exception('You are not authorized to perform this action.'); } - $teamId = currentTeam()->id; - DB::transaction(function () use ($teamId): void { - $this->member->teams()->updateExistingPivot($teamId, ['role' => Role::OWNER->value]); - RevokeUserTeamTokens::forUserTeam($this->member, $teamId); - }); - $this->auditRoleUpdate($teamId, Role::OWNER); + $this->updateRole(Role::OWNER); $this->dispatch('reloadWindow'); } catch (\Exception $e) { $this->dispatch('error', $e->getMessage()); @@ -67,12 +57,7 @@ class Member extends Component || Role::from($this->getMemberRole())->gt(auth()->user()->role())) { throw new \Exception('You are not authorized to perform this action.'); } - $teamId = currentTeam()->id; - DB::transaction(function () use ($teamId): void { - $this->member->teams()->updateExistingPivot($teamId, ['role' => Role::MEMBER->value]); - RevokeUserTeamTokens::forUserTeam($this->member, $teamId); - }); - $this->auditRoleUpdate($teamId, Role::MEMBER); + $this->updateRole(Role::MEMBER); $this->dispatch('reloadWindow'); } catch (\Exception $e) { $this->dispatch('error', $e->getMessage()); @@ -114,8 +99,17 @@ class Member extends Component return $this->member->teams()->where('teams.id', currentTeam()->id)->first()?->pivot?->role; } - private function auditRoleUpdate(int $teamId, Role $role): void + private function updateRole(Role $role): void { + $teamId = currentTeam()->id; + $previousRole = Role::from($this->getMemberRole()); + DB::transaction(function () use ($teamId, $role, $previousRole): void { + $this->member->teams()->updateExistingPivot($teamId, ['role' => $role->value]); + // A promotion keeps the tokens: the API checks the abilities against the role on each request. + if ($role->lt($previousRole)) { + RevokeUserTeamTokens::forUserTeam($this->member, $teamId); + } + }); auditLog('ui.team_member.role_updated', [ 'team_id' => $teamId, 'member_id' => $this->member->id, diff --git a/app/Models/User.php b/app/Models/User.php index 1257f3c857..223813c8f3 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -171,7 +171,6 @@ class User extends Authenticatable implements PasskeyUser, SendsEmail if ($found_other_member_who_is_not_owner) { $found_other_member_who_is_not_owner->pivot->role = 'owner'; $found_other_member_who_is_not_owner->pivot->save(); - RevokeUserTeamTokens::forUserTeam($found_other_member_who_is_not_owner, $team->id); $team->members()->detach($user->id); } else { static::finalizeTeamDeletion($user, $team); diff --git a/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php b/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php index a6698833dc..c6a04c4432 100644 --- a/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php +++ b/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php @@ -1,5 +1,6 @@ where('id', $token->id)->exists())->toBeFalse(); }); +test('owner demotion to admin through team member component revokes team tokens', function () { + $owner = User::factory()->create(); + $this->team->members()->attach($owner->id, ['role' => 'owner']); + $this->team->members()->updateExistingPivot($this->user->id, ['role' => 'owner']); + $token = $this->user->createToken('write-token', ['write'])->accessToken; + + $this->actingAs($owner); + session(['currentTeam' => $this->team]); + + Livewire::test(Member::class, ['member' => $this->user]) + ->call('makeAdmin'); + + expect(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeFalse(); +}); + +test('promotion through team member component keeps team tokens', function (string $from, string $action, string $to) { + $owner = User::factory()->create(); + $this->team->members()->attach($owner->id, ['role' => 'owner']); + $this->team->members()->updateExistingPivot($this->user->id, ['role' => $from]); + $token = $this->user->createToken('ci-token', ['read'])->accessToken; + + $this->actingAs($owner); + session(['currentTeam' => $this->team]); + + Livewire::test(Member::class, ['member' => $this->user]) + ->call($action) + ->assertNotDispatched('error'); + + expect($this->user->fresh()->teams()->findOrFail($this->team->id)->pivot->role)->toBe($to) + ->and(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeTrue(); +})->with([ + 'admin to owner' => ['admin', 'makeOwner', 'owner'], + 'member to owner' => ['member', 'makeOwner', 'owner'], + 'member to admin' => ['member', 'makeAdmin', 'admin'], +]); + +test('ownership transfer on account deletion keeps the new owner tokens', function () { + $owner = User::factory()->create(); + $this->team->members()->attach($owner->id, ['role' => 'owner']); + $token = $this->user->createToken('ci-token', ['write'])->accessToken; + + (new DeleteUserTeams($owner))->execute(); + + expect($this->user->fresh()->teams()->findOrFail($this->team->id)->pivot->role)->toBe('owner') + ->and(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeTrue(); +}); + test('member removal rolls back when token revocation fails', function () { $owner = User::factory()->create(); $this->team->members()->attach($owner->id, ['role' => 'owner']);