fix(federated-sharing): avoid duplicate shares

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>

Author: hamza221

apps/federatedfilesharing/lib/FederatedShareProvider.php

@@ -30,6 +30,7 @@
 use OCP\IUserManager;
 use OCP\Security\ISecureRandom;
 use OCP\Server;
+use OCP\Share\Exceptions\AlreadySharedException;
 use OCP\Share\Exceptions\GenericShareException;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IShare;
@@ -101,20 +102,25 @@ public function create(IShare $share): IShare {
 			throw new \Exception($message_t);
 		}
 
+		$cloudId = $this->cloudIdManager->resolveCloudId($shareWith);
+
 		/*
-		 * Check if file is not already shared with the remote user
+		 * Check if file is not already shared with the remote user.
+		 * Has to be looked up by the normalized cloud ID, because that is what
+		 * gets stored below. Otherwise spellings like "user@server.com/" slip
+		 * past this check and create a duplicate share.
 		 */
-		$alreadyShared = $this->getSharedWith($shareWith, IShare::TYPE_REMOTE, $share->getNode(), 1, 0);
-		$alreadySharedGroup = $this->getSharedWith($shareWith, IShare::TYPE_REMOTE_GROUP, $share->getNode(), 1, 0);
-		if (!empty($alreadyShared) || !empty($alreadySharedGroup)) {
+		// getSharedWith() ignores its $shareType argument and always
+		// queries all supported remote types, so a single lookup covers both.
+		$alreadyShared = $this->getSharedWith($cloudId->getId(), IShare::TYPE_REMOTE, $share->getNode(), 1, 0);
+		if (!empty($alreadyShared)) {
 			$message = 'Sharing %1$s failed, because this item is already shared with %2$s';
 			$message_t = $this->l->t('Sharing %1$s failed, because this item is already shared with the account %2$s', [$share->getNode()->getName(), $shareWith]);
 			$this->logger->debug(sprintf($message, $share->getNode()->getName(), $shareWith), ['app' => 'Federated File Sharing']);
-			throw new \Exception($message_t);
+			throw new AlreadySharedException($message_t, $alreadyShared[0]);
 		}
 
 		// don't allow federated shares if source and target server are the same
-		$cloudId = $this->cloudIdManager->resolveCloudId($shareWith);
 		$currentServer = $this->addressHandler->generateRemoteURL();
 		$currentUser = $sharedBy;
 		if ($this->addressHandler->compareAddresses($cloudId->getUser(), $cloudId->getRemote(), $currentUser, $currentServer)) {

apps/federatedfilesharing/tests/FederatedShareProviderTest.php

@@ -32,6 +32,7 @@
 use OCP\IUserManager;
 use OCP\Security\ISecureRandom;
 use OCP\Server;
+use OCP\Share\Exceptions\AlreadySharedException;
 use OCP\Share\IManager;
 use OCP\Share\IShare;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -418,6 +419,66 @@ public function testCreateAlreadyShared(): void {
 		}
 	}
 
+	public static function dataTestCreateAlreadySharedWithEquivalentCloudId(): array {
+		return [
+			['user@server.com'],
+			['user@server.com/'],
+			['user@server.com/index.php'],
+		];
+	}
+
+	/**
+	 * Sharing a node with a recipient it is already shared with has to be
+	 * rejected with an AlreadySharedException, for every spelling that
+	 * normalizes to the same cloud ID.
+	 */
+	#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestCreateAlreadySharedWithEquivalentCloudId')]
+	public function testCreateAlreadySharedWithEquivalentCloudId(string $shareWith): void {
+		$node = $this->createMock(File::class);
+		$node->method('getId')->willReturn(42);
+		$node->method('getName')->willReturn('myFile');
+
+		$this->addressHandler->expects($this->any())->method('splitUserRemote')
+			->willReturn(['user', 'server.com']);
+		$this->addressHandler->expects($this->any())->method('generateRemoteURL')
+			->willReturn('http://localhost/');
+		$this->tokenHandler->method('generateToken')->willReturn('token');
+		$this->contactsManager->expects($this->any())->method('search')
+			->willReturn([]);
+		$this->notifications->expects($this->once())
+			->method('sendRemoteShare')
+			->willReturn(true);
+
+		$share = $this->shareManager->newShare();
+		$share->setSharedWith('user@server.com')
+			->setSharedBy('sharedBy')
+			->setShareOwner('shareOwner')
+			->setPermissions(19)
+			->setShareType(IShare::TYPE_REMOTE)
+			->setNode($node)
+			->setTarget('');
+		$existingShare = $this->provider->create($share);
+
+		// a recipient of the node re-sharing it with the same account
+		$duplicate = $this->shareManager->newShare();
+		$duplicate->setSharedWith($shareWith)
+			->setSharedBy('otherRecipient')
+			->setShareOwner('shareOwner')
+			->setPermissions(19)
+			->setShareType(IShare::TYPE_REMOTE)
+			->setNode($node)
+			->setTarget('');
+
+		try {
+			$this->provider->create($duplicate);
+			$this->fail('Expected an AlreadySharedException');
+		} catch (AlreadySharedException $e) {
+			$this->assertEquals($existingShare->getId(), $e->getExistingShare()->getId());
+		}
+
+		$this->assertCount(1, $this->provider->getSharesByPath($node));
+	}
+
 	#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestUpdate')]
 	public function testUpdate(string $owner, string $sharedBy, ?\DateTime $expirationDate): void {
 		$this->provider = $this->getMockBuilder(FederatedShareProvider::class)