From 2d5b6aa0c897c5124956a40f3b2a7e969190e851 Mon Sep 17 00:00:00 2001
From: lukinovec <lukinovec@gmail.com>
Date: Wed, 6 Aug 2025 12:58:18 +0200
Subject: [PATCH] Clean up impersonation tokens before aborting, add tests

---
 src/Features/UserImpersonation.php    | 12 +++++-
 tests/TenantUserImpersonationTest.php | 58 +++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/src/Features/UserImpersonation.php b/src/Features/UserImpersonation.php
index 3db563a4..90740cea 100644
--- a/src/Features/UserImpersonation.php
+++ b/src/Features/UserImpersonation.php
@@ -44,12 +44,20 @@ class UserImpersonation implements Feature
 
         $tokenExpired = $token->created_at->diffInSeconds(now()) > $ttl;
 
-        abort_if($tokenExpired, 403);
+        if ($tokenExpired) {
+            $token->delete();
+
+            abort(403);
+        }
 
         $tokenTenantId = (string) $token->getAttribute(Tenancy::tenantKeyColumn());
         $currentTenantId = (string) tenant()->getTenantKey();
 
-        abort_unless($tokenTenantId === $currentTenantId, 403);
+        if ($tokenTenantId !== $currentTenantId) {
+            $token->delete();
+
+            abort(403);
+        }
 
         Auth::guard($token->auth_guard)->loginUsingId($token->user_id, $token->remember);
 
diff --git a/tests/TenantUserImpersonationTest.php b/tests/TenantUserImpersonationTest.php
index 8c9c4124..0f43522c 100644
--- a/tests/TenantUserImpersonationTest.php
+++ b/tests/TenantUserImpersonationTest.php
@@ -26,6 +26,7 @@ use Stancl\Tenancy\Middleware\InitializeTenancyByDomain;
 use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper;
 use Stancl\Tenancy\Exceptions\StatefulGuardRequiredException;
 use function Stancl\Tenancy\Tests\pest;
+use Symfony\Component\HttpKernel\Exception\HttpException;
 
 beforeEach(function () {
     pest()->artisan('migrate', [
@@ -292,6 +293,63 @@ test('impersonation tokens can be created only with stateful guards', function (
         ->toBeInstanceOf(ImpersonationToken::class);
 });
 
+test('expired tokens are cleaned up before aborting', function () {
+    $tenant = Tenant::create();
+    migrateTenants();
+
+    $user = $tenant->run(function () {
+        return ImpersonationUser::create([
+            'name' => 'foo',
+            'email' => 'foo@bar',
+            'password' => bcrypt('password'),
+        ]);
+    });
+
+    $token = tenancy()->impersonate($tenant, $user->id, '/dashboard');
+
+    // Make the token expired
+    $token->update([
+        'created_at' => Carbon::now()->subSeconds(100),
+    ]);
+
+    expect(ImpersonationToken::find($token->token))->not()->toBeNull();
+
+    tenancy()->initialize($tenant);
+
+    // Try to use the expired token - should clean up and abort
+    expect(fn() => UserImpersonation::makeResponse($token->token))
+        ->toThrow(HttpException::class); // Abort with 403
+
+    expect(ImpersonationToken::find($token->token))->toBeNull();
+});
+
+test('tokens are cleaned up when in wrong tenant context before aborting', function () {
+    $tenant1 = Tenant::create();
+    $tenant2 = Tenant::create();
+
+    migrateTenants();
+
+    $user = $tenant1->run(function () {
+        return ImpersonationUser::create([
+            'name' => 'foo',
+            'email' => 'foo@bar',
+            'password' => bcrypt('password'),
+        ]);
+    });
+
+    $token = tenancy()->impersonate($tenant1, $user->id, '/dashboard');
+
+    expect(ImpersonationToken::find($token->token))->not->toBeNull();
+
+    tenancy()->initialize($tenant2);
+
+    // Try to use the token in wrong tenant context - should clean up and abort
+    expect(fn() => UserImpersonation::makeResponse($token->token))
+        ->toThrow(HttpException::class); // Abort with 403
+
+    expect(ImpersonationToken::find($token->token))->toBeNull();
+});
+
 function migrateTenants()
 {
     pest()->artisan('tenants:migrate')->assertExitCode(0);