Commit 82c529c4 authored by Vincent Petry's avatar Vincent Petry Committed by GitHub

Merge pull request #27277 from imujjwal96/master

Email confirmation for changing email in Settings
parents 9fbd9620 6ff91682
......@@ -10,6 +10,7 @@
* @author Roeland Jago Douma <rullzer@owncloud.com>
* @author Thomas Müller <thomas.mueller@tmit.eu>
* @author Tom Needham <tom@owncloud.com>
* @author Ujjwal Bhardwaj <ujjwalb1996@gmail.com>
*
* @copyright Copyright (c) 2017, ownCloud GmbH
* @license AGPL-3.0
......@@ -32,6 +33,7 @@ namespace OC\Settings;
use OC\Files\View;
use OC\Server;
use OC\AppFramework\Utility\TimeFactory;
use OC\Settings\Controller\SettingsPageController;
use OC\Settings\Controller\AppSettingsController;
use OC\Settings\Controller\AuthSettingsController;
......@@ -161,11 +163,13 @@ class Application extends App {
$c->query('GroupManager'),
$c->query('UserSession'),
$c->query('Config'),
$c->query('SecureRandom'),
$c->query('IsAdmin'),
$c->query('L10N'),
$c->query('Logger'),
$c->query('Defaults'),
$c->query('Mailer'),
$c->query('TimeFactory'),
$c->query('DefaultMailAddress'),
$c->query('URLGenerator'),
$c->query('OCP\\App\\IAppManager'),
......@@ -283,6 +287,12 @@ class Application extends App {
$server = $c->query('ServerContainer');
return $server->getIntegrityCodeChecker();
});
$container->registerService('TimeFactory', function(IContainer $c) {
return new TimeFactory();
});
$container->registerService('SecureRandom', function(IContainer $c) {
return $c->query('ServerContainer')->getSecureRandom();
});
$container->registerService('SettingsManager', function(IContainer $c) {
return $c->query('ServerContainer')->getSettingsManager();
});
......
......@@ -8,6 +8,7 @@
* @author Robin Appelman <icewind@owncloud.com>
* @author Roeland Jago Douma <rullzer@owncloud.com>
* @author Thomas Müller <thomas.mueller@tmit.eu>
* @author Ujjwal Bhardwaj <ujjwalb1996@gmail.com>
* @author Vincent Petry <pvince81@owncloud.com>
*
* @copyright Copyright (c) 2017, ownCloud GmbH
......@@ -34,7 +35,9 @@ use OC\User\User;
use OCP\App\IAppManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IL10N;
......@@ -46,6 +49,7 @@ use OCP\IUserManager;
use OCP\IUserSession;
use OCP\Mail\IMailer;
use OCP\IAvatarManager;
use OCP\Security\ISecureRandom;
/**
* @package OC\Settings\Controller
......@@ -79,6 +83,10 @@ class UsersController extends Controller {
private $isRestoreEnabled = false;
/** @var IAvatarManager */
private $avatarManager;
/** @var ISecureRandom */
protected $secureRandom;
/** @var ITimeFactory */
protected $timeFactory;
/**
* @param string $appName
......@@ -87,14 +95,17 @@ class UsersController extends Controller {
* @param IGroupManager $groupManager
* @param IUserSession $userSession
* @param IConfig $config
* @param bool $isAdmin
* @param ISecureRandom $secureRandom
* @param $isAdmin
* @param IL10N $l10n
* @param ILogger $log
* @param \OC_Defaults $defaults
* @param IMailer $mailer
* @param string $fromMailAddress
* @param ITimeFactory $timeFactory
* @param $fromMailAddress
* @param IURLGenerator $urlGenerator
* @param IAppManager $appManager
* @param IAvatarManager $avatarManager
*/
public function __construct($appName,
IRequest $request,
......@@ -102,11 +113,13 @@ class UsersController extends Controller {
IGroupManager $groupManager,
IUserSession $userSession,
IConfig $config,
ISecureRandom $secureRandom,
$isAdmin,
IL10N $l10n,
ILogger $log,
\OC_Defaults $defaults,
IMailer $mailer,
ITimeFactory $timeFactory,
$fromMailAddress,
IURLGenerator $urlGenerator,
IAppManager $appManager,
......@@ -118,9 +131,11 @@ class UsersController extends Controller {
$this->config = $config;
$this->isAdmin = $isAdmin;
$this->l10n = $l10n;
$this->secureRandom = $secureRandom;
$this->log = $log;
$this->defaults = $defaults;
$this->mailer = $mailer;
$this->timeFactory = $timeFactory;
$this->fromMailAddress = $fromMailAddress;
$this->urlGenerator = $urlGenerator;
$this->avatarManager = $avatarManager;
......@@ -213,6 +228,35 @@ class UsersController extends Controller {
return $users;
}
/**
* @param string $token
* @param string $userId
* @throws \Exception
*/
private function checkEmailChangeToken($token, $userId) {
$user = $this->userManager->get($userId);
if ($user === null) {
throw new \Exception($this->l10n->t('Couldn\'t change the email address because the user does not exist'));
}
$splittedToken = explode(':', $this->config->getUserValue($userId, 'owncloud', 'changeMail', null));
if(count($splittedToken) !== 2) {
$this->config->deleteUserValue($userId, 'owncloud', 'changeMail');
throw new \Exception($this->l10n->t('Couldn\'t change the email address because the token is invalid'));
}
if ($splittedToken[0] < ($this->timeFactory->getTime() - 60*60*12)) {
$this->config->deleteUserValue($userId, 'owncloud', 'changeMail');
throw new \Exception($this->l10n->t('Couldn\'t change the email address because the token is invalid'));
}
if (!hash_equals($splittedToken[1], $token)) {
$this->config->deleteUserValue($userId, 'owncloud', 'changeMail');
throw new \Exception($this->l10n->t('Couldn\'t change the email address because the token is invalid'));
}
}
/**
* @NoAdminRequired
*
......@@ -555,20 +599,43 @@ class UsersController extends Controller {
);
}
// delete user value if email address is empty
$user->setEMailAddress($mailAddress);
if ($mailAddress === '') {
$this->setEmailAddress($userId, $mailAddress);
return new DataResponse(
[
'status' => 'success',
'data' => [
'message' => (string)$this->l10n->t('Email has been changed successfully.')
]
],
Http::STATUS_OK
);
}
return new DataResponse(
[
'status' => 'success',
'data' => [
'username' => $id,
'mailAddress' => $mailAddress,
'message' => (string)$this->l10n->t('Email saved')
try {
$this->sendEmail($userId, $mailAddress);
return new DataResponse(
[
'status' => 'success',
'data' => [
'username' => $id,
'mailAddress' => $mailAddress,
'message' => (string) $this->l10n->t('An email has been sent to this address for confirmation')
]
],
Http::STATUS_OK
);
} catch (\Exception $e){
return new DataResponse(
[
'status' => 'error',
'data' => [
'message' => (string)$e->getMessage()
]
]
],
Http::STATUS_OK
);
);
}
}
/**
......@@ -664,6 +731,114 @@ class UsersController extends Controller {
}
/**
* @param string $userId
* @param string $mailAddress
* @throws \Exception
* @return boolean
*/
public function sendEmail($userId, $mailAddress) {
$token = $this->config->getUserValue($userId, 'owncloud', 'changeMail');
if ($token !== '') {
$splittedToken = explode(':', $token);
if ((count($splittedToken)) === 2 && $splittedToken[0] > ($this->timeFactory->getTime() - 60 * 5)) {
$this->log->alert('The email is not sent because an email change confirmation mail was sent recently.');
return false;
}
}
$token = $this->secureRandom->generate(21,
ISecureRandom::CHAR_DIGITS .
ISecureRandom::CHAR_LOWER .
ISecureRandom::CHAR_UPPER);
$this->config->setUserValue($userId, 'owncloud', 'changeMail', $this->timeFactory->getTime() . ':' . $token);
$link = $this->urlGenerator->linkToRouteAbsolute('settings.Users.changeMail', ['userId' => $userId, 'token' => $token, 'mailAddress' => $mailAddress]);
$tmpl = new \OC_Template('settings', 'changemail/email');
$tmpl->assign('link', $link);
$msg = $tmpl->fetchPage();
try {
$message = $this->mailer->createMessage();
$message->setTo([$mailAddress => $userId]);
$message->setSubject($this->l10n->t('%s email address confirm', [$this->defaults->getName()]));
$message->setPlainBody($msg);
$message->setFrom([$this->fromMailAddress => $this->defaults->getName()]);
$this->mailer->send($message);
} catch (\Exception $e) {
throw new \Exception($this->l10n->t(
'Couldn\'t send email address change confirmation mail. Please contact your administrator.'
));
}
return true;
}
/**
* @param string $id
* @param string $mailAddress
*/
public function setEmailAddress($id, $mailAddress) {
$user = $this->userManager->get($id);
$user->setEMailAddress($mailAddress);
if ($this->config->getUserValue($id, 'owncloud', 'changeMail') !== '') {
$this->config->deleteUserValue($id, 'owncloud', 'changeMail');
}
}
/**
* @NoCSRFRequired
* @NoAdminRequired
* @NoSubadminRequired
*
* @param $token
* @param $userId
* @param $mailAddress
* @return RedirectResponse
* @throws \Exception
*/
public function changeMail($token, $userId, $mailAddress) {
$user = $this->userManager->get($userId);
$sessionUser = $this->userSession->getUser();
if ($user !== $sessionUser) {
$this->log->error("The logged in user is different than expected.", ['app' => 'settings']);
return new RedirectResponse($this->urlGenerator->linkToRoute('settings.SettingsPage.getPersonal', ['changestatus' => 'error']));
}
try {
$this->checkEmailChangeToken($token, $userId);
} catch (\Exception $e) {
$this->log->error($e->getMessage(), ['app' => 'settings']);
return new RedirectResponse($this->urlGenerator->linkToRoute('settings.SettingsPage.getPersonal', ['changestatus' => 'error']));
}
$oldEmailAddress = $user->getEMailAddress();
$this->setEmailAddress($userId, $mailAddress);
if ($oldEmailAddress !== null) {
$tmpl = new \OC_Template('settings', 'changemail/notify');
$tmpl->assign('mailAddress', $mailAddress);
$msg = $tmpl->fetchPage();
try {
$message = $this->mailer->createMessage();
$message->setTo([$oldEmailAddress => $userId]);
$message->setSubject($this->l10n->t('%s email address changed successfully', [$this->defaults->getName()]));
$message->setPlainBody($msg);
$message->setFrom([$this->fromMailAddress => $this->defaults->getName()]);
$this->mailer->send($message);
} catch (\Exception $e) {
throw new \Exception($this->l10n->t(
'Couldn\'t send email address change notification mail. Please contact your administrator.'
));
}
}
return new RedirectResponse($this->urlGenerator->linkToRoute('settings.SettingsPage.getPersonal', ['changestatus' => 'success']));
}
/*
* @NoAdminRequired
*
* @param string $id
......
......@@ -149,6 +149,16 @@ function avatarResponseHandler (data) {
$(document).ready(function () {
var query = OC.parseQueryString(location.search);
if (query && query.changestatus) {
if (query.changestatus === 'error') {
OC.Notification.showTemporary(t('settings', 'Failed to change the email address.'));
} else if (query.changestatus === 'success') {
OC.Notification.showTemporary(t('settings', 'Email changed successfully.'));
}
OC.Util.History.replaceState({});
}
if($('#pass2').length) {
$('#pass2').showPassword().keyup();
}
......@@ -194,7 +204,10 @@ $(document).ready(function () {
});
$('#displayName').keyUpDelayedOrEnter(changeDisplayName);
$('#email').keyUpDelayedOrEnter(changeEmailAddress, true);
$('#email').enter(changeEmailAddress, true);
$('#emailbutton').click(function () {
changeEmailAddress();
});
$("#languageinput").change(function () {
// Serialize the data
......
......@@ -34,6 +34,33 @@ jQuery.fn.keyUpDelayedOrEnter = function (callback, allowEmptyValue) {
});
};
/**
* The callback will be fired as soon as enter is pressed by the user
*
* @param callback
* @param allowEmptyValue if this is set to true the callback is also called when the value is empty
*/
jQuery.fn.enter = function (callback, allowEmptyValue) {
var cb = callback;
var that = this;
this.keypress(function (event) {
if (event.keyCode === 13 && (allowEmptyValue || that.val !== '')) {
event.preventDefault();
cb();
}
});
this.bind('paste', null, function (e) {
if(!e.keyCode){
if (allowEmptyValue || that.val() !== '') {
cb();
}
}
});
};
$(document).ready(function () {
// 'redirect' to anchor sections
// anchors are lost on redirects (e.g. while solving the 2fa challenge) otherwise
......
......@@ -823,7 +823,7 @@ $(document).ready(function () {
$input.attr('disabled', 'disabled');
$.ajax({
type: 'PUT',
url: OC.generateUrl('/settings/users/{id}/mailAddress', {id: uid}),
url: OC.generateUrl('/settings/admin/{id}/mailAddress', {id: uid}),
data: {
mailAddress: $(this).val()
}
......
......@@ -49,6 +49,7 @@ $application->registerRoutes($this, [
['name' => 'SecuritySettings#trustedDomains', 'url' => '/settings/admin/security/trustedDomains', 'verb' => 'POST'],
['name' => 'Users#setDisplayName', 'url' => '/settings/users/{username}/displayName', 'verb' => 'POST'],
['name' => 'Users#setMailAddress', 'url' => '/settings/users/{id}/mailAddress', 'verb' => 'PUT'],
['name' => 'Users#setEmailAddress', 'url' => '/settings/admin/{id}/mailAddress', 'verb' => 'PUT'],
['name' => 'Users#setEnabled', 'url' => '/settings/users/{id}/enabled', 'verb' => 'POST'],
['name' => 'Users#stats', 'url' => '/settings/users/stats', 'verb' => 'GET'],
['name' => 'LogSettings#setLogLevel', 'url' => '/settings/admin/log/level', 'verb' => 'POST'],
......@@ -62,6 +63,7 @@ $application->registerRoutes($this, [
['name' => 'Certificate#removeSystemRootCertificate', 'url' => '/settings/admin/certificate/{certificateIdentifier}', 'verb' => 'DELETE'],
['name' => 'SettingsPage#getPersonal', 'url' => '/settings/personal', 'verb' => 'GET'],
['name' => 'SettingsPage#getAdmin', 'url' => '/settings/admin', 'verb' => 'GET'],
['name' => 'Users#changeMail', 'url' => '/settings/mailaddress/change/{token}/{userId}/{mailAddress}', 'verb' => 'GET'],
]
]);
......
<?php
echo str_replace('{link}', $_['link'], $l->t('Use the following link to confirm your changes to the email address: {link}'));
<?php
echo str_replace('{mailAddress}', $_['mailAddress'], $l->t('Email address changed to {mailAddress} successfully.'));
......@@ -64,6 +64,7 @@ if($_['displayNameChangeSupported']) {
<input type="email" name="email" id="email" value="<?php p($_['email']); ?>"
placeholder="<?php p($l->t('Your email address'));?>"
autocomplete="on" autocapitalize="off" autocorrect="off" />
<input id="emailbutton" type="button" value="<?php if(isset($_['email'][0])) { echo $l->t('Change email'); } else { echo $l->t('Set email'); }?>" />
<span class="msg"></span><br />
<em><?php p($l->t('For password recovery and notifications'));?></em>
</form>
......
......@@ -13,6 +13,7 @@ namespace Tests\Settings\Controller;
use \OC\Settings\Application;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\RedirectResponse;
/**
* @group DB
......@@ -55,6 +56,14 @@ class UsersControllerTest extends \Test\TestCase {
->disableOriginalConstructor()->getMock();
$this->container['OCP\\App\\IAppManager'] = $this->getMockBuilder('OCP\\App\\IAppManager')
->disableOriginalConstructor()->getMock();
$this->container['SecureRandom'] = $this->getMockBuilder('\OCP\Security\ISecureRandom')
->disableOriginalConstructor()->getMock();
$this->container['TimeFactory'] = $this->getMockBuilder('\OCP\AppFramework\Utility\ITimeFactory')
->disableOriginalConstructor()->getMock();
$this->existingUser = $this->getMockBuilder('OCP\IUser')
->disableOriginalConstructor()->getMock();
$this->container['Mailer'] = $this->getMockBuilder('\OCP\Mail\IMailer')
->disableOriginalConstructor()->getMock();
/*
......@@ -1800,7 +1809,8 @@ class UsersControllerTest extends \Test\TestCase {
* @param string $mailAddress
* @param bool $isValid
* @param bool $expectsUpdate
* @param bool $expectsDelete
* @param bool $canChangeDisplayName
* @param bool $responseCode
*/
public function testSetEmailAddress($mailAddress, $isValid, $expectsUpdate, $canChangeDisplayName, $responseCode) {
$this->container['IsAdmin'] = true;
......@@ -1811,12 +1821,16 @@ class UsersControllerTest extends \Test\TestCase {
->expects($this->any())
->method('getUID')
->will($this->returnValue('foo'));
$user
->expects($this->any())
->method('getEMailAddress')
->will($this->returnValue('foo@local'));
$user
->expects($this->any())
->method('canChangeDisplayName')
->will($this->returnValue($canChangeDisplayName));
$user
->expects($expectsUpdate ? $this->once() : $this->never())
->expects($this->any())
->method('setEMailAddress')
->with(
$this->equalTo($mailAddress)
......@@ -1836,16 +1850,64 @@ class UsersControllerTest extends \Test\TestCase {
$user->expects($this->atLeastOnce())
->method('canChangeDisplayName')
->willReturn(true);
$this->container['UserManager']
->expects($this->atLeastOnce())
->method('get')
->with('foo')
->will($this->returnValue($user));
}
$response = $this->container['UsersController']->setMailAddress($user->getUID(), $mailAddress);
$this->container['Config']
->expects($this->any())
->method('getUserValue')
->with('foo', 'owncloud', 'changeMail')
->will($this->returnValue('12000:AVerySecretToken'));
$this->container['TimeFactory']
->expects($this->any())
->method('getTime')
->willReturnOnConsecutiveCalls(12301, 12348);
$this->container['UserManager']
->expects($this->atLeastOnce())
->method('get')
->with('foo')
->will($this->returnValue($user));
$this->container['SecureRandom']
->expects($this->any())
->method('generate')
->with('21')
->will($this->returnValue('ThisIsMaybeANotSoSecretToken!'));
$this->container['Config']
->expects($this->any())
->method('setUserValue')
->with('foo', 'owncloud', 'changeMail', '12348:ThisIsMaybeANotSoSecretToken!');
$this->container['URLGenerator']
->expects($this->any())
->method('linkToRouteAbsolute')
->will($this->returnValue('https://ownCloud.com/index.php/mailaddress/'));
$message = $this->getMockBuilder('\OC\Mail\Message')
->disableOriginalConstructor()->getMock();
$message
->expects($this->any())
->method('setTo')
->with(['foo@local' => 'foo']);
$message
->expects($this->any())
->method('setSubject')
->with(' email address confirm');
$message
->expects($this->any())
->method('setPlainBody')
->with('Use the following link to confirm your changes to the email address: https://ownCloud.com/index.php/mailaddress/');
$message
->expects($this->any())
->method('setFrom')
->with(['changemail-noreply@localhost' => null]);
$this->container['Mailer']
->expects($this->any())
->method('createMessage')
->will($this->returnValue($message));
$this->container['Mailer']
->expects($this->any())
->method('send')
->with($message);
$response = $this->container['UsersController']->setMailAddress($user->getUID(), $mailAddress);
$this->assertSame($responseCode, $response->getStatus());
}
......@@ -2031,8 +2093,8 @@ class UsersControllerTest extends \Test\TestCase {
'message' => 'Authentication error',
],
]
);
}
);
}
$response = $this->container['UsersController']->setDisplayName($editUser->getUID(), 'newDisplayName');
$this->assertEquals($expectedResponse, $response);
......@@ -2087,6 +2149,67 @@ class UsersControllerTest extends \Test\TestCase {
$response = $this->container['UsersController']->setDisplayName($user->getUID(), 'newDisplayName');
$this->assertEquals($expectedResponse, $response);
}
public function testDifferentLoggedUserAndRequestUser() {
$token = 'AVerySecretToken';
$userId = 'ExistingUser';
$mailAddress = 'sample@email.com';
$userObject = $this->getMockBuilder('OCP\IUser')
->disableOriginalConstructor()->getMock();
$diffUserObject = $this->getMockBuilder('OCP\IUser')
->disableOriginalConstructor()->getMock();
$this->container['UserManager']
->expects($this->once())
->method('get')
->with($userId)
->will($this->returnValue($userObject));
$this->container['UserSession']
->expects($this->once())
->method('getUser')
->will($this->returnValue($diffUserObject));
$this->container['Logger']
->expects($this->once())
->method('error')
->with('The logged in user is different than expected.');
$expectedResponse = new RedirectResponse(
$this->container['URLGenerator']->linkToRoute('settings.SettingsPage.getPersonal', ['changestatus' => 'error'])
);
$response = $this->container['UsersController']->changeMail($token, $userId, $mailAddress);
$this->assertEquals($expectedResponse, $response);
}