diff --git a/appinfo/info.xml b/appinfo/info.xml index 82f62a09401..7084bde6a58 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -18,7 +18,7 @@ * 🌉 **Sync with other chat solutions** With [Matterbridge](https://github.com/42wim/matterbridge/) being integrated in Talk, you can easily sync a lot of other chat solutions to Nextcloud Talk and vice-versa. ]]> - 26.0.0-dev.15 + 26.0.0-dev.16 AGPL-3.0-or-later Christian Lorang diff --git a/docs/settings.md b/docs/settings.md index 9520dfaa1f7..1ca4b6f7917 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -84,7 +84,7 @@ Legend: | `sip_bridge_dialin_info` | string | | Yes | 🖌️ | Additional information added in the SIP dial-in invitation mail and sidebar | | `sip_bridge_dialout_anonymous` | bool | `false` | No | | When set to true, all out-going SIP calls will be anonymous calls (CLIR) | | `sip_bridge_dialout_number` | string | | No | | Fallback phone number that is used as caller for out-going calls when the user has no phone-number configured (has to be E164 `+49123456789` formatted) | -| `sip_bridge_dialout_prefix` | string | `+` | No | | Prefix that is added before the first configured phone-number of a user to make it a valid E164 number to be used as a caller for out-going calls | +| `sip_bridge_dialout_prefix` | string | `+` | No | | Deprecated: Only used for direct dial-in from older SIP bridges, which send phone numbers without this prefix | | `sip_bridge_shared_secret` | string | | Yes | 🖌️ | Shared secret allowing the SIP bridge to authenticate on the Nextcloud server | | `signaling_ticket_secret` | string | | Yes | | Secret used to secure the signaling tickets for guests (255 character random string) | | `signaling_token_alg` | string
`ES256`, `ES384`, `RS256`, `RS384`, `RS512` or `EdDSA` | `ES256` | Yes | | Algorithm for the signaling tickets | diff --git a/lib/Command/PhoneNumber/AddPhoneNumber.php b/lib/Command/PhoneNumber/AddPhoneNumber.php index 632fd6602a9..4da3893545c 100644 --- a/lib/Command/PhoneNumber/AddPhoneNumber.php +++ b/lib/Command/PhoneNumber/AddPhoneNumber.php @@ -66,8 +66,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $userId = $user->getUID(); + $phoneNumber = $this->phoneNumberValidation->cleanNumber($phoneNumber); + + if ($phoneNumber === '') { + $output->writeln('Phone number is invalid.'); + return self::FAILURE; + } + try { - $phoneNumber = $this->phoneNumberValidation->validateNumber($phoneNumber); + $this->phoneNumberValidation->validateNumber($phoneNumber); } catch (\InvalidArgumentException) { $output->writeln('Not a valid phone number ' . $phoneNumber . '. The format is invalid.'); return self::FAILURE; diff --git a/lib/Command/PhoneNumber/FindPhoneNumber.php b/lib/Command/PhoneNumber/FindPhoneNumber.php index 1922a7f79dc..05571179ee9 100644 --- a/lib/Command/PhoneNumber/FindPhoneNumber.php +++ b/lib/Command/PhoneNumber/FindPhoneNumber.php @@ -10,6 +10,7 @@ use OC\Core\Command\Base; use OCA\Talk\Model\PhoneNumberMapper; +use OCA\Talk\Service\PhoneNumberValidation; use OCP\AppFramework\Db\DoesNotExistException; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -19,6 +20,7 @@ class FindPhoneNumber extends Base { public function __construct( private readonly PhoneNumberMapper $mapper, + private readonly PhoneNumberValidation $phoneNumberValidation, ) { parent::__construct(); } @@ -48,6 +50,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int $userId = (string)$input->getOption('user'); if ($phoneNumber !== '') { + $phoneNumber = $this->phoneNumberValidation->cleanNumber($phoneNumber); + try { $entry = $this->mapper->findByPhoneNumber($phoneNumber); } catch (DoesNotExistException) { diff --git a/lib/Command/PhoneNumber/ImportPhoneNumbers.php b/lib/Command/PhoneNumber/ImportPhoneNumbers.php index 9914d61e400..9ae68fae4f9 100644 --- a/lib/Command/PhoneNumber/ImportPhoneNumbers.php +++ b/lib/Command/PhoneNumber/ImportPhoneNumbers.php @@ -72,8 +72,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int continue; } + $row[0] = $this->phoneNumberValidation->cleanNumber($row[0]); + try { - $row[0] = $this->phoneNumberValidation->validateNumber($row[0]); + $this->phoneNumberValidation->validateNumber($row[0]); } catch (\InvalidArgumentException) { $output->writeln('Not a valid phone number ' . $row[0] . '. The format is invalid.'); return self::FAILURE; diff --git a/lib/Command/PhoneNumber/RemovePhoneNumber.php b/lib/Command/PhoneNumber/RemovePhoneNumber.php index 0ae6132ac5d..77a77d4fdb8 100644 --- a/lib/Command/PhoneNumber/RemovePhoneNumber.php +++ b/lib/Command/PhoneNumber/RemovePhoneNumber.php @@ -10,6 +10,7 @@ use OC\Core\Command\Base; use OCA\Talk\Model\PhoneNumberMapper; +use OCA\Talk\Service\PhoneNumberValidation; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -18,6 +19,7 @@ class RemovePhoneNumber extends Base { public function __construct( private readonly PhoneNumberMapper $mapper, + private readonly PhoneNumberValidation $phoneNumberValidation, ) { parent::__construct(); } @@ -36,7 +38,13 @@ protected function configure(): void { } protected function execute(InputInterface $input, OutputInterface $output): int { - $phoneNumber = $input->getArgument('phone'); + $phoneNumber = $this->phoneNumberValidation->cleanNumber($input->getArgument('phone')); + + if ($phoneNumber === '') { + $output->writeln('Phone number is invalid.'); + return self::FAILURE; + } + $this->mapper->deleteByPhoneNumber($phoneNumber); return self::SUCCESS; } diff --git a/lib/Controller/CallController.php b/lib/Controller/CallController.php index 3b5df5713ee..2ae3c405128 100644 --- a/lib/Controller/CallController.php +++ b/lib/Controller/CallController.php @@ -454,14 +454,12 @@ public function sipDialOut(int $attendeeId): DataResponse { } // No elseif, so we have the fallback to sip_bridge_dialout_number when the caller is no user or doesn't have a number - if ($callerNumber !== false && $this->appConfig->getAppValueString('sip_bridge_dialout_prefix', '+') !== '') { + if ($callerNumber !== false) { $attendee = $this->participant->getAttendee(); if ($attendee->getActorType() === Attendee::ACTOR_USERS) { $numbers = $this->phoneNumberMapper->findByUser($attendee->getActorId()); if (!empty($numbers)) { - $number = array_shift($numbers); - $callerNumber = $this->appConfig->getAppValueString('sip_bridge_dialout_prefix', '+'); - $callerNumber .= $number->getPhoneNumber(); + $callerNumber = array_shift($numbers)->getPhoneNumber(); } } } diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index abfb125e492..a882eb631b5 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -2663,12 +2663,26 @@ public function directDialIn(string $phoneNumber, string $caller): DataResponse try { $entity = $this->phoneService->getAccountToCallForPhoneNumber($phoneNumber); } catch (DoesNotExistException) { - $this->logger->info('No account found for direct dial-in with number: ' . $phoneNumber); - return new DataResponse(null, Http::STATUS_NOT_FOUND); + $dialoutPrefix = $this->appConfig->getAppValueString('sip_bridge_dialout_prefix', '+'); + + if ($dialoutPrefix === '') { + $this->logger->info('No account found for direct dial-in with number: ' . $phoneNumber); + return new DataResponse(null, Http::STATUS_NOT_FOUND); + } + + try { + // Try to find the number with added dialout prefix as a fallback for old sip-bridges + // TODO: Log as error in 26, remove fallback and dialout prefix with 27 + $entity = $this->phoneService->getAccountToCallForPhoneNumber($dialoutPrefix . $phoneNumber); + } catch (DoesNotExistException) { + $this->logger->info('No account found for direct dial-in with number: ' . $phoneNumber); + return new DataResponse(null, Http::STATUS_NOT_FOUND); + } + + $this->logger->warning('Found account for direct dial-in only by adding the dial-out prefix. Please adjust the mapping for number: ' . $phoneNumber); } $caller = trim($caller); - // TODO: Use later and get name from addressbook? $cleanedCaller = $this->phoneNumberUtil->convertToStandardFormat($caller); $user = $this->userManager->get($entity->getActorId()); try { $room = $this->roomService->createConversation( @@ -2684,7 +2698,14 @@ public function directDialIn(string $phoneNumber, string $caller): DataResponse return new DataResponse(null, Http::STATUS_INTERNAL_SERVER_ERROR); } - $participant = $this->participantService->joinRoomAsNewGuest($this->roomService, $room, '', true, displayName: $caller); + $participant = $this->participantService->joinRoomAsNewGuest( + $this->roomService, + $room, + '', + true, + displayName: $caller, + phoneNumber: $caller, + ); return new DataResponse($this->formatRoom($room, $participant)); } diff --git a/lib/Migration/Version23001Date20260918142246.php b/lib/Migration/Version23001Date20260918142246.php new file mode 100644 index 00000000000..6cbca6118d0 --- /dev/null +++ b/lib/Migration/Version23001Date20260918142246.php @@ -0,0 +1,79 @@ +appConfig->getAppValueString('sip_bridge_dialout_prefix', '+'); + + if ($dialoutPrefix === '') { + return; + } + + $query = $this->connection->getQueryBuilder(); + $query->select('id', 'phone_number') + ->from('talk_phone_numbers'); + + $phoneNumbers = []; + $result = $query->executeQuery(); + while ($row = $result->fetchAssociative()) { + $phoneNumbers[(int)$row['id']] = (string)$row['phone_number']; + } + $result->closeCursor(); + + $update = $this->connection->getQueryBuilder(); + $update->update('talk_phone_numbers') + ->set('phone_number', $update->createParameter('phone_number')) + ->where($update->expr()->eq('id', $update->createParameter('id'))); + + foreach ($phoneNumbers as $id => $phoneNumber) { + if (strlen($phoneNumber) < 6 + // Numbers added before the leading + was stripped already have it + || str_starts_with($phoneNumber, '+') + || str_starts_with($phoneNumber, $dialoutPrefix)) { + continue; + } + + $update->setParameter('phone_number', $dialoutPrefix . $phoneNumber) + ->setParameter('id', $id, IQueryBuilder::PARAM_INT); + try { + $update->executeStatement(); + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } + $output->warning('Phone number ' . $phoneNumber . ' was not migrated, because ' . $dialoutPrefix . $phoneNumber . ' is already assigned'); + } + } + } +} diff --git a/lib/Service/ParticipantService.php b/lib/Service/ParticipantService.php index 0ad0fb2aa1b..713c0a01059 100644 --- a/lib/Service/ParticipantService.php +++ b/lib/Service/ParticipantService.php @@ -621,7 +621,7 @@ public function joinRoomAsFederatedUser(Room $room, string $actorType, string $a * @throws InvalidPasswordException * @throws UnauthorizedException */ - public function joinRoomAsNewGuest(RoomService $roomService, Room $room, string $password, bool $passedPasswordProtection = false, ?Participant $previousParticipant = null, ?string $displayName = null): Participant { + public function joinRoomAsNewGuest(RoomService $roomService, Room $room, string $password, bool $passedPasswordProtection = false, ?Participant $previousParticipant = null, ?string $displayName = null, ?string $phoneNumber = null): Participant { $event = new BeforeGuestJoinedRoomEvent($room, $password, $passedPasswordProtection); $this->dispatcher->dispatchTyped($event); @@ -655,6 +655,10 @@ public function joinRoomAsNewGuest(RoomService $roomService, Room $room, string if ($displayName !== null && $displayName !== '') { $attendee->setDisplayName($displayName); } + if ($phoneNumber !== null && $phoneNumber !== '') { + $attendee->setPhoneNumber($phoneNumber); + $attendee->setActorType(Attendee::ACTOR_PHONES); + } $this->attendeeMapper->insert($attendee); diff --git a/lib/Service/PhoneNumberValidation.php b/lib/Service/PhoneNumberValidation.php index c1ce47af600..6b5dcfd5a3a 100644 --- a/lib/Service/PhoneNumberValidation.php +++ b/lib/Service/PhoneNumberValidation.php @@ -23,20 +23,17 @@ public function __construct( * Validate input as a phone number * * - Local number: allow - * - International number - * a. If valid, strip + and allow - * b. If invalid, throw + * - International number: If invalid, throw * @throws \InvalidArgumentException When the number is invalid */ - public function validateNumber(string $phoneNumber): string { - + public function validateNumber(string $phoneNumber): void { if ( // Not an internation number !str_starts_with($phoneNumber, '0') // And matches a local number or dial-through && preg_match('/^[0-9]{1,20}$/', $phoneNumber) ) { - return $phoneNumber; + return; } $defaultRegion = $this->config->getSystemValueString('default_phone_region') ?: null; @@ -45,11 +42,14 @@ public function validateNumber(string $phoneNumber): string { if ($standardPhoneNumber === null) { throw new \InvalidArgumentException(); } + } - if (str_starts_with($standardPhoneNumber, '+')) { - return substr($standardPhoneNumber, 1); - } - - throw new \InvalidArgumentException(); + /** + * Removes any character that is not a number or "+" + * + * @param string $phoneNumber Phonenumber to clean + */ + public function cleanNumber(string $phoneNumber): string { + return preg_replace('/[^0-9+]/', '', $phoneNumber) ?? ''; } } diff --git a/lib/SetupCheck/SIPConfiguration.php b/lib/SetupCheck/SIPConfiguration.php index 42eec9b9288..21a06c460a5 100644 --- a/lib/SetupCheck/SIPConfiguration.php +++ b/lib/SetupCheck/SIPConfiguration.php @@ -10,7 +10,6 @@ use OCA\Talk\Config; use OCP\AppFramework\Services\IAppConfig; -use OCP\IDBConnection; use OCP\IL10N; use OCP\SetupCheck\ISetupCheck; use OCP\SetupCheck\SetupResult; @@ -19,7 +18,6 @@ class SIPConfiguration implements ISetupCheck { public function __construct( private readonly IAppConfig $appConfig, private readonly Config $talkConfig, - private readonly IDBConnection $connection, private readonly IL10N $l, ) { } @@ -44,30 +42,6 @@ public function run(): SetupResult { return SetupResult::success($this->l->t('Using the SIP functionality requires a High-performance backend.')); } - $query = $this->connection->getQueryBuilder(); - $query->select('phone_number') - ->from('talk_phone_numbers') - ->where($query->expr()->like('phone_number', $query->createNamedParameter( - $this->connection->escapeLikeParameter('+') . '%' - ))) - ->orWhere($query->expr()->like('phone_number', $query->createNamedParameter( - $this->connection->escapeLikeParameter('0') . '%' - ))); - - $result = $query->executeQuery(); - $invalidNumbers = $result->fetchFirstColumn(); - $result->closeCursor(); - - if (!empty($invalidNumbers)) { - $message = $this->l->t("Assigned Talk phone numbers must not start with + or 0. Please remove or update the following numbers:\n{list}"); - $message = str_replace( - '{list}', - implode("\n", $invalidNumbers), - $message - ); - return SetupResult::error($message, 'https://portal.nextcloud.com/article/Nextcloud-Talk/Nextcloud-Talk-Phone/Direct-Dial-in#content-provisioning'); - } - if ($this->talkConfig->getSIPSharedSecret() === '' && $this->talkConfig->getDialInInfo() === '' && $this->appConfig->getAppValueInt(Config::FEATURE_HINTS_HIDDEN) < 34) { diff --git a/src/components/AdminSettings/SIPBridge.vue b/src/components/AdminSettings/SIPBridge.vue index 0bb3621b044..de6ee533181 100644 --- a/src/components/AdminSettings/SIPBridge.vue +++ b/src/components/AdminSettings/SIPBridge.vue @@ -63,7 +63,7 @@ :disabled="loading" labelOutside />

- {{ t('spreed', 'Prefix to configured user number for outgoing calls (default is `+`)') }} + {{ t('spreed', 'Deprecated: Only used for direct dial-in from older SIP bridges, which send phone numbers without this prefix (default is `+`)') }}

diff --git a/tests/integration/features/command/phone-number.feature b/tests/integration/features/command/phone-number.feature index 432e1ae2904..a73d24fd8b1 100644 --- a/tests/integration/features/command/phone-number.feature +++ b/tests/integration/features/command/phone-number.feature @@ -6,7 +6,7 @@ Feature: command/phone-number Given user "participant2" exists Given invoking occ with "talk:phone-number:add abc participant1" Then the command failed with exit code 1 - And the command output contains the text "Not a valid phone number abc. The format is invalid." + And the command output contains the text "Phone number is invalid." Given invoking occ with "talk:phone-number:add +49-160-123-12-12 participant3" Then the command failed with exit code 1 @@ -14,19 +14,23 @@ Feature: command/phone-number Given invoking occ with "talk:phone-number:add +49-160-123-12-12 participant1" Then the command failed with exit code 0 - And the command output contains the text "Phone number 491601231212 is now assigned to participant1" + And the command output contains the text "Phone number +491601231212 is now assigned to participant1" - Given invoking occ with "talk:phone-number:find --phone 491601231212" + Given invoking occ with "talk:phone-number:find --phone +491601231212" Then the command failed with exit code 0 - And the command output contains the text "Phone number 491601231212 is assigned to participant1" + And the command output contains the text "Phone number +491601231212 is assigned to participant1" - Given invoking occ with "talk:phone-number:find --phone 49160123121234" + Given invoking occ with "talk:phone-number:find --phone +49-160-123-12-12" + Then the command failed with exit code 0 + And the command output contains the text "Phone number +491601231212 is assigned to participant1" + + Given invoking occ with "talk:phone-number:find --phone +49160123121234" Then the command failed with exit code 1 - And the command output contains the text "Phone number 49160123121234 could not be found" + And the command output contains the text "Phone number +49160123121234 could not be found" Given invoking occ with "talk:phone-number:find --user participant1" Then the command failed with exit code 0 - And the command output contains the text "participant1 has phone number 491601231212 assigned" + And the command output contains the text "participant1 has phone number +491601231212 assigned" Given invoking occ with "talk:phone-number:find --user participant2" Then the command failed with exit code 1 @@ -34,13 +38,13 @@ Feature: command/phone-number Given invoking occ with "talk:phone-number:add +49-160-123-1213 participant1" Then the command failed with exit code 0 - And the command output contains the text "Phone number 491601231213 is now assigned to participant1" + And the command output contains the text "Phone number +491601231213 is now assigned to participant1" Given invoking occ with "talk:phone-number:find --user participant1" Then the command failed with exit code 0 And the command output contains the text "participant1 has the following phone numbers assigned:" - And the command output contains the text "- 491601231212" - And the command output contains the text "- 491601231213" + And the command output contains the text "- +491601231212" + And the command output contains the text "- +491601231213" Given invoking occ with "talk:phone-number:add +49-160-123-1212 participant2" Then the command failed with exit code 1 @@ -48,7 +52,7 @@ Feature: command/phone-number Given invoking occ with "talk:phone-number:add --force '+49-160-123-12-12' participant2" Then the command failed with exit code 0 - And the command output contains the text "Phone number 491601231212 is now assigned to participant2" + And the command output contains the text "Phone number +491601231212 is now assigned to participant2" And the command output contains the text "Was assigned to participant1" Given invoking occ with "talk:phone-number:add 23 participant2" @@ -58,14 +62,14 @@ Feature: command/phone-number Given invoking occ with "talk:phone-number:find --user participant2" Then the command failed with exit code 0 And the command output contains the text "participant2 has the following phone numbers assigned:" - And the command output contains the text "- 491601231212" + And the command output contains the text "- +491601231212" And the command output contains the text "- 23" Given invoking occ with "talk:phone-number:find --user participant1" Then the command failed with exit code 0 - And the command output contains the text "participant1 has phone number 491601231213 assigned" + And the command output contains the text "participant1 has phone number +491601231213 assigned" - Given invoking occ with "talk:phone-number:remove 491601231213" + Given invoking occ with "talk:phone-number:remove +49-160-123-1213" Then the command failed with exit code 0 Given invoking occ with "talk:phone-number:find --user participant1" @@ -89,19 +93,19 @@ Feature: command/phone-number # Valid German number Given invoking occ with "talk:phone-number:add 004971112347 participant1" Then the command failed with exit code 0 - And the command output contains the text "Phone number 4971112347 is now assigned to participant1" + And the command output contains the text "Phone number 004971112347 is now assigned to participant1" Given invoking occ with "talk:phone-number:add +4971112346 participant1" Then the command failed with exit code 0 - And the command output contains the text "Phone number 4971112346 is now assigned to participant1" + And the command output contains the text "Phone number +4971112346 is now assigned to participant1" - Given invoking occ with "talk:phone-number:add 4971112345 participant1" + Given invoking occ with "talk:phone-number:add +4971112345 participant1" Then the command failed with exit code 0 - And the command output contains the text "Phone number 4971112345 is now assigned to participant1" + And the command output contains the text "Phone number +4971112345 is now assigned to participant1" Given invoking occ with "talk:phone-number:add 071112348 participant1" Then the command failed with exit code 0 - And the command output contains the text "Phone number 4971112348 is now assigned to participant1" + And the command output contains the text "Phone number 071112348 is now assigned to participant1" # 01122 is not a valid prefix in Germany Given invoking occ with "talk:phone-number:add 011223344 participant1" @@ -126,4 +130,4 @@ Feature: command/phone-number # Valid US number Given invoking occ with "talk:phone-number:add 00112345678901 participant1" Then the command failed with exit code 0 - And the command output contains the text "Phone number 12345678901 is now assigned to participant1" + And the command output contains the text "Phone number 00112345678901 is now assigned to participant1" diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index d82d5f9fd39..5a96c5e913b 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -36,6 +36,7 @@ + @@ -60,6 +61,7 @@ +