Skip to content

Commit

Permalink
fix(federation): Don't post any messages into proxy conversations
Browse files Browse the repository at this point in the history
Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
nickvergessen committed Mar 6, 2024
1 parent 0064a95 commit 30c01dd
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 0 deletions.
17 changes: 17 additions & 0 deletions lib/Chat/ChatManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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');
Expand All @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
46 changes: 46 additions & 0 deletions lib/Chat/SystemMessage/Listener.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -177,13 +181,22 @@ protected function sendSystemMessageAboutConversationRenamed(RoomModifiedEvent $
return;
}

if ($event->getRoom()->getRemoteServer() !== '') {
return;
}


$this->sendSystemMessage($event->getRoom(), 'conversation_renamed', [
'newName' => $event->getNewValue(),
'oldName' => $event->getOldValue(),
]);
}

protected function sendSystemMessageAboutRoomDescriptionChanges(RoomModifiedEvent $event): void {
if ($event->getRoom()->getRemoteServer() !== '') {
return;
}

if ($event->getNewValue() !== '') {
if ($this->isCreatingNoteToSelf($event)) {
return;
Expand All @@ -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 {
Expand All @@ -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) {
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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 : [];
Expand Down Expand Up @@ -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()]);
Expand All @@ -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()]);
Expand Down
36 changes: 36 additions & 0 deletions lib/Exceptions/MessagingNotAllowedException.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
6 changes: 6 additions & 0 deletions lib/Flow/Operation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions tests/php/Chat/ChatManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
use OCP\Share\IManager;
use OCP\Share\IShare;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;

/**
Expand Down Expand Up @@ -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 {
Expand All @@ -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();
}
Expand Down Expand Up @@ -133,6 +136,7 @@ protected function getManager(array $methods = []): ChatManager {
$this->referenceManager,
$this->rateLimiter,
$this->request,
$this->logger,
])
->onlyMethods($methods)
->getMock();
Expand All @@ -155,6 +159,7 @@ protected function getManager(array $methods = []): ChatManager {
$this->referenceManager,
$this->rateLimiter,
$this->request,
$this->logger,
);
}

Expand Down

0 comments on commit 30c01dd

Please sign in to comment.