summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoas Schilling <213943+nickvergessen@users.noreply.github.com>2024-03-14 16:17:49 +0100
committerGitHub <noreply@github.com>2024-03-14 16:17:49 +0100
commitb197842ae23df0f5de7107a4b3ce9cfe5f4ab3db (patch)
tree60521a865427227829958f0db965c81357575b52
parent8b8a5cd017626928eceac9fbfd164e641280b419 (diff)
parente71ff6efc6ad1a58d814f4b37d75c24be9e8c57f (diff)
Merge pull request #11799 from nextcloud/bugfix/noid/handle-federation-configs-on-invite
fix(federation): Handle federation configs when inviting instead of g…
-rw-r--r--lib/Controller/RoomController.php8
-rw-r--r--lib/Federation/BackendNotifier.php52
-rw-r--r--lib/Federation/FederationManager.php14
-rw-r--r--lib/Federation/RestrictionValidator.php89
-rw-r--r--tests/php/Federation/FederationTest.php22
5 files changed, 135 insertions, 50 deletions
diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php
index e9ba26e55..466478da3 100644
--- a/lib/Controller/RoomController.php
+++ b/lib/Controller/RoomController.php
@@ -1089,6 +1089,7 @@ class RoomController extends AEnvironmentAwareController {
}
}
+ /** @var IUser $addedBy */
$addedBy = $this->userManager->get($this->userId);
// list of participants to attempt adding,
@@ -1153,7 +1154,12 @@ class RoomController extends AEnvironmentAwareController {
$this->logger->error($e->getMessage(), [
'exception' => $e,
]);
- return new DataResponse([], Http::STATUS_BAD_REQUEST);
+ return new DataResponse(['error' => 'cloudId'], Http::STATUS_BAD_REQUEST);
+ }
+ try {
+ $this->federationManager->isAllowedToInvite($addedBy, $newUser);
+ } catch (\InvalidArgumentException $e) {
+ return new DataResponse(['error' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
}
$participantsToAdd[] = [
diff --git a/lib/Federation/BackendNotifier.php b/lib/Federation/BackendNotifier.php
index c45bedff6..a70d8e4be 100644
--- a/lib/Federation/BackendNotifier.php
+++ b/lib/Federation/BackendNotifier.php
@@ -26,27 +26,23 @@ declare(strict_types=1);
namespace OCA\Talk\Federation;
use OCA\FederatedFileSharing\AddressHandler;
-use OCA\Federation\TrustedServers;
-use OCA\Talk\Config;
use OCA\Talk\Exceptions\RoomHasNoModeratorException;
use OCA\Talk\Model\Attendee;
use OCA\Talk\Model\RetryNotification;
use OCA\Talk\Model\RetryNotificationMapper;
use OCA\Talk\Room;
-use OCP\App\IAppManager;
use OCP\AppFramework\Http;
-use OCP\AppFramework\Services\IAppConfig;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\DB\Exception;
use OCP\Federation\ICloudFederationFactory;
use OCP\Federation\ICloudFederationNotification;
use OCP\Federation\ICloudFederationProviderManager;
+use OCP\Federation\ICloudIdManager;
use OCP\HintException;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
use OCP\OCM\Exceptions\OCMProviderException;
-use OCP\Server;
use Psr\Log\LoggerInterface;
use SensitiveParameter;
@@ -59,11 +55,10 @@ class BackendNotifier {
private ICloudFederationProviderManager $federationProviderManager,
private IUserManager $userManager,
private IURLGenerator $url,
- private IAppManager $appManager,
- private Config $talkConfig,
- private IAppConfig $appConfig,
private RetryNotificationMapper $retryNotificationMapper,
private ITimeFactory $timeFactory,
+ private ICloudIdManager $cloudIdManager,
+ private RestrictionValidator $restrictionValidator,
) {
}
@@ -84,52 +79,25 @@ class BackendNotifier {
Room $room,
Attendee $roomOwnerAttendee,
): bool {
- [$user, $remote] = $this->addressHandler->splitUserRemote($shareWith);
+ $invitedCloudId = $this->cloudIdManager->resolveCloudId($shareWith);
$roomName = $room->getName();
$roomType = $room->getType();
$roomToken = $room->getToken();
- if (!($user && $remote)) {
- $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());
+ try {
+ $this->restrictionValidator->isAllowedToInvite($sharedBy, $invitedCloudId);
+ } catch (\InvalidArgumentException) {
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 $roomOwner */
$roomOwner = $this->userManager->get($roomOwnerAttendee->getActorId());
- $invitedCloudId = $user . '@' . $remote;
- $remote = $this->prepareRemoteUrl($remote);
+ $remote = $this->prepareRemoteUrl($invitedCloudId->getRemote());
$share = $this->cloudFederationFactory->getCloudFederationShare(
- $user . '@' . $remote,
+ $invitedCloudId->getUser() . '@' . $remote,
$roomToken,
'',
$providerId,
@@ -144,7 +112,7 @@ class BackendNotifier {
// Put room name info in the share
$protocol = $share->getProtocol();
- $protocol['invitedCloudId'] = $invitedCloudId;
+ $protocol['invitedCloudId'] = $invitedCloudId->getId();
$protocol['roomName'] = $roomName;
$protocol['roomType'] = $roomType;
$protocol['name'] = FederationManager::TALK_PROTOCOL_NAME;
diff --git a/lib/Federation/FederationManager.php b/lib/Federation/FederationManager.php
index d5753aad8..4d1e7a15a 100644
--- a/lib/Federation/FederationManager.php
+++ b/lib/Federation/FederationManager.php
@@ -39,6 +39,7 @@ use OCA\Talk\Participant;
use OCA\Talk\Room;
use OCA\Talk\Service\ParticipantService;
use OCP\AppFramework\Db\DoesNotExistException;
+use OCP\Federation\ICloudId;
use OCP\IUser;
use OCP\Notification\IManager;
use SensitiveParameter;
@@ -67,9 +68,22 @@ class FederationManager {
private InvitationMapper $invitationMapper,
private BackendNotifier $backendNotifier,
private IManager $notificationManager,
+ private RestrictionValidator $restrictionValidator,
) {
}
+ /**
+ * Check if $sharedBy is allowed to invite $shareWith
+ *
+ * @throws \InvalidArgumentException
+ */
+ public function isAllowedToInvite(
+ IUser $user,
+ ICloudId $cloudIdToInvite,
+ ): void {
+ $this->restrictionValidator->isAllowedToInvite($user, $cloudIdToInvite);
+ }
+
public function addRemoteRoom(
IUser $user,
int $remoteAttendeeId,
diff --git a/lib/Federation/RestrictionValidator.php b/lib/Federation/RestrictionValidator.php
new file mode 100644
index 000000000..1bdfbcf16
--- /dev/null
+++ b/lib/Federation/RestrictionValidator.php
@@ -0,0 +1,89 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2024 Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Talk\Federation;
+
+use OCA\FederatedFileSharing\AddressHandler;
+use OCA\Federation\TrustedServers;
+use OCA\Talk\Config;
+use OCP\App\IAppManager;
+use OCP\AppFramework\Services\IAppConfig;
+use OCP\Federation\ICloudId;
+use OCP\IUser;
+use OCP\Server;
+use Psr\Log\LoggerInterface;
+
+class RestrictionValidator {
+ public function __construct(
+ private AddressHandler $addressHandler,
+ private IAppManager $appManager,
+ private Config $talkConfig,
+ private IAppConfig $appConfig,
+ private LoggerInterface $logger,
+ ) {
+ }
+
+ /**
+ * Check if $sharedBy is allowed to invite $shareWith
+ *
+ * @throws \InvalidArgumentException
+ */
+ public function isAllowedToInvite(
+ IUser $user,
+ ICloudId $cloudIdToInvite,
+ ): void {
+ if (!($cloudIdToInvite->getUser() && $cloudIdToInvite->getRemote())) {
+ $this->logger->debug('Could not share conversation as the recipient is invalid: ' . $cloudIdToInvite->getId());
+ throw new \InvalidArgumentException('cloudId');
+ }
+
+ if (!$this->appConfig->getAppValueBool('federation_outgoing_enabled', true)) {
+ $this->logger->debug('Could not share conversation as outgoing federation is disabled');
+ throw new \InvalidArgumentException('outgoing');
+ }
+
+ if (!$this->talkConfig->isFederationEnabledForUserId($user)) {
+ $this->logger->debug('Talk federation not allowed for user ' . $user->getUID());
+ throw new \InvalidArgumentException('federation');
+ }
+
+ 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');
+ throw new \InvalidArgumentException('trusted_servers');
+ }
+
+ $trustedServers = Server::get(TrustedServers::class);
+ $serverUrl = $this->addressHandler->removeProtocolFromUrl($cloudIdToInvite->getRemote());
+ if (!$trustedServers->isTrustedServer($serverUrl)) {
+ $this->logger->warning(
+ 'Tried to send Talk federation invite to untrusted server {serverUrl}',
+ ['serverUrl' => $serverUrl]
+ );
+ throw new \InvalidArgumentException('trusted_servers');
+ }
+ }
+ }
+}
diff --git a/tests/php/Federation/FederationTest.php b/tests/php/Federation/FederationTest.php
index 10e4c1c93..94e8690c0 100644
--- a/tests/php/Federation/FederationTest.php
+++ b/tests/php/Federation/FederationTest.php
@@ -29,6 +29,7 @@ use OCA\Talk\Federation\BackendNotifier;
use OCA\Talk\Federation\CloudFederationProviderTalk;
use OCA\Talk\Federation\FederationManager;
use OCA\Talk\Federation\Proxy\TalkV1\UserConverter;
+use OCA\Talk\Federation\RestrictionValidator;
use OCA\Talk\Manager;
use OCA\Talk\Model\Attendee;
use OCA\Talk\Model\AttendeeMapper;
@@ -49,6 +50,7 @@ use OCP\Federation\ICloudFederationFactory;
use OCP\Federation\ICloudFederationNotification;
use OCP\Federation\ICloudFederationProviderManager;
use OCP\Federation\ICloudFederationShare;
+use OCP\Federation\ICloudId;
use OCP\Federation\ICloudIdManager;
use OCP\Http\Client\IResponse;
use OCP\ICacheFactory;
@@ -104,6 +106,7 @@ class FederationTest extends TestCase {
protected ICacheFactory|MockObject $cacheFactory;
protected RetryNotificationMapper|MockObject $retryNotificationMapper;
protected ITimeFactory|MockObject $timeFactory;
+ protected RestrictionValidator|MockObject $restrictionValidator;
public function setUp(): void {
parent::setUp();
@@ -123,6 +126,7 @@ class FederationTest extends TestCase {
$this->cacheFactory = $this->createMock(ICacheFactory::class);
$this->retryNotificationMapper = $this->createMock(RetryNotificationMapper::class);
$this->timeFactory = $this->createMock(ITimeFactory::class);
+ $this->restrictionValidator = $this->createMock(RestrictionValidator::class);
$this->backendNotifier = new BackendNotifier(
$this->cloudFederationFactory,
@@ -131,11 +135,10 @@ class FederationTest extends TestCase {
$this->cloudFederationProviderManager,
$this->userManager,
$this->url,
- $this->appManager,
- $this->config,
- $this->appConfig,
$this->retryNotificationMapper,
$this->timeFactory,
+ $this->cloudIdManager,
+ $this->restrictionValidator,
);
$this->federationManager = $this->createMock(FederationManager::class);
@@ -170,7 +173,6 @@ class FederationTest extends TestCase {
$cloudShare = $this->createMock(ICloudFederationShare::class);
$providerId = '3';
- $roomId = 5;
$token = 'abcdefghijklmno';
$shareWith = 'test@https://remote.test.local';
$name = 'abcdefgh';
@@ -246,10 +248,16 @@ class FederationTest extends TestCase {
->method('sendCloudShare')
->with($cloudShare);
- $this->addressHandler->expects($this->once())
- ->method('splitUserRemote')
+ $cloudId = $this->createMock(ICloudId::class);
+ $cloudId->method('getRemote')
+ ->willReturn('remote.test.local');
+ $cloudId->method('getUser')
+ ->willReturn('test');
+
+ $this->cloudIdManager->expects($this->once())
+ ->method('resolveCloudId')
->with($shareWith)
- ->willReturn(['test', 'remote.test.local']);
+ ->willReturn($cloudId);
$this->appConfig->method('getAppValueBool')
->willReturnMap([