From 283476a2f7c200912352eb4db097f540e3993e75 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 12 Feb 2015 16:03:51 +0100 Subject: [PATCH] Use new IMailer and add unit-tests for lostcontroller --- core/application.php | 6 +- .../controller/lostcontroller.php | 24 +++-- .../controller/lostcontrollertest.php | 101 ++++++++++++++++-- 3 files changed, 111 insertions(+), 20 deletions(-) diff --git a/core/application.php b/core/application.php index 568fc34db7..34f817aa39 100644 --- a/core/application.php +++ b/core/application.php @@ -46,7 +46,8 @@ class Application extends App { $c->query('Config'), $c->query('SecureRandom'), $c->query('DefaultEmailAddress'), - $c->query('IsEncryptionEnabled') + $c->query('IsEncryptionEnabled'), + $c->query('Mailer') ); }); $container->registerService('UserController', function(SimpleContainer $c) { @@ -104,6 +105,9 @@ class Application extends App { $container->registerService('Defaults', function() { return new \OC_Defaults; }); + $container->registerService('Mailer', function(SimpleContainer $c) { + return $c->query('ServerContainer')->getMailer(); + }); $container->registerService('DefaultEmailAddress', function() { return Util::getDefaultEmailAddress('lostpassword-noreply'); }); diff --git a/core/lostpassword/controller/lostcontroller.php b/core/lostpassword/controller/lostcontroller.php index d1a4528d44..08f5246503 100644 --- a/core/lostpassword/controller/lostcontroller.php +++ b/core/lostpassword/controller/lostcontroller.php @@ -15,6 +15,7 @@ use \OCP\IRequest; use \OCP\IL10N; use \OCP\IConfig; use OCP\IUserManager; +use OCP\Mail\IMailer; use OCP\Security\ISecureRandom; use \OC_Defaults; use OCP\Security\StringUtils; @@ -32,6 +33,7 @@ class LostController extends Controller { protected $urlGenerator; /** @var IUserManager */ protected $userManager; + // FIXME: Inject a non-static factory of OC_Defaults for better unit-testing /** @var OC_Defaults */ protected $defaults; /** @var IL10N */ @@ -44,6 +46,8 @@ class LostController extends Controller { protected $config; /** @var ISecureRandom */ protected $secureRandom; + /** @var IMailer */ + protected $mailer; /** * @param string $appName @@ -56,6 +60,7 @@ class LostController extends Controller { * @param ISecureRandom $secureRandom * @param string $from * @param string $isDataEncrypted + * @param IMailer $mailer */ public function __construct($appName, IRequest $request, @@ -66,7 +71,8 @@ class LostController extends Controller { IConfig $config, ISecureRandom $secureRandom, $from, - $isDataEncrypted) { + $isDataEncrypted, + IMailer $mailer) { parent::__construct($appName, $request); $this->urlGenerator = $urlGenerator; $this->userManager = $userManager; @@ -76,6 +82,7 @@ class LostController extends Controller { $this->from = $from; $this->isDataEncrypted = $isDataEncrypted; $this->config = $config; + $this->mailer = $mailer; } /** @@ -200,15 +207,12 @@ class LostController extends Controller { $msg = $tmpl->fetchPage(); try { - // FIXME: should be added to the container and injected in here - \OCP\Util::sendMail( - $email, - $user, - $this->l10n->t('%s password reset', array($this->defaults->getName())), - $msg, - $this->from, - $this->defaults->getName() - ); + $message = $this->mailer->createMessage(); + $message->setTo([$email => $user]); + $message->setSubject($this->l10n->t('%s password reset', [$this->defaults->getName()])); + $message->setPlainBody($msg); + $message->setFrom([$this->from => $this->defaults->getName()]); + $this->mailer->send($message); } catch (\Exception $e) { throw new \Exception($this->l10n->t( 'Couldn\'t send reset email. Please contact your administrator.' diff --git a/tests/core/lostpassword/controller/lostcontrollertest.php b/tests/core/lostpassword/controller/lostcontrollertest.php index b03cbd7c42..f82fc1ba3f 100644 --- a/tests/core/lostpassword/controller/lostcontrollertest.php +++ b/tests/core/lostpassword/controller/lostcontrollertest.php @@ -1,6 +1,6 @@ + * Copyright (c) 2014-2015 Lukas Reschke * This file is licensed under the Affero General Public License version 3 or * later. * See the COPYING-README file. @@ -43,6 +43,8 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase { ->disableOriginalConstructor()->getMock(); $this->container['URLGenerator'] = $this->getMockBuilder('\OCP\IURLGenerator') ->disableOriginalConstructor()->getMock(); + $this->container['Mailer'] = $this->getMockBuilder('\OCP\Mail\IMailer') + ->disableOriginalConstructor()->getMock(); $this->container['SecureRandom'] = $this->getMockBuilder('\OCP\Security\ISecureRandom') ->disableOriginalConstructor()->getMock(); $this->container['IsEncryptionEnabled'] = true; @@ -103,14 +105,6 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase { } public function testEmailSuccessful() { - /** - * FIXME: Disable test for systems where no sendmail is available since code is static. - * @link https://github.com/owncloud/core/pull/12085 - */ - if (is_null(\OC_Helper::findBinaryPath('sendmail'))) { - $this->markTestSkipped('sendmail is not available'); - } - $randomToken = $this->container['SecureRandom']; $this->container['SecureRandom'] ->expects($this->once()) @@ -140,12 +134,101 @@ class LostControllerTest extends \PHPUnit_Framework_TestCase { ->method('linkToRouteAbsolute') ->with('core.lost.resetform', array('userId' => 'ExistingUser', 'token' => 'ThisIsMaybeANotSoSecretToken!')) ->will($this->returnValue('https://ownCloud.com/index.php/lostpassword/')); + $message = $this->getMockBuilder('\OC\Mail\Message') + ->disableOriginalConstructor()->getMock(); + $message + ->expects($this->at(0)) + ->method('setTo') + ->with(['test@example.com' => 'ExistingUser']); + $message + ->expects($this->at(1)) + ->method('setSubject') + ->with(' password reset'); + $message + ->expects($this->at(2)) + ->method('setPlainBody') + ->with('Use the following link to reset your password: https://ownCloud.com/index.php/lostpassword/'); + $message + ->expects($this->at(3)) + ->method('setFrom') + ->with(['lostpassword-noreply@localhost' => null]); + $this->container['Mailer'] + ->expects($this->at(0)) + ->method('createMessage') + ->will($this->returnValue($message)); + $this->container['Mailer'] + ->expects($this->at(1)) + ->method('send') + ->with($message); $response = $this->lostController->email('ExistingUser'); $expectedResponse = array('status' => 'success'); $this->assertSame($expectedResponse, $response); } + public function testEmailCantSendException() { + $randomToken = $this->container['SecureRandom']; + $this->container['SecureRandom'] + ->expects($this->once()) + ->method('generate') + ->with('21') + ->will($this->returnValue('ThisIsMaybeANotSoSecretToken!')); + $this->container['UserManager'] + ->expects($this->once()) + ->method('userExists') + ->with('ExistingUser') + ->will($this->returnValue(true)); + $this->container['Config'] + ->expects($this->once()) + ->method('getUserValue') + ->with('ExistingUser', 'settings', 'email') + ->will($this->returnValue('test@example.com')); + $this->container['SecureRandom'] + ->expects($this->once()) + ->method('getMediumStrengthGenerator') + ->will($this->returnValue($randomToken)); + $this->container['Config'] + ->expects($this->once()) + ->method('setUserValue') + ->with('ExistingUser', 'owncloud', 'lostpassword', 'ThisIsMaybeANotSoSecretToken!'); + $this->container['URLGenerator'] + ->expects($this->once()) + ->method('linkToRouteAbsolute') + ->with('core.lost.resetform', array('userId' => 'ExistingUser', 'token' => 'ThisIsMaybeANotSoSecretToken!')) + ->will($this->returnValue('https://ownCloud.com/index.php/lostpassword/')); + $message = $this->getMockBuilder('\OC\Mail\Message') + ->disableOriginalConstructor()->getMock(); + $message + ->expects($this->at(0)) + ->method('setTo') + ->with(['test@example.com' => 'ExistingUser']); + $message + ->expects($this->at(1)) + ->method('setSubject') + ->with(' password reset'); + $message + ->expects($this->at(2)) + ->method('setPlainBody') + ->with('Use the following link to reset your password: https://ownCloud.com/index.php/lostpassword/'); + $message + ->expects($this->at(3)) + ->method('setFrom') + ->with(['lostpassword-noreply@localhost' => null]); + $this->container['Mailer'] + ->expects($this->at(0)) + ->method('createMessage') + ->will($this->returnValue($message)); + $this->container['Mailer'] + ->expects($this->at(1)) + ->method('send') + ->with($message) + ->will($this->throwException(new \Exception())); + + $response = $this->lostController->email('ExistingUser'); + $expectedResponse = ['status' => 'error', 'msg' => 'Couldn\'t send reset email. Please contact your administrator.']; + $this->assertSame($expectedResponse, $response); + } + public function testSetPasswordUnsuccessful() { $this->container['Config'] ->expects($this->once())