diff --git a/lib/private/Settings/Admin/Additional.php b/lib/private/Settings/Admin/Additional.php index d133e4737a76b..59058851a649c 100644 --- a/lib/private/Settings/Admin/Additional.php +++ b/lib/private/Settings/Admin/Additional.php @@ -65,6 +65,10 @@ public function getForm() { 'mail_smtppassword' => $this->config->getSystemValue('mail_smtppassword', ''), ]; + if ($parameters['mail_smtppassword'] !== '') { + $parameters['mail_smtppassword'] = '********'; + } + return new TemplateResponse('settings', 'admin/additional-mail', $parameters, ''); } diff --git a/settings/Controller/MailSettingsController.php b/settings/Controller/MailSettingsController.php index 8137b4da53cbe..f0fd7a52f0b9e 100644 --- a/settings/Controller/MailSettingsController.php +++ b/settings/Controller/MailSettingsController.php @@ -1,5 +1,6 @@ * @copyright Copyright (c) 2016, ownCloud, Inc. * * @author Joas Schilling @@ -25,6 +26,8 @@ namespace OC\Settings\Controller; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\DataResponse; use OCP\IRequest; use OCP\IL10N; use OCP\IConfig; @@ -84,7 +87,7 @@ public function __construct($appName, * @param string $mail_smtpauthtype * @param int $mail_smtpauth * @param string $mail_smtpport - * @return array + * @return DataResponse */ public function setMailSettings($mail_domain, $mail_from_address, @@ -109,12 +112,7 @@ public function setMailSettings($mail_domain, $this->config->setSystemValues($configs); - return array('data' => - array('message' => - (string) $this->l10n->t('Saved') - ), - 'status' => 'success' - ); + return new DataResponse(); } /** @@ -124,25 +122,24 @@ public function setMailSettings($mail_domain, * * @param string $mail_smtpname * @param string $mail_smtppassword - * @return array + * @return DataResponse */ public function storeCredentials($mail_smtpname, $mail_smtppassword) { + if ($mail_smtppassword === '********') { + return new DataResponse($this->l10n->t('Invalid SMTP password.'), Http::STATUS_BAD_REQUEST); + } + $this->config->setSystemValues([ 'mail_smtpname' => $mail_smtpname, 'mail_smtppassword' => $mail_smtppassword, ]); - return array('data' => - array('message' => - (string) $this->l10n->t('Saved') - ), - 'status' => 'success' - ); + return new DataResponse(); } /** * Send a mail to test the settings - * @return array + * @return DataResponse */ public function sendTestMail() { $email = $this->config->getUserValue($this->userSession->getUser()->getUID(), $this->appName, 'email', ''); @@ -157,29 +154,13 @@ public function sendTestMail() { if (!empty($errors)) { throw new \RuntimeException($this->l10n->t('Mail could not be sent. Check your mail server log')); } + return new DataResponse(); } catch (\Exception $e) { - 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 new DataResponse($this->l10n->t('A problem occurred while sending the email. Please revise your settings. (Error: %s)', [$e->getMessage()]), Http::STATUS_BAD_REQUEST); } - - return array('data' => - array('message' => - (string) $this->l10n->t('Email sent') - ), - 'status' => 'success' - ); } - return array('data' => - array('message' => - (string) $this->l10n->t('You need to set your user email before being able to send test emails.'), - ), - 'status' => 'error' - ); + return new DataResponse($this->l10n->t('You need to set your user email before being able to send test emails.'), Http::STATUS_BAD_REQUEST); } } diff --git a/settings/js/admin.js b/settings/js/admin.js index 985e318e34b01..475fecf604eca 100644 --- a/settings/js/admin.js +++ b/settings/js/admin.js @@ -186,11 +186,11 @@ $(document).ready(function(){ url: OC.generateUrl('/settings/admin/mailsettings'), type: 'POST', data: $('#mail_general_settings_form').serialize(), - success: function(data){ - OC.msg.finishedSaving('#mail_settings_msg', data); + success: function(){ + OC.msg.finishedSuccess('#mail_settings_msg', t('settings', 'Saved')); }, - error: function(data){ - OC.msg.finishedError('#mail_settings_msg', data.responseJSON.message); + error: function(xhr){ + OC.msg.finishedError('#mail_settings_msg', xhr.responseJSON); } }); }; @@ -206,21 +206,39 @@ $(document).ready(function(){ url: OC.generateUrl('/settings/admin/mailsettings/credentials'), type: 'POST', data: $('#mail_credentials_settings').serialize(), - success: function(data){ - OC.msg.finishedSaving('#mail_settings_msg', data); + success: function(){ + OC.msg.finishedSuccess('#mail_settings_msg', t('settings', 'Saved')); }, - error: function(data){ - OC.msg.finishedError('#mail_settings_msg', data.responseJSON.message); + error: function(xhr){ + OC.msg.finishedError('#mail_settings_msg', xhr.responseJSON); } }); }; $('#mail_general_settings_form').change(changeEmailSettings); $('#mail_credentials_settings_submit').click(toggleEmailCredentials); + $('#mail_smtppassword').click(function() { + if (this.type === 'text' && this.value === '********') { + this.type = 'password'; + this.value = ''; + } + }); $('#sendtestemail').click(function(event){ event.preventDefault(); - OC.msg.startAction('#sendtestmail_msg', t('settings', 'Sending...')); + OC.msg.startAction('#sendtestmail_msg', t('settings', 'Sending…')); + + $.ajax({ + url: OC.generateUrl('/settings/admin/mailtest'), + type: 'POST', + data: $('#mail_credentials_settings').serialize(), + success: function(){ + OC.msg.finishedSuccess('#sendtestmail_msg', t('settings', 'Email sent')); + }, + error: function(xhr){ + OC.msg.finishedError('#sendtestmail_msg', xhr.responseJSON); + } + }); $.post(OC.generateUrl('/settings/admin/mailtest'), '', function(data){ OC.msg.finishedAction('#sendtestmail_msg', data); }); diff --git a/settings/templates/admin/additional-mail.php b/settings/templates/admin/additional-mail.php index 23723a423c0bd..7f8706274f987 100644 --- a/settings/templates/admin/additional-mail.php +++ b/settings/templates/admin/additional-mail.php @@ -44,7 +44,7 @@ if ($_['sendmail_is_available']) { $mail_smtpmode[] = ['sendmail', 'Sendmail']; } -if ($_['mail_smtpmode'] == 'qmail') { +if ($_['mail_smtpmode'] === 'qmail') { $mail_smtpmode[] = ['qmail', 'qmail']; } @@ -60,81 +60,81 @@

t('This is used for sending out notifications.')); ?>

- - - +

- - ' />@ - ' /> + + @ +

- -
-

- t( 'Test email settings' )); ?> - + t('Test email settings')); ?> + diff --git a/tests/Settings/Controller/MailSettingsControllerTest.php b/tests/Settings/Controller/MailSettingsControllerTest.php index 2012de886c8cd..c8f9e476801df 100644 --- a/tests/Settings/Controller/MailSettingsControllerTest.php +++ b/tests/Settings/Controller/MailSettingsControllerTest.php @@ -1,7 +1,10 @@ + * + * @author Lukas Reschke + * @author Joas Schilling * * This file is licensed under the Affero General Public License version 3 or * later. @@ -12,11 +15,13 @@ use OC\Mail\Message; use OC\Settings\Controller\MailSettingsController; +use OCP\AppFramework\Http; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; use OCP\IUserSession; use OCP\Mail\IMailer; +use OC\User\User; /** * @package Tests\Settings\Controller @@ -42,45 +47,39 @@ protected function setUp() { $this->config = $this->createMock(IConfig::class); $this->userSession = $this->createMock(IUserSession::class); $this->mailer = $this->createMock(IMailer::class); -// $this->mailer = $this->getMockBuilder(IMailer::class) -// ->setMethods(['send']) -// ->getMock(); + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject $request */ + $request = $this->createMock(IRequest::class); $this->mailController = new MailSettingsController( 'settings', - $this->createMock(IRequest::class), + $request, $this->l, $this->config, $this->userSession, $this->mailer, - 'no-reply@owncloud.com' + 'no-reply@nextcloud.com' ); } public function testSetMailSettings() { - $this->l - ->expects($this->exactly(2)) - ->method('t') - ->will($this->returnValue('Saved')); - $this->config->expects($this->exactly(2)) ->method('setSystemValues') ->withConsecutive( [[ - 'mail_domain' => 'owncloud.com', - 'mail_from_address' => 'demo@owncloud.com', + 'mail_domain' => 'nextcloud.com', + 'mail_from_address' => 'demo@nextcloud.com', 'mail_smtpmode' => 'smtp', 'mail_smtpsecure' => 'ssl', - 'mail_smtphost' => 'mx.owncloud.org', + 'mail_smtphost' => 'mx.nextcloud.org', 'mail_smtpauthtype' => 'NTLM', 'mail_smtpauth' => 1, 'mail_smtpport' => '25', ]], [[ - 'mail_domain' => 'owncloud.com', - 'mail_from_address' => 'demo@owncloud.com', + 'mail_domain' => 'nextcloud.com', + 'mail_from_address' => 'demo@nextcloud.com', 'mail_smtpmode' => 'smtp', 'mail_smtpsecure' => 'ssl', - 'mail_smtphost' => 'mx.owncloud.org', + 'mail_smtphost' => 'mx.nextcloud.org', 'mail_smtpauthtype' => 'NTLM', 'mail_smtpauth' => null, 'mail_smtpport' => '25', @@ -91,40 +90,33 @@ public function testSetMailSettings() { // With authentication $response = $this->mailController->setMailSettings( - 'owncloud.com', - 'demo@owncloud.com', + 'nextcloud.com', + 'demo@nextcloud.com', 'smtp', 'ssl', - 'mx.owncloud.org', + 'mx.nextcloud.org', 'NTLM', 1, '25' ); - $expectedResponse = array('data' => array('message' =>'Saved'), 'status' => 'success'); - $this->assertSame($expectedResponse, $response); + $this->assertSame(Http::STATUS_OK, $response->getStatus()); // Without authentication (testing the deletion of the stored password) $response = $this->mailController->setMailSettings( - 'owncloud.com', - 'demo@owncloud.com', + 'nextcloud.com', + 'demo@nextcloud.com', 'smtp', 'ssl', - 'mx.owncloud.org', + 'mx.nextcloud.org', 'NTLM', 0, '25' ); - $expectedResponse = array('data' => array('message' =>'Saved'), 'status' => 'success'); - $this->assertSame($expectedResponse, $response); + $this->assertSame(Http::STATUS_OK, $response->getStatus()); } public function testStoreCredentials() { - $this->l - ->expects($this->once()) - ->method('t') - ->will($this->returnValue('Saved')); - $this->config ->expects($this->once()) ->method('setSystemValues') @@ -134,15 +126,11 @@ public function testStoreCredentials() { ]); $response = $this->mailController->storeCredentials('UsernameToStore', 'PasswordToStore'); - $expectedResponse = array('data' => array('message' =>'Saved'), 'status' => 'success'); - - $this->assertSame($expectedResponse, $response); + $this->assertSame(Http::STATUS_OK, $response->getStatus()); } public function testSendTestMail() { - $user = $this->getMockBuilder('\OC\User\User') - ->disableOriginalConstructor() - ->getMock(); + $user = $this->createMock(User::class); $user->expects($this->any()) ->method('getUID') ->will($this->returnValue('Werner')); @@ -150,22 +138,11 @@ public function testSendTestMail() { ->method('getDisplayName') ->will($this->returnValue('Werner Brösel')); - $this->l - ->expects($this->any()) + $this->l->expects($this->any()) ->method('t') - ->will( - $this->returnValueMap( - array( - array('You need to set your user email before being able to send test emails.', array(), - 'You need to set your user email before being able to send test emails.'), - array('A problem occurred while sending the e-mail. Please revisit your settings.', array(), - 'A problem occurred while sending the e-mail. Please revisit your settings.'), - array('Email sent', array(), 'Email sent'), - array('test email settings', array(), 'test email settings'), - array('If you received this email, the settings seem to be correct.', array(), - 'If you received this email, the settings seem to be correct.') - ) - )); + ->willReturnCallback(function($text, $parameters = []) { + return vsprintf($text, $parameters); + }); $this->userSession ->expects($this->any()) ->method('getUser') @@ -173,8 +150,8 @@ public function testSendTestMail() { // Ensure that it fails when no mail address has been specified $response = $this->mailController->sendTestMail(); - $expectedResponse = array('data' => array('message' =>'You need to set your user email before being able to send test emails.'), 'status' => 'error'); - $this->assertSame($expectedResponse, $response); + $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); + $this->assertSame('You need to set your user email before being able to send test emails.', $response->getData()); // If no exception is thrown it should work $this->config @@ -185,8 +162,7 @@ public function testSendTestMail() { ->method('createMessage') ->willReturn($this->createMock(Message::class)); $response = $this->mailController->sendTestMail(); - $expectedResponse = array('data' => array('message' =>'Email sent'), 'status' => 'success'); - $this->assertSame($expectedResponse, $response); + $this->assertSame(Http::STATUS_OK, $response->getStatus(), $response->getData()); } } diff --git a/tests/lib/Settings/Admin/AdditionalTest.php b/tests/lib/Settings/Admin/AdditionalTest.php index 3a99893cf7c56..420a7110c134a 100644 --- a/tests/lib/Settings/Admin/AdditionalTest.php +++ b/tests/lib/Settings/Admin/AdditionalTest.php @@ -109,7 +109,7 @@ public function testGetForm() { 'mail_smtpauthtype' => 'login', 'mail_smtpauth' => true, 'mail_smtpname' => 'smtp.sender.com', - 'mail_smtppassword' => 'mypassword', + 'mail_smtppassword' => '********', ], '' );