Fix encryption issues after the impersonator logs out

appinfo/info.xml

@@ -13,6 +13,7 @@ As a security measure, the application lets ownCloud administrators restrict the
 	<licence>AGPL</licence>
 	<author>Jörn Friedrich Dreyer, Sujith Haridasan</author>
 	<version>0.5.0</version>
+	<namespace>Impersonate</namespace>
 	<documentation>
 	<admin>https://doc.owncloud.org/server/latest/admin_manual/issues/impersonate_users.html</admin>
 	</documentation>

lib/Util.php

@@ -1,8 +1,9 @@
 <?php
 /**
  * @author Sujith Haridasan <[email protected]>
+ * @author Jannik Stehle <[email protected]>
  *
- * @copyright Copyright (c) 2017, ownCloud GmbH
+ * @copyright Copyright (c) 2021, ownCloud GmbH
  * @license AGPL-3.0
  *
  * This code is free software: you can redistribute it and/or modify
@@ -58,23 +59,51 @@ public function __construct(
 	}
 
 	/**
-	 * Switch from admin/subAdmin $impersonator to $user
+	 * Switch from admin/subAdmin $impersonator to $user (and vice versa).
+	 * $impersonator is empty in case the impersonator switches back to their profile.
+	 * The impersonator must be given via the $user attribute then.
+	 *
 	 * @param IUser $user
 	 * @param mixed $impersonator
 	 */
 	public function switchUser(IUser $user, $impersonator) {
 		$this->tokenProvider->invalidateToken($this->session->getId());
 		$this->userSession->setUser($user);
 		$this->userSession->createSessionToken($this->request, $user->getUID(), $user->getUID());
-		//Store the session var impersonator with the impersonator value
-		if (($this->session->get('impersonator') === null) &&
-			($impersonator !== null)) {
-			$this->session->set('impersonator', $impersonator);
-		}
 
-		//Remove the session variable impersonator set as it is a logout
-		if (($impersonator === '') || ($impersonator === null)) {
+		if ($impersonator !== null) {
+			// Store the session var impersonator with the impersonator value
+			if ($this->session->get('impersonator') === null) {
+				$this->session->set('impersonator', $impersonator);
+			}
+
+			// Save the impersonator's encryption key in another session variable to reset it again later.
+			// This is neccessary because the "privateKey" var gets removed on logout.
+			$encryptionInitialized = $this->session->get('encryptionInitialized');
+			if ($encryptionInitialized !== null) {
+				$this->session->set('impersonatorEncryptionInitialized', $encryptionInitialized);
+			}
+
+			$privateKey = $this->session->get('privateKey');
+			if ($privateKey !== null) {
+				$this->session->set('impersonatorPrivateKey', $privateKey);
+			}
+		} else {
+			// Remove the session variable impersonator set as it is a logout
 			$this->session->remove('impersonator');
+
+			// Reset encryption key for the impersonator.
+			$impersonatorEncryptionInitialized = $this->session->get('impersonatorEncryptionInitialized');
+			if ($impersonatorEncryptionInitialized !== null) {
+				$this->session->set('encryptionInitialized', $impersonatorEncryptionInitialized);
+				$this->session->remove('impersonatorEncryptionInitialized');
+			}
+
+			$impersonatorPrivateKey = $this->session->get('impersonatorPrivateKey');
+			if ($impersonatorPrivateKey !== null) {
+				$this->session->set('privateKey', $impersonatorPrivateKey);
+				$this->session->remove('impersonatorPrivateKey');
+			}
 		}
 	}
 }

tests/lib/UtilTest.php

@@ -1,8 +1,9 @@
 <?php
 /**
  * @author Sujith Haridasan <[email protected]>
+ * @author Jannik Stehle <[email protected]>
  *
- * @copyright Copyright (c) 2017, ownCloud GmbH
+ * @copyright Copyright (c) 2021, ownCloud GmbH
  * @license AGPL-3.0
  *
  * This code is free software: you can redistribute it and/or modify
@@ -30,48 +31,79 @@
 use Test\TestCase;
 
 class UtilTest extends TestCase {
-	/** @var  ISession | \PHPUnit\Framework\MockObject\MockObject */
+	/** @var ISession | \PHPUnit\Framework\MockObject\MockObject */
 	private $session;
