Merge pull request #11728 from nextcloud/bugfix/noid/dont-post-system-messages-into-proxy-chats

fix(federation): Don't post any messages into proxy conversations

lib/Chat/ChatManager.php

@@ -31,6 +31,7 @@ use OCA\Talk\Events\BeforeChatMessageSentEvent;
 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\Security\RateLimiting\IRateLimitExceededException;
 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 @@ class ChatManager {
 		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 @@ class ChatManager {
 	 *             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 @@ class ChatManager {
 		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 @@ class ChatManager {
 	 *
 	 * @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);

lib/Chat/SystemMessage/Listener.php

@@ -164,6 +164,10 @@ class Listener implements IEventListener {
 	}
 
 	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 @@ class Listener implements IEventListener {
 			return;
 		}
 
+		if ($event->getRoom()->getRemoteServer() !== '') {
+			return;
+		}
+
+
 		$this->sendSystemMessage($event->getRoom(), 'conversation_renamed', [
 			'newName' => $event->getNewValue(),
 			'oldName' => $event->getOldValue(),
@@ -184,6 +193,10 @@ class Listener implements IEventListener {
 	}
 
 	protected function sendSystemMessageAboutRoomDescriptionChanges(RoomModifiedEvent $event): void {
+		if ($event->getRoom()->getRemoteServer() !== '') {
+			return;
+		}
+
 		if ($event->getNewValue() !== '') {
 			if ($this->isCreatingNoteToSelf($event)) {
 				return;
@@ -198,6 +211,10 @@ class Listener implements IEventListener {
 	}
 
 	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 @@ class Listener implements IEventListener {
 			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 @@ class Listener implements IEventListener {
 			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 @@ class Listener implements IEventListener {
 			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 @@ class Listener implements IEventListener {
 			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 @@ class Listener implements IEventListener {
 			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 @@ class Listener implements IEventListener {
 	}
 
 	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 @@ class Listener implements IEventListener {
 	}
 
 	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()]);

lib/Exceptions/MessagingNotAllowedException.php

@@ -0,0 +1,36 @@
+<?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\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);
+	}
+}

lib/Flow/Operation.php

@@ -113,6 +113,11 @@ class Operation implements IOperation {
 					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

tests/php/Chat/ChatManagerTest.php

@@ -50,6 +50,7 @@ use OCP\Share\Exceptions\ShareNotFound;
 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 @@ class ChatManagerTest extends TestCase {
 		$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 @@ class ChatManagerTest extends TestCase {
 					$this->referenceManager,
 					$this->rateLimiter,
 					$this->request,
+					$this->logger,
 				])
 				->onlyMethods($methods)
 				->getMock();
@@ -155,6 +159,7 @@ class ChatManagerTest extends TestCase {
 			$this->referenceManager,
 			$this->rateLimiter,
 			$this->request,
+			$this->logger,
 		);
 	}