Browse Source

Make LostController use IInitialState and LoggerInterface

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
pull/31637/head
Thomas Citharel 4 years ago
parent
commit
abe5ff3654
No known key found for this signature in database GPG Key ID: A061B9DDE0CA0773
  1. 30
      core/Controller/LostController.php
  2. 24
      tests/Core/Controller/LostControllerTest.php

30
core/Controller/LostController.php

@ -35,6 +35,7 @@
*/ */
namespace OC\Core\Controller; namespace OC\Core\Controller;
use Exception;
use OC\Authentication\TwoFactorAuth\Manager; use OC\Authentication\TwoFactorAuth\Manager;
use OC\Core\Events\BeforePasswordResetEvent; use OC\Core\Events\BeforePasswordResetEvent;
use OC\Core\Events\PasswordResetEvent; use OC\Core\Events\PasswordResetEvent;
@ -42,13 +43,13 @@ use OC\Core\Exception\ResetPasswordException;
use OCP\AppFramework\Controller; use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Services\IInitialState;
use OCP\Defaults; use OCP\Defaults;
use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IEncryptionModule;
use OCP\Encryption\IManager; use OCP\Encryption\IManager;
use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\IEventDispatcher;
use OCP\HintException; use OCP\HintException;
use OCP\IConfig; use OCP\IConfig;
use OCP\IInitialStateService;
use OCP\IL10N; use OCP\IL10N;
use OCP\IRequest; use OCP\IRequest;
use OCP\IURLGenerator; use OCP\IURLGenerator;
@ -80,9 +81,8 @@ class LostController extends Controller {
protected IMailer $mailer; protected IMailer $mailer;
private LoggerInterface $logger; private LoggerInterface $logger;
private Manager $twoFactorManager; private Manager $twoFactorManager;
private IInitialStateService $initialStateService;
private IInitialState $initialState;
private IVerificationToken $verificationToken; private IVerificationToken $verificationToken;
private IEventDispatcher $eventDispatcher; private IEventDispatcher $eventDispatcher;
public function __construct( public function __construct(
@ -93,12 +93,12 @@ class LostController extends Controller {
Defaults $defaults, Defaults $defaults,
IL10N $l10n, IL10N $l10n,
IConfig $config, IConfig $config,
$defaultMailAddress,
string $defaultMailAddress,
IManager $encryptionManager, IManager $encryptionManager,
IMailer $mailer, IMailer $mailer,
LoggerInterface $logger, LoggerInterface $logger,
Manager $twoFactorManager, Manager $twoFactorManager,
IInitialStateService $initialStateService,
IInitialState $initialState,
IVerificationToken $verificationToken, IVerificationToken $verificationToken,
IEventDispatcher $eventDispatcher IEventDispatcher $eventDispatcher
) { ) {
@ -113,7 +113,7 @@ class LostController extends Controller {
$this->mailer = $mailer; $this->mailer = $mailer;
$this->logger = $logger; $this->logger = $logger;
$this->twoFactorManager = $twoFactorManager; $this->twoFactorManager = $twoFactorManager;
$this->initialStateService = $initialStateService;
$this->initialState = $initialState;
$this->verificationToken = $verificationToken; $this->verificationToken = $verificationToken;
$this->eventDispatcher = $eventDispatcher; $this->eventDispatcher = $eventDispatcher;
} }
@ -127,7 +127,7 @@ class LostController extends Controller {
public function resetform(string $token, string $userId): TemplateResponse { public function resetform(string $token, string $userId): TemplateResponse {
try { try {
$this->checkPasswordResetToken($token, $userId); $this->checkPasswordResetToken($token, $userId);
} catch (\Exception $e) {
} catch (Exception $e) {
if ($this->config->getSystemValue('lost_password_link', '') !== 'disabled' if ($this->config->getSystemValue('lost_password_link', '') !== 'disabled'
|| ($e instanceof InvalidTokenException || ($e instanceof InvalidTokenException
&& !in_array($e->getCode(), [InvalidTokenException::TOKEN_NOT_FOUND, InvalidTokenException::USER_UNKNOWN])) && !in_array($e->getCode(), [InvalidTokenException::TOKEN_NOT_FOUND, InvalidTokenException::USER_UNKNOWN]))
@ -145,8 +145,8 @@ class LostController extends Controller {
TemplateResponse::RENDER_AS_GUEST 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]) $this->urlGenerator->linkToRouteAbsolute('core.lost.setPassword', ['userId' => $userId, 'token' => $token])
); );
@ -159,7 +159,7 @@ class LostController extends Controller {
} }
/** /**
* @throws \Exception
* @throws Exception
*/ */
protected function checkPasswordResetToken(string $token, string $userId): void { protected function checkPasswordResetToken(string $token, string $userId): void {
try { try {
@ -169,7 +169,7 @@ class LostController extends Controller {
$error = $e->getCode() === InvalidTokenException::TOKEN_EXPIRED $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 expired')
: $this->l10n->t('Could not reset password because the token is invalid'); : $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);
} }
} }
@ -203,7 +203,7 @@ class LostController extends Controller {
} catch (ResetPasswordException $e) { } catch (ResetPasswordException $e) {
// Ignore the error since we do not want to leak this info // Ignore the error since we do not want to leak this info
$this->logger->warning('Could not send password reset email: ' . $e->getMessage()); $this->logger->warning('Could not send password reset email: ' . $e->getMessage());
} catch (\Exception $e) {
} catch (Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]); $this->logger->error($e->getMessage(), ['exception' => $e]);
} }
@ -236,7 +236,7 @@ class LostController extends Controller {
\OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'pre_passwordReset', ['uid' => $userId, 'password' => $password]); \OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'pre_passwordReset', ['uid' => $userId, 'password' => $password]);
if (!$user->setPassword($password)) { if (!$user->setPassword($password)) {
throw new \Exception();
throw new Exception();
} }
$this->eventDispatcher->dispatchTyped(new PasswordResetEvent($user, $password)); $this->eventDispatcher->dispatchTyped(new PasswordResetEvent($user, $password));
@ -248,7 +248,7 @@ class LostController extends Controller {
@\OC::$server->getUserSession()->unsetMagicInCookie(); @\OC::$server->getUserSession()->unsetMagicInCookie();
} catch (HintException $e) { } catch (HintException $e) {
return $this->error($e->getHint()); return $this->error($e->getHint());
} catch (\Exception $e) {
} catch (Exception $e) {
return $this->error($e->getMessage()); return $this->error($e->getMessage());
} }
@ -301,7 +301,7 @@ class LostController extends Controller {
$message->setFrom([$this->from => $this->defaults->getName()]); $message->setFrom([$this->from => $this->defaults->getName()]);
$message->useTemplate($emailTemplate); $message->useTemplate($emailTemplate);
$this->mailer->send($message); $this->mailer->send($message);
} catch (\Exception $e) {
} catch (Exception $e) {
// Log the exception and continue // Log the exception and continue
$this->logger->error($e->getMessage(), ['app' => 'core', 'exception' => $e]); $this->logger->error($e->getMessage(), ['app' => 'core', 'exception' => $e]);
} }

