Skip to content

fix(sip): Handle direct incoming calls as phone participants #18567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nickvergessen wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(sip): Handle direct incoming calls as phone participants #18567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a339572
fix(sip): Handle direct incoming calls as phone participants
nickvergessen Jul 7, 2026
547c17a
fix(sip): Allow adding phone numbers with leading +
nickvergessen Jul 9, 2026
f8369ff
fix(sip): Disable setupcheck complaining about leading +
nickvergessen Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion lib/Controller/RoomController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2507,6 +2507,7 @@ public function directDialIn(string $phoneNumber, string $caller): DataResponse

$caller = trim($caller);
// TODO: Use later and get name from addressbook? $cleanedCaller = $this->phoneNumberUtil->convertToStandardFormat($caller);
$inBoundPhoneNumber = $this->phoneNumberUtil->convertToStandardFormat($caller);

@SystemKeeper SystemKeeper Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns null in my case. In this method $caller is 4912345 instead of +4912345. If the + is added in front, it is correctly recognized. (or alternatively the default region is set, but it should also work without,

spreed/lib/Service/PhoneNumberValidation.php

Lines 42 to 43 in 8f9dd1d

$defaultRegion = $this->config->getSystemValueString('default_phone_region') ?: null;
$standardPhoneNumber = $this->phoneNumberUtil->convertToStandardFormat($phoneNumber, $defaultRegion);
)

Since the + is missing, we add the phone attendee as 4912345. That means when calling back we dial +494912345 (which is then - depending on the sip config - converted to 00494912345).

@SystemKeeper SystemKeeper Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fancycode I guess it is expected that the sipbridge calls the direct-call-in endpoint without the + in front of the number? Would it be safe to add it on our side to have a correct converting?

@nickvergessen nickvergessen Jul 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a setting for the dial-out prefix:
grafik

Not sure if it makes sense to use that?

@fancycode fancycode Jul 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fancycode I guess it is expected that the sipbridge calls the direct-call-in endpoint without the + in front of the number?

Yes, the numbers received by the SIP bridge in (To / From) might be an extension only in case of a on-premise PBX and internal calls, so it can't just prefix it with +.

Would it be safe to add it on our side to have a correct converting?

Similar to the SIP bridge you also can't add the + unconditionally.

$user = $this->userManager->get($entity->getActorId());
try {
$room = $this->roomService->createConversation(
Expand All @@ -2522,7 +2523,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: $inBoundPhoneNumber,
);
return new DataResponse($this->formatRoom($room, $participant));
}

Expand Down
6 changes: 5 additions & 1 deletion lib/Service/ParticipantService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,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);

Expand Down Expand Up @@ -531,6 +531,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);

Expand Down
10 changes: 2 additions & 8 deletions lib/Service/PhoneNumberValidation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ 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 {
Expand All @@ -46,10 +44,6 @@ public function validateNumber(string $phoneNumber): string {
throw new \InvalidArgumentException();
}

if (str_starts_with($standardPhoneNumber, '+')) {
return substr($standardPhoneNumber, 1);
}

throw new \InvalidArgumentException();
return $standardPhoneNumber;
}
}
8 changes: 5 additions & 3 deletions lib/SetupCheck/SIPConfiguration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ public function run(): SetupResult {
$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(
// TODO: Temporary disabled while we migrate to leading `+` for international numbers
// TODO: Either to be deleted later or should complain for numbers without it
// $this->connection->escapeLikeParameter('+') . '%'
// )))
// ->orWhere($query->expr()->like('phone_number', $query->createNamedParameter(
$this->connection->escapeLikeParameter('0') . '%'
)));

Expand Down
Loading