Merge pull request #11464 from nextcloud/bugfix/5723/send-disinvite-when-removing-remote-user

fix(federation): Send disinvite on remote user removal and remove invite

7 files changed, 94 insertions, 5 deletions

diff --git a/lib/Federation/BackendNotifier.php b/lib/Federation/BackendNotifier.php
index 2ca4fb9e3..db4d9c9af 100644
--- a/lib/Federation/BackendNotifier.php
+++ b/lib/Federation/BackendNotifier.php
@@ -132,7 +132,12 @@ class BackendNotifier {
 	 *
 	 * @return bool success
 	 */
-	public function sendShareAccepted(string $remoteServerUrl, int $remoteAttendeeId, string $accessToken): bool {
+	public function sendShareAccepted(
+		string $remoteServerUrl,
+		int $remoteAttendeeId,
+		#[SensitiveParameter]
+		string $accessToken,
+	): bool {
 		$remote = $this->prepareRemoteUrl($remoteServerUrl);
 
 		$notification = $this->cloudFederationFactory->getCloudFederationNotification();
@@ -160,7 +165,12 @@ class BackendNotifier {
 	 * The invited participant declined joining the federated room
 	 * Sent from Remote participant server to Host server
 	 */
-	public function sendShareDeclined(string $remoteServerUrl, int $remoteAttendeeId, string $accessToken): bool {
+	public function sendShareDeclined(
+		string $remoteServerUrl,
+		int $remoteAttendeeId,
+		#[SensitiveParameter]
+		string $accessToken,
+	): bool {
 		$remote = $this->prepareRemoteUrl($remoteServerUrl);
 
 		$notification = $this->cloudFederationFactory->getCloudFederationNotification();
@@ -185,7 +195,12 @@ class BackendNotifier {
 		return true;
 	}
 
-	public function sendRemoteUnShare(string $remoteServerUrl, int $localAttendeeId, string $accessToken): void {
+	public function sendRemoteUnShare(
+		string $remoteServerUrl,
+		int $localAttendeeId,
+		#[SensitiveParameter]
+		string $accessToken,
+	): void {
 		$remote = $this->prepareRemoteUrl($remoteServerUrl);
 
 		$notification = $this->cloudFederationFactory->getCloudFederationNotification();
diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php
index 19a14abda..224aad911 100644
--- a/lib/Federation/CloudFederationProviderTalk.php
+++ b/lib/Federation/CloudFederationProviderTalk.php
@@ -229,6 +229,7 @@ class CloudFederationProviderTalk implements ICloudFederationProvider {
 			throw new ShareNotFound();
 		}
 
+		$this->invitationMapper->delete($invite);
 		$participant = $this->participantService->getParticipantByActor($room, Attendee::ACTOR_USERS, $invite->getUserId());
 		$this->participantService->removeAttendee($room, $participant, AAttendeeRemovedEvent::REASON_REMOVED);
 		return [];
diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php
index 4b825f1f7..bdfc67679 100644
--- a/lib/Notification/Notifier.php
+++ b/lib/Notification/Notifier.php
@@ -405,6 +405,9 @@ class Notifier implements INotifier {
 				throw new AlreadyProcessedException();
 			}
 			$room = $this->manager->getRoomById($invite->getLocalRoomId());
+		} catch (DoesNotExistException) {
+			// Invitation does not exist
+			throw new AlreadyProcessedException();
 		} catch (RoomNotFoundException) {
 			// Room does not exist
 			throw new AlreadyProcessedException();
diff --git a/lib/Service/ParticipantService.php b/lib/Service/ParticipantService.php
index 587e1ed00..5a455b981 100644
--- a/lib/Service/ParticipantService.php
+++ b/lib/Service/ParticipantService.php
@@ -75,6 +75,7 @@ use OCP\Comments\IComment;
 use OCP\DB\Exception;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Federation\ICloudIdManager;
 use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -102,6 +103,7 @@ class ParticipantService {
 		protected IDBConnection $connection,
 		private IEventDispatcher $dispatcher,
 		private IUserManager $userManager,
+		private ICloudIdManager $cloudIdManager,
 		private IGroupManager $groupManager,
 		private MembershipService $membershipService,
 		private BackendNotifier $backendNotifier,
@@ -822,6 +824,17 @@ class ParticipantService {
 	 * @psalm-param AAttendeeRemovedEvent::REASON_* $reason
 	 */
 	public function removeAttendee(Room $room, Participant $participant, string $reason, bool $attendeeEventIsTriggeredAlready = false): void {
+		if ($participant->getAttendee()->getActorType() === Attendee::ACTOR_FEDERATED_USERS) {
+			$attendee = $participant->getAttendee();
+			$cloudId = $this->cloudIdManager->resolveCloudId($attendee->getActorId());
+
+			$this->backendNotifier->sendRemoteUnShare(
+				$cloudId->getRemote(),
+				$attendee->getId(),
+				$attendee->getAccessToken(),
+			);
+		}
+
 		$sessions = $this->sessionService->getAllSessionsForAttendee($participant->getAttendee());
 
 		if ($room->getBreakoutRoomMode() !== BreakoutRoom::MODE_NOT_CONFIGURED) {
diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php
index 5cb5d7108..5cb001b2c 100644
--- a/tests/integration/features/bootstrap/FeatureContext.php
+++ b/tests/integration/features/bootstrap/FeatureContext.php
@@ -505,7 +505,7 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 		$invites = $this->getDataFromResponse($this->response);
 
 		if ($formData === null) {
-			Assert::assertEmpty($invites);
+			Assert::assertEmpty($invites, json_encode($invites, JSON_PRETTY_PRINT));
 			return;
 		}
 
@@ -1370,7 +1370,7 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	}
 
 	/**
-	 * @Then /^user "([^"]*)" removes (user|group|email) "([^"]*)" from room "([^"]*)" with (\d+) \((v4)\)$/
+	 * @Then /^user "([^"]*)" removes (user|group|email|remote) "([^"]*)" from room "([^"]*)" with (\d+) \((v4)\)$/
 	 *
 	 * @param string $user
 	 * @param string $actorType
@@ -1383,6 +1383,11 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 		if ($actorId === 'stranger') {
 			$attendeeId = 123456789;
 		} else {
+			if ($actorType === 'remote') {
+				$actorId .= '@' . rtrim($this->baseRemoteUrl, '/');
+				$actorType = 'federated_user';
+			}
+
 			$attendeeId = $this->getAttendeeId($actorType . 's', $actorId, $identifier, $statusCode === 200 ? $user : null);
 		}
 
diff --git a/tests/integration/features/federation/invite.feature b/tests/integration/features/federation/invite.feature
index f74a89993..355e062df 100644
--- a/tests/integration/features/federation/invite.feature
+++ b/tests/integration/features/federation/invite.feature
@@ -62,6 +62,19 @@ Feature: federation/invite
       | 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"}} |
+    # 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
@@ -95,6 +108,41 @@ 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: Remove remote user before they accept
+    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 remote "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 force run "OCA\Talk\BackgroundJob\RemoveEmptyRooms" background jobs
+    And user "participant2" has the following invitations (v1)
+      | remote_server_url | remote_token | state |
+      | LOCAL             | room         | 0     |
+    Then user "participant2" has the following notifications
+      | app    | object_type       | object_id              | subject                                                                      |
+      | spreed | remote_talk_share | INVITE_ID(LOCAL::room) | @participant1-displayname shared room room on http://localhost:8080 with you |
+    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)
+    Then user "participant2" has the following notifications
+    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 | 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: Authenticate as a federation user
     Given the following "spreed" app config is set
       | federation_enabled | yes |
diff --git a/tests/php/Service/ParticipantServiceTest.php b/tests/php/Service/ParticipantServiceTest.php
index 9ef28a8bf..aa99a35d6 100644
--- a/tests/php/Service/ParticipantServiceTest.php
+++ b/tests/php/Service/ParticipantServiceTest.php
@@ -37,6 +37,7 @@ use OCA\Talk\Service\SessionService;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Federation\ICloudIdManager;
 use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IGroupManager;
@@ -63,6 +64,7 @@ class ParticipantServiceTest extends TestCase {
 	protected $dispatcher;
 	/** @var IUserManager|MockObject */
 	protected $userManager;
+	protected ICloudIdManager|MockObject $cloudIdManager;
 	/** @var IGroupManager|MockObject */
 	protected $groupManager;
 	/** @var MembershipService|MockObject */
@@ -87,6 +89,7 @@ class ParticipantServiceTest extends TestCase {
 		$this->secureRandom = $this->createMock(ISecureRandom::class);
 		$this->dispatcher = $this->createMock(IEventDispatcher::class);
 		$this->userManager = $this->createMock(IUserManager::class);
+		$this->cloudIdManager = $this->createMock(ICloudIdManager::class);
 		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->membershipService = $this->createMock(MembershipService::class);
 		$this->federationBackendNotifier = $this->createMock(BackendNotifier::class);
@@ -102,6 +105,7 @@ class ParticipantServiceTest extends TestCase {
 			\OC::$server->getDatabaseConnection(),
 			$this->dispatcher,
 			$this->userManager,
+			$this->cloudIdManager,
 			$this->groupManager,
 			$this->membershipService,
 			$this->federationBackendNotifier,