upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-04-20 22:00:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 16

fix(phonenumber): Use the newly introduced API to limit 3rdparty lib usage

Browse source 
Signed-off-by: Joas Schilling <coding@schilljs.com>
This commit is contained in:
Joas Schilling 2023-09-25 15:33:40 +02:00
parent 5092278b0f
commit 0956b493b6
No known key found for this signature in database
GPG key ID: 74434EFE0D2E2205
4 changed files with 39 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
apps/provisioning_api/lib/Controller/UsersController.php
View file

@ -45,9 +45,6 @@ declare(strict_types=1);
namespace OCA\Provisioning_API\Controller;
use InvalidArgumentException;
use libphonenumber\NumberParseException;
use libphonenumber\PhoneNumberFormat;
use libphonenumber\PhoneNumberUtil;
use OC\Authentication\Token\RemoteWipe;
use OC\KnownUser\KnownUserService;
use OC\User\Backend;
@ -66,6 +63,7 @@ use OCP\HintException;
use OCP\IConfig;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IPhoneNumberUtil;
use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\IUser;
@ -113,7 +111,8 @@ class UsersController extends AUserData {
ISecureRandom $secureRandom,
RemoteWipe $remoteWipe,
KnownUserService $knownUserService,
IEventDispatcher $eventDispatcher
IEventDispatcher $eventDispatcher,
private IPhoneNumberUtil $phoneNumberUtil,
) {
parent::__construct(
$appName,
@ -243,9 +242,7 @@ class UsersController extends AUserData {
* 400: Invalid location
*/
public function searchByPhoneNumbers(string $location, array $search): DataResponse {
$phoneUtil = PhoneNumberUtil::getInstance();
if ($phoneUtil->getCountryCodeForRegion($location) === 0) {
if ($this->phoneNumberUtil->getCountryCodeForRegion($location) === null) {
// Not a valid region code
return new DataResponse([], Http::STATUS_BAD_REQUEST);
}
@ -258,26 +255,18 @@ class UsersController extends AUserData {
$normalizedNumberToKey = [];
foreach ($search as $key => $phoneNumbers) {
foreach ($phoneNumbers as $phone) {
try {
$phoneNumber = $phoneUtil->parse($phone, $location);
if ($phoneUtil->isValidNumber($phoneNumber)) {
$normalizedNumber = $phoneUtil->format($phoneNumber, PhoneNumberFormat::E164);
$normalizedNumberToKey[$normalizedNumber] = (string) $key;
}
} catch (NumberParseException $e) {
$normalizedNumber = $this->phoneNumberUtil->convertToStandardFormat($phone, $location);
if ($normalizedNumber !== null) {
$normalizedNumberToKey[$normalizedNumber] = (string) $key;
}
if ($defaultPhoneRegion !== '' && $defaultPhoneRegion !== $location && strpos($phone, '0') === 0) {
if ($defaultPhoneRegion !== '' && $defaultPhoneRegion !== $location && str_starts_with($phone, '0')) {
// If the number has a leading zero (no country code),
// we also check the default phone region of the instance,
// when it's different to the user's given region.
try {
$phoneNumber = $phoneUtil->parse($phone, $defaultPhoneRegion);
if ($phoneUtil->isValidNumber($phoneNumber)) {
$normalizedNumber = $phoneUtil->format($phoneNumber, PhoneNumberFormat::E164);
$normalizedNumberToKey[$normalizedNumber] = (string) $key;
}
} catch (NumberParseException $e) {
$normalizedNumber = $this->phoneNumberUtil->convertToStandardFormat($phone, $defaultPhoneRegion);
if ($normalizedNumber !== null) {
$normalizedNumberToKey[$normalizedNumber] = (string) $key;
}
}
}

19
apps/provisioning_api/tests/Controller/UsersControllerTest.php
View file

@ -46,6 +46,7 @@ use Exception;
use OC\Authentication\Token\RemoteWipe;
use OC\Group\Manager;
use OC\KnownUser\KnownUserService;
use OC\PhoneNumberUtil;
use OC\SubAdmin;
use OCA\Provisioning_API\Controller\UsersController;
use OCA\Settings\Mailer\NewUserMailHelper;
@ -59,6 +60,7 @@ use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IGroup;
use OCP\IL10N;
use OCP\IPhoneNumberUtil;
use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\IUser;
@ -103,8 +105,10 @@ class UsersControllerTest extends TestCase {
private $remoteWipe;
/** @var KnownUserService|MockObject */
private $knownUserService;
/** @var IEventDispatcher */
/** @var IEventDispatcher|MockObject */
private $eventDispatcher;
/** @var IPhoneNumberUtil */
private $phoneNumberUtil;
protected function setUp(): void {
parent::setUp();
@ -123,6 +127,7 @@ class UsersControllerTest extends TestCase {
$this->remoteWipe = $this->createMock(RemoteWipe::class);
$this->knownUserService = $this->createMock(KnownUserService::class);
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->phoneNumberUtil = new PhoneNumberUtil();
$this->api = $this->getMockBuilder(UsersController::class)
->setConstructorArgs([
@ -141,8 +146,9 @@ class UsersControllerTest extends TestCase {
$this->remoteWipe,
$this->knownUserService,
$this->eventDispatcher,
$this->phoneNumberUtil,
])
->setMethods(['fillStorageInfo'])
->onlyMethods(['fillStorageInfo'])
->getMock();
}
@ -411,8 +417,9 @@ class UsersControllerTest extends TestCase {
$this->remoteWipe,
$this->knownUserService,
$this->eventDispatcher,
$this->phoneNumberUtil,
])
->setMethods(['editUser'])
->onlyMethods(['editUser'])
->getMock();
$this->userManager
@ -3693,8 +3700,9 @@ class UsersControllerTest extends TestCase {
$this->remoteWipe,
$this->knownUserService,
$this->eventDispatcher,
$this->phoneNumberUtil,
])
->setMethods(['getUserData'])
->onlyMethods(['getUserData'])
->getMock();
$api->expects($this->once())->method('getUserData')->with('UID', true)
@ -3779,8 +3787,9 @@ class UsersControllerTest extends TestCase {
$this->remoteWipe,
$this->knownUserService,
$this->eventDispatcher,
$this->phoneNumberUtil,
])
->setMethods(['getUserData'])
->onlyMethods(['getUserData'])
->getMock();
$expected = [

15
lib/private/Accounts/AccountManager.php
View file

@ -37,9 +37,6 @@ namespace OC\Accounts;
use Exception;
use InvalidArgumentException;
use libphonenumber\NumberParseException;
use libphonenumber\PhoneNumberFormat;
use libphonenumber\PhoneNumberUtil;
use OC\Profile\TProfileHelper;
use OCP\Accounts\UserUpdatedEvent;
use OCP\Cache\CappedMemoryCache;
@ -56,6 +53,7 @@ use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IL10N;
use OCP\IPhoneNumberUtil;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\L10N\IFactory;
@ -119,6 +117,7 @@ class AccountManager implements IAccountManager {
private IFactory $l10nFactory,
private IURLGenerator $urlGenerator,
private ICrypto $crypto,
private IPhoneNumberUtil $phoneNumberUtil,
) {
$this->internalCache = new CappedMemoryCache();
}
@ -139,13 +138,9 @@ class AccountManager implements IAccountManager {
$defaultRegion = 'EN';
}
$phoneUtil = PhoneNumberUtil::getInstance();
try {
$phoneNumber = $phoneUtil->parse($input, $defaultRegion);
if ($phoneUtil->isValidNumber($phoneNumber)) {
return $phoneUtil->format($phoneNumber, PhoneNumberFormat::E164);
}
} catch (NumberParseException $e) {
$phoneNumber = $this->phoneNumberUtil->convertToStandardFormat($input, $defaultRegion);
if ($phoneNumber !== null) {
return $phoneNumber;
}
throw new InvalidArgumentException(self::PROPERTY_PHONE);

11
tests/lib/Accounts/AccountManagerTest.php
View file

@ -26,6 +26,7 @@ namespace Test\Accounts;
use OC\Accounts\Account;
use OC\Accounts\AccountManager;
use OC\PhoneNumberUtil;
use OCA\Settings\BackgroundJobs\VerifyUserData;
use OCP\Accounts\IAccountManager;
use OCP\Accounts\UserUpdatedEvent;
@ -34,6 +35,7 @@ use OCP\Defaults;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IPhoneNumberUtil;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\L10N\IFactory;
@ -75,6 +77,8 @@ class AccountManagerTest extends TestCase {
/** @var IJobList|MockObject */
private $jobList;
/** @var IPhoneNumberUtil */
private $phoneNumberUtil;
/** accounts table name */
private string $table = 'accounts';
@ -97,6 +101,7 @@ class AccountManagerTest extends TestCase {
$this->l10nFactory = $this->createMock(IFactory::class);
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->crypto = $this->createMock(ICrypto::class);
$this->phoneNumberUtil = new PhoneNumberUtil();
$this->accountManager = new AccountManager(
$this->connection,
@ -109,7 +114,8 @@ class AccountManagerTest extends TestCase {
$this->defaults,
$this->l10nFactory,
$this->urlGenerator,
$this->crypto
$this->crypto,
$this->phoneNumberUtil,
);
}
@ -473,7 +479,8 @@ class AccountManagerTest extends TestCase {
$this->defaults,
$this->l10nFactory,
$this->urlGenerator,
$this->crypto
$this->crypto,
$this->phoneNumberUtil,
])
->onlyMethods($mockedMethods)
->getMock();