From 184da5f301e66af6c8510e6cbb4304ff137b4efd Mon Sep 17 00:00:00 2001 From: Pascal Kienast Date: Sat, 11 Jul 2026 16:46:22 +0200 Subject: [PATCH 1/3] feat(call): add setting to limit starting calls to specific groups Adds a new app config 'start_calls_groups' (JSON array of group ids, empty by default). When set, only members of the configured groups can start a call, while everyone can still chat and join ongoing calls. This complements the existing 'start_calls' level and follows the 'start_conversations' pattern. - Config::getAllowedStartCallGroupIds() / isNotAllowedToStartCalls() - Enforced via Participant::canStartCall() so both the API enforcement (RestrictStartingCalls listener) and the clients (canStartCall room property) share the same source of truth - Admin settings UI: group multiselect next to the existing options - Deleted groups are cleaned from the setting - Talk hash includes the new config to invalidate client state Resolves #17465 Signed-off-by: Pascal Kienast Assisted-by: ClaudeCode:claude-fable-5 Assisted-by: OpenAI Codex:GPT-5 --- docs/settings.md | 1 + lib/Config.php | 44 +++++++++++++ lib/Controller/RoomController.php | 1 + lib/Listener/GroupDeletedListener.php | 1 + lib/Listener/RestrictStartingCalls.php | 4 +- lib/Participant.php | 6 +- lib/Service/RoomFormatter.php | 2 +- lib/Settings/Admin/AdminSettings.php | 3 + .../AdminSettings/AllowedGroups.vue | 63 ++++++++++++++++++- 9 files changed, 120 insertions(+), 5 deletions(-) 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/Config.php b/lib/Config.php index 376c232449e..176bb75d551 100644 --- a/lib/Config.php +++ b/lib/Config.php @@ -285,6 +285,50 @@ public function isNotAllowedToCreateConversations(IUser $user): bool { return empty(array_intersect($allowedGroups, $userGroups)); } + /** + * @return string[] + */ + public function getAllowedStartCallGroupIds(): array { + $groups = $this->config->getAppValue('spreed', 'start_calls_groups', '[]'); + $groups = json_decode($groups, true); + return \is_array($groups) ? $groups : []; + } + + /** + * Unlike {@see self::isNotAllowedToCreateConversations()} this check works + * on the participant level rather than the user level, because the room + * context is required: for federated conversations the host server decides + * who can start a call, and non-user actors (guests, email guests and + * federated users) can never be a member of a local group. Config is used + * as the location for this check as it owns the user and group managers, + * which {@see Participant} does not. + */ + public function isNotAllowedToStartCalls(Participant $participant): bool { + $allowedGroups = $this->getAllowedStartCallGroupIds(); + if (empty($allowedGroups)) { + return false; + } + + if ($participant->getRoom()->isFederatedConversation()) { + // The host server of the conversation decides who can start a call + return false; + } + + $attendee = $participant->getAttendee(); + if ($attendee->getActorType() !== Attendee::ACTOR_USERS) { + // Guests, email guests and federated users can never be a member of a local group + return true; + } + + $user = $this->userManager->get($attendee->getActorId()); + if (!$user instanceof IUser) { + return true; + } + + $userGroups = $this->groupManager->getUserGroupIds($user); + return empty(array_intersect($allowedGroups, $userGroups)); + } + /** * @return int<0, 511> * @psalm-return int-mask-of 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..e965258cca7 100644 --- a/lib/Listener/RestrictStartingCalls.php +++ b/lib/Listener/RestrictStartingCalls.php @@ -8,6 +8,7 @@ namespace OCA\Talk\Listener; +use OCA\Talk\Config; use OCA\Talk\Events\AParticipantModifiedEvent; use OCA\Talk\Events\BeforeParticipantModifiedEvent; use OCA\Talk\Exceptions\ForbiddenException; @@ -24,6 +25,7 @@ class RestrictStartingCalls implements IEventListener { public function __construct( private readonly IConfig $serverConfig, + private readonly Config $talkConfig, private readonly ParticipantService $participantService, ) { } @@ -62,7 +64,7 @@ public function handle(Event $event): void { return; } - if (!$event->getParticipant()->canStartCall($this->serverConfig) + if (!$event->getParticipant()->canStartCall($this->serverConfig, $this->talkConfig) && !$this->participantService->hasActiveSessionsInCall($room)) { throw new ForbiddenException('Can not start a call'); } diff --git a/lib/Participant.php b/lib/Participant.php index a1a424f7191..631f36863ea 100644 --- a/lib/Participant.php +++ b/lib/Participant.php @@ -86,7 +86,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, Config $talkConfig): bool { if ($this->room->getType() === Room::TYPE_NOTE_TO_SELF) { return false; } @@ -101,6 +101,10 @@ public function canStartCall(IConfig $config): bool { return false; } + if ($talkConfig->isNotAllowedToStartCalls($this)) { + return false; + } + if ($defaultStartCall === Room::START_CALL_EVERYONE) { return true; } diff --git a/lib/Service/RoomFormatter.php b/lib/Service/RoomFormatter.php index a7a13b0a5ed..2c16bc64fe5 100644 --- a/lib/Service/RoomFormatter.php +++ b/lib/Service/RoomFormatter.php @@ -387,7 +387,7 @@ public function formatRoomV4( return $roomData; } - $roomData['canStartCall'] = $currentParticipant->canStartCall($this->serverConfig) + $roomData['canStartCall'] = $currentParticipant->canStartCall($this->serverConfig, $this->talkConfig) || ($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..b97eb999e9b 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->talkConfig->getAllowedStartCallGroupIds(), '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 From d2ec9032a589ab3abcf277ffc2ab3c07af38c808 Mon Sep 17 00:00:00 2001 From: Pascal Kienast Date: Sat, 11 Jul 2026 16:46:22 +0200 Subject: [PATCH 2/3] test(call): cover group-limited call start Unit tests for Config::isNotAllowedToStartCalls() and the adjusted RestrictStartingCalls listener, plus an integration feature covering: empty config (regression), allowed group member starting, non-member being blocked, and non-member joining an ongoing call. Resolves #17465 Signed-off-by: Pascal Kienast Assisted-by: ClaudeCode:claude-fable-5 --- .../callapi/limit-starting-calls.feature | 37 ++++++++++++++ tests/php/ConfigTest.php | 49 +++++++++++++++++++ .../Listener/RestrictStartingCallsTest.php | 7 ++- 3 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 tests/integration/features/callapi/limit-starting-calls.feature 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..914b7155e78 100644 --- a/tests/php/ConfigTest.php +++ b/tests/php/ConfigTest.php @@ -10,6 +10,9 @@ use OCA\Talk\Config; use OCA\Talk\Events\BeforeTurnServersGetEvent; +use OCA\Talk\Model\Attendee; +use OCA\Talk\Participant; +use OCA\Talk\Room; use OCA\Talk\Tests\php\Mocks\GetTurnServerListener; use OCA\Talk\Vendor\Firebase\JWT\JWT; use OCA\Talk\Vendor\Firebase\JWT\Key; @@ -411,4 +414,50 @@ public function testSignalingTicketV2Anonymous(string $algo): void { $this->assertEquals($now, $decoded->iat); $this->assertEquals('https://domain.invalid/nextcloud', $decoded->iss); } + + public static function dataIsNotAllowedToStartCalls(): array { + return [ + 'no restriction' => ['[]', Attendee::ACTOR_USERS, ['group2'], false, false], + 'invalid json' => ['{invalid', Attendee::ACTOR_USERS, ['group2'], false, false], + 'member of allowed group' => ['["group1"]', Attendee::ACTOR_USERS, ['group1', 'group2'], false, false], + 'not a member' => ['["group1"]', Attendee::ACTOR_USERS, ['group2'], false, true], + 'guests are blocked' => ['["group1"]', Attendee::ACTOR_GUESTS, [], false, true], + 'email guests are blocked' => ['["group1"]', Attendee::ACTOR_EMAILS, [], false, true], + 'federated users are blocked' => ['["group1"]', Attendee::ACTOR_FEDERATED_USERS, [], false, true], + 'host decides for proxy conversations' => ['["group1"]', Attendee::ACTOR_USERS, ['group2'], true, false], + ]; + } + + #[DataProvider('dataIsNotAllowedToStartCalls')] + public function testIsNotAllowedToStartCalls(string $groupConfig, string $actorType, array $userGroups, bool $isFederatedConversation, bool $expected): void { + $this->config + ->expects($this->once()) + ->method('getAppValue') + ->with('spreed', 'start_calls_groups', '[]') + ->willReturn($groupConfig); + + /** @var IUser&MockObject $user */ + $user = $this->createMock(IUser::class); + $this->userManager + ->method('get') + ->with('user1') + ->willReturn($user); + $this->groupManager + ->method('getUserGroupIds') + ->with($user) + ->willReturn($userGroups); + + $room = $this->createMock(Room::class); + $room->method('isFederatedConversation') + ->willReturn($isFederatedConversation); + + $attendee = new Attendee(); + $attendee->setActorType($actorType); + $attendee->setActorId('user1'); + + $participant = new Participant($room, $attendee, null); + + $helper = $this->getConfig(); + $this->assertSame($expected, $helper->isNotAllowedToStartCalls($participant)); + } } diff --git a/tests/php/Listener/RestrictStartingCallsTest.php b/tests/php/Listener/RestrictStartingCallsTest.php index 6ef0b382feb..2b2dda74abe 100644 --- a/tests/php/Listener/RestrictStartingCallsTest.php +++ b/tests/php/Listener/RestrictStartingCallsTest.php @@ -8,6 +8,7 @@ namespace OCA\Talk\Tests\php\Listener; +use OCA\Talk\Config; use OCA\Talk\Events\BeforeParticipantModifiedEvent; use OCA\Talk\Exceptions\ForbiddenException; use OCA\Talk\Listener\RestrictStartingCalls; @@ -23,6 +24,7 @@ #[Group('DB')] class RestrictStartingCallsTest extends TestCase { protected IConfig&MockObject $serverConfig; + protected Config&MockObject $talkConfig; protected ParticipantService&MockObject $participantService; protected ?RestrictStartingCalls $listener = null; @@ -30,8 +32,9 @@ public function setUp(): void { parent::setUp(); $this->serverConfig = $this->createMock(IConfig::class); + $this->talkConfig = $this->createMock(Config::class); $this->participantService = $this->createMock(ParticipantService::class); - $this->listener = new RestrictStartingCalls($this->serverConfig, $this->participantService); + $this->listener = new RestrictStartingCalls($this->serverConfig, $this->talkConfig, $this->participantService); } public static function dataCheckStartCallPermissions(): array { @@ -55,7 +58,7 @@ public function testCheckStartCallPermissions(int $roomType, string $roomObjectT $participant = $this->createMock(Participant::class); $participant->method('canStartCall') - ->with($this->serverConfig) + ->with($this->serverConfig, $this->talkConfig) ->willReturn($canStart); $this->participantService->method('hasActiveSessionsInCall') From eba1aec4b484aaa830b31e1d574837f35baa356f Mon Sep 17 00:00:00 2001 From: Pascal Kienast Date: Sat, 18 Jul 2026 13:32:56 +0200 Subject: [PATCH 3/3] refactor(call): centralize start-call group check Use IAppConfig for the group array and keep participant-level call authorization in Participant::canStartCall. Assisted-by: OpenAI Codex:GPT-5 Signed-off-by: Pascal Kienast --- lib/Config.php | 44 ------------ lib/Listener/RestrictStartingCalls.php | 8 ++- lib/Participant.php | 31 +++++++- lib/Service/RoomFormatter.php | 4 +- lib/Settings/Admin/AdminSettings.php | 2 +- tests/php/ConfigTest.php | 48 ------------- .../Listener/RestrictStartingCallsTest.php | 13 ++-- tests/php/ParticipantTest.php | 71 +++++++++++++++++++ .../php/Settings/Admin/AdminSettingsTest.php | 45 ++++++++++++ 9 files changed, 162 insertions(+), 104 deletions(-) create mode 100644 tests/php/ParticipantTest.php diff --git a/lib/Config.php b/lib/Config.php index 176bb75d551..376c232449e 100644 --- a/lib/Config.php +++ b/lib/Config.php @@ -285,50 +285,6 @@ public function isNotAllowedToCreateConversations(IUser $user): bool { return empty(array_intersect($allowedGroups, $userGroups)); } - /** - * @return string[] - */ - public function getAllowedStartCallGroupIds(): array { - $groups = $this->config->getAppValue('spreed', 'start_calls_groups', '[]'); - $groups = json_decode($groups, true); - return \is_array($groups) ? $groups : []; - } - - /** - * Unlike {@see self::isNotAllowedToCreateConversations()} this check works - * on the participant level rather than the user level, because the room - * context is required: for federated conversations the host server decides - * who can start a call, and non-user actors (guests, email guests and - * federated users) can never be a member of a local group. Config is used - * as the location for this check as it owns the user and group managers, - * which {@see Participant} does not. - */ - public function isNotAllowedToStartCalls(Participant $participant): bool { - $allowedGroups = $this->getAllowedStartCallGroupIds(); - if (empty($allowedGroups)) { - return false; - } - - if ($participant->getRoom()->isFederatedConversation()) { - // The host server of the conversation decides who can start a call - return false; - } - - $attendee = $participant->getAttendee(); - if ($attendee->getActorType() !== Attendee::ACTOR_USERS) { - // Guests, email guests and federated users can never be a member of a local group - return true; - } - - $user = $this->userManager->get($attendee->getActorId()); - if (!$user instanceof IUser) { - return true; - } - - $userGroups = $this->groupManager->getUserGroupIds($user); - return empty(array_intersect($allowedGroups, $userGroups)); - } - /** * @return int<0, 511> * @psalm-return int-mask-of diff --git a/lib/Listener/RestrictStartingCalls.php b/lib/Listener/RestrictStartingCalls.php index e965258cca7..44b1527b58a 100644 --- a/lib/Listener/RestrictStartingCalls.php +++ b/lib/Listener/RestrictStartingCalls.php @@ -8,16 +8,17 @@ namespace OCA\Talk\Listener; -use OCA\Talk\Config; use OCA\Talk\Events\AParticipantModifiedEvent; use OCA\Talk\Events\BeforeParticipantModifiedEvent; use OCA\Talk\Exceptions\ForbiddenException; 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 @@ -25,7 +26,8 @@ class RestrictStartingCalls implements IEventListener { public function __construct( private readonly IConfig $serverConfig, - private readonly Config $talkConfig, + private readonly IAppConfig $appConfig, + private readonly IGroupManager $groupManager, private readonly ParticipantService $participantService, ) { } @@ -64,7 +66,7 @@ public function handle(Event $event): void { return; } - if (!$event->getParticipant()->canStartCall($this->serverConfig, $this->talkConfig) + 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 631f36863ea..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, Config $talkConfig): bool { + public function canStartCall(IConfig $config, IAppConfig $appConfig, IGroupManager $groupManager): bool { if ($this->room->getType() === Room::TYPE_NOTE_TO_SELF) { return false; } @@ -101,7 +103,7 @@ public function canStartCall(IConfig $config, Config $talkConfig): bool { return false; } - if ($talkConfig->isNotAllowedToStartCalls($this)) { + if (!$this->isAllowedToStartCallByGroup($appConfig, $groupManager)) { return false; } @@ -120,6 +122,31 @@ public function canStartCall(IConfig $config, Config $talkConfig): 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 2c16bc64fe5..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, $this->talkConfig) + $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 b97eb999e9b..92487c9cf9e 100644 --- a/lib/Settings/Admin/AdminSettings.php +++ b/lib/Settings/Admin/AdminSettings.php @@ -79,7 +79,7 @@ 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->talkConfig->getAllowedStartCallGroupIds(), 'start_calls_groups'); + $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'); diff --git a/tests/php/ConfigTest.php b/tests/php/ConfigTest.php index 914b7155e78..869ded35431 100644 --- a/tests/php/ConfigTest.php +++ b/tests/php/ConfigTest.php @@ -10,9 +10,6 @@ use OCA\Talk\Config; use OCA\Talk\Events\BeforeTurnServersGetEvent; -use OCA\Talk\Model\Attendee; -use OCA\Talk\Participant; -use OCA\Talk\Room; use OCA\Talk\Tests\php\Mocks\GetTurnServerListener; use OCA\Talk\Vendor\Firebase\JWT\JWT; use OCA\Talk\Vendor\Firebase\JWT\Key; @@ -415,49 +412,4 @@ public function testSignalingTicketV2Anonymous(string $algo): void { $this->assertEquals('https://domain.invalid/nextcloud', $decoded->iss); } - public static function dataIsNotAllowedToStartCalls(): array { - return [ - 'no restriction' => ['[]', Attendee::ACTOR_USERS, ['group2'], false, false], - 'invalid json' => ['{invalid', Attendee::ACTOR_USERS, ['group2'], false, false], - 'member of allowed group' => ['["group1"]', Attendee::ACTOR_USERS, ['group1', 'group2'], false, false], - 'not a member' => ['["group1"]', Attendee::ACTOR_USERS, ['group2'], false, true], - 'guests are blocked' => ['["group1"]', Attendee::ACTOR_GUESTS, [], false, true], - 'email guests are blocked' => ['["group1"]', Attendee::ACTOR_EMAILS, [], false, true], - 'federated users are blocked' => ['["group1"]', Attendee::ACTOR_FEDERATED_USERS, [], false, true], - 'host decides for proxy conversations' => ['["group1"]', Attendee::ACTOR_USERS, ['group2'], true, false], - ]; - } - - #[DataProvider('dataIsNotAllowedToStartCalls')] - public function testIsNotAllowedToStartCalls(string $groupConfig, string $actorType, array $userGroups, bool $isFederatedConversation, bool $expected): void { - $this->config - ->expects($this->once()) - ->method('getAppValue') - ->with('spreed', 'start_calls_groups', '[]') - ->willReturn($groupConfig); - - /** @var IUser&MockObject $user */ - $user = $this->createMock(IUser::class); - $this->userManager - ->method('get') - ->with('user1') - ->willReturn($user); - $this->groupManager - ->method('getUserGroupIds') - ->with($user) - ->willReturn($userGroups); - - $room = $this->createMock(Room::class); - $room->method('isFederatedConversation') - ->willReturn($isFederatedConversation); - - $attendee = new Attendee(); - $attendee->setActorType($actorType); - $attendee->setActorId('user1'); - - $participant = new Participant($room, $attendee, null); - - $helper = $this->getConfig(); - $this->assertSame($expected, $helper->isNotAllowedToStartCalls($participant)); - } } diff --git a/tests/php/Listener/RestrictStartingCallsTest.php b/tests/php/Listener/RestrictStartingCallsTest.php index 2b2dda74abe..91c49a8ffd6 100644 --- a/tests/php/Listener/RestrictStartingCallsTest.php +++ b/tests/php/Listener/RestrictStartingCallsTest.php @@ -8,14 +8,15 @@ namespace OCA\Talk\Tests\php\Listener; -use OCA\Talk\Config; use OCA\Talk\Events\BeforeParticipantModifiedEvent; use OCA\Talk\Exceptions\ForbiddenException; use OCA\Talk\Listener\RestrictStartingCalls; 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; @@ -24,7 +25,8 @@ #[Group('DB')] class RestrictStartingCallsTest extends TestCase { protected IConfig&MockObject $serverConfig; - protected Config&MockObject $talkConfig; + protected IAppConfig&MockObject $appConfig; + protected IGroupManager&MockObject $groupManager; protected ParticipantService&MockObject $participantService; protected ?RestrictStartingCalls $listener = null; @@ -32,9 +34,10 @@ public function setUp(): void { parent::setUp(); $this->serverConfig = $this->createMock(IConfig::class); - $this->talkConfig = $this->createMock(Config::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->talkConfig, $this->participantService); + $this->listener = new RestrictStartingCalls($this->serverConfig, $this->appConfig, $this->groupManager, $this->participantService); } public static function dataCheckStartCallPermissions(): array { @@ -58,7 +61,7 @@ public function testCheckStartCallPermissions(int $roomType, string $roomObjectT $participant = $this->createMock(Participant::class); $participant->method('canStartCall') - ->with($this->serverConfig, $this->talkConfig) + ->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')