Browse Source

fix(federation): Prevent duplicated invites with different casing and heal the cloudId

Signed-off-by: Joas Schilling <coding@schilljs.com>
pull/11922/head
Joas Schilling 2 years ago
parent
commit
a5b1eb6692
No known key found for this signature in database GPG Key ID: 74434EFE0D2E2205
  1. 2
      lib/Federation/BackendNotifier.php
  2. 5
      lib/Federation/CloudFederationProviderTalk.php
  3. 21
      lib/Federation/FederationManager.php
  4. 10
      lib/Model/InvitationMapper.php
  5. 3
      tests/integration/features/bootstrap/FeatureContext.php
  6. 64
      tests/integration/features/federation/invite.feature
  7. 3
      tests/php/Federation/FederationTest.php

2
lib/Federation/BackendNotifier.php

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

5
lib/Federation/CloudFederationProviderTalk.php

@ -219,6 +219,11 @@ class CloudFederationProviderTalk implements ICloudFederationProvider {
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);
}

21
lib/Federation/FederationManager.php

@ -39,7 +39,10 @@ use OCA\Talk\Participant;
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 @@ class FederationManager {
private InvitationMapper $invitationMapper,
private BackendNotifier $backendNotifier,
private IManager $notificationManager,
private ICloudIdManager $cloudIdManager,
private RestrictionValidator $restrictionValidator,
) {
}
@ -97,12 +101,23 @@ class FederationManager {
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 @@ class FederationManager {
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 @@ class FederationManager {
$attendee = array_pop($attendees);
$invitation->setState(Invitation::STATE_ACCEPTED);
$invitation->setLocalCloudId($cloudId->getId());
$this->invitationMapper->update($invitation);
$this->markNotificationProcessed($user->getUID(), $shareId);

10
lib/Model/InvitationMapper.php

@ -98,14 +98,18 @@ class InvitationMapper extends QBMapper {
/**
* @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);
}

3
tests/integration/features/bootstrap/FeatureContext.php

@ -586,6 +586,9 @@ class FeatureContext implements Context, SnippetAcceptingContext {
if (isset($expectedInvite['state'])) {
$data['state'] = $invite['state'];
}
if (isset($expectedInvite['localCloudId'])) {
$data['localCloudId'] = $invite['localCloudId'];
}
return $data;
}, $invites, $expectedInvites));

64
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 |

3
tests/php/Federation/FederationTest.php

@ -417,6 +417,7 @@ class FederationTest extends TestCase {
'message' => 'Recipient accepted the share',
'remoteServerUrl' => 'http://example.tld',
'displayName' => 'Foo Bar',
'cloudId' => 'cloudId@example.tld',
]
);
@ -441,7 +442,7 @@ class FederationTest extends TestCase {
->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', 'cloudId@example.tld');
$this->assertTrue($success);
}

Loading…
Cancel
Save