diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 08c541ffa6..edb1fc5121 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 4267b8be4c..5b3c1574f9 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 75e71435b0..23e3ef5ec0 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 39921dae9d..4f4a146bdd 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; } /**