From 30c01ddcb710f3f5c66a323587254a53bfa6815e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 6 Mar 2024 10:16:07 +0100 Subject: [PATCH] fix(federation): Don't post any messages into proxy conversations Signed-off-by: Joas Schilling --- lib/Chat/ChatManager.php | 17 +++++++ lib/Chat/SystemMessage/Listener.php | 46 +++++++++++++++++++ .../MessagingNotAllowedException.php | 36 +++++++++++++++ lib/Flow/Operation.php | 6 +++ tests/php/Chat/ChatManagerTest.php | 5 ++ 5 files changed, 110 insertions(+) create mode 100644 lib/Exceptions/MessagingNotAllowedException.php diff --git a/lib/Chat/ChatManager.php b/lib/Chat/ChatManager.php index fc145c8858d7..7a731213da35 100644 --- a/lib/Chat/ChatManager.php +++ b/lib/Chat/ChatManager.php @@ -31,6 +31,7 @@ use OCA\Talk\Events\BeforeSystemMessageSentEvent; use OCA\Talk\Events\ChatMessageSentEvent; use OCA\Talk\Events\SystemMessageSentEvent; +use OCA\Talk\Exceptions\MessagingNotAllowedException; use OCA\Talk\Exceptions\ParticipantNotFoundException; use OCA\Talk\Model\Attendee; use OCA\Talk\Model\Poll; @@ -59,6 +60,7 @@ use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IShare; +use Psr\Log\LoggerInterface; /** * Basic polling chat manager. @@ -105,6 +107,7 @@ public function __construct( protected IReferenceManager $referenceManager, protected ILimiter $rateLimiter, protected IRequest $request, + protected LoggerInterface $logger, ) { $this->cache = $cacheFactory->createDistributed('talk/lastmsgid'); $this->unreadCountCache = $cacheFactory->createDistributed('talk/unreadcount'); @@ -118,6 +121,7 @@ public function __construct( * message and last activity update until the last entry was created * and then update with those values. * This will replace O(n) with 1 database update. + * @throws MessagingNotAllowedException */ public function addSystemMessage( Room $chat, @@ -131,6 +135,12 @@ public function addSystemMessage( bool $shouldSkipLastMessageUpdate = false, bool $silent = false, ): IComment { + if ($chat->getRemoteServer() !== '') { + $e = new MessagingNotAllowedException(); + $this->logger->error('Attempt to post system message into proxy conversation', ['exception' => $e]); + throw $e; + } + $comment = $this->commentsManager->create($actorType, $actorId, 'chat', (string) $chat->getId()); $comment->setMessage($message, self::MAX_CHAT_LENGTH); $comment->setCreationDateTime($creationDateTime); @@ -272,8 +282,15 @@ public function addChangelogMessage(Room $chat, string $message): IComment { * * @throws IRateLimitExceededException Only when $rateLimitGuestMentions is true and the author is a guest participant * @throws MessageTooLongException + * @throws MessagingNotAllowedException */ public function sendMessage(Room $chat, ?Participant $participant, string $actorType, string $actorId, string $message, \DateTime $creationDateTime, ?IComment $replyTo = null, string $referenceId = '', bool $silent = false, bool $rateLimitGuestMentions = true): IComment { + if ($chat->getRemoteServer() !== '') { + $e = new MessagingNotAllowedException(); + $this->logger->error('Attempt to post system message into proxy conversation', ['exception' => $e]); + throw $e; + } + $comment = $this->commentsManager->create($actorType, $actorId, 'chat', (string) $chat->getId()); $comment->setMessage($message, self::MAX_CHAT_LENGTH); $comment->setCreationDateTime($creationDateTime); diff --git a/lib/Chat/SystemMessage/Listener.php b/lib/Chat/SystemMessage/Listener.php index 52513ba74ed0..ddd680ab22db 100644 --- a/lib/Chat/SystemMessage/Listener.php +++ b/lib/Chat/SystemMessage/Listener.php @@ -164,6 +164,10 @@ protected function sendSystemMessageAboutCallLeft(ParticipantModifiedEvent $even } protected function sendSystemMessageAboutConversationCreated(RoomCreatedEvent $event): void { + if ($event->getRoom()->getRemoteServer() !== '') { + return; + } + if ($event->getRoom()->getType() === Room::TYPE_CHANGELOG || $this->isCreatingNoteToSelfAutomatically($event)) { $this->sendSystemMessage($event->getRoom(), 'conversation_created', forceSystemAsActor: true); } else { @@ -177,6 +181,11 @@ protected function sendSystemMessageAboutConversationRenamed(RoomModifiedEvent $ return; } + if ($event->getRoom()->getRemoteServer() !== '') { + return; + } + + $this->sendSystemMessage($event->getRoom(), 'conversation_renamed', [ 'newName' => $event->getNewValue(), 'oldName' => $event->getOldValue(), @@ -184,6 +193,10 @@ protected function sendSystemMessageAboutConversationRenamed(RoomModifiedEvent $ } protected function sendSystemMessageAboutRoomDescriptionChanges(RoomModifiedEvent $event): void { + if ($event->getRoom()->getRemoteServer() !== '') { + return; + } + if ($event->getNewValue() !== '') { if ($this->isCreatingNoteToSelf($event)) { return; @@ -198,6 +211,10 @@ protected function sendSystemMessageAboutRoomDescriptionChanges(RoomModifiedEven } protected function sendSystemMessageAboutRoomPassword(RoomModifiedEvent $event): void { + if ($event->getRoom()->getRemoteServer() !== '') { + return; + } + if ($event->getNewValue() !== '') { $this->sendSystemMessage($event->getRoom(), 'password_set'); } else { @@ -224,6 +241,10 @@ protected function sendSystemReadOnlyMessage(RoomModifiedEvent $event): void { return; } + if ($room->getRemoteServer() !== '') { + return; + } + if ($event->getNewValue() === Room::READ_ONLY) { $this->sendSystemMessage($room, 'read_only'); } elseif ($event->getNewValue() === Room::READ_WRITE) { @@ -272,6 +293,10 @@ protected function addSystemMessageUserAdded(AttendeesAddedEvent $event, Attende return; } + if ($room->getRemoteServer() !== '') { + return; + } + $userJoinedFileRoom = $room->getObjectType() === Room::OBJECT_TYPE_FILE && $attendee->getParticipantType() !== Participant::USER_SELF_JOINED; // add a message "X joined the conversation", whenever user $userId: @@ -305,6 +330,10 @@ protected function sendSystemMessageUserRemoved(AttendeeRemovedEvent $event): vo return; } + if ($room->getRemoteServer() !== '') { + return; + } + if ($event->getReason() === AAttendeeRemovedEvent::REASON_LEFT && $event->getAttendee()->getParticipantType() === Participant::USER_SELF_JOINED) { // Self-joined user closes the tab/window or leaves via the menu @@ -322,6 +351,10 @@ public function sendSystemMessageAboutPromoteOrDemoteModerator(ParticipantModifi return; } + if ($event->getRoom()->getRemoteServer() !== '') { + return; + } + if ($event->getNewValue() === Participant::MODERATOR) { $this->sendSystemMessage($room, 'moderator_promoted', ['user' => $attendee->getActorId()]); } elseif ($event->getNewValue() === Participant::USER) { @@ -367,6 +400,11 @@ protected function fixMimeTypeOfVoiceMessage(ShareCreatedEvent|BeforeDuplicateSh return; } $room = $this->manager->getRoomByToken($share->getSharedWith()); + if ($room->getRemoteServer() !== '') { + // FIXME this should be blocked up front + return; + } + $metaData = $this->request->getParam('talkMetaData') ?? ''; $metaData = json_decode($metaData, true); $metaData = is_array($metaData) ? $metaData : []; @@ -397,6 +435,10 @@ protected function fixMimeTypeOfVoiceMessage(ShareCreatedEvent|BeforeDuplicateSh } protected function attendeesAddedEvent(AttendeesAddedEvent $event): void { + if ($event->getRoom()->getRemoteServer() !== '') { + return; + } + foreach ($event->getAttendees() as $attendee) { if ($attendee->getActorType() === Attendee::ACTOR_GROUPS) { $this->sendSystemMessage($event->getRoom(), 'group_added', ['group' => $attendee->getActorId()]); @@ -413,6 +455,10 @@ protected function attendeesAddedEvent(AttendeesAddedEvent $event): void { } protected function attendeesRemovedEvent(AttendeesRemovedEvent $event): void { + if ($event->getRoom()->getRemoteServer() !== '') { + return; + } + foreach ($event->getAttendees() as $attendee) { if ($attendee->getActorType() === Attendee::ACTOR_GROUPS) { $this->sendSystemMessage($event->getRoom(), 'group_removed', ['group' => $attendee->getActorId()]); diff --git a/lib/Exceptions/MessagingNotAllowedException.php b/lib/Exceptions/MessagingNotAllowedException.php new file mode 100644 index 000000000000..99c442861fc0 --- /dev/null +++ b/lib/Exceptions/MessagingNotAllowedException.php @@ -0,0 +1,36 @@ + + * + * @author Joas Schilling + * + * @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 . + * + */ + + +namespace OCA\Talk\Exceptions; + +/** + * Thrown when a room is a proxy room and a message is trying to be posted. + */ +class MessagingNotAllowedException extends \OutOfBoundsException { + public function __construct(?\Throwable $previous = null) { + parent::__construct('Messaging is not allowed in proxy rooms', 1, $previous); + } +} diff --git a/lib/Flow/Operation.php b/lib/Flow/Operation.php index caf0753a6cee..8dc5fdd6d46b 100644 --- a/lib/Flow/Operation.php +++ b/lib/Flow/Operation.php @@ -26,6 +26,7 @@ namespace OCA\Talk\Flow; use OCA\Talk\Chat\ChatManager; +use OCA\Talk\Exceptions\MessagingNotAllowedException; use OCA\Talk\Exceptions\ParticipantNotFoundException; use OCA\Talk\Exceptions\RoomNotFoundException; use OCA\Talk\Manager as TalkManager; @@ -113,6 +114,11 @@ public function onEvent(string $eventName, Event $event, IRuleMatcher $ruleMatch continue; } + if ($room->getRemoteServer() !== '') { + // Ignore conversation because it is a proxy conversation + continue; + } + $participant = $this->participantService->getParticipant($room, $uid, false); if (!($participant->getPermissions() & Attendee::PERMISSIONS_CHAT)) { // Ignore conversation because the user has no permissions diff --git a/tests/php/Chat/ChatManagerTest.php b/tests/php/Chat/ChatManagerTest.php index 83463c7db32a..b8defc616b41 100644 --- a/tests/php/Chat/ChatManagerTest.php +++ b/tests/php/Chat/ChatManagerTest.php @@ -50,6 +50,7 @@ use OCP\Share\IManager; use OCP\Share\IShare; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; use Test\TestCase; /** @@ -84,6 +85,7 @@ class ChatManagerTest extends TestCase { protected $rateLimiter; /** @var IRequest|MockObject */ protected $request; + protected LoggerInterface|MockObject $logger; protected ?ChatManager $chatManager = null; public function setUp(): void { @@ -103,6 +105,7 @@ public function setUp(): void { $this->referenceManager = $this->createMock(IReferenceManager::class); $this->rateLimiter = $this->createMock(ILimiter::class); $this->request = $this->createMock(IRequest::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->chatManager = $this->getManager(); } @@ -133,6 +136,7 @@ protected function getManager(array $methods = []): ChatManager { $this->referenceManager, $this->rateLimiter, $this->request, + $this->logger, ]) ->onlyMethods($methods) ->getMock(); @@ -155,6 +159,7 @@ protected function getManager(array $methods = []): ChatManager { $this->referenceManager, $this->rateLimiter, $this->request, + $this->logger, ); }