mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Add bruteforce protection to changePersonalPassword 

While the risk is actually quite low because one would already have the user session and could potentially do other havoc it makes sense to throttle here in case of invalid previous password attempts.

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
This commit is contained in:
Lukas Reschke 2017-04-18 17:55:51 +02:00
parent b072d2c49d
commit 805419bb95
No known key found for this signature in database
GPG Key ID: B9F6980CF6E759B1
2 changed files with 35 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
settings/Controller/ChangePasswordController.php
View File

@ -85,6 +85,7 @@ class ChangePasswordController extends Controller {
/**
* @NoAdminRequired
* @NoSubadminRequired
* @BruteForceProtection(action=changePersonalPassword)
*
* @param string $oldpassword
* @param string $newpassword
@ -95,12 +96,14 @@ class ChangePasswordController extends Controller {
/** @var IUser $user */
$user = $this->userManager->checkPassword($this->userId, $oldpassword);
if ($user === false) {
return new JSONResponse([
$response = new JSONResponse([
'status' => 'error',
'data' => [
'message' => $this->l->t('Wrong password'),
],
]);
$response->throttle();
return $response;
}
try {

70
tests/Core/Controller/ChangePasswordControllerTest.php
View File

@ -25,45 +25,40 @@ use OC\HintException;
use OC\Settings\Controller\ChangePasswordController;
use OC\User\Session;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\JSONResponse;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IUserManager;
class ChangePasswordControllerTest extends \Test\TestCase {
/** @var string */
private $userId = 'currentUser';
/** @var IUserManager */
/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
private $userManager;
/** @var Session */
/** @var Session|\PHPUnit_Framework_MockObject_MockObject */
private $userSession;
/** @var IGroupManager */
/** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
private $groupManager;
/** @var IAppManager */
/** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */
private $appManager;
/** @var IL10N */
/** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */
private $l;
/** @var ChangePasswordController */
private $controller;
public function setUp() {
parent::setUp();
$this->userManager = $this->getMockBuilder('OCP\IUserManager')->getMock();
$this->userSession = $this->getMockBuilder('OC\User\Session')->disableOriginalConstructor()->getMock();
$this->groupManager = $this->getMockBuilder('OCP\IGroupManager')->getMock();
$this->appManager = $this->getMockBuilder('OCP\App\IAppManager')->getMock();
$this->l = $this->getMockBuilder('OCP\IL10N')->getMock();
$this->userManager = $this->createMock(IUserManager::class);
$this->userSession = $this->createMock(Session::class);
$this->groupManager = $this->createMock(IGroupManager::class);
$this->appManager = $this->createMock(IAppManager::class);
$this->l = $this->createMock(IL10N::class);
$this->l->method('t')->will($this->returnArgument(0));
$request = $this->getMockBuilder('OCP\IRequest')->getMock();
/** @var IRequest|\PHPUnit_Framework_MockObject_MockObject $request */
$request = $this->createMock(IRequest::class);
$this->controller = new ChangePasswordController(
'core',
@ -83,16 +78,16 @@ class ChangePasswordControllerTest extends \Test\TestCase {
->with($this->userId, 'old')
->willReturn(false);
$expects = [
$expects = new JSONResponse([
'status' => 'error',
'data' => [
'message' => 'Wrong password',
],
];
]);
$expects->throttle();
$res = $this->controller->changePersonalPassword('old', 'new');
$this->assertEquals($expects, $res->getData());
$actual = $this->controller->changePersonalPassword('old', 'new');
$this->assertEquals($expects, $actual);
}
public function testChangePersonalPasswordCommonPassword() {
@ -107,16 +102,15 @@ class ChangePasswordControllerTest extends \Test\TestCase {
->with('new')
->will($this->throwException(new HintException('Common password')));
$expects = [
$expects = new JSONResponse([
'status' => 'error',
'data' => [
'message' => 'Common password',
],
];
]);
$res = $this->controller->changePersonalPassword('old', 'new');
$this->assertEquals($expects, $res->getData());
$actual = $this->controller->changePersonalPassword('old', 'new');
$this->assertEquals($expects, $actual);
}
public function testChangePersonalPasswordNoNewPassword() {
@ -147,13 +141,12 @@ class ChangePasswordControllerTest extends \Test\TestCase {
->with('new')
->willReturn(false);
$expects = [
$expects = new JSONResponse([
'status' => 'error',
];
]);
$res = $this->controller->changePersonalPassword('old', 'new');
$this->assertEquals($expects, $res->getData());
$actual = $this->controller->changePersonalPassword('old', 'new');
$this->assertEquals($expects, $actual);
}
public function testChangePersonalPassword() {
@ -172,15 +165,14 @@ class ChangePasswordControllerTest extends \Test\TestCase {
->method('updateSessionTokenPassword')
->with('new');
$expects = [
$expects = new JSONResponse([
'status' => 'success',
'data' => [
'message' => 'Saved',
],
];
]);
$res = $this->controller->changePersonalPassword('old', 'new');
$this->assertEquals($expects, $res->getData());
$actual = $this->controller->changePersonalPassword('old', 'new');
$this->assertEquals($expects, $actual);
}
}