Skip to content

feat(sip): Allow the sipbridge to create a conversation with a user for a phone number #14731

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

Jump to bottom
Merged
nickvergessen merged 5 commits into from
Apr 30, 2025
Merged

feat(sip): Allow the sipbridge to create a conversation with a user for a phone number #14731

nickvergessen merged 5 commits into from
Apr 30, 2025

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Mar 27, 2025 •
edited
Loading

☑️ Resolves

🛠️ API Checklist

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • 📘 API documentation in docs/ has been updated or is not required
  • 🔖 Capability is added or not needed

@nickvergessen nickvergessen added this to the 🪺 Next Major (32) milestone Mar 27, 2025
@nickvergessen nickvergessen self-assigned this Mar 27, 2025
@nickvergessen nickvergessen force-pushed the feat/14418/sip-dialin-calling branch from 89d895b to 52483d9 Compare April 2, 2025 15:43
fancycode
fancycode reviewed Apr 10, 2025
View reviewed changes
@nickvergessen
Copy link
Member Author

nickvergessen commented Apr 14, 2025

/backport to stable31

@nickvergessen nickvergessen force-pushed the feat/14418/sip-dialin-calling branch from 52483d9 to f4bc5d9 Compare April 14, 2025 19:46
nickvergessen
nickvergessen commented Apr 15, 2025
View reviewed changes
nickvergessen
nickvergessen commented Apr 16, 2025
View reviewed changes
fancycode
fancycode reviewed Apr 17, 2025
View reviewed changes
fancycode
fancycode reviewed Apr 17, 2025
View reviewed changes
@fancycode
Copy link
Member

fancycode commented Apr 17, 2025

On a more general note: some customers might use internal phone extensions, i.e. not fully E.164 formatted. This will cause the validation to fail both here and in #14729. In such cases, numbers will be short.

I would suggest supporting these both in #14729 and here, i.e. everything that is all-numeric (with an optional leading +) will be considered to be a phone number. This then matches both short internal extensions and full E.164 numbers.

@fancycode
Copy link
Member

fancycode commented Apr 17, 2025

We probably need a dedicated response code for when the regular call workflow should be used, or should that always happen on 404?

@nickvergessen
Copy link
Member Author

nickvergessen commented Apr 22, 2025

or should that always happen on 404?

I'm not really sure I can distinguish a random number from a workflow one. So yeah I'd say 404 means there is no direct-dialin => normal workflow?

@nickvergessen nickvergessen force-pushed the feat/14418/sip-dialin-calling branch 4 times, most recently from 0d7cdf8 to b227252 Compare April 24, 2025 14:11
@nickvergessen
Copy link
Member Author

nickvergessen commented Apr 28, 2025 •
edited
Loading

  • Conversation is not marked as "In call" yet
  • Neither does the called user receive a notification

@fancycode fancycode changed the title feat(sip): All the sipbridge to create a conversation with a user for a phone number feat(sip): Allow the sipbridge to create a conversation with a user for a phone number Apr 28, 2025
@nickvergessen nickvergessen force-pushed the feat/14418/sip-dialin-calling branch 4 times, most recently from 35d8780 to a5eee09 Compare April 28, 2025 15:08
@nickvergessen
Copy link
Member Author

nickvergessen commented Apr 28, 2025 •
edited
Loading

/backport 2dad1f7 to stable30

@nickvergessen
Copy link
Member Author

nickvergessen commented Apr 28, 2025 •
edited
Loading

/backport 2dad1f7 to stable31.0

@fancycode
Copy link
Member

fancycode commented Apr 29, 2025

Web-UI tries to call the phone user when clicking the button to join the call.

POST-URL is https://myserver/ocs/v2.php/apps/spreed/api/v4/call/<token>/dialout/%7BattendeeId%7D.

fancycode
fancycode requested changes Apr 29, 2025
View reviewed changes
Copy link
Member

@fancycode fancycode left a comment

Choose a reason for hiding this comment

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

The triggered dial-out when joining from the web should be fixed imho, the display name is nice to have.

@nickvergessen
Copy link
Member Author

nickvergessen commented Apr 29, 2025

The triggered dial-out when joining from the web should be fixed imho, the display name is nice to have.

It was done from frontend, but fixed it myself

@nickvergessen
feat(sip): All the sipbridge to create a conversation with a user for…
98deea6 
… a phone number

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
chore(assets): Recompile assets
9967477 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
fix(sip): Fix SIP dial-in starting a call
2dad1f7 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
fix(call): Don't call the attendee again
e7291d0 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
fix(sip): First check if SIP is configured
da463a9 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the feat/14418/sip-dialin-calling branch from 9c68e0e to da463a9 Compare April 30, 2025 07:37
miaulalala
miaulalala approved these changes Apr 30, 2025
View reviewed changes
lib/Controller/RoomController.php
#[OpenAPI(scope: 'backend-sipbridge')]
public function directDialIn(string $phoneNumber, string $caller): DataResponse {
if (!$this->talkConfig->isSIPConfigured()) {
return new DataResponse(null, Http::STATUS_NOT_IMPLEMENTED);
Copy link
Contributor

@miaulalala miaulalala Apr 30, 2025

Choose a reason for hiding this comment

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

should we also throttle the "not implemented" response?

Copy link
Member Author

@nickvergessen nickvergessen Apr 30, 2025

Choose a reason for hiding this comment

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

you are not able to brute force the sip secret, so I don't think it's needed

@nickvergessen nickvergessen merged commit 74c26bd into main Apr 30, 2025
80 checks passed
@nickvergessen nickvergessen deleted the feat/14418/sip-dialin-calling branch April 30, 2025 13:25
This was referenced Apr 30, 2025
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miaulalala miaulalala miaulalala approved these changes

@fancycode fancycode Awaiting requested review from fancycode

@Antreesy Antreesy Awaiting requested review from Antreesy

Assignees

@nickvergessen nickvergessen

Labels

3. to review enhancement feature: api 🛠️ OCS API for conversations, chats and participants feature: SFU & SIP ☎️

Projects

None yet

Milestone

v22.0.0-beta.1

Development

Successfully merging this pull request may close these issues.

4 participants

@nickvergessen @fancycode @miaulalala