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 --- lib/private/Accounts/AccountManager.php | 2 +- lib/private/Accounts/Hooks.php | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) (limited to 'lib/private') diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index d18555d296c..0c488eec635 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -173,7 +173,7 @@ class AccountManager implements IAccountManager { 'lastRun' => time() ] ); - $newData[AccountManager::PROPERTY_EMAIL]['verified'] = AccountManager::VERIFICATION_IN_PROGRESS; + $newData[self::PROPERTY_EMAIL]['verified'] = self::VERIFICATION_IN_PROGRESS; } return $newData; diff --git a/lib/private/Accounts/Hooks.php b/lib/private/Accounts/Hooks.php index 82f55f5a9d1..c5e7c34deaf 100644 --- a/lib/private/Accounts/Hooks.php +++ b/lib/private/Accounts/Hooks.php @@ -24,6 +24,7 @@ namespace OC\Accounts; +use OCP\Accounts\IAccountManager; use OCP\IUser; use Psr\Log\LoggerInterface; @@ -61,14 +62,14 @@ class Hooks { switch ($feature) { case 'eMailAddress': - if ($accountData[AccountManager::PROPERTY_EMAIL]['value'] !== $newValue) { - $accountData[AccountManager::PROPERTY_EMAIL]['value'] = $newValue; + if ($accountData[IAccountManager::PROPERTY_EMAIL]['value'] !== $newValue) { + $accountData[IAccountManager::PROPERTY_EMAIL]['value'] = $newValue; $accountManager->updateUser($user, $accountData); } break; case 'displayName': - if ($accountData[AccountManager::PROPERTY_DISPLAYNAME]['value'] !== $newValue) { - $accountData[AccountManager::PROPERTY_DISPLAYNAME]['value'] = $newValue; + if ($accountData[IAccountManager::PROPERTY_DISPLAYNAME]['value'] !== $newValue) { + $accountData[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $newValue; $accountManager->updateUser($user, $accountData); } break; -- cgit v1.2.3 From eaba155a09ea7d226b538c5c8d046d2ea839b452 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 2 Dec 2020 11:40:36 +0100 Subject: Add a database table for the accounts data so we can search it better Signed-off-by: Joas Schilling --- core/Migrations/Version21000Date20201202095923.php | 75 ++++++++++++++++++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Accounts/AccountManager.php | 49 +++++++++++++- version.php | 2 +- 5 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 core/Migrations/Version21000Date20201202095923.php (limited to 'lib/private') diff --git a/core/Migrations/Version21000Date20201202095923.php b/core/Migrations/Version21000Date20201202095923.php new file mode 100644 index 00000000000..6433d8c9b7a --- /dev/null +++ b/core/Migrations/Version21000Date20201202095923.php @@ -0,0 +1,75 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Core\Migrations; + +use Closure; +use Doctrine\DBAL\Types\Types; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version21000Date20201202095923 extends SimpleMigrationStep { + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + if (!$schema->hasTable('accounts_data')) { + $table = $schema->createTable('accounts_data'); + $table->addColumn('id', Types::BIGINT, [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 20, + ]); + $table->addColumn('uid', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('name', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('value', Types::STRING, [ + 'notnull' => false, + 'length' => 255, + 'default' => '', + ]); + $table->setPrimaryKey(['id']); + $table->addIndex(['uid'], 'accounts_data_uid'); + $table->addIndex(['name'], 'accounts_data_name'); + $table->addIndex(['value'], 'accounts_data_value'); + + return $schema; + } + + return null; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 0400e681090..c043ef4d3ba 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -930,6 +930,7 @@ return array( 'OC\\Core\\Migrations\\Version20000Date20201109081918' => $baseDir . '/core/Migrations/Version20000Date20201109081918.php', 'OC\\Core\\Migrations\\Version20000Date20201109081919' => $baseDir . '/core/Migrations/Version20000Date20201109081919.php', 'OC\\Core\\Migrations\\Version20000Date20201111081915' => $baseDir . '/core/Migrations/Version20000Date20201111081915.php', + 'OC\\Core\\Migrations\\Version21000Date20201202095923' => $baseDir . '/core/Migrations/Version21000Date20201202095923.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index b9b4f2f307b..f9177dbb048 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -959,6 +959,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version20000Date20201109081918' => __DIR__ . '/../../..' . '/core/Migrations/Version20000Date20201109081918.php', 'OC\\Core\\Migrations\\Version20000Date20201109081919' => __DIR__ . '/../../..' . '/core/Migrations/Version20000Date20201109081919.php', 'OC\\Core\\Migrations\\Version20000Date20201111081915' => __DIR__ . '/../../..' . '/core/Migrations/Version20000Date20201111081915.php', + 'OC\\Core\\Migrations\\Version21000Date20201202095923' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20201202095923.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index 0c488eec635..c473d563781 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -58,6 +58,9 @@ class AccountManager implements IAccountManager { /** @var string table name */ private $table = 'accounts'; + /** @var string table name */ + private $dataTable = 'accounts_data'; + /** @var EventDispatcherInterface */ private $eventDispatcher; @@ -116,6 +119,21 @@ class AccountManager implements IAccountManager { $query->delete($this->table) ->where($query->expr()->eq('uid', $query->createNamedParameter($uid))) ->execute(); + + $this->deleteUserData($user); + } + + /** + * delete user from accounts table + * + * @param IUser $user + */ + public function deleteUserData(IUser $user): void { + $uid = $user->getUID(); + $query = $this->connection->getQueryBuilder(); + $query->delete($this->dataTable) + ->where($query->expr()->eq('uid', $query->createNamedParameter($uid))) + ->execute(); } /** @@ -256,7 +274,7 @@ class AccountManager implements IAccountManager { * @param IUser $user * @param array $data */ - protected function insertNewUser(IUser $user, $data) { + protected function insertNewUser(IUser $user, array $data): void { $uid = $user->getUID(); $jsonEncodedData = json_encode($data); $query = $this->connection->getQueryBuilder(); @@ -268,6 +286,9 @@ class AccountManager implements IAccountManager { ] ) ->execute(); + + $this->deleteUserData($user); + $this->writeUserData($user, $data); } /** @@ -276,7 +297,7 @@ class AccountManager implements IAccountManager { * @param IUser $user * @param array $data */ - protected function updateExistingUser(IUser $user, $data) { + protected function updateExistingUser(IUser $user, array $data): void { $uid = $user->getUID(); $jsonEncodedData = json_encode($data); $query = $this->connection->getQueryBuilder(); @@ -284,6 +305,30 @@ class AccountManager implements IAccountManager { ->set('data', $query->createNamedParameter($jsonEncodedData)) ->where($query->expr()->eq('uid', $query->createNamedParameter($uid))) ->execute(); + + $this->deleteUserData($user); + $this->writeUserData($user, $data); + } + + protected function writeUserData(IUser $user, array $data): void { + $query = $this->connection->getQueryBuilder(); + $query->insert($this->dataTable) + ->values( + [ + 'uid' => $query->createNamedParameter($user->getUID()), + 'name' => $query->createParameter('name'), + 'value' => $query->createParameter('value'), + ] + ); + foreach ($data as $propertyName => $property) { + if ($propertyName === self::PROPERTY_AVATAR) { + continue; + } + + $query->setParameter('name', $propertyName) + ->setParameter('value', $property['value']); + $query->execute(); + } } /** diff --git a/version.php b/version.php index 24e3e8db6ae..299855816a9 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [21, 0, 0, 7]; +$OC_Version = [21, 0, 0, 8]; // The human readable string $OC_VersionString = '21.0.0 alpha'; -- cgit v1.2.3 From fe9c46e595cba2d8e52db13cf15848a1f9d6f141 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 2 Dec 2020 14:11:47 +0100 Subject: Add an endpoint to search for accounts based on phone number Signed-off-by: Joas Schilling --- apps/provisioning_api/appinfo/routes.php | 1 + .../lib/Controller/UsersController.php | 62 ++++++++++++++++++++++ .../tests/Controller/UsersControllerTest.php | 22 ++++++-- lib/private/Accounts/AccountManager.php | 19 +++++++ lib/public/Accounts/IAccountManager.php | 11 ++++ 5 files changed, 112 insertions(+), 3 deletions(-) (limited to 'lib/private') diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php index fd1579ca843..912dd82e853 100644 --- a/apps/provisioning_api/appinfo/routes.php +++ b/apps/provisioning_api/appinfo/routes.php @@ -48,6 +48,7 @@ return [ // Users ['root' => '/cloud', 'name' => 'Users#getUsers', 'url' => '/users', 'verb' => 'GET'], ['root' => '/cloud', 'name' => 'Users#getUsersDetails', 'url' => '/users/details', 'verb' => 'GET'], + ['root' => '/cloud', 'name' => 'Users#searchByPhoneNumbers', 'url' => '/users/search/by-phone', 'verb' => 'POST'], ['root' => '/cloud', 'name' => 'Users#addUser', 'url' => '/users', 'verb' => 'POST'], ['root' => '/cloud', 'name' => 'Users#getUser', 'url' => '/users/{userId}', 'verb' => 'GET'], ['root' => '/cloud', 'name' => 'Users#getCurrentUser', 'url' => '/user', 'verb' => 'GET'], diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index ddde254ed41..08c541ffa6c 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -41,6 +41,10 @@ declare(strict_types=1); namespace OCA\Provisioning_API\Controller; +use libphonenumber\NumberParseException; +use libphonenumber\PhoneNumber; +use libphonenumber\PhoneNumberFormat; +use libphonenumber\PhoneNumberUtil; use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\HintException; @@ -51,11 +55,13 @@ use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSForbiddenException; +use OCP\Federation\ICloudIdManager; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; use OCP\ILogger; use OCP\IRequest; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -68,6 +74,10 @@ class UsersController extends AUserData { /** @var IAppManager */ private $appManager; + /** @var ICloudIdManager */ + protected $cloudIdManager; + /** @var IURLGenerator */ + protected $urlGenerator; /** @var ILogger */ private $logger; /** @var IFactory */ @@ -91,6 +101,8 @@ class UsersController extends AUserData { IGroupManager $groupManager, IUserSession $userSession, AccountManager $accountManager, + ICloudIdManager $cloudIdManager, + IURLGenerator $urlGenerator, ILogger $logger, IFactory $l10nFactory, NewUserMailHelper $newUserMailHelper, @@ -108,6 +120,8 @@ class UsersController extends AUserData { $l10nFactory); $this->appManager = $appManager; + $this->cloudIdManager = $cloudIdManager; + $this->urlGenerator = $urlGenerator; $this->logger = $logger; $this->l10nFactory = $l10nFactory; $this->newUserMailHelper = $newUserMailHelper; @@ -202,6 +216,54 @@ class UsersController extends AUserData { ]); } + + /** + * @NoAdminRequired + * @NoSubAdminRequired + * + * @param string $location + * @param array $search + * @return DataResponse + */ + public function searchByPhoneNumbers(string $location, array $search): DataResponse { + $phoneUtil = PhoneNumberUtil::getInstance(); + + $normalizedNumberToKey = []; + foreach ($search as $key => $phoneNumbers) { + foreach ($phoneNumbers as $phone) { + try { + $phoneNumber = $phoneUtil->parse($phone, $location); + if ($phoneNumber instanceof PhoneNumber && $phoneUtil->isValidNumber($phoneNumber)) { + $normalizedNumber = $phoneUtil->format($phoneNumber, PhoneNumberFormat::E164); + $normalizedNumberToKey[$normalizedNumber] = (string) $key; + } + } catch (NumberParseException $e) { + } + } + } + + $phoneNumbers = array_keys($normalizedNumberToKey); + + if (empty($phoneNumbers)) { + return new DataResponse(); + } + + $userMatches = $this->accountManager->searchUsers(IAccountManager::PROPERTY_PHONE, $phoneNumbers); + + if (empty($userMatches)) { + return new DataResponse(); + } + + $cloudUrl = $this->urlGenerator->getAbsoluteURL('/'); + + $matches = []; + foreach ($userMatches as $phone => $userId) { + $matches[$normalizedNumberToKey[$phone]] = $this->cloudIdManager->getCloudId($userId, $cloudUrl)->getId(); + } + + return new DataResponse($matches); + } + /** * @throws OCSException */ diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 8bc60551005..db00963626c 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -52,11 +52,13 @@ use OCP\Accounts\IAccountManager; use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Federation\ICloudIdManager; use OCP\IConfig; use OCP\IGroup; use OCP\IL10N; use OCP\ILogger; use OCP\IRequest; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -86,6 +88,10 @@ class UsersControllerTest extends TestCase { protected $api; /** @var AccountManager|MockObject */ protected $accountManager; + /** @var ICloudIdManager|MockObject */ + protected $cloudIdManager; + /** @var IURLGenerator|MockObject */ + protected $urlGenerator; /** @var IRequest|MockObject */ protected $request; /** @var IFactory|MockObject */ @@ -112,6 +118,8 @@ class UsersControllerTest extends TestCase { $this->logger = $this->createMock(ILogger::class); $this->request = $this->createMock(IRequest::class); $this->accountManager = $this->createMock(AccountManager::class); + $this->cloudIdManager = $this->createMock(ICloudIdManager::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->newUserMailHelper = $this->createMock(NewUserMailHelper::class); $this->federatedShareProviderFactory = $this->createMock(FederatedShareProviderFactory::class); @@ -129,6 +137,8 @@ class UsersControllerTest extends TestCase { $this->groupManager, $this->userSession, $this->accountManager, + $this->cloudIdManager, + $this->urlGenerator, $this->logger, $this->l10nFactory, $this->newUserMailHelper, @@ -382,7 +392,7 @@ class UsersControllerTest extends TestCase { } public function testAddUserSuccessfulWithDisplayName() { - $api = $this->getMockBuilder('OCA\Provisioning_API\Controller\UsersController') + $api = $this->getMockBuilder(UsersController::class) ->setConstructorArgs([ 'provisioning_api', $this->request, @@ -392,6 +402,8 @@ class UsersControllerTest extends TestCase { $this->groupManager, $this->userSession, $this->accountManager, + $this->cloudIdManager, + $this->urlGenerator, $this->logger, $this->l10nFactory, $this->newUserMailHelper, @@ -3163,7 +3175,7 @@ class UsersControllerTest extends TestCase { ->willReturn($user); /** @var UsersController | MockObject $api */ - $api = $this->getMockBuilder('OCA\Provisioning_API\Controller\UsersController') + $api = $this->getMockBuilder(UsersController::class) ->setConstructorArgs([ 'provisioning_api', $this->request, @@ -3173,6 +3185,8 @@ class UsersControllerTest extends TestCase { $this->groupManager, $this->userSession, $this->accountManager, + $this->cloudIdManager, + $this->urlGenerator, $this->logger, $this->l10nFactory, $this->newUserMailHelper, @@ -3228,7 +3242,7 @@ class UsersControllerTest extends TestCase { public function testGetUser() { /** @var UsersController | MockObject $api */ - $api = $this->getMockBuilder('OCA\Provisioning_API\Controller\UsersController') + $api = $this->getMockBuilder(UsersController::class) ->setConstructorArgs([ 'provisioning_api', $this->request, @@ -3238,6 +3252,8 @@ class UsersControllerTest extends TestCase { $this->groupManager, $this->userSession, $this->accountManager, + $this->cloudIdManager, + $this->urlGenerator, $this->logger, $this->l10nFactory, $this->newUserMailHelper, diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index c473d563781..39921dae9db 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -34,6 +34,7 @@ use OCA\Settings\BackgroundJobs\VerifyUserData; use OCP\Accounts\IAccount; use OCP\Accounts\IAccountManager; use OCP\BackgroundJob\IJobList; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IUser; use Psr\Log\LoggerInterface; @@ -171,6 +172,24 @@ class AccountManager implements IAccountManager { return $userDataArray; } + public function searchUsers(string $property, array $values): array { + $query = $this->connection->getQueryBuilder(); + $query->select('*') + ->from($this->dataTable) + ->where($query->expr()->eq('name', $query->createNamedParameter($property))) + ->andWhere($query->expr()->in('value', $query->createNamedParameter($values, IQueryBuilder::PARAM_STR_ARRAY))); + + $result = $query->execute(); + $matches = []; + + while ($row = $result->fetch()) { + $matches[$row['value']] = $row['uid']; + } + $result->closeCursor(); + + return $matches; + } + /** * check if we need to ask the server for email verification, if yes we create a cronjob * diff --git a/lib/public/Accounts/IAccountManager.php b/lib/public/Accounts/IAccountManager.php index 3306abc55f1..63824ed94b9 100644 --- a/lib/public/Accounts/IAccountManager.php +++ b/lib/public/Accounts/IAccountManager.php @@ -65,4 +65,15 @@ interface IAccountManager { * @return IAccount */ public function getAccount(IUser $user): IAccount; + + /** + * Search for users based on account data + * + * @param string $property + * @param string[] $values + * @return array + * + * @since 21.0.0 + */ + public function searchUsers(string $property, array $values): array; } -- 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 'lib/private') 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 46b073d7ce49d9797caf29522358562179846abd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 3 Dec 2020 11:12:41 +0100 Subject: Add a config for default region of phone numbers Signed-off-by: Joas Schilling --- .../lib/Controller/CheckSetupController.php | 1 + .../tests/Controller/CheckSetupControllerTest.php | 1 + build/integration/features/provisioning-v1.feature | 4 +- config/config.sample.php | 10 ++++ core/js/setupchecks.js | 6 ++ core/js/tests/specs/setupchecksSpec.js | 69 ++++++++++++++++++++++ lib/private/Accounts/AccountManager.php | 48 +++++++++++---- 7 files changed, 125 insertions(+), 14 deletions(-) (limited to 'lib/private') diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 7929e9c3962..1ebeb41adfb 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -752,6 +752,7 @@ Raw output PhpOutputBuffering::class => ['pass' => $phpOutputBuffering->run(), 'description' => $phpOutputBuffering->description(), 'severity' => $phpOutputBuffering->severity()], LegacySSEKeyFormat::class => ['pass' => $legacySSEKeyFormat->run(), 'description' => $legacySSEKeyFormat->description(), 'severity' => $legacySSEKeyFormat->severity(), 'linkToDocumentation' => $legacySSEKeyFormat->linkToDocumentation()], CheckUserCertificates::class => ['pass' => $checkUserCertificates->run(), 'description' => $checkUserCertificates->description(), 'severity' => $checkUserCertificates->severity(), 'elements' => $checkUserCertificates->elements()], + 'isDefaultPhoneRegionSet' => $this->config->getSystemValueString('default_phone_region', '') !== '', ] ); } diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 43ec984041c..965d7586343 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -605,6 +605,7 @@ class CheckSetupControllerTest extends TestCase { 'OCA\Settings\SetupChecks\LegacySSEKeyFormat' => ['pass' => true, 'description' => 'The old server-side-encryption format is enabled. We recommend disabling this.', 'severity' => 'warning', 'linkToDocumentation' => ''], 'OCA\Settings\SetupChecks\CheckUserCertificates' => ['pass' => false, 'description' => 'There are some user imported SSL certificates present, that are not used anymore with Nextcloud 21. They can be imported on the command line via "occ security:certificates:import" command. Their paths inside the data directory are shown below.', 'severity' => 'warning', 'elements' => ['a', 'b']], 'imageMagickLacksSVGSupport' => false, + 'isDefaultPhoneRegionSet' => false, ] ); $this->assertEquals($expected, $this->checkSetupController->check()); diff --git a/build/integration/features/provisioning-v1.feature b/build/integration/features/provisioning-v1.feature index eefaa574e03..123e58a869c 100644 --- a/build/integration/features/provisioning-v1.feature +++ b/build/integration/features/provisioning-v1.feature @@ -76,7 +76,7 @@ Feature: provisioning And the HTTP status code should be "200" And sending "PUT" to "/cloud/users/brand-new-user" with | key | phone | - | value | 0711 / 25 24 28-90 | + | value | +49 711 / 25 24 28-90 | And the OCS status code should be "100" And the HTTP status code should be "200" And sending "PUT" to "/cloud/users/brand-new-user" with @@ -108,7 +108,7 @@ Feature: provisioning And user "phone-user" exists And sending "PUT" to "/cloud/users/phone-user" with | key | phone | - | value | 0711 / 25 24 28-90 | + | value | +49 711 / 25 24 28-90 | And the OCS status code should be "100" And the HTTP status code should be "200" Then search users by phone for region "DE" with diff --git a/config/config.sample.php b/config/config.sample.php index 2710fbf5fdb..8adb5bf2168 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -194,6 +194,16 @@ $CONFIG = [ */ 'default_locale' => 'en_US', +/** + * This sets the default region for phone numbers on your Nextcloud server, + * using ISO 3166-1 country codes such as ``DE`` for Germany, ``FR`` for France, … + * It is required to allow inserting phone numbers in the user profiles starting + * without the country code (e.g. +49 for Germany). + * + * No default value! + */ +'default_phone_region' => 'EN', + /** * With this setting a locale can be forced for all users. If a locale is * forced, the users are also unable to change their locale in the personal diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 214f148fa94..22c8589f73b 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -216,6 +216,12 @@ type: OC.SetupChecks.MESSAGE_TYPE_WARNING }); } + if (!data.isDefaultPhoneRegionSet) { + messages.push({ + msg: t('core', 'Your installation has no default phone region set. This is required to be able to validate phone numbers in the profile settings without a country code. To allow numbers without a country code, please add "default_phone_region" with the respective ISO 3166-1 code of the wished region.'), + type: OC.SetupChecks.MESSAGE_TYPE_INFO + }); + } if (data.cronErrors.length > 0) { var listOfCronErrors = ""; data.cronErrors.forEach(function(element){ diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index a0a3c2a4ba9..c3cddb88a9d 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -251,6 +251,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -306,6 +307,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -362,6 +364,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -416,6 +419,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -468,6 +472,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -522,6 +527,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -574,6 +580,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -626,6 +633,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -678,6 +686,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -751,6 +760,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -804,6 +814,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -857,6 +868,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -910,6 +922,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -962,6 +975,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: true, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyGeneratedURL: 'https://server', }) @@ -1014,6 +1028,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', reverseProxyGeneratedURL: 'http://server', @@ -1067,6 +1082,7 @@ describe('OC.SetupChecks tests', function() { recommendedPHPModules: [], pendingBigIntConversionColumns: [], isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: false, reverseProxyGeneratedURL: 'https://server', }) @@ -1080,6 +1096,59 @@ describe('OC.SetupChecks tests', function() { done(); }); }); + + it('should return an info if there is no default phone region', function(done) { + var async = OC.SetupChecks.checkSetup(); + + suite.server.requests[0].respond( + 200, + { + 'Content-Type': 'application/json', + }, + JSON.stringify({ + hasFileinfoInstalled: true, + isGetenvServerWorking: true, + isReadOnlyConfig: false, + hasWorkingFileLocking: true, + hasValidTransactionIsolationLevel: true, + suggestedOverwriteCliURL: '', + isRandomnessSecure: true, + securityDocs: 'https://docs.owncloud.org/myDocs.html', + serverHasInternetConnectionProblems: false, + isMemcacheConfigured: true, + forwardedForHeadersWorking: true, + isCorrectMemcachedPHPModuleInstalled: true, + hasPassedCodeIntegrityCheck: true, + isOpcacheProperlySetup: true, + hasOpcacheLoaded: true, + isSettimelimitAvailable: true, + hasFreeTypeSupport: true, + missingIndexes: [], + missingPrimaryKeys: [], + missingColumns: [], + cronErrors: [], + cronInfo: { + diffInSeconds: 0 + }, + isMemoryLimitSufficient: true, + appDirsWithDifferentOwner: [], + recommendedPHPModules: [], + pendingBigIntConversionColumns: [], + isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: false, + isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, + reverseProxyGeneratedURL: 'https://server', + }) + ); + + async.done(function( data, s, x ){ + expect(data).toEqual([{ + msg: 'Your installation has no default phone region set. This is required to be able to validate phone numbers in the profile settings without a country code. To allow numbers without a country code, please add "default_phone_region" with the respective ISO 3166-1 code of the wished region.', + type: OC.SetupChecks.MESSAGE_TYPE_INFO + }]); + done(); + }); + }); }); describe('checkGeneric', function() { diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index 4f4a146bdd9..8640ce269ea 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -39,6 +39,7 @@ use OCP\Accounts\IAccount; use OCP\Accounts\IAccountManager; use OCP\BackgroundJob\IJobList; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IConfig; use OCP\IDBConnection; use OCP\IUser; use Psr\Log\LoggerInterface; @@ -60,6 +61,9 @@ class AccountManager implements IAccountManager { /** @var IDBConnection database connection */ private $connection; + /** @var IConfig */ + private $config; + /** @var string table name */ private $table = 'accounts'; @@ -76,15 +80,46 @@ class AccountManager implements IAccountManager { private $logger; public function __construct(IDBConnection $connection, + IConfig $config, EventDispatcherInterface $eventDispatcher, IJobList $jobList, LoggerInterface $logger) { $this->connection = $connection; + $this->config = $config; $this->eventDispatcher = $eventDispatcher; $this->jobList = $jobList; $this->logger = $logger; } + /** + * @param string $input + * @return string Provided phone number in E.164 format when it was a valid number + * @throws \InvalidArgumentException When the phone number was invalid or no default region is set and the number doesn't start with a country code + */ + protected function parsePhoneNumber(string $input): string { + $defaultRegion = $this->config->getSystemValueString('default_phone_region', ''); + + if ($defaultRegion === '') { + // When no default region is set, only +49… numbers are valid + if (strpos($input, '+') !== 0) { + throw new \InvalidArgumentException(self::PROPERTY_PHONE); + } + + $defaultRegion = 'EN'; + } + + $phoneUtil = PhoneNumberUtil::getInstance(); + try { + $phoneNumber = $phoneUtil->parse($input, $defaultRegion); + if ($phoneNumber instanceof PhoneNumber && $phoneUtil->isValidNumber($phoneNumber)) { + return $phoneUtil->format($phoneNumber, PhoneNumberFormat::E164); + } + } catch (NumberParseException $e) { + } + + throw new \InvalidArgumentException(self::PROPERTY_PHONE); + } + /** * update user record * @@ -98,18 +133,7 @@ class AccountManager implements IAccountManager { $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); - } + $data[self::PROPERTY_PHONE]['value'] = $this->parsePhoneNumber($data[self::PROPERTY_PHONE]['value']); } if (empty($userData)) { -- 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 'lib/private') 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 From 354c5ff024570bad20ff2315372d4e78a5d1394c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 7 Dec 2020 16:36:49 +0100 Subject: Add a repairstep to validate the phone numbers Signed-off-by: Joas Schilling --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Repair.php | 4 +- lib/private/Repair/NC21/ValidatePhoneNumber.php | 89 +++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 lib/private/Repair/NC21/ValidatePhoneNumber.php (limited to 'lib/private') diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index c043ef4d3ba..b7dbc6675d2 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1267,6 +1267,7 @@ return array( 'OC\\Repair\\NC20\\EncryptionMigration' => $baseDir . '/lib/private/Repair/NC20/EncryptionMigration.php', 'OC\\Repair\\NC20\\ShippedDashboardEnable' => $baseDir . '/lib/private/Repair/NC20/ShippedDashboardEnable.php', 'OC\\Repair\\NC21\\AddCheckForUserCertificatesJob' => $baseDir . '/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php', + 'OC\\Repair\\NC21\\ValidatePhoneNumber' => $baseDir . '/lib/private/Repair/NC21/ValidatePhoneNumber.php', 'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\DropAccountTermsTable' => $baseDir . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php', 'OC\\Repair\\Owncloud\\SaveAccountsTableData' => $baseDir . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index f9177dbb048..a8984b486f3 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1296,6 +1296,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Repair\\NC20\\EncryptionMigration' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/EncryptionMigration.php', 'OC\\Repair\\NC20\\ShippedDashboardEnable' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/ShippedDashboardEnable.php', 'OC\\Repair\\NC21\\AddCheckForUserCertificatesJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php', + 'OC\\Repair\\NC21\\ValidatePhoneNumber' => __DIR__ . '/../../..' . '/lib/private/Repair/NC21/ValidatePhoneNumber.php', 'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\DropAccountTermsTable' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php', 'OC\\Repair\\Owncloud\\SaveAccountsTableData' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php', diff --git a/lib/private/Repair.php b/lib/private/Repair.php index ec748355567..847a41aeb25 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -53,6 +53,7 @@ use OC\Repair\NC20\EncryptionLegacyCipher; use OC\Repair\NC20\EncryptionMigration; use OC\Repair\NC20\ShippedDashboardEnable; use OC\Repair\NC21\AddCheckForUserCertificatesJob; +use OC\Repair\NC21\ValidatePhoneNumber; use OC\Repair\OldGroupMembershipShares; use OC\Repair\Owncloud\DropAccountTermsTable; use OC\Repair\Owncloud\SaveAccountsTableData; @@ -177,7 +178,8 @@ class Repair implements IOutput { */ public static function getExpensiveRepairSteps() { return [ - new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()) + new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()), + \OC::$server->get(ValidatePhoneNumber::class), ]; } diff --git a/lib/private/Repair/NC21/ValidatePhoneNumber.php b/lib/private/Repair/NC21/ValidatePhoneNumber.php new file mode 100644 index 00000000000..06bd560a153 --- /dev/null +++ b/lib/private/Repair/NC21/ValidatePhoneNumber.php @@ -0,0 +1,89 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Repair\NC21; + +use OC\Accounts\AccountManager; +use OCP\Accounts\IAccountManager; +use OCP\IConfig; +use OCP\IUser; +use OCP\IUserManager; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +class ValidatePhoneNumber implements IRepairStep { + + /** @var IConfig */ + protected $config; + /** @var IUserManager */ + protected $userManager; + /** @var AccountManager */ + private $accountManager; + + public function __construct(IUserManager $userManager, + AccountManager $accountManager, + IConfig $config) { + $this->config = $config; + $this->userManager = $userManager; + $this->accountManager = $accountManager; + } + + public function getName(): string { + return 'Validate the phone number and store it in a known format for search'; + } + + private function shouldRun(): bool { + return true; + } + + public function run(IOutput $output): void { + if ($this->config->getSystemValueString('default_phone_region', '') === '') { + throw new \Exception('Can not validate phone numbers without `default_phone_region` being set in the config file'); + } + + $numUpdated = 0; + $numRemoved = 0; + + $this->userManager->callForSeenUsers(function(IUser $user) use(&$numUpdated, &$numRemoved) { + $account = $this->accountManager->getUser($user); + + if ($account[IAccountManager::PROPERTY_PHONE]['value'] !== '') { + $updated = $this->accountManager->updateUser($user, $account); + + if ($account[IAccountManager::PROPERTY_PHONE]['value'] !== $updated[IAccountManager::PROPERTY_PHONE]['value']) { + if ($updated[IAccountManager::PROPERTY_PHONE]['value'] === '') { + $numRemoved++; + } else { + $numUpdated++; + } + } + } + }); + + if ($numRemoved > 0 || $numUpdated > 0) { + $output->info('Updated ' . $numUpdated . ' entries and cleaned ' . $numRemoved . ' invalid phone numbers'); + } + } +} -- cgit v1.2.3 From 13a438b3224f3f42a0f552230f680a243e1af705 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 8 Dec 2020 13:08:47 +0100 Subject: Fix PHP code style Signed-off-by: Joas Schilling --- lib/private/Repair/NC21/ValidatePhoneNumber.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/private') diff --git a/lib/private/Repair/NC21/ValidatePhoneNumber.php b/lib/private/Repair/NC21/ValidatePhoneNumber.php index 06bd560a153..6e25ff26b7e 100644 --- a/lib/private/Repair/NC21/ValidatePhoneNumber.php +++ b/lib/private/Repair/NC21/ValidatePhoneNumber.php @@ -66,7 +66,7 @@ class ValidatePhoneNumber implements IRepairStep { $numUpdated = 0; $numRemoved = 0; - $this->userManager->callForSeenUsers(function(IUser $user) use(&$numUpdated, &$numRemoved) { + $this->userManager->callForSeenUsers(function (IUser $user) use (&$numUpdated, &$numRemoved) { $account = $this->accountManager->getUser($user); if ($account[IAccountManager::PROPERTY_PHONE]['value'] !== '') { -- cgit v1.2.3