diff --git a/lib/private/Server.php b/lib/private/Server.php index 8345a0b66e..ab2e7b1fdf 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -235,7 +235,7 @@ class Server extends ServerContainer implements IServerContainer { } else { $defaultTokenProvider = null; } - + $userSession = new \OC\User\Session($manager, $session, $timeFactory, $defaultTokenProvider, $c->getConfig()); $userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password) { \OC_Hook::emit('OC_User', 'pre_createUser', array('run' => true, 'uid' => $uid, 'password' => $password)); @@ -674,7 +674,8 @@ class Server extends ServerContainer implements IServerContainer { $c->getL10N('core'), $factory, $c->getUserManager(), - $c->getLazyRootFolder() + $c->getLazyRootFolder(), + $c->getEventDispatcher() ); return $manager; diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 2857a394e1..478643e939 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -26,6 +26,7 @@ namespace OC\Share20; use OC\Cache\CappedMemoryCache; use OC\Files\Mount\MoveableMount; +use OC\HintException; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IRootFolder; @@ -42,6 +43,8 @@ use OCP\Share\Exceptions\GenericShareException; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IProviderFactory; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\EventDispatcher\GenericEvent; /** * This class is the communication hub for all sharing related operations. @@ -70,6 +73,8 @@ class Manager implements IManager { private $rootFolder; /** @var CappedMemoryCache */ private $sharingDisabledForUsersCache; + /** @var EventDispatcher */ + private $eventDispatcher; /** @@ -85,6 +90,7 @@ class Manager implements IManager { * @param IProviderFactory $factory * @param IUserManager $userManager * @param IRootFolder $rootFolder + * @param EventDispatcher $eventDispatcher */ public function __construct( ILogger $logger, @@ -96,7 +102,8 @@ class Manager implements IManager { IL10N $l, IProviderFactory $factory, IUserManager $userManager, - IRootFolder $rootFolder + IRootFolder $rootFolder, + EventDispatcher $eventDispatcher ) { $this->logger = $logger; $this->config = $config; @@ -108,6 +115,7 @@ class Manager implements IManager { $this->factory = $factory; $this->userManager = $userManager; $this->rootFolder = $rootFolder; + $this->eventDispatcher = $eventDispatcher; $this->sharingDisabledForUsersCache = new CappedMemoryCache(); } @@ -138,16 +146,11 @@ class Manager implements IManager { } // Let others verify the password - $accepted = true; - $message = ''; - \OCP\Util::emitHook('\OC\Share', 'verifyPassword', [ - 'password' => $password, - 'accepted' => &$accepted, - 'message' => &$message - ]); - - if (!$accepted) { - throw new \Exception($message); + try { + $event = new GenericEvent($password); + $this->eventDispatcher->dispatch('OCP\PasswordPolicy::validate', $event); + } catch (HintException $e) { + throw new \Exception($e->getHint()); } } diff --git a/lib/private/User/Database.php b/lib/private/User/Database.php index 1dcac287e1..85cbddca35 100644 --- a/lib/private/User/Database.php +++ b/lib/private/User/Database.php @@ -51,6 +51,8 @@ namespace OC\User; use OC\Cache\CappedMemoryCache; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\EventDispatcher\GenericEvent; /** * Class for user management in a SQL Database (e.g. MySQL, SQLite) @@ -58,12 +60,14 @@ use OC\Cache\CappedMemoryCache; class Database extends \OC\User\Backend implements \OCP\IUserBackend { /** @var CappedMemoryCache */ private $cache; - + /** @var EventDispatcher */ + private $eventDispatcher; /** * OC_User_Database constructor. */ - public function __construct() { + public function __construct($eventDispatcher = null) { $this->cache = new CappedMemoryCache(); + $this->eventDispatcher = $eventDispatcher ? $eventDispatcher : \OC::$server->getEventDispatcher(); } /** @@ -115,6 +119,8 @@ class Database extends \OC\User\Backend implements \OCP\IUserBackend { */ public function setPassword($uid, $password) { if ($this->userExists($uid)) { + $event = new GenericEvent($password); + $this->eventDispatcher->dispatch('OCP\PasswordPolicy::validate', $event); $query = \OC_DB::prepare('UPDATE `*PREFIX*users` SET `password` = ? WHERE `uid` = ?'); $result = $query->execute(array(\OC::$server->getHasher()->hash($password), $uid)); diff --git a/settings/ChangePassword/Controller.php b/settings/ChangePassword/Controller.php index 1f3ea1b446..94fb1e4e7a 100644 --- a/settings/ChangePassword/Controller.php +++ b/settings/ChangePassword/Controller.php @@ -30,6 +30,8 @@ */ namespace OC\Settings\ChangePassword; +use OC\HintException; + class Controller { public static function changePersonalPassword($args) { // Check if we are an user @@ -39,17 +41,22 @@ class Controller { $username = \OC_User::getUser(); $password = isset($_POST['personal-password']) ? $_POST['personal-password'] : null; $oldPassword = isset($_POST['oldpassword']) ? $_POST['oldpassword'] : ''; + $l = new \OC_L10n('settings'); if (!\OC_User::checkPassword($username, $oldPassword)) { - $l = new \OC_L10n('settings'); \OC_JSON::error(array("data" => array("message" => $l->t("Wrong password")) )); exit(); } - if (!is_null($password) && \OC_User::setPassword($username, $password)) { - \OC::$server->getUserSession()->updateSessionTokenPassword($password); - \OC_JSON::success(); - } else { - \OC_JSON::error(); + + try { + if (!is_null($password) && \OC_User::setPassword($username, $password)) { + \OC::$server->getUserSession()->updateSessionTokenPassword($password); + \OC_JSON::success(['data' => ['message' => $l->t('Saved')]]); + } else { + \OC_JSON::error(); + } + } catch (HintException $e) { + \OC_JSON::error(['data' => ['message' => $e->getHint()]]); } } @@ -150,10 +157,14 @@ class Controller { } } else { // if encryption is disabled, proceed - if (!is_null($password) && \OC_User::setPassword($username, $password)) { - \OC_JSON::success(array('data' => array('username' => $username))); - } else { - \OC_JSON::error(array('data' => array('message' => $l->t('Unable to change password')))); + try { + if (!is_null($password) && \OC_User::setPassword($username, $password)) { + \OC_JSON::success(array('data' => array('username' => $username))); + } else { + \OC_JSON::error(array('data' => array('message' => $l->t('Unable to change password')))); + } + } catch (HintException $e) { + \OC_JSON::error(array('data' => array('message' => $e->getHint()))); } } } diff --git a/settings/js/personal.js b/settings/js/personal.js index c9e575afd6..16a8d184da 100644 --- a/settings/js/personal.js +++ b/settings/js/personal.js @@ -192,6 +192,7 @@ $(document).ready(function () { $('#pass2').showPassword().keyup(); } $("#passwordbutton").click(function () { + OC.msg.startSaving('#password-error-msg'); var isIE8or9 = $('html').hasClass('lte9'); // FIXME - TODO - once support for IE8 and IE9 is dropped // for IE8 and IE9 this will check additionally if the typed in password @@ -208,25 +209,32 @@ $(document).ready(function () { if (data.status === "success") { $('#pass1').val(''); $('#pass2').val('').change(); - // Hide a possible errormsg and show successmsg - $('#password-changed').removeClass('hidden').addClass('inlineblock'); - $('#password-error').removeClass('inlineblock').addClass('hidden'); + OC.msg.finishedSaving('#password-error-msg', data); } else { if (typeof(data.data) !== "undefined") { - $('#password-error').text(data.data.message); + OC.msg.finishedSaving('#password-error-msg', data); } else { - $('#password-error').text(t('Unable to change password')); + OC.msg.finishedSaving('#password-error-msg', + { + 'status' : 'error', + 'data' : { + 'message' : t('core', 'Unable to change password') + } + } + ); } - // Hide a possible successmsg and show errormsg - $('#password-changed').removeClass('inlineblock').addClass('hidden'); - $('#password-error').removeClass('hidden').addClass('inlineblock'); } }); return false; } else { - // Hide a possible successmsg and show errormsg - $('#password-changed').removeClass('inlineblock').addClass('hidden'); - $('#password-error').removeClass('hidden').addClass('inlineblock'); + OC.msg.finishedSaving('#password-error-msg', + { + 'status' : 'error', + 'data' : { + 'message' : t('core', 'Unable to change password') + } + } + ); return false; } diff --git a/settings/templates/personal.php b/settings/templates/personal.php index e86a84dfa0..716570cad8 100644 --- a/settings/templates/personal.php +++ b/settings/templates/personal.php @@ -118,8 +118,7 @@ if($_['passwordChangeSupported']) { ?>

t('Password'));?>

- - +
logger = $this->getMock('\OCP\ILogger'); $this->config = $this->getMock('\OCP\IConfig'); $this->secureRandom = $this->getMock('\OCP\Security\ISecureRandom'); @@ -81,6 +86,7 @@ class ManagerTest extends \Test\TestCase { $this->groupManager = $this->getMock('\OCP\IGroupManager'); $this->userManager = $this->getMock('\OCP\IUserManager'); $this->rootFolder = $this->getMock('\OCP\Files\IRootFolder'); + $this->eventDispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcher'); $this->l = $this->getMock('\OCP\IL10N'); $this->l->method('t') @@ -100,7 +106,8 @@ class ManagerTest extends \Test\TestCase { $this->l, $this->factory, $this->userManager, - $this->rootFolder + $this->rootFolder, + $this->eventDispatcher ); $this->defaultProvider = $this->getMockBuilder('\OC\Share20\DefaultShareProvider') @@ -127,7 +134,8 @@ class ManagerTest extends \Test\TestCase { $this->l, $this->factory, $this->userManager, - $this->rootFolder + $this->rootFolder, + $this->eventDispatcher ]); } @@ -146,7 +154,7 @@ class ManagerTest extends \Test\TestCase { $group = $this->getMock('\OCP\IGroup'); $group->method('getGID')->willReturn('sharedWithGroup'); - + return [ [\OCP\Share::SHARE_TYPE_USER, 'sharedWithUser'], [\OCP\Share::SHARE_TYPE_GROUP, 'sharedWithGroup'], @@ -543,16 +551,12 @@ class ManagerTest extends \Test\TestCase { ['core', 'shareapi_enforce_links_password', 'no', 'no'], ])); - $hookListner = $this->getMockBuilder('Dummy')->setMethods(['listner'])->getMock(); - \OCP\Util::connectHook('\OC\Share', 'verifyPassword', $hookListner, 'listner'); - - $hookListner->expects($this->once()) - ->method('listner') - ->with([ - 'password' => 'password', - 'accepted' => true, - 'message' => '' - ]); + $this->eventDispatcher->expects($this->once())->method('dispatch') + ->willReturnCallback(function($eventName, GenericEvent $event) { + $this->assertSame('OCP\PasswordPolicy::validate', $eventName); + $this->assertSame('password', $event->getSubject()); + } + ); $result = $this->invokePrivate($this->manager, 'verifyPassword', ['password']); $this->assertNull($result); @@ -567,8 +571,14 @@ class ManagerTest extends \Test\TestCase { ['core', 'shareapi_enforce_links_password', 'no', 'no'], ])); - $dummy = new DummyPassword(); - \OCP\Util::connectHook('\OC\Share', 'verifyPassword', $dummy, 'listner'); + $this->eventDispatcher->expects($this->once())->method('dispatch') + ->willReturnCallback(function($eventName, GenericEvent $event) { + $this->assertSame('OCP\PasswordPolicy::validate', $eventName); + $this->assertSame('password', $event->getSubject()); + throw new HintException('message', 'password not accepted'); + } + ); + $this->invokePrivate($this->manager, 'verifyPassword', ['password']); } @@ -2022,7 +2032,8 @@ class ManagerTest extends \Test\TestCase { $this->l, $factory, $this->userManager, - $this->rootFolder + $this->rootFolder, + $this->eventDispatcher ); $share = $this->getMock('\OCP\Share\IShare'); @@ -2054,7 +2065,8 @@ class ManagerTest extends \Test\TestCase { $this->l, $factory, $this->userManager, - $this->rootFolder + $this->rootFolder, + $this->eventDispatcher ); $share = $this->getMock('\OCP\Share\IShare'); @@ -2531,13 +2543,6 @@ class ManagerTest extends \Test\TestCase { } } -class DummyPassword { - public function listner($array) { - $array['accepted'] = false; - $array['message'] = 'password not accepted'; - } -} - class DummyFactory implements IProviderFactory { /** @var IShareProvider */ diff --git a/tests/lib/User/DatabaseTest.php b/tests/lib/User/DatabaseTest.php index 270d90b35b..16275f3b6c 100644 --- a/tests/lib/User/DatabaseTest.php +++ b/tests/lib/User/DatabaseTest.php @@ -21,6 +21,9 @@ */ namespace Test\User; +use OC\HintException; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\EventDispatcher\GenericEvent; /** * Class DatabaseTest @@ -30,6 +33,8 @@ namespace Test\User; class DatabaseTest extends Backend { /** @var array */ private $users; + /** @var EventDispatcher | \PHPUnit_Framework_MockObject_MockObject */ + private $eventDispatcher; public function getUser() { $user = parent::getUser(); @@ -39,7 +44,10 @@ class DatabaseTest extends Backend { protected function setUp() { parent::setUp(); - $this->backend=new \OC\User\Database(); + + $this->eventDispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcher'); + + $this->backend=new \OC\User\Database($this->eventDispatcher); } protected function tearDown() { @@ -51,4 +59,41 @@ class DatabaseTest extends Backend { } parent::tearDown(); } + + public function testVerifyPasswordEvent() { + $user = $this->getUser(); + $this->backend->createUser($user, 'pass1'); + + $this->eventDispatcher->expects($this->once())->method('dispatch') + ->willReturnCallback( + function ($eventName, GenericEvent $event) { + $this->assertSame('OCP\PasswordPolicy::validate', $eventName); + $this->assertSame('newpass', $event->getSubject()); + } + ); + + $this->backend->setPassword($user, 'newpass'); + $this->assertSame($user, $this->backend->checkPassword($user, 'newpass')); + } + + /** + * @expectedException \OC\HintException + * @expectedExceptionMessage password change failed + */ + public function testVerifyPasswordEventFail() { + $user = $this->getUser(); + $this->backend->createUser($user, 'pass1'); + + $this->eventDispatcher->expects($this->once())->method('dispatch') + ->willReturnCallback( + function ($eventName, GenericEvent $event) { + $this->assertSame('OCP\PasswordPolicy::validate', $eventName); + $this->assertSame('newpass', $event->getSubject()); + throw new HintException('password change failed', 'password change failed'); + } + ); + + $this->backend->setPassword($user, 'newpass'); + $this->assertSame($user, $this->backend->checkPassword($user, 'newpass')); + } }