24
tests/Core/Controller/LostControllerTest.php

@ -28,12 +28,12 @@ use OC\Core\Events\PasswordResetEvent;
use OC\Mail\Message; use OC\Mail\Message;
use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Services\IInitialState;
use OCP\Defaults; use OCP\Defaults;
use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IEncryptionModule;
use OCP\Encryption\IManager; use OCP\Encryption\IManager;
use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig; use OCP\IConfig;
use OCP\IInitialStateService;
use OCP\IL10N; use OCP\IL10N;
use OCP\IRequest; use OCP\IRequest;
use OCP\IURLGenerator; use OCP\IURLGenerator;
@ -72,12 +72,12 @@ class LostControllerTest extends TestCase {
private $encryptionManager; private $encryptionManager;
/** @var IRequest|MockObject */ /** @var IRequest|MockObject */
private $request; private $request;
/** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
/** @var LoggerInterface|MockObject */
private $logger; private $logger;
/** @var Manager|MockObject */ /** @var Manager|MockObject */
private $twofactorManager; private $twofactorManager;
/** @var IInitialStateService|MockObject */
private $initialStateService;
/** @var IInitialState|MockObject */
private $initialState;
/** @var IVerificationToken|MockObject */ /** @var IVerificationToken|MockObject */
private $verificationToken; private $verificationToken;
/** @var IEventDispatcher|MockObject */ /** @var IEventDispatcher|MockObject */
@ -126,7 +126,7 @@ class LostControllerTest extends TestCase {
->willReturn(true); ->willReturn(true);
$this->logger = $this->createMock(LoggerInterface::class); $this->logger = $this->createMock(LoggerInterface::class);
$this->twofactorManager = $this->createMock(Manager::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->verificationToken = $this->createMock(IVerificationToken::class);
$this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->lostController = new LostController( $this->lostController = new LostController(
@ -142,7 +142,7 @@ class LostControllerTest extends TestCase {
$this->mailer, $this->mailer,
$this->logger, $this->logger,
$this->twofactorManager, $this->twofactorManager,
$this->initialStateService,
$this->initialState,
$this->verificationToken, $this->verificationToken,
$this->eventDispatcher $this->eventDispatcher
); );
@ -176,6 +176,18 @@ class LostControllerTest extends TestCase {
$this->verificationToken->expects($this->once()) $this->verificationToken->expects($this->once())
->method('check') ->method('check')
->with('MySecretToken', $this->existingUser, 'lostpassword', 'test@example.com'); ->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'); $response = $this->lostController->resetform('MySecretToken', 'ValidTokenUser');
$expectedResponse = new TemplateResponse('core', $expectedResponse = new TemplateResponse('core',

Loading…
Cancel
Save