Skip to content

Commit

Permalink
fix(phonenumber): Use the newly introduced API to limit 3rdparty lib …
Browse files Browse the repository at this point in the history
…usage

Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
nickvergessen committed Sep 25, 2023
1 parent 5092278 commit 4f7dc69
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 32 deletions.
33 changes: 11 additions & 22 deletions apps/provisioning_api/lib/Controller/UsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@
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;
Expand All @@ -66,6 +63,7 @@
use OCP\IConfig;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IPhoneNumberUtil;
use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\IUser;
Expand Down Expand Up @@ -113,7 +111,8 @@ public function __construct(
ISecureRandom $secureRandom,
RemoteWipe $remoteWipe,
KnownUserService $knownUserService,
IEventDispatcher $eventDispatcher
IEventDispatcher $eventDispatcher,
private IPhoneNumberUtil $phoneNumberUtil,
) {
parent::__construct(
$appName,
Expand Down Expand Up @@ -243,9 +242,7 @@ public function getUsersDetails(string $search = '', int $limit = null, int $off
* 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);
}
Expand All @@ -258,26 +255,18 @@ public function searchByPhoneNumbers(string $location, array $search): DataRespo
$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;

Check notice

Code scanning / Psalm

RedundantCastGivenDocblockType Note

Redundant cast to string given docblock-provided type
}

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;

Check notice

Code scanning / Psalm

RedundantCastGivenDocblockType Note

Redundant cast to string given docblock-provided type
}
}
}
Expand Down
15 changes: 5 additions & 10 deletions lib/private/Accounts/AccountManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@

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;
Expand All @@ -56,6 +53,7 @@
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IL10N;
use OCP\IPhoneNumber;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\L10N\IFactory;
Expand Down Expand Up @@ -119,6 +117,7 @@ public function __construct(
private IFactory $l10nFactory,
private IURLGenerator $urlGenerator,
private ICrypto $crypto,
private IPhoneNumberUtil $phoneNumberUtil,

Check failure on line 120 in lib/private/Accounts/AccountManager.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

UndefinedClass

lib/private/Accounts/AccountManager.php:120:3: UndefinedClass: Class, interface or enum named OC\Accounts\IPhoneNumberUtil does not exist (see https://psalm.dev/019)

Check failure

Code scanning / Psalm

UndefinedClass Error

Class, interface or enum named OC\Accounts\IPhoneNumberUtil does not exist
) {
$this->internalCache = new CappedMemoryCache();
}
Expand All @@ -139,13 +138,9 @@ protected function parsePhoneNumber(string $input): string {
$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->parse($input, $defaultRegion);

Check failure on line 141 in lib/private/Accounts/AccountManager.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

UndefinedClass

lib/private/Accounts/AccountManager.php:141:18: UndefinedClass: Class, interface or enum named OC\Accounts\IPhoneNumberUtil does not exist (see https://psalm.dev/019)

Check failure

Code scanning / Psalm

UndefinedClass Error

Class, interface or enum named OC\Accounts\IPhoneNumberUtil does not exist
if ($phoneNumber !== null) {
return $phoneNumber;
}

throw new InvalidArgumentException(self::PROPERTY_PHONE);
Expand Down

0 comments on commit 4f7dc69

Please sign in to comment.