From f893de413670877aa91af2559a1eee369b4590d6 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 25 Aug 2015 15:05:55 +0200 Subject: [PATCH 1/2] use login name for password reset --- apps/encryption/appinfo/application.php | 3 +- .../controller/settingscontroller.php | 16 +++++++++- .../controller/SettingsControllerTest.php | 32 +++++++++++++------ 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/apps/encryption/appinfo/application.php b/apps/encryption/appinfo/application.php index cba8964eef..75107b2723 100644 --- a/apps/encryption/appinfo/application.php +++ b/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() ); }); diff --git a/apps/encryption/controller/settingscontroller.php b/apps/encryption/controller/settingscontroller.php index 2a668f7cd4..8e6de19e78 100644 --- a/apps/encryption/controller/settingscontroller.php +++ b/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); diff --git a/apps/encryption/tests/controller/SettingsControllerTest.php b/apps/encryption/tests/controller/SettingsControllerTest.php index d985c7d7d2..34aa5a27a7 100644 --- a/apps/encryption/tests/controller/SettingsControllerTest.php +++ b/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 - ->expects($this->once()) - ->method('getUID') - ->willReturn('testUser'); + $this->ocSessionMock->expects($this->once()) + ->method('get')->with('loginname')->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()) From 166e57cf611b659892748fc7799e85a82553b042 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 25 Aug 2015 15:58:07 +0200 Subject: [PATCH 2/2] return false if private key is not valid --- apps/encryption/lib/crypto/crypt.php | 2 +- apps/encryption/tests/lib/crypto/cryptTest.php | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index 6c4c108f50..5d1bb92460 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -398,7 +398,7 @@ class Crypt { } } - return true; + return false; } /** diff --git a/apps/encryption/tests/lib/crypto/cryptTest.php b/apps/encryption/tests/lib/crypto/cryptTest.php index 3c7767a890..c6f16e952d 100644 --- a/apps/encryption/tests/lib/crypto/cryptTest.php +++ b/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']) + ); + } + }