From 65c1631d7daedf808e10d19224536b3d417b3d1a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 1 Sep 2026 11:54:33 +0200 Subject: [PATCH] fix(sip): Correct restrict SIP creation Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Joas Schilling --- lib/Controller/RoomController.php | 21 ++++++++ .../features/conversation-1/create.feature | 53 ++++++++++++++++++- .../conversation-5/sip-dialin.feature | 10 ++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 977e20596f9..1f7d9f22152 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -62,6 +62,7 @@ use OCA\Talk\RoomPresets\Announcement; use OCA\Talk\RoomPresets\Channel; use OCA\Talk\RoomPresets\Classified; +use OCA\Talk\RoomPresets\DefaultPreset; use OCA\Talk\RoomPresets\Forced; use OCA\Talk\RoomPresets\Parameter; use OCA\Talk\RoomPresets\VoiceRoom; @@ -167,6 +168,7 @@ public function __construct( private readonly IL10N $l, private readonly ThreadService $threadService, private readonly ConversationTagService $conversationTagService, + private readonly DefaultPreset $defaultParameters, private readonly Forced $forcedParameters, private readonly ?string $userId, ) { @@ -824,6 +826,25 @@ public function createRoom( $isClassified = $preset === Classified::getIdentifier(); + // Enabling SIP dial-in is restricted to the configured groups, so requesting + // it at creation time requires the same permission as toggling it later on. + // An administrator forced value is not user input, and classified + // conversations have SIP disabled during creation anyway. + if ($isClassified) { + $sipEnabled = Webinary::SIP_DISABLED; + } elseif ($sipEnabled !== Webinary::SIP_DISABLED + && $this->forcedParameters->getForcedParameter(Parameter::SIP_ENABLED) === null + && (!$this->talkConfig->isSIPConfigured() + || !$this->talkConfig->canUserEnableSIP($user))) { + if ($sipEnabled === $this->defaultParameters->getParameters()[Parameter::SIP_ENABLED->value]) { + // Clients send the administrator configured default value also when the + // user did not request SIP themselves, so it is silently disabled instead. + $sipEnabled = Webinary::SIP_DISABLED; + } else { + return new DataResponse(['error' => CreationException::REASON_SIP_ENABLED], Http::STATUS_FORBIDDEN); + } + } + $invitationList = $this->invitationService->validateInvitations($participants, $user, isClassified: $isClassified); if ($invitationList->hasInvalidInvitations() && !$invitationList->hasValidInvitations()) { // FIXME add the list of failed invitations? diff --git a/tests/integration/features/conversation-1/create.feature b/tests/integration/features/conversation-1/create.feature index 713046bc3c2..ba53224c2ae 100644 --- a/tests/integration/features/conversation-1/create.feature +++ b/tests/integration/features/conversation-1/create.feature @@ -63,10 +63,61 @@ Feature: conversation-1/create | room | 3 | 1 | 1 | GREATER_THAN_ZERO | Scenario: Enable SIP during creation - Given the following "spreed" app config is set + Given group "group1" exists + And user "participant1" is member of group "group1" + And the following "spreed" app config is set + | sip_bridge_dialin_info | +49-1234-567890 | + | sip_bridge_shared_secret | 1234567890abcdef | + | sip_bridge_groups | ["group1"] | + Given user "participant1" creates room "room" (v4) + | roomType | 3 | + | roomName | room | + | sipEnabled | 1 | + Then user "participant1" is participant of the following rooms (v4) + | id | type | participantType | sipEnabled | + | room | 3 | 1 | 1 | + + Scenario: Enable SIP during creation without being allowed to enable SIP + Given group "group1" exists + And the following "spreed" app config is set + | sip_bridge_dialin_info | +49-1234-567890 | + | sip_bridge_shared_secret | 1234567890abcdef | + | sip_bridge_groups | ["group1"] | + When user "participant1" creates room "room" with 403 (v4) + | roomType | 3 | + | roomName | room | + | sipEnabled | 1 | + And user "participant1" creates room "room" with 403 (v4) + | roomType | 3 | + | roomName | room | + | sipEnabled | 2 | + Then user "participant1" is participant of the following rooms (v4) + + Scenario: Send the default SIP state during creation without being allowed to enable SIP + Given group "group1" exists + And the following "spreed" app config is set | sip_bridge_dialin_info | +49-1234-567890 | | sip_bridge_shared_secret | 1234567890abcdef | | sip_bridge_groups | ["group1"] | + | default_sip_enabled | 1 | + # The default value is sent by clients without the user requesting SIP, + # so it is disabled instead of failing the creation + When user "participant1" creates room "room" (v4) + | roomType | 3 | + | roomName | room | + | sipEnabled | 1 | + And user "participant1" creates room "room2" with 403 (v4) + | roomType | 3 | + | roomName | room2 | + | sipEnabled | 2 | + Then user "participant1" is participant of the following rooms (v4) + | id | type | participantType | sipEnabled | + | room | 3 | 1 | 0 | + + Scenario: Enable SIP during creation while not being restricted by groups + Given the following "spreed" app config is set + | sip_bridge_dialin_info | +49-1234-567890 | + | sip_bridge_shared_secret | 1234567890abcdef | Given user "participant1" creates room "room" (v4) | roomType | 3 | | roomName | room | diff --git a/tests/integration/features/conversation-5/sip-dialin.feature b/tests/integration/features/conversation-5/sip-dialin.feature index a5f50a25202..8989744fe0d 100644 --- a/tests/integration/features/conversation-5/sip-dialin.feature +++ b/tests/integration/features/conversation-5/sip-dialin.feature @@ -93,3 +93,13 @@ Feature: conversation-5/sip-dialin | name | type | sipEnabled | | room 1 | 2 | 1 | | room 2 | 2 | 1 | + # The forced state is not user input, so it also applies to users that are + # not allowed to enable SIP themselves + Given user "participant2" exists + When user "participant2" creates room "room 3" (v4) + | roomType | 2 | + | roomName | room 3 | + | sipEnabled | 2 | + Then user "participant2" is participant of the following rooms (v4) + | name | type | sipEnabled | + | room 3 | 2 | 1 |