Merge pull request #11581 from nextcloud/bugfix/11278/add-inviter-inf…

…o-to-invitation-list feat(federation): Add inviter information to the invitations DB for s…
nextcloud · Feb 14, 2024 · db77116 · db77116
2 parents 020add2 + 2bfa298
commit db77116
Show file tree

Hide file tree

Showing 10 changed files with 138 additions and 29 deletions.
diff --git 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
 
 	]]></description>
 
-	<version>19.0.0-dev.0</version>
+	<version>19.0.0-dev.1</version>
 	<licence>agpl</licence>
 
 	<author>Daniel Calviño Sánchez</author>

diff --git 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
@@ -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
@@ -0,0 +1,64 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 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\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
@@ -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
@@ -109,6 +109,8 @@
  *     remoteToken: string,
  *     roomName: string,
  *     userId: string,
+ *     inviterCloudId: string,
+ *     inviterDisplayName: string,
  * }
  *
  * @psalm-type TalkMatterbridgeConfigFields = array<array<string, mixed>>

diff --git 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
@@ -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
@@ -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
@@ -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        |