Repository navigation
feat: let the mobile app make and answer calls - #1312
rohitsasi720 wants to merge 1 commit into
Conversation
The signed /api/v1 gets the web's one-to-one call signalling, so an app can ring someone, answer, hang up and fetch relay credentials, and calls work between the app and the web either way round. The endpoints mirror the web routes and reuse their actions, query and TURN provider, with the same rules, validation and throttles. The incoming call sends its caller as a UserResource. Org calls stay web-only for now.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Call request authorization and validation app/Http/Requests/CreateApiConversationCallRequest.php, app/Http/Requests/ShowApiConversationCallRequest.php, app/Http/Requests/UpdateApiConversationCallRequest.php, app/Http/Requests/DeleteApiConversationCallRequest.php, app/Http/Requests/ListApiConversationCallIceServersRequest.php |
Request classes check messaging permission, call eligibility, conversation availability, and TURN availability. They validate SDP or unanswered-call status where required. |
Call status and signaling endpoints app/Http/Controllers/Api/CallController.php, app/Http/Controllers/Api/ConversationCallController.php, routes/api.php, tests/Feature/Controllers/Api/CallControllerTest.php, tests/Feature/Controllers/Api/ConversationCallControllerTest.php |
Controllers return call status and handle conversation call creation, updates, and termination. Routes apply throttling and posting-timeout middleware. Feature tests cover call flows, rejection cases, and authentication. |
ICE-server endpoint app/Http/Controllers/Api/ConversationCallIceServerController.php, tests/Feature/Controllers/Api/ConversationCallIceServerControllerTest.php |
The endpoint returns TURN credentials and relay-only status. Tests cover successful responses, missing conversations, and unavailable TURN configuration. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant Client as API client
participant Routes as API routes
participant Request as CreateApiConversationCallRequest
participant Controller as ConversationCallController
participant Action as CreateConversationCall
Client->>Routes: Submit call request
Routes->>Request: Authorize and validate request
Request-->>Routes: Valid request
Routes->>Controller: Invoke store
Controller->>Action: Pass submitted SDP
Action-->>Controller: Complete call creation
Controller-->>Client: Return no content
Possibly related issues
- dev-null-sh/dev-null.sh#857 — This PR adds call-signaling endpoints to
/api/v1, extending the API surface that the issue says should be documented.
Possibly related PRs
- dev-null-sh/dev-null.sh#1179 — The call responses use
UserResource, whose identity contract this PR changes. - dev-null-sh/dev-null.sh#1156 — The call responses use
UserResourceand therefore depend on its user identity behavior. - dev-null-sh/dev-null.sh#1274 — The call responses use
UserResource, whose JSON shape this PR changes.
Suggested reviewers: mnapoli, xheaven
Merge Risk: 
9e091
-
The call API changes are mergeable after normal checks; no actionable defect was established in the reviewed paths.
Security architecture risk: 
9e091
-
The new endpoints preserve the existing participant permissions, signaling validation, throttling, and relay-credential checks. No newly introduced authorization bypass was established. Concurrent call transitions retain existing limitations, and deployment-wide behavior was not fully verified.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The added authority is available to approved-device principals acting as their owning users. Sensitive outcomes include reading participant SDP, changing shared call signaling, and obtaining provider-issued relay credentials. Credential issuance is gated by an eligible conversation, but the controller does not establish that issued credentials are technically bound to that partner.
Trust Boundaries and Controls
- observed — The new routes use required signed authentication, not the optional guest mode. Authentication verifies the signature, requires an approved Device, rejects banned users for these routes, and installs the device owner's User identity before downstream authorization.
- observed — API request gates preserve the web call permissions and validation. Answering requires the route partner to be the incoming caller; offer and answer SDP are required strings beginning with v=0 and limited to 10,000 characters. Routes retain named conversation or message throttles, with posting-timeout prevention on creation.
Resilience and Maintainability Implications
- observed — Encrypted cache lifetimes bound signaling retention. Repeated sequential deletion becomes a no-op after matching state is gone, and sequential answer retries fail once incoming lookup excludes the answered call. Cache expiry itself does not execute the explicit terminal-event path; this is inherited behavior rather than a new API recovery guarantee.
Hardening Proposals
- proposed — If stronger cross-client lifecycle guarantees are required, add call-instance identity and atomic ownership/version checks to shared transitions, with a defined event-idempotency and recovery policy. This would harden inherited concurrency behavior, not remediate an established PR-introduced vulnerability.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "feat: let the mobile app make and answer calls" matches the main change. The PR adds one-to-one call-signalling API endpoints under /api/v1 so the mobile app can place and answer calls, cov… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "feat: let the mobile app make and answer calls" matches the main change. The PR adds one-to-one call-signalling API endpoints under /api/v1 so the mobile app can place and answer calls, covering incoming-call lookup, conversation call create/show/update/delete, and ICE-server retrieval. The title is short, specific, and follows the conventional-commit style. It does not mention the organization-call exclusion or the test coverage, which is expected for a title.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 7 functions across 12 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
@xHeaven @nunomaduro can we get this one in for the app too? |
Summary
One-to-one Remote calls only work in the browser today: their signalling lives on session routes in
routes/web.php, which the mobile app can't use. This adds the same signalling to the signed/api/v1, so the app can ring someone, answer, hang up and get relay credentials, and calls work between the app and the web in both directions.Changes
All six routes are in the
AuthenticateSignedRequestgroup and mirror the web ones:GET remotes/callapi-call.show(the call ringing for you)GET remotes/@{partner}/callapi-conversation-call.showPOST remotes/@{partner}/callapi-conversation-call.storePUT remotes/@{partner}/callapi-conversation-call.updateDELETE remotes/@{partner}/callapi-conversation-call.destroyGET remotes/@{partner}/call/ice-serversapi-conversation-call-ice-server.indexCreateConversationCall,UpdateConversationCall,DeleteConversationCall,CallQueryandTurnProvider, with the same throttles andPreventPostingInTimeoutas the web routes.Controllers/Api, and an*Api*form request per endpoint that copies the web request's checks (asCreateApiConversationPingRequestdoes).GET remotes/callreturns the caller as aUserResourcerather than the raw model.Validation
Local, on the PR head:
tests/Feature/Controllers/Api/(CallControllerTest,ConversationCallControllerTest,ConversationCallIceServerControllerTest): 26 passed. They include calls placed on the web and answered through the API, and the other way round.pint --test,rector --dry-run,phpstan,pest --type-coverage --min=100andtests/Unit/ArchTest.phpall pass.pest --parallel --ci --coverage --exactly=100.0): 4980/4980 tests passed (before the last test was added). The coverage gate fails at 98.3%. Every file under 100% is existing call and org-call code from 92fb6b5 and acd211a, which shipped without tests (for exampleOrgCallQuery,CreateOrgCall, the webConversationCallController). None of the files in this PR are under 100%, and the gate fails the same way onproduction.fpcode on both endsIndependent review
A separate reviewer agent read the change against
acd211ae(no tests run) and gave PASS, with no blockers. It confirmed the authorization matches the web request for request, the routes don't clash, nothing leaks beyond what the web sends, and every branch of the new files is tested. Afterwards I added a test for the caller having Remotes turned off, which was one of its nits. It changes tests only.Limitations
Summary by CodeRabbit