From da9462b482c77b71d6623be43a6aa1eda3d7cc06 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 1 Dec 2020 14:33:22 +0100 Subject: Make code strict Signed-off-by: Joas Schilling --- apps/settings/lib/Controller/UsersController.php | 98 ++++++++++++------------ 1 file changed, 50 insertions(+), 48 deletions(-) (limited to 'apps/settings/lib/Controller/UsersController.php') diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index cad21c5f3b3..e540698c485 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -1,4 +1,6 @@ usersList(); } @@ -152,7 +154,7 @@ class UsersController extends Controller { * * @return TemplateResponse */ - public function usersList() { + public function usersList(): TemplateResponse { $user = $this->userSession->getUser(); $uid = $user->getUID(); @@ -309,7 +311,7 @@ class UsersController extends Controller { * * @return bool */ - protected function canAdminChangeUserPasswords() { + protected function canAdminChangeUserPasswords(): bool { $isEncryptionEnabled = $this->encryptionManager->isEnabled(); try { $noUserSpecificEncryptionKeys = !$this->encryptionManager->getEncryptionModule()->needDetailedAccessList(); @@ -344,19 +346,19 @@ class UsersController extends Controller { * @param string $twitterScope * @return DataResponse */ - public function setUserSettings($avatarScope, - $displayname, - $displaynameScope, - $phone, - $phoneScope, - $email, - $emailScope, - $website, - $websiteScope, - $address, - $addressScope, - $twitter, - $twitterScope + public function setUserSettings(string $avatarScope, + string $displayname, + string $displaynameScope, + string $phone, + string $phoneScope, + string $email, + string $emailScope, + string $website, + string $websiteScope, + string $address, + string $addressScope, + string $twitter, + string $twitterScope ) { $email = strtolower($email); if (!empty($email) && !$this->mailer->validateMailAddress($email)) { @@ -372,18 +374,18 @@ class UsersController extends Controller { } $user = $this->userSession->getUser(); $data = $this->accountManager->getUser($user); - $data[AccountManager::PROPERTY_AVATAR] = ['scope' => $avatarScope]; + $data[IAccountManager::PROPERTY_AVATAR] = ['scope' => $avatarScope]; if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { - $data[AccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope]; - $data[AccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope]; + $data[IAccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope]; + $data[IAccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope]; } if ($this->appManager->isEnabledForUser('federatedfilesharing')) { $shareProvider = \OC::$server->query(FederatedShareProvider::class); if ($shareProvider->isLookupServerUploadEnabled()) { - $data[AccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope]; - $data[AccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope]; - $data[AccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope]; - $data[AccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope]; + $data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope]; + $data[IAccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope]; + $data[IAccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope]; + $data[IAccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope]; } } try { @@ -393,15 +395,15 @@ class UsersController extends Controller { 'status' => 'success', 'data' => [ 'userId' => $user->getUID(), - 'avatarScope' => $data[AccountManager::PROPERTY_AVATAR]['scope'], - 'displayname' => $data[AccountManager::PROPERTY_DISPLAYNAME]['value'], - 'displaynameScope' => $data[AccountManager::PROPERTY_DISPLAYNAME]['scope'], - 'email' => $data[AccountManager::PROPERTY_EMAIL]['value'], - 'emailScope' => $data[AccountManager::PROPERTY_EMAIL]['scope'], - 'website' => $data[AccountManager::PROPERTY_WEBSITE]['value'], - 'websiteScope' => $data[AccountManager::PROPERTY_WEBSITE]['scope'], - 'address' => $data[AccountManager::PROPERTY_ADDRESS]['value'], - 'addressScope' => $data[AccountManager::PROPERTY_ADDRESS]['scope'], + 'avatarScope' => $data[IAccountManager::PROPERTY_AVATAR]['scope'], + 'displayname' => $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'], + 'displaynameScope' => $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'], + 'email' => $data[IAccountManager::PROPERTY_EMAIL]['value'], + 'emailScope' => $data[IAccountManager::PROPERTY_EMAIL]['scope'], + 'website' => $data[IAccountManager::PROPERTY_WEBSITE]['value'], + 'websiteScope' => $data[IAccountManager::PROPERTY_WEBSITE]['scope'], + 'address' => $data[IAccountManager::PROPERTY_ADDRESS]['value'], + 'addressScope' => $data[IAccountManager::PROPERTY_ADDRESS]['scope'], 'message' => $this->l10n->t('Settings saved') ] ], @@ -423,30 +425,30 @@ class UsersController extends Controller { * @param array $data * @throws ForbiddenException */ - protected function saveUserSettings(IUser $user, array $data) { + protected function saveUserSettings(IUser $user, array $data): void { // keep the user back-end up-to-date with the latest display name and email // address $oldDisplayName = $user->getDisplayName(); $oldDisplayName = is_null($oldDisplayName) ? '' : $oldDisplayName; - if (isset($data[AccountManager::PROPERTY_DISPLAYNAME]['value']) - && $oldDisplayName !== $data[AccountManager::PROPERTY_DISPLAYNAME]['value'] + if (isset($data[IAccountManager::PROPERTY_DISPLAYNAME]['value']) + && $oldDisplayName !== $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'] ) { - $result = $user->setDisplayName($data[AccountManager::PROPERTY_DISPLAYNAME]['value']); + $result = $user->setDisplayName($data[IAccountManager::PROPERTY_DISPLAYNAME]['value']); if ($result === false) { throw new ForbiddenException($this->l10n->t('Unable to change full name')); } } $oldEmailAddress = $user->getEMailAddress(); $oldEmailAddress = is_null($oldEmailAddress) ? '' : strtolower($oldEmailAddress); - if (isset($data[AccountManager::PROPERTY_EMAIL]['value']) - && $oldEmailAddress !== $data[AccountManager::PROPERTY_EMAIL]['value'] + if (isset($data[IAccountManager::PROPERTY_EMAIL]['value']) + && $oldEmailAddress !== $data[IAccountManager::PROPERTY_EMAIL]['value'] ) { // this is the only permission a backend provides and is also used // for the permission of setting a email address if (!$user->canChangeDisplayName()) { throw new ForbiddenException($this->l10n->t('Unable to change email address')); } - $user->setEMailAddress($data[AccountManager::PROPERTY_EMAIL]['value']); + $user->setEMailAddress($data[IAccountManager::PROPERTY_EMAIL]['value']); } $this->accountManager->updateUser($user, $data); } @@ -479,19 +481,19 @@ class UsersController extends Controller { switch ($account) { case 'verify-twitter': - $accountData[AccountManager::PROPERTY_TWITTER]['verified'] = AccountManager::VERIFICATION_IN_PROGRESS; + $accountData[IAccountManager::PROPERTY_TWITTER]['verified'] = AccountManager::VERIFICATION_IN_PROGRESS; $msg = $this->l10n->t('In order to verify your Twitter account, post the following tweet on Twitter (please make sure to post it without any line breaks):'); $code = $codeMd5; - $type = AccountManager::PROPERTY_TWITTER; - $data = $accountData[AccountManager::PROPERTY_TWITTER]['value']; - $accountData[AccountManager::PROPERTY_TWITTER]['signature'] = $signature; + $type = IAccountManager::PROPERTY_TWITTER; + $data = $accountData[IAccountManager::PROPERTY_TWITTER]['value']; + $accountData[IAccountManager::PROPERTY_TWITTER]['signature'] = $signature; break; case 'verify-website': - $accountData[AccountManager::PROPERTY_WEBSITE]['verified'] = AccountManager::VERIFICATION_IN_PROGRESS; + $accountData[IAccountManager::PROPERTY_WEBSITE]['verified'] = AccountManager::VERIFICATION_IN_PROGRESS; $msg = $this->l10n->t('In order to verify your Website, store the following content in your web-root at \'.well-known/CloudIdVerificationCode.txt\' (please make sure that the complete text is in one line):'); - $type = AccountManager::PROPERTY_WEBSITE; - $data = $accountData[AccountManager::PROPERTY_WEBSITE]['value']; - $accountData[AccountManager::PROPERTY_WEBSITE]['signature'] = $signature; + $type = IAccountManager::PROPERTY_WEBSITE; + $data = $accountData[IAccountManager::PROPERTY_WEBSITE]['value']; + $accountData[IAccountManager::PROPERTY_WEBSITE]['signature'] = $signature; break; default: return new DataResponse([], Http::STATUS_BAD_REQUEST); -- cgit v1.2.3 From c2913f18d21ad411e53642f8cbac2ca51029a9f9 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 1 Dec 2020 14:40:06 +0100 Subject: Also return the phone number and twitter handle on the API Signed-off-by: Joas Schilling --- apps/settings/lib/Controller/UsersController.php | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'apps/settings/lib/Controller/UsersController.php') diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index e540698c485..c23e004d2c6 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -398,12 +398,16 @@ class UsersController extends Controller { 'avatarScope' => $data[IAccountManager::PROPERTY_AVATAR]['scope'], 'displayname' => $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'], 'displaynameScope' => $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'], + 'phone' => $data[IAccountManager::PROPERTY_PHONE]['value'], + 'phoneScope' => $data[IAccountManager::PROPERTY_PHONE]['scope'], 'email' => $data[IAccountManager::PROPERTY_EMAIL]['value'], 'emailScope' => $data[IAccountManager::PROPERTY_EMAIL]['scope'], 'website' => $data[IAccountManager::PROPERTY_WEBSITE]['value'], 'websiteScope' => $data[IAccountManager::PROPERTY_WEBSITE]['scope'], 'address' => $data[IAccountManager::PROPERTY_ADDRESS]['value'], 'addressScope' => $data[IAccountManager::PROPERTY_ADDRESS]['scope'], + 'twitter' => $data[IAccountManager::PROPERTY_TWITTER]['value'], + 'twitterScope' => $data[IAccountManager::PROPERTY_TWITTER]['scope'], 'message' => $this->l10n->t('Settings saved') ] ], -- cgit v1.2.3 From efe79f293764225b6036565cb0b5f5a29a16d0ca Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 1 Dec 2020 15:38:43 +0100 Subject: Validate and standardize the phone number on saving Signed-off-by: Joas Schilling --- apps/settings/lib/Controller/UsersController.php | 36 ++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) (limited to 'apps/settings/lib/Controller/UsersController.php') diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index c23e004d2c6..4267b8be4c4 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -35,6 +35,10 @@ declare(strict_types=1); namespace OCA\Settings\Controller; +use libphonenumber\NumberParseException; +use libphonenumber\PhoneNumber; +use libphonenumber\PhoneNumberFormat; +use libphonenumber\PhoneNumberUtil; use OC\Accounts\AccountManager; use OC\AppFramework\Http; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; @@ -389,7 +393,7 @@ class UsersController extends Controller { } } try { - $this->saveUserSettings($user, $data); + $data = $this->saveUserSettings($user, $data); return new DataResponse( [ 'status' => 'success', @@ -420,6 +424,13 @@ class UsersController extends Controller { 'message' => $e->getMessage() ], ]); + } catch (\InvalidArgumentException $e) { + return new DataResponse([ + 'status' => 'error', + 'data' => [ + 'message' => $e->getMessage() + ], + ]); } } /** @@ -427,9 +438,11 @@ class UsersController extends Controller { * * @param IUser $user * @param array $data + * @return array * @throws ForbiddenException + * @throws \InvalidArgumentException */ - protected function saveUserSettings(IUser $user, array $data): void { + protected function saveUserSettings(IUser $user, array $data): array { // keep the user back-end up-to-date with the latest display name and email // address $oldDisplayName = $user->getDisplayName(); @@ -442,6 +455,7 @@ class UsersController extends Controller { throw new ForbiddenException($this->l10n->t('Unable to change full name')); } } + $oldEmailAddress = $user->getEMailAddress(); $oldEmailAddress = is_null($oldEmailAddress) ? '' : strtolower($oldEmailAddress); if (isset($data[IAccountManager::PROPERTY_EMAIL]['value']) @@ -454,7 +468,25 @@ class UsersController extends Controller { } $user->setEMailAddress($data[IAccountManager::PROPERTY_EMAIL]['value']); } + + if (isset($data[AccountManager::PROPERTY_PHONE])) { + $phoneUtil = PhoneNumberUtil::getInstance(); + try { + $phoneValue = $data[AccountManager::PROPERTY_PHONE]['value']; + $phoneNumber = $phoneUtil->parse($phoneValue, 'DE'); // FIXME need a reasonable default + if ($phoneNumber instanceof PhoneNumber && $phoneUtil->isValidNumber($phoneNumber)) { + $data[AccountManager::PROPERTY_PHONE]['value'] = $phoneUtil->format($phoneNumber, PhoneNumberFormat::E164); + } else { + throw new \InvalidArgumentException($this->l10n->t('Unable to set invalid phone number')); + } + } catch (NumberParseException $e) { + throw new \InvalidArgumentException($this->l10n->t('Unable to set invalid phone number')); + } + } + $this->accountManager->updateUser($user, $data); + + return $data; } /** -- cgit v1.2.3 From 9e04e6f99ad7a8d78b0bf09a414ed0f1aac3e5db Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 2 Dec 2020 16:03:08 +0100 Subject: Also translate the phone number when coming in via the accounts manager API directly Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 6 ++++- apps/settings/lib/Controller/UsersController.php | 29 ++++++---------------- .../tests/Controller/UsersControllerTest.php | 8 +++--- lib/private/Accounts/AccountManager.php | 28 +++++++++++++++++++-- 4 files changed, 43 insertions(+), 28 deletions(-) (limited to 'apps/settings/lib/Controller/UsersController.php') diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 08c541ffa6c..edb1fc5121a 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -654,7 +654,11 @@ class UsersController extends AUserData { $userAccount = $this->accountManager->getUser($targetUser); if ($userAccount[$key]['value'] !== $value) { $userAccount[$key]['value'] = $value; - $this->accountManager->updateUser($targetUser, $userAccount); + try { + $this->accountManager->updateUser($targetUser, $userAccount); + } catch (\InvalidArgumentException $e) { + throw new OCSException('Invalid ' . $e->getMessage(), 102); + } } break; default: diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 4267b8be4c4..5b3c1574f96 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -35,10 +35,6 @@ declare(strict_types=1); namespace OCA\Settings\Controller; -use libphonenumber\NumberParseException; -use libphonenumber\PhoneNumber; -use libphonenumber\PhoneNumberFormat; -use libphonenumber\PhoneNumberUtil; use OC\Accounts\AccountManager; use OC\AppFramework\Http; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; @@ -469,24 +465,14 @@ class UsersController extends Controller { $user->setEMailAddress($data[IAccountManager::PROPERTY_EMAIL]['value']); } - if (isset($data[AccountManager::PROPERTY_PHONE])) { - $phoneUtil = PhoneNumberUtil::getInstance(); - try { - $phoneValue = $data[AccountManager::PROPERTY_PHONE]['value']; - $phoneNumber = $phoneUtil->parse($phoneValue, 'DE'); // FIXME need a reasonable default - if ($phoneNumber instanceof PhoneNumber && $phoneUtil->isValidNumber($phoneNumber)) { - $data[AccountManager::PROPERTY_PHONE]['value'] = $phoneUtil->format($phoneNumber, PhoneNumberFormat::E164); - } else { - throw new \InvalidArgumentException($this->l10n->t('Unable to set invalid phone number')); - } - } catch (NumberParseException $e) { + try { + return $this->accountManager->updateUser($user, $data); + } catch (\InvalidArgumentException $e) { + if ($e->getMessage() === IAccountManager::PROPERTY_PHONE) { throw new \InvalidArgumentException($this->l10n->t('Unable to set invalid phone number')); } + throw new \InvalidArgumentException($this->l10n->t('Some account data was invalid')); } - - $this->accountManager->updateUser($user, $data); - - return $data; } /** @@ -521,14 +507,12 @@ class UsersController extends Controller { $msg = $this->l10n->t('In order to verify your Twitter account, post the following tweet on Twitter (please make sure to post it without any line breaks):'); $code = $codeMd5; $type = IAccountManager::PROPERTY_TWITTER; - $data = $accountData[IAccountManager::PROPERTY_TWITTER]['value']; $accountData[IAccountManager::PROPERTY_TWITTER]['signature'] = $signature; break; case 'verify-website': $accountData[IAccountManager::PROPERTY_WEBSITE]['verified'] = AccountManager::VERIFICATION_IN_PROGRESS; $msg = $this->l10n->t('In order to verify your Website, store the following content in your web-root at \'.well-known/CloudIdVerificationCode.txt\' (please make sure that the complete text is in one line):'); $type = IAccountManager::PROPERTY_WEBSITE; - $data = $accountData[IAccountManager::PROPERTY_WEBSITE]['value']; $accountData[IAccountManager::PROPERTY_WEBSITE]['signature'] = $signature; break; default: @@ -536,7 +520,8 @@ class UsersController extends Controller { } if ($onlyVerificationCode === false) { - $this->accountManager->updateUser($user, $accountData); + $accountData = $this->accountManager->updateUser($user, $accountData); + $data = $accountData[$type]['value']; $this->jobList->add(VerifyUserData::class, [ diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 75e71435b0f..23e3ef5ec01 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -239,12 +239,14 @@ class UsersControllerTest extends \Test\TestCase { ], ]); - $controller->expects($this->once())->method('saveUserSettings'); + $controller->expects($this->once()) + ->method('saveUserSettings') + ->willReturnArgument(1); } else { $controller->expects($this->never())->method('saveUserSettings'); } - $result = $controller->setUserSettings( + $result = $controller->setUserSettings(// AccountManager::VISIBILITY_CONTACTS_ONLY, 'displayName', AccountManager::VISIBILITY_CONTACTS_ONLY, @@ -471,7 +473,7 @@ class UsersControllerTest extends \Test\TestCase { $controller->expects($this->any())->method('getCurrentTime')->willReturn(1234567); if ($onlyVerificationCode === false) { - $this->accountManager->expects($this->once())->method('updateUser')->with($user, $expectedData); + $this->accountManager->expects($this->once())->method('updateUser')->with($user, $expectedData)->willReturnArgument(1); $this->jobList->expects($this->once())->method('add') ->with('OCA\Settings\BackgroundJobs\VerifyUserData', [ diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index 39921dae9db..4f4a146bdd9 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -30,6 +30,10 @@ namespace OC\Accounts; +use libphonenumber\NumberParseException; +use libphonenumber\PhoneNumber; +use libphonenumber\PhoneNumberFormat; +use libphonenumber\PhoneNumberUtil; use OCA\Settings\BackgroundJobs\VerifyUserData; use OCP\Accounts\IAccount; use OCP\Accounts\IAccountManager; @@ -85,11 +89,29 @@ class AccountManager implements IAccountManager { * update user record * * @param IUser $user - * @param $data + * @param array $data + * @return array The potentially modified data (e.g. phone numbers are converted to E.164 format) + * @throws \InvalidArgumentException Message is the property that was invalid */ - public function updateUser(IUser $user, $data) { + public function updateUser(IUser $user, array $data): array { $userData = $this->getUser($user); $updated = true; + + if (isset($data[self::PROPERTY_PHONE])) { + $phoneUtil = PhoneNumberUtil::getInstance(); + try { + $phoneValue = $data[self::PROPERTY_PHONE]['value']; + $phoneNumber = $phoneUtil->parse($phoneValue, 'DE'); // FIXME need a reasonable default + if ($phoneNumber instanceof PhoneNumber && $phoneUtil->isValidNumber($phoneNumber)) { + $data[self::PROPERTY_PHONE]['value'] = $phoneUtil->format($phoneNumber, PhoneNumberFormat::E164); + } else { + throw new \InvalidArgumentException(self::PROPERTY_PHONE); + } + } catch (NumberParseException $e) { + throw new \InvalidArgumentException(self::PROPERTY_PHONE); + } + } + if (empty($userData)) { $this->insertNewUser($user, $data); } elseif ($userData !== $data) { @@ -107,6 +129,8 @@ class AccountManager implements IAccountManager { new GenericEvent($user, $data) ); } + + return $data; } /** -- cgit v1.2.3 From f648635758e34bab0173e02eb9b75aafc5e6a9ff Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 3 Dec 2020 12:16:39 +0100 Subject: Make the throwing optional, so background tasks don't break Signed-off-by: Joas Schilling --- apps/provisioning_api/lib/Controller/UsersController.php | 2 +- apps/settings/lib/Controller/UsersController.php | 2 +- lib/private/Accounts/AccountManager.php | 14 +++++++++++--- 3 files changed, 13 insertions(+), 5 deletions(-) (limited to 'apps/settings/lib/Controller/UsersController.php') diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index edb1fc5121a..18baa07a395 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -655,7 +655,7 @@ class UsersController extends AUserData { if ($userAccount[$key]['value'] !== $value) { $userAccount[$key]['value'] = $value; try { - $this->accountManager->updateUser($targetUser, $userAccount); + $this->accountManager->updateUser($targetUser, $userAccount, true); } catch (\InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); } diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 5b3c1574f96..dba5ec69b2b 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -466,7 +466,7 @@ class UsersController extends Controller { } try { - return $this->accountManager->updateUser($user, $data); + return $this->accountManager->updateUser($user, $data, true); } catch (\InvalidArgumentException $e) { if ($e->getMessage() === IAccountManager::PROPERTY_PHONE) { throw new \InvalidArgumentException($this->l10n->t('Unable to set invalid phone number')); diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index 8640ce269ea..05feaf87b8f 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -125,15 +125,23 @@ class AccountManager implements IAccountManager { * * @param IUser $user * @param array $data + * @param bool $throwOnData Set to true if you can inform the user about invalid data * @return array The potentially modified data (e.g. phone numbers are converted to E.164 format) * @throws \InvalidArgumentException Message is the property that was invalid */ - public function updateUser(IUser $user, array $data): array { + public function updateUser(IUser $user, array $data, bool $throwOnData = false): array { $userData = $this->getUser($user); $updated = true; - if (isset($data[self::PROPERTY_PHONE])) { - $data[self::PROPERTY_PHONE]['value'] = $this->parsePhoneNumber($data[self::PROPERTY_PHONE]['value']); + if (isset($data[self::PROPERTY_PHONE]) && $data[self::PROPERTY_PHONE]['value'] !== '') { + try { + $data[self::PROPERTY_PHONE]['value'] = $this->parsePhoneNumber($data[self::PROPERTY_PHONE]['value']); + } catch (\InvalidArgumentException $e) { + if ($throwOnData) { + throw $e; + } + $data[self::PROPERTY_PHONE]['value'] = ''; + } } if (empty($userData)) { -- cgit v1.2.3