From de1f3f05fdb47b4cde9b8cde468d54cdc856ef08 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Tue, 26 Apr 2016 16:19:10 +0200 Subject: [PATCH] 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 --- settings/Controller/UsersController.php | 193 ++++++++---------- settings/routes.php | 3 +- .../Controller/UsersControllerTest.php | 1 + 3 files changed, 87 insertions(+), 110 deletions(-) diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php index 6791475ce52..6bbf8786520 100644 --- 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 d4286cd56ff..62cfc398fdc 100644 --- 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 03c3a2e2ab4..e2b13b24b8c 100644 --- 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') -- 2.39.5