Skip to content

Commit 6e45ff8

Browse files
skjnldsvbackportbot[bot]
authored andcommitted
fix(dav): check the owner displayName scope before giving attribute
Signed-off-by: skjnldsv <skjnldsv@protonmail.com> [skip ci]
1 parent 02bd400 commit 6e45ff8

File tree

6 files changed

+148
-4
lines changed

6 files changed

+148
-4
lines changed

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ public function __construct(
9090
private IPreview $previewManager,
9191
private IUserSession $userSession,
9292
private IFilenameValidator $validator,
93+
private IAccountManager $accountManager,
9394
private bool $isPublic = false,
9495
private bool $downloadAttachment = true,
9596
) {
@@ -360,9 +361,26 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)
360361
$owner = $node->getOwner();
361362
if (!$owner) {
362363
return null;
363-
} else {
364+
}
365+
366+
// Get current user to see if we're in a public share or not
367+
$user = $this->userSession->getUser();
368+
369+
// If the user is logged in, we can return the display name
370+
if ($user !== null) {
364371
return $owner->getDisplayName();
365372
}
373+
374+
// Check if the user published their display name
375+
$ownerAccount = $this->accountManager->getAccount($owner);
376+
$ownerNameProperty = $ownerAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME);
377+
378+
// Since we are not logged in, we need to have at least the published scope
379+
if ($ownerNameProperty->getScope() === IAccountManager::SCOPE_PUBLISHED) {
380+
return $owner->getDisplayName();
381+
}
382+
383+
return null;
366384
});
367385

