fix(federation): Prevent duplicated invites with different casing and…

lib/Federation/BackendNotifier.php

@@ -148,6 +148,7 @@ public function sendShareAccepted(
 		#[SensitiveParameter]
 		string $accessToken,
 		string $displayName,
+		string $cloudId,
 	): bool {
 		$remote = $this->prepareRemoteUrl($remoteServerUrl);
 
@@ -161,6 +162,7 @@ public function sendShareAccepted(
 				'sharedSecret' => $accessToken,
 				'message' => 'Recipient accepted the share',
 				'displayName' => $displayName,
+				'cloudId' => $cloudId,
 			]
 		);
 

lib/Federation/CloudFederationProviderTalk.php

@@ -219,6 +219,11 @@ private function shareAccepted(int $id, array $notification): array {
 		if (!empty($notification['displayName'])) {
 			$attendee->setDisplayName($notification['displayName']);
 			$attendee->setState(Invitation::STATE_ACCEPTED);
+
+			if (!empty($notification['cloudId'])) {
+				$attendee->setActorId($notification['cloudId']);
+			}
+
 			$this->attendeeMapper->update($attendee);
 		}
 

lib/Federation/FederationManager.php

@@ -39,7 +39,10 @@
 use OCA\Talk\Room;
 use OCA\Talk\Service\ParticipantService;
 use OCP\AppFramework\Db\DoesNotExistException;
+use OCP\AppFramework\Http;
+use OCP\Federation\Exceptions\ProviderCouldNotAddShareException;
 use OCP\Federation\ICloudId;
+use OCP\Federation\ICloudIdManager;
 use OCP\IUser;
 use OCP\Notification\IManager;
 use SensitiveParameter;
@@ -68,6 +71,7 @@ public function __construct(
 		private InvitationMapper $invitationMapper,
 		private BackendNotifier $backendNotifier,
 		private IManager $notificationManager,
+		private ICloudIdManager $cloudIdManager,
 		private RestrictionValidator $restrictionValidator,
 	) {
 	}
@@ -97,12 +101,23 @@ public function addRemoteRoom(
 		string $inviterDisplayName,
 		string $localCloudId,
 	): Invitation {
+		$couldHaveInviteWithOtherCasing = false;
 		try {
 			$room = $this->manager->getRoomByToken($remoteToken, null, $remoteServerUrl);
+			$couldHaveInviteWithOtherCasing = true;
 		} catch (RoomNotFoundException) {
 			$room = $this->manager->createRemoteRoom($roomType, $roomName, $remoteToken, $remoteServerUrl);
 		}
 
+		if ($couldHaveInviteWithOtherCasing) {
+			try {
+				$this->invitationMapper->getInvitationForUserByLocalRoom($room, $user->getUID(), true);
+				throw new ProviderCouldNotAddShareException('User already invited', '', Http::STATUS_BAD_REQUEST);
+			} catch (DoesNotExistException) {
+				// Not invited with any casing already, so all good.
+			}
+		}
+
 		$invitation = new Invitation();
 		$invitation->setUserId($user->getUID());
 		$invitation->setState(Invitation::STATE_PENDING);
@@ -145,10 +160,13 @@ public function acceptRemoteRoomShare(IUser $user, int $shareId): Participant {
 			throw new \InvalidArgumentException('state');
 		}
 
+
+		$cloudId = $this->cloudIdManager->getCloudId($user->getUID(), null);
+
 		// Add user to the room
 		$room = $this->manager->getRoomById($invitation->getLocalRoomId());
 		if (
-			!$this->backendNotifier->sendShareAccepted($invitation->getRemoteServerUrl(), $invitation->getRemoteAttendeeId(), $invitation->getAccessToken(), $user->getDisplayName())
+			!$this->backendNotifier->sendShareAccepted($invitation->getRemoteServerUrl(), $invitation->getRemoteAttendeeId(), $invitation->getAccessToken(), $user->getDisplayName(), $cloudId->getId())
 		) {
 			throw new CannotReachRemoteException();
 		}
@@ -169,6 +187,7 @@ public function acceptRemoteRoomShare(IUser $user, int $shareId): Participant {
 		$attendee = array_pop($attendees);
 
 		$invitation->setState(Invitation::STATE_ACCEPTED);
+		$invitation->setLocalCloudId($cloudId->getId());
 		$this->invitationMapper->update($invitation);
 
 		$this->markNotificationProcessed($user->getUID(), $shareId);

lib/Model/InvitationMapper.php

@@ -98,14 +98,18 @@ public function getInvitationsForUser(IUser $user): array {
 	/**
 	 * @throws DoesNotExistException
 	 */
-	public function getInvitationForUserByLocalRoom(Room $room, string $userId): Invitation {
+	public function getInvitationForUserByLocalRoom(Room $room, string $userId, bool $caseInsensitive = false): Invitation {
 		$query = $this->db->getQueryBuilder();
 
 		$query->select('*')
 			->from($this->getTableName())
-			->where($query->expr()->eq('user_id', $query->createNamedParameter($userId)))
-			->andWhere($query->expr()->eq('local_room_id', $query->createNamedParameter($room->getId())));
+			->where($query->expr()->eq('local_room_id', $query->createNamedParameter($room->getId())));
 
+		if ($caseInsensitive) {
+			$query->andWhere($query->expr()->eq($query->func()->lower('user_id'), $query->createNamedParameter(strtolower($userId))));
+		} else {
+			$query->andWhere($query->expr()->eq('user_id', $query->createNamedParameter($userId)));
+		}
 		return $this->findEntity($query);
 	}
 

tests/integration/features/bootstrap/FeatureContext.php

@@ -586,6 +586,9 @@ private function assertInvites($invites, TableNode $formData) {
 			if (isset($expectedInvite['state'])) {
 				$data['state'] = $invite['state'];
 			}
+			if (isset($expectedInvite['localCloudId'])) {
+				$data['localCloudId'] = $invite['localCloudId'];
+			}
 
 			return $data;
 		}, $invites, $expectedInvites));

tests/integration/features/federation/invite.feature

@@ -89,6 +89,70 @@ 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"}} |
 
+  Scenario: Invite user with wrong casing
+    Given the following "spreed" app config is set
+      | federation_enabled | yes |
+    Given user "participant1" creates room "room" (v4)
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant1" adds federated_user "PARTICIPANT2" to room "room" with 200 (v4)
+    When user "participant1" sees the following attendees in room "room" with 200 (v4)
+      | actorType       | actorId      | participantType |
+      | users           | participant1 | 1               |
+      | federated_users | PARTICIPANT2 | 3               |
+    Then user "participant1" sees the following system messages in room "room" with 200
+      | room | actorType     | actorId      | systemMessage        | message                      | messageParameters |
+      | 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 "participant1" adds federated_user "participant2" to room "room" with 404 (v4)
+    When user "participant1" sees the following attendees in room "room" with 200 (v4)
+      | actorType       | actorId      | participantType |
+      | users           | participant1 | 1               |
+      | federated_users | PARTICIPANT2 | 3               |
+    Then user "participant1" sees the following system messages in room "room" with 200
+      | room | actorType     | actorId      | systemMessage        | message                      | messageParameters |
+      | 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 force run "OCA\Talk\BackgroundJob\RemoveEmptyRooms" background jobs
+    And user "participant2" has the following invitations (v1)
+      | remoteServerUrl | remoteToken | state | inviterCloudId                     | inviterDisplayName       | localCloudId |
+      | LOCAL           | room        | 0     | participant1@http://localhost:8080 | participant1-displayname | PARTICIPANT2@http://localhost:8180 |
+    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 |
+    And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1)
+      | id   | name | type | remoteServer | remoteToken |
+      | room | room | 3    | LOCAL        | room        |
+    And user "participant2" accepts invite to room "room" of server "LOCAL" with 400 (v1)
+      | error | state |
+    And user "participant2" declines invite to room "room" of server "LOCAL" with 400 (v1)
+      | error | state |
+    And user "participant2" has the following invitations (v1)
+      | 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               |
+      | federated_users | participant2 | 3               |
+    Then user "participant1" sees the following system messages in room "room" with 200
+      | room | actorType     | actorId      | systemMessage           | message                      | messageParameters |
+      | room | federated_users | participant2@http://localhost:8180 | federated_user_added | {federated_user} accepted the invitation | {"actor":{"type":"user","id":"participant2","name":"participant2-displayname","server":"http:\/\/localhost:8180"},"federated_user":{"type":"user","id":"participant2","name":"participant2-displayname","server":"http:\/\/localhost:8180"}} |
+      | 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"}} |
+    # Remove a remote user after they joined
+    When user "participant1" removes remote "participant2" from room "room" with 200 (v4)
+    And user "participant2" has the following invitations (v1)
+    Then user "participant2" is participant of the following rooms (v4)
+    When user "participant1" sees the following attendees in room "room" with 200 (v4)
+      | actorType       | actorId      | participantType |
+      | users           | participant1 | 1               |
+    Then user "participant1" sees the following system messages in room "room" with 200
+      | room | actorType     | actorId      | systemMessage           | message                      | messageParameters |
+      | room | users         | participant1 | federated_user_removed  | You removed {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 | federated_users | participant2@http://localhost:8180 | federated_user_added | {federated_user} accepted the invitation | {"actor":{"type":"user","id":"participant2","name":"participant2@localhost:8180","server":"http:\/\/localhost:8180"},"federated_user":{"type":"user","id":"participant2","name":"participant2@localhost:8180","server":"http:\/\/localhost:8180"}} |
+      | 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"}} |
+
   Scenario: Declining an invite
     Given the following "spreed" app config is set
       | federation_enabled | yes |

tests/php/Federation/FederationTest.php

@@ -417,6 +417,7 @@ public function testSendAcceptNotification() {
 					'message' => 'Recipient accepted the share',
 					'remoteServerUrl' => 'http://example.tld',
 					'displayName' => 'Foo Bar',
+					'cloudId' => '[email protected]',
 				]
 			);
 
@@ -441,7 +442,7 @@ public function testSendAcceptNotification() {
 			->with('/')
 			->willReturn('http://example.tld/index.php/');
 
-		$success = $this->backendNotifier->sendShareAccepted($remote, $id, $token, 'Foo Bar');
+		$success = $this->backendNotifier->sendShareAccepted($remote, $id, $token, 'Foo Bar', '[email protected]');
 
 		$this->assertTrue($success);
 	}