From 9e04e6f99ad7a8d78b0bf09a414ed0f1aac3e5db Mon Sep 17 00:00:00 2001
From: Joas Schilling <coding@schilljs.com>
Date: Wed, 2 Dec 2020 16:03:08 +0100
Subject: [PATCH] Also translate the phone number when coming in via the
accounts manager API directly
Signed-off-by: Joas Schilling <coding@schilljs.com>
---
.../lib/Controller/UsersController.php | 6 +++-
.../lib/Controller/UsersController.php | 29 +++++--------------
.../tests/Controller/UsersControllerTest.php | 8 +++--
lib/private/Accounts/AccountManager.php | 28 ++++++++++++++++--
4 files changed, 43 insertions(+), 28 deletions(-)
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;
}
/**
--
GitLab