diff options
| -rw-r--r-- | core/Controller/LostController.php | 112 | ||||
| -rw-r--r-- | lib/composer/composer/autoload_classmap.php | 3 | ||||
| -rw-r--r-- | lib/composer/composer/autoload_static.php | 3 | ||||
| -rw-r--r-- | lib/private/Security/VerificationToken/VerificationToken.php | 111 | ||||
| -rw-r--r-- | lib/public/Security/VerificationToken/IVerificationToken.php | 55 | ||||
| -rw-r--r-- | lib/public/Security/VerificationToken/InvalidTokenException.php | 74 | ||||
| -rw-r--r-- | tests/Core/Controller/LostControllerTest.php | 301 | ||||
| -rw-r--r-- | tests/lib/Security/VerificationToken/VerificationTokenTest.php | 272 |
8 files changed, 591 insertions, 340 deletions
diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php index bed96eeec83..78a649cdb6d 100644 --- a/core/Controller/LostController.php +++ b/core/Controller/LostController.php @@ -40,7 +40,6 @@ use OC\Core\Exception\ResetPasswordException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; @@ -54,8 +53,8 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\Mail\IMailer; -use OCP\Security\ICrypto; -use OCP\Security\ISecureRandom; +use OCP\Security\VerificationToken\InvalidTokenException; +use OCP\Security\VerificationToken\IVerificationToken; use function array_filter; use function count; use function reset; @@ -82,67 +81,46 @@ class LostController extends Controller { protected $encryptionManager; /** @var IConfig */ protected $config; - /** @var ISecureRandom */ - protected $secureRandom; /** @var IMailer */ protected $mailer; - /** @var ITimeFactory */ - protected $timeFactory; - /** @var ICrypto */ - protected $crypto; /** @var ILogger */ private $logger; /** @var Manager */ private $twoFactorManager; /** @var IInitialStateService */ private $initialStateService; - - /** - * @param string $appName - * @param IRequest $request - * @param IURLGenerator $urlGenerator - * @param IUserManager $userManager - * @param Defaults $defaults - * @param IL10N $l10n - * @param IConfig $config - * @param ISecureRandom $secureRandom - * @param string $defaultMailAddress - * @param IManager $encryptionManager - * @param IMailer $mailer - * @param ITimeFactory $timeFactory - * @param ICrypto $crypto - */ - public function __construct($appName, - IRequest $request, - IURLGenerator $urlGenerator, - IUserManager $userManager, - Defaults $defaults, - IL10N $l10n, - IConfig $config, - ISecureRandom $secureRandom, - $defaultMailAddress, - IManager $encryptionManager, - IMailer $mailer, - ITimeFactory $timeFactory, - ICrypto $crypto, - ILogger $logger, - Manager $twoFactorManager, - IInitialStateService $initialStateService) { + /** @var IVerificationToken */ + private $verificationToken; + + public function __construct( + $appName, + IRequest $request, + IURLGenerator $urlGenerator, + IUserManager $userManager, + Defaults $defaults, + IL10N $l10n, + IConfig $config, + $defaultMailAddress, + IManager $encryptionManager, + IMailer $mailer, + ILogger $logger, + Manager $twoFactorManager, + IInitialStateService $initialStateService, + IVerificationToken $verificationToken + ) { parent::__construct($appName, $request); $this->urlGenerator = $urlGenerator; $this->userManager = $userManager; $this->defaults = $defaults; $this->l10n = $l10n; - $this->secureRandom = $secureRandom; $this->from = $defaultMailAddress; $this->encryptionManager = $encryptionManager; $this->config = $config; $this->mailer = $mailer; - $this->timeFactory = $timeFactory; - $this->crypto = $crypto; $this->logger = $logger; $this->twoFactorManager = $twoFactorManager; $this->initialStateService = $initialStateService; + $this->verificationToken = $verificationToken; } /** @@ -192,36 +170,14 @@ class LostController extends Controller { * @param string $userId * @throws \Exception */ - protected function checkPasswordResetToken($token, $userId) { - $user = $this->userManager->get($userId); - if ($user === null || !$user->isEnabled()) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); - } - - $encryptedToken = $this->config->getUserValue($userId, 'core', 'lostpassword', null); - if ($encryptedToken === null) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); - } - + protected function checkPasswordResetToken(string $token, string $userId): void { try { - $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) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); - } - - if ($splittedToken[0] < ($this->timeFactory->getTime() - 60 * 60 * 24 * 7) || - $user->getLastLogin() > $splittedToken[0]) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is expired')); - } - - if (!hash_equals($splittedToken[1], $token)) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); + $this->verificationToken->check($token, $this->userManager->get($userId), 'lostpassword'); + } catch (InvalidTokenException $e) { + $error = $e->getCode() === InvalidTokenException::TOKEN_EXPIRED + ? $this->l10n->t('Could not reset password because the token is expired') + : $this->l10n->t('Could not reset password because the token is invalid'); + throw new \Exception($error, (int)$e->getCode(), $e); } } @@ -343,15 +299,7 @@ class LostController extends Controller { // 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_LOWER. - ISecureRandom::CHAR_UPPER - ); - $tokenValue = $this->timeFactory->getTime() .':'. $token; - $encryptedValue = $this->crypto->encrypt($tokenValue, $email . $this->config->getSystemValue('secret')); - $this->config->setUserValue($user->getUID(), 'core', 'lostpassword', $encryptedValue); + $token = $this->verificationToken->create($user, 'lostpassword', $email); $link = $this->urlGenerator->linkToRouteAbsolute('core.lost.resetform', ['userId' => $user->getUID(), 'token' => $token]); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index ea1473f27dc..fa7aa3955a2 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -488,6 +488,8 @@ return array( 'OCP\\Security\\ICrypto' => $baseDir . '/lib/public/Security/ICrypto.php', 'OCP\\Security\\IHasher' => $baseDir . '/lib/public/Security/IHasher.php', 'OCP\\Security\\ISecureRandom' => $baseDir . '/lib/public/Security/ISecureRandom.php', + 'OCP\\Security\\VerificationToken\\IVerificationToken' => $baseDir . '/lib/public/Security/VerificationToken/IVerificationToken.php', + 'OCP\\Security\\VerificationToken\\InvalidTokenException' => $baseDir . '/lib/public/Security/VerificationToken/InvalidTokenException.php', 'OCP\\Session\\Exceptions\\SessionNotAvailableException' => $baseDir . '/lib/public/Session/Exceptions/SessionNotAvailableException.php', 'OCP\\Settings\\IIconSection' => $baseDir . '/lib/public/Settings/IIconSection.php', 'OCP\\Settings\\IManager' => $baseDir . '/lib/public/Settings/IManager.php', @@ -1371,6 +1373,7 @@ return array( 'OC\\Security\\RateLimiting\\Limiter' => $baseDir . '/lib/private/Security/RateLimiting/Limiter.php', 'OC\\Security\\SecureRandom' => $baseDir . '/lib/private/Security/SecureRandom.php', 'OC\\Security\\TrustedDomainHelper' => $baseDir . '/lib/private/Security/TrustedDomainHelper.php', + 'OC\\Security\\VerificationToken\\VerificationToken' => $baseDir . '/lib/private/Security/VerificationToken/VerificationToken.php', 'OC\\Server' => $baseDir . '/lib/private/Server.php', 'OC\\ServerContainer' => $baseDir . '/lib/private/ServerContainer.php', 'OC\\ServerNotAvailableException' => $baseDir . '/lib/private/ServerNotAvailableException.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 93208f6ff15..2f268b92ece 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -517,6 +517,8 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\Security\\ICrypto' => __DIR__ . '/../../..' . '/lib/public/Security/ICrypto.php', 'OCP\\Security\\IHasher' => __DIR__ . '/../../..' . '/lib/public/Security/IHasher.php', 'OCP\\Security\\ISecureRandom' => __DIR__ . '/../../..' . '/lib/public/Security/ISecureRandom.php', + 'OCP\\Security\\VerificationToken\\IVerificationToken' => __DIR__ . '/../../..' . '/lib/public/Security/VerificationToken/IVerificationToken.php', + 'OCP\\Security\\VerificationToken\\InvalidTokenException' => __DIR__ . '/../../..' . '/lib/public/Security/VerificationToken/InvalidTokenException.php', 'OCP\\Session\\Exceptions\\SessionNotAvailableException' => __DIR__ . '/../../..' . '/lib/public/Session/Exceptions/SessionNotAvailableException.php', 'OCP\\Settings\\IIconSection' => __DIR__ . '/../../..' . '/lib/public/Settings/IIconSection.php', 'OCP\\Settings\\IManager' => __DIR__ . '/../../..' . '/lib/public/Settings/IManager.php', @@ -1400,6 +1402,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Security\\RateLimiting\\Limiter' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Limiter.php', 'OC\\Security\\SecureRandom' => __DIR__ . '/../../..' . '/lib/private/Security/SecureRandom.php', 'OC\\Security\\TrustedDomainHelper' => __DIR__ . '/../../..' . '/lib/private/Security/TrustedDomainHelper.php', + 'OC\\Security\\VerificationToken\\VerificationToken' => __DIR__ . '/../../..' . '/lib/private/Security/VerificationToken/VerificationToken.php', 'OC\\Server' => __DIR__ . '/../../..' . '/lib/private/Server.php', 'OC\\ServerContainer' => __DIR__ . '/../../..' . '/lib/private/ServerContainer.php', 'OC\\ServerNotAvailableException' => __DIR__ . '/../../..' . '/lib/private/ServerNotAvailableException.php', diff --git a/lib/private/Security/VerificationToken/VerificationToken.php b/lib/private/Security/VerificationToken/VerificationToken.php new file mode 100644 index 00000000000..4ac5605eecf --- /dev/null +++ b/lib/private/Security/VerificationToken/VerificationToken.php @@ -0,0 +1,111 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2021 Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @author Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <https://www.gnu.org/licenses/>. + * + */ + +namespace OC\Security\VerificationToken; + +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IConfig; +use OCP\IUser; +use OCP\Security\ICrypto; +use OCP\Security\ISecureRandom; +use OCP\Security\VerificationToken\InvalidTokenException; +use OCP\Security\VerificationToken\IVerificationToken; + +class VerificationToken implements IVerificationToken { + + /** @var IConfig */ + private $config; + /** @var ICrypto */ + private $crypto; + /** @var ITimeFactory */ + private $timeFactory; + /** @var ISecureRandom */ + private $secureRandom; + + public function __construct( + IConfig $config, + ICrypto $crypto, + ITimeFactory $timeFactory, + ISecureRandom $secureRandom + ) { + $this->config = $config; + $this->crypto = $crypto; + $this->timeFactory = $timeFactory; + $this->secureRandom = $secureRandom; + } + + /** + * @throws InvalidTokenException + */ + protected function throwInvalidTokenException(int $code): void { + throw new InvalidTokenException($code); + } + + public function check(string $token, ?IUser $user, string $subject, string $passwordPrefix = ''): void { + if ($user === null || !$user->isEnabled()) { + $this->throwInvalidTokenException(InvalidTokenException::USER_UNKNOWN); + } + + $encryptedToken = $this->config->getUserValue($user->getUID(), 'core', $subject, null); + if ($encryptedToken === null) { + $this->throwInvalidTokenException(InvalidTokenException::TOKEN_NOT_FOUND); + } + + try { + $decryptedToken = $this->crypto->decrypt($encryptedToken, $passwordPrefix.$this->config->getSystemValue('secret')); + } catch (\Exception $e) { + $this->throwInvalidTokenException(InvalidTokenException::TOKEN_DECRYPTION_ERROR); + } + + $splitToken = explode(':', $decryptedToken ?? ''); + if (count($splitToken) !== 2) { + $this->throwInvalidTokenException(InvalidTokenException::TOKEN_INVALID_FORMAT); + } + + if ($splitToken[0] < ($this->timeFactory->getTime() - 60 * 60 * 24 * 7) || + $user->getLastLogin() > $splitToken[0]) { + $this->throwInvalidTokenException(InvalidTokenException::TOKEN_EXPIRED); + } + + if (!hash_equals($splitToken[1], $token)) { + $this->throwInvalidTokenException(InvalidTokenException::TOKEN_MISMATCH); + } + } + + public function create(IUser $user, string $subject, string $passwordPrefix = ''): string { + $token = $this->secureRandom->generate( + 21, + ISecureRandom::CHAR_DIGITS. + ISecureRandom::CHAR_LOWER. + ISecureRandom::CHAR_UPPER + ); + $tokenValue = $this->timeFactory->getTime() .':'. $token; + $encryptedValue = $this->crypto->encrypt($tokenValue, $passwordPrefix . $this->config->getSystemValue('secret')); + $this->config->setUserValue($user->getUID(), 'core', $subject, $encryptedValue); + + return $token; + } +} diff --git a/lib/public/Security/VerificationToken/IVerificationToken.php b/lib/public/Security/VerificationToken/IVerificationToken.php new file mode 100644 index 00000000000..12c03178fb6 --- /dev/null +++ b/lib/public/Security/VerificationToken/IVerificationToken.php @@ -0,0 +1,55 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2021 Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @author Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <https://www.gnu.org/licenses/>. + * + */ + +namespace OCP\Security\VerificationToken; + +use OCP\IUser; + +/** + * @since 23.0.0 + */ +interface IVerificationToken { + + /** + * Checks whether the a provided tokent matches a stored token and its + * constraints. An InvalidTokenException is thrown on issues, otherwise + * the check is successful. + * + * null can be passed as $user, but mind that this is for conveniently + * passing the return of IUserManager::getUser() to this method. When + * $user is null, InvalidTokenException is thrown for all the issued + * tokens are user related. + * + * @throws InvalidTokenException + * @since 23.0.0 + */ + public function check(string $token, ?IUser $user, string $subject, string $passwordPrefix = ''): void; + + /** + * @since 23.0.0 + */ + public function create(IUser $user, string $subject, string $passwordPrefix = ''): string; +} diff --git a/lib/public/Security/VerificationToken/InvalidTokenException.php b/lib/public/Security/VerificationToken/InvalidTokenException.php new file mode 100644 index 00000000000..5c8144c5c74 --- /dev/null +++ b/lib/public/Security/VerificationToken/InvalidTokenException.php @@ -0,0 +1,74 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2021 Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @author Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <https://www.gnu.org/licenses/>. + * + */ + +namespace OCP\Security\VerificationToken; + +/** @since 23.0.0 */ +class InvalidTokenException extends \Exception { + + /** + * @since 23.0.0 + */ + public function __construct(int $code) { + parent::__construct('', $code); + } + + /** + * @var int + * @since 23.0.0 + */ + public const USER_UNKNOWN = 1; + + /** + * @var int + * @since 23.0.0 + */ + public const TOKEN_NOT_FOUND = 2; + + /** + * @var int + * @since 23.0.0 + */ + public const TOKEN_DECRYPTION_ERROR = 3; + + /** + * @var int + * @since 23.0.0 + */ + public const TOKEN_INVALID_FORMAT = 4; + + /** + * @var int + * @since 23.0.0 + */ + public const TOKEN_EXPIRED = 5; + + /** + * @var int + * @since 23.0.0 + */ + public const TOKEN_MISMATCH = 6; +} diff --git a/tests/Core/Controller/LostControllerTest.php b/tests/Core/Controller/LostControllerTest.php index fd4e27d47f1..a9dd4c1797b 100644 --- a/tests/Core/Controller/LostControllerTest.php +++ b/tests/Core/Controller/LostControllerTest.php @@ -26,7 +26,6 @@ use OC\Core\Controller\LostController; use OC\Mail\Message; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; @@ -40,8 +39,8 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; -use OCP\Security\ICrypto; -use OCP\Security\ISecureRandom; +use OCP\Security\VerificationToken\InvalidTokenException; +use OCP\Security\VerificationToken\IVerificationToken; /** * Class LostControllerTest @@ -66,22 +65,18 @@ class LostControllerTest extends \Test\TestCase { private $config; /** @var IMailer | \PHPUnit\Framework\MockObject\MockObject */ private $mailer; - /** @var ISecureRandom | \PHPUnit\Framework\MockObject\MockObject */ - private $secureRandom; /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ private $encryptionManager; - /** @var ITimeFactory | \PHPUnit\Framework\MockObject\MockObject */ - private $timeFactory; /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */ private $request; - /** @var ICrypto|\PHPUnit\Framework\MockObject\MockObject */ - private $crypto; /** @var ILogger|\PHPUnit\Framework\MockObject\MockObject */ private $logger; /** @var Manager|\PHPUnit\Framework\MockObject\MockObject */ private $twofactorManager; /** @var IInitialStateService|\PHPUnit\Framework\MockObject\MockObject */ private $initialStateService; + /** @var IVerificationToken|\PHPUnit\Framework\MockObject\MockObject */ + private $verificationToken; protected function setUp(): void { parent::setUp(); @@ -123,10 +118,6 @@ class LostControllerTest extends \Test\TestCase { ->disableOriginalConstructor()->getMock(); $this->mailer = $this->getMockBuilder('\OCP\Mail\IMailer') ->disableOriginalConstructor()->getMock(); - $this->secureRandom = $this->getMockBuilder('\OCP\Security\ISecureRandom') - ->disableOriginalConstructor()->getMock(); - $this->timeFactory = $this->getMockBuilder('\OCP\AppFramework\Utility\ITimeFactory') - ->disableOriginalConstructor()->getMock(); $this->request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor()->getMock(); $this->encryptionManager = $this->getMockBuilder(IManager::class) @@ -134,10 +125,10 @@ class LostControllerTest extends \Test\TestCase { $this->encryptionManager->expects($this->any()) ->method('isEnabled') ->willReturn(true); - $this->crypto = $this->createMock(ICrypto::class); $this->logger = $this->createMock(ILogger::class); $this->twofactorManager = $this->createMock(Manager::class); $this->initialStateService = $this->createMock(IInitialStateService::class); + $this->verificationToken = $this->createMock(IVerificationToken::class); $this->lostController = new LostController( 'Core', $this->request, @@ -146,89 +137,31 @@ class LostControllerTest extends \Test\TestCase { $this->defaults, $this->l10n, $this->config, - $this->secureRandom, 'lostpassword-noreply@localhost', $this->encryptionManager, $this->mailer, - $this->timeFactory, - $this->crypto, $this->logger, $this->twofactorManager, - $this->initialStateService + $this->initialStateService, + $this->verificationToken ); } - public function testResetFormWithNotExistingUser() { - $this->userManager->method('get') - ->with('NotExistingUser') - ->willReturn(null); - - $expectedResponse = new TemplateResponse( - 'core', - 'error', - [ - 'errors' => [ - ['error' => 'Couldn\'t reset password because the token is invalid'], - ] - ], - 'guest' - ); - $this->assertEquals($expectedResponse, $this->lostController->resetform('MySecretToken', 'NotExistingUser')); - } - - public function testResetFormInvalidTokenMatch() { - $this->config->method('getUserValue') - ->with('ValidTokenUser', 'core', 'lostpassword', null) - ->willReturn('encryptedToken'); - $this->existingUser->method('getLastLogin') - ->willReturn(12344); + public function testResetFormTokenError() { $this->userManager->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedToken'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); + $this->verificationToken->expects($this->once()) + ->method('check') + ->with('12345:MySecretToken', $this->existingUser, 'lostpassword') + ->willThrowException(new InvalidTokenException(InvalidTokenException::TOKEN_DECRYPTION_ERROR)); $response = $this->lostController->resetform('12345:MySecretToken', 'ValidTokenUser'); $expectedResponse = new TemplateResponse('core', 'error', [ 'errors' => [ - ['error' => 'Couldn\'t reset password because the token is invalid'], - ] - ], - 'guest'); - $this->assertEquals($expectedResponse, $response); - } - - - public function testResetFormExpiredToken() { - $this->userManager->method('get') - ->with('ValidTokenUser') - ->willReturn($this->existingUser); - $this->config - ->expects($this->once()) - ->method('getUserValue') - ->with('ValidTokenUser', 'core', 'lostpassword', null) - ->willReturn('encryptedToken'); - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedToken'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(999999); - - $response = $this->lostController->resetform('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser'); - $expectedResponse = new TemplateResponse('core', - 'error', - [ - 'errors' => [ - ['error' => 'Couldn\'t reset password because the token is expired'], + ['error' => 'Could not reset password because the token is invalid'], ] ], 'guest'); @@ -236,39 +169,14 @@ class LostControllerTest extends \Test\TestCase { } public function testResetFormValidToken() { - $this->existingUser->method('getLastLogin') - ->willReturn(12344); $this->userManager->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(12348); + $this->verificationToken->expects($this->once()) + ->method('check') + ->with('MySecretToken', $this->existingUser, 'lostpassword'); - $this->config->method('getUserValue') - ->with('ValidTokenUser', 'core', 'lostpassword', null) - ->willReturn('encryptedToken'); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedToken'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); - $this->urlGenerator - ->expects($this->once()) - ->method('linkToRouteAbsolute') - ->with('core.lost.setPassword', ['userId' => 'ValidTokenUser', 'token' => 'TheOnlyAndOnlyOneTokenToResetThePassword']) - ->willReturn('https://example.tld/index.php/lostpassword/'); - - $this->initialStateService->expects($this->at(0)) - ->method('provideInitialState') - ->with('core', 'resetPasswordUser', 'ValidTokenUser'); - $this->initialStateService->expects($this->at(1)) - ->method('provideInitialState') - ->with('core', 'resetPasswordTarget', 'https://example.tld/index.php/lostpassword/'); - - $response = $this->lostController->resetform('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser'); + $response = $this->lostController->resetform('MySecretToken', 'ValidTokenUser'); $expectedResponse = new TemplateResponse('core', 'login', [], @@ -319,24 +227,14 @@ class LostControllerTest extends \Test\TestCase { } public function testEmailSuccessful() { - $this->secureRandom - ->expects($this->once()) - ->method('generate') - ->with('21') - ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->userManager ->expects($this->any()) ->method('get') ->with('ExistingUser') ->willReturn($this->existingUser); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(12348); - $this->config - ->expects($this->once()) - ->method('setUserValue') - ->with('ExistingUser', 'core', 'lostpassword', 'encryptedToken'); + $this->verificationToken->expects($this->once()) + ->method('create') + ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->urlGenerator ->expects($this->once()) ->method('linkToRouteAbsolute') @@ -379,12 +277,6 @@ class LostControllerTest extends \Test\TestCase { ->method('send') ->with($message); - $this->crypto->method('encrypt') - ->with( - $this->equalTo('12348:ThisIsMaybeANotSoSecretToken!'), - $this->equalTo('test@example.comSECRET') - )->willReturn('encryptedToken'); - $response = $this->lostController->email('ExistingUser'); $expectedResponse = new JSONResponse(['status' => 'success']); $expectedResponse->throttle(); @@ -392,11 +284,6 @@ class LostControllerTest extends \Test\TestCase { } public function testEmailWithMailSuccessful() { - $this->secureRandom - ->expects($this->once()) - ->method('generate') - ->with('21') - ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->userManager ->expects($this->any()) ->method('get') @@ -407,14 +294,9 @@ class LostControllerTest extends \Test\TestCase { ->method('getByEmail') ->with('test@example.com') ->willReturn([$this->existingUser]); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(12348); - $this->config - ->expects($this->once()) - ->method('setUserValue') - ->with('ExistingUser', 'core', 'lostpassword', 'encryptedToken'); + $this->verificationToken->expects($this->once()) + ->method('create') + ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->urlGenerator ->expects($this->once()) ->method('linkToRouteAbsolute') @@ -457,12 +339,6 @@ class LostControllerTest extends \Test\TestCase { ->method('send') ->with($message); - $this->crypto->method('encrypt') - ->with( - $this->equalTo('12348:ThisIsMaybeANotSoSecretToken!'), - $this->equalTo('test@example.comSECRET') - )->willReturn('encryptedToken'); - $response = $this->lostController->email('test@example.com'); $expectedResponse = new JSONResponse(['status' => 'success']); $expectedResponse->throttle(); @@ -470,24 +346,14 @@ class LostControllerTest extends \Test\TestCase { } public function testEmailCantSendException() { - $this->secureRandom - ->expects($this->once()) - ->method('generate') - ->with('21') - ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->userManager ->expects($this->any()) ->method('get') ->with('ExistingUser') ->willReturn($this->existingUser); - $this->config - ->expects($this->once()) - ->method('setUserValue') - ->with('ExistingUser', 'core', 'lostpassword', 'encryptedToken'); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(12348); + $this->verificationToken->expects($this->once()) + ->method('create') + ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->urlGenerator ->expects($this->once()) ->method('linkToRouteAbsolute') @@ -530,12 +396,6 @@ class LostControllerTest extends \Test\TestCase { ->with($message) ->will($this->throwException(new \Exception())); - $this->crypto->method('encrypt') - ->with( - $this->equalTo('12348:ThisIsMaybeANotSoSecretToken!'), - $this->equalTo('test@example.comSECRET') - )->willReturn('encryptedToken'); - $this->logger->expects($this->exactly(1)) ->method('logException'); @@ -560,14 +420,6 @@ class LostControllerTest extends \Test\TestCase { ->willReturn($this->existingUser); $this->config->expects($this->never()) ->method('deleteUserValue'); - $this->timeFactory->method('getTime') - ->willReturn(12348); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); $expectedResponse = ['status' => 'error', 'msg' => '']; @@ -590,14 +442,6 @@ class LostControllerTest extends \Test\TestCase { $this->config->expects($this->once()) ->method('deleteUserValue') ->with('ValidTokenUser', 'core', 'lostpassword'); - $this->timeFactory->method('getTime') - ->willReturn(12348); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); $expectedResponse = ['user' => 'ValidTokenUser', 'status' => 'success']; @@ -611,19 +455,14 @@ class LostControllerTest extends \Test\TestCase { $this->userManager->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); - $this->timeFactory->method('getTime') - ->willReturn(617146); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); + $this->verificationToken->expects($this->atLeastOnce()) + ->method('check') + ->willThrowException(new InvalidTokenException(InvalidTokenException::TOKEN_EXPIRED)); $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); $expectedResponse = [ 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is expired', + 'msg' => 'Could not reset password because the token is expired', ]; $this->assertSame($expectedResponse, $response); } @@ -636,45 +475,14 @@ class LostControllerTest extends \Test\TestCase { ->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('invalidEncryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('TheOnlyAndOnlyOneTokenToResetThePassword'); + $this->verificationToken->expects($this->atLeastOnce()) + ->method('check') + ->willThrowException(new InvalidTokenException(InvalidTokenException::TOKEN_INVALID_FORMAT)); $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); $expectedResponse = [ 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is invalid', - ]; - $this->assertSame($expectedResponse, $response); - } - - public function testSetPasswordExpiredTokenDueToLogin() { - $this->config->method('getUserValue') - ->with('ValidTokenUser', 'core', 'lostpassword', null) - ->willReturn('encryptedData'); - $this->existingUser->method('getLastLogin') - ->willReturn(12346); - $this->userManager - ->method('get') - ->with('ValidTokenUser') - ->willReturn($this->existingUser); - $this->timeFactory - ->method('getTime') - ->willReturn(12345); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); - - $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); - $expectedResponse = [ - 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is expired', + 'msg' => 'Could not reset password because the token is invalid', ]; $this->assertSame($expectedResponse, $response); } @@ -686,33 +494,14 @@ class LostControllerTest extends \Test\TestCase { $this->userManager->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('aValidtoken'), - $this->equalTo('test@example.comSECRET') - )->willThrowException(new \Exception()); - - $response = $this->lostController->setPassword('', 'ValidTokenUser', 'NewPassword', true); - $expectedResponse = [ - 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is invalid' - ]; - $this->assertSame($expectedResponse, $response); - } - - public function testIsSetPasswordTokenNullFailing() { - $this->config->method('getUserValue') - ->with('ValidTokenUser', 'core', 'lostpassword', null) - ->willReturn(null); - $this->userManager->method('get') - ->with('ValidTokenUser') - ->willReturn($this->existingUser); + $this->verificationToken->expects($this->atLeastOnce()) + ->method('check') + ->willThrowException(new InvalidTokenException(InvalidTokenException::TOKEN_MISMATCH)); $response = $this->lostController->setPassword('', 'ValidTokenUser', 'NewPassword', true); $expectedResponse = [ 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is invalid' + 'msg' => 'Could not reset password because the token is invalid' ]; $this->assertSame($expectedResponse, $response); } @@ -732,10 +521,14 @@ class LostControllerTest extends \Test\TestCase { ->with('DisabledUser') ->willReturn($user); + $this->verificationToken->expects($this->atLeastOnce()) + ->method('check') + ->willThrowException(new InvalidTokenException(InvalidTokenException::USER_UNKNOWN)); + $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'DisabledUser', 'NewPassword', true); $expectedResponse = [ 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is invalid' + 'msg' => 'Could not reset password because the token is invalid' ]; $this->assertSame($expectedResponse, $response); } @@ -798,14 +591,6 @@ class LostControllerTest extends \Test\TestCase { $this->config->expects($this->once()) ->method('deleteUserValue') ->with('ValidTokenUser', 'core', 'lostpassword'); - $this->timeFactory->method('getTime') - ->willReturn(12348); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', false); $expectedResponse = ['user' => 'ValidTokenUser', 'status' => 'success']; diff --git a/tests/lib/Security/VerificationToken/VerificationTokenTest.php b/tests/lib/Security/VerificationToken/VerificationTokenTest.php new file mode 100644 index 00000000000..d1faf18dd8f --- /dev/null +++ b/tests/lib/Security/VerificationToken/VerificationTokenTest.php @@ -0,0 +1,272 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2021 Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @author Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <https://www.gnu.org/licenses/>. + * + */ + +namespace Test\Security\VerificationToken; + +use OC\Security\VerificationToken\VerificationToken; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IConfig; +use OCP\IUser; +use OCP\Security\ICrypto; +use OCP\Security\ISecureRandom; +use OCP\Security\VerificationToken\InvalidTokenException; +use Test\TestCase; + +class VerificationTokenTest extends TestCase { + /** @var VerificationToken */ + protected $token; + /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ + protected $config; + /** @var ISecureRandom|\PHPUnit\Framework\MockObject\MockObject */ + protected $secureRandom; + /** @var ICrypto|\PHPUnit\Framework\MockObject\MockObject */ + protected $crypto; + /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ + protected $timeFactory; + + protected function setUp(): void { + parent::setUp(); + + $this->config = $this->createMock(IConfig::class); + $this->crypto = $this->createMock(ICrypto::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->secureRandom = $this->createMock(ISecureRandom::class); + + $this->token = new VerificationToken( + $this->config, + $this->crypto, + $this->timeFactory, + $this->secureRandom + ); + } + + public function testTokenUserUnknown() { + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::USER_UNKNOWN); + $this->token->check('encryptedToken', null, 'fingerprintToken', 'foobar'); + } + + public function testTokenUserUnknown2() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(false); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::USER_UNKNOWN); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenNotFound() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + + // implicit: IConfig::getUserValue returns null by default + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_NOT_FOUND); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenDecryptionError() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willThrowException(new \Exception('decryption failed')); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_DECRYPTION_ERROR); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenInvalidFormat() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willReturn('decrypted^nonsense'); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_INVALID_FORMAT); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenExpired() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + $user->expects($this->any()) + ->method('getLastLogin') + ->willReturn(604803); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willReturn('604800:mY70K3n'); + + $this->timeFactory->expects($this->any()) + ->method('getTime') + ->willReturn(604801); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_EXPIRED); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenMismatch() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + $user->expects($this->any()) + ->method('getLastLogin') + ->willReturn(604703); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willReturn('604802:mY70K3n'); + + $this->timeFactory->expects($this->any()) + ->method('getTime') + ->willReturn(604801); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_MISMATCH); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenSuccess() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + $user->expects($this->any()) + ->method('getLastLogin') + ->willReturn(604703); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willReturn('604802:barfoo'); + + $this->timeFactory->expects($this->any()) + ->method('getTime') + ->willReturn(604801); + + $this->token->check('barfoo', $user, 'fingerprintToken', 'foobar'); + } + + public function testCreate() { + $user = $this->createMock(IUser::class); + $user->expects($this->any()) + ->method('getUID') + ->willReturn('alice'); + + $this->secureRandom->expects($this->atLeastOnce()) + ->method('generate') + ->willReturn('barfoo'); + $this->crypto->expects($this->atLeastOnce()) + ->method('encrypt') + ->willReturn('encryptedToken'); + $this->config->expects($this->atLeastOnce()) + ->method('setUserValue') + ->with('alice', 'core', 'fingerprintToken', 'encryptedToken'); + + $vToken = $this->token->create($user, 'fingerprintToken', 'foobar'); + $this->assertSame('barfoo', $vToken); + } +} |