From 62709127257e4807f296ca690a565fd0444086ca Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 5 Dec 2024 11:53:52 +0100 Subject: [PATCH 1/3] fix(federation): Implement new federation method to validate origin of OCM messages Signed-off-by: Joas Schilling --- .../CloudFederationProviderTalk.php | 27 ++++++++++++++++++- lib/Model/AttendeeMapper.php | 14 ++++++++++ lib/Model/InvitationMapper.php | 19 +++++++++++++ tests/psalm-baseline.xml | 2 +- 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php index 36dea0bf39d..394d3fcf08c 100644 --- a/lib/Federation/CloudFederationProviderTalk.php +++ b/lib/Federation/CloudFederationProviderTalk.php @@ -9,6 +9,7 @@ namespace OCA\Talk\Federation; use Exception; +use NCU\Federation\ISignedCloudFederationProvider; use OCA\FederatedFileSharing\AddressHandler; use OCA\Talk\AppInfo\Application; use OCA\Talk\CachePrefix; @@ -59,7 +60,7 @@ use Psr\Log\LoggerInterface; use SensitiveParameter; -class CloudFederationProviderTalk implements ICloudFederationProvider { +class CloudFederationProviderTalk implements ICloudFederationProvider, ISignedCloudFederationProvider { protected ?ICache $proxyCacheMessages; public function __construct( @@ -632,4 +633,28 @@ private function validSharedRoomTypes(): array { public function getSupportedShareTypes(): array { return ['user']; } + + /** + * @inheritDoc + */ + public function getFederationIdFromSharedSecret( + #[SensitiveParameter] + string $sharedSecret, + array $payload, + ): string { + try { + $invite = $this->invitationMapper->getByRemoteServerOnlyWithAccessToken($payload['remoteServerUrl'], $sharedSecret); + return $invite->getInviterCloudId(); + } catch (DoesNotExistException) { + } + + $attendees = $this->attendeeMapper->getByAccessToken($sharedSecret); + foreach ($attendees as $attendee) { + if (str_ends_with($attendee->getActorId(), $payload['remoteServerUrl'])) { + return $attendee->getActorId(); + } + } + + return ''; + } } diff --git a/lib/Model/AttendeeMapper.php b/lib/Model/AttendeeMapper.php index b3e73c5c582..55fb2298cad 100644 --- a/lib/Model/AttendeeMapper.php +++ b/lib/Model/AttendeeMapper.php @@ -53,6 +53,20 @@ public function getById(int $id): Attendee { return $this->findEntity($query); } + /** + * @return list + */ + public function getByAccessToken(string $accessToken): array { + $query = $this->db->getQueryBuilder(); + $query->select('*') + ->from($this->getTableName()) + ->where($query->expr()->eq('access_token', $query->createNamedParameter($accessToken))); + // There could be multiple in case of local federation, + // so we have to get all and afterwards check + // the actor id for the serverUrl. + return $this->findEntities($query); + } + /** * @throws DoesNotExistException * @throws MultipleObjectsReturnedException diff --git a/lib/Model/InvitationMapper.php b/lib/Model/InvitationMapper.php index 07b9197506c..5751d8ed28b 100644 --- a/lib/Model/InvitationMapper.php +++ b/lib/Model/InvitationMapper.php @@ -44,6 +44,25 @@ public function getInvitationById(int $id): Invitation { return $this->findEntity($qb); } + /** + * @throws DoesNotExistException + * @internal Does not check user relation + */ + public function getByRemoteServerOnlyWithAccessToken( + string $remoteServerUrl, + #[SensitiveParameter] + string $accessToken, + ): Invitation { + $qb = $this->db->getQueryBuilder(); + + $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->eq('remote_server_url', $qb->createNamedParameter($remoteServerUrl))) + ->andWhere($qb->expr()->eq('access_token', $qb->createNamedParameter($accessToken))); + + return $this->findEntity($qb); + } + /** * @throws DoesNotExistException */ diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index b3031a6c16a..6699ce52890 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,5 +1,5 @@ - + From 408300ba438897c84faf634d692754df9653b280 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 5 Dec 2024 16:34:07 +0100 Subject: [PATCH 2/3] test(federation): Can not test local federation anymore with signed OCM notifications Signed-off-by: Joas Schilling --- .../features/federation/invite.feature | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/tests/integration/features/federation/invite.feature b/tests/integration/features/federation/invite.feature index dabe572e755..1a5104c41f9 100644 --- a/tests/integration/features/federation/invite.feature +++ b/tests/integration/features/federation/invite.feature @@ -1,5 +1,10 @@ Feature: federation/invite Background: + Given using server "REMOTE" + Given user "participant2" exists + And the following "spreed" app config is set + | federation_enabled | yes | + Given using server "LOCAL" Given user "participant1" exists Given user "participant2" exists @@ -235,25 +240,26 @@ Feature: federation/invite Given user "participant1" creates room "room" (v4) | roomType | 3 | | roomName | room | - And user "participant1" adds federated_user "participant2" to room "room" with 200 (v4) + And user "participant1" adds federated_user "participant2@REMOTE" 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 | + | actorType | actorId | participantType | + | users | participant1 | 1 | + | federated_users | participant2@{$REMOTE_URL} | 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-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-displayname","server":"http:\/\/localhost:8280"}} | | 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 200 (v4) + And user "participant1" adds federated_user "participant2@REMOTE" 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 | + | actorType | actorId | participantType | + | users | participant1 | 1 | + | federated_users | participant2@{$REMOTE_URL} | 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-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-displayname","server":"http:\/\/localhost:8280"}} | | 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 using server "REMOTE" And user "participant2" has the following invitations (v1) | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | @@ -270,14 +276,15 @@ Feature: federation/invite And user "participant2" removes themselves from room "LOCAL::room" with 200 (v4) And user "participant2" has the following invitations (v1) Then user "participant2" is participant of the following rooms (v4) + And using server "LOCAL" 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 | federated_users | participant2@http://localhost:8180 | federated_user_removed | {federated_user} declined 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 | 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 | federated_users | participant2@http://localhost:8280 | federated_user_removed | {federated_user} declined the invitation | {"actor":{"type":"user","id":"participant2","name":"participant2@localhost:8280","server":"http:\/\/localhost:8280"},"federated_user":{"type":"user","id":"participant2","name":"participant2@localhost:8280","server":"http:\/\/localhost:8280"}} | + | room | federated_users | participant2@http://localhost:8280 | federated_user_added | {federated_user} accepted the invitation | {"actor":{"type":"user","id":"participant2","name":"participant2@localhost:8280","server":"http:\/\/localhost:8280"},"federated_user":{"type":"user","id":"participant2","name":"participant2@localhost:8280","server":"http:\/\/localhost:8280"}} | + | 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:8280","server":"http:\/\/localhost:8280"}} | | room | users | participant1 | conversation_created | You created the conversation | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | Scenario: Federate conversation meta data From a7d34f452e1f33da7e115fe5edabcf95c4eab685 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 10 Dec 2024 12:41:05 +0100 Subject: [PATCH 3/3] fix: Rename method Signed-off-by: Joas Schilling --- lib/Federation/CloudFederationProviderTalk.php | 2 +- lib/Model/InvitationMapper.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php index 394d3fcf08c..c0e5c7871c8 100644 --- a/lib/Federation/CloudFederationProviderTalk.php +++ b/lib/Federation/CloudFederationProviderTalk.php @@ -643,7 +643,7 @@ public function getFederationIdFromSharedSecret( array $payload, ): string { try { - $invite = $this->invitationMapper->getByRemoteServerOnlyWithAccessToken($payload['remoteServerUrl'], $sharedSecret); + $invite = $this->invitationMapper->getByRemoteServerAndAccessToken($payload['remoteServerUrl'], $sharedSecret); return $invite->getInviterCloudId(); } catch (DoesNotExistException) { } diff --git a/lib/Model/InvitationMapper.php b/lib/Model/InvitationMapper.php index 5751d8ed28b..82f8bef4b5d 100644 --- a/lib/Model/InvitationMapper.php +++ b/lib/Model/InvitationMapper.php @@ -48,7 +48,7 @@ public function getInvitationById(int $id): Invitation { * @throws DoesNotExistException * @internal Does not check user relation */ - public function getByRemoteServerOnlyWithAccessToken( + public function getByRemoteServerAndAccessToken( string $remoteServerUrl, #[SensitiveParameter] string $accessToken,