Skip to content

Commit

Permalink
fix(federation): Send disinvite on remote user removal and remove invite
Browse files Browse the repository at this point in the history
Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
nickvergessen committed Jan 29, 2024
1 parent b26caa8 commit b3bdf65
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 5 deletions.
21 changes: 18 additions & 3 deletions lib/Federation/BackendNotifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,12 @@ public function sendRemoteShare(
*
* @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();
Expand Down Expand Up @@ -160,7 +165,12 @@ public function sendShareAccepted(string $remoteServerUrl, int $remoteAttendeeId
* 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();
Expand All @@ -185,7 +195,12 @@ public function sendShareDeclined(string $remoteServerUrl, int $remoteAttendeeId
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();
Expand Down
1 change: 1 addition & 0 deletions lib/Federation/CloudFederationProviderTalk.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ private function shareUnshared(int $remoteAttendeeId, array $notification): arra
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 [];
Expand Down
13 changes: 13 additions & 0 deletions lib/Service/ParticipantService.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
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;
Expand Down Expand Up @@ -102,6 +103,7 @@ public function __construct(
protected IDBConnection $connection,
private IEventDispatcher $dispatcher,
private IUserManager $userManager,
private ICloudIdManager $cloudIdManager,
private IGroupManager $groupManager,
private MembershipService $membershipService,
private BackendNotifier $backendNotifier,
Expand Down Expand Up @@ -822,6 +824,17 @@ public function leaveRoomAsSession(Room $room, Participant $participant, bool $d
* @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) {
Expand Down
9 changes: 7 additions & 2 deletions tests/integration/features/bootstrap/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ public function userHasInvites(string $user, string $apiVersion, TableNode $form
$invites = $this->getDataFromResponse($this->response);

if ($formData === null) {
Assert::assertEmpty($invites);
Assert::assertEmpty($invites, json_encode($invites, JSON_PRETTY_PRINT));
return;
}

Expand Down Expand Up @@ -1370,7 +1370,7 @@ public function userRemovesUserFromRoom(string $user, string $toRemove, string $
}

/**
* @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
Expand All @@ -1383,6 +1383,11 @@ public function userRemovesAttendeeFromRoom(string $user, string $actorType, str
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);
}

Expand Down
47 changes: 47 additions & 0 deletions tests/integration/features/federation/invite.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -95,6 +108,40 @@ 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)
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 |
Expand Down
4 changes: 4 additions & 0 deletions tests/php/Service/ParticipantServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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;
Expand All @@ -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 */
Expand All @@ -87,6 +89,7 @@ public function setUp(): void {
$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);
Expand All @@ -102,6 +105,7 @@ public function setUp(): void {
\OC::$server->getDatabaseConnection(),
$this->dispatcher,
$this->userManager,
$this->cloudIdManager,
$this->groupManager,
$this->membershipService,
$this->federationBackendNotifier,
Expand Down

0 comments on commit b3bdf65

Please sign in to comment.