From 5bbcc9cdba980dd58d80b7124055fa1c5e3d40a7 Mon Sep 17 00:00:00 2001 From: Kent Delante Date: Mon, 20 Jul 2026 21:45:43 +0800 Subject: [PATCH 1/6] feat(files_sharing): fetch remote avatars for external shares Signed-off-by: Kent Delante Assisted-by: ClaudeCode:claude-sonnet-5 --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Avatar/AvatarManager.php | 16 +++ lib/private/Avatar/RemoteAvatar.php | 129 ++++++++++++++++++++ 4 files changed, 147 insertions(+) create mode 100644 lib/private/Avatar/RemoteAvatar.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 8b8de8c3ca5db..df174d50c9577 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1309,6 +1309,7 @@ 'OC\\Avatar\\AvatarManager' => $baseDir . '/lib/private/Avatar/AvatarManager.php', 'OC\\Avatar\\GuestAvatar' => $baseDir . '/lib/private/Avatar/GuestAvatar.php', 'OC\\Avatar\\PlaceholderAvatar' => $baseDir . '/lib/private/Avatar/PlaceholderAvatar.php', + 'OC\\Avatar\\RemoteAvatar' => $baseDir . '/lib/private/Avatar/RemoteAvatar.php', 'OC\\Avatar\\UserAvatar' => $baseDir . '/lib/private/Avatar/UserAvatar.php', 'OC\\BackgroundJob\\JobClassesRegistry' => $baseDir . '/lib/private/BackgroundJob/JobClassesRegistry.php', 'OC\\BackgroundJob\\JobList' => $baseDir . '/lib/private/BackgroundJob/JobList.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 33a003ab19ace..c87a60f9d1194 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1350,6 +1350,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Avatar\\AvatarManager' => __DIR__ . '/../../..' . '/lib/private/Avatar/AvatarManager.php', 'OC\\Avatar\\GuestAvatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/GuestAvatar.php', 'OC\\Avatar\\PlaceholderAvatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/PlaceholderAvatar.php', + 'OC\\Avatar\\RemoteAvatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/RemoteAvatar.php', 'OC\\Avatar\\UserAvatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/UserAvatar.php', 'OC\\BackgroundJob\\JobClassesRegistry' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/JobClassesRegistry.php', 'OC\\BackgroundJob\\JobList' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/JobList.php', diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index b99038d1098bd..4724ab05a1505 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -13,6 +13,7 @@ use OC\User\Manager; use OCP\Accounts\IAccountManager; use OCP\Accounts\PropertyDoesNotExistException; +use OCP\Federation\ICloudIdManager; use OCP\Files\IAppData; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; @@ -55,6 +56,11 @@ public function __construct( public function getAvatar(string $userId): IAvatar { $user = $this->userManager->get($userId); if ($user === null) { + $cloudIdManager = \OCP\Server::get(ICloudIdManager::class); + if ($cloudIdManager->isValidCloudId($userId)) { + return $this->getRemoteAvatar($userId); + } + throw new \Exception('user does not exist'); } @@ -134,4 +140,14 @@ public function deleteUserAvatar(string $userId): void { public function getGuestAvatar(string $name): IAvatar { return new GuestAvatar($name, $this->config, $this->logger); } + + /** + * Returns a RemoteAvatar + * + * @param string $userId The \OCP\Federation\ICloudId of the remote account, e.g. account@example.com + */ + private function getRemoteAvatar(string $userId): IAvatar { + $folder = $this->appData->newFolder($userId); + return new RemoteAvatar($folder, $userId, $this->config, $this->logger); + } } diff --git a/lib/private/Avatar/RemoteAvatar.php b/lib/private/Avatar/RemoteAvatar.php new file mode 100644 index 0000000000000..681edb60d07d7 --- /dev/null +++ b/lib/private/Avatar/RemoteAvatar.php @@ -0,0 +1,129 @@ +cloudId = $cloudIdManager->resolveCloudId($userId); + } + + #[\Override] + public function exists(): bool { + return true; + } + + #[\Override] + public function getDisplayName(): string { + return $this->cloudId->getDisplayId(); + } + + /** + * Setting avatars isn't implemented for remote accounts + */ + #[\Override] + public function set($data): void { + } + + /** + * Removing avatars isn't implemented for remote accounts + */ + #[\Override] + public function remove(bool $silent = false): void { + } + + #[\Override] + public function getFile(int $size, bool $darkTheme = false): ISimpleFile { + $avatarExists = false; + if ($size === -1) { + $filename = 'avatar' . ($darkTheme ? '-dark' : ''); + } else { + $filename = 'avatar' . ($darkTheme ? '-dark' : '') . '.' . $size; + } + + // check first if a png avatar exists + $ext = 'png'; + $avatarExists = $this->folder->fileExists($filename . '.' . $ext); + // if the png avatar doesn't exist, check if there's a jpg + if (!$avatarExists) { + $ext = 'jpg'; + $avatarExists = $this->folder->fileExists($filename . '.' . $ext); + } + + if ($avatarExists) { + $file = $this->folder->getFile($filename . '.' . $ext); + + // check if a remote avatar is at least a day old, in case a new avatar was uploaded + $isAvatarOld = (time() - $file->getMTime()) >= self::IMAGE_CACHE_AGE; + if (!$isAvatarOld) { + return $file; + } + } + + $url = rtrim($this->cloudId->getRemote(), '/') . '/index.php/avatar/' . rawurlencode($this->cloudId->getUser()) . '/' . $size; + if ($darkTheme) { + $url .= '/dark'; + } + + $clientService = \OCP\Server::get(IClientService::class); + $client = $clientService->newClient(); + $response = $client->get($url, [ + 'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates', false) + ]); + + $contentType = $response->getHeader('Content-Type'); + if (!in_array($contentType, self::ALLOWED_CONTENT_TYPES)) { + throw new \Exception('Unknown filetype'); + } + + $avatar = $response->getBody(); + if (is_resource($avatar)) { + $avatar = stream_get_contents($avatar); + if ($avatar === false) { + throw new \Exception('Failed to fetch remote avatar'); + } + } + + $ext = $contentType === 'image/jpg' || $contentType === 'image/jpeg' ? 'jpg' : 'png'; + return $this->folder->newFile($filename . '.' . $ext, $avatar); + } + + /** + * Handling user changes isn't implemented for remote accounts + */ + #[\Override] + public function userChanged(string $feature, $oldValue, $newValue): void { + } + + #[\Override] + public function isCustomAvatar(): bool { + return true; + } +} From 7551692359a0cf151cc91108c830f485fac3f780 Mon Sep 17 00:00:00 2001 From: Kent Delante Date: Tue, 21 Jul 2026 13:31:02 +0800 Subject: [PATCH 2/6] test(files_sharing): add tests for fetching remote avatars Signed-off-by: Kent Delante Assisted-by: ClaudeCode:claude-sonnet-5 --- tests/lib/Avatar/AvatarManagerTest.php | 85 +++++++++++--- tests/lib/Avatar/RemoteAvatarTest.php | 150 +++++++++++++++++++++++++ 2 files changed, 222 insertions(+), 13 deletions(-) create mode 100644 tests/lib/Avatar/RemoteAvatarTest.php diff --git a/tests/lib/Avatar/AvatarManagerTest.php b/tests/lib/Avatar/AvatarManagerTest.php index afbc63ac0e3f3..7e55723955c8d 100644 --- a/tests/lib/Avatar/AvatarManagerTest.php +++ b/tests/lib/Avatar/AvatarManagerTest.php @@ -10,6 +10,7 @@ use OC\Avatar\AvatarManager; use OC\Avatar\PlaceholderAvatar; +use OC\Avatar\RemoteAvatar; use OC\Avatar\UserAvatar; use OC\KnownUser\KnownUserService; use OC\User\Manager; @@ -17,6 +18,8 @@ use OCP\Accounts\IAccount; use OCP\Accounts\IAccountManager; use OCP\Accounts\IAccountProperty; +use OCP\Federation\ICloudId; +use OCP\Federation\ICloudIdManager; use OCP\Files\IAppData; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; @@ -73,19 +76,6 @@ protected function setUp(): void { ); } - public function testGetAvatarInvalidUser(): void { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('user does not exist'); - - $this->userManager - ->expects($this->once()) - ->method('get') - ->with('invalidUser') - ->willReturn(null); - - $this->avatarManager->getAvatar('invalidUser'); - } - public function testGetAvatarForSelf(): void { $user = $this->createMock(User::class); $user @@ -276,4 +266,73 @@ public function testGetAvatarScopes($avatarScope, $isPublicCall, $isKnownUser, $ } $this->assertEquals($expected, $this->avatarManager->getAvatar('valid-user')); } + + public function testGetAvatarInvalidUser(): void { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('user does not exist'); + + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('invalidUser') + ->willReturn(null); + + $this->avatarManager->getAvatar('invalidUser'); + } + + public function testGetAvatarForRemoteUser(): void { + $cloudId = 'user@https://remote.example.com'; + + $this->userManager + ->expects($this->once()) + ->method('get') + ->with($cloudId) + ->willReturn(null); + + $resolvedCloudId = $this->createMock(ICloudId::class); + $resolvedCloudId->method('getUser')->willReturn('user'); + $resolvedCloudId->method('getRemote')->willReturn('https://remote.example.com'); + $resolvedCloudId->method('getDisplayId')->willReturn('user@remote.example.com'); + + $cloudIdManager = $this->createMock(ICloudIdManager::class); + $cloudIdManager->expects($this->once()) + ->method('isValidCloudId') + ->with($cloudId) + ->willReturn(true); + $cloudIdManager->method('resolveCloudId') + ->with($cloudId) + ->willReturn($resolvedCloudId); + $this->overwriteService(ICloudIdManager::class, $cloudIdManager); + + // the remote branch must not touch local avatar storage + $this->appData->expects($this->never())->method('getFolder'); + $this->accountManager->expects($this->never())->method('getAccount'); + + $avatar = $this->avatarManager->getAvatar($cloudId); + + self::assertInstanceOf(RemoteAvatar::class, $avatar); + self::assertTrue($avatar->exists()); + self::assertTrue($avatar->isCustomAvatar()); + self::assertSame('user@remote.example.com', $avatar->getDisplayName()); + } + + public function testGetAvatarThrowsForUnknownUserThatIsNotACloudId(): void { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('user does not exist'); + + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('invalidUser') + ->willReturn(null); + + $cloudIdManager = $this->createMock(ICloudIdManager::class); + $cloudIdManager->expects($this->once()) + ->method('isValidCloudId') + ->with('invalidUser') + ->willReturn(false); + $this->overwriteService(ICloudIdManager::class, $cloudIdManager); + + $this->avatarManager->getAvatar('invalidUser'); + } } diff --git a/tests/lib/Avatar/RemoteAvatarTest.php b/tests/lib/Avatar/RemoteAvatarTest.php new file mode 100644 index 0000000000000..079a1ceb27dfe --- /dev/null +++ b/tests/lib/Avatar/RemoteAvatarTest.php @@ -0,0 +1,150 @@ +config = $this->createMock(IConfig::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $cloudId = $this->createMock(ICloudId::class); + $cloudId->method('getUser')->willReturn('user'); + $cloudId->method('getRemote')->willReturn('https://remote.example.com'); + $cloudId->method('getDisplayId')->willReturn('user@remote.example.com'); + + $this->cloudIdManager = $this->createMock(ICloudIdManager::class); + $this->cloudIdManager->method('resolveCloudId') + ->with(self::CLOUD_ID) + ->willReturn($cloudId); + $this->overwriteService(ICloudIdManager::class, $this->cloudIdManager); + + $this->clientService = $this->createMock(IClientService::class); + $this->overwriteService(IClientService::class, $this->clientService); + + $this->folder = $this->createMock(ISimpleFolder::class); + $this->avatar = new RemoteAvatar($this->folder, self::CLOUD_ID, $this->config, $this->logger); + } + + /** + * Stubs the client returned by IClientService::newClient() to respond to + * a single GET request, optionally asserting the requested URL/options. + * + * @param string|resource|false $body + */ + private function mockRemoteClient(string $contentType, string $body, ?string $expectedUrl = null, ?array $expectedOptions = null): void { + $response = $this->createMock(IResponse::class); + $response->method('getHeader')->with('Content-Type')->willReturn($contentType); + $response->method('getBody')->willReturn($body); + + $client = $this->createMock(IClient::class); + $matcher = $client->expects($this->once())->method('get'); + if ($expectedUrl !== null) { + $matcher->with($expectedUrl, $expectedOptions ?? $this->anything()); + } + $matcher->willReturn($response); + + $this->clientService->method('newClient')->willReturn($client); + } + + public function testExists(): void { + $this->assertTrue($this->avatar->exists()); + } + + public function testGetDisplayName(): void { + $this->assertSame('user@remote.example.com', $this->avatar->getDisplayName()); + } + + public function testGetFileFetchesAvatarFromRemoteInstance(): void { + $this->config->method('getSystemValueBool') + ->with('sharing.federation.allowSelfSignedCertificates', false) + ->willReturn(false); + + $fileContents = 'png-bytes'; + + $this->mockRemoteClient( + 'image/png', + $fileContents, + 'https://remote.example.com/index.php/avatar/user/64', + ['verify' => true], + ); + + $this->folder->expects($this->exactly(2)) + ->method('fileExists') + ->willReturn(false); + + $expectedFile = $this->createMock(ISimpleFile::class); + $expectedFile->expects($this->once())->method('getContent')->willReturn($fileContents); + + $this->folder->expects($this->once())->method('newFile')->willReturn($expectedFile); + + $file = $this->avatar->getFile(64); + $this->assertInstanceOf(ISimpleFile::class, $file); + $this->assertSame($fileContents, $file->getContent()); + } + + public function testGetFileFetchesAvatarFromCache(): void { + $this->config->method('getSystemValueBool') + ->with('sharing.federation.allowSelfSignedCertificates', false) + ->willReturn(false); + + $fileContents = 'png-bytes'; + + $this->folder->expects($this->once())->method('fileExists')->willReturn(true); + + $expectedFile = $this->createMock(ISimpleFile::class); + $expectedFile->expects($this->once())->method('getContent')->willReturn($fileContents); + $expectedFile->expects($this->once())->method('getMTime')->willReturn(time() + (60 * 60 * 24)); + $this->folder->expects($this->once())->method('getFile')->willReturn($expectedFile); + + $this->clientService->expects($this->never())->method('newClient'); + + $file = $this->avatar->getFile(64); + $this->assertInstanceOf(ISimpleFile::class, $file); + $this->assertSame($fileContents, $file->getContent()); + } + + public function testGetFileThrowsOnUnexpectedContentType(): void { + $this->mockRemoteClient('text/html', ''); + + $this->expectException(\Exception::class); + $this->expectExceptionMessage('Unknown filetype'); + + $this->avatar->getFile(64); + } + + public function testIsCustomAvatar(): void { + $this->assertTrue($this->avatar->isCustomAvatar()); + } +} From 32a4bfb0018696a6d87a0c7feb4cfb35db874270 Mon Sep 17 00:00:00 2001 From: Kent Delante Date: Fri, 24 Jul 2026 15:34:17 +0800 Subject: [PATCH 3/6] fixup! feat(files_sharing): fetch remote avatars for external shares --- lib/private/Avatar/RemoteAvatar.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/private/Avatar/RemoteAvatar.php b/lib/private/Avatar/RemoteAvatar.php index 681edb60d07d7..957ffe9688a95 100644 --- a/lib/private/Avatar/RemoteAvatar.php +++ b/lib/private/Avatar/RemoteAvatar.php @@ -18,7 +18,6 @@ use Psr\Log\LoggerInterface; class RemoteAvatar extends Avatar { - private const ALLOWED_CONTENT_TYPES = [ 'image/png', 'image/jpg', 'image/jpeg' ]; private const IMAGE_CACHE_AGE = 60 * 60 * 24; // One day private ICloudId $cloudId; @@ -99,7 +98,7 @@ public function getFile(int $size, bool $darkTheme = false): ISimpleFile { ]); $contentType = $response->getHeader('Content-Type'); - if (!in_array($contentType, self::ALLOWED_CONTENT_TYPES)) { + if (strpos($contentType, 'image/') === false) { throw new \Exception('Unknown filetype'); } From 7b9f2f1070db7a418a9d01198247eb4fe900ae01 Mon Sep 17 00:00:00 2001 From: Kent Delante Date: Tue, 28 Jul 2026 21:00:08 +0800 Subject: [PATCH 4/6] fixup! fixup! feat(files_sharing): fetch remote avatars for external shares --- lib/private/Avatar/RemoteAvatar.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/private/Avatar/RemoteAvatar.php b/lib/private/Avatar/RemoteAvatar.php index 957ffe9688a95..ce3f32dd28770 100644 --- a/lib/private/Avatar/RemoteAvatar.php +++ b/lib/private/Avatar/RemoteAvatar.php @@ -11,6 +11,7 @@ use OCP\Federation\ICloudId; use OCP\Federation\ICloudIdManager; +use OCP\Files\NotFoundException; use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\Http\Client\IClientService; @@ -69,11 +70,22 @@ public function getFile(int $size, bool $darkTheme = false): ISimpleFile { // check first if a png avatar exists $ext = 'png'; - $avatarExists = $this->folder->fileExists($filename . '.' . $ext); + try { + $file = $this->folder->getFile($filename . '.' . $ext); + $avatarExists = true; + } catch (NotFoundException $e) { + $this->logger->debug('Unable to find avatar with .png extension. Trying .jpg'); + } + // if the png avatar doesn't exist, check if there's a jpg if (!$avatarExists) { $ext = 'jpg'; - $avatarExists = $this->folder->fileExists($filename . '.' . $ext); + try { + $file = $this->folder->getFile($filename . '.' . $ext); + $avatarExists = true; + } catch (NotFoundException $e) { + $this->logger->debug('No avatar found'); + } } if ($avatarExists) { From eb74733920d51b5949933fcc2a3a3677e9db3a08 Mon Sep 17 00:00:00 2001 From: Kent Delante Date: Tue, 28 Jul 2026 22:01:17 +0800 Subject: [PATCH 5/6] fixup! fixup! fixup! feat(files_sharing): fetch remote avatars for external shares --- lib/private/Avatar/RemoteAvatar.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/private/Avatar/RemoteAvatar.php b/lib/private/Avatar/RemoteAvatar.php index ce3f32dd28770..9e70961921f81 100644 --- a/lib/private/Avatar/RemoteAvatar.php +++ b/lib/private/Avatar/RemoteAvatar.php @@ -89,8 +89,6 @@ public function getFile(int $size, bool $darkTheme = false): ISimpleFile { } if ($avatarExists) { - $file = $this->folder->getFile($filename . '.' . $ext); - // check if a remote avatar is at least a day old, in case a new avatar was uploaded $isAvatarOld = (time() - $file->getMTime()) >= self::IMAGE_CACHE_AGE; if (!$isAvatarOld) { From 56fde46e3e37c402621dd8a5a1a7f04164f77948 Mon Sep 17 00:00:00 2001 From: Kent Delante Date: Tue, 28 Jul 2026 22:01:27 +0800 Subject: [PATCH 6/6] fixup! test(files_sharing): add tests for fetching remote avatars --- tests/lib/Avatar/RemoteAvatarTest.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/lib/Avatar/RemoteAvatarTest.php b/tests/lib/Avatar/RemoteAvatarTest.php index 079a1ceb27dfe..2a88214c3d0a1 100644 --- a/tests/lib/Avatar/RemoteAvatarTest.php +++ b/tests/lib/Avatar/RemoteAvatarTest.php @@ -100,10 +100,6 @@ public function testGetFileFetchesAvatarFromRemoteInstance(): void { ['verify' => true], ); - $this->folder->expects($this->exactly(2)) - ->method('fileExists') - ->willReturn(false); - $expectedFile = $this->createMock(ISimpleFile::class); $expectedFile->expects($this->once())->method('getContent')->willReturn($fileContents); @@ -121,12 +117,10 @@ public function testGetFileFetchesAvatarFromCache(): void { $fileContents = 'png-bytes'; - $this->folder->expects($this->once())->method('fileExists')->willReturn(true); - $expectedFile = $this->createMock(ISimpleFile::class); $expectedFile->expects($this->once())->method('getContent')->willReturn($fileContents); $expectedFile->expects($this->once())->method('getMTime')->willReturn(time() + (60 * 60 * 24)); - $this->folder->expects($this->once())->method('getFile')->willReturn($expectedFile); + $this->folder->expects($this->once())->method('getFile')->with('avatar.64.png')->willReturn($expectedFile); $this->clientService->expects($this->never())->method('newClient');