Merge pull request #24486 from nextcloud/feature/noid/phone-number-validation

Phone number validation and search

7 files changed, 237 insertions, 10 deletions

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 0400e681090..b7dbc6675d2 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',
@@ -1266,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 b9b4f2f307b..a8984b486f3 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',
@@ -1295,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/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php
index d18555d296c..05feaf87b8f 100644
--- a/lib/private/Accounts/AccountManager.php
+++ b/lib/private/Accounts/AccountManager.php
@@ -30,10 +30,16 @@
 
 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;
 use OCP\BackgroundJob\IJobList;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IUser;
 use Psr\Log\LoggerInterface;
@@ -55,9 +61,15 @@ class AccountManager implements IAccountManager {
 	/** @var  IDBConnection database connection */
 	private $connection;
 
+	/** @var IConfig */
+	private $config;
+
 	/** @var string table name */
 	private $table = 'accounts';
 
+	/** @var string table name */
+	private $dataTable = 'accounts_data';
+
 	/** @var EventDispatcherInterface */
 	private $eventDispatcher;
 
@@ -68,24 +80,70 @@ 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
 	 *
 	 * @param IUser $user
-	 * @param $data
+	 * @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, $data) {
+	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'] !== '') {
+			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)) {
 			$this->insertNewUser($user, $data);
 		} elseif ($userData !== $data) {
@@ -103,6 +161,8 @@ class AccountManager implements IAccountManager {
 				new GenericEvent($user, $data)
 			);
 		}
+
+		return $data;
 	}
 
 	/**
@@ -116,6 +176,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();
 	}
 
 	/**
@@ -153,6 +228,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
 	 *
@@ -173,7 +266,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;
@@ -256,7 +349,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 +361,9 @@ class AccountManager implements IAccountManager {
 				]
 			)
 			->execute();
+
+		$this->deleteUserData($user);
+		$this->writeUserData($user, $data);
 	}
 
 	/**
@@ -276,7 +372,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 +380,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/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;
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..6e25ff26b7e
--- /dev/null
+++ b/lib/private/Repair/NC21/ValidatePhoneNumber.php
@@ -0,0 +1,89 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2020 Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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');
+		}
+	}
+}
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;
 }