From 2bfa2987f6b22f7f8eeada60b62dc0dfb25b2b3d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 12 Feb 2024 17:23:53 +0100 Subject: [PATCH] feat(federation): Add inviter information to the invitations DB for serving Signed-off-by: Joas Schilling --- appinfo/info.xml | 2 +- .../CloudFederationProviderTalk.php | 14 ++-- lib/Federation/FederationManager.php | 4 ++ .../Version19000Date20240212155937.php | 64 +++++++++++++++++++ lib/Model/Invitation.php | 12 +++- lib/ResponseDefinitions.php | 2 + openapi-federation.json | 10 ++- openapi-full.json | 10 ++- .../features/bootstrap/FeatureContext.php | 21 ++++-- .../features/federation/invite.feature | 28 ++++---- 10 files changed, 138 insertions(+), 29 deletions(-) create mode 100644 lib/Migration/Version19000Date20240212155937.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 81000692357..b82ed096eac 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -16,7 +16,7 @@ And in the works for the [coming versions](https://github.com/nextcloud/spreed/m ]]> - 19.0.0-dev.0 + 19.0.0-dev.1 agpl Daniel Calviño Sánchez diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php index 0a331cdb466..fc3f6c682cb 100644 --- a/lib/Federation/CloudFederationProviderTalk.php +++ b/lib/Federation/CloudFederationProviderTalk.php @@ -121,20 +121,20 @@ public function shareReceived(ICloudFederationShare $share): string { $roomToken = $share->getResourceName(); $roomName = $share->getProtocol()['roomName']; $roomType = (int) $roomType; - $sharedBy = $share->getSharedByDisplayName(); + $sharedByDisplayName = $share->getSharedByDisplayName(); $sharedByFederatedId = $share->getSharedBy(); - $owner = $share->getOwnerDisplayName(); + $ownerDisplayName = $share->getOwnerDisplayName(); $ownerFederatedId = $share->getOwner(); [, $remote] = $this->addressHandler->splitUserRemote($ownerFederatedId); - // if no explicit information about the person who created the share was send + // if no explicit information about the person who created the share was sent // we assume that the share comes from the owner if ($sharedByFederatedId === null) { - $sharedBy = $owner; + $sharedByDisplayName = $ownerDisplayName; $sharedByFederatedId = $ownerFederatedId; } - if ($remote && $shareSecret && $shareWith && $roomToken && $remoteId && is_string($roomName) && $roomName && $owner) { + if ($remote && $shareSecret && $shareWith && $roomToken && $remoteId && is_string($roomName) && $roomName && $ownerDisplayName) { $shareWith = $this->userManager->get($shareWith); if ($shareWith === null) { $this->logger->debug('Received a federation invite for user that could not be found'); @@ -151,9 +151,9 @@ public function shareReceived(ICloudFederationShare $share): string { throw new ProviderCouldNotAddShareException('User does not exist', '', Http::STATUS_BAD_REQUEST); } - $invite = $this->federationManager->addRemoteRoom($shareWith, (int) $remoteId, $roomType, $roomName, $roomToken, $remote, $shareSecret); + $invite = $this->federationManager->addRemoteRoom($shareWith, (int) $remoteId, $roomType, $roomName, $roomToken, $remote, $shareSecret, $sharedByFederatedId, $sharedByDisplayName); - $this->notifyAboutNewShare($shareWith, (string) $invite->getId(), $sharedByFederatedId, $sharedBy, $roomName, $roomToken, $remote); + $this->notifyAboutNewShare($shareWith, (string) $invite->getId(), $sharedByFederatedId, $sharedByDisplayName, $roomName, $roomToken, $remote); return (string) $invite->getId(); } diff --git a/lib/Federation/FederationManager.php b/lib/Federation/FederationManager.php index d2149d0f427..fe2b82225be 100644 --- a/lib/Federation/FederationManager.php +++ b/lib/Federation/FederationManager.php @@ -77,6 +77,8 @@ public function addRemoteRoom( string $remoteServerUrl, #[SensitiveParameter] string $sharedSecret, + string $inviterCloudId, + string $inviterDisplayName, ): Invitation { try { $room = $this->manager->getRoomByToken($remoteToken, null, $remoteServerUrl); @@ -92,6 +94,8 @@ public function addRemoteRoom( $invitation->setRemoteServerUrl($remoteServerUrl); $invitation->setRemoteToken($remoteToken); $invitation->setRemoteAttendeeId($remoteAttendeeId); + $invitation->setInviterCloudId($inviterCloudId); + $invitation->setInviterDisplayName($inviterDisplayName); $this->invitationMapper->insert($invitation); return $invitation; diff --git a/lib/Migration/Version19000Date20240212155937.php b/lib/Migration/Version19000Date20240212155937.php new file mode 100644 index 00000000000..825b1baaeac --- /dev/null +++ b/lib/Migration/Version19000Date20240212155937.php @@ -0,0 +1,64 @@ + + * + * @author Joas Schilling + * + * @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 . + * + */ + +namespace OCA\Talk\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Add inviter information to the invites for rendering them outside of notifications later + */ +class Version19000Date20240212155937 extends SimpleMigrationStep { + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $table = $schema->getTable('talk_invitations'); + if (!$table->hasColumn('inviter_cloud_id')) { + $table->addColumn('inviter_cloud_id', Types::STRING, [ + 'notnull' => false, + 'length' => 255, + ]); + $table->addColumn('inviter_display_name', Types::STRING, [ + 'notnull' => false, + 'length' => 255, + ]); + return $schema; + } + + return null; + } +} diff --git a/lib/Model/Invitation.php b/lib/Model/Invitation.php index 33918e0a663..f2aea046fbc 100644 --- a/lib/Model/Invitation.php +++ b/lib/Model/Invitation.php @@ -43,6 +43,10 @@ * @method string getRemoteToken() * @method void setRemoteAttendeeId(int $remoteAttendeeId) * @method int getRemoteAttendeeId() + * @method void setInviterCloudId(string $inviterCloudId) + * @method string getInviterCloudId() + * @method void setInviterDisplayName(string $inviterDisplayName) + * @method string getInviterDisplayName() */ class Invitation extends Entity implements \JsonSerializable { public const STATE_PENDING = 0; @@ -55,6 +59,8 @@ class Invitation extends Entity implements \JsonSerializable { protected string $remoteServerUrl = ''; protected string $remoteToken = ''; protected int $remoteAttendeeId = 0; + protected string $inviterCloudId = ''; + protected string $inviterDisplayName = ''; public function __construct() { $this->addType('userId', 'string'); @@ -64,10 +70,12 @@ public function __construct() { $this->addType('remoteServerUrl', 'string'); $this->addType('remoteToken', 'string'); $this->addType('remoteAttendeeId', 'int'); + $this->addType('inviterCloudId', 'string'); + $this->addType('inviterDisplayName', 'string'); } /** - * @return array{accessToken: string, id: int, localRoomId: int, remoteAttendeeId: int, remoteServerUrl: string, remoteToken: string, state: int, userId: string} + * @return array{accessToken: string, id: int, localRoomId: int, remoteAttendeeId: int, remoteServerUrl: string, remoteToken: string, state: int, userId: string, inviterCloudId: string, inviterDisplayName: string} */ public function jsonSerialize(): array { return [ @@ -79,6 +87,8 @@ public function jsonSerialize(): array { 'remoteServerUrl' => $this->getRemoteServerUrl(), 'remoteToken' => $this->getRemoteToken(), 'remoteAttendeeId' => $this->getRemoteAttendeeId(), + 'inviterCloudId' => $this->getInviterCloudId(), + 'inviterDisplayName' => $this->getInviterDisplayName() ?: $this->getInviterCloudId(), ]; } } diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 835dfeb400a..3c51ae53fe3 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -109,6 +109,8 @@ * remoteToken: string, * roomName: string, * userId: string, + * inviterCloudId: string, + * inviterDisplayName: string, * } * * @psalm-type TalkMatterbridgeConfigFields = array> diff --git a/openapi-federation.json b/openapi-federation.json index 56007fb351f..2eb71a8eb7b 100644 --- a/openapi-federation.json +++ b/openapi-federation.json @@ -133,7 +133,9 @@ "remoteServerUrl", "remoteToken", "roomName", - "userId" + "userId", + "inviterCloudId", + "inviterDisplayName" ], "properties": { "accessToken": { @@ -166,6 +168,12 @@ }, "userId": { "type": "string" + }, + "inviterCloudId": { + "type": "string" + }, + "inviterDisplayName": { + "type": "string" } } }, diff --git a/openapi-full.json b/openapi-full.json index d62a8b85076..dc8617f56cd 100644 --- a/openapi-full.json +++ b/openapi-full.json @@ -330,7 +330,9 @@ "remoteServerUrl", "remoteToken", "roomName", - "userId" + "userId", + "inviterCloudId", + "inviterDisplayName" ], "properties": { "accessToken": { @@ -363,6 +365,12 @@ }, "userId": { "type": "string" + }, + "inviterCloudId": { + "type": "string" + }, + "inviterDisplayName": { + "type": "string" } } }, diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index b0dcd23ab11..6b6d5bcb0bf 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -555,13 +555,26 @@ public function userAcceptsDeclinesRemoteInvite(string $user, string $acceptsDec */ private function assertInvites($invites, TableNode $formData) { Assert::assertCount(count($formData->getHash()), $invites, 'Invite count does not match'); - Assert::assertEquals($formData->getHash(), array_map(function ($invite, $expectedInvite) { + $expectedInvites = array_map(static function ($expectedInvite): array { + if (isset($expectedInvite['state'])) { + $expectedInvite['state'] = (int) $expectedInvite['state']; + } + return $expectedInvite; + }, $formData->getHash()); + + Assert::assertEquals($expectedInvites, array_map(function ($invite, $expectedInvite): array { $data = []; if (isset($expectedInvite['id'])) { $data['id'] = self::$tokenToIdentifier[$invite['token']]; } if (isset($expectedInvite['accessToken'])) { - $data['accessToken'] = (string) $invite['accessToken']; + $data['accessToken'] = $invite['accessToken']; + } + if (isset($expectedInvite['inviterCloudId'])) { + $data['inviterCloudId'] = $invite['inviterCloudId']; + } + if (isset($expectedInvite['inviterDisplayName'])) { + $data['inviterDisplayName'] = $invite['inviterDisplayName']; } if (isset($expectedInvite['remoteToken'])) { $data['remoteToken'] = self::$tokenToIdentifier[$invite['remoteToken']] ?? 'unknown-token'; @@ -570,11 +583,11 @@ private function assertInvites($invites, TableNode $formData) { $data['remoteServerUrl'] = $this->translateRemoteServer($invite['remoteServerUrl']); } if (isset($expectedInvite['state'])) { - $data['state'] = (int) $invite['state']; + $data['state'] = $invite['state']; } return $data; - }, $invites, $formData->getHash())); + }, $invites, $expectedInvites)); } protected function translateRemoteServer(string $server): string { diff --git a/tests/integration/features/federation/invite.feature b/tests/integration/features/federation/invite.feature index dcb728d5533..9c325228bf1 100644 --- a/tests/integration/features/federation/invite.feature +++ b/tests/integration/features/federation/invite.feature @@ -51,8 +51,8 @@ Feature: federation/invite | room | users | participant1 | conversation_created | You created the conversation | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | And force run "OCA\Talk\BackgroundJob\RemoveEmptyRooms" background jobs And user "participant2" has the following invitations (v1) - | remoteServerUrl | remoteToken | state | - | LOCAL | room | 0 | + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | Then user "participant2" has the following notifications | app | object_type | object_id | subject | message | | spreed | remote_talk_share | INVITE_ID(LOCAL::room) | @participant1-displayname invited you to a federated conversation | @participant1-displayname invited you to join room on http://localhost:8080 | @@ -62,8 +62,8 @@ Feature: federation/invite And user "participant2" accepts invite to room "room" of server "LOCAL" with 400 (v1) | error | state | And user "participant2" has the following invitations (v1) - | remoteServerUrl | remoteToken | state | - | LOCAL | room | 1 | + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 1 | participant1@http://localhost:8080 | participant1-displayname | When user "participant1" sees the following attendees in room "room" with 200 (v4) | actorType | actorId | participantType | | users | participant1 | 1 | @@ -103,8 +103,8 @@ Feature: federation/invite | room | users | participant1 | federated_user_added | You invited {federated_user} | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"},"federated_user":{"type":"user","id":"participant2","name":"participant2@localhost:8180","server":"http:\/\/localhost:8180"}} | | room | users | participant1 | conversation_created | You created the conversation | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | And user "participant2" has the following invitations (v1) - | remoteServerUrl | remoteToken | state | - | LOCAL | room | 0 | + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | Then user "participant2" has the following notifications | app | object_type | object_id | subject | message | | spreed | remote_talk_share | INVITE_ID(LOCAL::room) | @participant1-displayname invited you to a federated conversation | @participant1-displayname invited you to join room on http://localhost:8080 | @@ -138,8 +138,8 @@ Feature: federation/invite | room | users | participant1 | conversation_created | You created the conversation | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | And force run "OCA\Talk\BackgroundJob\RemoveEmptyRooms" background jobs And user "participant2" has the following invitations (v1) - | remoteServerUrl | remoteToken | state | - | LOCAL | room | 0 | + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | Then user "participant2" has the following notifications | app | object_type | object_id | subject | message | | spreed | remote_talk_share | INVITE_ID(LOCAL::room) | @participant1-displayname invited you to a federated conversation | @participant1-displayname invited you to join room on http://localhost:8080 | @@ -164,14 +164,14 @@ Feature: federation/invite | roomName | room | And user "participant1" adds remote "participant2" to room "room" with 200 (v4) And user "participant2" has the following invitations (v1) - | remoteServerUrl | remoteToken | state | - | LOCAL | room | 0 | + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) | id | name | type | remoteServer | remoteToken | | room | room | 2 | LOCAL | room | And user "participant2" has the following invitations (v1) - | remoteServerUrl | remoteToken | state | - | LOCAL | room | 1 | + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 1 | participant1@http://localhost:8080 | participant1-displayname | Then user "participant2" is participant of the following rooms (v4) | id | type | | room | 2 | @@ -190,8 +190,8 @@ Feature: federation/invite | roomName | room | And user "participant1" adds remote "participant2" to room "room" with 200 (v4) And user "participant2" has the following invitations (v1) - | remoteServerUrl | remoteToken | state | - | LOCAL | room | 0 | + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) | id | name | type | remoteServer | remoteToken | | room | room | 2 | LOCAL | room |