From 1b05831b07fb97a1db2932bdf3300e9b7dd6befa Mon Sep 17 00:00:00 2001 From: djmaze <> Date: Fri, 4 Nov 2022 10:50:23 +0100 Subject: [PATCH 1/5] Add ImpersonateEvent for SnappyMail --- lib/Controller/LogoutController.php | 12 +++++++++++- lib/Controller/SettingsController.php | 12 +++++++++++- lib/Events/ImpersonateEvent.php | 26 ++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 lib/Events/ImpersonateEvent.php diff --git a/lib/Controller/LogoutController.php b/lib/Controller/LogoutController.php index 02c539d..ce46275 100644 --- a/lib/Controller/LogoutController.php +++ b/lib/Controller/LogoutController.php @@ -10,6 +10,8 @@ use OCP\ISession; use OCP\IUserManager; use OCP\IUserSession; +use OCP\EventDispatcher\IEventDispatcher; +use OCA\Impersonate\Events\ImpersonateEvent; class LogoutController extends Controller { /** @var IUserManager */ @@ -20,6 +22,8 @@ class LogoutController extends Controller { private $logger; /** @var ISession */ private $session; + /** @var IEventDispatcher */ + private $eventDispatcher; /** * @param string $appName @@ -34,12 +38,14 @@ public function __construct($appName, IUserManager $userManager, IUserSession $userSession, ISession $session, - LoggerInterface $logger) { + LoggerInterface $logger, + IEventDispatcher $eventDispatcher) { parent::__construct($appName, $request); $this->userManager = $userManager; $this->userSession = $userSession; $this->session = $session; $this->logger = $logger; + $this->eventDispatcher = $eventDispatcher; } /** @@ -60,6 +66,10 @@ public function logout(string $userId): JSONResponse { ); } + $currentUser = $this->userManager->get($userId); + $this->eventDispatcher->dispatchTyped(new ImpersonateEvent($currentUser, $user)); + $this->userSession->getManager()->emit('\OC\User', 'impersonate', [$currentUser, $user]); + $this->userSession->setUser($user); $this->logger->info( diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 087b3ef..0b4fbdb 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -24,6 +24,8 @@ use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; +use OCP\EventDispatcher\IEventDispatcher; +use OCA\Impersonate\Events\ImpersonateEvent; class SettingsController extends Controller { /** @var IUserManager */ @@ -40,6 +42,8 @@ class SettingsController extends Controller { private $logger; /** @var IL10N */ private $l; + /** @var IEventDispatcher */ + private $eventDispatcher; /** * @param string $appName @@ -60,7 +64,8 @@ public function __construct($appName, ISession $session, IConfig $config, LoggerInterface $logger, - IL10N $l) { + IL10N $l, + IEventDispatcher $eventDispatcher) { parent::__construct($appName, $request); $this->userManager = $userManager; $this->groupManager = $groupManager; @@ -69,6 +74,7 @@ public function __construct($appName, $this->config = $config; $this->logger = $logger; $this->l = $l; + $this->eventDispatcher = $eventDispatcher; } /** @@ -154,6 +160,10 @@ public function impersonate(string $userId): JSONResponse { if ($this->session->get('oldUserId') === null) { $this->session->set('oldUserId', $currentUser->getUID()); } + + $this->eventDispatcher->dispatchTyped(new ImpersonateEvent($currentUser, $user)); + $this->userSession->getManager()->emit('\OC\User', 'impersonate', [$currentUser, $user]); + $this->userSession->setUser($user); return new JSONResponse(); } diff --git a/lib/Events/ImpersonateEvent.php b/lib/Events/ImpersonateEvent.php new file mode 100644 index 0000000..6ab77ec --- /dev/null +++ b/lib/Events/ImpersonateEvent.php @@ -0,0 +1,26 @@ +oldUser = $oldUser; + $this->newUser = $newUser; + } + + public function getOldUser(): IUser { + return $this->oldUser; + } + + public function getNewUser(): IUser { + return $this->newUser; + } +} From 997238d2ba385f42e1f98f7dc08d2d778407700b Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 8 Nov 2022 12:33:34 +0100 Subject: [PATCH 2/5] split event into BeginImpersonateEvent and EndImpersonateEvent Signed-off-by: Arthur Schiwon --- lib/Controller/LogoutController.php | 15 +++--- lib/Controller/SettingsController.php | 42 ++++++--------- lib/Events/AImpersonateEvent.php | 51 +++++++++++++++++++ lib/Events/BeginImpersonateEvent.php | 30 +++++++++++ lib/Events/EndImpersonateEvent.php | 30 +++++++++++ lib/Events/ImpersonateEvent.php | 26 ---------- .../Controller/SettingsControllerTest.php | 31 ++++++++++- 7 files changed, 163 insertions(+), 62 deletions(-) create mode 100644 lib/Events/AImpersonateEvent.php create mode 100644 lib/Events/BeginImpersonateEvent.php create mode 100644 lib/Events/EndImpersonateEvent.php delete mode 100644 lib/Events/ImpersonateEvent.php diff --git a/lib/Controller/LogoutController.php b/lib/Controller/LogoutController.php index ce46275..de9f51d 100644 --- a/lib/Controller/LogoutController.php +++ b/lib/Controller/LogoutController.php @@ -11,7 +11,7 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\EventDispatcher\IEventDispatcher; -use OCA\Impersonate\Events\ImpersonateEvent; +use OCA\Impersonate\Events\EndImpersonateEvent; class LogoutController extends Controller { /** @var IUserManager */ @@ -53,10 +53,10 @@ public function __construct($appName, * @NoAdminRequired */ public function logout(string $userId): JSONResponse { - $user = $this->session->get('oldUserId'); - $user = $this->userManager->get($user); + $impersonator = $this->session->get('oldUserId'); + $impersonator = $this->userManager->get($impersonator); - if ($user === null) { + if ($impersonator === null) { return new JSONResponse( sprintf( 'No user found for %s', @@ -66,11 +66,10 @@ public function logout(string $userId): JSONResponse { ); } - $currentUser = $this->userManager->get($userId); - $this->eventDispatcher->dispatchTyped(new ImpersonateEvent($currentUser, $user)); - $this->userSession->getManager()->emit('\OC\User', 'impersonate', [$currentUser, $user]); + $impersonatee = $this->userManager->get($userId); + $this->eventDispatcher->dispatchTyped(new EndImpersonateEvent($impersonator, $impersonatee)); - $this->userSession->setUser($user); + $this->userSession->setUser($impersonator); $this->logger->info( sprintf( diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 0b4fbdb..1011b72 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -25,7 +25,7 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\EventDispatcher\IEventDispatcher; -use OCA\Impersonate\Events\ImpersonateEvent; +use OCA\Impersonate\Events\BeginImpersonateEvent; class SettingsController extends Controller { /** @var IUserManager */ @@ -45,18 +45,7 @@ class SettingsController extends Controller { /** @var IEventDispatcher */ private $eventDispatcher; - /** - * @param string $appName - * @param IRequest $request - * @param IUserManager $userManager - * @param IGroupManager $groupManager - * @param IUserSession $userSession - * @param ISession $session - * @param IConfig $config - * @param LoggerInterface $logger - * @param IL10N $l - */ - public function __construct($appName, + public function __construct(string $appName, IRequest $request, IUserManager $userManager, IGroupManager $groupManager, @@ -82,13 +71,13 @@ public function __construct($appName, * @NoAdminRequired */ public function impersonate(string $userId): JSONResponse { - /** @var IUser $currentUser */ - $currentUser = $this->userSession->getUser(); + /** @var IUser $impersonator */ + $impersonator = $this->userSession->getUser(); $this->logger->warning( sprintf( 'User %s trying to impersonate user %s', - $currentUser->getUID(), + $impersonator->getUID(), $userId ), [ @@ -96,8 +85,8 @@ public function impersonate(string $userId): JSONResponse { ] ); - $user = $this->userManager->get($userId); - if ($user === null) { + $impersonatee = $this->userManager->get($userId); + if ($impersonatee === null) { return new JSONResponse( [ 'message' => $this->l->t('User not found'), @@ -106,8 +95,8 @@ public function impersonate(string $userId): JSONResponse { ); } - if (!$this->groupManager->isAdmin($currentUser->getUID()) - && !$this->groupManager->getSubAdmin()->isUserAccessible($currentUser, $user)) { + if (!$this->groupManager->isAdmin($impersonator->getUID()) + && !$this->groupManager->getSubAdmin()->isUserAccessible($impersonator, $impersonatee)) { return new JSONResponse( [ 'message' => $this->l->t('Insufficient permissions to impersonate user'), @@ -118,7 +107,7 @@ public function impersonate(string $userId): JSONResponse { $authorized = json_decode($this->config->getAppValue('impersonate', 'authorized', '["admin"]')); if (!empty($authorized)) { - $userGroups = $this->groupManager->getUserGroupIds($currentUser); + $userGroups = $this->groupManager->getUserGroupIds($impersonator); if (!array_intersect($userGroups, $authorized)) { return new JSONResponse( @@ -130,7 +119,7 @@ public function impersonate(string $userId): JSONResponse { } } - if ($user->getLastLogin() === 0) { + if ($impersonatee->getLastLogin() === 0) { return new JSONResponse( [ 'message' => $this->l->t('Cannot impersonate the user because it was never logged in'), @@ -139,7 +128,7 @@ public function impersonate(string $userId): JSONResponse { ); } - if ($user->getUID() === $currentUser->getUID()) { + if ($impersonatee->getUID() === $impersonator->getUID()) { return new JSONResponse( [ 'message' => $this->l->t('Cannot impersonate yourself'), @@ -158,13 +147,12 @@ public function impersonate(string $userId): JSONResponse { ] ); if ($this->session->get('oldUserId') === null) { - $this->session->set('oldUserId', $currentUser->getUID()); + $this->session->set('oldUserId', $impersonator->getUID()); } - $this->eventDispatcher->dispatchTyped(new ImpersonateEvent($currentUser, $user)); - $this->userSession->getManager()->emit('\OC\User', 'impersonate', [$currentUser, $user]); + $this->eventDispatcher->dispatchTyped(new BeginImpersonateEvent($impersonator, $impersonatee)); - $this->userSession->setUser($user); + $this->userSession->setUser($impersonatee); return new JSONResponse(); } } diff --git a/lib/Events/AImpersonateEvent.php b/lib/Events/AImpersonateEvent.php new file mode 100644 index 0000000..6c94d24 --- /dev/null +++ b/lib/Events/AImpersonateEvent.php @@ -0,0 +1,51 @@ + + * @author the-djmaze + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Impersonate\Events; + +use OCP\EventDispatcher\Event; +use OCP\IUser; + +abstract class AImpersonateEvent extends Event { + private IUser $impersonator; + + private IUser $impersonatee; + + public function __construct(IUser $impersonator, IUser $impersonatee) { + parent::__construct(); + $this->impersonator = $impersonator; + $this->impersonatee = $impersonatee; + } + + public function getImpersonator(): IUser { + return $this->impersonator; + } + + public function getImpersonatedUser(): IUser { + return $this->impersonatee; + } +} diff --git a/lib/Events/BeginImpersonateEvent.php b/lib/Events/BeginImpersonateEvent.php new file mode 100644 index 0000000..357661b --- /dev/null +++ b/lib/Events/BeginImpersonateEvent.php @@ -0,0 +1,30 @@ + + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Impersonate\Events; + +class BeginImpersonateEvent extends AImpersonateEvent { +} diff --git a/lib/Events/EndImpersonateEvent.php b/lib/Events/EndImpersonateEvent.php new file mode 100644 index 0000000..a57e09b --- /dev/null +++ b/lib/Events/EndImpersonateEvent.php @@ -0,0 +1,30 @@ + + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Impersonate\Events; + +class EndImpersonateEvent extends AImpersonateEvent { +} diff --git a/lib/Events/ImpersonateEvent.php b/lib/Events/ImpersonateEvent.php deleted file mode 100644 index 6ab77ec..0000000 --- a/lib/Events/ImpersonateEvent.php +++ /dev/null @@ -1,26 +0,0 @@ -oldUser = $oldUser; - $this->newUser = $newUser; - } - - public function getOldUser(): IUser { - return $this->oldUser; - } - - public function getNewUser(): IUser { - return $this->newUser; - } -} diff --git a/tests/unit/Controller/SettingsControllerTest.php b/tests/unit/Controller/SettingsControllerTest.php index 7168ae6..1f34918 100644 --- a/tests/unit/Controller/SettingsControllerTest.php +++ b/tests/unit/Controller/SettingsControllerTest.php @@ -14,8 +14,10 @@ use OC\Group\Manager; use OC\SubAdmin; use OCA\Impersonate\Controller\SettingsController; +use OCA\Impersonate\Events\BeginImpersonateEvent; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IGroupManager; use OCP\IL10N; @@ -57,6 +59,8 @@ class SettingsControllerTest extends TestCase { private $l; /** @var SettingsController */ private $controller; + /** @var IEventDispatcher|IEventDispatcher&MockObject|MockObject */ + private $eventDispatcher; protected function setUp(): void { parent::setUp(); @@ -79,6 +83,7 @@ protected function setUp(): void { $this->groupManager->expects($this->any()) ->method('getSubAdmin') ->willReturn($this->subadmin); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->controller = new SettingsController( $this->appName, @@ -89,7 +94,8 @@ protected function setUp(): void { $this->session, $this->config, $this->logger, - $this->l + $this->l, + $this->eventDispatcher ); } @@ -104,6 +110,9 @@ public function testImpersonateNotFound() { $this->userSession->expects($this->never()) ->method('setUser'); + $this->eventDispatcher->expects($this->never()) + ->method('dispatchTyped'); + $this->assertEquals( new JSONResponse(['message' => 'User not found'], Http::STATUS_NOT_FOUND), $this->controller->impersonate('notexisting@uid') @@ -161,6 +170,13 @@ public function testAdminImpersonate($query, $uid) { ->with('impersonate', 'authorized', '["admin"]') ->willReturnArgument(2); + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->willReturnCallback(function (BeginImpersonateEvent $event) use ($currentUser, $user) { + $this->assertSame($currentUser, $event->getImpersonator()); + $this->assertSame($user, $event->getImpersonatedUser()); + }); + $this->assertEquals( new JSONResponse(), $this->controller->impersonate($query) @@ -216,6 +232,13 @@ public function testSubAdminImpersonate($query, $uid) { ->with('impersonate', 'authorized', '["admin"]') ->willReturn(json_encode(['admin', 'subadmin'])); + $this->eventDispatcher->expects($this->once()) + ->method('dispatchTyped') + ->willReturnCallback(function (BeginImpersonateEvent $event) use ($currentUser, $user) { + $this->assertSame($currentUser, $event->getImpersonator()); + $this->assertSame($user, $event->getImpersonatedUser()); + }); + $this->assertEquals( new JSONResponse(), $this->controller->impersonate($query) @@ -271,6 +294,9 @@ public function testSubAdminImpersonateNotAllowed($query, $uid) { ->with('impersonate', 'authorized', '["admin"]') ->willReturnArgument(2); + $this->eventDispatcher->expects($this->never()) + ->method('dispatchTyped'); + $this->assertEquals( new JSONResponse(['message' => 'Insufficient permissions to impersonate user'], Http::STATUS_FORBIDDEN), $this->controller->impersonate($query) @@ -316,6 +342,9 @@ public function testSubAdminImpersonateNotAccessible($query, $uid) { ->method('setUser') ->with($user); + $this->eventDispatcher->expects($this->never()) + ->method('dispatchTyped'); + $this->assertEquals( new JSONResponse(['message' => 'Insufficient permissions to impersonate user'], Http::STATUS_FORBIDDEN), $this->controller->impersonate($query) From cc3595186a43e5c66d33197fa3f3f3c55a498252 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 8 Nov 2022 12:39:06 +0100 Subject: [PATCH 3/5] fix phpunit conf Signed-off-by: Arthur Schiwon --- tests/phpunit.xml | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/phpunit.xml b/tests/phpunit.xml index 6c0ce78..cc0da19 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -1,6 +1,5 @@ . - - - - ../../impersonate - - ../../impersonate/tests - - - - - - - From 92b78be6b0f1d341f6e8ba57f624653ca566f435 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sat, 3 Dec 2022 23:13:24 +0100 Subject: [PATCH 4/5] improve naming Signed-off-by: Arthur Schiwon --- lib/Controller/LogoutController.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/Controller/LogoutController.php b/lib/Controller/LogoutController.php index de9f51d..53a4cfa 100644 --- a/lib/Controller/LogoutController.php +++ b/lib/Controller/LogoutController.php @@ -53,8 +53,9 @@ public function __construct($appName, * @NoAdminRequired */ public function logout(string $userId): JSONResponse { - $impersonator = $this->session->get('oldUserId'); - $impersonator = $this->userManager->get($impersonator); + /** @var ?string $impersonatorUid */ + $impersonatorUid = $this->session->get('oldUserId'); + $impersonator = $this->userManager->get($impersonatorUid); if ($impersonator === null) { return new JSONResponse( @@ -66,8 +67,8 @@ public function logout(string $userId): JSONResponse { ); } - $impersonatee = $this->userManager->get($userId); - $this->eventDispatcher->dispatchTyped(new EndImpersonateEvent($impersonator, $impersonatee)); + $impersonatedUser = $this->userManager->get($userId); + $this->eventDispatcher->dispatchTyped(new EndImpersonateEvent($impersonator, $impersonatedUser)); $this->userSession->setUser($impersonator); From 2bb059c19d8f5accb228f56f4d57d110aec127d6 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 27 Feb 2023 17:40:02 +0100 Subject: [PATCH 5/5] fix logout as impersonated user Signed-off-by: Arthur Schiwon --- js/impersonate_logout.js | 8 +++----- lib/Controller/LogoutController.php | 14 ++++++-------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/js/impersonate_logout.js b/js/impersonate_logout.js index 281b8f2..b2327ea 100644 --- a/js/impersonate_logout.js +++ b/js/impersonate_logout.js @@ -14,10 +14,9 @@ document.addEventListener('DOMContentLoaded', function() { } ); - function logoutHandler(userId) { + function logoutHandler() { var promisObj = $.post( - OC.generateUrl('apps/impersonate/logout'), - {userId: userId} + OC.generateUrl('apps/impersonate/logout') ).promise() promisObj.done(function () { @@ -27,7 +26,6 @@ document.addEventListener('DOMContentLoaded', function() { $('#settings ul li:last').on('click', function (event) { event.preventDefault() - var userId = $("#expandDisplayName").text() - logoutHandler(userId) + logoutHandler() }) }) diff --git a/lib/Controller/LogoutController.php b/lib/Controller/LogoutController.php index 53a4cfa..44ec922 100644 --- a/lib/Controller/LogoutController.php +++ b/lib/Controller/LogoutController.php @@ -52,30 +52,28 @@ public function __construct($appName, * @UseSession * @NoAdminRequired */ - public function logout(string $userId): JSONResponse { + public function logout(): JSONResponse { /** @var ?string $impersonatorUid */ $impersonatorUid = $this->session->get('oldUserId'); $impersonator = $this->userManager->get($impersonatorUid); if ($impersonator === null) { return new JSONResponse( - sprintf( - 'No user found for %s', - $userId - ), + 'No impersonating user found.', Http::STATUS_NOT_FOUND ); } - $impersonatedUser = $this->userManager->get($userId); + $impersonatedUser = $this->userSession->getUser(); + $this->eventDispatcher->dispatchTyped(new EndImpersonateEvent($impersonator, $impersonatedUser)); $this->userSession->setUser($impersonator); $this->logger->info( sprintf( - 'Switching back to previous user %s', - $userId + 'Switching back to previous user %s from user %s', + $impersonatorUid, $impersonatedUser->getUID() ), [ 'app' => 'impersonate',