[WIP] Use mail for encrypting the password reset token as well

This commit is contained in:
Lukas Reschke 2016-08-28 14:22:29 +02:00 committed by Roeland Jago Douma
parent b129adfb58
commit 6d686c213b
No known key found for this signature in database
GPG Key ID: 1E152838F164D13B
3 changed files with 75 additions and 12 deletions

View File

@ -44,6 +44,7 @@ class Application extends App {
parent::__construct('core'); parent::__construct('core');
$container = $this->getContainer(); $container = $this->getContainer();
$container->registerService('defaultMailAddress', function() { $container->registerService('defaultMailAddress', function() {
return Util::getDefaultEmailAddress('lostpassword-noreply'); return Util::getDefaultEmailAddress('lostpassword-noreply');
}); });

View File

@ -40,6 +40,7 @@ use \OCP\IL10N;
use \OCP\IConfig; use \OCP\IConfig;
use OCP\IUserManager; use OCP\IUserManager;
use OCP\Mail\IMailer; use OCP\Mail\IMailer;
use OCP\Security\ICrypto;
use OCP\Security\ISecureRandom; use OCP\Security\ISecureRandom;
/** /**
@ -71,6 +72,8 @@ class LostController extends Controller {
protected $mailer; protected $mailer;
/** @var ITimeFactory */ /** @var ITimeFactory */
protected $timeFactory; protected $timeFactory;
/** @var ICrypto */
protected $crypto;
/** /**
* @param string $appName * @param string $appName
@ -85,6 +88,7 @@ class LostController extends Controller {
* @param IManager $encryptionManager * @param IManager $encryptionManager
* @param IMailer $mailer * @param IMailer $mailer
* @param ITimeFactory $timeFactory * @param ITimeFactory $timeFactory
* @param ICrypto $crypto
*/ */
public function __construct($appName, public function __construct($appName,
IRequest $request, IRequest $request,
@ -97,7 +101,8 @@ class LostController extends Controller {
$defaultMailAddress, $defaultMailAddress,
IManager $encryptionManager, IManager $encryptionManager,
IMailer $mailer, IMailer $mailer,
ITimeFactory $timeFactory) { ITimeFactory $timeFactory,
ICrypto $crypto) {
parent::__construct($appName, $request); parent::__construct($appName, $request);
$this->urlGenerator = $urlGenerator; $this->urlGenerator = $urlGenerator;
$this->userManager = $userManager; $this->userManager = $userManager;
@ -109,6 +114,7 @@ class LostController extends Controller {
$this->config = $config; $this->config = $config;
$this->mailer = $mailer; $this->mailer = $mailer;
$this->timeFactory = $timeFactory; $this->timeFactory = $timeFactory;
$this->crypto = $crypto;
} }
/** /**
@ -150,8 +156,19 @@ class LostController extends Controller {
*/ */
private function checkPasswordResetToken($token, $userId) { private function checkPasswordResetToken($token, $userId) {
$user = $this->userManager->get($userId); $user = $this->userManager->get($userId);
if($user === null) {
throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid'));
}
$splittedToken = explode(':', $this->config->getUserValue($userId, 'core', 'lostpassword', null)); try {
$encryptedToken = $this->config->getUserValue($userId, 'core', 'lostpassword', null);
$mailAddress = !is_null($user->getEMailAddress()) ? $user->getEMailAddress() : '';
$decryptedToken = $this->crypto->decrypt($encryptedToken, $mailAddress.$this->config->getSystemValue('secret'));
} catch (\Exception $e) {
throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid'));
}
$splittedToken = explode(':', $decryptedToken);
if(count($splittedToken) !== 2) { if(count($splittedToken) !== 2) {
throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid'));
} }
@ -249,11 +266,20 @@ class LostController extends Controller {
); );
} }
$token = $this->secureRandom->generate(21, // Generate the token. It is stored encrypted in the database with the
// secret being the users' email address appended with the system secret.
// This makes the token automatically invalidate once the user changes
// their email address.
$token = $this->secureRandom->generate(
21,
ISecureRandom::CHAR_DIGITS. ISecureRandom::CHAR_DIGITS.
ISecureRandom::CHAR_LOWER. ISecureRandom::CHAR_LOWER.
ISecureRandom::CHAR_UPPER); ISecureRandom::CHAR_UPPER
$this->config->setUserValue($user, 'core', 'lostpassword', $this->timeFactory->getTime() .':'. $token); );
$tokenValue = $this->timeFactory->getTime() .':'. $token;
$mailAddress = !is_null($userObject->getEMailAddress()) ? $userObject->getEMailAddress() : '';
$encryptedValue = $this->crypto->encrypt($tokenValue, $mailAddress.$this->config->getSystemValue('secret'));
$this->config->setUserValue($user, 'core', 'lostpassword', $encryptedValue);
$link = $this->urlGenerator->linkToRouteAbsolute('core.lost.resetform', array('userId' => $user, 'token' => $token)); $link = $this->urlGenerator->linkToRouteAbsolute('core.lost.resetform', array('userId' => $user, 'token' => $token));

View File

@ -32,6 +32,7 @@ use OCP\IURLGenerator;
use OCP\IUser; use OCP\IUser;
use OCP\IUserManager; use OCP\IUserManager;
use OCP\Mail\IMailer; use OCP\Mail\IMailer;
use OCP\Security\ICrypto;
use OCP\Security\ISecureRandom; use OCP\Security\ISecureRandom;
use PHPUnit_Framework_MockObject_MockObject; use PHPUnit_Framework_MockObject_MockObject;
@ -66,6 +67,8 @@ class LostControllerTest extends \Test\TestCase {
private $timeFactory; private $timeFactory;
/** @var IRequest */ /** @var IRequest */
private $request; private $request;
/** @var ICrypto */
private $crypto;
protected function setUp() { protected function setUp() {
parent::setUp(); parent::setUp();
@ -107,6 +110,7 @@ class LostControllerTest extends \Test\TestCase {
$this->encryptionManager->expects($this->any()) $this->encryptionManager->expects($this->any())
->method('isEnabled') ->method('isEnabled')
->willReturn(true); ->willReturn(true);
$this->crypto = $this->createMock(ICrypto::class);
$this->lostController = new LostController( $this->lostController = new LostController(
'Core', 'Core',
$this->request, $this->request,
@ -119,23 +123,55 @@ class LostControllerTest extends \Test\TestCase {
'lostpassword-noreply@localhost', 'lostpassword-noreply@localhost',
$this->encryptionManager, $this->encryptionManager,
$this->mailer, $this->mailer,
$this->timeFactory $this->timeFactory,
$this->crypto
); );
} }
public function testResetFormInvalidToken() { public function testResetFormWithNotExistingUser() {
$userId = 'admin'; $userId = 'NotExistingUser';
$token = 'MySecretToken'; $token = 'MySecretToken';
$response = $this->lostController->resetform($token, $userId); $this->userManager
$expectedResponse = new TemplateResponse('core', ->expects($this->once())
->method('get')
->with('NotExistingUser')
->willReturn(null);
$expectedResponse = new TemplateResponse(
'core',
'error', 'error',
[ [
'errors' => [ 'errors' => [
['error' => 'Couldn\'t reset password because the token is invalid'], ['error' => 'Couldn\'t reset password because the token is invalid'],
] ]
], ],
'guest'); 'guest'
$this->assertEquals($expectedResponse, $response); );
$this->assertEquals($expectedResponse, $this->lostController->resetform($token, $userId));
}
public function testResetFormInvalidTokenFormatting() {
$userId = 'admin';
$token = 'MySecretToken';
$user = $this->getMockBuilder('\OCP\IUser')->getMock();
$this->userManager
->expects($this->once())
->method('get')
->with('admin')
->willReturn($user);
$expectedResponse = new TemplateResponse(
'core',
'error',
[
'errors' => [
['error' => 'Couldn\'t reset password because the token is invalid'],
]
],
'guest'
);
$this->assertEquals($expectedResponse, $this->lostController->resetform($token, $userId));
} }
public function testResetFormInvalidTokenMatch() { public function testResetFormInvalidTokenMatch() {