Merge pull request #12921 from owncloud/user-mail-user-management

Mail address of users is now changable in the user management

settings/ajax/lostpassword.php

-<?php
-
-OC_JSON::checkLoggedIn();
-OCP\JSON::callCheck();
-
-$l = \OC::$server->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") )));
-}

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
+		);
+	}
+
 }

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 {

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);
 	});
 }
 

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 = $('<select multiple="multiple" class="groupsselect multiselect button" data-placehoder="Groups" title="' + t('settings', 'no group') + '"></select>')
-			.data('username', username)
-			.data('user-groups', groups);
+			.data('username', user.name)
+			.data('user-groups', user.groups);
 		if ($tr.find('td.subadmins').length > 0) {
 			subAdminSelect = $('<select multiple="multiple" class="subadminsselect multiselect button" data-placehoder="subadmins" title="' + t('settings', 'no group') + '">')
-				.data('username', username)
-				.data('user-groups', groups)
-				.data('subadmin', subadmin);
+				.data('username', user.name)
+				.data('user-groups', user.groups)
+				.data('subadmin', user.subadmin);
 			$tr.find('td.subadmins').empty();
 		}
 		$.each(this.availableGroups, function (i, group) {
@@ -82,7 +102,7 @@ var UserList = {
 		/**
 		 * remove action
 		 */
-		if ($tr.find('td.remove img').length === 0 && OC.currentUser !== username) {
+		if ($tr.find('td.remove img').length === 0 && OC.currentUser !== user.name) {
 			var deleteImage = $('<img class="svg action">').attr({
 				src: OC.imagePath('core', 'actions/delete')
 			});
@@ -90,7 +110,7 @@ var UserList = {
 				.attr({ href: '#', 'original-title': t('settings', 'Delete')})
 				.append(deleteImage);
 			$tr.find('td.remove').append(deleteLink);
-		} else if (OC.currentUser === username) {
+		} else if (OC.currentUser === user.name) {
 			$tr.find('td.remove a').remove();
 		}
 
@@ -98,37 +118,37 @@ var UserList = {
 		 * quota
 		 */
 		var $quotaSelect = $tr.find('.quota-user');
-		if (quota === 'default') {
+		if (user.quota === 'default') {
 			$quotaSelect
 				.data('previous', 'default')
 				.find('option').attr('selected', null)
 				.first().attr('selected', 'selected');
 		} else {
-			if ($quotaSelect.find('option').filterAttr('value', quota).length > 0) {
-				$quotaSelect.find('option').filterAttr('value', quota).attr('selected', 'selected');
+			if ($quotaSelect.find('option').filterAttr('value', user.quota).length > 0) {
+				$quotaSelect.find('option').filterAttr('value', user.quota).attr('selected', 'selected');
 			} else {
-				$quotaSelect.append('<option value="' + escapeHTML(quota) + '" selected="selected">' + escapeHTML(quota) + '</option>');
+				$quotaSelect.append('<option value="' + escapeHTML(user.quota) + '" selected="selected">' + escapeHTML(user.quota) + '</option>');
 			}
 		}
 
 		/**
 		 * storage location
 		 */
-		$tr.find('td.storageLocation').text(storageLocation);
+		$tr.find('td.storageLocation').text(user.storageLocation);
 
 		/**
 		 * user backend
 		 */
-		$tr.find('td.userBackend').text(backend);
+		$tr.find('td.userBackend').text(user.backend);
 
 		/**
 		 * last login
 		 */
 		var lastLoginRel = t('settings', 'never');
 		var lastLoginAbs = lastLoginRel;
-		if(lastLogin !== 0) {
-			lastLoginRel = OC.Util.relativeModifiedDate(lastLogin);
-			lastLoginAbs = OC.Util.formatDate(lastLogin);
+		if(user.lastLogin !== 0) {
+			lastLoginRel = OC.Util.relativeModifiedDate(user.lastLogin);
+			lastLoginAbs = OC.Util.formatDate(user.lastLogin);
 		}
 		var $tdLastLogin = $tr.find('td.lastLogin');
 		$tdLastLogin.text(lastLoginRel);
@@ -329,6 +349,9 @@ var UserList = {
 	getDisplayName: function(element) {
 		return ($(element).closest('tr').data('displayname') || '').toString();
 	},
+	getMailAddress: function(element) {
+		return ($(element).closest('tr').data('mailAddress') || '').toString();
+	},
 	initDeleteHandling: function() {
 		//set up handler
 		UserDeleteHandler = new DeleteHandler('/settings/users/users', 'username',
@@ -380,7 +403,7 @@ var UserList = {
 					if(UserList.has(user.name)) {
 						return true;
 					}
-					var $tr = UserList.add(user.name, user.displayname, user.groups, user.subadmin, user.quota, user.storageLocation, user.lastLogin, false, user.backend);
+					var $tr = UserList.add(user, user.lastLogin, false, user.backend);
 					$tr.addClass('appear transparent');
 					trs.push($tr);
 					loadedUsers++;
@@ -686,6 +709,45 @@ $(document).ready(function () {
 			});
 	});
 
+	$userListBody.on('click', '.mailAddress', function (event) {
+		event.stopPropagation();
+		var $td = $(this).closest('td');
+		var $tr = $td.closest('tr');
+		var uid = UserList.getUID($td);
+		var mailAddress = escapeHTML(UserList.getMailAddress($td));
+		var $input = $('<input type="text">').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 = $('<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();

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')

settings/templates/users/main.php

@@ -65,7 +65,13 @@ translation('settings');
 				<p>
 					<input type="checkbox" name="MailOnUserCreate" value="MailOnUserCreate" id="CheckboxMailOnUserCreate">
 					<label for="CheckboxMailOnUserCreate">
-						<?php p($l->t('Send mail to new user')) ?>
+						<?php p($l->t('Send email to new user')) ?>
+					</label>
+				</p>
+				<p>
+					<input type="checkbox" name="EmailAddress" value="EmailAddress" id="CheckboxEmailAddress">
+					<label for="CheckboxEmailAddress">
+						<?php p($l->t('Show email address')) ?>
 					</label>
 				</p>
 			</div>

settings/templates/users/part.userlist.php

@@ -7,6 +7,7 @@
 			<th id='headerName'><?php p($l->t('Username'))?></th>
 			<th id="headerDisplayName"><?php p($l->t( 'Full Name' )); ?></th>
 			<th id="headerPassword"><?php p($l->t( 'Password' )); ?></th>
+			<th class="mailAddress"><?php p($l->t( 'Email' )); ?></th>
 			<th id="headerGroups"><?php p($l->t( 'Groups' )); ?></th>
 			<?php if(is_array($_['subadmins']) || $_['subadmins']): ?>
 			<th id="headerSubAdmins"><?php p($l->t('Group Admin for')); ?></th>
@@ -33,6 +34,10 @@
 				src="<?php print_unescaped(image_path('core', 'actions/rename.svg'))?>"
 				alt="<?php p($l->t("set new password"))?>" title="<?php p($l->t("set new password"))?>"/>
 			</td>
+			<td class="mailAddress"><span></span> <img class="svg action"
+				src="<?php p(image_path('core', 'actions/rename.svg'))?>"
+				alt="<?php p($l->t('change email address'))?>" title="<?php p($l->t('change email address'))?>"/>
+			</td>
 			<td class="groups"></td>
 			<?php if(is_array($_['subadmins']) || $_['subadmins']): ?>
 				<td class="subadmins"></td>

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
 		);