From 5913af8a72e384f8fee89501b3a297b70460c1e0 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 15 Dec 2014 12:43:42 +0100 Subject: [PATCH] Mail address of users is now changable in the user management * introduced new route settings/users/{id}/mailAddress * kept old responses * better error messages * dropped lostpassword.php from settings/ajax * cleaned up the UserList.add() and hand in user object instead of each attribute as another parameter * check for change permission of mail address * proper response messages --- settings/ajax/lostpassword.php | 15 -- settings/controller/userscontroller.php | 99 ++++++++++++- settings/css/settings.css | 1 + settings/js/personal.js | 17 ++- settings/js/users/users.js | 134 +++++++++++++----- settings/routes.php | 3 +- settings/templates/users/main.php | 8 +- settings/templates/users/part.userlist.php | 5 + .../controller/userscontrollertest.php | 50 +++++-- 9 files changed, 253 insertions(+), 79 deletions(-) delete mode 100644 settings/ajax/lostpassword.php diff --git a/settings/ajax/lostpassword.php b/settings/ajax/lostpassword.php deleted file mode 100644 index b0fb20c4a7..0000000000 --- a/settings/ajax/lostpassword.php +++ /dev/null @@ -1,15 +0,0 @@ -getL10N('settings'); - -// Get data -if( isset( $_POST['email'] ) && OC_Mail::validateAddress($_POST['email']) ) { - $email=trim($_POST['email']); - \OC::$server->getConfig()->setUserValue(OC_User::getUser(), 'settings', 'email', $email); - OC_JSON::success(array("data" => array( "message" => $l->t("Email saved") ))); -}else{ - OC_JSON::error(array("data" => array( "message" => $l->t("Invalid email") ))); -} diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php index 0349a4c3d1..844ed4759e 100644 --- a/settings/controller/userscontroller.php +++ b/settings/controller/userscontroller.php @@ -108,7 +108,8 @@ class UsersController extends Controller { 'quota' => $this->config->getUserValue($user->getUID(), 'files', 'quota', 'default'), 'storageLocation' => $user->getHome(), 'lastLogin' => $user->getLastLogin(), - 'backend' => $user->getBackendClassName() + 'backend' => $user->getBackendClassName(), + 'email' => $this->config->getUserValue($user->getUID(), 'settings', 'email', '') ); } @@ -277,16 +278,20 @@ class UsersController extends Controller { $this->log->error("Can't send new user mail to $email: " . $e->getMessage(), array('app' => 'settings')); } } + // fetch users groups + $userGroups = $this->groupManager->getUserGroupIds($user); + + return new DataResponse( + $this->formatUserForIndex($user, $userGroups), + Http::STATUS_CREATED + ); } return new DataResponse( array( - 'username' => $username, - 'groups' => $this->groupManager->getUserGroupIds($user), - 'storageLocation' => $user->getHome(), - 'backend' => $user->getBackendClassName() + 'message' => (string)$this->l10n->t('Unable to create user.') ), - Http::STATUS_CREATED + Http::STATUS_FORBIDDEN ); } @@ -351,4 +356,86 @@ class UsersController extends Controller { ); } + /** + * Set the mail address of a user + * + * @NoAdminRequired + * @NoSubadminRequired + * + * @param string $id + * @param string $mailAddress + * @return DataResponse + * + * TODO: Tidy up and write unit tests - code is mainly static method calls + */ + public function setMailAddress($id, $mailAddress) { + // FIXME: Remove this static function call at some point… + if($this->userSession->getUser()->getUID() !== $id + && !$this->isAdmin + && !\OC_SubAdmin::isUserAccessible($this->userSession->getUser()->getUID(), $id)) { + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Forbidden') + ) + ), + Http::STATUS_FORBIDDEN + ); + } + + if($mailAddress !== '' && !$this->mail->validateAddress($mailAddress)) { + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Invalid mail address') + ) + ), + Http::STATUS_UNPROCESSABLE_ENTITY + ); + } + + $user = $this->userManager->get($id); + if(!$user){ + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Invalid user') + ) + ), + Http::STATUS_UNPROCESSABLE_ENTITY + ); + } + + // this is the only permission a backend provides and is also used + // for the permission of setting a email address + if(!$user->canChangeDisplayName()){ + return new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => (string)$this->l10n->t('Unable to change mail address') + ) + ), + Http::STATUS_FORBIDDEN + ); + } + + $this->config->setUserValue($id, 'settings', 'email', $mailAddress); + + return new DataResponse( + array( + 'status' => 'success', + 'data' => array( + 'username' => $id, + 'mailAddress' => $mailAddress, + 'message' => (string)$this->l10n->t('Email saved') + ) + ), + Http::STATUS_OK + ); + } + } diff --git a/settings/css/settings.css b/settings/css/settings.css index c951f98f9c..9a4e54971c 100644 --- a/settings/css/settings.css +++ b/settings/css/settings.css @@ -94,6 +94,7 @@ td.password>img,td.displayName>img, td.remove>a, td.quota>img { visibility:hidde td.password, td.quota, td.displayName { width:12em; cursor:pointer; } td.password>span, td.quota>span, rd.displayName>span { margin-right: 1.2em; color: #C7C7C7; } span.usersLastLoginTooltip { white-space: nowrap; } +#userlist .mailAddress, #userlist .storageLocation, #userlist .userBackend, #userlist .lastLogin { diff --git a/settings/js/personal.js b/settings/js/personal.js index b2efa7c37f..1ce9da55d8 100644 --- a/settings/js/personal.js +++ b/settings/js/personal.js @@ -45,9 +45,20 @@ function changeEmailAddress () { } emailInfo.defaultValue = emailInfo.val(); OC.msg.startSaving('#lostpassword .msg'); - var post = $("#lostpassword").serialize(); - $.post('ajax/lostpassword.php', post, function (data) { - OC.msg.finishedSaving('#lostpassword .msg', data); + var post = $("#lostpassword").serializeArray(); + $.ajax({ + type: 'PUT', + url: OC.generateUrl('/settings/users/{id}/mailAddress', {id: OC.currentUser}), + data: { + mailAddress: post[0].value + } + }).done(function(result){ + // I know the following 4 lines look weird, but that is how it works + // in jQuery - for success the first parameter is the result + // for failure the first parameter is the result object + OC.msg.finishedSaving('#lostpassword .msg', result); + }).fail(function(result){ + OC.msg.finishedSaving('#lostpassword .msg', result.responseJSON); }); } diff --git a/settings/js/users/users.js b/settings/js/users/users.js index e0eb5ff160..3e05d12c9a 100644 --- a/settings/js/users/users.js +++ b/settings/js/users/users.js @@ -28,7 +28,25 @@ var UserList = { this.$el.find('.quota-user').singleSelect().on('change', this.onQuotaSelect); }, - add: function (username, displayname, groups, subadmin, quota, storageLocation, lastLogin, sort, backend) { + /** + * Add a user row from user object + * + * @param user object containing following keys: + * { + * 'name': 'username', + * 'displayname': 'Users display name', + * 'groups': ['group1', 'group2'], + * 'subadmin': ['group4', 'group5'], + * 'quota': '10 GB', + * 'storageLocation': '/srv/www/owncloud/data/username', + * 'lastLogin': '1418632333' + * 'backend': 'LDAP', + * 'email': 'username@example.org' + * } + * @param sort + * @returns table row created for this user + */ + add: function (user, sort) { var $tr = $userListBody.find('tr:first-child').clone(); // this removes just the `display:none` of the template row $tr.removeAttr('style'); @@ -40,17 +58,19 @@ var UserList = { * Avatar or placeholder */ if ($tr.find('div.avatardiv').length){ - $tr.find('.avatardiv').imageplaceholder(username, displayname); - $('div.avatardiv', $tr).avatar(username, 32); + $tr.find('.avatardiv').imageplaceholder(user.name, user.displayname); + $('div.avatardiv', $tr).avatar(user.name, 32); } /** * add username and displayname to row (in data and visible markup */ - $tr.data('uid', username); - $tr.data('displayname', displayname); - $tr.find('td.name').text(username); - $tr.find('td.displayName > span').text(displayname); + $tr.data('uid', user.name); + $tr.data('displayname', user.displayname); + $tr.data('mailAddress', user.email); + $tr.find('td.name').text(user.name); + $tr.find('td.displayName > span').text(user.displayname); + $tr.find('td.mailAddress > span').text(user.email); /** * groups and subadmins @@ -58,13 +78,13 @@ var UserList = { // make them look like the multiselect buttons // until they get time to really get initialized groupsSelect = $('') - .data('username', username) - .data('user-groups', groups); + .data('username', user.name) + .data('user-groups', user.groups); if ($tr.find('td.subadmins').length > 0) { subAdminSelect = $('').val(mailAddress); + $td.children('span').replaceWith($input); + $input + .focus() + .keypress(function (event) { + if (event.keyCode === 13) { + if ($(this).val().length > 0) { + $input.blur(); + $.ajax({ + type: 'PUT', + url: OC.generateUrl('/settings/users/{id}/mailAddress', {id: uid}), + data: { + mailAddress: $(this).val() + } + }).fail(function (result) { + OC.Notification.show(result.responseJSON.data.message); + // reset the values + $tr.data('mailAddress', mailAddress); + $tr.children('.mailAddress').children('span').text(mailAddress); + }); + } else { + $input.blur(); + } + } + }) + .blur(function () { + var mailAddress = $input.val(); + var $span = $('').text(mailAddress); + $tr.data('mailAddress', mailAddress); + $input.replaceWith($span); + }); + }); + // init the quota field select box after it is shown the first time $('#app-settings').one('show', function() { $(this).find('#default_quota').singleSelect().on('change', UserList.onQuotaSelect); @@ -739,20 +801,8 @@ $(document).ready(function () { GroupList.setUserCount($li, userCount + 1); } } - if (result.homeExists){ - OC.Notification.hide(); - OC.Notification.show(t('settings', 'Warning: Home directory for user "{user}" already exists', {user: result.username})); - if (UserList.notificationTimeout){ - window.clearTimeout(UserList.notificationTimeout); - } - UserList.notificationTimeout = window.setTimeout( - function(){ - OC.Notification.hide(); - UserList.notificationTimeout = null; - }, 10000); - } if(!UserList.has(username)) { - UserList.add(username, username, result.groups, null, 'default', result.storageLocation, 0, true, result.backend); + UserList.add(result, true); } $('#newusername').focus(); GroupList.incEveryoneCount(); @@ -777,7 +827,15 @@ $(document).ready(function () { $("#userlist .lastLogin").hide(); } }); - // Option to display/hide the "Last Login" column + // Option to display/hide the "Mail Address" column + $('#CheckboxEmailAddress').click(function() { + if ($('#CheckboxEmailAddress').is(':checked')) { + $("#userlist .mailAddress").show(); + } else { + $("#userlist .mailAddress").hide(); + } + }); + // Option to display/hide the "User Backend" column $('#CheckboxUserBackend').click(function() { if ($('#CheckboxUserBackend').is(':checked')) { $("#userlist .userBackend").show(); diff --git a/settings/routes.php b/settings/routes.php index 1b7a918fa7..4be7785670 100644 --- a/settings/routes.php +++ b/settings/routes.php @@ -23,6 +23,7 @@ $application->registerRoutes($this, array( array('name' => 'SecuritySettings#enforceSSL', 'url' => '/settings/admin/security/ssl', 'verb' => 'POST'), array('name' => 'SecuritySettings#enforceSSLForSubdomains', 'url' => '/settings/admin/security/ssl/subdomains', 'verb' => 'POST'), array('name' => 'SecuritySettings#trustedDomains', 'url' => '/settings/admin/security/trustedDomains', 'verb' => 'POST'), + array('name' => 'Users#setMailAddress', 'url' => '/settings/users/{id}/mailAddress', 'verb' => 'PUT'), ) )); @@ -62,8 +63,6 @@ $this->create('settings_ajax_changegorupname', '/settings/ajax/changegroupname.p $this->create('settings_personal_changepassword', '/settings/personal/changepassword') ->post() ->action('OC\Settings\ChangePassword\Controller', 'changePersonalPassword'); -$this->create('settings_ajax_lostpassword', '/settings/ajax/lostpassword.php') - ->actionInclude('settings/ajax/lostpassword.php'); $this->create('settings_ajax_setlanguage', '/settings/ajax/setlanguage.php') ->actionInclude('settings/ajax/setlanguage.php'); $this->create('settings_ajax_decryptall', '/settings/ajax/decryptall.php') diff --git a/settings/templates/users/main.php b/settings/templates/users/main.php index 2004c10b9a..73552f8ad2 100644 --- a/settings/templates/users/main.php +++ b/settings/templates/users/main.php @@ -65,7 +65,13 @@ translation('settings');

