Skip to content

Commit

Permalink
Merge pull request #11513 from nextcloud/bugfix/noid/fix-accepting-an…
Browse files Browse the repository at this point in the history 
…d-declining-invites

fix(federation): Fix accepting or declining an invite multiple times
  • Loading branch information
@nickvergessen
nickvergessen authored Feb 5, 2024
2 parents 6e12a09 + 903ecd3 commit 9f7ebde
Show file tree
Hide file tree
Showing 8 changed files with 352 additions and 67 deletions.
43 changes: 28 additions & 15 deletions lib/Controller/FederationController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

declare(strict_types=1);
/**
* @copyright Copyright (c) 2021, Gary Kim <[email protected]>
* @copyright Copyright (c) 2024 Joas Schilling <[email protected]>
* @copyright Copyright (c) 2021 Gary Kim <[email protected]>
*
* @author Gary Kim <[email protected]>
* @author Joas Schilling <[email protected]>
* @author Kate Döen <[email protected]>
*
* @license GNU AGPL version 3 or any later version
Expand All @@ -27,20 +29,19 @@
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;
use OCA\Talk\Manager;
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;
Expand Down Expand Up @@ -90,21 +91,29 @@ public function getResponseFormat(): string {
*
* @param int $id ID of the share
* @psalm-param non-negative-int $id
* @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>
* @throws UnauthorizedException
* @throws DBException
* @throws MultipleObjectsReturnedException
* @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_GONE, array{error: string}, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{error?: string}, array{}>
*
* 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(),
[],
Expand All @@ -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<Http::STATUS_OK, array<empty>, array{}>
* @throws UnauthorizedException
* @throws DBException
* @throws MultipleObjectsReturnedException
* @return DataResponse<Http::STATUS_OK, array<empty>, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{error?: string}, array{}>
*
* 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();
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Controller/RoomController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
27 changes: 20 additions & 7 deletions lib/Federation/FederationManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

declare(strict_types=1);
/**
* @copyright Copyright (c) 2024 Joas Schilling <[email protected]>
* @copyright Copyright (c) 2021 Gary Kim <[email protected]>
*
* @author Gary Kim <[email protected]>
* @author Joas Schilling <[email protected]>
*
* @license GNU AGPL version 3 or any later version
*
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 3 additions & 13 deletions lib/Notification/Notifier.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' => [
Expand All @@ -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(
Expand All @@ -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;
}
Expand Down
142 changes: 134 additions & 8 deletions openapi-federation.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
}
}
}
}
}
}
Expand Down Expand Up @@ -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"
}
}
}
}
}
}
}
}
}
Expand Down
Loading

0 comments on commit 9f7ebde

Please sign in to comment.