Skip to content
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.

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

[stable10] Throw an exception when attempting to set an empty password #32581

Merged
merged 1 commit into from
Sep 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 10 additions & 3 deletions lib/private/User/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* @author Joas Schilling <coding@schilljs.com>
* @author Jörn Friedrich Dreyer <jfd@butonic.de>
* @author Lukas Reschke <lukas@statuscode.ch>
* @author Matthew Setter <matthew@matthewsetter.com>
* @author Michael Gapczynski <GapczynskiM@gmail.com>
* @author Morris Jobke <hey@morrisjobke.de>
* @author nishiki <nishiki@yaegashi.fr>
Expand Down Expand Up @@ -116,14 +117,20 @@ public function deleteUser($uid) {
}

/**
* Set password
* @param string $uid The username
* Change a user's password
*
* @param string $uid The user's username
* @param string $password The new password
* @return bool
*
* Change the password of a user
* @throws \OC\DatabaseException
* @throws \InvalidArgumentException
*/
public function setPassword($uid, $password) {
if (Util::isEmptyString($password)) {
throw new \InvalidArgumentException('Password cannot be empty');
}

if ($this->userExists($uid)) {
$query = \OC_DB::prepare('UPDATE `*PREFIX*users` SET `password` = ? WHERE `uid` = ?');
$result = $query->execute([\OC::$server->getHasher()->hash($password), $uid]);
Expand Down
8 changes: 7 additions & 1 deletion lib/private/User/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@

class User implements IUser {
use EventEmitterTrait;

/** @var Account */
private $account;

Expand Down Expand Up @@ -251,13 +252,18 @@ public function delete() {
}

/**
* Set the password of the user
* Set the user's password
*
* @param string $password
* @param string $recoveryPassword for the encryption app to reset encryption keys
* @return bool
* @throws \InvalidArgumentException
*/
public function setPassword($password, $recoveryPassword = null) {
if (\OCP\Util::isEmptyString($password)) {
throw new \InvalidArgumentException('Password cannot be empty');
}

return $this->emittingCall(function () use (&$password, &$recoveryPassword) {
if ($this->emitter) {
$this->emitter->emit('\OC\User', 'preSetPassword', [$this, $password, $recoveryPassword]);
Expand Down
12 changes: 12 additions & 0 deletions lib/public/Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,18 @@ public static function isValidFileName($file) {
return \OC_Util::isValidFileName($file);
}

/**
* Determine if the string is, strictly-speaking, empty or not
*
* @param $value
* @return bool
* @since 10.0.1
*/
public static function isEmptyString($value) {
$value = \trim(\strval($value));
return ($value === '' || $value === '0');
}

/**
* Generates a cryptographic secure pseudo-random string
* @param int $length of the random string
Expand Down
12 changes: 12 additions & 0 deletions tests/lib/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,18 @@ public function getCurrentUser() {
return $processUser['name'];
}

/**
* @return array A list of items equivalent to an empty values
*/
public function getEmptyValues() {
return [
[''],
[0],
[null],
[false],
];
}

/**
* @param string $string
* @return bool|resource
Expand Down
18 changes: 17 additions & 1 deletion tests/lib/User/DatabaseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* ownCloud
*
* @author Robin Appelman
* @author Matthew Setter <matthew@matthewsetter.com>
* @copyright Copyright (c) 2012 Robin Appelman icewind@owncloud.com
*
* This library is free software; you can redistribute it and/or
Expand All @@ -21,6 +22,7 @@
*/

namespace Test\User;
use Test\Traits\PasswordTrait;

/**
* Class DatabaseTest
Expand All @@ -39,7 +41,7 @@ public function getUser($prefix = 'test_') {

protected function setUp() {
parent::setUp();
$this->backend=new \OC\User\Database();
$this->backend = new \OC\User\Database();
}

protected function tearDown() {
Expand All @@ -52,6 +54,20 @@ protected function tearDown() {
parent::tearDown();
}

/**
* Tests that a \OC\User\ArgumentNotSetException is thrown when the supplied password is empty.
*
* @param string $password
* @dataProvider getEmptyValues
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage Password cannot be empty
*/
public function testCannotSetEmptyPassword($password) {
$user1 = $this->getUser();
$this->backend->createUser($this->getUser(), 'pw');
$this->backend->setPassword($user1, $password);
}

public function testCreateUserInvalidatesCache() {
$user1 = $this->getUser();
$this->assertFalse($this->backend->userExists($user1));
Expand Down
17 changes: 16 additions & 1 deletion tests/lib/User/UserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\EventDispatcher\GenericEvent;
use Test\TestCase;
use Test\Traits\PasswordTrait;

/**
* Class UserTest
Expand All @@ -33,7 +34,6 @@
* @package Test\User
*/
class UserTest extends TestCase {

/** @var AccountMapper | \PHPUnit_Framework_MockObject_MockObject */
private $accountMapper;
/** @var Account */
Expand Down Expand Up @@ -134,6 +134,21 @@ public function testSetPassword() {
$this->assertEquals('bar', $calledEvents['user.aftersetpassword']->getArgument('password'));
$this->assertEquals('', $calledEvents['user.aftersetpassword']->getArgument('recoveryPassword'));
}

/**
* @param string $password
* @dataProvider getEmptyValues
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage Password cannot be empty
* @throws \InvalidArgumentException
*/
public function testSetEmptyPasswordNotPermitted($password) {
(new User(
$this->createMock(Account::class),
$this->accountMapper
))->setPassword($password, 'bar');
}

public function testSetPasswordNotSupported() {
$this->config->expects($this->never())
->method('deleteUserValue')
Expand Down