Merge pull request #18556 from owncloud/fix_password_reset

[encryption] fix password reset for ldap users

apps/encryption/appinfo/application.php

@@ -198,7 +198,8 @@ class Application extends \OCP\AppFramework\App {
 				$server->getUserSession(),
 				$c->query('KeyManager'),
 				$c->query('Crypt'),
-				$c->query('Session')
+				$c->query('Session'),
+				$server->getSession()
 			);
 		});
 

apps/encryption/controller/settingscontroller.php

@@ -31,6 +31,7 @@ use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\IL10N;
 use OCP\IRequest;
+use OCP\ISession;
 use OCP\IUserManager;
 use OCP\IUserSession;
 
@@ -54,6 +55,9 @@ class SettingsController extends Controller {
 	/** @var Session */
 	private $session;
 
+	/** @var ISession  */
+	private $ocSession;
+
 	/**
 	 * @param string $AppName
 	 * @param IRequest $request
@@ -63,6 +67,7 @@ class SettingsController extends Controller {
 	 * @param KeyManager $keyManager
 	 * @param Crypt $crypt
 	 * @param Session $session
+	 * @param ISession $ocSession
 	 */
 	public function __construct($AppName,
 								IRequest $request,
@@ -71,7 +76,8 @@ class SettingsController extends Controller {
 								IUserSession $userSession,
 								KeyManager $keyManager,
 								Crypt $crypt,
-								Session $session) {
+								Session $session,
+								ISession $ocSession) {
 		parent::__construct($AppName, $request);
 		$this->l = $l10n;
 		$this->userSession = $userSession;
@@ -79,6 +85,7 @@ class SettingsController extends Controller {
 		$this->keyManager = $keyManager;
 		$this->crypt = $crypt;
 		$this->session = $session;
+		$this->ocSession = $ocSession;
 	}
 
 
@@ -97,6 +104,13 @@ class SettingsController extends Controller {
 
 		//check if password is correct
 		$passwordCorrect = $this->userManager->checkPassword($uid, $newPassword);
+		if ($passwordCorrect === false) {
+			// if check with uid fails we need to check the password with the login name
+			// e.g. in the ldap case. For local user we need to check the password with
+			// the uid because in this case the login name is case insensitive
+			$loginName = $this->ocSession->get('loginname');
+			$passwordCorrect = $this->userManager->checkPassword($loginName, $newPassword);
+		}
 
 		if ($passwordCorrect !== false) {
 			$encryptedKey = $this->keyManager->getPrivateKey($uid);

apps/encryption/lib/crypto/crypt.php

@@ -398,7 +398,7 @@ class Crypt {
 			}
 		}
 
-		return true;
+		return false;
 	}
 
 	/**

apps/encryption/tests/controller/SettingsControllerTest.php

@@ -54,6 +54,9 @@ class SettingsControllerTest extends TestCase {
 	/** @var \PHPUnit_Framework_MockObject_MockObject */
 	private $sessionMock;
 
+	/** @var  \PHPUnit_Framework_MockObject_MockObject */
+	private $ocSessionMock;
+
 	protected function setUp() {
 
 		parent::setUp();
@@ -91,9 +94,11 @@ class SettingsControllerTest extends TestCase {
 			])
 			->getMock();
 
+		$this->ocSessionMock = $this->getMockBuilder('\OCP\ISession')->disableOriginalConstructor()->getMock();
+
 		$this->userSessionMock->expects($this->any())
 			->method('getUID')
-			->willReturn('testUser');
+			->willReturn('testUserUid');
 
 		$this->userSessionMock->expects($this->any())
 			->method($this->anything())
@@ -110,7 +115,8 @@ class SettingsControllerTest extends TestCase {
 			$this->userSessionMock,
 			$this->keyManagerMock,
 			$this->cryptMock,
-			$this->sessionMock
+			$this->sessionMock,
+			$this->ocSessionMock
 		);
 	}
 
@@ -122,8 +128,10 @@ class SettingsControllerTest extends TestCase {
 		$oldPassword = 'old';
 		$newPassword = 'new';
 
+		$this->userSessionMock->expects($this->once())->method('getUID')->willReturn('uid');
+
 		$this->userManagerMock
-			->expects($this->once())
+			->expects($this->exactly(2))
 			->method('checkPassword')
 			->willReturn(false);
 
@@ -171,16 +179,22 @@ class SettingsControllerTest extends TestCase {
 		$oldPassword = 'old';
 		$newPassword = 'new';
 
-		$this->userSessionMock
+		$this->ocSessionMock->expects($this->once())
-			->expects($this->once())
+			->method('get')->with('loginname')->willReturn('testUser');
-			->method('getUID')
-			->willReturn('testUser');
 
 		$this->userManagerMock
-			->expects($this->once())
+			->expects($this->at(0))
 			->method('checkPassword')
+			->with('testUserUid', 'new')
+			->willReturn(false);
+		$this->userManagerMock
+			->expects($this->at(1))
+			->method('checkPassword')
+			->with('testUser', 'new')
 			->willReturn(true);
 
+
+
 		$this->cryptMock
 			->expects($this->once())
 			->method('decryptPrivateKey')
@@ -200,7 +214,7 @@ class SettingsControllerTest extends TestCase {
 		$this->keyManagerMock
 			->expects($this->once())
 			->method('setPrivateKey')
-			->with($this->equalTo('testUser'), $this->equalTo('header.encryptedKey'));
+			->with($this->equalTo('testUserUid'), $this->equalTo('header.encryptedKey'));
 
 		$this->sessionMock
 			->expects($this->once())

apps/encryption/tests/lib/crypto/cryptTest.php

@@ -363,4 +363,19 @@ class cryptTest extends TestCase {
 		];
 	}
 
+	public function testIsValidPrivateKey() {
+		$res = openssl_pkey_new();
+		openssl_pkey_export($res, $privateKey);
+
+		// valid private key
+		$this->assertTrue(
+			$this->invokePrivate($this->crypt, 'isValidPrivateKey', [$privateKey])
+		);
+
+		// invalid private key
+		$this->assertFalse(
+			$this->invokePrivate($this->crypt, 'isValidPrivateKey', ['foo'])
+		);
+	}
+
 }