From cb237c689b981f784042d0dc121c0af3786b6ac4 Mon Sep 17 00:00:00 2001
From: Imani Manyara <imanimanyara@users.noreply.github.com>
Date: Thu, 8 Oct 2026 16:09:09 -0400
Subject: [PATCH] Revoke a user's own Sanctum tokens, never another model's

SanctumTokenIssuer::revokeAll(), called on every password reset and
update, queried the token table by a morph type taken from configuration
instead of the user's own. A second token-bearing model with a colliding
id (an Admin with id 5) therefore revoked User 5's tokens and kept its
own. It also queried that table for user models that hold no Sanctum
tokens, so a reset failed where the table does not exist.

Revoke through the user's own tokens() relation, and only for models
that use Sanctum's HasApiTokens: the behaviour before the registry, now
scoped to the Sanctum issuer so Passport's tokens stay with its own.
Restores the rationale for the ability and expiry defaults that the move
into SanctumTokenIssuer dropped. Three tests, two failing before.
---
 CHANGELOG.md                             |  1 +
 src/Actions/SanctumTokenIssuer.php       | 40 +++++++------
 tests/Feature/SanctumTokenIssuerTest.php | 73 ++++++++++++++++++++++++
 3 files changed, 98 insertions(+), 16 deletions(-)
 create mode 100644 tests/Feature/SanctumTokenIssuerTest.php

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0bdeb44..113172f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -95,6 +95,7 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixed
 
+- **Revoking all of a user's tokens could delete another user's.** With the token issuer registry, `SanctumTokenIssuer::revokeAll()` (called on every password reset and update) queried the token table by a morph type taken from configuration rather than the user's own, so a second token-bearing model with a colliding id (an Admin with id 5) revoked User 5's tokens and kept its own. It also queried that table for user models that hold no Sanctum tokens, failing the reset where the table does not exist. It now revokes through the user's own `tokens()` relation, and only for models that use Sanctum's `HasApiTokens`, as before the registry.
 - The user-model exception named the old package.
 
 [Unreleased]: https://github.com/laranail/authkit/compare/v0.1.0...HEAD
diff --git a/src/Actions/SanctumTokenIssuer.php b/src/Actions/SanctumTokenIssuer.php
index 1a25c37..b196d63 100644
--- a/src/Actions/SanctumTokenIssuer.php
+++ b/src/Actions/SanctumTokenIssuer.php
@@ -5,9 +5,8 @@
 namespace Simtabi\Laranail\AuthKit\Actions;
 
 use DateTimeInterface;
-use Laravel\Sanctum\Sanctum;
 use InvalidArgumentException;
-use Illuminate\Database\Eloquent\Model;
+use Laravel\Sanctum\HasApiTokens;
 use Illuminate\Contracts\Auth\Authenticatable;
 use Simtabi\Laranail\AuthKit\Support\TokenResult;
 use Simtabi\Laranail\AuthKit\Contracts\TokenIssuerInterface;
@@ -50,27 +49,32 @@ public function revokeCurrent(Authenticatable $user): void
         }
     }
 
