From 46b073d7ce49d9797caf29522358562179846abd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 3 Dec 2020 11:12:41 +0100 Subject: [PATCH] Add a config for default region of phone numbers Signed-off-by: Joas Schilling --- .../lib/Controller/CheckSetupController.php | 1 + .../Controller/CheckSetupControllerTest.php | 1 + .../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(-) diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 7929e9c396..1ebeb41adf 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 43ec984041..965d758634 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 eefaa574e0..123e58a869 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 2710fbf5fd..8adb5bf216 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 214f148fa9..22c8589f73 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 a0a3c2a4ba..c3cddb88a9 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 4f4a146bdd..8640ce269e 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)) {