Merge pull request #62925 from nextcloud/leftybournes/fix/trashbin_show_deletedby

fix(trashbin): properly show deleted by for federated shares

Author: leftybournes

apps/files_trashbin/lib/Trash/LegacyTrashBackend.php

@@ -12,6 +12,7 @@
 use OCA\Files_Trashbin\Helper;
 use OCA\Files_Trashbin\Storage;
 use OCA\Files_Trashbin\Trashbin;
+use OCP\Federation\ICloudIdManager;
 use OCP\Files\FileInfo;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
@@ -28,6 +29,7 @@ class LegacyTrashBackend implements ITrashBackend {
 	public function __construct(
 		private readonly IRootFolder $rootFolder,
 		private readonly IUserManager $userManager,
+		private readonly ICloudIdManager $cloudIdManager,
 	) {
 	}
 
@@ -40,7 +42,7 @@ private function mapTrashItem(FileInfo $file, IUser $user, ?ITrashItem $parent =
 			$originalLocation = $file->getName();
 		}
 		/** @psalm-suppress UndefinedInterfaceMethod */
-		$deletedBy = $this->userManager->get($file['deletedBy']) ?? $parent?->getDeletedBy();
+		$deletedBy = $this->resolveDeletedBy($file['deletedBy']) ?? $parent?->getDeletedBy();
 		$trashFilename = Trashbin::getTrashFilename($file->getName(), $file->getMtime());
 		return new TrashItem(
 			$this,
@@ -127,4 +129,29 @@ public function getTrashNodeById(IUser $user, int $fileId): ?Node {
 			return null;
 		}
 	}
+
+	/**
+	 * Resolve the user that deleted a trash item. Files deleted by a federated share
+	 * recipient only carry the recipient's remote cloud ID, which no local IUserManager
+	 * backend can resolve, so fall back to a display-only user for the cloud ID in that
+	 * case instead of leaving the item without an "Unknown" deleted by user.
+	 */
+	private function resolveDeletedBy(?string $uid): ?IUser {
+		if (!$uid) {
+			return null;
+		}
+
+		$user = $this->userManager->get($uid);
+		if ($user !== null) {
+			return $user;
+		}
+
+		try {
+			$cloudId = $this->cloudIdManager->resolveCloudId($uid);
+		} catch (\InvalidArgumentException $e) {
+			return null;
+		}
+
+		return $this->userManager->getFederatedUser($cloudId);
+	}
 }

lib/private/User/Manager.php

@@ -13,6 +13,7 @@
 use OCP\Config\IUserConfig;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Federation\ICloudId;
 use OCP\HintException;
 use OCP\ICache;
 use OCP\ICacheFactory;
@@ -883,4 +884,9 @@ public function getAvatarUrlLight(string $userId, int $size): string {
 	public function getAvatarUrlDark(string $userId, int $size): string {
 		return ($this->urlGenerator ??= Server::get(IURLGenerator::class))->linkToRouteAbsolute('core.avatar.getAvatarDark', ['userId' => $userId, 'size' => $size]);
 	}
+
+	#[\Override]
+	public function getFederatedUser(ICloudId $cloudId): IUser {
+		return new LazyUser($cloudId->getDisplayId(), $this, $cloudId->getDisplayId());
+	}
 }

lib/public/IUserManager.php

@@ -282,4 +282,15 @@ public function getAvatarUrlLight(string $userId, int $size): string;
 	 * @since 34.0.0
 	 */
 	public function getAvatarUrlDark(string $userId, int $size): string;
+
+	/**
+	 * Get a read-only user from a cloud ID for showing the display name of a remote
+	 * federation user (e.g. the "deleted by" user of a federated share) that has no
+	 * local account.
+	 *
+	 * @param \OCP\Federation\ICloudId $federatedUserId A cloud ID of the federated user
+	 * @return IUser
+	 * @since 35.0.0
+	 */
+	public function getFederatedUser(\OCP\Federation\ICloudId $cloudId): IUser;
 }

tests/lib/User/ManagerTest.php

@@ -15,6 +15,7 @@
 use OC\User\User;
 use OCP\Config\IUserConfig;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Federation\ICloudId;
 use OCP\ICache;
 use OCP\ICacheFactory;
 use OCP\IConfig;
@@ -736,4 +737,18 @@ public function testGetAvatarUrlLight(): void {
 	public function testGetAvatarUrlDark(): void {
 		$this->assertEquals('http://localhost/index.php/avatar/userid/64/dark', $this->manager->getAvatarUrlDark('userid', 64));
 	}
+
+	public function testGetFederatedUser(): void {
+		$userId = 'test@example.com';
+
+		$cloudId = $this->createMock(ICloudId::class);
+		$cloudId->expects($this->exactly(2))
+			->method('getDisplayId')
+			->willReturn($userId);
+
+		$user = $this->manager->getFederatedUser($cloudId);
+
+		$this->assertEquals($userId, $user->getUID());
+		$this->assertEquals($userId, $user->getDisplayName());
+	}
 }