Skip to content

Commit 2ae70cf

Browse files
Merge pull request #62966 from nextcloud/backport/62925/stable33
[stable33] fix(trashbin): properly show deleted by for federated shares
2 parents 7acdcec + 4a6a910 commit 2ae70cf

4 files changed

Lines changed: 60 additions & 1 deletion

File tree

apps/files_trashbin/lib/Trash/LegacyTrashBackend.php

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use OCA\Files_Trashbin\Helper;
1212
use OCA\Files_Trashbin\Storage;
1313
use OCA\Files_Trashbin\Trashbin;
14+
use OCP\Federation\ICloudIdManager;
1415
use OCP\Files\FileInfo;
1516
use OCP\Files\Folder;
1617
use OCP\Files\IRootFolder;
@@ -26,6 +27,7 @@ class LegacyTrashBackend implements ITrashBackend {
2627
public function __construct(
2728
private IRootFolder $rootFolder,
2829
private IUserManager $userManager,
30+
private ICloudIdManager $cloudIdManager,
2931
) {
3032
}
3133

@@ -38,7 +40,7 @@ private function mapTrashItem(FileInfo $file, IUser $user, ?ITrashItem $parent =
3840
$originalLocation = $file->getName();
3941
}
4042
/** @psalm-suppress UndefinedInterfaceMethod */
41-
$deletedBy = $this->userManager->get($file['deletedBy']) ?? $parent?->getDeletedBy();
43+
$deletedBy = $this->resolveDeletedBy($file['deletedBy']) ?? $parent?->getDeletedBy();
4244
$trashFilename = Trashbin::getTrashFilename($file->getName(), $file->getMtime());
4345
return new TrashItem(
4446
$this,
@@ -119,4 +121,29 @@ public function getTrashNodeById(IUser $user, int $fileId) {
119121
return null;
120122
}
121123
}
124+
125+
/**
126+
* Resolve the user that deleted a trash item. Files deleted by a federated share
127+
* recipient only carry the recipient's remote cloud ID, which no local IUserManager
128+
* backend can resolve, so fall back to a display-only user for the cloud ID in that
129+
* case instead of leaving the item without an "Unknown" deleted by user.
130+
*/
131+
private function resolveDeletedBy(?string $uid): ?IUser {
132+
if (!$uid) {
133+
return null;
134+
}
135+
136+
$user = $this->userManager->get($uid);
137+
if ($user !== null) {
138+
return $user;
139+
}
140+
141+
try {
142+
$cloudId = $this->cloudIdManager->resolveCloudId($uid);
143+
} catch (\InvalidArgumentException $e) {
144+
return null;
145+
}
146+
147+
return $this->userManager->getFederatedUser($cloudId);
148+
}
122149
}

lib/private/User/Manager.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OCP\Config\IUserConfig;
1313
use OCP\DB\QueryBuilder\IQueryBuilder;
1414
use OCP\EventDispatcher\IEventDispatcher;
15+
use OCP\Federation\ICloudId;
1516
use OCP\HintException;
1617
use OCP\ICache;
1718
use OCP\ICacheFactory;
@@ -851,4 +852,8 @@ public function getSeenUsers(int $offset = 0, ?int $limit = null): \Iterator {
851852
public function getExistingUser(string $userId, ?string $displayName = null): IUser {
852853
return new LazyUser($userId, $this, $displayName);
853854
}
855+
856+
public function getFederatedUser(ICloudId $cloudId): IUser {
857+
return new LazyUser($cloudId->getDisplayId(), $this, $cloudId->getDisplayId());
858+
}
854859
}

lib/public/IUserManager.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,4 +267,15 @@ public function getSeenUsers(int $offset = 0, ?int $limit = null): \Iterator;
267267
* @since 33.0.0
268268
*/
269269
public function getExistingUser(string $userId, ?string $displayName = null): IUser;
270+
271+
/**
272+
* Get a read-only user from a cloud ID for showing the display name of a remote
273+
* federation user (e.g. the "deleted by" user of a federated share) that has no
274+
* local account.
275+
*
276+
* @param \OCP\Federation\ICloudId $federatedUserId A cloud ID of the federated user
277+
* @return IUser
278+
* @since 35.0.0
279+
*/
280+
public function getFederatedUser(\OCP\Federation\ICloudId $cloudId): IUser;
270281
}

tests/lib/User/ManagerTest.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OC\User\User;
1616
use OCP\Config\IUserConfig;
1717
use OCP\EventDispatcher\IEventDispatcher;
18+
use OCP\Federation\ICloudId;
1819
use OCP\ICache;
1920
use OCP\ICacheFactory;
2021
use OCP\IConfig;
@@ -762,4 +763,19 @@ public function testGetExistingUser() {
762763
$this->assertEquals('nobody', $user->getUID());
763764
$this->assertEquals('None', $user->getDisplayName());
764765
}
766+
767+
public function testGetFederatedUser(): void {
768+
$userId = 'test@example.com';
769+
770+
$cloudId = $this->createMock(ICloudId::class);
771+
$cloudId->expects($this->exactly(2))
772+
->method('getDisplayId')
773+
->willReturn($userId);
774+
775+
$manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
776+
$user = $manager->getFederatedUser($cloudId);
777+
778+
$this->assertEquals($userId, $user->getUID());
779+
$this->assertEquals($userId, $user->getDisplayName());
780+
}
765781
}

0 commit comments

Comments
 (0)