-	/** @var  DefaultTokenProvider | \PHPUnit\Framework\MockObject\MockObject */
+	/** @var DefaultTokenProvider | \PHPUnit\Framework\MockObject\MockObject */
 	private $tokenProvider;
-	/** @var  Session | \PHPUnit\Framework\MockObject\MockObject */
+	/** @var Session | \PHPUnit\Framework\MockObject\MockObject */
 	private $userSession;
-	/** @var  IRequest | \PHPUnit\Framework\MockObject\MockObject */
+	/** @var IRequest | \PHPUnit\Framework\MockObject\MockObject */
 	private $request;
-	/** @var  Util */
+	/** @var Util */
 	private $util;
+	/** @var IUser | \PHPUnit\Framework\MockObject\MockObject */
+	private $user;
+
 	public function setUp(): void {
 		$this->session = $this->createMock(ISession::class);
 		$this->tokenProvider = $this->createMock(DefaultTokenProvider::class);
 		$this->userSession = $this->createMock(Session::class);
 		$this->request = $this->createMock(IRequest::class);
+		$this->util = new Util($this->session, $this->userSession, $this->request, $this->tokenProvider);
+		$this->user = $this->createMock(IUser::class);
 		parent::setUp();
 	}
 
-	public function impersonator() {
-		return [
-			[null],
-			['admin']
-			];
+	public function testSwitchUserWithImpersonator() {
+		$impersonator = 'admin';
+		$encryptionInitialized = 1;
+		$privateKey = 'privateKey';
+		$this->session
+			->expects($this->exactly(3))
+			->method('get')
+			->withConsecutive(
+				['impersonator'],
+				['encryptionInitialized'],
+				['privateKey'],
+			)
+			->willReturn(null, $encryptionInitialized, $privateKey);
+		$this->session
+			->expects($this->exactly(3))
+			->method('set')
+			->withConsecutive(
+				['impersonator', $impersonator],
+				['impersonatorEncryptionInitialized', $encryptionInitialized],
+				['impersonatorPrivateKey', $privateKey],
+			);
+		$this->util->switchUser($this->user, $impersonator);
 	}
 
-	/**
-	 * @dataProvider impersonator
-	 * @param $impersonator
-	 */
-	public function testSwitchUser($impersonator) {
-		$user = $this->createMock(IUser::class);
-		$this->util = new Util($this->session, $this->userSession, $this->request, $this->tokenProvider);
-		if ($impersonator !== null) {
-			$this->session
-				->method('get')
-				->willReturn($impersonator);
-		}
-		$this->util->switchUser($user, $impersonator);
-		if ($impersonator === null) {
-			$this->assertEquals($this->session->exists('impersonator'), false);
-		} else {
-			$this->assertEquals($this->session->get('impersonator'), $impersonator);
-		}
+	public function testSwitchUserWithoutImpersonator() {
+		$encryptionInitialized = 1;
+		$privateKey = 'privateKey';
+		$this->session
+			->expects($this->exactly(2))
+			->method('get')
+			->withConsecutive(
+				['impersonatorEncryptionInitialized'],
+				['impersonatorPrivateKey'],
+			)
+			->willReturn($encryptionInitialized, $privateKey);
+		$this->session
+			->expects($this->exactly(2))
+			->method('set')
+			->withConsecutive(
+				['encryptionInitialized', $encryptionInitialized],
+				['privateKey', $privateKey],
+			);
+		$this->session
+			->expects($this->exactly(3))
+			->method('remove')
+			->withConsecutive(
+				['impersonator'],
+				['impersonatorEncryptionInitialized'],
+				['impersonatorPrivateKey'],
+			);
+		$this->util->switchUser($this->user, null);
 	}
 }