[6.x] Impersonation fixes

CHANGELOG.md

@@ -14,6 +14,7 @@
 - Fixed a bug where bulk entry moves could assign entries to sections that didn’t support their entry types. ([#19267](https://github.com/craftcms/cms/pull/19267))
 - Fixed an issue where disabled and archived user accounts could still authenticate. ([#19265](https://github.com/craftcms/cms/pull/19265))
 - Fixed a [high-severity](https://github.com/craftcms/cms/security/policy#severity--remediation) authorization bypass vulnerability.
+- Fixed a bug where two-factor authentication could lose login state or verify the wrong user during impersonation. ([#19274](https://github.com/craftcms/cms/pull/19274))
 
 ## 6.0.0-alpha.13 - 2026-07-16
 

src/Auth/AuthMethods.php

@@ -33,7 +33,6 @@
 use SensitiveParameter;
 use Webauthn\Exception\InvalidUserHandleException;
 
-use function CraftCms\Cms\currentUser;
 use function CraftCms\Cms\currentUserElement;
 use function CraftCms\Cms\t;
 
@@ -58,7 +57,6 @@ public function __construct(
         private readonly Hasher $hasher,
         private readonly Passkeys $passkeys,
         private readonly ProjectConfig $projectConfig,
-        private readonly Impersonation $impersonation,
     ) {
         $this->methods = new Collection;
     }
@@ -198,15 +196,17 @@ public function getUser(): ?User
         return $this->user;
     }
 
-    public function setUser(?CraftUser $user): void
+    public function setUser(?CraftUser $user, bool $remember = false, ?CraftUser $loginUser = null): void
     {
         $this->user = $user?->asElement();
 
         if ($this->user) {
             Session::put('user.id', $this->user->id);
+            Session::put('user.login_id', ($loginUser ?? $user)->getCraftUserId());
+            Session::put('user.remember', $remember);
             Session::put('user.pending_2fa_at', now()->timestamp);
         } else {
-            Session::forget(['user.id', 'user.pending_2fa_at']);
+            Session::forget(['user.id', 'user.login_id', 'user.remember', 'user.pending_2fa_at']);
         }
     }
 
@@ -353,20 +353,27 @@ public function verifyMethod(string $methodClass, mixed ...$args): bool
 
         // success!
         if ($user) {
-            $this->setUser(null);
+            $user = User::findOne($user->id);
+            $this->authError = $user
+                ? $this->getAuthError($user)
+                : AuthError::InvalidCredentials;
+
+            if ($this->authError) {
+                $this->setUser(null);
 
-            // if we're impersonating, pass the user we're impersonating to the complete the login
-            if ($this->impersonation->isImpersonating()) {
-                $authUser = currentUser();
+                return false;
             }
 
-            $authUser ??= auth()->getProvider()->retrieveById($user->id);
+            $authUser = auth()->getProvider()->retrieveById(Session::get('user.login_id', $user->id));
+            $remember = (bool) Session::get('user.remember', false);
+
+            $this->setUser(null);
 
             if (! $authUser) {
                 return false;
             }
 
-            auth()->login($authUser, true);
+            auth()->login($authUser, $remember);
         }
 
         return true;
@@ -429,9 +436,9 @@ public function getAuthError(CraftUser $user): ?AuthError
     public function getAuthMethodErrorMessage(?string $defaultMessage = null): string
     {
         $user = $this->getUser();
-        $authError = null;
+        $authError = $this->authError;
 
-        if ($user) {
+        if (! $authError && $user) {
             $authError = $this->getAuthError($user);
         }
 

src/Http/Controllers/Auth/AuthenticationController.php

@@ -64,13 +64,16 @@ protected function finalizeLogin(
         CraftUser $user,
         bool $remember,
         bool $skipTwoFactor = false,
+        ?CraftUser $loginUser = null,
     ): Response {
+        $loginUser ??= $user;
+
         if (! $skipTwoFactor && ! $this->generalConfig->disable2fa && $this->auth->hasActiveMethod($user)) {
-            $this->auth->setUser($user);
+            $this->auth->setUser($user, $remember, $loginUser);
 
             if (! $request->isCpRequest() && ! $request->wantsJson()) {
                 if (! $loginPath = $this->generalConfig->getLoginPath()) {
-                    $request->session()->forget('user.id');
+                    $this->auth->setUser(null);
                     throw new RuntimeException('User requires two-step verification, but the loginPath config setting is disabled.');
                 }
 
@@ -83,7 +86,7 @@ protected function finalizeLogin(
             return redirect()->action([TwoFactorAuthenticationController::class, 'showForm']);
         }
 
-        return $this->completeLogin($request, $user, $remember);
+        return $this->completeLogin($request, $loginUser, $remember);
     }
 
     protected function handleLoginFailure(Request $request, ?AuthError $authError = null, ?CraftUser $user = null): Response

src/Http/Controllers/Auth/LoginController.php

@@ -145,12 +145,20 @@ public function attemptLogin(Request $request, Impersonation $impersonation, Log
                 $provider->rehashPasswordIfRequired($user, ['password' => $request->input('password')]);
             }
 
-            // if we're impersonating, pass the user we're impersonating to the complete method
+            $loginUser = $user;
+            $remember = $request->boolean('rememberMe');
+
             if ($impersonation->isImpersonating()) {
-                $user = $request->craftUser() ?? $user;
+                $loginUser = $request->craftUser() ?? $user;
+                $remember = false;
             }
 
-            return $this->finalizeLogin($request, $user, $request->boolean('rememberMe'));
+            return $this->finalizeLogin(
+                $request,
+                $user,
+                $remember,
+                loginUser: $loginUser,
+            );
         }, 30_000);
     }
 

src/Http/Middleware/EnsureTwoFactorChallengeIsRecent.php

@@ -34,7 +34,7 @@ public function handle(Request $request, Closure $next): mixed
             return $next($request);
         }
 
-        Session::forget(['user.id', 'user.pending_2fa_at']);
+        Session::forget(['user.id', 'user.login_id', 'user.remember', 'user.pending_2fa_at']);
 
         if ($request->wantsJson()) {
             return new JsonResponse(['message' => t('Your verification session has expired. Please sign in again.')], 419);

tests/Feature/Http/Controllers/Auth/TwoFactorAuthenticationControllerTest.php

@@ -2,14 +2,20 @@
 
 declare(strict_types=1);
 
+use CraftCms\Cms\Auth\Impersonation;
 use CraftCms\Cms\Auth\Models\Authenticator;
 use CraftCms\Cms\Auth\Models\RecoveryCodes;
 use CraftCms\Cms\Auth\TwoFactorRateLimiter;
+use CraftCms\Cms\Http\Controllers\Auth\LoginController;
 use CraftCms\Cms\Http\Controllers\Auth\TwoFactorAuthenticationController;
 use CraftCms\Cms\User\Elements\User;
+use CraftCms\Cms\User\Models\User as UserModel;
+use CraftCms\Cms\User\Users;
+use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Route;
 use PragmaRX\Google2FA\Google2FA;
 
+use function Pest\Laravel\actingAs;
 use function Pest\Laravel\get;
 use function Pest\Laravel\getJson;
 use function Pest\Laravel\postJson;
@@ -74,6 +80,126 @@
     ])->assertOk();
 });
 
+test('verify rejects a user suspended after the first factor', function () {
+    $user = User::findOne();
+    $secret = (new Google2FA)->generateSecretKey();
+
+    Authenticator::create([
+        'userId' => $user->id,
+        'auth2faSecret' => $secret,
+    ]);
+
+    withSession([
+        'user.id' => $user->id,
+        'user.pending_2fa_at' => now()->timestamp,
+    ]);
+
+    app(Users::class)->suspendUser($user);
+
+    postJson(action([TwoFactorAuthenticationController::class, 'verify']), [
+        'code' => (new Google2FA)->getCurrentOtp($secret),
+    ])
+        ->assertStatus(400)
+        ->assertJsonPath('message', 'Account suspended.');
+
+    expect(Auth::check())->toBeFalse()
+        ->and(session()->has('user.id'))->toBeFalse()
+        ->and(session()->has('user.pending_2fa_at'))->toBeFalse();
+});
+
+test('verify preserves the remember-me choice', function (bool $remember) {
+    $user = User::findOne();
+    $secret = (new Google2FA)->generateSecretKey();
+
+    Authenticator::create([
+        'userId' => $user->id,
+        'auth2faSecret' => $secret,
+    ]);
+
+    postJson(action([LoginController::class, 'attemptLogin']), [
+        'loginName' => $user->email,
+        'password' => 'craftcms2018!!',
+        'rememberMe' => $remember,
+    ])->assertRedirect();
+
+    $response = postJson(action([TwoFactorAuthenticationController::class, 'verify']), [
+        'code' => (new Google2FA)->getCurrentOtp($secret),
+    ])->assertOk();
+
+    $remember
+        ? $response->assertCookie(Auth::guard()->getRecallerName())
+        : $response->assertCookieMissing(Auth::guard()->getRecallerName());
+})->with([
+    'remembered' => true,
+    'not remembered' => false,
+]);
+
+test('impersonation verifies the impersonator and retains the impersonated user', function () {
+    $impersonator = User::findOne();
+    $impersonatedUser = UserModel::factory()->createElement();
+    $impersonatorSecret = (new Google2FA)->generateSecretKey();
+    $impersonatedSecret = (new Google2FA)->generateSecretKey();
+
+    Authenticator::create([
+        'userId' => $impersonator->id,
+        'auth2faSecret' => $impersonatorSecret,
+    ]);
+    Authenticator::create([
+        'userId' => $impersonatedUser->id,
+        'auth2faSecret' => $impersonatedSecret,
+    ]);
+
+    actingAs($impersonatedUser);
+    app(Impersonation::class)->setImpersonatorId($impersonator->id);
+
+    postJson(action([LoginController::class, 'attemptLogin']), [
+        'loginName' => $impersonator->email,
+        'password' => 'craftcms2018!!',
+        'forElevatedSession' => true,
+        'rememberMe' => true,
+    ])->assertRedirect();
+
+    postJson(action([TwoFactorAuthenticationController::class, 'verify']), [
+        'code' => (new Google2FA)->getCurrentOtp($impersonatedSecret),
+    ])->assertStatus(400);
+
+    $response = postJson(action([TwoFactorAuthenticationController::class, 'verify']), [
+        'code' => (new Google2FA)->getCurrentOtp($impersonatorSecret),
+    ])->assertOk();
+
+    expect(Auth::id())->toBe($impersonatedUser->id);
+    $response->assertCookieMissing(Auth::guard()->getRecallerName());
+});
+
+test('impersonation requires the impersonator second factor when the impersonated user has none', function () {
+    $impersonator = User::findOne();
+    $impersonatedUser = UserModel::factory()->createElement();
+    $secret = (new Google2FA)->generateSecretKey();
+
+    Authenticator::create([
+        'userId' => $impersonator->id,
+        'auth2faSecret' => $secret,
+    ]);
+
+    actingAs($impersonatedUser);
+    app(Impersonation::class)->setImpersonatorId($impersonator->id);
+
+    postJson(action([LoginController::class, 'attemptLogin']), [
+        'loginName' => $impersonator->email,
+        'password' => 'craftcms2018!!',
+        'forElevatedSession' => true,
+    ])->assertRedirect();
+
+    expect(session('user.id'))->toBe($impersonator->id)
+        ->and(session('user.login_id'))->toBe($impersonatedUser->id);
+
+    postJson(action([TwoFactorAuthenticationController::class, 'verify']), [
+        'code' => (new Google2FA)->getCurrentOtp($secret),
+    ])->assertOk();
+
+    expect(Auth::id())->toBe($impersonatedUser->id);
+});
+
 test('verifyRecoveryCode returns success with valid recovery code', function () {
     $user = User::findOne();
 

tests/Feature/Http/Middleware/EnsureTwoFactorChallengeIsRecentTest.php

@@ -52,10 +52,14 @@
 test('clears user session keys when challenge has expired', function () {
     $this->withSession([
         'user.id' => 1,
+        'user.login_id' => 2,
+        'user.remember' => true,
         'user.pending_2fa_at' => now()->subSeconds(301)->timestamp,
     ])->get('/admin/test-2fa-recent');
 
     expect(session()->has('user.id'))->toBeFalse()
+        ->and(session()->has('user.login_id'))->toBeFalse()
+        ->and(session()->has('user.remember'))->toBeFalse()
         ->and(session()->has('user.pending_2fa_at'))->toBeFalse();
 });