Skip to content

Commit

Permalink
fix(federation): Fix message parameters when a federated author delet…
Browse files Browse the repository at this point in the history 
…es a message

Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
@nickvergessen
nickvergessen committed Feb 28, 2024
1 parent 9f1b8e0 commit 295564a
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 10 deletions.
47 changes: 42 additions & 5 deletions lib/Chat/Parser/SystemMessage.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use OCA\Talk\Chat\ChatManager;
use OCA\Talk\Events\MessageParseEvent;
use OCA\Talk\Exceptions\ParticipantNotFoundException;
use OCA\Talk\Federation\Authenticator;
use OCA\Talk\GuestManager;
use OCA\Talk\Model\Attendee;
use OCA\Talk\Model\Message;
Expand Down Expand Up @@ -77,6 +78,9 @@ class SystemMessage implements IEventListener {
/** @var array<string, array<string, string>> */
protected array $phoneNames = [];


protected array $currentFederatedUserDetails = [];

public function __construct(
protected IUserManager $userManager,
protected IGroupManager $groupManager,
Expand All @@ -89,6 +93,7 @@ public function __construct(
protected ICloudIdManager $cloudIdManager,
protected IURLGenerator $url,
protected FilesMetadataCache $metadataCache,
protected Authenticator $federationAuthenticator,
) {
}

Expand Down Expand Up @@ -135,6 +140,19 @@ protected function parseMessage(Message $chatMessage): void {
if ($participant === null) {
$currentActorId = null;
$currentUserIsActor = false;
} elseif ($this->federationAuthenticator->isFederationRequest()) {
if (empty($this->currentFederatedUserDetails)) {
$cloudId = $this->cloudIdManager->resolveCloudId($this->federationAuthenticator->getCloudId());
$this->currentFederatedUserDetails = [
'user' => $cloudId->getUser(),
'server' => $cloudId->getRemote(),
];
}

$currentUserIsActor = isset($parsedParameters['actor']['server']) &&
$parsedParameters['actor']['type'] === 'user' &&
$this->currentFederatedUserDetails['user'] === $parsedParameters['actor']['id'] &&
$this->currentFederatedUserDetails['server'] === $parsedParameters['actor']['server'];
} elseif (!$participant->isGuest()) {
$currentActorId = $participant->getAttendee()->getActorId();
$currentUserIsActor = $parsedParameters['actor']['type'] === 'user' &&
Expand Down Expand Up @@ -338,7 +356,7 @@ protected function parseMessage(Message $chatMessage): void {
}
}
} elseif ($message === 'federated_user_added') {
$parsedParameters['federated_user'] = $this->getRemoteUser($parameters['federated_user']);
$parsedParameters['federated_user'] = $this->getRemoteUser($room, $parameters['federated_user']);
$parsedMessage = $this->l->t('{actor} invited {federated_user}');
if ($currentUserIsActor) {
$parsedMessage = $this->l->t('You invited {federated_user}');
Expand All @@ -348,7 +366,7 @@ protected function parseMessage(Message $chatMessage): void {
$parsedMessage = $this->l->t('{federated_user} accepted the invitation');
}
} elseif ($message === 'federated_user_removed') {
$parsedParameters['federated_user'] = $this->getRemoteUser($parameters['federated_user']);
$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}');
Expand Down Expand Up @@ -647,6 +665,19 @@ protected function parseDeletedMessage(Message $chatMessage): void {

if ($participant === null) {
$currentUserIsActor = false;
} elseif ($this->federationAuthenticator->isFederationRequest()) {
if (empty($this->currentFederatedUserDetails)) {
$cloudId = $this->cloudIdManager->resolveCloudId($this->federationAuthenticator->getCloudId());
$this->currentFederatedUserDetails = [
'user' => $cloudId->getUser(),
'server' => $cloudId->getRemote(),
];
}

$currentUserIsActor = isset($parsedParameters['actor']['server']) &&
$parsedParameters['actor']['type'] === 'user' &&
$this->currentFederatedUserDetails['user'] === $parsedParameters['actor']['id'] &&
$this->currentFederatedUserDetails['server'] === $parsedParameters['actor']['server'];
} elseif (!$participant->isGuest()) {
$currentUserIsActor = $parsedParameters['actor']['type'] === 'user' &&
$participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS &&
Expand Down Expand Up @@ -797,7 +828,7 @@ protected function getActor(Room $room, string $actorType, string $actorId): arr
return $this->getPhone($room, $actorId, '');
}
if ($actorType === Attendee::ACTOR_FEDERATED_USERS) {
return $this->getRemoteUser($actorId);
return $this->getRemoteUser($room, $actorId);
}

return $this->getUser($actorId);
Expand Down Expand Up @@ -827,13 +858,19 @@ protected function getUser(string $uid): array {
];
}

protected function getRemoteUser(string $federationId): array {
protected function getRemoteUser(Room $room, string $federationId): array {
$cloudId = $this->cloudIdManager->resolveCloudId($federationId);
$displayName = $cloudId->getDisplayId();
try {
$participant = $this->participantService->getParticipantByActor($room, Attendee::ACTOR_FEDERATED_USERS, $federationId);
$displayName = $participant->getAttendee()->getDisplayName();
} catch (ParticipantNotFoundException) {
}

return [
'type' => 'user',
'id' => $cloudId->getUser(),
'name' => $cloudId->getDisplayId(),
'name' => $displayName,
'server' => $cloudId->getRemote(),
];
}
Expand Down
8 changes: 7 additions & 1 deletion tests/integration/features/bootstrap/FeatureContext.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2657,14 +2657,20 @@ protected function compareDataResponse(TableNode $formData = null) {
}
$expected[$i]['message'] = str_replace('\n', "\n", $expected[$i]['message']);


if (str_ends_with($expected[$i]['actorId'], '@{$BASE_URL}')) {
$expected[$i]['actorId'] = str_replace('{$BASE_URL}', rtrim($this->baseUrl, '/'), $expected[$i]['actorId']);
}
if (str_ends_with($expected[$i]['actorId'], '@{$REMOTE_URL}')) {
$expected[$i]['actorId'] = str_replace('{$REMOTE_URL}', rtrim($this->baseRemoteUrl, '/'), $expected[$i]['actorId']);
}

if (str_contains($expected[$i]['messageParameters'], '{$BASE_URL}')) {
$expected[$i]['messageParameters'] = str_replace('{$BASE_URL}', str_replace('/', '\/', rtrim($this->baseUrl, '/')), $expected[$i]['messageParameters']);
}
if (str_contains($expected[$i]['messageParameters'], '{$REMOTE_URL}')) {
$expected[$i]['messageParameters'] = str_replace('{$REMOTE_URL}', str_replace('/', '\/', rtrim($this->baseRemoteUrl, '/')), $expected[$i]['messageParameters']);
}

if (isset($expected[$i]['lastEditActorId'])) {
if (str_ends_with($expected[$i]['lastEditActorId'], '@{$BASE_URL}')) {
$expected[$i]['lastEditActorId'] = str_replace('{$BASE_URL}', rtrim($this->baseUrl, '/'), $expected[$i]['lastEditActorId']);
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/features/federation/chat.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,14 @@ Feature: federation/chat
And user "participant2" deletes message "Message 1-1 - Edit 1" from room "LOCAL::room" with 200
Then user "participant1" sees the following messages in room "room" with 200
| room | actorType | actorId | actorDisplayName | message | messageParameters | parentMessage |
| room | federated_users | participant2@{$REMOTE_URL} | participant2-displayname | Message deleted by author | {"actor":{"type":"user","id":"participant2","name":"participant2@localhost:8180","server":"http:\/\/localhost:8180"}} | Message deleted by you |
| room | users | participant1 | participant1-displayname | Message deleted by you | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | |
| room | federated_users | participant2@{$REMOTE_URL} | participant2-displayname | Message deleted by author | {"actor":{"type":"user","id":"participant2","name":"participant2-displayname","server":"{$REMOTE_URL}"}} | Message deleted by you |
| room | users | participant1 | participant1-displayname | Message deleted by you | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | |
When next message request has the following parameters set
| timeout | 0 |
And user "participant2" sees the following messages in room "LOCAL::room" with 200
| room | actorType | actorId | actorDisplayName | message | messageParameters | parentMessage |
| room | users | participant2 | participant2-displayname | Message deleted by author | {"actor":{"type":"user","id":"participant2","name":"participant2@localhost:8180","server":"http:\/\/localhost:8180"}} | Message deleted by author |
| room | federated_users | participant1@{$BASE_URL} | participant1-displayname | Message deleted by author | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | |
| room | users | participant2 | participant2-displayname | Message deleted by you | {"actor":{"type":"user","id":"participant2","name":"participant2-displayname"}} | Message deleted by author |
| room | federated_users | participant1@{$BASE_URL} | participant1-displayname | Message deleted by author | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname","server":"{$BASE_URL}"}} | |

Scenario: Error handling of chatting (posting a too long message)
Given the following "spreed" app config is set
Expand Down

0 comments on commit 295564a

Please sign in to comment.