From 13486a5ada62e4355473ca01e07371c162951e84 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 12 Feb 2015 13:53:27 +0100 Subject: Migrate to SwiftMail Replaces the OC_Mail and phpmailer with SwiftMail allowing us to mock it properly. Fixes the unit test execution on master on local machines and https://github.com/owncloud/core/issues/12014 Conflicts: 3rdparty lib/private/server.php lib/public/iservercontainer.php tests/lib/mail.php tests/settings/controller/mailsettingscontrollertest.php Conflicts: 3rdparty lib/private/mail.php lib/private/server.php lib/public/iservercontainer.php settings/ajax/lostpassword.php settings/application.php --- tests/lib/mail.php | 53 ------- tests/lib/mail/mailer.php | 118 +++++++++++++++ tests/lib/mail/message.php | 161 +++++++++++++++++++++ tests/lib/mail/util.php | 39 +++++ .../controller/mailsettingscontrollertest.php | 8 +- tests/settings/controller/userscontrollertest.php | 116 +++++++++------ 6 files changed, 393 insertions(+), 102 deletions(-) delete mode 100644 tests/lib/mail.php create mode 100644 tests/lib/mail/mailer.php create mode 100644 tests/lib/mail/message.php create mode 100644 tests/lib/mail/util.php (limited to 'tests') diff --git a/tests/lib/mail.php b/tests/lib/mail.php deleted file mode 100644 index 813dde1944f..00000000000 --- a/tests/lib/mail.php +++ /dev/null @@ -1,53 +0,0 @@ - - * This file is licensed under the Affero General Public License version 3 or - * later. - * See the COPYING-README file. - */ - -class Test_Mail extends \Test\TestCase { - - /** - * @dataProvider buildAsciiEmailProvider - * @param $expected - * @param $address - */ - public function testBuildAsciiEmail($expected, $address) { - if (!function_exists('idn_to_ascii')) { - $this->markTestSkipped( - 'The intl extension is not available.' - ); - } - - $actual = \OC_Mail::buildAsciiEmail($address); - $this->assertEquals($expected, $actual); - } - - public function buildAsciiEmailProvider() { - return array( - array('info@example.com', 'info@example.com'), - array('info@xn--cjr6vy5ejyai80u.com', 'info@國際化域名.com'), - array('info@xn--mller-kva.de', 'info@müller.de'), - array('info@xn--mller-kva.xn--mller-kva.de', 'info@müller.müller.de'), - ); - } - - public function validateMailProvider() { - return array( - array('infoatexample.com', false), - array('info', false), - ); - } - - /** - * @dataProvider validateMailProvider - * @param $address - * @param $expected - */ - public function testValidateEmail($address, $expected) { - $actual = \OC_Mail::validateAddress($address); - $this->assertEquals($expected, $actual); - } - -} diff --git a/tests/lib/mail/mailer.php b/tests/lib/mail/mailer.php new file mode 100644 index 00000000000..bd410dd3383 --- /dev/null +++ b/tests/lib/mail/mailer.php @@ -0,0 +1,118 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test; +use OC\Mail\Mailer; +use OCP\IConfig; +use OC_Defaults; + +class MailerTest extends TestCase { + /** @var IConfig */ + private $config; + /** @var OC_Defaults */ + private $defaults; + /** @var Mailer */ + private $mailer; + + function setUp() { + parent::setUp(); + + $this->config = $this->getMockBuilder('\OCP\IConfig') + ->disableOriginalConstructor()->getMock(); + $this->defaults = $this->getMockBuilder('\OC_Defaults') + ->disableOriginalConstructor()->getMock(); + $this->mailer = new Mailer($this->config, $this->defaults); + } + + public function testGetMailInstance() { + $this->assertEquals(\Swift_MailTransport::newInstance(), \Test_Helper::invokePrivate($this->mailer, 'getMailinstance')); + } + + public function testGetSendMailInstanceSendMail() { + $this->config + ->expects($this->once()) + ->method('getSystemValue') + ->with('mail_smtpmode', 'sendmail') + ->will($this->returnValue('sendmail')); + + $this->assertEquals(\Swift_SendmailTransport::newInstance('/usr/sbin/sendmail -bs'), \Test_Helper::invokePrivate($this->mailer, 'getSendMailInstance')); + } + + public function testGetSendMailInstanceSendMailQmail() { + $this->config + ->expects($this->once()) + ->method('getSystemValue') + ->with('mail_smtpmode', 'sendmail') + ->will($this->returnValue('qmail')); + + $this->assertEquals(\Swift_SendmailTransport::newInstance('/var/qmail/bin/sendmail -bs'), \Test_Helper::invokePrivate($this->mailer, 'getSendMailInstance')); + } + + public function testGetSmtpInstanceDefaults() { + $expected = \Swift_SmtpTransport::newInstance(); + $expected->setHost('127.0.0.1'); + $expected->setTimeout(10); + $expected->setPort(25); + + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->will($this->returnArgument(1)); + + $this->assertEquals($expected, \Test_Helper::invokePrivate($this->mailer, 'getSmtpInstance')); + } + + public function testGetInstanceDefault() { + $this->assertInstanceOf('\Swift_MailTransport', \Test_Helper::invokePrivate($this->mailer, 'getInstance')); + } + + public function testGetInstancePhp() { + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->will($this->returnValue('php')); + + $this->assertInstanceOf('\Swift_MailTransport', \Test_Helper::invokePrivate($this->mailer, 'getInstance')); + } + + public function testGetInstanceSmtp() { + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->will($this->returnValue('smtp')); + + $this->assertInstanceOf('\Swift_SmtpTransport', \Test_Helper::invokePrivate($this->mailer, 'getInstance')); + } + + public function testGetInstanceSendmail() { + $this->config + ->expects($this->any()) + ->method('getSystemValue') + ->will($this->returnValue('sendmail')); + + $this->assertInstanceOf('\Swift_SendmailTransport', \Test_Helper::invokePrivate($this->mailer, 'getInstance')); + } + + public function testCreateMessage() { + $this->assertInstanceOf('\OC\Mail\Message', $this->mailer->createMessage()); + } + + /** + * @expectedException \Exception + */ + public function testSendInvalidMailException() { + $message = $this->getMockBuilder('\OC\Mail\Message') + ->disableOriginalConstructor()->getMock(); + $message->expects($this->once()) + ->method('getSwiftMessage') + ->will($this->returnValue(new \Swift_Message())); + + $this->mailer->send($message); + } + +} diff --git a/tests/lib/mail/message.php b/tests/lib/mail/message.php new file mode 100644 index 00000000000..0db2017d81e --- /dev/null +++ b/tests/lib/mail/message.php @@ -0,0 +1,161 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test; + +use OC\Mail\Message; +use Swift_Message; + +class MessageTest extends TestCase { + /** @var Swift_Message */ + private $swiftMessage; + /** @var Message */ + private $message; + + /** + * @return array + */ + public function mailAddressProvider() { + return array( + array(array('lukas@owncloud.com' => 'Lukas Reschke'), array('lukas@owncloud.com' => 'Lukas Reschke')), + array(array('lukas@owncloud.com' => 'Lukas Reschke', 'lukas@öwnclöüd.com', 'lukäs@owncloud.örg' => 'Lükäs Réschke'), + array('lukas@owncloud.com' => 'Lukas Reschke', 'lukas@xn--wncld-iuae2c.com', 'lukäs@owncloud.xn--rg-eka' => 'Lükäs Réschke')), + array(array('lukas@öwnclöüd.com'), array('lukas@xn--wncld-iuae2c.com')) + ); + } + + function setUp() { + parent::setUp(); + + $this->swiftMessage = $this->getMockBuilder('\Swift_Message') + ->disableOriginalConstructor()->getMock(); + + $this->message = new Message($this->swiftMessage); + } + + /** + * @dataProvider mailAddressProvider + */ + public function testConvertAddresses($unconverted, $expected) { + $this->assertSame($expected, \Test_Helper::invokePrivate($this->message, 'convertAddresses', array($unconverted))); + } + + public function testSetFrom() { + $this->swiftMessage + ->expects($this->once()) + ->method('setFrom') + ->with(array('lukas@owncloud.com')); + $this->message->setFrom(array('lukas@owncloud.com')); + } + + public function testGetFrom() { + $this->swiftMessage + ->expects($this->once()) + ->method('getFrom') + ->will($this->returnValue(array('lukas@owncloud.com'))); + + $this->assertSame(array('lukas@owncloud.com'), $this->message->getFrom()); + } + + public function testSetTo() { + $this->swiftMessage + ->expects($this->once()) + ->method('setTo') + ->with(array('lukas@owncloud.com')); + $this->message->setTo(array('lukas@owncloud.com')); + } + + public function testGetTo() { + $this->swiftMessage + ->expects($this->once()) + ->method('getTo') + ->will($this->returnValue(array('lukas@owncloud.com'))); + + $this->assertSame(array('lukas@owncloud.com'), $this->message->getTo()); + } + + public function testSetCc() { + $this->swiftMessage + ->expects($this->once()) + ->method('setCc') + ->with(array('lukas@owncloud.com')); + $this->message->setCc(array('lukas@owncloud.com')); + } + + public function testGetCc() { + $this->swiftMessage + ->expects($this->once()) + ->method('getCc') + ->will($this->returnValue(array('lukas@owncloud.com'))); + + $this->assertSame(array('lukas@owncloud.com'), $this->message->getCc()); + } + + public function testSetBcc() { + $this->swiftMessage + ->expects($this->once()) + ->method('setBcc') + ->with(array('lukas@owncloud.com')); + $this->message->setBcc(array('lukas@owncloud.com')); + } + + public function testGetBcc() { + $this->swiftMessage + ->expects($this->once()) + ->method('getBcc') + ->will($this->returnValue(array('lukas@owncloud.com'))); + + $this->assertSame(array('lukas@owncloud.com'), $this->message->getBcc()); + } + + public function testSetSubject() { + $this->swiftMessage + ->expects($this->once()) + ->method('setSubject') + ->with('Fancy Subject'); + + $this->message->setSubject('Fancy Subject'); + } + + public function testGetSubject() { + $this->swiftMessage + ->expects($this->once()) + ->method('getSubject') + ->will($this->returnValue('Fancy Subject')); + + $this->assertSame('Fancy Subject', $this->message->getSubject()); + } + + public function testSetPlainBody() { + $this->swiftMessage + ->expects($this->once()) + ->method('setBody') + ->with('Fancy Body'); + + $this->message->setPlainBody('Fancy Body'); + } + + public function testGetPlainBody() { + $this->swiftMessage + ->expects($this->once()) + ->method('getBody') + ->will($this->returnValue('Fancy Body')); + + $this->assertSame('Fancy Body', $this->message->getPlainBody()); + } + + public function testSetHtmlBody() { + $this->swiftMessage + ->expects($this->once()) + ->method('addPart') + ->with('Fancy Body', 'text/html'); + + $this->message->setHtmlBody('Fancy Body'); + } + +} diff --git a/tests/lib/mail/util.php b/tests/lib/mail/util.php new file mode 100644 index 00000000000..04d9d5df256 --- /dev/null +++ b/tests/lib/mail/util.php @@ -0,0 +1,39 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test; +use OCP\Mail\Util; + +/** + * Class Util + * + * @package OC\Mail + */ +class UtilTest extends TestCase { + + /** + * @return array + */ + public function mailAddressProvider() { + return array( + array('lukas@owncloud.com', true), + array('lukas@localhost', true), + array('lukas@192.168.1.1', true), + array('lukas@éxämplè.com', true), + array('asdf', false), + array('lukas@owncloud.org@owncloud.com', false) + ); + } + + /** + * @dataProvider mailAddressProvider + */ + public function testValidateMailAddress($email, $expected) { + $this->assertSame($expected, Util::validateMailAddress($email)); + } +} diff --git a/tests/settings/controller/mailsettingscontrollertest.php b/tests/settings/controller/mailsettingscontrollertest.php index ed33d7fbe49..84432f656bf 100644 --- a/tests/settings/controller/mailsettingscontrollertest.php +++ b/tests/settings/controller/mailsettingscontrollertest.php @@ -30,7 +30,7 @@ class MailSettingsControllerTest extends \Test\TestCase { $this->container['AppName'] = 'settings'; $this->container['UserSession'] = $this->getMockBuilder('\OC\User\Session') ->disableOriginalConstructor()->getMock(); - $this->container['Mail'] = $this->getMockBuilder('\OC_Mail') + $this->container['MailMessage'] = $this->getMockBuilder('\OCP\Mail\IMessage') ->disableOriginalConstructor()->getMock(); $this->container['Defaults'] = $this->getMockBuilder('\OC_Defaults') ->disableOriginalConstructor()->getMock(); @@ -152,12 +152,6 @@ class MailSettingsControllerTest extends \Test\TestCase { } public function testSendTestMail() { - /** - * FIXME: Disabled due to missing DI on mail class. - * TODO: Re-enable when https://github.com/owncloud/core/pull/12085 is merged. - */ - $this->markTestSkipped('Disable test until OC_Mail is rewritten.'); - $user = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor() ->getMock(); diff --git a/tests/settings/controller/userscontrollertest.php b/tests/settings/controller/userscontrollertest.php index b813da038a3..3f69d1e7d18 100644 --- a/tests/settings/controller/userscontrollertest.php +++ b/tests/settings/controller/userscontrollertest.php @@ -45,7 +45,7 @@ class UsersControllerTest extends \Test\TestCase { })); $this->container['Defaults'] = $this->getMockBuilder('\OC_Defaults') ->disableOriginalConstructor()->getMock(); - $this->container['Mail'] = $this->getMockBuilder('\OC_Mail') + $this->container['Mailer'] = $this->getMockBuilder('\OCP\Mail\IMailer') ->disableOriginalConstructor()->getMock(); $this->container['DefaultMailAddress'] = 'no-reply@owncloud.com'; $this->container['Logger'] = $this->getMockBuilder('\OCP\ILogger') @@ -1151,24 +1151,12 @@ class UsersControllerTest extends \Test\TestCase { public function testCreateUnsuccessfulWithInvalidEmailAdmin() { $this->container['IsAdmin'] = true; - /** - * FIXME: Disabled due to missing DI on mail class. - * TODO: Re-enable when https://github.com/owncloud/core/pull/12085 is merged. - */ - $this->markTestSkipped('Disable test until OC_Mail is rewritten.'); - - $this->container['Mail'] - ->expects($this->once()) - ->method('validateAddress') - ->will($this->returnValue(false)); - - $expectedResponse = new DataResponse( - array( - 'message' => 'Invalid mail address' - ), + $expectedResponse = new DataResponse([ + 'message' => 'Invalid mail address', + ], Http::STATUS_UNPROCESSABLE_ENTITY ); - $response = $this->container['UsersController']->create('foo', 'password', array(), 'invalidMailAdress'); + $response = $this->container['UsersController']->create('foo', 'password', [], 'invalidMailAdress'); $this->assertEquals($expectedResponse, $response); } @@ -1177,35 +1165,79 @@ class UsersControllerTest extends \Test\TestCase { */ public function testCreateSuccessfulWithValidEmailAdmin() { $this->container['IsAdmin'] = true; + $message = $this->getMockBuilder('\OC\Mail\Message') + ->disableOriginalConstructor()->getMock(); + $message + ->expects($this->at(0)) + ->method('setTo') + ->with(['validMail@Adre.ss' => 'foo']); + $message + ->expects($this->at(1)) + ->method('setSubject') + ->with('Your account was created'); + $htmlBody = new Http\TemplateResponse( + 'settings', + 'email.new_user', + [ + 'username' => 'foo', + 'url' => '', + ], + 'blank' + ); + $message + ->expects($this->at(2)) + ->method('setHtmlBody') + ->with($htmlBody->render()); + $plainBody = new Http\TemplateResponse( + 'settings', + 'email.new_user_plain_text', + [ + 'username' => 'foo', + 'url' => '', + ], + 'blank' + ); + $message + ->expects($this->at(3)) + ->method('setPlainBody') + ->with($plainBody->render()); + $message + ->expects($this->at(4)) + ->method('setFrom') + ->with(['no-reply@owncloud.com' => 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); - /** - * FIXME: Disabled due to missing DI on mail class. - * TODO: Re-enable when https://github.com/owncloud/core/pull/12085 is merged. - */ - $this->markTestSkipped('Disable test until OC_Mail is rewritten.'); - - $this->container['Mail'] + $user = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $user + ->method('getHome') + ->will($this->returnValue('/home/user')); + $user + ->method('getHome') + ->will($this->returnValue('/home/user')); + $user + ->method('getUID') + ->will($this->returnValue('foo')); + $user ->expects($this->once()) - ->method('validateAddress') - ->will($this->returnValue(true)); - $this->container['Mail'] + ->method('getBackendClassName') + ->will($this->returnValue('bar')); + + $this->container['UserManager'] ->expects($this->once()) - ->method('send') - ->with( - $this->equalTo('validMail@Adre.ss'), - $this->equalTo('foo'), - $this->anything(), - $this->anything(), - $this->anything(), - $this->equalTo('no-reply@owncloud.com'), - $this->equalTo(1), - $this->anything() - ); - $this->container['Logger'] - ->expects($this->never()) - ->method('error'); + ->method('createUser') + ->will($this->onConsecutiveCalls($user)); + - $response = $this->container['UsersController']->create('foo', 'password', array(), 'validMail@Adre.ss'); + $response = $this->container['UsersController']->create('foo', 'password', [], 'validMail@Adre.ss'); $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); } -- cgit v1.2.3 From 283476a2f7c200912352eb4db097f540e3993e75 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 12 Feb 2015 16:03:51 +0100 Subject: Use new IMailer and add unit-tests for lostcontroller --- core/application.php | 6 +- core/lostpassword/controller/lostcontroller.php | 24 +++-- .../lostpassword/controller/lostcontrollertest.php | 101 +++++++++++++++++++-- 3 files changed, 111 insertions(+), 20 deletions(-) (limited to 'tests') diff --git a/core/application.php b/core/application.php index 568fc34db7d..34f817aa391 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 d1a4528d449..08f5246503f 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 b03cbd7c42f..f82fc1ba3ff 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()) -- cgit v1.2.3 From f92f3a1a6ecbce06fbe24204ef12b2eeeb0f0d15 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 19 Feb 2015 18:55:27 +0100 Subject: Incorporate review changes --- lib/private/mail/mailer.php | 30 +++++++++++++++++++++- lib/private/mail/util.php | 45 --------------------------------- lib/public/mail/imailer.php | 8 ++++++ lib/public/mail/util.php | 26 ------------------- lib/public/util.php | 3 ++- settings/controller/userscontroller.php | 5 ++-- tests/lib/mail/mailer.php | 23 ++++++++++++++++- tests/lib/mail/util.php | 39 ---------------------------- 8 files changed, 63 insertions(+), 116 deletions(-) delete mode 100644 lib/private/mail/util.php delete mode 100644 lib/public/mail/util.php delete mode 100644 tests/lib/mail/util.php (limited to 'tests') diff --git a/lib/private/mail/mailer.php b/lib/private/mail/mailer.php index ff4b0a0ef62..92e6b91ce1c 100644 --- a/lib/private/mail/mailer.php +++ b/lib/private/mail/mailer.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. @@ -77,6 +77,34 @@ class Mailer implements IMailer { return $failedRecipients; } + /** + * Checks if an e-mail address is valid + * + * @param string $email Email address to be validated + * @return bool True if the mail address is valid, false otherwise + */ + public function validateMailAddress($email) { + return \Swift_Validate::email($this->convertEmail($email)); + } + + /** + * SwiftMailer does currently not work with IDN domains, this function therefore converts the domains + * + * FIXME: Remove this once SwiftMailer supports IDN + * + * @param string $email + * @return string Converted mail address if `idn_to_ascii` exists + */ + protected function convertEmail($email) { + if (!function_exists('idn_to_ascii') || strpos($email, '@') === false) { + return $email; + } + + list($name, $domain) = explode('@', $email, 2); + $domain = idn_to_ascii($domain); + return $name.'@'.$domain; + } + /** * Returns whatever transport is configured within the config * diff --git a/lib/private/mail/util.php b/lib/private/mail/util.php deleted file mode 100644 index b301e79d492..00000000000 --- a/lib/private/mail/util.php +++ /dev/null @@ -1,45 +0,0 @@ - - * This file is licensed under the Affero General Public License version 3 or - * later. - * See the COPYING-README file. - */ - -namespace OC\Mail; - -/** - * Class Util - * - * @package OC\Mail - */ -class Util { - /** - * Checks if an e-mail address is valid - * - * @param string $email Email address to be validated - * @return bool True if the mail address is valid, false otherwise - */ - public static function validateMailAddress($email) { - return \Swift_Validate::email(self::convertEmail($email)); - } - - /** - * SwiftMailer does currently not work with IDN domains, this function therefore converts the domains - * - * FIXME: Remove this once SwiftMailer supports IDN - * - * @param string $email - * @return string Converted mail address if `idn_to_ascii` exists - */ - protected static function convertEmail($email) { - if (!function_exists('idn_to_ascii') || strpos($email, '@') === false) { - return $email; - } - - list($name, $domain) = explode('@', $email, 2); - $domain = idn_to_ascii($domain); - return $name.'@'.$domain; - } - -} diff --git a/lib/public/mail/imailer.php b/lib/public/mail/imailer.php index 2a20ead612b..dc6fe5ba869 100644 --- a/lib/public/mail/imailer.php +++ b/lib/public/mail/imailer.php @@ -46,4 +46,12 @@ interface IMailer { * has been supplied.) */ public function send(Message $message); + + /** + * Checks if an e-mail address is valid + * + * @param string $email Email address to be validated + * @return bool True if the mail address is valid, false otherwise + */ + public function validateMailAddress($email); } diff --git a/lib/public/mail/util.php b/lib/public/mail/util.php deleted file mode 100644 index ea59649e620..00000000000 --- a/lib/public/mail/util.php +++ /dev/null @@ -1,26 +0,0 @@ - - * This file is licensed under the Affero General Public License version 3 or - * later. - * See the COPYING-README file. - */ - -namespace OCP\Mail; - -/** - * Class Util provides some helpers for mail addresses - * - * @package OCP\Mail - */ -class Util { - /** - * Checks if an e-mail address is valid - * - * @param string $email Email address to be validated - * @return bool True if the mail address is valid, false otherwise - */ - public static function validateMailAddress($email) { - return \OC\Mail\Util::validateMailAddress($email); - } -} diff --git a/lib/public/util.php b/lib/public/util.php index 71ab8cabce5..338c216f255 100644 --- a/lib/public/util.php +++ b/lib/public/util.php @@ -294,7 +294,8 @@ class Util { $host_name = \OC_Config::getValue('mail_domain', $host_name); $defaultEmailAddress = $user_part.'@'.$host_name; - if (\OCP\Mail\Util::validateMailAddress($defaultEmailAddress)) { + $mailer = \OC::$server->getMailer(); + if ($mailer->validateMailAddress($defaultEmailAddress)) { return $defaultEmailAddress; } diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php index 507e57ef940..11afe514016 100644 --- a/settings/controller/userscontroller.php +++ b/settings/controller/userscontroller.php @@ -263,8 +263,7 @@ class UsersController extends Controller { * @return DataResponse */ public function create($username, $password, array $groups=array(), $email='') { - - if($email !== '' && !\OCP\Mail\Util::validateMailAddress($email)) { + if($email !== '' && !$this->mailer->validateMailAddress($email)) { return new DataResponse( array( 'message' => (string)$this->l10n->t('Invalid mail address') @@ -443,7 +442,7 @@ class UsersController extends Controller { ); } - if($mailAddress !== '' && ! \OCP\Mail\Util::validateMailAddress($mailAddress)) { + if($mailAddress !== '' && !$this->mailer->validateMailAddress($mailAddress)) { return new DataResponse( array( 'status' => 'error', diff --git a/tests/lib/mail/mailer.php b/tests/lib/mail/mailer.php index bd410dd3383..2cb4c5cfde3 100644 --- a/tests/lib/mail/mailer.php +++ b/tests/lib/mail/mailer.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. @@ -115,4 +115,25 @@ class MailerTest extends TestCase { $this->mailer->send($message); } + /** + * @return array + */ + public function mailAddressProvider() { + return [ + ['lukas@owncloud.com', true], + ['lukas@localhost', true], + ['lukas@192.168.1.1', true], + ['lukas@éxämplè.com', true], + ['asdf', false], + ['lukas@owncloud.org@owncloud.com', false], + ]; + } + + /** + * @dataProvider mailAddressProvider + */ + public function testValidateMailAddress($email, $expected) { + $this->assertSame($expected, $this->mailer->validateMailAddress($email)); + } + } diff --git a/tests/lib/mail/util.php b/tests/lib/mail/util.php deleted file mode 100644 index 04d9d5df256..00000000000 --- a/tests/lib/mail/util.php +++ /dev/null @@ -1,39 +0,0 @@ - - * This file is licensed under the Affero General Public License version 3 or - * later. - * See the COPYING-README file. - */ - -namespace Test; -use OCP\Mail\Util; - -/** - * Class Util - * - * @package OC\Mail - */ -class UtilTest extends TestCase { - - /** - * @return array - */ - public function mailAddressProvider() { - return array( - array('lukas@owncloud.com', true), - array('lukas@localhost', true), - array('lukas@192.168.1.1', true), - array('lukas@éxämplè.com', true), - array('asdf', false), - array('lukas@owncloud.org@owncloud.com', false) - ); - } - - /** - * @dataProvider mailAddressProvider - */ - public function testValidateMailAddress($email, $expected) { - $this->assertSame($expected, Util::validateMailAddress($email)); - } -} -- cgit v1.2.3 From dfd70337d6db5e6e15f6763d5e8762f189e9fd71 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 19 Feb 2015 19:40:05 +0100 Subject: Adjust unit test --- tests/settings/controller/userscontrollertest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/settings/controller/userscontrollertest.php b/tests/settings/controller/userscontrollertest.php index 3f69d1e7d18..25c935bef58 100644 --- a/tests/settings/controller/userscontrollertest.php +++ b/tests/settings/controller/userscontrollertest.php @@ -1208,10 +1208,15 @@ class UsersControllerTest extends \Test\TestCase { $this->container['Mailer'] ->expects($this->at(0)) + ->method('validateMailAddress') + ->with('validMail@Adre.ss') + ->will($this->returnValue(true)); + $this->container['Mailer'] + ->expects($this->at(1)) ->method('createMessage') ->will($this->returnValue($message)); $this->container['Mailer'] - ->expects($this->at(1)) + ->expects($this->at(2)) ->method('send') ->with($message); -- cgit v1.2.3 From e32f1582c76252ae80802c0851a9583504131dff Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 19 Feb 2015 21:55:02 +0100 Subject: Show more detailed error message --- lib/private/mail/mailer.php | 1 + settings/controller/mailsettingscontroller.php | 24 +++++++++++----------- tests/lib/mail/mailer.php | 23 --------------------- .../controller/mailsettingscontrollertest.php | 3 +++ 4 files changed, 16 insertions(+), 35 deletions(-) (limited to 'tests') diff --git a/lib/private/mail/mailer.php b/lib/private/mail/mailer.php index 92e6b91ce1c..83dd050edbc 100644 --- a/lib/private/mail/mailer.php +++ b/lib/private/mail/mailer.php @@ -149,6 +149,7 @@ class Mailer implements IMailer { if (!empty($smtpSecurity)) { $transport->setEncryption($smtpSecurity); } + $transport->start(); return $transport; } diff --git a/settings/controller/mailsettingscontroller.php b/settings/controller/mailsettingscontroller.php index 43715b82237..53365cf253b 100644 --- a/settings/controller/mailsettingscontroller.php +++ b/settings/controller/mailsettingscontroller.php @@ -135,19 +135,19 @@ class MailSettingsController extends Controller { $email = $this->config->getUserValue($this->userSession->getUser()->getUID(), $this->appName, 'email', ''); if (!empty($email)) { try { - $this->mail->send($email, $this->userSession->getUser()->getDisplayName(), - $this->l10n->t('test email settings'), - $this->l10n->t('If you received this email, the settings seem to be correct.'), - $this->defaultMailAddress, - $this->defaults->getName() - ); + $message = $this->mailer->createMessage(); + $message->setTo([$email => $this->userSession->getUser()->getDisplayName()]); + $message->setFrom([$this->defaultMailAddress]); + $message->setSubject($this->l10n->t('test email settings')); + $message->setPlainBody('If you received this email, the settings seem to be correct.'); + $this->mailer->send($message); } catch (\Exception $e) { - return array('data' => - array('message' => - (string) $this->l10n->t('A problem occurred while sending the email. Please revise your settings.'), - ), - 'status' => 'error' - ); + return [ + 'data' => [ + 'message' => (string) $this->l10n->t('A problem occurred while sending the email. Please revise your settings. (Error: %s)', [$e->getMessage()]), + ], + 'status' => 'error', + ]; } return array('data' => diff --git a/tests/lib/mail/mailer.php b/tests/lib/mail/mailer.php index 2cb4c5cfde3..8929bfdf99d 100644 --- a/tests/lib/mail/mailer.php +++ b/tests/lib/mail/mailer.php @@ -53,20 +53,6 @@ class MailerTest extends TestCase { $this->assertEquals(\Swift_SendmailTransport::newInstance('/var/qmail/bin/sendmail -bs'), \Test_Helper::invokePrivate($this->mailer, 'getSendMailInstance')); } - public function testGetSmtpInstanceDefaults() { - $expected = \Swift_SmtpTransport::newInstance(); - $expected->setHost('127.0.0.1'); - $expected->setTimeout(10); - $expected->setPort(25); - - $this->config - ->expects($this->any()) - ->method('getSystemValue') - ->will($this->returnArgument(1)); - - $this->assertEquals($expected, \Test_Helper::invokePrivate($this->mailer, 'getSmtpInstance')); - } - public function testGetInstanceDefault() { $this->assertInstanceOf('\Swift_MailTransport', \Test_Helper::invokePrivate($this->mailer, 'getInstance')); } @@ -80,15 +66,6 @@ class MailerTest extends TestCase { $this->assertInstanceOf('\Swift_MailTransport', \Test_Helper::invokePrivate($this->mailer, 'getInstance')); } - public function testGetInstanceSmtp() { - $this->config - ->expects($this->any()) - ->method('getSystemValue') - ->will($this->returnValue('smtp')); - - $this->assertInstanceOf('\Swift_SmtpTransport', \Test_Helper::invokePrivate($this->mailer, 'getInstance')); - } - public function testGetInstanceSendmail() { $this->config ->expects($this->any()) diff --git a/tests/settings/controller/mailsettingscontrollertest.php b/tests/settings/controller/mailsettingscontrollertest.php index 84432f656bf..cc25fda52f7 100644 --- a/tests/settings/controller/mailsettingscontrollertest.php +++ b/tests/settings/controller/mailsettingscontrollertest.php @@ -32,6 +32,9 @@ class MailSettingsControllerTest extends \Test\TestCase { ->disableOriginalConstructor()->getMock(); $this->container['MailMessage'] = $this->getMockBuilder('\OCP\Mail\IMessage') ->disableOriginalConstructor()->getMock(); + $this->container['Mailer'] = $this->getMockBuilder('\OC\Mail\Mailer') + ->setMethods(['send']) + ->disableOriginalConstructor()->getMock(); $this->container['Defaults'] = $this->getMockBuilder('\OC_Defaults') ->disableOriginalConstructor()->getMock(); $this->container['DefaultMailAddress'] = 'no-reply@owncloud.com'; -- cgit v1.2.3 From d7c7808a5e789180f86259ef6a91b17c1bcf737b Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 16 Mar 2015 13:01:17 +0100 Subject: Add debug log message back --- lib/private/mail/mailer.php | 15 ++++++++++++--- lib/private/server.php | 6 +++++- tests/lib/mail/mailer.php | 7 ++++++- 3 files changed, 23 insertions(+), 5 deletions(-) (limited to 'tests') diff --git a/lib/private/mail/mailer.php b/lib/private/mail/mailer.php index 83dd050edbc..a995322d89e 100644 --- a/lib/private/mail/mailer.php +++ b/lib/private/mail/mailer.php @@ -10,6 +10,7 @@ namespace OC\Mail; use OCP\IConfig; use OCP\Mail\IMailer; +use OCP\ILogger; /** * Class Mailer provides some basic functions to create a mail message that can be used in combination with @@ -34,15 +35,21 @@ class Mailer implements IMailer { private $instance = null; /** @var IConfig */ private $config; + /** @var ILogger */ + private $logger; /** @var \OC_Defaults */ private $defaults; /** * @param IConfig $config + * @param ILogger $logger * @param \OC_Defaults $defaults */ - function __construct(IConfig $config, \OC_Defaults $defaults) { + function __construct(IConfig $config, + ILogger $logger, + \OC_Defaults $defaults) { $this->config = $config; + $this->logger = $logger; $this->defaults = $defaults; } @@ -67,12 +74,14 @@ class Mailer implements IMailer { */ public function send(Message $message) { if (sizeof($message->getFrom()) === 0) { - $message->setFrom(array(\OCP\Util::getDefaultEmailAddress($this->defaults->getName()))); + $message->setFrom([\OCP\Util::getDefaultEmailAddress($this->defaults->getName())]); } - $failedRecipients = array(); + $failedRecipients = []; $this->getInstance()->send($message->getSwiftMessage(), $failedRecipients); + $logMessage = sprintf('Sent mail to "%s" with subject "%s"', print_r($message->getTo(), true), $message->getSubject()); + $this->logger->debug($logMessage, ['app' => 'core']); return $failedRecipients; } diff --git a/lib/private/server.php b/lib/private/server.php index a3682dea6b3..2f688c47f09 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -314,7 +314,11 @@ class Server extends SimpleContainer implements IServerContainer { ); }); $this->registerService('Mailer', function(Server $c) { - return new Mailer($c->getConfig(), new \OC_Defaults()); + return new Mailer( + $c->getConfig(), + $c->getLogger(), + new \OC_Defaults() + ); }); } diff --git a/tests/lib/mail/mailer.php b/tests/lib/mail/mailer.php index 8929bfdf99d..7e5e689741a 100644 --- a/tests/lib/mail/mailer.php +++ b/tests/lib/mail/mailer.php @@ -10,12 +10,15 @@ namespace Test; use OC\Mail\Mailer; use OCP\IConfig; use OC_Defaults; +use OCP\ILogger; class MailerTest extends TestCase { /** @var IConfig */ private $config; /** @var OC_Defaults */ private $defaults; + /** @var ILogger */ + private $logger; /** @var Mailer */ private $mailer; @@ -26,7 +29,9 @@ class MailerTest extends TestCase { ->disableOriginalConstructor()->getMock(); $this->defaults = $this->getMockBuilder('\OC_Defaults') ->disableOriginalConstructor()->getMock(); - $this->mailer = new Mailer($this->config, $this->defaults); + $this->logger = $this->getMockBuilder('\OCP\ILogger') + ->disableOriginalConstructor()->getMock(); + $this->mailer = new Mailer($this->config, $this->logger, $this->defaults); } public function testGetMailInstance() { -- cgit v1.2.3