diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php index e0f16226bff62..0172d94ac95db 100644 --- a/core/Controller/LostController.php +++ b/core/Controller/LostController.php @@ -35,17 +35,21 @@ */ namespace OC\Core\Controller; +use Exception; use OC\Authentication\TwoFactorAuth\Manager; +use OC\Core\Events\BeforePasswordResetEvent; +use OC\Core\Events\PasswordResetEvent; use OC\Core\Exception\ResetPasswordException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\AppFramework\Services\IInitialState; use OCP\Defaults; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; +use OCP\EventDispatcher\IEventDispatcher; use OCP\HintException; use OCP\IConfig; -use OCP\IInitialStateService; use OCP\IL10N; use OCP\IRequest; use OCP\IURLGenerator; @@ -77,8 +81,9 @@ class LostController extends Controller { protected IMailer $mailer; private LoggerInterface $logger; private Manager $twoFactorManager; - private IInitialStateService $initialStateService; + private IInitialState $initialState; private IVerificationToken $verificationToken; + private IEventDispatcher $eventDispatcher; public function __construct( string $appName, @@ -88,13 +93,14 @@ public function __construct( Defaults $defaults, IL10N $l10n, IConfig $config, - $defaultMailAddress, + string $defaultMailAddress, IManager $encryptionManager, IMailer $mailer, LoggerInterface $logger, Manager $twoFactorManager, - IInitialStateService $initialStateService, - IVerificationToken $verificationToken + IInitialState $initialState, + IVerificationToken $verificationToken, + IEventDispatcher $eventDispatcher ) { parent::__construct($appName, $request); $this->urlGenerator = $urlGenerator; @@ -107,8 +113,9 @@ public function __construct( $this->mailer = $mailer; $this->logger = $logger; $this->twoFactorManager = $twoFactorManager; - $this->initialStateService = $initialStateService; + $this->initialState = $initialState; $this->verificationToken = $verificationToken; + $this->eventDispatcher = $eventDispatcher; } /** @@ -120,7 +127,7 @@ public function __construct( public function resetform(string $token, string $userId): TemplateResponse { try { $this->checkPasswordResetToken($token, $userId); - } catch (\Exception $e) { + } catch (Exception $e) { if ($this->config->getSystemValue('lost_password_link', '') !== 'disabled' || ($e instanceof InvalidTokenException && !in_array($e->getCode(), [InvalidTokenException::TOKEN_NOT_FOUND, InvalidTokenException::USER_UNKNOWN])) @@ -138,8 +145,8 @@ public function resetform(string $token, string $userId): TemplateResponse { TemplateResponse::RENDER_AS_GUEST ); } - $this->initialStateService->provideInitialState('core', 'resetPasswordUser', $userId); - $this->initialStateService->provideInitialState('core', 'resetPasswordTarget', + $this->initialState->provideInitialState('resetPasswordUser', $userId); + $this->initialState->provideInitialState('resetPasswordTarget', $this->urlGenerator->linkToRouteAbsolute('core.lost.setPassword', ['userId' => $userId, 'token' => $token]) ); @@ -152,7 +159,7 @@ public function resetform(string $token, string $userId): TemplateResponse { } /** - * @throws \Exception + * @throws Exception */ protected function checkPasswordResetToken(string $token, string $userId): void { try { @@ -162,7 +169,7 @@ protected function checkPasswordResetToken(string $token, string $userId): void $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); + throw new Exception($error, (int)$e->getCode(), $e); } } @@ -196,7 +203,7 @@ public function email(string $user): JSONResponse { } catch (ResetPasswordException $e) { // Ignore the error since we do not want to leak this info $this->logger->warning('Could not send password reset email: ' . $e->getMessage()); - } catch (\Exception $e) { + } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); } @@ -225,12 +232,14 @@ public function setPassword(string $token, string $userId, string $password, boo $this->checkPasswordResetToken($token, $userId); $user = $this->userManager->get($userId); + $this->eventDispatcher->dispatchTyped(new BeforePasswordResetEvent($user, $password)); \OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'pre_passwordReset', ['uid' => $userId, 'password' => $password]); if (!$user->setPassword($password)) { - throw new \Exception(); + throw new Exception(); } + $this->eventDispatcher->dispatchTyped(new PasswordResetEvent($user, $password)); \OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'post_passwordReset', ['uid' => $userId, 'password' => $password]); $this->twoFactorManager->clearTwoFactorPending($userId); @@ -239,7 +248,7 @@ public function setPassword(string $token, string $userId, string $password, boo @\OC::$server->getUserSession()->unsetMagicInCookie(); } catch (HintException $e) { return $this->error($e->getHint()); - } catch (\Exception $e) { + } catch (Exception $e) { return $this->error($e->getMessage()); } @@ -292,7 +301,7 @@ protected function sendEmail(string $input): void { $message->setFrom([$this->from => $this->defaults->getName()]); $message->useTemplate($emailTemplate); $this->mailer->send($message); - } catch (\Exception $e) { + } catch (Exception $e) { // Log the exception and continue $this->logger->error($e->getMessage(), ['app' => 'core', 'exception' => $e]); } diff --git a/core/Events/BeforePasswordResetEvent.php b/core/Events/BeforePasswordResetEvent.php new file mode 100644 index 0000000000000..d560a723dde6b --- /dev/null +++ b/core/Events/BeforePasswordResetEvent.php @@ -0,0 +1,63 @@ + + * + * @author Christoph Wurst + * @author Morris Jobke + * + * @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 . + * + */ +namespace OC\Core\Events; + +use OCP\EventDispatcher\Event; +use OCP\IUser; + +/** + * Emitted before the user password is reset. + * + * @since 25.0.0 + */ +class BeforePasswordResetEvent extends Event { + private IUser $user; + private string $password; + + /** + * @since 25.0.0 + */ + public function __construct(IUser $user, string $password) { + parent::__construct(); + $this->user = $user; + $this->password = $password; + } + + /** + * @since 25.0.0 + */ + public function getUser(): IUser { + return $this->user; + } + + /** + * @since 25.0.0 + */ + public function getPassword(): string { + return $this->password; + } +} diff --git a/core/Events/PasswordResetEvent.php b/core/Events/PasswordResetEvent.php new file mode 100644 index 0000000000000..8846004920c16 --- /dev/null +++ b/core/Events/PasswordResetEvent.php @@ -0,0 +1,63 @@ + + * + * @author Christoph Wurst + * @author Morris Jobke + * + * @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 . + * + */ +namespace OC\Core\Events; + +use OCP\EventDispatcher\Event; +use OCP\IUser; + +/** + * Emitted after the user password is reset. + * + * @since 25.0.0 + */ +class PasswordResetEvent extends Event { + private IUser $user; + private string $password; + + /** + * @since 25.0.0 + */ + public function __construct(IUser $user, string $password) { + parent::__construct(); + $this->user = $user; + $this->password = $password; + } + + /** + * @since 25.0.0 + */ + public function getUser(): IUser { + return $this->user; + } + + /** + * @since 25.0.0 + */ + public function getPassword(): string { + return $this->password; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index f12ee47642af9..1edc39c52f2f3 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -981,6 +981,8 @@ 'OC\\Core\\Db\\LoginFlowV2Mapper' => $baseDir . '/core/Db/LoginFlowV2Mapper.php', 'OC\\Core\\Db\\ProfileConfig' => $baseDir . '/core/Db/ProfileConfig.php', 'OC\\Core\\Db\\ProfileConfigMapper' => $baseDir . '/core/Db/ProfileConfigMapper.php', + 'OC\\Core\\Events\\BeforePasswordResetEvent' => $baseDir . '/core/Events/BeforePasswordResetEvent.php', + 'OC\\Core\\Events\\PasswordResetEvent' => $baseDir . '/core/Events/PasswordResetEvent.php', 'OC\\Core\\Exception\\LoginFlowV2NotFoundException' => $baseDir . '/core/Exception/LoginFlowV2NotFoundException.php', 'OC\\Core\\Exception\\ResetPasswordException' => $baseDir . '/core/Exception/ResetPasswordException.php', 'OC\\Core\\Middleware\\TwoFactorMiddleware' => $baseDir . '/core/Middleware/TwoFactorMiddleware.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 502556d3f7fb5..2efd6effc9111 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1014,6 +1014,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Db\\LoginFlowV2Mapper' => __DIR__ . '/../../..' . '/core/Db/LoginFlowV2Mapper.php', 'OC\\Core\\Db\\ProfileConfig' => __DIR__ . '/../../..' . '/core/Db/ProfileConfig.php', 'OC\\Core\\Db\\ProfileConfigMapper' => __DIR__ . '/../../..' . '/core/Db/ProfileConfigMapper.php', + 'OC\\Core\\Events\\BeforePasswordResetEvent' => __DIR__ . '/../../..' . '/core/Events/BeforePasswordResetEvent.php', + 'OC\\Core\\Events\\PasswordResetEvent' => __DIR__ . '/../../..' . '/core/Events/PasswordResetEvent.php', 'OC\\Core\\Exception\\LoginFlowV2NotFoundException' => __DIR__ . '/../../..' . '/core/Exception/LoginFlowV2NotFoundException.php', 'OC\\Core\\Exception\\ResetPasswordException' => __DIR__ . '/../../..' . '/core/Exception/ResetPasswordException.php', 'OC\\Core\\Middleware\\TwoFactorMiddleware' => __DIR__ . '/../../..' . '/core/Middleware/TwoFactorMiddleware.php', diff --git a/tests/Core/Controller/LostControllerTest.php b/tests/Core/Controller/LostControllerTest.php index 8252e38b568a8..31f2767ea4f2d 100644 --- a/tests/Core/Controller/LostControllerTest.php +++ b/tests/Core/Controller/LostControllerTest.php @@ -23,14 +23,17 @@ use OC\Authentication\TwoFactorAuth\Manager; use OC\Core\Controller\LostController; +use OC\Core\Events\BeforePasswordResetEvent; +use OC\Core\Events\PasswordResetEvent; use OC\Mail\Message; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\AppFramework\Services\IInitialState; use OCP\Defaults; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; -use OCP\IInitialStateService; use OCP\IL10N; use OCP\IRequest; use OCP\IURLGenerator; @@ -41,42 +44,44 @@ use OCP\Security\VerificationToken\InvalidTokenException; use OCP\Security\VerificationToken\IVerificationToken; use Psr\Log\LoggerInterface; +use PHPUnit\Framework\MockObject\MockObject; +use Test\TestCase; /** * Class LostControllerTest * * @package OC\Core\Controller */ -class LostControllerTest extends \Test\TestCase { - - /** @var LostController */ - private $lostController; +class LostControllerTest extends TestCase { + private LostController $lostController; /** @var IUser */ private $existingUser; - /** @var IURLGenerator | \PHPUnit\Framework\MockObject\MockObject */ + /** @var IURLGenerator | MockObject */ private $urlGenerator; /** @var IL10N */ private $l10n; - /** @var IUserManager | \PHPUnit\Framework\MockObject\MockObject */ + /** @var IUserManager | MockObject */ private $userManager; /** @var Defaults */ private $defaults; - /** @var IConfig | \PHPUnit\Framework\MockObject\MockObject */ + /** @var IConfig | MockObject */ private $config; - /** @var IMailer | \PHPUnit\Framework\MockObject\MockObject */ + /** @var IMailer | MockObject */ private $mailer; - /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IManager|MockObject */ private $encryptionManager; - /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IRequest|MockObject */ private $request; - /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ + /** @var LoggerInterface|MockObject */ private $logger; - /** @var Manager|\PHPUnit\Framework\MockObject\MockObject */ + /** @var Manager|MockObject */ private $twofactorManager; - /** @var IInitialStateService|\PHPUnit\Framework\MockObject\MockObject */ - private $initialStateService; - /** @var IVerificationToken|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IInitialState|MockObject */ + private $initialState; + /** @var IVerificationToken|MockObject */ private $verificationToken; + /** @var IEventDispatcher|MockObject */ + private $eventDispatcher; protected function setUp(): void { parent::setUp(); @@ -110,25 +115,20 @@ protected function setUp(): void { ->willReturnCallback(function ($text, $parameters = []) { return vsprintf($text, $parameters); }); - $this->defaults = $this->getMockBuilder('\OCP\Defaults') - ->disableOriginalConstructor()->getMock(); - $this->userManager = $this->getMockBuilder(IUserManager::class) - ->disableOriginalConstructor()->getMock(); - $this->urlGenerator = $this->getMockBuilder(IURLGenerator::class) - ->disableOriginalConstructor()->getMock(); - $this->mailer = $this->getMockBuilder('\OCP\Mail\IMailer') - ->disableOriginalConstructor()->getMock(); - $this->request = $this->getMockBuilder(IRequest::class) - ->disableOriginalConstructor()->getMock(); - $this->encryptionManager = $this->getMockBuilder(IManager::class) - ->disableOriginalConstructor()->getMock(); + $this->defaults = $this->createMock(Defaults::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->mailer = $this->createMock(IMailer::class); + $this->request = $this->createMock(IRequest::class); + $this->encryptionManager = $this->createMock(IManager::class); $this->encryptionManager->expects($this->any()) ->method('isEnabled') ->willReturn(true); $this->logger = $this->createMock(LoggerInterface::class); $this->twofactorManager = $this->createMock(Manager::class); - $this->initialStateService = $this->createMock(IInitialStateService::class); + $this->initialState = $this->createMock(IInitialState::class); $this->verificationToken = $this->createMock(IVerificationToken::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->lostController = new LostController( 'Core', $this->request, @@ -142,8 +142,9 @@ protected function setUp(): void { $this->mailer, $this->logger, $this->twofactorManager, - $this->initialStateService, - $this->verificationToken + $this->initialState, + $this->verificationToken, + $this->eventDispatcher ); } @@ -175,6 +176,18 @@ public function testResetFormValidToken() { $this->verificationToken->expects($this->once()) ->method('check') ->with('MySecretToken', $this->existingUser, 'lostpassword', 'test@example.com'); + $this->urlGenerator + ->expects($this->once()) + ->method('linkToRouteAbsolute') + ->with('core.lost.setPassword', ['userId' => 'ValidTokenUser', 'token' => 'MySecretToken']) + ->willReturn('https://example.tld/index.php/lostpassword/set/sometoken/someuser'); + $this->initialState + ->expects($this->exactly(2)) + ->method('provideInitialState') + ->withConsecutive( + ['resetPasswordUser', 'ValidTokenUser'], + ['resetPasswordTarget', 'https://example.tld/index.php/lostpassword/set/sometoken/someuser'] + ); $response = $this->lostController->resetform('MySecretToken', 'ValidTokenUser'); $expectedResponse = new TemplateResponse('core', @@ -243,11 +256,11 @@ public function testEmailSuccessful() { $message = $this->getMockBuilder('\OC\Mail\Message') ->disableOriginalConstructor()->getMock(); $message - ->expects($this->at(0)) + ->expects($this->once()) ->method('setTo') ->with(['test@example.com' => 'Existing User']); $message - ->expects($this->at(1)) + ->expects($this->once()) ->method('setFrom') ->with(['lostpassword-noreply@localhost' => null]); @@ -260,20 +273,20 @@ public function testEmailSuccessful() { ->willReturn('text body'); $message - ->expects($this->at(2)) + ->expects($this->once()) ->method('useTemplate') ->with($emailTemplate); $this->mailer - ->expects($this->at(0)) + ->expects($this->once()) ->method('createEMailTemplate') ->willReturn($emailTemplate); $this->mailer - ->expects($this->at(1)) + ->expects($this->once()) ->method('createMessage') ->willReturn($message); $this->mailer - ->expects($this->at(2)) + ->expects($this->once()) ->method('send') ->with($message); @@ -305,11 +318,11 @@ public function testEmailWithMailSuccessful() { $message = $this->getMockBuilder('\OC\Mail\Message') ->disableOriginalConstructor()->getMock(); $message - ->expects($this->at(0)) + ->expects($this->once()) ->method('setTo') ->with(['test@example.com' => 'Existing User']); $message - ->expects($this->at(1)) + ->expects($this->once()) ->method('setFrom') ->with(['lostpassword-noreply@localhost' => null]); @@ -322,20 +335,20 @@ public function testEmailWithMailSuccessful() { ->willReturn('text body'); $message - ->expects($this->at(2)) + ->expects($this->once()) ->method('useTemplate') ->with($emailTemplate); $this->mailer - ->expects($this->at(0)) + ->expects($this->once()) ->method('createEMailTemplate') ->willReturn($emailTemplate); $this->mailer - ->expects($this->at(1)) + ->expects($this->once()) ->method('createMessage') ->willReturn($message); $this->mailer - ->expects($this->at(2)) + ->expects($this->once()) ->method('send') ->with($message); @@ -361,11 +374,11 @@ public function testEmailCantSendException() { ->willReturn('https://example.tld/index.php/lostpassword/'); $message = $this->createMock(Message::class); $message - ->expects($this->at(0)) + ->expects($this->once()) ->method('setTo') ->with(['test@example.com' => 'Existing User']); $message - ->expects($this->at(1)) + ->expects($this->once()) ->method('setFrom') ->with(['lostpassword-noreply@localhost' => null]); @@ -378,20 +391,20 @@ public function testEmailCantSendException() { ->willReturn('text body'); $message - ->expects($this->at(2)) + ->expects($this->once()) ->method('useTemplate') ->with($emailTemplate); $this->mailer - ->expects($this->at(0)) + ->expects($this->once()) ->method('createEMailTemplate') ->willReturn($emailTemplate); $this->mailer - ->expects($this->at(1)) + ->expects($this->once()) ->method('createMessage') ->willReturn($message); $this->mailer - ->expects($this->at(2)) + ->expects($this->once()) ->method('send') ->with($message) ->will($this->throwException(new \Exception())); @@ -418,6 +431,11 @@ public function testSetPasswordUnsuccessful() { $this->userManager->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); + $beforePasswordResetEvent = new BeforePasswordResetEvent($this->existingUser, 'NewPassword'); + $this->eventDispatcher + ->expects($this->once()) + ->method('dispatchTyped') + ->with($beforePasswordResetEvent); $this->config->expects($this->never()) ->method('deleteUserValue'); @@ -439,6 +457,12 @@ public function testSetPasswordSuccessful() { $this->userManager->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); + $beforePasswordResetEvent = new BeforePasswordResetEvent($this->existingUser, 'NewPassword'); + $passwordResetEvent = new PasswordResetEvent($this->existingUser, 'NewPassword'); + $this->eventDispatcher + ->expects($this->exactly(2)) + ->method('dispatchTyped') + ->withConsecutive([$beforePasswordResetEvent], [$passwordResetEvent]); $this->config->expects($this->once()) ->method('deleteUserValue') ->with('ValidTokenUser', 'core', 'lostpassword'); @@ -560,7 +584,7 @@ public function testSendEmailNoEmail() { } public function testSetPasswordEncryptionDontProceedPerUserKey() { - /** @var IEncryptionModule|\PHPUnit\Framework\MockObject\MockObject $encryptionModule */ + /** @var IEncryptionModule|MockObject $encryptionModule */ $encryptionModule = $this->createMock(IEncryptionModule::class); $encryptionModule->expects($this->once())->method('needDetailedAccessList')->willReturn(true); $this->encryptionManager->expects($this->once())->method('getEncryptionModules')