+

+

+ +

diff --git a/settings/templates/users/part.userlist.php b/settings/templates/users/part.userlist.php index 6a6b0b69fa..4346920e43 100644 --- a/settings/templates/users/part.userlist.php +++ b/settings/templates/users/part.userlist.php @@ -7,6 +7,7 @@ t('Username'))?> t( 'Full Name' )); ?> t( 'Password' )); ?> + t( 'Email' )); ?> t( 'Groups' )); ?> t('Group Admin for')); ?> @@ -33,6 +34,10 @@ src="" alt="t("set new password"))?>" title="t("set new password"))?>"/> + <?php p($l->t('change email address'))?> + diff --git a/tests/settings/controller/userscontrollertest.php b/tests/settings/controller/userscontrollertest.php index 207943c4c8..4162273702 100644 --- a/tests/settings/controller/userscontrollertest.php +++ b/tests/settings/controller/userscontrollertest.php @@ -67,7 +67,7 @@ class UsersControllerTest extends \Test\TestCase { $foo = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $foo - ->expects($this->exactly(3)) + ->expects($this->exactly(4)) ->method('getUID') ->will($this->returnValue('foo')); $foo @@ -87,7 +87,7 @@ class UsersControllerTest extends \Test\TestCase { $admin = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $admin - ->expects($this->exactly(3)) + ->expects($this->exactly(4)) ->method('getUID') ->will($this->returnValue('admin')); $admin @@ -109,7 +109,7 @@ class UsersControllerTest extends \Test\TestCase { $bar = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $bar - ->expects($this->exactly(3)) + ->expects($this->exactly(4)) ->method('getUID') ->will($this->returnValue('bar')); $bar @@ -151,9 +151,11 @@ class UsersControllerTest extends \Test\TestCase { ->with('bar') ->will($this->returnValue($bar)); $this->container['Config'] - ->expects($this->exactly(3)) + ->expects($this->exactly(6)) ->method('getUserValue') - ->will($this->onConsecutiveCalls(1024, 404, 2323)); + ->will($this->onConsecutiveCalls(1024, 'foo@bar.com', + 404, 'admin@bar.com', + 2323, 'bar@dummy.com')); $expectedResponse = new DataResponse( array( @@ -165,7 +167,8 @@ class UsersControllerTest extends \Test\TestCase { 'quota' => 1024, 'storageLocation' => '/home/foo', 'lastLogin' => 500, - 'backend' => 'OC_User_Database' + 'backend' => 'OC_User_Database', + 'email' => 'foo@bar.com' ), 1 => array( 'name' => 'admin', @@ -175,7 +178,8 @@ class UsersControllerTest extends \Test\TestCase { 'quota' => 404, 'storageLocation' => '/home/admin', 'lastLogin' => 12, - 'backend' => 'OC_User_Dummy' + 'backend' => 'OC_User_Dummy', + 'email' => 'admin@bar.com' ), 2 => array( 'name' => 'bar', @@ -185,7 +189,8 @@ class UsersControllerTest extends \Test\TestCase { 'quota' => 2323, 'storageLocation' => '/home/bar', 'lastLogin' => 3999, - 'backend' => 'OC_User_Dummy' + 'backend' => 'OC_User_Dummy', + 'email' => 'bar@dummy.com' ), ) ); @@ -197,7 +202,7 @@ class UsersControllerTest extends \Test\TestCase { $user = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $user - ->expects($this->exactly(3)) + ->expects($this->exactly(4)) ->method('getUID') ->will($this->returnValue('foo')); $user @@ -241,7 +246,8 @@ class UsersControllerTest extends \Test\TestCase { 'quota' => null, 'storageLocation' => '/home/foo', 'lastLogin' => 500, - 'backend' => 'OC_User_Database' + 'backend' => 'OC_User_Database', + 'email' => null ) ) ); @@ -275,6 +281,9 @@ class UsersControllerTest extends \Test\TestCase { $user ->method('getHome') ->will($this->returnValue('/home/user')); + $user + ->method('getUID') + ->will($this->returnValue('foo')); $user ->expects($this->once()) ->method('getBackendClassName') @@ -288,10 +297,15 @@ class UsersControllerTest extends \Test\TestCase { $expectedResponse = new DataResponse( array( - 'username' => 'foo', + 'name' => 'foo', 'groups' => null, 'storageLocation' => '/home/user', - 'backend' => 'bar' + 'backend' => 'bar', + 'lastLogin' => null, + 'displayname' => null, + 'quota' => null, + 'subadmin' => array(), + 'email' => null ), Http::STATUS_CREATED ); @@ -312,6 +326,9 @@ class UsersControllerTest extends \Test\TestCase { $user ->method('getHome') ->will($this->returnValue('/home/user')); + $user + ->method('getUID') + ->will($this->returnValue('foo')); $user ->expects($this->once()) ->method('getBackendClassName') @@ -350,10 +367,15 @@ class UsersControllerTest extends \Test\TestCase { $expectedResponse = new DataResponse( array( - 'username' => 'foo', + 'name' => 'foo', 'groups' => array('NewGroup', 'ExistingGroup'), 'storageLocation' => '/home/user', - 'backend' => 'bar' + 'backend' => 'bar', + 'lastLogin' => null, + 'displayname' => null, + 'quota' => null, + 'subadmin' => array(), + 'email' => null ), Http::STATUS_CREATED );