Merge pull request #52535 from nextcloud/fix/public-owner-scope

Author: skjnldsv

apps/dav/lib/Connector/Sabre/FilesPlugin.php

@@ -11,6 +11,7 @@
 use OC\FilesMetadata\Model\FilesMetadata;
 use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
 use OCA\Files_Sharing\External\Mount as SharingExternalMount;
+use OCP\Accounts\IAccountManager;
 use OCP\Constants;
 use OCP\Files\ForbiddenException;
 use OCP\Files\IFilenameValidator;
@@ -91,6 +92,7 @@ public function __construct(
 		private IPreview $previewManager,
 		private IUserSession $userSession,
 		private IFilenameValidator $validator,
+		private IAccountManager $accountManager,
 		private bool $isPublic = false,
 		private bool $downloadAttachment = true,
 	) {
@@ -361,9 +363,26 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)
 				$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) {

apps/dav/lib/Connector/Sabre/ServerFactory.php

@@ -14,6 +14,7 @@
 use OCA\DAV\DAV\ViewOnlyPlugin;
 use OCA\DAV\Files\BrowserErrorPagePlugin;
 use OCA\Theming\ThemingDefaults;
+use OCP\Accounts\IAccountManager;
 use OCP\App\IAppManager;
 use OCP\Comments\ICommentsManager;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -128,6 +129,7 @@ public function createServer(string $baseUri,
 					$this->previewManager,
 					$this->userSession,
 					\OCP\Server::get(IFilenameValidator::class),
+					\OCP\Server::get(IAccountManager::class),
 					false,
 					!$this->config->getSystemValue('debug', false)
 				)

apps/dav/lib/Server.php

@@ -64,6 +64,7 @@
 use OCA\DAV\Upload\ChunkingPlugin;
 use OCA\DAV\Upload\ChunkingV2Plugin;
 use OCA\Theming\ThemingDefaults;
+use OCP\Accounts\IAccountManager;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -287,6 +288,7 @@ public function __construct(
 						\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,
 					)

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 @@ protected function setUp(): void {
 		$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 @@ protected function setUp(): void {
 			$this->previewManager,
 			$this->userSession,
 			$this->filenameValidator,
+			$this->accountManager,
 		);
 
 		$response = $this->getMockBuilder(ResponseInterface::class)
@@ -154,13 +160,32 @@ public function testGetPropertiesForFile(): void {
 			->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/']);
 		$node->expects($this->exactly(2))
 			->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 @@ public function testGetPropertiesForFile(): void {
 		$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 File|\PHPUnit\Framework\MockObject\MockObject $node */
 		$node = $this->createTestNode('\OCA\DAV\Connector\Sabre\File');
@@ -215,6 +335,7 @@ public function testGetPublicPermissions(): void {
 			$this->previewManager,
 			$this->userSession,
 			$this->filenameValidator,
+			$this->accountManager,
 			true,
 		);
 		$this->plugin->initialize($this->server);

apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php

@@ -11,6 +11,7 @@
 use OCA\DAV\Connector\Sabre\Directory;
 use OCA\DAV\Connector\Sabre\FilesPlugin;
 use OCA\DAV\Connector\Sabre\FilesReportPlugin as FilesReportPluginImplementation;
+use OCP\Accounts\IAccountManager;
 use OCP\App\IAppManager;
 use OCP\Files\File;
 use OCP\Files\FileInfo;
@@ -389,6 +390,7 @@ public function testPrepareResponses(): void {
 			->getMock();
 
 		$validator = $this->createMock(IFilenameValidator::class);
+		$accountManager = $this->createMock(IAccountManager::class);
 
 		$this->server->addPlugin(
 			new FilesPlugin(
@@ -398,6 +400,7 @@ public function testPrepareResponses(): void {
 				$this->previewManager,
 				$this->createMock(IUserSession::class),
 				$validator,
+				$accountManager,
 			)
 		);
 		$this->plugin->initialize($this->server);

apps/files_sharing/src/views/FilesHeaderNoteToRecipient.vue

@@ -6,7 +6,7 @@
 	<NcNoteCard v-if="note.length > 0"
 		class="note-to-recipient"
 		type="info">
-		<p v-if="user" class="note-to-recipient__heading">
+		<p v-if="displayName" class="note-to-recipient__heading">
 			{{ t('files_sharing', 'Note from') }}
 			<NcUserBubble :user="user.id" :display-name="user.displayName" />
 		</p>
@@ -28,13 +28,13 @@ import NcUserBubble from '@nextcloud/vue/components/NcUserBubble'
 
 const folder = ref<Folder>()
 const note = computed<string>(() => folder.value?.attributes.note ?? '')
+const displayName = computed<string>(() => 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