fix(api-tokens): scope token access to current team (#11396)

app/Jobs/ApiTokenExpirationWarningJob.php

@@ -41,6 +41,10 @@ class ApiTokenExpirationWarningJob implements ShouldBeEncrypted, ShouldQueue, Si
                         continue;
                     }
 
+                    if (! $team->members()->whereKey($token->tokenable_id)->exists()) {
+                        continue;
+                    }
+
                     $warningSentAt = now();
 
                     $team->notify(new ApiTokenExpiringNotification($token));

app/Livewire/Security/ApiTokens.php

@@ -59,7 +59,10 @@ class ApiTokens extends Component
 
     private function getTokens()
     {
-        $this->tokens = auth()->user()->tokens->sortByDesc('created_at');
+        $this->tokens = auth()->user()->tokens()
+            ->where('team_id', currentTeam()->id)
+            ->latest()
+            ->get();
     }
 
     public function updatedPermissions($permissionToUpdate)
@@ -148,7 +151,10 @@ class ApiTokens extends Component
     public function revoke(int $id)
     {
         try {
-            $token = auth()->user()->tokens()->where('id', $id)->firstOrFail();
+            $token = auth()->user()->tokens()
+                ->where('team_id', currentTeam()->id)
+                ->where('id', $id)
+                ->firstOrFail();
             $this->authorize('delete', $token);
             $token->delete();
             $this->getTokens();

app/Policies/ApiTokenPolicy.php

@@ -20,7 +20,7 @@ class ApiTokenPolicy
      */
     public function view(User $user, PersonalAccessToken $token): bool
     {
-        return $user->id === $token->tokenable_id && $token->tokenable_type === User::class;
+        return $this->belongsToUserAndCurrentTeam($user, $token);
     }
 
     /**
@@ -36,7 +36,7 @@ class ApiTokenPolicy
      */
     public function update(User $user, PersonalAccessToken $token): bool
     {
-        return $user->id === $token->tokenable_id && $token->tokenable_type === User::class;
+        return $this->belongsToUserAndCurrentTeam($user, $token);
     }
 
     /**
@@ -44,7 +44,7 @@ class ApiTokenPolicy
      */
     public function delete(User $user, PersonalAccessToken $token): bool
     {
-        return $user->id === $token->tokenable_id && $token->tokenable_type === User::class;
+        return $this->belongsToUserAndCurrentTeam($user, $token);
     }
 
     /**
@@ -86,4 +86,19 @@ class ApiTokenPolicy
     {
         return $user->isAdmin() || $user->isOwner();
     }
+
+    private function belongsToUserAndCurrentTeam(User $user, PersonalAccessToken $token): bool
+    {
+        if ($user->id !== $token->tokenable_id || $token->tokenable_type !== User::class) {
+            return false;
+        }
+
+        $currentTeamId = $user->currentTeam()?->id;
+
+        if ($currentTeamId === null || $token->team_id === null) {
+            return false;
+        }
+
+        return (string) $currentTeamId === (string) $token->team_id;
+    }
 }

tests/Feature/ApiTokenExpirationWarningTest.php

@@ -43,7 +43,7 @@ function createTokenExpiring(User $user, Team $team, ?Carbon $expiresAt, ?Carbon
 
 describe('ApiTokenExpirationWarningJob', function () {
     test('notifies team when token expires within 24h', function () {
-        $token = createTokenExpiring($this->user, $this->team, now()->addHours(23));
+        $token = createTokenExpiring($this->user, $this->team, Carbon::now()->addHours(23));
 
         (new ApiTokenExpirationWarningJob)->handle();
 
@@ -52,7 +52,7 @@ describe('ApiTokenExpirationWarningJob', function () {
     });
 
     test('does not mark token as warned when notification fails', function () {
-        $token = createTokenExpiring($this->user, $this->team, now()->addHours(23));
+        $token = createTokenExpiring($this->user, $this->team, Carbon::now()->addHours(23));
         $dispatcher = Mockery::mock(Dispatcher::class);
         $dispatcher->shouldReceive('send')
             ->once()
@@ -67,7 +67,7 @@ describe('ApiTokenExpirationWarningJob', function () {
     });
 
     test('database marker prevents duplicate warnings on repeat runs', function () {
-        createTokenExpiring($this->user, $this->team, now()->addHours(12));
+        createTokenExpiring($this->user, $this->team, Carbon::now()->addHours(12));
 
         (new ApiTokenExpirationWarningJob)->handle();
         (new ApiTokenExpirationWarningJob)->handle();
@@ -76,7 +76,7 @@ describe('ApiTokenExpirationWarningJob', function () {
     });
 
     test('database marker prevents duplicate warnings after cache is flushed', function () {
-        createTokenExpiring($this->user, $this->team, now()->addHours(12));
+        createTokenExpiring($this->user, $this->team, Carbon::now()->addHours(12));
 
         (new ApiTokenExpirationWarningJob)->handle();
 
@@ -88,7 +88,7 @@ describe('ApiTokenExpirationWarningJob', function () {
     });
 
     test('skips tokens that already have an expiration warning marker', function () {
-        createTokenExpiring($this->user, $this->team, now()->addHours(12), now()->subHour());
+        createTokenExpiring($this->user, $this->team, Carbon::now()->addHours(12), Carbon::now()->subHour());
 
         (new ApiTokenExpirationWarningJob)->handle();
 
@@ -96,8 +96,8 @@ describe('ApiTokenExpirationWarningJob', function () {
     });
 
     test('notifies once for each unmarked expiring token', function () {
-        createTokenExpiring($this->user, $this->team, now()->addHours(12));
-        createTokenExpiring($this->user, $this->team, now()->addHours(23));
+        createTokenExpiring($this->user, $this->team, Carbon::now()->addHours(12));
+        createTokenExpiring($this->user, $this->team, Carbon::now()->addHours(23));
 
         (new ApiTokenExpirationWarningJob)->handle();
 
@@ -105,7 +105,7 @@ describe('ApiTokenExpirationWarningJob', function () {
     });
 
     test('skips tokens expiring more than 24h out', function () {
-        createTokenExpiring($this->user, $this->team, now()->addDays(3));
+        createTokenExpiring($this->user, $this->team, Carbon::now()->addDays(3));
 
         (new ApiTokenExpirationWarningJob)->handle();
 
@@ -113,7 +113,7 @@ describe('ApiTokenExpirationWarningJob', function () {
     });
 
     test('skips already-expired tokens', function () {
-        createTokenExpiring($this->user, $this->team, now()->subHour());
+        createTokenExpiring($this->user, $this->team, Carbon::now()->subHour());
 
         (new ApiTokenExpirationWarningJob)->handle();
 
@@ -127,4 +127,14 @@ describe('ApiTokenExpirationWarningJob', function () {
 
         Notification::assertNothingSent();
     });
+
+    test('skips tokens whose owner is no longer a team member', function () {
+        $token = createTokenExpiring($this->user, $this->team, Carbon::now()->addHours(12));
+        $this->team->members()->detach($this->user);
+
+        (new ApiTokenExpirationWarningJob)->handle();
+
+        Notification::assertNothingSent();
+        expect($token->fresh()->api_token_expiration_warning_sent_at)->toBeNull();
+    });
 });

tests/Feature/ApiTokenLivewireAuthorizationTest.php

@@ -76,6 +76,42 @@ test('member can still create read token', function () {
         ->and($token->abilities)->toBe(['read']);
 });
 
+test('api token list only contains tokens for the current team', function () {
+    $user = User::factory()->create();
+    $otherTeam = Team::factory()->create();
+    $this->team->members()->attach($user->id, ['role' => 'admin']);
+    $otherTeam->members()->attach($user->id, ['role' => 'admin']);
+
+    session(['currentTeam' => $this->team]);
+    $currentTeamToken = $user->createToken('current-team-token', ['read'])->accessToken;
+
+    session(['currentTeam' => $otherTeam]);
+    $user->createToken('other-team-token', ['read']);
+
+    $this->actingAs($user);
+    session(['currentTeam' => $this->team]);
+
+    Livewire::test(ApiTokens::class)
+        ->assertSet('tokens', fn ($tokens) => $tokens->pluck('id')->all() === [$currentTeamToken->id]);
+});
+
+test('user cannot revoke a token from another team through the current team', function () {
+    $user = User::factory()->create();
+    $otherTeam = Team::factory()->create();
+    $this->team->members()->attach($user->id, ['role' => 'admin']);
+    $otherTeam->members()->attach($user->id, ['role' => 'admin']);
+
+    session(['currentTeam' => $otherTeam]);
+    $otherTeamToken = $user->createToken('other-team-token', ['read'])->accessToken;
+
+    $this->actingAs($user);
+    session(['currentTeam' => $this->team]);
+
+    Livewire::test(ApiTokens::class)->call('revoke', $otherTeamToken->id);
+
+    expect($user->tokens()->whereKey($otherTeamToken->id)->exists())->toBeTrue();
+});
+
 test('owner can create root token', function () {
     $owner = User::factory()->create();
     $this->team->members()->attach($owner->id, ['role' => 'owner']);

tests/Unit/Policies/ApiTokenPolicyTest.php

@@ -1,9 +1,28 @@
 <?php
 
+use App\Models\Team;
 use App\Models\User;
 use App\Policies\ApiTokenPolicy;
 use Laravel\Sanctum\PersonalAccessToken;
 
+function apiTokenForUserAndTeam(int $userId, int $teamId): PersonalAccessToken
+{
+    $token = Mockery::mock(PersonalAccessToken::class)->makePartial();
+    $token->tokenable_id = $userId;
+    $token->tokenable_type = User::class;
+    $token->team_id = $teamId;
+
+    return $token;
+}
+
+function apiTokenTeam(int $teamId): Team
+{
+    $team = new Team;
+    $team->id = $teamId;
+
+    return $team;
+}
+
 it('allows any user to view any api tokens', function () {
     $user = Mockery::mock(User::class)->makePartial();
 
@@ -28,10 +47,9 @@ it('allows any user to manage api tokens', function () {
 it('allows owner to view their own api token', function () {
     $user = Mockery::mock(User::class)->makePartial();
     $user->id = 1;
+    $user->shouldReceive('currentTeam')->andReturn(apiTokenTeam(10));
 
-    $token = Mockery::mock(PersonalAccessToken::class)->makePartial();
-    $token->tokenable_id = 1;
-    $token->tokenable_type = User::class;
+    $token = apiTokenForUserAndTeam(1, 10);
 
     $policy = new ApiTokenPolicy;
     expect($policy->view($user, $token))->toBeTrue();
@@ -52,10 +70,9 @@ it('denies non-owner from viewing api token', function () {
 it('allows owner to update their own api token', function () {
     $user = Mockery::mock(User::class)->makePartial();
     $user->id = 1;
+    $user->shouldReceive('currentTeam')->andReturn(apiTokenTeam(10));
 
-    $token = Mockery::mock(PersonalAccessToken::class)->makePartial();
-    $token->tokenable_id = 1;
-    $token->tokenable_type = User::class;
+    $token = apiTokenForUserAndTeam(1, 10);
 
     $policy = new ApiTokenPolicy;
     expect($policy->update($user, $token))->toBeTrue();
@@ -76,10 +93,9 @@ it('denies non-owner from updating api token', function () {
 it('allows owner to delete their own api token', function () {
     $user = Mockery::mock(User::class)->makePartial();
     $user->id = 1;
+    $user->shouldReceive('currentTeam')->andReturn(apiTokenTeam(10));
 
-    $token = Mockery::mock(PersonalAccessToken::class)->makePartial();
-    $token->tokenable_id = 1;
-    $token->tokenable_type = User::class;
+    $token = apiTokenForUserAndTeam(1, 10);
 
     $policy = new ApiTokenPolicy;
     expect($policy->delete($user, $token))->toBeTrue();
@@ -97,6 +113,31 @@ it('denies non-owner from deleting api token', function () {
     expect($policy->delete($user, $token))->toBeFalse();
 });
 
+it('denies access to an owned api token from another team', function (string $ability) {
+    $user = Mockery::mock(User::class)->makePartial();
+    $user->id = 1;
+    $user->shouldReceive('currentTeam')->andReturn(apiTokenTeam(10));
+
+    $token = apiTokenForUserAndTeam(1, 20);
+
+    $policy = new ApiTokenPolicy;
+    expect($policy->{$ability}($user, $token))->toBeFalse();
+})->with(['view', 'update', 'delete']);
+
+it('denies access to an owned api token without team identifiers', function (string $ability) {
+    $user = Mockery::mock(User::class)->makePartial();
+    $user->id = 1;
+    $user->shouldReceive('currentTeam')->andReturnNull();
+
+    $token = Mockery::mock(PersonalAccessToken::class)->makePartial();
+    $token->tokenable_id = 1;
+    $token->tokenable_type = User::class;
+    $token->team_id = null;
+
+    $policy = new ApiTokenPolicy;
+    expect($policy->{$ability}($user, $token))->toBeFalse();
+})->with(['view', 'update', 'delete']);
+
 it('allows admin to use root permissions', function () {
     $user = Mockery::mock(User::class)->makePartial();
     $user->shouldReceive('isAdmin')->andReturn(true);