Merge pull request #18983 from owncloud/fix_18926

check for the right user if we can change his password

apps/encryption/appinfo/application.php

@@ -84,6 +84,7 @@ class Application extends \OCP\AppFramework\App {
 
 			$hookManager->registerHook([
 				new UserHooks($container->query('KeyManager'),
+					$server->getUserManager(),
 					$server->getLogger(),
 					$container->query('UserSetup'),
 					$server->getUserSession(),

apps/encryption/hooks/userhooks.php

@@ -24,6 +24,7 @@
 namespace OCA\Encryption\Hooks;
 
 
+use OCP\IUserManager;
 use OCP\Util as OCUtil;
 use OCA\Encryption\Hooks\Contracts\IHook;
 use OCA\Encryption\KeyManager;
@@ -41,6 +42,10 @@ class UserHooks implements IHook {
 	 * @var KeyManager
 	 */
 	private $keyManager;
+	/**
+	 * @var IUserManager
+	 */
+	private $userManager;
 	/**
 	 * @var ILogger
 	 */
@@ -74,6 +79,7 @@ class UserHooks implements IHook {
 	 * UserHooks constructor.
 	 *
 	 * @param KeyManager $keyManager
+	 * @param IUserManager $userManager
 	 * @param ILogger $logger
 	 * @param Setup $userSetup
 	 * @param IUserSession $user
@@ -83,6 +89,7 @@ class UserHooks implements IHook {
 	 * @param Recovery $recovery
 	 */
 	public function __construct(KeyManager $keyManager,
+								IUserManager $userManager,
 								ILogger $logger,
 								Setup $userSetup,
 								IUserSession $user,
@@ -92,6 +99,7 @@ class UserHooks implements IHook {
 								Recovery $recovery) {
 
 		$this->keyManager = $keyManager;
+		$this->userManager = $userManager;
 		$this->logger = $logger;
 		$this->userSetup = $userSetup;
 		$this->user = $user;
@@ -196,7 +204,7 @@ class UserHooks implements IHook {
 	public function preSetPassphrase($params) {
 		if (App::isEnabled('encryption')) {
 
-			$user = $this->user->getUser();
+			$user = $this->userManager->get($params['uid']);
 
 			if ($user && !$user->canChangePassword()) {
 				$this->setPassphrase($params);

apps/encryption/tests/hooks/UserHooksTest.php

@@ -47,6 +47,11 @@ class UserHooksTest extends TestCase {
 	 * @var \PHPUnit_Framework_MockObject_MockObject
 	 */
 	private $keyManagerMock;
+	/**
+	 * @var \PHPUnit_Framework_MockObject_MockObject
+	 */
+	private $userManagerMock;
+
 	/**
 	 * @var \PHPUnit_Framework_MockObject_MockObject
 	 */
@@ -101,11 +106,58 @@ class UserHooksTest extends TestCase {
 		$this->assertNull($this->instance->postDeleteUser($this->params));
 	}
 
-	public function testPreSetPassphrase() {
+	/**
-		$this->userSessionMock->expects($this->once())
+	 * @dataProvider dataTestPreSetPassphrase
-			->method('canChangePassword');
+	 */
+	public function testPreSetPassphrase($canChange) {
 
-		$this->assertNull($this->instance->preSetPassphrase($this->params));
+		/** @var UserHooks | \PHPUnit_Framework_MockObject_MockObject  $instance */
+		$instance = $this->getMockBuilder('OCA\Encryption\Hooks\UserHooks')
+			->setConstructorArgs(
+				[
+					$this->keyManagerMock,
+					$this->userManagerMock,
+					$this->loggerMock,
+					$this->userSetupMock,
+					$this->userSessionMock,
+					$this->utilMock,
+					$this->sessionMock,
+					$this->cryptMock,
+					$this->recoveryMock
+				]
+			)
+			->setMethods(['setPassphrase'])
+			->getMock();
+
+		$userMock = $this->getMock('OCP\IUser');
+
+		$this->userManagerMock->expects($this->once())
+			->method('get')
+			->with($this->params['uid'])
+			->willReturn($userMock);
+		$userMock->expects($this->once())
+			->method('canChangePassword')
+			->willReturn($canChange);
+
+		if ($canChange) {
+			// in this case the password will be changed in the post hook
+			$instance->expects($this->never())->method('setPassphrase');
+		} else {
+			// if user can't change the password we update the encryption
+			// key password already in the pre hook
+			$instance->expects($this->once())
+				->method('setPassphrase')
+				->with($this->params);
+		}
+
+		$instance->preSetPassphrase($this->params);
+	}
+
+	public function dataTestPreSetPassphrase() {
+		return [
+			[true],
+			[false]
+		];
 	}
 
 	public function testSetPassphrase() {
@@ -186,6 +238,7 @@ class UserHooksTest extends TestCase {
 			->willReturn(false);
 
 		$userHooks = new UserHooks($this->keyManagerMock,
+			$this->userManagerMock,
 			$this->loggerMock,
 			$this->userSetupMock,
 			$userSessionMock,
@@ -216,6 +269,9 @@ class UserHooksTest extends TestCase {
 		$this->keyManagerMock = $this->getMockBuilder('OCA\Encryption\KeyManager')
 			->disableOriginalConstructor()
 			->getMock();
+		$this->userManagerMock = $this->getMockBuilder('OCP\IUserManager')
+			->disableOriginalConstructor()
+			->getMock();
 		$this->userSetupMock = $this->getMockBuilder('OCA\Encryption\Users\Setup')
 			->disableOriginalConstructor()
 			->getMock();
@@ -258,6 +314,7 @@ class UserHooksTest extends TestCase {
 		$this->recoveryMock = $recoveryMock;
 		$this->utilMock = $utilMock;
 		$this->instance = new UserHooks($this->keyManagerMock,
+			$this->userManagerMock,
 			$this->loggerMock,
 			$this->userSetupMock,
 			$this->userSessionMock,