From 19cf6bfd7966a44a4416470c00e1b0abc41fd34b Mon Sep 17 00:00:00 2001 From: Iwona Just Date: Fri, 8 Nov 2024 09:01:06 +0000 Subject: [PATCH 1/3] require elevated session to impersonate and get impersonation url --- src/controllers/UsersController.php | 2 ++ src/elements/User.php | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/controllers/UsersController.php b/src/controllers/UsersController.php index 5111e1ed784..0c1cec7ea83 100644 --- a/src/controllers/UsersController.php +++ b/src/controllers/UsersController.php @@ -340,6 +340,7 @@ private function _findLoginUser(string $loginName): ?User public function actionImpersonate(): ?Response { $this->requirePostRequest(); + $this->requireElevatedSession(); $userSession = Craft::$app->getUser(); $userId = $this->request->getRequiredBodyParam('userId'); @@ -378,6 +379,7 @@ public function actionImpersonate(): ?Response public function actionGetImpersonationUrl(): Response { $this->requirePostRequest(); + $this->requireElevatedSession(); $userId = $this->request->getBodyParam('userId'); $user = Craft::$app->getUsers()->getUserById($userId); diff --git a/src/elements/User.php b/src/elements/User.php index e98fc072d23..d5e39ac1b96 100644 --- a/src/elements/User.php +++ b/src/elements/User.php @@ -567,7 +567,7 @@ public static function findIdentity($id): ?self return $user; } - // If the current user is being impersonated by an admin, ignore their status + // If the current user is being impersonated, ignore their status if ($previousUserId = Session::get(self::IMPERSONATE_KEY)) { /** @var self|null $previousUser */ $previousUser = self::find() @@ -1931,6 +1931,7 @@ protected function safeActionMenuItems(): array 'params' => [ 'userId' => $this->id, ], + 'requireElevatedSession' => true, ]; $copyImpersonationUrlId = sprintf('action-copy-impersonation-url-%s', mt_rand()); @@ -1942,13 +1943,15 @@ protected function safeActionMenuItems(): array $view->registerJsWithVars(fn($id, $userId, $message) => << { - Craft.sendActionRequest('POST', 'users/get-impersonation-url', { - data: {userId: $userId}, - }).then((response) => { - Craft.ui.createCopyTextPrompt({ - label: $message, - value: response.data.url, - }); + Craft.elevatedSessionManager.requireElevatedSession(() => { + Craft.sendActionRequest('POST', 'users/get-impersonation-url', { + data: {userId: $userId}, + }).then((response) => { + Craft.ui.createCopyTextPrompt({ + label: $message, + value: response.data.url, + }); + }); }); }); JS, [ From 57f2fad0b4661f2929fa694d6fbb6c26977ad590 Mon Sep 17 00:00:00 2001 From: Iwona Just Date: Fri, 8 Nov 2024 09:02:43 +0000 Subject: [PATCH 2/3] when elevating session while impersonating, require original user's creds --- src/controllers/UsersController.php | 38 ++++++++++++++++++++++++++--- src/services/Auth.php | 8 ++++++ src/web/assets/cp/CpAsset.php | 12 ++++++++- 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/src/controllers/UsersController.php b/src/controllers/UsersController.php index 0c1cec7ea83..489020b5ac2 100644 --- a/src/controllers/UsersController.php +++ b/src/controllers/UsersController.php @@ -243,6 +243,11 @@ public function actionLogin(): ?Response return $this->runAction('auth-form'); } + // if we're impersonating, pass the user we're impersonating to the complete method + if (Session::get(User::IMPERSONATE_KEY) !== null) { + $user = Craft::$app->getUser()->getIdentity() ?? $user; + } + return $this->_completeLogin($user, $duration); } @@ -277,6 +282,11 @@ public function actionLoginWithPasskey(): ?Response return $this->_handleLoginFailure($user->authError, $user); } + // if we're impersonating, pass the user we're impersonating to the complete method + if (Session::get(User::IMPERSONATE_KEY) !== null) { + $user = Craft::$app->getUser()->getIdentity(); + } + return $this->_completeLogin($user, $duration); } @@ -506,10 +516,23 @@ public function actionLoginModal(): Response $this->requirePostRequest(); $this->requireCpRequest(); + $forElevatedSession = (bool)$this->request->getBodyParam('forElevatedSession'); + + // If the current user is being impersonated get the "original" user instead + if ($forElevatedSession && $previousUserId = Session::get(User::IMPERSONATE_KEY)) { + /** @var User $user */ + $user = User::find() + ->id($previousUserId) + ->one(); + $staticEmail = $user->email; + } else { + $staticEmail = $this->request->getRequiredBodyParam('email'); + } + $view = $this->getView(); $html = $view->renderTemplate('_special/login-modal.twig', [ - 'staticEmail' => $this->request->getRequiredBodyParam('email'), - 'forElevatedSession' => (bool)$this->request->getBodyParam('forElevatedSession'), + 'staticEmail' => $staticEmail, + 'forElevatedSession' => $forElevatedSession, ]); return $this->asJson([ @@ -2321,7 +2344,16 @@ private function _handleLoginFailure(?string $authError = null, ?User $user = nu public function actionAuthForm(): Response { - $activeMethods = Craft::$app->getAuth()->getActiveMethods(); + $user = null; + // If the current user is being impersonated get the "original" user instead + if ($previousUserId = Session::get(User::IMPERSONATE_KEY)) { + /** @var User $user */ + $user = User::find() + ->id($previousUserId) + ->one(); + } + + $activeMethods = Craft::$app->getAuth()->getActiveMethods($user); $methodClass = $this->request->getParam('method'); if ($methodClass) { diff --git a/src/services/Auth.php b/src/services/Auth.php index 466fe2a8739..e585431ac5c 100644 --- a/src/services/Auth.php +++ b/src/services/Auth.php @@ -20,6 +20,7 @@ use craft\helpers\Component as ComponentHelper; use craft\helpers\DateTimeHelper; use craft\helpers\Json; +use craft\helpers\Session as SessionHelper; use craft\models\UserGroup; use craft\records\WebAuthn as WebAuthnRecord; use craft\web\Session; @@ -197,6 +198,13 @@ public function verify(string $methodClass, mixed ...$args): bool // success! if ($user) { $this->setUser(null); + + // if we're impersonating, pass the user we're impersonating to the complete the login + if (SessionHelper::get(User::IMPERSONATE_KEY) !== null) { + /** @var User $user */ + $user = Craft::$app->getUser()->getIdentity(); + } + Craft::$app->getUser()->login($user, $sessionDuration); } diff --git a/src/web/assets/cp/CpAsset.php b/src/web/assets/cp/CpAsset.php index d9c953149eb..39c7c086a32 100644 --- a/src/web/assets/cp/CpAsset.php +++ b/src/web/assets/cp/CpAsset.php @@ -18,6 +18,7 @@ use craft\helpers\DateTimeHelper; use craft\helpers\Html; use craft\helpers\Json; +use craft\helpers\Session; use craft\helpers\StringHelper; use craft\helpers\UrlHelper; use craft\i18n\Locale; @@ -512,6 +513,15 @@ private function _craftData(): array ]; } + $impersonator = null; + // if we're impersonating, we need to check if the original user has passkey + if ($previousUserId = Session::get(User::IMPERSONATE_KEY)) { + /** @var User|null $impersonator */ + $impersonator = User::find() + ->id($previousUserId) + ->one(); + } + $data += [ 'allowAdminChanges' => $generalConfig->allowAdminChanges, 'allowUpdates' => $generalConfig->allowUpdates, @@ -547,7 +557,7 @@ private function _craftData(): array 'siteToken' => $generalConfig->siteToken, 'slugWordSeparator' => $generalConfig->slugWordSeparator, 'userEmail' => $currentUser->email, - 'userHasPasskeys' => Craft::$app->getAuth()->hasPasskeys($currentUser), + 'userHasPasskeys' => Craft::$app->getAuth()->hasPasskeys($impersonator ?? $currentUser), 'userIsAdmin' => $currentUser->admin, 'username' => $currentUser->username, ]; From f13692217c379ad0390a11e0d4db8bcfcefbd236 Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Fri, 8 Nov 2024 16:54:01 -0800 Subject: [PATCH 3/3] Release notes [ci skip] --- CHANGELOG-WIP.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG-WIP.md b/CHANGELOG-WIP.md index 1bdd0c9c77e..3e012e1c69f 100644 --- a/CHANGELOG-WIP.md +++ b/CHANGELOG-WIP.md @@ -40,6 +40,8 @@ - It’s now possible to control whether entry types’ Title fields are required. ([#15942](https://github.com/craftcms/cms/pull/15942)) - Added the “Step Size” Number field setting. - Added the “Default View Mode” element source setting. ([#15824](https://github.com/craftcms/cms/pull/15824)) +- User impersonation now requires an elevated session. ([#16052](https://github.com/craftcms/cms/pull/16052)) +- Elevated session prompts now authenticate against the original user, when impersonating a user. ([#16052](https://github.com/craftcms/cms/pull/16052)) - Added several new icons. - Added `pc/*` commands as an alias of `project-config/*`. - Added the `resave/all` command.