[4.x] UserImpersonation: store auth guard in session, add $logout param to stopImpersonating()

src/Features/UserImpersonation.php

@@ -4,6 +4,7 @@
 
 namespace Stancl\Tenancy\Features;
 
+use Exception;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Support\Facades\Auth;
@@ -61,9 +62,9 @@ public static function makeResponse(#[\SensitiveParameter] string|Model $token,
 
         Auth::guard($token->auth_guard)->loginUsingId($token->user_id, $token->remember);
 
-        $token->delete();
+        session()->put('tenancy_impersonation_guard', $token->auth_guard);
 
-        session()->put('tenancy_impersonating', true);
+        $token->delete();
 
         return redirect($token->redirect_url);
     }
@@ -76,16 +77,30 @@ public static function modelClass(): string
 
     public static function isImpersonating(): bool
     {
-        return session()->has('tenancy_impersonating');
+        return session()->has('tenancy_impersonation_guard');
     }
 
     /**
-     * Logout from the current domain and forget impersonation session.
+     * Stop user impersonation by forgetting the impersonation session.
+     *
+     * When $logout is true, the user will also be logged out
+     * from the impersonation guard stored in the session.
+     *
+     * Throws an exception if impersonation is not active
+     * (= the impersonation guard is not in the session).
      */
-    public static function stopImpersonating(): void
+    public static function stopImpersonating(bool $logout = true): void
     {
-        auth()->logout();
+        if (! static::isImpersonating()) {
+            throw new Exception('Not currently impersonating any user.');
+        }
+
+        if ($logout) {
+            $guard = session()->get('tenancy_impersonation_guard');
+
+            auth($guard)->logout();
+        }
 
-        session()->forget('tenancy_impersonating');
+        session()->forget('tenancy_impersonation_guard');
     }
 }

tests/TenantUserImpersonationTest.php

@@ -89,13 +89,14 @@
         ->assertSee('You are logged in as Joe');
 
     expect(UserImpersonation::isImpersonating())->toBeTrue();
-    expect(session('tenancy_impersonating'))->toBeTrue();
+    expect(session('tenancy_impersonation_guard'))->toBe('web');
+    expect($token->auth_guard)->toBe('web');
 
     // Leave impersonation
     UserImpersonation::stopImpersonating();
 
     expect(UserImpersonation::isImpersonating())->toBeFalse();
-    expect(session('tenancy_impersonating'))->toBeNull();
+    expect(session('tenancy_impersonation_guard'))->toBeNull();
 
     // Assert can't access the tenant dashboard
     pest()->get('http://foo.localhost/dashboard')
@@ -135,19 +136,114 @@
         ->assertSee('You are logged in as Joe');
 
     expect(UserImpersonation::isImpersonating())->toBeTrue();
-    expect(session('tenancy_impersonating'))->toBeTrue();
+    expect(session('tenancy_impersonation_guard'))->toBe('web');
+    expect($token->auth_guard)->toBe('web');
 
     // Leave impersonation
     UserImpersonation::stopImpersonating();
 
     expect(UserImpersonation::isImpersonating())->toBeFalse();
-    expect(session('tenancy_impersonating'))->toBeNull();
+    expect(session('tenancy_impersonation_guard'))->toBeNull();
 
     // Assert can't access the tenant dashboard
     pest()->get('/acme/dashboard')
         ->assertRedirect('/login');
 });
 
+test('stopImpersonating can keep the user authenticated', function() {
+    makeLoginRoute();
+
+    Route::middleware(InitializeTenancyByPath::class)->prefix('/{tenant}')->group(getRoutes(false));
+
+    $tenant = Tenant::create([
+        'id' => 'acme',
+        'tenancy_db_name' => 'db' . Str::random(16),
+    ]);
+
+    migrateTenants();
+
+    $user = $tenant->run(function () {
+        return ImpersonationUser::create([
+            'name' => 'Joe',
+            'email' => 'joe@local',
+            'password' => bcrypt('secret'),
+        ]);
+    });
+
+    // Impersonate the user
+    $token = tenancy()->impersonate($tenant, $user->id, '/acme/dashboard');
+
+    pest()->get('/acme/impersonate/' . $token->token)
+        ->assertRedirect('/acme/dashboard');
+
+    expect(UserImpersonation::isImpersonating())->toBeTrue();
+
+    // Stop impersonating without logging out
+    UserImpersonation::stopImpersonating(false);
+
+    // The impersonation session key should be cleared
+    expect(UserImpersonation::isImpersonating())->toBeFalse();
+    expect(session('tenancy_impersonation_guard'))->toBeNull();
+
+    // The user should still be authenticated
+    pest()->get('/acme/dashboard')
+        ->assertSuccessful()
+        ->assertSee('You are logged in as Joe');
+});
+
+test('stopImpersonating logs out the user from the impersonation guard stored in session', function() {
+    Route::middleware(InitializeTenancyByPath::class)->prefix('/{tenant}')->group(getRoutes(false));
+
+    $tenant = Tenant::create([
+        'id' => 'acme',
+        'tenancy_db_name' => 'db' . Str::random(16),
+    ]);
+
+    migrateTenants();
+
+    $user = $tenant->run(function () {
+        return ImpersonationUser::create([
+            'name' => 'Joe',
+            'email' => 'joe@local',
+            'password' => bcrypt('secret'),
+        ]);
+    });
+
+    // Impersonate the user
+    $token = tenancy()->impersonate($tenant, $user->id, '/acme/dashboard');
+
+    pest()->get('/acme/impersonate/' . $token->token)
+        ->assertRedirect('/acme/dashboard');
+
+    expect(session('tenancy_impersonation_guard'))->toBe('web');
+
+    // Impersonation logged in the user using the current guard ('web')
+    expect(auth('web')->check())->toBeTrue();
+
+    config(['auth.guards.test' => [
+        'driver' => 'session',
+        'provider' => 'users',
+    ]]);
+
+    // Switch guard from 'web' to 'test' and manually log in the user through 'test'
+    auth()->shouldUse('test');
+    auth()->loginUsingId($user->id);
+
+    // Should log out the user from the guard used for impersonation ('web')
+    UserImpersonation::stopImpersonating();
+
+    expect(auth('web')->check())->toBeFalse();
+    expect(auth('test')->check())->toBeTrue();
+
+    expect(UserImpersonation::isImpersonating())->toBeFalse();
+
+    // tenancy_impersonation_guard isn't in the session anymore,
+    // stopImpersonating should throw an exception instead of logging out
+    expect(fn() => UserImpersonation::stopImpersonating())->toThrow(Exception::class);
+
+    expect(auth()->check())->toBeTrue();
+});
+
 test('tokens have a limited ttl', function () {
     Route::middleware(InitializeTenancyByDomain::class)->group(getRoutes());