368386
$propFind->handle(self::HAS_PREVIEW_PROPERTYNAME, function () use ($node) {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ public function createServer(string $baseUri,
123123
$this->previewManager,
124124
$this->userSession,
125125
\OCP\Server::get(IFilenameValidator::class),
126+
\OCP\Server::get(IAccountManager::class),
126127
false,
127128
!$this->config->getSystemValue('debug', false)
128129
)

apps/dav/lib/Server.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ public function __construct(
275275
\OCP\Server::get(IPreview::class),
276276
\OCP\Server::get(IUserSession::class),
277277
\OCP\Server::get(IFilenameValidator::class),
278+
\OCP\Server::get(IAccountManager::class),
278279
false,
279280
$config->getSystemValueBool('debug', false) === false,
280281
)

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

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,15 @@
77
*/
88
namespace OCA\DAV\Tests\unit\Connector\Sabre;
99

10+
use OC\Accounts\Account;
11+
use OC\Accounts\AccountProperty;
1012
use OC\User\User;
1113
use OCA\DAV\Connector\Sabre\Directory;
1214
use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
1315
use OCA\DAV\Connector\Sabre\File;
1416
use OCA\DAV\Connector\Sabre\FilesPlugin;
1517
use OCA\DAV\Connector\Sabre\Node;
18+
use OCP\Accounts\IAccountManager;
1619
use OCP\Files\FileInfo;
1720
use OCP\Files\IFilenameValidator;
1821
use OCP\Files\InvalidPathException;
@@ -43,6 +46,7 @@ class FilesPluginTest extends TestCase {
4346
private IPreview&MockObject $previewManager;
4447
private IUserSession&MockObject $userSession;
4548
private IFilenameValidator&MockObject $filenameValidator;
49+
private IAccountManager&MockObject $accountManager;
4650
private FilesPlugin $plugin;
4751

4852
protected function setUp(): void {
@@ -57,6 +61,7 @@ protected function setUp(): void {
5761
$this->previewManager = $this->createMock(IPreview::class);
5862
$this->userSession = $this->createMock(IUserSession::class);
5963
$this->filenameValidator = $this->createMock(IFilenameValidator::class);
64+
$this->accountManager = $this->createMock(IAccountManager::class);
6065

6166
$this->plugin = new FilesPlugin(
6267
$this->tree,
@@ -65,6 +70,7 @@ protected function setUp(): void {
6570
$this->previewManager,
6671
$this->userSession,
6772
$this->filenameValidator,
73+
$this->accountManager,
6874
);
6975

7076
$response = $this->getMockBuilder(ResponseInterface::class)
@@ -154,13 +160,32 @@ public function testGetPropertiesForFile(): void {
154160
->method('getDisplayName')
155161
->willReturn('M. Foo');
156162

163+
$owner = $this->getMockBuilder(Account::class)
164+
->disableOriginalConstructor()->getMock();
165+
$this->accountManager->expects($this->once())
166+
->method('getAccount')
167+
->with($user)
168+
->willReturn($owner);
169+
157170
$node->expects($this->once())
158171
->method('getDirectDownload')
159172
->willReturn(['url' => 'http://example.com/']);
160173
$node->expects($this->exactly(2))
161174
->method('getOwner')
162175
->willReturn($user);
163176

177+
$displayNameProp = $this->getMockBuilder(AccountProperty::class)
178+
->disableOriginalConstructor()->getMock();
179+
$owner
180+
->expects($this->once())
181+
->method('getProperty')
182+
->with(IAccountManager::PROPERTY_DISPLAYNAME)
183+
->willReturn($displayNameProp);
184+
$displayNameProp
185+
->expects($this->once())
186+
->method('getScope')
187+
->willReturn(IAccountManager::SCOPE_PUBLISHED);
188+
164189
$this->plugin->handleGetProperties(
165190
$propFind,
166191
$node
@@ -179,6 +204,101 @@ public function testGetPropertiesForFile(): void {
179204
$this->assertEquals([], $propFind->get404Properties());
180205
}
181206

207+
public function testGetDisplayNamePropertyWhenNotPublished(): void {
208+
/** @var File|\PHPUnit\Framework\MockObject\MockObject $node */
209+
$node = $this->createTestNode('\OCA\DAV\Connector\Sabre\File');
210+
211+
$propFind = new PropFind(
212+
'/dummyPath',
213+
[
214+
FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME,
215+
],
216+
0
217+
);
218+
219+
$this->userSession->expects($this->once())
220+
->method('getUser')
221+
->willReturn(null);
222+
223+
$user = $this->getMockBuilder(User::class)
224+
->disableOriginalConstructor()->getMock();
225+
226+
$user
227+
->expects($this->never())
228+
->method('getDisplayName');
229+
230+
$owner = $this->getMockBuilder(Account::class)
231+
->disableOriginalConstructor()->getMock();
232+
$this->accountManager->expects($this->once())
233+
->method('getAccount')
234+
->with($user)
235+
->willReturn($owner);
236+
237+
$node->expects($this->once())
238+
->method('getOwner')
239+
->willReturn($user);
240+
241+
$displayNameProp = $this->getMockBuilder(AccountProperty::class)
242+
->disableOriginalConstructor()->getMock();
243+
$owner
244+
->expects($this->once())
245+
->method('getProperty')
246+
->with(IAccountManager::PROPERTY_DISPLAYNAME)
247+
->willReturn($displayNameProp);
248+
$displayNameProp
249+
->expects($this->once())
250+
->method('getScope')
251+
->willReturn(IAccountManager::SCOPE_PRIVATE);
252+
253+
$this->plugin->handleGetProperties(
254+
$propFind,
255+
$node
256+
);
257+
258+
$this->assertEquals(null, $propFind->get(FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME));
259+
}
260+
261+
public function testGetDisplayNamePropertyWhenNotPublishedButLoggedIn(): void {
262+
/** @var File|\PHPUnit\Framework\MockObject\MockObject $node */
263+
$node = $this->createTestNode('\OCA\DAV\Connector\Sabre\File');
264+
265+
$propFind = new PropFind(
266+
'/dummyPath',
267+
[
268+
FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME,
269+
],
270+
0
271+
);
272+
273+
$user = $this->getMockBuilder(User::class)
274+
->disableOriginalConstructor()->getMock();
275+
276+
$node->expects($this->once())
277+
->method('getOwner')
278+
->willReturn($user);
279+
280+
$loggedInUser = $this->getMockBuilder(User::class)
281+
->disableOriginalConstructor()->getMock();
282+
$this->userSession->expects($this->once())
283+
->method('getUser')
284+
->willReturn($loggedInUser);
285+
286+
$user
287+
->expects($this->once())
288+
->method('getDisplayName')
289+
->willReturn('M. Foo');
290+
291+
$this->accountManager->expects($this->never())
292+
->method('getAccount');
293+
294+
$this->plugin->handleGetProperties(
295+
$propFind,
296+
$node
297+
);
298+
299+
$this->assertEquals('M. Foo', $propFind->get(FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME));
300+
}
301+
182302
public function testGetPropertiesStorageNotAvailable(): void {
183303
/** @var File|\PHPUnit\Framework\MockObject\MockObject $node */
184304
$node = $this->createTestNode('\OCA\DAV\Connector\Sabre\File');
@@ -215,6 +335,7 @@ public function testGetPublicPermissions(): void {
215335
$this->previewManager,
216336
$this->userSession,
217337
$this->filenameValidator,
338+
$this->accountManager,
218339
true,
219340
);
220341
$this->plugin->initialize($this->server);

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use OCA\DAV\Connector\Sabre\Directory;
1212
use OCA\DAV\Connector\Sabre\FilesPlugin;
1313
use OCA\DAV\Connector\Sabre\FilesReportPlugin as FilesReportPluginImplementation;
14+
use OCP\Accounts\IAccountManager;
1415
use OCP\App\IAppManager;
1516
use OCP\Files\File;
1617
use OCP\Files\FileInfo;
@@ -389,6 +390,7 @@ public function testPrepareResponses(): void {
389390
->getMock();
390391

391392
$validator = $this->createMock(IFilenameValidator::class);
393+
$accountManager = $this->createMock(IAccountManager::class);
392394

393395
$this->server->addPlugin(
394396
new FilesPlugin(
@@ -398,6 +400,7 @@ public function testPrepareResponses(): void {
398400
$this->previewManager,
399401
$this->createMock(IUserSession::class),
400402
$validator,
403+
$accountManager,
401404
)
402405
);
403406
$this->plugin->initialize($this->server);

apps/files_sharing/src/views/FilesHeaderNoteToRecipient.vue

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<NcNoteCard v-if="note.length > 0"
77
class="note-to-recipient"
88
type="info">
9-
<p v-if="user" class="note-to-recipient__heading">
9+
<p v-if="displayName" class="note-to-recipient__heading">
1010
{{ t('files_sharing', 'Note from') }}
1111
<NcUserBubble :user="user.id" :display-name="user.displayName" />
1212
</p>
@@ -28,13 +28,13 @@ import NcUserBubble from '@nextcloud/vue/dist/Components/NcUserBubble.js'
2828
2929
const folder = ref<Folder>()
3030
const note = computed<string>(() => folder.value?.attributes.note ?? '')
31+
const displayName = computed<string>(() => folder.value?.attributes['owner-display-name'] ?? '')
3132
const user = computed(() => {
3233
const id = folder.value?.owner
33-
const displayName = folder.value?.attributes?.['owner-display-name']
3434
if (id !== getCurrentUser()?.uid) {
3535
return {
3636
id,
37-
displayName,
37+
displayName: displayName.value,
3838
}
3939
}
4040
return null

0 commit comments

Comments
 (0)