diff --git a/lib/Controller/FederationController.php b/lib/Controller/FederationController.php index a237ecdf274..cb1f23940d3 100644 --- a/lib/Controller/FederationController.php +++ b/lib/Controller/FederationController.php @@ -2,9 +2,11 @@ declare(strict_types=1); /** - * @copyright Copyright (c) 2021, Gary Kim + * @copyright Copyright (c) 2024 Joas Schilling + * @copyright Copyright (c) 2021 Gary Kim * * @author Gary Kim + * @author Joas Schilling * @author Kate Döen * * @license GNU AGPL version 3 or any later version @@ -27,6 +29,7 @@ namespace OCA\Talk\Controller; use OCA\Talk\AppInfo\Application; +use OCA\Talk\Exceptions\CannotReachRemoteException; use OCA\Talk\Exceptions\RoomNotFoundException; use OCA\Talk\Exceptions\UnauthorizedException; use OCA\Talk\Federation\FederationManager; @@ -34,13 +37,11 @@ use OCA\Talk\Model\Invitation; use OCA\Talk\ResponseDefinitions; use OCA\Talk\Service\RoomFormatter; -use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCSController; -use OCP\DB\Exception as DBException; use OCP\IRequest; use OCP\IUser; use OCP\IUserSession; @@ -90,21 +91,29 @@ public function getResponseFormat(): string { * * @param int $id ID of the share * @psalm-param non-negative-int $id - * @return DataResponse - * @throws UnauthorizedException - * @throws DBException - * @throws MultipleObjectsReturnedException + * @return DataResponse|DataResponse|DataResponse * * 200: Invite accepted successfully + * 400: Invite can not be accepted (maybe it was accepted already) + * 404: Invite can not be found + * 410: Remote server could not be reached to notify about the acceptance */ #[NoAdminRequired] #[OpenAPI(scope: OpenAPI::SCOPE_FEDERATION)] public function acceptShare(int $id): DataResponse { $user = $this->userSession->getUser(); if (!$user instanceof IUser) { - throw new UnauthorizedException(); + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + try { + $participant = $this->federationManager->acceptRemoteRoomShare($user, $id); + } catch (CannotReachRemoteException) { + return new DataResponse(['error' => 'remote'], Http::STATUS_GONE); + } catch (UnauthorizedException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (\InvalidArgumentException $e) { + return new DataResponse(['error' => $e->getMessage()], $e->getMessage() === 'invitation' ? Http::STATUS_NOT_FOUND : Http::STATUS_BAD_REQUEST); } - $participant = $this->federationManager->acceptRemoteRoomShare($user, $id); return new DataResponse($this->roomFormatter->formatRoom( $this->getResponseFormat(), [], @@ -120,21 +129,25 @@ public function acceptShare(int $id): DataResponse { * * @param int $id ID of the share * @psalm-param non-negative-int $id - * @return DataResponse, array{}> - * @throws UnauthorizedException - * @throws DBException - * @throws MultipleObjectsReturnedException + * @return DataResponse, array{}>|DataResponse * * 200: Invite declined successfully + * 404: Invite can not be found */ #[NoAdminRequired] #[OpenAPI(scope: OpenAPI::SCOPE_FEDERATION)] public function rejectShare(int $id): DataResponse { $user = $this->userSession->getUser(); if (!$user instanceof IUser) { - throw new UnauthorizedException(); + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + try { + $this->federationManager->rejectRemoteRoomShare($user, $id); + } catch (UnauthorizedException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (\InvalidArgumentException $e) { + return new DataResponse(['error' => $e->getMessage()], Http::STATUS_NOT_FOUND); } - $this->federationManager->rejectRemoteRoomShare($user, $id); return new DataResponse(); } diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php index 6f5818275d3..a2169cec825 100644 --- a/lib/Controller/RoomController.php +++ b/lib/Controller/RoomController.php @@ -1051,7 +1051,7 @@ public function addParticipantToRoom(string $newParticipant, string $source = 'u foreach ($participants as $participant) { if ($participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS) { $participantsByUserId[$participant->getAttendee()->getActorId()] = $participant; - } elseif ($participant->getAttendee()->getAccessToken() === Attendee::ACTOR_FEDERATED_USERS) { + } elseif ($participant->getAttendee()->getActorType() === Attendee::ACTOR_FEDERATED_USERS) { $remoteParticipantsByFederatedId[$participant->getAttendee()->getActorId()] = $participant; } } diff --git a/lib/Federation/FederationManager.php b/lib/Federation/FederationManager.php index 31ed3aec2af..d2149d0f427 100644 --- a/lib/Federation/FederationManager.php +++ b/lib/Federation/FederationManager.php @@ -2,9 +2,11 @@ declare(strict_types=1); /** + * @copyright Copyright (c) 2024 Joas Schilling * @copyright Copyright (c) 2021 Gary Kim * * @author Gary Kim + * @author Joas Schilling * * @license GNU AGPL version 3 or any later version * @@ -104,14 +106,21 @@ protected function markNotificationProcessed(string $userId, int $shareId): void } /** - * @throws UnauthorizedException - * @throws DoesNotExistException + * @throws \InvalidArgumentException * @throws CannotReachRemoteException */ public function acceptRemoteRoomShare(IUser $user, int $shareId): Participant { - $invitation = $this->invitationMapper->getInvitationById($shareId); + try { + $invitation = $this->invitationMapper->getInvitationById($shareId); + } catch (DoesNotExistException $e) { + throw new \InvalidArgumentException('invitation'); + } if ($invitation->getUserId() !== $user->getUID()) { - throw new UnauthorizedException('invitation is for a different user'); + throw new UnauthorizedException('user'); + } + + if ($invitation->getState() === Invitation::STATE_ACCEPTED) { + throw new \InvalidArgumentException('state'); } // Add user to the room @@ -151,13 +160,17 @@ public function getRemoteShareById(int $shareId): Invitation { } /** + * @throws \InvalidArgumentException * @throws UnauthorizedException - * @throws DoesNotExistException */ public function rejectRemoteRoomShare(IUser $user, int $shareId): void { - $invitation = $this->invitationMapper->getInvitationById($shareId); + try { + $invitation = $this->invitationMapper->getInvitationById($shareId); + } catch (DoesNotExistException $e) { + throw new \InvalidArgumentException('invitation'); + } if ($invitation->getUserId() !== $user->getUID()) { - throw new UnauthorizedException('invitation is for a different user'); + throw new UnauthorizedException('user'); } $this->invitationMapper->delete($invitation); diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index bdfc67679cc..71837dbc729 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -415,7 +415,7 @@ protected function parseRemoteInvitationMessage(INotification $notification, IL1 [$sharedById, $sharedByServer] = $this->addressHandler->splitUserRemote($subjectParameters['sharedByFederatedId']); - $message = $l->t('{user1} shared room {roomName} on {remoteServer} with you'); + $message = $l->t('{user1} invited you to join {roomName} on {remoteServer}'); $rosParameters = [ 'user1' => [ @@ -436,16 +436,6 @@ protected function parseRemoteInvitationMessage(INotification $notification, IL1 ] ]; - $placeholders = $replacements = []; - foreach ($rosParameters as $placeholder => $parameter) { - $placeholders[] = '{' . $placeholder .'}'; - if ($parameter['type'] === 'user') { - $replacements[] = '@' . $parameter['name']; - } else { - $replacements[] = $parameter['name']; - } - } - $acceptAction = $notification->createAction(); $acceptAction->setParsedLabel($l->t('Accept')); $acceptAction->setLink($this->url->linkToOCSRouteAbsolute( @@ -463,8 +453,8 @@ protected function parseRemoteInvitationMessage(INotification $notification, IL1 ), IAction::TYPE_DELETE); $notification->addParsedAction($declineAction); - $notification->setParsedSubject(str_replace($placeholders, $replacements, $message)); - $notification->setRichSubject($message, $rosParameters); + $notification->setRichSubject($l->t('{user1} invited you to a federated conversation'), ['user1' => $rosParameters['user1']]); + $notification->setRichMessage($message, $rosParameters); return $notification; } diff --git a/openapi-federation.json b/openapi-federation.json index 3fd79dc83fc..56007fb351f 100644 --- a/openapi-federation.json +++ b/openapi-federation.json @@ -718,12 +718,113 @@ } } }, - "500": { - "description": "", + "400": { + "description": "Invite can not be accepted (maybe it was accepted already)", "content": { - "text/plain": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, + "410": { + "description": "Remote server could not be reached to notify about the acceptance", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, + "404": { + "description": "Invite can not be found", + "content": { + "application/json": { "schema": { - "type": "string" + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } } } } @@ -809,12 +910,37 @@ } } }, - "500": { - "description": "", + "404": { + "description": "Invite can not be found", "content": { - "text/plain": { + "application/json": { "schema": { - "type": "string" + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } } } } diff --git a/openapi-full.json b/openapi-full.json index 8cc474fa06c..d62a8b85076 100644 --- a/openapi-full.json +++ b/openapi-full.json @@ -17787,12 +17787,113 @@ } } }, - "500": { - "description": "", + "400": { + "description": "Invite can not be accepted (maybe it was accepted already)", "content": { - "text/plain": { + "application/json": { "schema": { - "type": "string" + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, + "410": { + "description": "Remote server could not be reached to notify about the acceptance", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, + "404": { + "description": "Invite can not be found", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } } } } @@ -17878,12 +17979,37 @@ } } }, - "500": { - "description": "", + "404": { + "description": "Invite can not be found", "content": { - "text/plain": { + "application/json": { "schema": { - "type": "string" + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } } } } diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 278f9f59785..b0dcd23ab11 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -518,7 +518,7 @@ public function userHasInvites(string $user, string $apiVersion, TableNode $form } /** - * @Then /^user "([^"]*)" (accepts|declines) invite to room "([^"]*)" of server "([^"]*)" \((v1)\)$/ + * @Then /^user "([^"]*)" (accepts|declines) invite to room "([^"]*)" of server "([^"]*)" with (\d+) \((v1)\)$/ * * @param string $user * @param string $roomName @@ -526,7 +526,7 @@ public function userHasInvites(string $user, string $apiVersion, TableNode $form * @param string $apiVersion * @param TableNode|null $formData */ - public function userAcceptsDeclinesRemoteInvite(string $user, string $acceptsDeclines, string $roomName, string $server, string $apiVersion, TableNode $formData = null): void { + public function userAcceptsDeclinesRemoteInvite(string $user, string $acceptsDeclines, string $roomName, string $server, int $status, string $apiVersion, TableNode $formData = null): void { $inviteId = self::$remoteToInviteId[$server . '::' . $roomName]; $verb = $acceptsDeclines === 'accepts' ? 'POST' : 'DELETE'; @@ -535,11 +535,15 @@ public function userAcceptsDeclinesRemoteInvite(string $user, string $acceptsDec if ($server === 'LOCAL') { $this->sendRemoteRequest($verb, '/apps/spreed/api/' . $apiVersion . '/federation/invitation/' . $inviteId); } - $this->assertStatusCode($this->response, 200); + $this->assertStatusCode($this->response, $status); $response = $this->getDataFromResponse($this->response); if ($formData) { - $this->assertRooms([$response], $formData); + if ($status === 200) { + $this->assertRooms([$response], $formData); + } else { + Assert::assertSame($formData->getRowsHash(), $response); + } } else { Assert::assertEmpty($response); } @@ -2755,7 +2759,7 @@ public function userSeesTheFollowingSystemMessagesInRoom($user, $identifier, $st $expected = $formData->getHash(); - Assert::assertCount(count($expected), $messages, 'Message count does not match'); + Assert::assertCount(count($expected), $messages, 'Message count does not match:' . "\n" . json_encode($messages, JSON_PRETTY_PRINT)); Assert::assertEquals($expected, array_map(function ($message, $expected) { $data = [ 'room' => self::$tokenToIdentifier[$message['token']], diff --git a/tests/integration/features/federation/invite.feature b/tests/integration/features/federation/invite.feature index 7b4192cf3a7..dcb728d5533 100644 --- a/tests/integration/features/federation/invite.feature +++ b/tests/integration/features/federation/invite.feature @@ -32,6 +32,15 @@ Feature: federation/invite | 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 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 | @@ -45,11 +54,13 @@ Feature: federation/invite | remoteServerUrl | remoteToken | 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 | - And user "participant2" accepts invite to room "room" of server "LOCAL" (v1) + | 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" has the following invitations (v1) | remoteServerUrl | remoteToken | state | | LOCAL | room | 1 | @@ -95,9 +106,11 @@ Feature: federation/invite | remoteServerUrl | remoteToken | 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 | - And user "participant2" declines invite to room "room" of server "LOCAL" (v1) + | 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" declines invite to room "room" of server "LOCAL" with 200 (v1) + And user "participant2" declines invite to room "room" of server "LOCAL" with 404 (v1) + | error | invitation | And user "participant2" has the following invitations (v1) When user "participant1" sees the following attendees in room "room" with 200 (v4) | actorType | actorId | participantType | @@ -128,8 +141,8 @@ Feature: federation/invite | remoteServerUrl | remoteToken | 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 | + | 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 | 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) @@ -153,7 +166,7 @@ Feature: federation/invite And user "participant2" has the following invitations (v1) | remoteServerUrl | remoteToken | state | | LOCAL | room | 0 | - And user "participant2" accepts invite to room "room" of server "LOCAL" (v1) + And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) | id | name | type | remoteServer | remoteToken | | room | room | 2 | LOCAL | room | And user "participant2" has the following invitations (v1) @@ -179,7 +192,7 @@ Feature: federation/invite And user "participant2" has the following invitations (v1) | remoteServerUrl | remoteToken | state | | LOCAL | room | 0 | - And user "participant2" accepts invite to room "room" of server "LOCAL" (v1) + And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) | id | name | type | remoteServer | remoteToken | | room | room | 2 | LOCAL | room | Then user "participant2" is participant of the following rooms (v4)