Skip to content

add events to check passwords with the password policy app #221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MorrisJobke merged 3 commits into from
Jun 27, 2016
Merged

add events to check passwords with the password policy app #221

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
630e4b1
check password for link shares
schiessle Jun 24, 2016
d4989c8
remove old hook, no longer needed
schiessle Jun 27, 2016
2a990a0
verify user password on change
schiessle Jun 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
verify user password on change
  • Loading branch information
@schiessle
schiessle committed Jun 27, 2016
commit 2a990a0db5199ac842b50b580300bbeb2d2e794c
10 changes: 8 additions & 2 deletions lib/private/User/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,23 @@
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)
*/
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();
}

/**
Expand Down Expand Up @@ -115,6 +119,8 @@ public function deleteUser($uid) {
*/
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));

Expand Down
31 changes: 21 additions & 10 deletions settings/ChangePassword/Controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
*/
namespace OC\Settings\ChangePassword;

use OC\HintException;

class Controller {
public static function changePersonalPassword($args) {
// Check if we are an user
Expand All @@ -39,17 +41,22 @@ public static function changePersonalPassword($args) {
$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')]]);
Copy link
Member

@LukasReschke LukasReschke Jun 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While touching also move to the modern OCP\JSON approach.

Copy link
Member Author

@schiessle schiessle Jun 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modern?? \OCP\JSON is already deprecated 🙊

} else {
\OC_JSON::error();
}
} catch (HintException $e) {
\OC_JSON::error(['data' => ['message' => $e->getHint()]]);
}
}

Expand Down Expand Up @@ -150,10 +157,14 @@ public static function changeUserPassword($args) {

}
} 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())));
}
}
}
Expand Down
30 changes: 19 additions & 11 deletions settings/js/personal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}

Expand Down
3 changes: 1 addition & 2 deletions settings/templates/personal.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@
?>
<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"
Expand Down
47 changes: 46 additions & 1 deletion tests/lib/User/DatabaseTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
*/

namespace Test\User;
use OC\HintException;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\EventDispatcher\GenericEvent;

/**
* Class DatabaseTest
Expand All @@ -30,6 +33,8 @@
class DatabaseTest extends Backend {
/** @var array */
private $users;
/** @var EventDispatcher | \PHPUnit_Framework_MockObject_MockObject */
private $eventDispatcher;

public function getUser() {
$user = parent::getUser();
Expand All @@ -39,7 +44,10 @@ public function getUser() {

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() {
Expand All @@ -51,4 +59,41 @@ protected function tearDown() {
}
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'));
}
}