From b658c8519c319d4e1ffb637260771743ad0e8d13 Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Tue, 29 Apr 2025 16:42:09 +0200 Subject: [PATCH] fix(dav): check the owner displayName scope before giving attribute Signed-off-by: skjnldsv [skip ci] --- apps/dav/lib/Connector/Sabre/FilesPlugin.php | 21 ++- .../dav/lib/Connector/Sabre/ServerFactory.php | 2 + apps/dav/lib/Server.php | 2 + .../unit/Connector/Sabre/FilesPluginTest.php | 121 ++++++++++++++++++ .../Connector/Sabre/FilesReportPluginTest.php | 3 + .../src/views/FilesHeaderNoteToRecipient.vue | 6 +- 6 files changed, 151 insertions(+), 4 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index cb779a74f5d..4c2537a3541 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -10,6 +10,7 @@ namespace OCA\DAV\Connector\Sabre; use OC\AppFramework\Http\Request; use OC\FilesMetadata\Model\FilesMetadata; use OCA\DAV\Connector\Sabre\Exception\InvalidPath; +use OCP\Accounts\IAccountManager; use OCP\Constants; use OCP\Files\ForbiddenException; use OCP\Files\IFilenameValidator; @@ -88,6 +89,7 @@ class FilesPlugin extends ServerPlugin { private IPreview $previewManager, private IUserSession $userSession, private IFilenameValidator $validator, + private IAccountManager $accountManager, private bool $isPublic = false, private bool $downloadAttachment = true, ) { @@ -357,9 +359,26 @@ class FilesPlugin extends ServerPlugin { $owner = $node->getOwner(); if (!$owner) { return null; - } else { + } + + // Get current user to see if we're in a public share or not + $user = $this->userSession->getUser(); + + // If the user is logged in, we can return the display name + if ($user !== null) { return $owner->getDisplayName(); } + + // Check if the user published their display name + $ownerAccount = $this->accountManager->getAccount($owner); + $ownerNameProperty = $ownerAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME); + + // Since we are not logged in, we need to have at least the published scope + if ($ownerNameProperty->getScope() === IAccountManager::SCOPE_PUBLISHED) { + return $owner->getDisplayName(); + } + + return null; }); $propFind->handle(self::HAS_PREVIEW_PROPERTYNAME, function () use ($node) { diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index d1b508af354..14afbb46914 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -12,6 +12,7 @@ use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\CalDAV\DefaultCalendarValidator; use OCA\DAV\DAV\ViewOnlyPlugin; use OCA\DAV\Files\BrowserErrorPagePlugin; +use OCP\Accounts\IAccountManager; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Folder; use OCP\Files\IFilenameValidator; @@ -132,6 +133,7 @@ class ServerFactory { $this->previewManager, $this->userSession, \OCP\Server::get(IFilenameValidator::class), + \OCP\Server::get(IAccountManager::class), false, !$this->config->getSystemValue('debug', false) ) diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 6ead145b8aa..d57a4554fe3 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -51,6 +51,7 @@ use OCA\DAV\Provisioning\Apple\AppleProvisioningPlugin; use OCA\DAV\SystemTag\SystemTagPlugin; use OCA\DAV\Upload\ChunkingPlugin; use OCA\DAV\Upload\ChunkingV2Plugin; +use OCP\Accounts\IAccountManager; use OCP\AppFramework\Http\Response; use OCP\Diagnostics\IEventLogger; use OCP\EventDispatcher\IEventDispatcher; @@ -252,6 +253,7 @@ class Server { \OCP\Server::get(IPreview::class), \OCP\Server::get(IUserSession::class), \OCP\Server::get(IFilenameValidator::class), + \OCP\Server::get(IAccountManager::class), false, $config->getSystemValueBool('debug', false) === false, ) diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php index 8233932b86c..de38f196420 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php @@ -7,12 +7,15 @@ */ namespace OCA\DAV\Tests\unit\Connector\Sabre; +use OC\Accounts\Account; +use OC\Accounts\AccountProperty; use OC\User\User; use OCA\DAV\Connector\Sabre\Directory; use OCA\DAV\Connector\Sabre\Exception\InvalidPath; use OCA\DAV\Connector\Sabre\File; use OCA\DAV\Connector\Sabre\FilesPlugin; use OCA\DAV\Connector\Sabre\Node; +use OCP\Accounts\IAccountManager; use OCP\Files\FileInfo; use OCP\Files\IFilenameValidator; use OCP\Files\InvalidPathException; @@ -43,6 +46,7 @@ class FilesPluginTest extends TestCase { private IPreview&MockObject $previewManager; private IUserSession&MockObject $userSession; private IFilenameValidator&MockObject $filenameValidator; + private IAccountManager&MockObject $accountManager; private FilesPlugin $plugin; protected function setUp(): void { @@ -57,6 +61,7 @@ class FilesPluginTest extends TestCase { $this->previewManager = $this->createMock(IPreview::class); $this->userSession = $this->createMock(IUserSession::class); $this->filenameValidator = $this->createMock(IFilenameValidator::class); + $this->accountManager = $this->createMock(IAccountManager::class); $this->plugin = new FilesPlugin( $this->tree, @@ -65,6 +70,7 @@ class FilesPluginTest extends TestCase { $this->previewManager, $this->userSession, $this->filenameValidator, + $this->accountManager, ); $response = $this->getMockBuilder(ResponseInterface::class) @@ -154,6 +160,13 @@ class FilesPluginTest extends TestCase { ->method('getDisplayName') ->willReturn('M. Foo'); + $owner = $this->getMockBuilder(Account::class) + ->disableOriginalConstructor()->getMock(); + $this->accountManager->expects($this->once()) + ->method('getAccount') + ->with($user) + ->willReturn($owner); + $node->expects($this->once()) ->method('getDirectDownload') ->willReturn(['url' => 'http://example.com/']); @@ -161,6 +174,18 @@ class FilesPluginTest extends TestCase { ->method('getOwner') ->willReturn($user); + $displayNameProp = $this->getMockBuilder(AccountProperty::class) + ->disableOriginalConstructor()->getMock(); + $owner + ->expects($this->once()) + ->method('getProperty') + ->with(IAccountManager::PROPERTY_DISPLAYNAME) + ->willReturn($displayNameProp); + $displayNameProp + ->expects($this->once()) + ->method('getScope') + ->willReturn(IAccountManager::SCOPE_PUBLISHED); + $this->plugin->handleGetProperties( $propFind, $node @@ -179,6 +204,101 @@ class FilesPluginTest extends TestCase { $this->assertEquals([], $propFind->get404Properties()); } + public function testGetDisplayNamePropertyWhenNotPublished(): void { + /** @var File|\PHPUnit\Framework\MockObject\MockObject $node */ + $node = $this->createTestNode('\OCA\DAV\Connector\Sabre\File'); + + $propFind = new PropFind( + '/dummyPath', + [ + FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME, + ], + 0 + ); + + $this->userSession->expects($this->once()) + ->method('getUser') + ->willReturn(null); + + $user = $this->getMockBuilder(User::class) + ->disableOriginalConstructor()->getMock(); + + $user + ->expects($this->never()) + ->method('getDisplayName'); + + $owner = $this->getMockBuilder(Account::class) + ->disableOriginalConstructor()->getMock(); + $this->accountManager->expects($this->once()) + ->method('getAccount') + ->with($user) + ->willReturn($owner); + + $node->expects($this->once()) + ->method('getOwner') + ->willReturn($user); + + $displayNameProp = $this->getMockBuilder(AccountProperty::class) + ->disableOriginalConstructor()->getMock(); + $owner + ->expects($this->once()) + ->method('getProperty') + ->with(IAccountManager::PROPERTY_DISPLAYNAME) + ->willReturn($displayNameProp); + $displayNameProp + ->expects($this->once()) + ->method('getScope') + ->willReturn(IAccountManager::SCOPE_PRIVATE); + + $this->plugin->handleGetProperties( + $propFind, + $node + ); + + $this->assertEquals(null, $propFind->get(FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME)); + } + + public function testGetDisplayNamePropertyWhenNotPublishedButLoggedIn(): void { + /** @var File|\PHPUnit\Framework\MockObject\MockObject $node */ + $node = $this->createTestNode('\OCA\DAV\Connector\Sabre\File'); + + $propFind = new PropFind( + '/dummyPath', + [ + FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME, + ], + 0 + ); + + $user = $this->getMockBuilder(User::class) + ->disableOriginalConstructor()->getMock(); + + $node->expects($this->once()) + ->method('getOwner') + ->willReturn($user); + + $loggedInUser = $this->getMockBuilder(User::class) + ->disableOriginalConstructor()->getMock(); + $this->userSession->expects($this->once()) + ->method('getUser') + ->willReturn($loggedInUser); + + $user + ->expects($this->once()) + ->method('getDisplayName') + ->willReturn('M. Foo'); + + $this->accountManager->expects($this->never()) + ->method('getAccount'); + + $this->plugin->handleGetProperties( + $propFind, + $node + ); + + $this->assertEquals('M. Foo', $propFind->get(FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME)); + } + public function testGetPropertiesStorageNotAvailable(): void { /** @var \OCA\DAV\Connector\Sabre\File | \PHPUnit\Framework\MockObject\MockObject $node */ $node = $this->createTestNode('\OCA\DAV\Connector\Sabre\File'); @@ -215,6 +335,7 @@ class FilesPluginTest extends TestCase { $this->previewManager, $this->userSession, $this->filenameValidator, + $this->accountManager, true, ); $this->plugin->initialize($this->server); diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php index 76a70a93e13..26f14292b35 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php @@ -10,6 +10,7 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre; use OC\Files\View; use OCA\DAV\Connector\Sabre\Directory; use OCA\DAV\Connector\Sabre\FilesReportPlugin as FilesReportPluginImplementation; +use OCP\Accounts\IAccountManager; use OCP\App\IAppManager; use OCP\Files\File; use OCP\Files\FileInfo; @@ -388,6 +389,7 @@ class FilesReportPluginTest extends \Test\TestCase { ->getMock(); $validator = $this->createMock(IFilenameValidator::class); + $accountManager = $this->createMock(IAccountManager::class); $this->server->addPlugin( new \OCA\DAV\Connector\Sabre\FilesPlugin( @@ -397,6 +399,7 @@ class FilesReportPluginTest extends \Test\TestCase { $this->previewManager, $this->createMock(IUserSession::class), $validator, + $accountManager, ) ); $this->plugin->initialize($this->server); diff --git a/apps/files_sharing/src/views/FilesHeaderNoteToRecipient.vue b/apps/files_sharing/src/views/FilesHeaderNoteToRecipient.vue index a8a39c41e5d..192d6103078 100644 --- a/apps/files_sharing/src/views/FilesHeaderNoteToRecipient.vue +++ b/apps/files_sharing/src/views/FilesHeaderNoteToRecipient.vue @@ -6,7 +6,7 @@ -

+

{{ t('files_sharing', 'Note from') }}

@@ -28,13 +28,13 @@ import NcUserBubble from '@nextcloud/vue/dist/Components/NcUserBubble.js' const folder = ref() const note = computed(() => folder.value?.attributes.note ?? '') +const displayName = computed(() => folder.value?.attributes['owner-display-name'] ?? '') const user = computed(() => { const id = folder.value?.owner - const displayName = folder.value?.attributes?.['owner-display-name'] if (id !== getCurrentUser()?.uid) { return { id, - displayName, + displayName: displayName.value, } } return null