Skip to content

Commit

Permalink
Merge pull request #11503 from nextcloud/bugfix/11278/restriction-opt…
Browse files Browse the repository at this point in the history
…ions

feat(federation): Add appconfig options to restrict federation
  • Loading branch information
nickvergessen authored Feb 12, 2024
2 parents 569316a + 7412cf4 commit 87fe884
Show file tree
Hide file tree
Showing 12 changed files with 224 additions and 32 deletions.
4 changes: 4 additions & 0 deletions docs/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,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 |
| `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 |
| `turn_servers` | array[] | `[]` | Yes | 🖌️💻 | List of TURN servers, should be configured via the web interface or the OCC commands |
Expand Down Expand Up @@ -97,6 +98,9 @@ Legend:
| `call_recording_transcription` | string<br>`yes` or `no` | `no` | No | | Whether call recordings should automatically be transcripted when a transcription provider is enabled. |
| `sip_dialout` | string<br>`yes` or `no` | `no` | Yes | | SIP dial-out is allowed when a SIP bridge is configured |
| `federation_enabled` | string<br>`yes` or `no` | `no` | Yes | | 🏗️ *Work in progress:* Whether or not federation with this instance is allowed |
| `federation_incoming_enabled` | string<br>`1` or `0` | `1` | Yes | | 🏗️ *Work in progress:* Whether users of this instance can be invited to federated conversations |
| `federation_outgoing_enabled` | string<br>`1` or `0` | `1` | Yes | | 🏗️ *Work in progress:* Whether users of this instance can invite federated users into conversations |
| `federation_only_trusted_servers` | string<br>`1` or `0` | `0` | Yes | | 🏗️ *Work in progress:* Whether federation should be limited to the list of "Trusted servers" |
| `conversations_files` | string<br>`1` or `0` | `1` | No | 🖌️ | Whether the files app integration is enabled allowing to start conversations in the right sidebar |
| `conversations_files_public_shares` | string<br>`1` or `0` | `1` | No | 🖌️ | Whether the public share integration is enabled allowing to start conversations in the right sidebar on the public share page (Requires `conversations_files` also to be enabled) |
| `enable_matterbridge` | string<br>`1` or `0` | `0` | No | 🖌️ | Whether the Matterbridge integration is enabled and can be configured |
12 changes: 12 additions & 0 deletions lib/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use OCA\Talk\Model\Attendee;
use OCA\Talk\Service\RecordingService;
use OCA\Talk\Vendor\Firebase\JWT\JWT;
use OCP\AppFramework\Services\IAppConfig;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
Expand Down Expand Up @@ -56,6 +57,7 @@ class Config {

public function __construct(
protected IConfig $config,
protected IAppConfig $appConfig,
private ISecureRandom $secureRandom,
private IGroupManager $groupManager,
private IUserManager $userManager,
Expand Down Expand Up @@ -111,6 +113,16 @@ public function isFederationEnabled(): bool {
return $this->config->getAppValue('spreed', 'federation_enabled', 'no') === 'yes';
}

public function isFederationEnabledForUserId(IUser $user): bool {
$allowedGroups = $this->appConfig->getAppValueArray('federation_allowed_groups', lazy: true);
if (empty($allowedGroups)) {
return true;
}

$userGroups = $this->groupManager->getUserGroupIds($user);
return empty(array_intersect($allowedGroups, $userGroups));
}

public function isBreakoutRoomsEnabled(): bool {
return $this->config->getAppValue('spreed', 'breakout_rooms', 'yes') === 'yes';
}
Expand Down
4 changes: 4 additions & 0 deletions lib/Controller/RoomController.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ protected function getTalkHashHeader(): array {
$this->config->getAppValue('spreed', 'recording_consent'),
$this->config->getAppValue('theming', 'cachebuster', '1'),
$this->config->getUserValue($this->userId, 'theming', 'userCacheBuster', '0'),
$this->config->getAppValue('spreed', 'federation_incoming_enabled'),
$this->config->getAppValue('spreed', 'federation_outgoing_enabled'),
$this->config->getAppValue('spreed', 'federation_only_trusted_servers'),
$this->config->getAppValue('spreed', 'federation_allowed_groups', '[]'),
];

return [
Expand Down
63 changes: 42 additions & 21 deletions lib/Federation/BackendNotifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@
namespace OCA\Talk\Federation;

use OCA\FederatedFileSharing\AddressHandler;
use OCA\Talk\AppInfo\Application;
use OCA\Federation\TrustedServers;
use OCA\Talk\BackgroundJob\RetryJob;
use OCA\Talk\Config;
use OCA\Talk\Exceptions\RoomHasNoModeratorException;
use OCA\Talk\Model\Attendee;
use OCA\Talk\Room;
use OCP\App\IAppManager;
use OCP\AppFramework\Services\IAppConfig;
use OCP\BackgroundJob\IJobList;
use OCP\DB\Exception;
use OCP\Federation\ICloudFederationFactory;
Expand All @@ -40,6 +43,7 @@
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Server;
use Psr\Log\LoggerInterface;
use SensitiveParameter;

Expand All @@ -53,6 +57,9 @@ public function __construct(
private IJobList $jobList,
private IUserManager $userManager,
private IURLGenerator $url,
private IAppManager $appManager,
private Config $talkConfig,
private IAppConfig $appConfig,
) {
}

Expand All @@ -68,8 +75,7 @@ public function sendRemoteShare(
string $providerId,
string $token,
string $shareWith,
string $sharedBy,
string $sharedByFederatedId,
IUser $sharedBy,
string $shareType,
Room $room,
Attendee $roomOwnerAttendee,
Expand All @@ -81,13 +87,37 @@ public function sendRemoteShare(
$roomToken = $room->getToken();

if (!($user && $remote)) {
$this->logger->info(
"could not share $roomToken, invalid contact $shareWith",
['app' => Application::APP_ID]
);
$this->logger->info("Could not share conversation $roomToken as the recipient is invalid: $shareWith");
return false;
}

if (!$this->appConfig->getAppValueBool('federation_outgoing_enabled', true)) {
$this->logger->info("Could not share conversation $roomToken as outgoing federation is disabled");
return false;
}

if (!$this->talkConfig->isFederationEnabledForUserId($sharedBy)) {
$this->logger->info('Talk federation not allowed for user ' . $sharedBy->getUID());
return false;
}

if ($this->appConfig->getAppValueBool('federation_only_trusted_servers')) {
if (!$this->appManager->isEnabledForUser('federation')) {
$this->logger->error('Federation is limited to trusted servers but the "federation" app is disabled');
return false;
}

$trustedServers = Server::get(TrustedServers::class);
$serverUrl = $this->addressHandler->removeProtocolFromUrl($remote);
if (!$trustedServers->isTrustedServer($serverUrl)) {
$this->logger->warning(
'Tried to send Talk federation invite to untrusted server {serverUrl}',
['serverUrl' => $serverUrl]
);
return false;
}
}

/** @var IUser|null $roomOwner */
$roomOwner = $this->userManager->get($roomOwnerAttendee->getActorId());

Expand All @@ -100,8 +130,8 @@ public function sendRemoteShare(
$providerId,
$roomOwner->getCloudId(),
$roomOwner->getDisplayName(),
$sharedByFederatedId,
$sharedBy,
$sharedBy->getCloudId(),
$sharedBy->getDisplayName(),
$token,
$shareType,
FederationManager::TALK_ROOM_RESOURCE
Expand All @@ -118,10 +148,7 @@ public function sendRemoteShare(
if (is_array($response)) {
return true;
}
$this->logger->info(
"failed sharing $roomToken with $shareWith",
['app' => Application::APP_ID]
);
$this->logger->info("Failed sharing $roomToken with $shareWith");

return false;
}
Expand Down Expand Up @@ -152,10 +179,7 @@ public function sendShareAccepted(
]);
$response = $this->federationProviderManager->sendNotification($remote, $notification);
if (!is_array($response)) {
$this->logger->info(
"failed to send share accepted notification for share from $remote",
['app' => Application::APP_ID]
);
$this->logger->info("Failed to send share accepted notification for share from $remote");
return false;
}
return true;
Expand Down Expand Up @@ -186,10 +210,7 @@ public function sendShareDeclined(
);
$response = $this->federationProviderManager->sendNotification($remote, $notification);
if (!is_array($response)) {
$this->logger->info(
"failed to send share declined notification for share from $remote",
['app' => Application::APP_ID]
);
$this->logger->info("Failed to send share declined notification for share from $remote");
return false;
}
return true;
Expand Down
16 changes: 16 additions & 0 deletions lib/Federation/CloudFederationProviderTalk.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
use OCA\Talk\Service\RoomService;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Http;
use OCP\AppFramework\Services\IAppConfig;
use OCP\DB\Exception as DBException;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Federation\Exceptions\ActionNotSupportedException;
Expand All @@ -68,6 +69,7 @@ public function __construct(
private AddressHandler $addressHandler,
private FederationManager $federationManager,
private Config $config,
private IAppConfig $appConfig,
private INotificationManager $notificationManager,
private ParticipantService $participantService,
private RoomService $roomService,
Expand Down Expand Up @@ -97,6 +99,10 @@ public function shareReceived(ICloudFederationShare $share): string {
$this->logger->debug('Received a federation invite but federation is disabled');
throw new ProviderCouldNotAddShareException('Server does not support talk federation', '', Http::STATUS_SERVICE_UNAVAILABLE);
}
if (!$this->appConfig->getAppValueBool('federation_incoming_enabled', true)) {
$this->logger->warning('Received a federation invite but incoming federation is disabled');
throw new ProviderCouldNotAddShareException('Server does not support talk federation', '', Http::STATUS_SERVICE_UNAVAILABLE);
}
if (!in_array($share->getShareType(), $this->getSupportedShareTypes(), true)) {
$this->logger->debug('Received a federation invite for invalid share type');
throw new ProviderCouldNotAddShareException('Support for sharing with non-users not implemented yet', '', Http::STATUS_NOT_IMPLEMENTED);
Expand Down Expand Up @@ -135,6 +141,16 @@ public function shareReceived(ICloudFederationShare $share): string {
throw new ProviderCouldNotAddShareException('User does not exist', '', Http::STATUS_BAD_REQUEST);
}

if ($this->config->isDisabledForUser($shareWith)) {
$this->logger->debug('Received a federation invite for user that is not allowed to use Talk');
throw new ProviderCouldNotAddShareException('User does not exist', '', Http::STATUS_BAD_REQUEST);
}

if (!$this->config->isFederationEnabledForUserId($shareWith)) {
$this->logger->debug('Received a federation invite for user that is not allowed to use Talk Federation');
throw new ProviderCouldNotAddShareException('User does not exist', '', Http::STATUS_BAD_REQUEST);
}

$invite = $this->federationManager->addRemoteRoom($shareWith, (int) $remoteId, $roomType, $roomName, $roomToken, $remote, $shareSecret);

$this->notifyAboutNewShare($shareWith, (string) $invite->getId(), $sharedByFederatedId, $sharedBy, $roomName, $roomToken, $remote);
Expand Down
2 changes: 1 addition & 1 deletion lib/Service/ParticipantService.php
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ public function addUsers(Room $room, array $participants, ?IUser $addedBy = null
$this->attendeeMapper->insert($attendee);

if ($attendee->getActorType() === Attendee::ACTOR_FEDERATED_USERS) {
$inviteSent = $this->backendNotifier->sendRemoteShare((string) $attendee->getId(), $attendee->getAccessToken(), $attendee->getActorId(), $addedBy->getDisplayName(), $addedBy->getCloudId(), 'user', $room, $this->getHighestPermissionAttendee($room));
$inviteSent = $this->backendNotifier->sendRemoteShare((string) $attendee->getId(), $attendee->getAccessToken(), $attendee->getActorId(), $addedBy, 'user', $room, $this->getHighestPermissionAttendee($room));
if (!$inviteSent) {
$this->attendeeMapper->delete($attendee);
throw new CannotReachRemoteException();
Expand Down
1 change: 1 addition & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
<file name="tests/stubs/oc_hooks_emitter.php" />
<file name="tests/stubs/oc_http_client_response.php" />
<file name="tests/stubs/oca_circles.php" />
<file name="tests/stubs/oca_federation_trustedservers.php" />
<file name="tests/stubs/oca_files_events.php" />
<file name="tests/stubs/GuzzleHttp_Exception_ClientException.php" />
<file name="tests/stubs/GuzzleHttp_Exception_ConnectException.php" />
Expand Down
22 changes: 17 additions & 5 deletions tests/php/ConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use OCA\Talk\Tests\php\Mocks\GetTurnServerListener;
use OCA\Talk\Vendor\Firebase\JWT\JWT;
use OCA\Talk\Vendor\Firebase\JWT\Key;
use OCP\AppFramework\Services\IAppConfig;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
Expand All @@ -38,6 +39,8 @@

class ConfigTest extends TestCase {
private function createConfig(IConfig $config) {
/** @var MockObject|IAppConfig $appConfig */
$appConfig = $this->createMock(IAppConfig::class);
/** @var MockObject|ITimeFactory $timeFactory */
$timeFactory = $this->createMock(ITimeFactory::class);
/** @var MockObject|ISecureRandom $secureRandom */
Expand All @@ -51,7 +54,7 @@ private function createConfig(IConfig $config) {
/** @var MockObject|IEventDispatcher $dispatcher */
$dispatcher = $this->createMock(IEventDispatcher::class);

$helper = new Config($config, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
return $helper;
}

Expand Down Expand Up @@ -149,6 +152,8 @@ public function testGenerateTurnSettings() {
->method('getTime')
->willReturn(1479743025);

/** @var MockObject|IAppConfig $appConfig */
$appConfig = $this->createMock(IAppConfig::class);
/** @var MockObject|IGroupManager $groupManager */
$groupManager = $this->createMock(IGroupManager::class);
/** @var MockObject|IUserManager $userManager */
Expand All @@ -165,7 +170,7 @@ public function testGenerateTurnSettings() {
->method('generate')
->with(16)
->willReturn('abcdefghijklmnop');
$helper = new Config($config, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);

//
$settings = $helper->getTurnSettings();
Expand Down Expand Up @@ -217,6 +222,9 @@ public function testGenerateTurnSettingsEvent() {
->with('spreed', 'turn_servers', '')
->willReturn(json_encode([]));

/** @var MockObject|IAppConfig $appConfig */
$appConfig = $this->createMock(IAppConfig::class);

/** @var MockObject|ITimeFactory $timeFactory */
$timeFactory = $this->createMock(ITimeFactory::class);

Expand Down Expand Up @@ -254,7 +262,7 @@ public function testGenerateTurnSettingsEvent() {

$dispatcher->addServiceListener(BeforeTurnServersGetEvent::class, GetTurnServerListener::class);

$helper = new Config($config, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);

$settings = $helper->getTurnSettings();
$this->assertSame($servers, $settings);
Expand Down Expand Up @@ -351,6 +359,8 @@ public static function dataTicketV2Algorithm() {
public function testSignalingTicketV2User(string $algo): void {
/** @var IConfig $config */
$config = \OC::$server->getConfig();
/** @var MockObject|IAppConfig $appConfig */
$appConfig = $this->createMock(IAppConfig::class);
/** @var MockObject|ITimeFactory $timeFactory */
$timeFactory = $this->createMock(ITimeFactory::class);
/** @var MockObject|ISecureRandom $secureRandom */
Expand Down Expand Up @@ -390,7 +400,7 @@ public function testSignalingTicketV2User(string $algo): void {
->method('getDisplayName')
->willReturn('Jane Doe');

$helper = new Config($config, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);

$config->setAppValue('spreed', 'signaling_token_alg', $algo);
// Make sure new keys are generated.
Expand All @@ -415,6 +425,8 @@ public function testSignalingTicketV2User(string $algo): void {
public function testSignalingTicketV2Anonymous(string $algo): void {
/** @var IConfig $config */
$config = \OC::$server->getConfig();
/** @var MockObject|IAppConfig $appConfig */
$appConfig = $this->createMock(IAppConfig::class);
/** @var MockObject|ITimeFactory $timeFactory */
$timeFactory = $this->createMock(ITimeFactory::class);
/** @var MockObject|ISecureRandom $secureRandom */
Expand All @@ -439,7 +451,7 @@ public function testSignalingTicketV2Anonymous(string $algo): void {
->with('')
->willReturn('https://domain.invalid/nextcloud');

$helper = new Config($config, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);

$config->setAppValue('spreed', 'signaling_token_alg', $algo);
// Make sure new keys are generated.
Expand Down
Loading

0 comments on commit 87fe884

Please sign in to comment.