allow to change display names in the user settings again

keep display name and email address in sync with the accounts table

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php
index 6791475ce527700df917213a93936cf50757ce5d..6bbf8786520789ed34de99307d033c36e151e2d5 100644 (file)
--- a/settings/Controller/UsersController.php
+++ b/settings/Controller/UsersController.php
@@ -32,6 +32,7 @@ namespace OC\Settings\Controller;
 
 use OC\Accounts\AccountManager;
 use OC\AppFramework\Http;
+use OC\ForbiddenException;
 use OC\User\User;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
@@ -48,6 +49,7 @@ use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\Mail\IMailer;
 use OCP\IAvatarManager;
+use Punic\Exception;
 
 /**
  * @package OC\Settings\Controller
@@ -61,7 +63,7 @@ class UsersController extends Controller {
        private $isAdmin;
        /** @var IUserManager */
        private $userManager;
-       /** @var IGroupManager */
+       /** @var \OC\Group\Manager */
        private $groupManager;
        /** @var IConfig */
        private $config;
@@ -506,7 +508,6 @@ class UsersController extends Controller {
         * @NoSubadminRequired
         * @PasswordConfirmationRequired
         *
-        * @param string $userId
         * @param string $avatarScope
         * @param string $displayname
         * @param string $displaynameScope
@@ -520,17 +521,29 @@ class UsersController extends Controller {
         * @param string $addressScope
         * @return DataResponse
         */
-       public function saveUserSettings($userId, $avatarScope,
-                                       $displayname, $displaynameScope,
-                                       $phone, $phoneScope,
-                                       $email, $emailScope,
-                                       $website, $websiteScope,
-                                       $address, $addressScope) {
-
-               if($userId === null) {
-                       $userId = $this->userSession->getUser()->getUID();
+       public function setUserSettings($avatarScope,
+                                                                        $displayname, $displaynameScope,
+                                                                        $phone, $phoneScope,
+                                                                        $email, $emailScope,
+                                                                        $website, $websiteScope,
+                                                                        $address, $addressScope
+       ) {
+
+
+               if(!$this->mailer->validateMailAddress($email)) {
+                       return new DataResponse(
+                               array(
+                                       'status' => 'error',
+                                       'data' => array(
+                                               'message' => (string)$this->l10n->t('Invalid mail address')
+                                       )
+                               ),
+                               Http::STATUS_UNPROCESSABLE_ENTITY
+                       );
                }
 
+               $userId = $this->userSession->getUser()->getUID();
+
                $data = [
                        'avatar' =>  ['scope' => $avatarScope],
                        'displayName' => ['value' => $displayname, 'scope' => $displaynameScope],
@@ -542,106 +555,66 @@ class UsersController extends Controller {
 
                $this->accountManager->updateUser($userId, $data);
 
-               return new DataResponse(
-                       array(
-                               'status' => 'success',
-                               'data' => array(
-                                       'userId' => $userId,
-                                       'avatarScope' => $avatarScope,
-                                       'displayname' => $displayname,
-                                       'displaynameScope' => 'public', // force value for test purposes
-                                       'email' => $email,
-                                       'emailScope' => $emailScope,
-                                       'website' => $website,
-                                       'websiteScope' => $websiteScope,
-                                       'address' => $address,
-                                       'addressScope' => $addressScope,
-                                       'message' => (string)$this->l10n->t('Settings saved')
-                               )
-                       ),
-                       Http::STATUS_OK
-               );
-       }
-
-       /**
-        * Set the mail address of a user
-        *
-        * @todo Merge into saveUserSettings
-        *
-        * @param string $id
-        * @param string $mailAddress
-        * @return DataResponse
-        */
-       private function setMailAddress($id, $mailAddress) {
-               $userId = $this->userSession->getUser()->getUID();
-               $user = $this->userManager->get($id);
-
-               if($userId !== $id
-                       && !$this->isAdmin
-                       && !$this->groupManager->getSubAdmin()->isUserAccessible($this->userSession->getUser(), $user)) {
+               try {
+                       $this->saveUserSettings($userId, $data);
                        return new DataResponse(
                                array(
-                                       'status' => 'error',
+                                       'status' => 'success',
                                        'data' => array(
-                                               'message' => (string)$this->l10n->t('Forbidden')
+                                               'userId' => $userId,
+                                               'avatarScope' => $avatarScope,
+                                               'displayname' => $displayname,
+                                               'displaynameScope' => $displaynameScope,
+                                               'email' => $email,
+                                               'emailScope' => $emailScope,
+                                               'website' => $website,
+                                               'websiteScope' => $websiteScope,
+                                               'address' => $address,
+                                               'addressScope' => $addressScope,
+                                               'message' => (string)$this->l10n->t('Settings saved')
                                        )
                                ),
-                               Http::STATUS_FORBIDDEN
+                               Http::STATUS_OK
                        );
+               } catch (ForbiddenException $e) {
+                       return new DataResponse([
+                               'status' => 'error',
+                               'data' => [
+                                       'message' => $e->getMessage()
+                               ],
+                       ]);
                }
 
-               if($mailAddress !== '' && !$this->mailer->validateMailAddress($mailAddress)) {
-                       return new DataResponse(
-                               array(
-                                       'status' => 'error',
-                                       'data' => array(
-                                               'message' => (string)$this->l10n->t('Invalid mail address')
-                                       )
-                               ),
-                               Http::STATUS_UNPROCESSABLE_ENTITY
-                       );
-               }
+       }
 
-               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
-                       );
+       /**
+        * update account manager with new user data
+        *
+        * @param string $userId
+        * @param array $data
+        * @throws ForbiddenException
+        */
+       private function saveUserSettings($userId, $data) {
+               $user = $this->userManager->get($userId);
+
+               // keep the user back-end up-to-date with the latest display name and email
+               // address
+               if (isset($data['displayName']['value'])  && $user->getDisplayName() !== $data['displayName']['value']) {
+                       $result = $user->setDisplayName($data['displayName']['value']);
+                       if ($result === false) {
+                               throw new ForbiddenException($this->l10n->t('Unable to change full name'));
+                       }
                }
 
-               // delete user value if email address is empty
-               $user->setEMailAddress($mailAddress);
+               if (isset($data['email'][0]['value']) && $user->getEMailAddress() !== $data['email'][0]['value']) {
+                       $result = $user->setEMailAddress($data['email'][0]['value']);
+                       if ($result === false) {
+                               throw new ForbiddenException($this->l10n->t('Unable to change mail address'));
+                       }
+               }
 
-               return new DataResponse(
-                       array(
-                               'status' => 'success',
-                               'data' => array(
-                                       'username' => $id,
-                                       'mailAddress' => $mailAddress,
-                                       'message' => (string)$this->l10n->t('Email saved')
-                               )
-                       ),
-                       Http::STATUS_OK
-               );
+               $this->accountManager->updateUser($userId, $data);
        }
 
        /**
@@ -694,13 +667,8 @@ class UsersController extends Controller {
         * @param string $displayName
         * @return DataResponse
         */
-       private function setDisplayName($username, $displayName) {
+       public function setDisplayName($username, $displayName) {
                $currentUser = $this->userSession->getUser();
-
-               if ($username === null) {
-                       $username = $currentUser->getUID();
-               }
-
                $user = $this->userManager->get($username);
 
                if ($user === null ||
@@ -708,8 +676,10 @@ class UsersController extends Controller {
                        (
                                !$this->groupManager->isAdmin($currentUser->getUID()) &&
                                !$this->groupManager->getSubAdmin()->isUserAccessible($currentUser, $user) &&
-                               $currentUser !== $user)
-                       ) {
+                               $currentUser->getUID() !== $username
+
+                       )
+               ) {
                        return new DataResponse([
                                'status' => 'error',
                                'data' => [
@@ -718,7 +688,12 @@ class UsersController extends Controller {
                        ]);
                }
 
-               if ($user->setDisplayName($displayName)) {
+               $userData = $this->accountManager->getUser($user->getUID());
+               $userData['displayName']['value'] = $displayName;
+
+
+               try {
+                       $this->saveUserSettings($username, $userData);
                        return new DataResponse([
                                'status' => 'success',
                                'data' => [
@@ -727,11 +702,11 @@ class UsersController extends Controller {
                                        'displayName' => $displayName,
                                ],
                        ]);
-               } else {
+               } catch (ForbiddenException $e) {
                        return new DataResponse([
                                'status' => 'error',
                                'data' => [
-                                       'message' => $this->l10n->t('Unable to change full name'),
+                                       'message' => $e->getMessage(),
                                        'displayName' => $user->getDisplayName(),
                                ],
                        ]);

diff --git a/settings/routes.php b/settings/routes.php
index d4286cd56ff52b5852ca2f0a5efa9e7a1276f2f8..62cfc398fdc99adb910831b504321483f861fb0c 100644 (file)
--- a/settings/routes.php
+++ b/settings/routes.php
@@ -50,7 +50,8 @@ $application->registerRoutes($this, [
                ['name' => 'AppSettings#viewApps', 'url' => '/settings/apps', 'verb' => 'GET'],
                ['name' => 'AppSettings#listApps', 'url' => '/settings/apps/list', 'verb' => 'GET'],
                ['name' => 'SecuritySettings#trustedDomains', 'url' => '/settings/admin/security/trustedDomains', 'verb' => 'POST'],
-               ['name' => 'Users#saveUserSettings', 'url' => '/settings/users/{username}/settings', 'verb' => 'PUT'],
+               ['name' => 'Users#setDisplayName', 'url' => '/settings/users/{username}/displayName', 'verb' => 'POST'],
+               ['name' => 'Users#setUserSettings', 'url' => '/settings/users/{username}/settings', 'verb' => 'PUT'],
                ['name' => 'Users#stats', 'url' => '/settings/users/stats', 'verb' => 'GET'],
                ['name' => 'LogSettings#setLogLevel', 'url' => '/settings/admin/log/level', 'verb' => 'POST'],
                ['name' => 'LogSettings#getEntries', 'url' => '/settings/admin/log/entries', 'verb' => 'GET'],

diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php
index 03c3a2e2ab4641377b914ab8595b65e4c46f7270..e2b13b24b8cad3100920983f7e7b9db20e2da3cc 100644 (file)
--- a/tests/Settings/Controller/UsersControllerTest.php
+++ b/tests/Settings/Controller/UsersControllerTest.php
@@ -2040,6 +2040,7 @@ class UsersControllerTest extends \Test\TestCase {
                        ->expects($this->once())
                        ->method('getUser')
                        ->willReturn($user);
+
                $this->userManager
                        ->expects($this->once())
                        ->method('get')