fix(sip): Correct restrict SIP creation

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?

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 |

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          |