From 2a990a0db5199ac842b50b580300bbeb2d2e794c Mon Sep 17 00:00:00 2001
From: Bjoern Schiessle <bjoern@schiessle.org>
Date: Mon, 27 Jun 2016 11:30:13 +0200
Subject: [PATCH] verify user password on change
---
lib/private/User/Database.php | 10 ++++--
settings/ChangePassword/Controller.php | 31 +++++++++++------
settings/js/personal.js | 30 ++++++++++------
settings/templates/personal.php | 3 +-
tests/lib/User/DatabaseTest.php | 47 +++++++++++++++++++++++++-
5 files changed, 95 insertions(+), 26 deletions(-)
diff --git a/lib/private/User/Database.php b/lib/private/User/Database.php
index 1dcac287e1e..85cbddca359 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 1f3ea1b446a..94fb1e4e7a2 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 c9e575afd6b..16a8d184da6 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 e86a84dfa08..716570cad84 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="hidden-visually"><?php echo $l->t('Current password');?>: </label>
<input type="password" id="pass1" name="oldpassword"
diff --git a/tests/lib/User/DatabaseTest.php b/tests/lib/User/DatabaseTest.php
index 270d90b35bc..16275f3b6c5 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'));
+ }
}
--
GitLab