Merge pull request #240 from nextcloud/password_policy_events_stable9

[stable9] add events to check passwords with the password policy app

8 files changed, 138 insertions, 63 deletions

diff --git a/lib/private/server.php b/lib/private/server.php
index 581a2b44cea..7d4b02fefee 100644
--- a/lib/private/server.php
+++ b/lib/private/server.php
@@ -618,7 +618,8 @@ class Server extends ServerContainer implements IServerContainer {
 				$c->getL10N('core'),
 				$factory,
 				$c->getUserManager(),
-				$c->getRootFolder()
+				$c->getRootFolder(),
+				$c->getEventDispatcher()
 			);
 
 			return $manager;
diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php
index 35a019b2334..5d53d0ef839 100644
--- a/lib/private/share20/manager.php
+++ b/lib/private/share20/manager.php
@@ -24,6 +24,7 @@
 namespace OC\Share20;
 
 use OC\Files\Mount\MoveableMount;
+use OC\HintException;
 use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
 use OCP\IUserManager;
@@ -42,6 +43,8 @@ use OCP\Files\Folder;
 
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\Exceptions\GenericShareException;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\EventDispatcher\GenericEvent;
 
 /**
  * This class is the communication hub for all sharing related operations.
@@ -82,6 +85,7 @@ class Manager implements IManager {
 	 * @param IProviderFactory $factory
 	 * @param IUserManager $userManager
 	 * @param IRootFolder $rootFolder
+	 * @param EventDispatcher $eventDispatcher
 	 */
 	public function __construct(
 			ILogger $logger,
@@ -93,7 +97,8 @@ class Manager implements IManager {
 			IL10N $l,
 			IProviderFactory $factory,
 			IUserManager $userManager,
-			IRootFolder $rootFolder
+			IRootFolder $rootFolder,
+			EventDispatcher $eventDispatcher
 	) {
 		$this->logger = $logger;
 		$this->config = $config;
@@ -105,6 +110,7 @@ class Manager implements IManager {
 		$this->factory = $factory;
 		$this->userManager = $userManager;
 		$this->rootFolder = $rootFolder;
+		$this->eventDispatcher = $eventDispatcher;
 	}
 
 	/**
@@ -134,16 +140,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 fd273055ae1..4622be1657d 100644
--- a/lib/private/user/database.php
+++ b/lib/private/user/database.php
@@ -49,6 +49,8 @@
  */
 
 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)
@@ -59,9 +61,12 @@ class OC_User_Database extends OC_User_Backend implements \OCP\IUserBackend {
 
 	/**
 	 * OC_User_Database constructor.
+	 *
+	 * @param EventDispatcher $eventDispatcher
 	 */
-	public function __construct() {
+	public function __construct(EventDispatcher $eventDispatcher = null) {
 		$this->cache = new CappedMemoryCache();
+		$this->eventDispatcher = $eventDispatcher ? $eventDispatcher : \OC::$server->getEventDispatcher();
 	}
 
 	/**
@@ -113,6 +118,8 @@ class OC_User_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 8469ec1423a..7484d3a170a 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,16 +41,21 @@ 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_JSON::success();
-		} else {
-			\OC_JSON::error();
+
+		try {
+			if (!is_null($password) && \OC_User::setPassword($username, $password)) {
+				\OC_JSON::success(['data' => ['message' => $l->t('Saved')]]);
+			} else {
+				\OC_JSON::error();
+			}
+		} catch (HintException $e) {
+			\OC_JSON::error(['data' => ['message' => $e->getHint()]]);
 		}
 	}
 
@@ -144,15 +151,19 @@ class Controller {
 				} elseif (!$result && !$recoveryEnabledForUser) {
 					\OC_JSON::error(array("data" => array( "message" => $l->t("Unable to change password" ) )));
 				} else {
-					\OC_JSON::success(array("data" => array( "username" => $username )));
+					\OC_JSON::success(array("data" => array("username" => $username, 'message' => $l->t('Saved'))));
 				}
 
 			}
 		} 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, 'message' => $l->t('Saved'))));
+				} 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 93820c49ff7..74f61af0bb6 100644
--- a/settings/js/personal.js
+++ b/settings/js/personal.js
@@ -185,6 +185,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
@@ -201,25 +202,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 ee79ac2392b..0e1d59b010a 100644
--- a/settings/templates/personal.php
+++ b/settings/templates/personal.php
@@ -118,8 +118,7 @@ if($_['passwordChangeSupported']) {
 ?>
 <form id="passwordform" class="section">
 	<h2 class="inlineblock"><?php p($l->t('Password'));?></h2>
-	<div class="hidden icon-checkmark" id="password-changed"></div>
-	<div class="hidden" id="password-error"><?php p($l->t('Unable to change your password'));?></div>
+	<div id="password-error-msg" class="msg success inlineblock" style="display: none;">Saved</div>
 	<br>
 	<label for="pass1" class="onlyInIE8"><?php echo $l->t('Current password');?>: </label>
 	<input type="password" id="pass1" name="oldpassword"
diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php
index 8ebe3d4b55f..ece4ae23847 100644
--- a/tests/lib/share20/managertest.php
+++ b/tests/lib/share20/managertest.php
@@ -20,6 +20,7 @@
  */
 namespace Test\Share20;
 
+use OC\HintException;
 use OCP\Files\IRootFolder;
 use OCP\IUserManager;
 use OCP\Share\Exceptions\ShareNotFound;
@@ -37,6 +38,8 @@ use OCP\Security\ISecureRandom;
 use OCP\Security\IHasher;
 use OCP\Files\Mount\IMountManager;
 use OCP\IGroupManager;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\EventDispatcher\GenericEvent;
 
 /**
  * Class ManagerTest
@@ -70,9 +73,11 @@ class ManagerTest extends \Test\TestCase {
 	protected $userManager;
 	/** @var IRootFolder | \PHPUnit_Framework_MockObject_MockObject */
 	protected $rootFolder;
+	/** @var  EventDispatcher | \PHPUnit_Framework_MockObject_MockObject */
+	protected $eventDispatcher;
 
 	public function setUp() {
-		
+
 		$this->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,17 +551,13 @@ 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,15 @@ 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 +2033,8 @@ class ManagerTest extends \Test\TestCase {
 			$this->l,
 			$factory,
 			$this->userManager,
-			$this->rootFolder
+			$this->rootFolder,
+			$this->eventDispatcher
 		);
 
 		$share = $this->getMock('\OCP\Share\IShare');
@@ -2465,13 +2477,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 */
@@ -2503,4 +2508,4 @@ class DummyFactory implements IProviderFactory {
 	public function getProviderForType($shareType) {
 		return $this->provider;
 	}
-}
\ No newline at end of file
+}
diff --git a/tests/lib/user/database.php b/tests/lib/user/database.php
index ba44d333a8c..e239ea9c3e4 100644
--- a/tests/lib/user/database.php
+++ b/tests/lib/user/database.php
@@ -19,6 +19,9 @@
 * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 *
 */
+use OC\HintException;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\EventDispatcher\GenericEvent;
 
 /**
  * Class Test_User_Database
@@ -28,6 +31,8 @@
 class Test_User_Database extends Test_User_Backend {
 	/** @var array */
 	private $users;
+	/** @var  EventDispatcher | PHPUnit_Framework_MockObject_MockObject */
+	private $eventDispatcher;
 
 	public function getUser() {
 		$user = parent::getUser();
@@ -37,7 +42,8 @@ class Test_User_Database extends Test_User_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() {
@@ -49,4 +55,41 @@ class Test_User_Database extends Test_User_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'));
+	}
 }