diff options
author | Joas Schilling <213943+nickvergessen@users.noreply.github.com> | 2024-03-14 16:17:49 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-03-14 16:17:49 +0100 |
commit | b197842ae23df0f5de7107a4b3ce9cfe5f4ab3db (patch) | |
tree | 60521a865427227829958f0db965c81357575b52 | |
parent | 8b8a5cd017626928eceac9fbfd164e641280b419 (diff) | |
parent | e71ff6efc6ad1a58d814f4b37d75c24be9e8c57f (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.php | 8 | ||||
-rw-r--r-- | lib/Federation/BackendNotifier.php | 52 | ||||
-rw-r--r-- | lib/Federation/FederationManager.php | 14 | ||||
-rw-r--r-- | lib/Federation/RestrictionValidator.php | 89 | ||||
-rw-r--r-- | tests/php/Federation/FederationTest.php | 22 |
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([ |