From 0dc187510b8d7fe24017483fccec71aaef32d95a Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 28 Oct 2025 14:14:52 +0100 Subject: [PATCH] [4.x] Clean up expired impersonation tokens instead of just aborting, add command for cleaning up expired tokens (#1387) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR makes the expired/invalid tenant impersonation tokens get deleted instead of just aborting with 403. The PR also adds a command (ClearExpiredImpersonationTokens) used like `php artisan tenants:purge-impersonation-tokens`. As the name suggests, it clears all expired impersonation tokens (= tokens older than `UserImpersonation::$ttl`). Resolves #1348 --------- Co-authored-by: Samuel Štancl --- src/Commands/PurgeImpersonationTokens.php | 38 ++++++++ src/Features/UserImpersonation.php | 12 ++- src/TenancyServiceProvider.php | 1 + tests/TenantUserImpersonationTest.php | 112 ++++++++++++++++++++++ 4 files changed, 161 insertions(+), 2 deletions(-) create mode 100644 src/Commands/PurgeImpersonationTokens.php diff --git a/src/Commands/PurgeImpersonationTokens.php b/src/Commands/PurgeImpersonationTokens.php new file mode 100644 index 00000000..b64b29f8 --- /dev/null +++ b/src/Commands/PurgeImpersonationTokens.php @@ -0,0 +1,38 @@ +components->info('Deleting expired impersonation tokens.'); + + $expirationDate = now()->subSeconds(UserImpersonation::$ttl); + + $impersonationTokenModel = UserImpersonation::modelClass(); + + $deletedTokenCount = $impersonationTokenModel::where('created_at', '<', $expirationDate) + ->delete(); + + $this->components->info($deletedTokenCount . ' expired impersonation ' . str('token')->plural($deletedTokenCount) . ' deleted.'); + + return 0; + } +} diff --git a/src/Features/UserImpersonation.php b/src/Features/UserImpersonation.php index ac478d07..d286b8ba 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/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index a7f27e63..9b32f088 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -119,6 +119,7 @@ class TenancyServiceProvider extends ServiceProvider Commands\MigrateFresh::class, Commands\ClearPendingTenants::class, Commands\CreatePendingTenants::class, + Commands\PurgeImpersonationTokens::class, Commands\CreateUserWithRLSPolicies::class, ]); diff --git a/tests/TenantUserImpersonationTest.php b/tests/TenantUserImpersonationTest.php index 48fbe691..ea679357 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', [ @@ -294,6 +295,117 @@ 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(); +}); + +test('expired impersonation tokens can be cleaned up using a command', function () { + $tenant = Tenant::create(); + migrateTenants(); + $user = $tenant->run(function () { + return ImpersonationUser::create([ + 'name' => 'foo', + 'email' => 'foo@bar', + 'password' => bcrypt('password'), + ]); + }); + + // Create tokens + $oldToken = tenancy()->impersonate($tenant, $user->id, '/dashboard'); + $anotherOldToken = tenancy()->impersonate($tenant, $user->id, '/dashboard'); + $activeToken = tenancy()->impersonate($tenant, $user->id, '/dashboard'); + + // Make two of the tokens expired by updating their created_at + $oldToken->update([ + 'created_at' => Carbon::now()->subSeconds(UserImpersonation::$ttl + 10), + ]); + + $anotherOldToken->update([ + 'created_at' => Carbon::now()->subSeconds(UserImpersonation::$ttl + 10), + ]); + + // All tokens exist + expect(ImpersonationToken::find($activeToken->token))->not()->toBeNull(); + expect(ImpersonationToken::find($oldToken->token))->not()->toBeNull(); + expect(ImpersonationToken::find($anotherOldToken->token))->not()->toBeNull(); + + pest()->artisan('tenants:purge-impersonation-tokens') + ->assertExitCode(0) + ->expectsOutputToContain('2 expired impersonation tokens deleted'); + + // The expired tokens were deleted + expect(ImpersonationToken::find($oldToken->token))->toBeNull(); + expect(ImpersonationToken::find($anotherOldToken->token))->toBeNull(); + // The active token still exists + expect(ImpersonationToken::find($activeToken->token))->not()->toBeNull(); + + // Update the active token to make it expired according to the default ttl (60s) + $activeToken->update([ + 'created_at' => Carbon::now()->subSeconds(70), + ]); + + // With ttl set to 80s, the active token should not be deleted (token is only considered expired if older than 80s) + UserImpersonation::$ttl = 80; + pest()->artisan('tenants:purge-impersonation-tokens') + ->assertExitCode(0) + ->expectsOutputToContain('0 expired impersonation tokens deleted'); + + expect(ImpersonationToken::find($activeToken->token))->not()->toBeNull(); +}); + function migrateTenants() { pest()->artisan('tenants:migrate')->assertExitCode(0);