diff --git a/docs/settings.md b/docs/settings.md index 8f84c076b53..59f5bea81f8 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -71,6 +71,7 @@ Legend: | `allowed_groups` | string[] | `[]` | Yes | 🖌️ | List of group ids that are allowed to use Talk | | `sip_bridge_groups` | string[] | `[]` | Yes | 🖌️ | List of group ids that are allowed to enable SIP dial-in in a conversation | | `start_conversations` | string[] | `[]` | Yes | 🖌️ | List of group ids that are allowed to create conversations | +| `start_calls_groups` | string[] | `[]` | Yes | 🖌️ | List of group ids that are allowed to start calls (everyone when empty). This also applies to moderators; guests and federated users can then never start calls. For federated conversations the host server decides. Others can still chat and join ongoing calls | | `federation_allowed_groups` | string[] | `[]` | Yes | 🖌️ | 🏗️ *Work in progress:* List of group ids that are allowed to invite federated users into their conversations (everyone when empty) | | `hosted-signaling-server-account` | array | `{}` | No | 🖌️ | Account information of the hosted signaling server | | `stun_servers` | array[] | `[]` | Yes | 🖌💻️ | List of STUN servers, should be configured via the web interface or the OCC commands | diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 090facc3365..474451d6f66 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -187,6 +187,7 @@ protected function getTalkHashHeader(): array { $this->config->getAppValue('spreed', 'recording_servers'), $this->config->getAppValue('spreed', 'allowed_groups'), $this->config->getAppValue('spreed', 'start_calls'), + $this->config->getAppValue('spreed', 'start_calls_groups'), $this->config->getAppValue('spreed', 'start_conversations'), $this->config->getAppValue('spreed', 'default_permissions'), $this->config->getAppValue('spreed', 'breakout_rooms'), diff --git a/lib/Listener/GroupDeletedListener.php b/lib/Listener/GroupDeletedListener.php index c39b329fdca..9722471e741 100644 --- a/lib/Listener/GroupDeletedListener.php +++ b/lib/Listener/GroupDeletedListener.php @@ -38,6 +38,7 @@ public function handle(Event $event): void { $gid = $event->getGroup()->getGID(); $this->removeGroupFromConfig('sip_bridge_groups', $gid); + $this->removeGroupFromConfig('start_calls_groups', $gid); $this->removeGroupFromConfig('start_conversations', $gid); $this->removeGroupFromConfig('allowed_groups', $gid); diff --git a/lib/Listener/RestrictStartingCalls.php b/lib/Listener/RestrictStartingCalls.php index 1b0f14cbcce..44b1527b58a 100644 --- a/lib/Listener/RestrictStartingCalls.php +++ b/lib/Listener/RestrictStartingCalls.php @@ -14,9 +14,11 @@ use OCA\Talk\Participant; use OCA\Talk\Room; use OCA\Talk\Service\ParticipantService; +use OCP\AppFramework\Services\IAppConfig; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; use OCP\IConfig; +use OCP\IGroupManager; /** * @template-implements IEventListener @@ -24,6 +26,8 @@ class RestrictStartingCalls implements IEventListener { public function __construct( private readonly IConfig $serverConfig, + private readonly IAppConfig $appConfig, + private readonly IGroupManager $groupManager, private readonly ParticipantService $participantService, ) { } @@ -62,7 +66,7 @@ public function handle(Event $event): void { return; } - if (!$event->getParticipant()->canStartCall($this->serverConfig) + if (!$event->getParticipant()->canStartCall($this->serverConfig, $this->appConfig, $this->groupManager) && !$this->participantService->hasActiveSessionsInCall($room)) { throw new ForbiddenException('Can not start a call'); } diff --git a/lib/Participant.php b/lib/Participant.php index a1a424f7191..8f9c76775f4 100644 --- a/lib/Participant.php +++ b/lib/Participant.php @@ -10,7 +10,9 @@ use OCA\Talk\Model\Attendee; use OCA\Talk\Model\Session; +use OCP\AppFramework\Services\IAppConfig; use OCP\IConfig; +use OCP\IGroupManager; use OCP\Server; class Participant { @@ -86,7 +88,7 @@ public function hasModeratorPermissions(bool $guestModeratorAllowed = true): boo return \in_array($participantType, [self::OWNER, self::MODERATOR, self::GUEST_MODERATOR], true); } - public function canStartCall(IConfig $config): bool { + public function canStartCall(IConfig $config, IAppConfig $appConfig, IGroupManager $groupManager): bool { if ($this->room->getType() === Room::TYPE_NOTE_TO_SELF) { return false; } @@ -101,6 +103,10 @@ public function canStartCall(IConfig $config): bool { return false; } + if (!$this->isAllowedToStartCallByGroup($appConfig, $groupManager)) { + return false; + } + if ($defaultStartCall === Room::START_CALL_EVERYONE) { return true; } @@ -116,6 +122,31 @@ public function canStartCall(IConfig $config): bool { return false; } + protected function isAllowedToStartCallByGroup(IAppConfig $appConfig, IGroupManager $groupManager): bool { + $allowedGroups = $appConfig->getAppValueArray('start_calls_groups'); + if (empty($allowedGroups)) { + return true; + } + + if ($this->room->isFederatedConversation()) { + // The host server of the conversation decides who can start a call + return true; + } + + if ($this->attendee->getActorType() !== Attendee::ACTOR_USERS) { + // Guests, email guests and federated users can never be a member of a local group + return false; + } + + foreach ($allowedGroups as $groupId) { + if ($groupManager->isInGroup($this->attendee->getActorId(), $groupId)) { + return true; + } + } + + return false; + } + public function getPermissions(): int { $permissions = $this->getPermissionsFromFallbackChain(); diff --git a/lib/Service/RoomFormatter.php b/lib/Service/RoomFormatter.php index a7a13b0a5ed..c6ed7594ff3 100644 --- a/lib/Service/RoomFormatter.php +++ b/lib/Service/RoomFormatter.php @@ -25,6 +25,7 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\Comments\IComment; use OCP\IConfig; +use OCP\IGroupManager; use OCP\IL10N; use OCP\IUser; use OCP\IUserManager; @@ -45,6 +46,7 @@ public function __construct( private readonly ChatManager $chatManager, private readonly MessageParser $messageParser, private readonly IConfig $serverConfig, + private readonly IGroupManager $groupManager, private readonly ITimeFactory $timeFactory, private readonly IAppManager $appManager, private readonly IManager $userStatusManager, @@ -387,7 +389,7 @@ public function formatRoomV4( return $roomData; } - $roomData['canStartCall'] = $currentParticipant->canStartCall($this->serverConfig) + $roomData['canStartCall'] = $currentParticipant->canStartCall($this->serverConfig, $this->appConfig, $this->groupManager) || ($room->getType() === Room::TYPE_PUBLIC && $room->getObjectType() === Room::OBJECT_TYPE_VIDEO_VERIFICATION); diff --git a/lib/Settings/Admin/AdminSettings.php b/lib/Settings/Admin/AdminSettings.php index 18c32f2e828..92487c9cf9e 100644 --- a/lib/Settings/Admin/AdminSettings.php +++ b/lib/Settings/Admin/AdminSettings.php @@ -79,6 +79,9 @@ protected function initGeneralSettings(): void { protected function initAllowedGroups(): void { $this->initialState->provideInitialState('start_calls', (int)$this->serverConfig->getAppValue('spreed', 'start_calls', (string)Room::START_CALL_EVERYONE)); + $groups = $this->getGroupDetailsArray($this->appConfig->getAppValueArray('start_calls_groups'), 'start_calls_groups'); + $this->initialState->provideInitialState('start_calls_groups', $groups); + $groups = $this->getGroupDetailsArray($this->talkConfig->getAllowedConversationsGroupIds(), 'start_conversations'); $this->initialState->provideInitialState('start_conversations', $groups); diff --git a/src/components/AdminSettings/AllowedGroups.vue b/src/components/AdminSettings/AllowedGroups.vue index c241807a014..6697dafcc96 100644 --- a/src/components/AdminSettings/AllowedGroups.vue +++ b/src/components/AdminSettings/AllowedGroups.vue @@ -69,6 +69,32 @@ {{ saveLabelStartConversations }} + + + {{ saveLabelStartCallsGroups }} + + +

+ {{ t('spreed', 'When at least one group is selected, only members of these groups can start a call. This also applies to moderators, while guests and federated users are never members of these groups. Everyone can still start a call when no group is selected.') }} +

+

+ {{ t('spreed', 'For conversations hosted on a federated server, the configuration of the host server applies instead.') }} +

{{ t('spreed', 'When a call has started, everyone with access to the conversation can join the call.') }}

@@ -122,8 +154,10 @@ export default { groups: [], allowedGroups: [], canStartConversations: [], + canStartCallsGroups: [], saveLabelAllowedGroups: t('spreed', 'Save changes'), saveLabelStartConversations: t('spreed', 'Save changes'), + saveLabelStartCallsGroups: t('spreed', 'Save changes'), startCallOptions, startCalls: startCallOptions[0], @@ -140,12 +174,16 @@ export default { this.canStartConversations = loadState('spreed', 'start_conversations', []).sort(function(a, b) { return a.displayname.localeCompare(b.displayname) }) + this.canStartCallsGroups = loadState('spreed', 'start_calls_groups', []).sort(function(a, b) { + return a.displayname.localeCompare(b.displayname) + }) this.startCalls = startCallOptions[parseInt(loadState('spreed', 'start_calls'))] - // Make a unique list with the groups we know from allowedGroups and canStartConversations + // Make a unique list with the groups we know from allowedGroups, + // canStartConversations and canStartCallsGroups // Unique checking is done by turning the group objects (with id and name) // into json strings and afterwards back again - const mergedGroups = Array.from(new Set(this.allowedGroups.concat(this.canStartConversations) + const mergedGroups = Array.from(new Set(this.allowedGroups.concat(this.canStartConversations).concat(this.canStartCallsGroups) .map((g) => JSON.stringify(g)))).map((g) => JSON.parse(g)) this.groups = mergedGroups.sort(function(a, b) { @@ -223,6 +261,27 @@ export default { }) }, + saveStartCallsGroups() { + this.loading = true + this.loadingGroups = true + this.saveLabelStartCallsGroups = t('spreed', 'Saving …') + + const groups = this.canStartCallsGroups.map((group) => { + return group.id + }) + + OCP.AppConfig.setValue('spreed', 'start_calls_groups', JSON.stringify(groups), { + success: () => { + this.loading = false + this.loadingGroups = false + this.saveLabelStartCallsGroups = t('spreed', 'Saved!') + setTimeout(() => { + this.saveLabelStartCallsGroups = t('spreed', 'Save changes') + }, 5000) + }, + }) + }, + saveStartCalls() { this.loadingStartCalls = true diff --git a/tests/integration/features/callapi/limit-starting-calls.feature b/tests/integration/features/callapi/limit-starting-calls.feature new file mode 100644 index 00000000000..56356cc8c58 --- /dev/null +++ b/tests/integration/features/callapi/limit-starting-calls.feature @@ -0,0 +1,37 @@ +Feature: callapi/limit-starting-calls + Background: + Given user "participant1" exists + Given user "participant2" exists + Given group "group1" exists + + Scenario: User (even as room owner) can not start a call while not being a member of the allowed groups + Given the following "spreed" app config is set + | start_calls_groups | ["group1"] | + Given user "participant1" creates room "room" (v4) + | roomType | 3 | + | roomName | room | + And user "participant1" joins room "room" with 200 (v4) + Then user "participant1" joins call "room" with 403 (v4) + Given user "participant1" is member of group "group1" + Then user "participant1" joins call "room" with 200 (v4) + + Scenario: User that is not a member of the allowed groups can join an ongoing call + Given the following "spreed" app config is set + | start_calls_groups | ["group1"] | + And user "participant1" is member of group "group1" + Given user "participant1" creates room "room" (v4) + | roomType | 3 | + | roomName | room | + And user "participant1" adds user "participant2" to room "room" with 200 (v4) + And user "participant1" joins room "room" with 200 (v4) + And user "participant2" joins room "room" with 200 (v4) + Then user "participant2" joins call "room" with 403 (v4) + When user "participant1" joins call "room" with 200 (v4) + Then user "participant2" joins call "room" with 200 (v4) + + Scenario: Without the app config being set every user can start a call + Given user "participant1" creates room "room" (v4) + | roomType | 3 | + | roomName | room | + And user "participant1" joins room "room" with 200 (v4) + Then user "participant1" joins call "room" with 200 (v4) diff --git a/tests/php/ConfigTest.php b/tests/php/ConfigTest.php index 59598d1d300..869ded35431 100644 --- a/tests/php/ConfigTest.php +++ b/tests/php/ConfigTest.php @@ -411,4 +411,5 @@ public function testSignalingTicketV2Anonymous(string $algo): void { $this->assertEquals($now, $decoded->iat); $this->assertEquals('https://domain.invalid/nextcloud', $decoded->iss); } + } diff --git a/tests/php/Listener/RestrictStartingCallsTest.php b/tests/php/Listener/RestrictStartingCallsTest.php index 6ef0b382feb..91c49a8ffd6 100644 --- a/tests/php/Listener/RestrictStartingCallsTest.php +++ b/tests/php/Listener/RestrictStartingCallsTest.php @@ -14,7 +14,9 @@ use OCA\Talk\Participant; use OCA\Talk\Room; use OCA\Talk\Service\ParticipantService; +use OCP\AppFramework\Services\IAppConfig; use OCP\IConfig; +use OCP\IGroupManager; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\MockObject\MockObject; @@ -23,6 +25,8 @@ #[Group('DB')] class RestrictStartingCallsTest extends TestCase { protected IConfig&MockObject $serverConfig; + protected IAppConfig&MockObject $appConfig; + protected IGroupManager&MockObject $groupManager; protected ParticipantService&MockObject $participantService; protected ?RestrictStartingCalls $listener = null; @@ -30,8 +34,10 @@ public function setUp(): void { parent::setUp(); $this->serverConfig = $this->createMock(IConfig::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->groupManager = $this->createMock(IGroupManager::class); $this->participantService = $this->createMock(ParticipantService::class); - $this->listener = new RestrictStartingCalls($this->serverConfig, $this->participantService); + $this->listener = new RestrictStartingCalls($this->serverConfig, $this->appConfig, $this->groupManager, $this->participantService); } public static function dataCheckStartCallPermissions(): array { @@ -55,7 +61,7 @@ public function testCheckStartCallPermissions(int $roomType, string $roomObjectT $participant = $this->createMock(Participant::class); $participant->method('canStartCall') - ->with($this->serverConfig) + ->with($this->serverConfig, $this->appConfig, $this->groupManager) ->willReturn($canStart); $this->participantService->method('hasActiveSessionsInCall') diff --git a/tests/php/ParticipantTest.php b/tests/php/ParticipantTest.php new file mode 100644 index 00000000000..4c0ed93a4a5 --- /dev/null +++ b/tests/php/ParticipantTest.php @@ -0,0 +1,71 @@ + [[], Attendee::ACTOR_USERS, [], false, true], + 'member of allowed group' => [['group1', 'group2'], Attendee::ACTOR_USERS, ['group2'], false, true], + 'not a member' => [['group1'], Attendee::ACTOR_USERS, ['group2'], false, false], + 'guests are blocked' => [['group1'], Attendee::ACTOR_GUESTS, [], false, false], + 'email guests are blocked' => [['group1'], Attendee::ACTOR_EMAILS, [], false, false], + 'federated users are blocked' => [['group1'], Attendee::ACTOR_FEDERATED_USERS, [], false, false], + 'host decides for proxy conversations' => [['group1'], Attendee::ACTOR_USERS, [], true, true], + ]; + } + + #[DataProvider('dataCanStartCallWithGroupRestriction')] + public function testCanStartCallWithGroupRestriction( + array $allowedGroups, + string $actorType, + array $userGroups, + bool $isFederatedConversation, + bool $expected, + ): void { + $room = $this->createMock(Room::class); + $room->method('getType')->willReturn(Room::TYPE_GROUP); + $room->method('isFederatedConversation')->willReturn($isFederatedConversation); + + $attendee = Attendee::fromRow([ + 'actor_type' => $actorType, + 'actor_id' => 'user1', + 'participant_type' => $actorType === Attendee::ACTOR_USERS ? Participant::USER : Participant::GUEST, + 'permissions' => Attendee::PERMISSIONS_CUSTOM | Attendee::PERMISSIONS_CALL_START, + ]); + $participant = new Participant($room, $attendee, null); + + $serverConfig = $this->createMock(IConfig::class); + $serverConfig->expects($this->once()) + ->method('getAppValue') + ->with('spreed', 'start_calls', (string)Room::START_CALL_EVERYONE) + ->willReturn((string)Room::START_CALL_EVERYONE); + + $appConfig = $this->createMock(IAppConfig::class); + $appConfig->expects($this->once()) + ->method('getAppValueArray') + ->with('start_calls_groups') + ->willReturn($allowedGroups); + + $groupManager = $this->createMock(IGroupManager::class); + $groupManager->method('isInGroup') + ->willReturnCallback(static fn (string $userId, string $groupId): bool => $userId === 'user1' && in_array($groupId, $userGroups, true)); + + $this->assertSame($expected, $participant->canStartCall($serverConfig, $appConfig, $groupManager)); + } +} diff --git a/tests/php/Settings/Admin/AdminSettingsTest.php b/tests/php/Settings/Admin/AdminSettingsTest.php index 423bffcf91d..6eab65f7892 100644 --- a/tests/php/Settings/Admin/AdminSettingsTest.php +++ b/tests/php/Settings/Admin/AdminSettingsTest.php @@ -10,11 +10,13 @@ use OCA\Talk\Config; use OCA\Talk\MatterbridgeManager; +use OCA\Talk\Room; use OCA\Talk\Settings\Admin\AdminSettings; use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Services\IInitialState; use OCP\ICacheFactory; use OCP\IConfig; +use OCP\IGroup; use OCP\IGroupManager; use OCP\IL10N; use OCP\IUserSession; @@ -133,6 +135,49 @@ public function testGetForm(): void { $this->assertCount(0, $form->getParams()); } + public function testInitAllowedGroups(): void { + $this->serverConfig->expects($this->once()) + ->method('getAppValue') + ->with('spreed', 'start_calls', (string)Room::START_CALL_EVERYONE) + ->willReturn((string)Room::START_CALL_EVERYONE); + $this->appConfig->expects($this->once()) + ->method('getAppValueArray') + ->with('start_calls_groups') + ->willReturn(['group1']); + $this->talkConfig->expects($this->once()) + ->method('getAllowedConversationsGroupIds') + ->willReturn([]); + $this->talkConfig->expects($this->once()) + ->method('getAllowedTalkGroupIds') + ->willReturn([]); + + $group = $this->createMock(IGroup::class); + $group->method('getGID')->willReturn('group1'); + $group->method('getDisplayName')->willReturn('Group 1'); + $this->groupManager->expects($this->once()) + ->method('get') + ->with('group1') + ->willReturn($group); + + $i = 0; + $expectedCalls = [ + ['start_calls', Room::START_CALL_EVERYONE], + ['start_calls_groups', [['id' => 'group1', 'displayname' => 'Group 1']]], + ['start_conversations', []], + ['allowed_groups', []], + ]; + $this->initialState->expects($this->exactly(4)) + ->method('provideInitialState') + ->willReturnCallback(function () use ($expectedCalls, &$i): void { + $this->assertArrayHasKey($i, $expectedCalls); + $this->assertSame($expectedCalls[$i], func_get_args()); + $i++; + }); + + $admin = $this->getAdminSettings(); + self::invokePrivate($admin, 'initAllowedGroups'); + } + public function testInitStunServers(): void { $this->talkConfig->expects($this->once()) ->method('getStunServers')