fix(federation): Fix accepting or declining an invite multiple times

Signed-off-by: Joas Schilling <coding@schilljs.com>

6 files changed, 333 insertions, 47 deletions

diff --git a/lib/Controller/FederationController.php b/lib/Controller/FederationController.php
index a237ecdf2..cb1f23940 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 <gary@garykim.dev>
+ * @copyright Copyright (c) 2024 Joas Schilling <coding@schilljs.com>
+ * @copyright Copyright (c) 2021 Gary Kim <gary@garykim.dev>
  *
  * @author Gary Kim <gary@garykim.dev>
+ * @author Joas Schilling <coding@schilljs.com>
  * @author Kate Döen <kate.doeen@nextcloud.com>
  *
  * @license GNU AGPL version 3 or any later version
@@ -27,6 +29,7 @@ declare(strict_types=1);
 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\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;
@@ -90,21 +91,29 @@ class FederationController extends OCSController {
 	 *
 	 * @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(),
 			[],
@@ -120,21 +129,25 @@ class FederationController extends OCSController {
 	 *
 	 * @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();
 	}
 
diff --git a/lib/Federation/FederationManager.php b/lib/Federation/FederationManager.php
index 31ed3aec2..d2149d0f4 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 <coding@schilljs.com>
  * @copyright Copyright (c) 2021 Gary Kim <gary@garykim.dev>
  *
  * @author Gary Kim <gary@garykim.dev>
+ * @author Joas Schilling <coding@schilljs.com>
  *
  * @license GNU AGPL version 3 or any later version
  *
@@ -104,14 +106,21 @@ class FederationManager {
 	}
 
 	/**
-	 * @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 @@ class FederationManager {
 	}
 
 	/**
+	 * @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/openapi-federation.json b/openapi-federation.json
index 3fd79dc83..56007fb35 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 8cc474fa0..d62a8b850 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 278f9f597..b0dcd23ab 100644
--- a/tests/integration/features/bootstrap/FeatureContext.php
+++ b/tests/integration/features/bootstrap/FeatureContext.php
@@ -518,7 +518,7 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	}
 
 	/**
-	 * @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 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	 * @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 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 		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 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 
 		$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 7b4192cf3..63843cbbd 100644
--- a/tests/integration/features/federation/invite.feature
+++ b/tests/integration/features/federation/invite.feature
@@ -47,9 +47,11 @@ Feature: federation/invite
     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)
+    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     |
@@ -97,7 +99,9 @@ Feature: federation/invite
     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)
+    And user "participant2" declines invite to room "room" of server "LOCAL" with 200 (v1)
+    And user