Skip to content

Commit f8f4d6f

Browse files
nickvergessennickvergessen
authored
Merge pull request #19242 from nextcloud/backport/19235/stable34
[stable34] fix(sip): Correct restrict SIP creation
2 parents 5ab5152 + 5fd6430 commit f8f4d6f

3 files changed

Lines changed: 81 additions & 1 deletion

File tree

‎lib/Controller/RoomController.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
use OCA\Talk\ResponseDefinitions;
5959
use OCA\Talk\Room;
6060
use OCA\Talk\RoomAttributes;
61+
use OCA\Talk\RoomPresets\DefaultPreset;
6162
use OCA\Talk\RoomPresets\Forced;
6263
use OCA\Talk\RoomPresets\Parameter;
6364
use OCA\Talk\RoomPresets\VoiceRoom;
@@ -163,6 +164,7 @@ public function __construct(
163164
private readonly IL10N $l,
164165
private readonly ThreadService $threadService,
165166
private readonly ConversationTagService $conversationTagService,
167+
private readonly DefaultPreset $defaultParameters,
166168
private readonly Forced $forcedParameters,
167169
private readonly ?string $userId,
168170
) {
@@ -818,6 +820,23 @@ public function createRoom(
818820
$password = '';
819821
}
820822

823+
// Enabling SIP dial-in is restricted to the configured groups, so requesting
824+
// it at creation time requires the same permission as toggling it later on.
825+
// An administrator forced value is not user input, and classified
826+
// conversations have SIP disabled during creation anyway.
827+
if ($sipEnabled !== Webinary::SIP_DISABLED
828+
&& $this->forcedParameters->getForcedParameter(Parameter::SIP_ENABLED) === null
829+
&& (!$this->talkConfig->isSIPConfigured()
830+
|| !$this->talkConfig->canUserEnableSIP($user))) {
831+
if ($sipEnabled === $this->defaultParameters->getParameters()[Parameter::SIP_ENABLED->value]) {
832+
// Clients send the administrator configured default value also when the
833+
// user did not request SIP themselves, so it is silently disabled instead.
834+
$sipEnabled = Webinary::SIP_DISABLED;
835+
} else {
836+
return new DataResponse(['error' => CreationException::REASON_SIP_ENABLED], Http::STATUS_FORBIDDEN);
837+
}
838+
}
839+
821840
$invitationList = $this->invitationService->validateInvitations($participants, $user);
822841
if ($invitationList->hasInvalidInvitations() && !$invitationList->hasValidInvitations()) {
823842
// FIXME add the list of failed invitations?

‎tests/integration/features/conversation-1/create.feature‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,61 @@ Feature: conversation-1/create
6363
| room | 3 | 1 | 1 | GREATER_THAN_ZERO |
6464

6565
Scenario: Enable SIP during creation
66-
Given the following "spreed" app config is set
66+
Given group "group1" exists
67+
And user "participant1" is member of group "group1"
68+
And the following "spreed" app config is set
69+
| sip_bridge_dialin_info | +49-1234-567890 |
70+
| sip_bridge_shared_secret | 1234567890abcdef |
71+
| sip_bridge_groups | ["group1"] |
72+
Given user "participant1" creates room "room" (v4)
73+
| roomType | 3 |
74+
| roomName | room |
75+
| sipEnabled | 1 |
76+
Then user "participant1" is participant of the following rooms (v4)
77+
| id | type | participantType | sipEnabled |
78+
| room | 3 | 1 | 1 |
79+
80+
Scenario: Enable SIP during creation without being allowed to enable SIP
81+
Given group "group1" exists
82+
And the following "spreed" app config is set
83+
| sip_bridge_dialin_info | +49-1234-567890 |
84+
| sip_bridge_shared_secret | 1234567890abcdef |
85+
| sip_bridge_groups | ["group1"] |
86+
When user "participant1" creates room "room" with 403 (v4)
87+
| roomType | 3 |
88+
| roomName | room |
89+
| sipEnabled | 1 |
90+
And user "participant1" creates room "room" with 403 (v4)
91+
| roomType | 3 |
92+
| roomName | room |
93+
| sipEnabled | 2 |
94+
Then user "participant1" is participant of the following rooms (v4)
95+
96+
Scenario: Send the default SIP state during creation without being allowed to enable SIP
97+
Given group "group1" exists
98+
And the following "spreed" app config is set
6799
| sip_bridge_dialin_info | +49-1234-567890 |
68100
| sip_bridge_shared_secret | 1234567890abcdef |
69101
| sip_bridge_groups | ["group1"] |
102+
| default_sip_enabled | 1 |
103+
# The default value is sent by clients without the user requesting SIP,
104+
# so it is disabled instead of failing the creation
105+
When user "participant1" creates room "room" (v4)
106+
| roomType | 3 |
107+
| roomName | room |
108+
| sipEnabled | 1 |
109+
And user "participant1" creates room "room2" with 403 (v4)
110+
| roomType | 3 |
111+
| roomName | room2 |
112+
| sipEnabled | 2 |
113+
Then user "participant1" is participant of the following rooms (v4)
114+
| id | type | participantType | sipEnabled |
115+
| room | 3 | 1 | 0 |
116+
117+
Scenario: Enable SIP during creation while not being restricted by groups
118+
Given the following "spreed" app config is set
119+
| sip_bridge_dialin_info | +49-1234-567890 |
120+
| sip_bridge_shared_secret | 1234567890abcdef |
70121
Given user "participant1" creates room "room" (v4)
71122
| roomType | 3 |
72123
| roomName | room |

‎tests/integration/features/conversation-5/sip-dialin.feature‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,13 @@ Feature: conversation-5/sip-dialin
9393
| name | type | sipEnabled |
9494
| room 1 | 2 | 1 |
9595
| room 2 | 2 | 1 |
96+
# The forced state is not user input, so it also applies to users that are
97+
# not allowed to enable SIP themselves
98+
Given user "participant2" exists
99+
When user "participant2" creates room "room 3" (v4)
100+
| roomType | 2 |
101+
| roomName | room 3 |
102+
| sipEnabled | 2 |
103+
Then user "participant2" is participant of the following rooms (v4)
104+
| name | type | sipEnabled |
105+
| room 3 | 2 | 1 |

0 commit comments

Comments
 (0)