+    /**
+     * Revoke every Sanctum token the given user holds, through the user's own `tokens()` relation.
+     *
+     * Only a model that uses Sanctum's `HasApiTokens` can hold one. Querying the token table by a
+     * morph type taken from configuration instead deleted another model's tokens whenever ids
+     * collided (an Admin with id 5 revoked User 5's tokens and kept its own), and a model without
+     * the trait made every password reset hit a token table the application may not have.
+     */
     public function revokeAll(Authenticatable $user): void
     {
-        if (! $user instanceof Model) {
+        if (! in_array(HasApiTokens::class, class_uses_recursive($user), true)) {
             return;
         }
 
-        $model = Sanctum::$personalAccessTokenModel;
-        $morphType = $user->getMorphClass();
-        $userModel = config('laranail.authkit.user_model') ?? config('auth.providers.users.model');
-
-        if (is_string($userModel) && class_exists($userModel) && is_subclass_of($userModel, Model::class)) {
-            $morphType = (new $userModel)->getMorphClass();
-        }
-
-        $model::query()
-            ->where('tokenable_type', $morphType)
-            ->where('tokenable_id', $user->getAuthIdentifier())
-            ->delete();
+        $user->tokens()->delete();
     }
 
-    /** @return array<int, string> */
+    /**
+     * Both defaults come from configuration rather than being fixed here, because both were once
+     * fixed here in the least safe way available: every token was minted with the wildcard ability
+     * `*` and no expiry. A wildcard token can do anything its owner can, so a leaked one is a full
+     * account compromise; a token with no expiry recovered from a log or an old backup never stops
+     * working, and Sanctum's own `sanctum.expiration` is null by default.
+     *
+     * @return array<int, string>
+     */
     private function defaultAbilities(): array
     {
         $abilities = config('laranail.authkit.tokens.abilities', ['*']);
@@ -82,6 +86,10 @@ private function defaultAbilities(): array
         return array_values(array_filter($abilities, is_string(...)));
     }
 
+    /**
+     * A null lifetime defers to Sanctum's own `sanctum.expiration`, which is the only way to opt out
+     * of expiry deliberately rather than silently inherit none.
+     */
     private function defaultExpiry(): ?DateTimeInterface
     {
         $minutes = config('laranail.authkit.tokens.expires_after_minutes');
diff --git a/tests/Feature/SanctumTokenIssuerTest.php b/tests/Feature/SanctumTokenIssuerTest.php
new file mode 100644
index 0000000..df4fd0d
--- /dev/null
+++ b/tests/Feature/SanctumTokenIssuerTest.php
@@ -0,0 +1,73 @@
+<?php
+
+declare(strict_types=1);
+
+use Illuminate\Support\Str;
+use Workbench\App\Models\User;
+use Illuminate\Support\Facades\Hash;
+use Illuminate\Support\Facades\Schema;
+use Simtabi\Laranail\AuthKit\Actions\ResetUserPassword;
+use Simtabi\Laranail\AuthKit\Actions\SanctumTokenIssuer;
+use Illuminate\Foundation\Auth\User as PlainAuthenticatable;
+
+/*
+ * revokeAll() runs on every password reset and update. It must revoke exactly the tokens of the
+ * user it was given: never another model's tokens that share the id, and never fail for a user
+ * model that does not issue Sanctum tokens at all.
+ */
+
+/** A second token-bearing model over the same table, as an Admin model would be. */
+final class SanctumIssuerTestAdmin extends User
+{
+    protected $table = 'users';
+
+    public function getMorphClass(): string
+    {
+        return 'sanctum-issuer-test-admin';
+    }
+}
+
+/** A user model with no Sanctum tokens: no HasApiTokens, so no tokens() relation. */
+final class SanctumIssuerTestPlainUser extends PlainAuthenticatable
+{
+    protected $table = 'users';
+}
+
+it('revokes every token of the given user', function (): void {
+    $user = User::factory()->create();
+    $user->createToken('one');
+    $user->createToken('two');
+
+    app(SanctumTokenIssuer::class)->revokeAll($user);
+
+    expect($user->tokens()->count())->toBe(0);
+});
+
+it('leaves another model\'s tokens alone when the ids collide', function (): void {
+    $user = User::factory()->create();
+    $user->createToken('belongs-to-the-user');
+
+    $admin = SanctumIssuerTestAdmin::query()->findOrFail($user->getKey());
+    $admin->createToken('belongs-to-the-admin');
+
+    app(SanctumTokenIssuer::class)->revokeAll($admin);
+
+    expect($admin->tokens()->count())->toBe(0)
+        ->and($user->tokens()->count())->toBe(1);
+});
+
+it('resets the password of a user model without Sanctum tokens', function (): void {
+    $user = User::factory()->create();
+    Schema::drop('personal_access_tokens');
+
+    $plain = SanctumIssuerTestPlainUser::query()->findOrFail($user->getKey());
+
+    $password = Str::password(16);
+
+    app(ResetUserPassword::class)->reset($plain, [
+        'password'              => $password,
+        'password_confirmation' => $password,
+    ]);
+
+    expect(Hash::check($password, $plain->fresh()->password))->toBeTrue();
+});