Skip to content

fix(federation): Correctly handle decline in system messages #13150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nickvergessen merged 1 commit into from
Aug 27, 2024
Merged

fix(federation): Correctly handle decline in system messages #13150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions lib/Chat/Parser/SystemMessage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'])) {
Expand All @@ -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') {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/features/federation/chat.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
109 changes: 96 additions & 13 deletions tests/php/Chat/Parser/SystemMessageTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -431,18 +432,52 @@ public static function dataParseMessage(): array {
'The shared location is malformed',
[],
],
['federated_user_added', ['federated_user' => '[email protected]'], 'actor',
'You invited {federated_user}',
['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
],
['federated_user_added', ['federated_user' => '[email protected]'], 'user',
'{actor} invited {federated_user}',
['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
],
['federated_user_added', ['federated_user' => '[email protected]'], 'fed::[email protected]',
'{actor} invited you',
['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
],
['federated_user_added', ['federated_user' => '[email protected]'], 'fed::[email protected]',
'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' => '[email protected]'], 'actor',
'You removed {federated_user}',
['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
],
['federated_user_removed', ['federated_user' => '[email protected]'], 'user',
'{actor} removed {federated_user}',
['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
],
['federated_user_removed', ['federated_user' => '[email protected]'], 'fed::[email protected]',
'{actor} removed you',
['actor' => ['id' => 'actor', 'type' => 'user'], 'federated_user' => ['type' => 'user', 'id' => 'actor', 'server' => 'federated.tld']],
],
['federated_user_removed', ['federated_user' => '[email protected]'], 'fed::[email protected]',
'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::')),
Expand All @@ -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')
Expand All @@ -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'] ?? '[email protected]')
->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')
Expand Down Expand Up @@ -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)
Expand All @@ -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',
Expand Down