Merge pull request #13150 from nextcloud/bugfix/13048/correctly-handl…

…e-deline-in-system-message fix(federation): Correctly handle decline in system messages
nextcloud · Aug 27, 2024 · 5fbbbea · 5fbbbea
2 parents e225c7e + 28238bf
commit 5fbbbea
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 24 deletions.
diff --git a/lib/Chat/Parser/SystemMessage.php b/lib/Chat/Parser/SystemMessage.php
@@ -351,9 +351,7 @@ protected function parseMessage(Message $chatMessage): void {
  $parsedMessage = $this->l->t('{actor} invited {federated_user}');
  if ($currentUserIsActor) {
  $parsedMessage = $this->l->t('You invited {federated_user}');
- if (isset($parsedParameters['federated_user']['server'], $parsedParameters['actor']['server'])
- && $parsedParameters['federated_user']['id'] === $parsedParameters['actor']['id']
- && $parsedParameters['federated_user']['server'] === $parsedParameters['actor']['server']) {
+ if ($this->isFederatedUserThemselvesActor($parsedParameters['federated_user'], $parsedParameters['actor'])) {
  $parsedMessage = $this->l->t('You accepted the invitation');
  }
  } elseif ($this->isCurrentParticipantChangedUser($currentActorType, $currentActorId, $parsedParameters['federated_user'])) {
@@ -363,26 +361,25 @@ protected function parseMessage(Message $chatMessage): void {
  }
  } elseif ($cliIsActor) {
  $parsedMessage = $this->l->t('An administrator invited {federated_user}');
- } elseif (isset($parsedParameters['federated_user']['server'], $parsedParameters['actor']['server'])
- && $parsedParameters['federated_user']['id'] === $parsedParameters['actor']['id']
- && $parsedParameters['federated_user']['server'] === $parsedParameters['actor']['server']) {
+ } elseif ($this->isFederatedUserThemselvesActor($parsedParameters['federated_user'], $parsedParameters['actor'])) {
  $parsedMessage = $this->l->t('{federated_user} accepted the invitation');
  }
  } elseif ($message === 'federated_user_removed') {
  $parsedParameters['federated_user'] = $this->getRemoteUser($room, $parameters['federated_user']);
  $parsedMessage = $this->l->t('{actor} removed {federated_user}');
  if ($currentUserIsActor) {
  $parsedMessage = $this->l->t('You removed {federated_user}');
+ if ($this->isFederatedUserThemselvesActor($parsedParameters['federated_user'], $parsedParameters['actor'])) {
+ $parsedMessage = $this->l->t('You declined the invitation');
+ }
  } elseif ($this->isCurrentParticipantChangedUser($currentActorType, $currentActorId, $parsedParameters['federated_user'])) {
  $parsedMessage = $this->l->t('{actor} removed you');
  if ($cliIsActor) {
  $parsedMessage = $this->l->t('An administrator removed you');
  }
  } elseif ($cliIsActor) {
  $parsedMessage = $this->l->t('An administrator removed {federated_user}');
- } elseif (isset($parsedParameters['federated_user']['server'], $parsedParameters['actor']['server'])
- && $parsedParameters['federated_user']['id'] === $parsedParameters['actor']['id']
- && $parsedParameters['federated_user']['server'] === $parsedParameters['actor']['server']) {
+ } elseif ($this->isFederatedUserThemselvesActor($parsedParameters['federated_user'], $parsedParameters['actor'])) {
  $parsedMessage = $this->l->t('{federated_user} declined the invitation');
  }
  } elseif ($message === 'group_added') {
@@ -857,7 +854,13 @@ protected function isCurrentParticipantChangedUser(?string $currentActorType, ?s
  && $this->currentFederatedUserDetails['server'] === $parameter['server'];
  }
 
- return $currentActorType === Attendee::ACTOR_USERS && $parameter['type'] === 'user' && $currentActorId === $parameter['id'];
+ return $currentActorType === Attendee::ACTOR_USERS && $parameter['type'] === 'user' && $currentActorId === $parameter['id'] && empty($parameter['server']);
+ }
+
+ protected function isFederatedUserThemselvesActor(array $federated, array $actor): bool {
+ return isset($federated['server'], $actor['server'])
+ && $federated['id'] === $actor['id']
+ && $federated['server'] === $actor['server'];
  }
 
  protected function getActorFromComment(Room $room, IComment $comment): array {

diff --git a/tests/integration/features/federation/chat.feature b/tests/integration/features/federation/chat.feature
@@ -158,7 +158,7 @@ Feature: federation/chat
  | LOCAL::room | room | 2 | LOCAL | room |
  Then user "participant1" is participant of the following unordered rooms (v4)
  | id | name | type | remoteServer | remoteToken | lastMessage |
- | room | room | 2 | | | {actor} invited you |
+ | room | room | 2 | | | {federated_user} accepted the invitation |
  | LOCAL::room | room | 2 | LOCAL | room | {federated_user} accepted the invitation |
  And user "participant1" sends message "Message 1" to room "room" with 201
  Then user "participant1" is participant of the following unordered rooms (v4)

diff --git a/tests/php/Chat/Parser/SystemMessageTest.php b/tests/php/Chat/Parser/SystemMessageTest.php
@@ -23,6 +23,7 @@
 use OCA\Talk\Share\Helper\FilesMetadataCache;
 use OCA\Talk\Share\RoomShareProvider;
 use OCP\Comments\IComment;
+use OCP\Federation\ICloudId;
 use OCP\Federation\ICloudIdManager;
 use OCP\Files\Folder;
 use OCP\Files\InvalidPathException;
@@ -431,18 +432,52 @@ public static function dataParseMessage(): array {
  'The shared location is malformed',
  [],
  ],
+ ['federated_user_added', ['federated_user' => 'actor@federated.tld'], 'actor',
+ 'You invited {federated_user}',
+ ['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
+ ],
+ ['federated_user_added', ['federated_user' => 'actor@federated.tld'], 'user',
+ '{actor} invited {federated_user}',
+ ['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
+ ],
+ ['federated_user_added', ['federated_user' => 'actor@federated.tld'], 'fed::actor@federated.tld',
+ '{actor} invited you',
+ ['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
+ ],
+ ['federated_user_added', ['federated_user' => 'actor@federated.tld'], 'fed::actor@federated.tld',
+ 'You accepted the invitation',
+ ['actor' => ['id' => 'actor', 'type' => 'user', 'server' => 'federated.tld'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
+ $federatedActor = true,
+ ],
+ ['federated_user_removed', ['federated_user' => 'actor@federated.tld'], 'actor',
+ 'You removed {federated_user}',
+ ['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
+ ],
+ ['federated_user_removed', ['federated_user' => 'actor@federated.tld'], 'user',
+ '{actor} removed {federated_user}',
+ ['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
+ ],
+ ['federated_user_removed', ['federated_user' => 'actor@federated.tld'], 'fed::actor@federated.tld',
+ '{actor} removed you',
+ ['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
+ ],
+ ['federated_user_removed', ['federated_user' => 'actor@federated.tld'], 'fed::actor@federated.tld',
+ 'You declined the invitation',
+ ['actor' => ['id' => 'actor', 'type' => 'user', 'server' => 'federated.tld'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
+ $federatedActor = true,
+ ],
  ];
  }
 
  /**
  * @dataProvider dataParseMessage
  */
- public function testParseMessage(string $message, array $parameters, ?string $recipientId, string $expectedMessage, array $expectedParameters): void {
+ public function testParseMessage(string $message, array $parameters, ?string $recipientId, string $expectedMessage, array $expectedParameters, bool $federatedActor = false): void {
  /** @var Participant&MockObject $participant */
  $participant = $this->createMock(Participant::class);
  if ($recipientId === null) {
  $participant = null;
- } elseif ($recipientId && strpos($recipientId, 'guest::') !== false) {
+ } elseif ($recipientId && str_starts_with($recipientId, 'guest::')) {
  $attendee = Attendee::fromRow([
  'actor_type' => 'guests',
  'actor_id' => substr($recipientId, strlen('guest::')),
@@ -459,6 +494,25 @@ public function testParseMessage(string $message, array $parameters, ?string $re
  $participant->expects($this->any())
  ->method('getSession')
  ->willReturn($session);
+ } elseif ($recipientId && str_starts_with($recipientId, 'fed::')) {
+ $attendee = Attendee::fromRow([
+ 'actor_type' => 'federated_users',
+ 'actor_id' => substr($recipientId, strlen('fed::')),
+ ]);
+ $session = Session::fromRow([
+ 'session_id' => substr($recipientId, strlen('fed::')),
+ ]);
+ $participant->method('isGuest')
+ ->willReturn(false);
+ $participant->method('getAttendee')
+ ->willReturn($attendee);
+ $participant->method('getSession')
+ ->willReturn($session);
+
+ $this->federationAuthenticator->method('isFederationRequest')
+ ->willReturn(true);
+ $this->federationAuthenticator->method('getCloudId')
+ ->willReturn(substr($recipientId, strlen('fed::')));
  } else {
  $participant->expects($this->atLeastOnce())
  ->method('isGuest')
@@ -478,30 +532,59 @@ public function testParseMessage(string $message, array $parameters, ?string $re
 
  /** @var IComment&MockObject $comment */
  $comment = $this->createMock(IComment::class);
- if ($recipientId && strpos($recipientId, 'guest::') !== false) {
+ if ($recipientId && str_starts_with($recipientId, 'guest::')) {
  $comment->method('getActorType')
  ->willReturn('guests');
  $comment->method('getActorId')
  ->willReturn(substr($recipientId, strlen('guest::')));
+ } elseif ($recipientId && str_starts_with($recipientId, 'fed::')) {
+ $comment->method('getActorType')
+ ->willReturn('federated_users');
+ $comment->method('getActorId')
+ ->willReturn(substr($recipientId, strlen('fed::')));
  } else {
  $comment->method('getActorType')
  ->willReturn('users');
  $comment->method('getActorId')
  ->willReturn($recipientId);
  }
 
+ $this->cloudIdManager->method('resolveCloudId')
+ ->willReturnCallback(function (string $id): ICloudId {
+ [$user, $remote] = explode('@', $id);
+ $cloudId = $this->createMock(ICloudId::class);
+ $cloudId->method('getUser')
+ ->willReturn($user);
+ $cloudId->method('getRemote')
+ ->willReturn($remote);
+ $cloudId->method('getDisplayId')
+ ->willReturn($id);
+ return $cloudId;
+ });
+
  /** @var Room&MockObject $room */
  $room = $this->createMock(Room::class);
 
- $parser = $this->getParser(['getActorFromComment', 'getUser', 'getGroup', 'getGuest', 'parseCall', 'getFileFromShare']);
+ $parser = $this->getParser(['getActorFromComment', 'getUser', 'getRemoteUser', 'getGroup', 'getGuest', 'parseCall', 'getFileFromShare']);
  $parser->expects($this->once())
  ->method('getActorFromComment')
  ->with($room, $comment)
- ->willReturn(['id' => 'actor', 'type' => 'user']);
+ ->willReturnCallback(function () use ($federatedActor): array {
+ if ($federatedActor) {
+ return ['id' => 'actor', 'type' => 'user', 'server' => 'federated.tld'];
+ }
+ return ['id' => 'actor', 'type' => 'user'];
+ });
  $parser->expects($this->any())
  ->method('getUser')
  ->with($parameters['user'] ?? 'user')
  ->willReturn(['id' => $parameters['user'] ?? 'user', 'type' => 'user']);
+ $parser->method('getRemoteUser')
+ ->with($room, $parameters['federated_user'] ?? 'federated_user@federation.tld')
+ ->willReturnCallback(function (Room $room, string $id): array {
+ [$user, $remote] = explode('@', $id);
+ return ['type' => 'user', 'id' => $user, 'server' => $remote];
+ });
  $parser->expects($this->any())
  ->method('getGroup')
  ->with($parameters['group'] ?? 'group')
@@ -679,32 +762,32 @@ public function testGetFileFromShareForGuestWithBlurhash(): void {
  $node->expects($this->once())
  ->method('getPermissions')
  ->willReturn(27);
- 
+
  $share = $this->createMock(IShare::class);
  $share->expects($this->once())
  ->method('getNode')
  ->willReturn($node);
  $share->expects($this->once())
  ->method('getToken')
  ->willReturn('token');
- 
+
  $this->shareProvider->expects($this->once())
  ->method('getShareById')
  ->with('23')
  ->willReturn($share);
- 
+
  $this->url->expects($this->once())
  ->method('linkToRouteAbsolute')
  ->with('files_sharing.sharecontroller.showShare', [
  'token' => 'token',
  ])
  ->willReturn('absolute-link');
- 
+
  $this->previewManager->expects($this->once())
  ->method('isAvailable')
  ->with($node)
  ->willReturn(true);
- 
+
  $this->filesMetadataCache->expects($this->once())
  ->method('getImageMetadataForFileId')
  ->with(54)
@@ -713,11 +796,11 @@ public function testGetFileFromShareForGuestWithBlurhash(): void {
  'height' => 4567,
  'blurhash' => 'LEHV9uae2yk8pyo0adR*.7kCMdnj'
  ]);
- 
+
  $participant = $this->createMock(Participant::class);
- 
+
  $parser = $this->getParser();
- 
+
  $this->assertSame([
  'type' => 'file',
  'id' => '54',