From 343e8236a1f42ddd3f141d9e5072192326c79901 Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Tue, 16 Sep 2025 17:51:01 +0200 Subject: [PATCH 1/2] refactor: extract logic to hide mounts and share information The permission string for directories and files can contain M or S depending if they are respectively coming from a mount or a share. This information is not to be disclosed when the share is a public one. Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- apps/dav/lib/Connector/Sabre/FilesPlugin.php | 8 ++------ apps/dav/lib/Connector/Sabre/Node.php | 7 +++++++ apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php | 9 ++++++--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index 7b2f144dfa1d8..f751883abec9a 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -40,6 +40,7 @@ use Sabre\HTTP\ResponseInterface; class FilesPlugin extends ServerPlugin { + // namespace public const NS_OWNCLOUD = 'http://owncloud.org/ns'; public const NS_NEXTCLOUD = 'http://nextcloud.org/ns'; @@ -316,12 +317,7 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node) }); $propFind->handle(self::PERMISSIONS_PROPERTYNAME, function () use ($node) { - $perms = $node->getDavPermissions(); - if ($this->isPublic) { - // remove mount information - $perms = str_replace(['S', 'M'], '', $perms); - } - return $perms; + return $this->isPublic ? $node->getPublicDavPermissions() : $node->getDavPermissions(); }); $propFind->handle(self::SHARE_PERMISSIONS_PROPERTYNAME, function () use ($node, $httpRequest) { diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index 14ac7063ace4e..62512b7eaf87d 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -337,6 +337,13 @@ public function getDavPermissions() { return DavUtil::getDavPermissions($this->info); } + /** + * Returns the DAV Permissions with share and mount infromation stripped. + */ + public function getPublicDavPermissions(): string { + return str_replace(['S', 'M'], '', $this->getDavPermissions()); + } + public function getOwner() { return $this->info->getOwner(); } diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php index 8d27bc337e4af..179c79f9ebc50 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php @@ -327,9 +327,12 @@ public function testGetPublicPermissions(): void { /** @var File&MockObject $node */ $node = $this->createTestNode(File::class); - $node->expects($this->any()) - ->method('getDavPermissions') - ->willReturn('DWCKMSR'); + $node->expects($this->once()) + ->method('getPublicDavPermissions') + ->willReturn('DWCKR'); + + $node->expects($this->never()) + ->method('getDavPermissions'); $this->plugin->handleGetProperties( $propFind, From 0b577efcefe05733b99ac4fbe408404416b95652 Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Wed, 20 Aug 2025 17:10:13 +0200 Subject: [PATCH 2/2] feat: add oc-ownerid and oc-permissions headers on PUT DAV requests Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + .../Connector/Sabre/AddExtraHeadersPlugin.php | 69 ++++++++++ .../dav/lib/Connector/Sabre/ServerFactory.php | 1 + apps/dav/lib/Server.php | 2 + .../Sabre/AddExtraHeadersPluginTest.php | 130 ++++++++++++++++++ 6 files changed, 204 insertions(+) create mode 100644 apps/dav/lib/Connector/Sabre/AddExtraHeadersPlugin.php create mode 100644 apps/dav/tests/unit/Connector/Sabre/AddExtraHeadersPluginTest.php diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 20e9e66cfd4f0..21b52d348fe07 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -209,6 +209,7 @@ 'OCA\\DAV\\ConfigLexicon' => $baseDir . '/../lib/ConfigLexicon.php', 'OCA\\DAV\\Connector\\LegacyDAVACL' => $baseDir . '/../lib/Connector/LegacyDAVACL.php', 'OCA\\DAV\\Connector\\LegacyPublicAuth' => $baseDir . '/../lib/Connector/LegacyPublicAuth.php', + 'OCA\\DAV\\Connector\\Sabre\\AddExtraHeadersPlugin' => $baseDir . '/../lib/Connector/Sabre/AddExtraHeadersPlugin.php', 'OCA\\DAV\\Connector\\Sabre\\AnonymousOptionsPlugin' => $baseDir . '/../lib/Connector/Sabre/AnonymousOptionsPlugin.php', 'OCA\\DAV\\Connector\\Sabre\\AppleQuirksPlugin' => $baseDir . '/../lib/Connector/Sabre/AppleQuirksPlugin.php', 'OCA\\DAV\\Connector\\Sabre\\Auth' => $baseDir . '/../lib/Connector/Sabre/Auth.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index c309c7467bf54..af95717abea45 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -224,6 +224,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\ConfigLexicon' => __DIR__ . '/..' . '/../lib/ConfigLexicon.php', 'OCA\\DAV\\Connector\\LegacyDAVACL' => __DIR__ . '/..' . '/../lib/Connector/LegacyDAVACL.php', 'OCA\\DAV\\Connector\\LegacyPublicAuth' => __DIR__ . '/..' . '/../lib/Connector/LegacyPublicAuth.php', + 'OCA\\DAV\\Connector\\Sabre\\AddExtraHeadersPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/AddExtraHeadersPlugin.php', 'OCA\\DAV\\Connector\\Sabre\\AnonymousOptionsPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/AnonymousOptionsPlugin.php', 'OCA\\DAV\\Connector\\Sabre\\AppleQuirksPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/AppleQuirksPlugin.php', 'OCA\\DAV\\Connector\\Sabre\\Auth' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Auth.php', diff --git a/apps/dav/lib/Connector/Sabre/AddExtraHeadersPlugin.php b/apps/dav/lib/Connector/Sabre/AddExtraHeadersPlugin.php new file mode 100644 index 0000000000000..e728c3f83b7d7 --- /dev/null +++ b/apps/dav/lib/Connector/Sabre/AddExtraHeadersPlugin.php @@ -0,0 +1,69 @@ +server = $server; + + $server->on('afterMethod:PUT', $this->afterPut(...)); + } + + private function afterPut(RequestInterface $request, ResponseInterface $response): void { + if ($this->server === null) { + return; + } + + $node = null; + try { + $node = $this->server->tree->getNodeForPath($request->getPath()); + } catch (NotFound) { + $this->logger->error("Cannot set extra headers for non-existing file '{$request->getPath()}'"); + return; + } + + if (!$node instanceof Node) { + $nodeType = get_debug_type($node); + $this->logger->error("Cannot set extra headers for node of type {$nodeType} for file '{$request->getPath()}'"); + return; + } + + if (!$this->isPublic) { + $ownerId = $node->getOwner()?->getUID(); + if ($ownerId !== null) { + $response->setHeader('X-NC-OwnerId', $ownerId); + } + } + + $permissions = $this->isPublic ? $node->getPublicDavPermissions() + : $node->getDavPermissions(); + + $response->setHeader('X-NC-Permissions', $permissions); + } +} diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 19dd5584c5167..5799a85b0df05 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -209,6 +209,7 @@ public function createServer( ); } $server->addPlugin(new CopyEtagHeaderPlugin()); + $server->addPlugin(new AddExtraHeadersPlugin($this->logger, $isPublicShare)); // Load dav plugins from apps $event = new SabrePluginEvent($server); diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 7398af83377d5..4d1f02993270e 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -27,6 +27,7 @@ use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin; use OCA\DAV\CardDAV\Validation\CardDavValidatePlugin; use OCA\DAV\Comments\CommentsPlugin; +use OCA\DAV\Connector\Sabre\AddExtraHeadersPlugin; use OCA\DAV\Connector\Sabre\AnonymousOptionsPlugin; use OCA\DAV\Connector\Sabre\AppleQuirksPlugin; use OCA\DAV\Connector\Sabre\Auth; @@ -384,6 +385,7 @@ public function __construct( ) ); } + $this->server->addPlugin(new AddExtraHeadersPlugin($logger, false)); $this->server->addPlugin(new EnablePlugin( \OCP\Server::get(IConfig::class), \OCP\Server::get(BirthdayService::class), diff --git a/apps/dav/tests/unit/Connector/Sabre/AddExtraHeadersPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/AddExtraHeadersPluginTest.php new file mode 100644 index 0000000000000..84a99f0c52d21 --- /dev/null +++ b/apps/dav/tests/unit/Connector/Sabre/AddExtraHeadersPluginTest.php @@ -0,0 +1,130 @@ + [ + 'user', true, 'PERMISSIONS', true, 2 + ], + 'permissions only' => [ + null, false, 'PERMISSIONS', true, 1 + ], + ]; + } + + public function testAfterPutNotFoundException(): void { + $afterPut = null; + $this->server->expects($this->once()) + ->method('on') + ->willReturnCallback( + function ($method, $callback) use (&$afterPut) { + $this->assertSame('afterMethod:PUT', $method); + $afterPut = $callback; + }); + + $this->plugin->initialize($this->server); + $node = $this->createMock(Node::class); + $this->tree->expects($this->once())->method('getNodeForPath') + ->willThrowException(new NotFound()); + + $this->logger->expects($this->once())->method('error'); + + $afterPut($this->request, $this->response); + } + + #[DataProvider('afterPutData')] + public function testAfterPut(?string $ownerId, bool $expectOwnerIdHeader, + ?string $permissions, bool $expectPermissionsHeader, + int $expectedInvocations): void { + $afterPut = null; + $this->server->expects($this->once()) + ->method('on') + ->willReturnCallback( + function ($method, $callback) use (&$afterPut) { + $this->assertSame('afterMethod:PUT', $method); + $afterPut = $callback; + }); + + $this->plugin->initialize($this->server); + $node = $this->createMock(Node::class); + $this->tree->expects($this->once())->method('getNodeForPath') + ->willReturn($node); + + $user = $this->createMock(IUser::class); + $node->expects($this->once())->method('getOwner')->willReturn($user); + $user->expects($this->once())->method('getUID')->willReturn($ownerId); + $node->expects($this->once())->method('getDavPermissions')->willReturn($permissions); + + $matcher = $this->exactly($expectedInvocations); + $this->response->expects($matcher)->method('setHeader') + ->willReturnCallback(function ($name, $value) use ( + $expectedInvocations, + $expectPermissionsHeader, + $expectOwnerIdHeader, + $matcher, + $ownerId, $permissions) { + $invocationNumber = $matcher->numberOfInvocations(); + if ($invocationNumber === 0) { + throw new LogicException('No invocations were expected'); + } + + if (($expectOwnerIdHeader && $expectedInvocations === 1) + || ($expectedInvocations + === 2 && $invocationNumber === 1)) { + $this->assertEquals('X-NC-OwnerId', $name); + $this->assertEquals($ownerId, $value); + } + + if (($expectPermissionsHeader && $expectedInvocations === 1) + || ($expectedInvocations + === 2 && $invocationNumber === 2)) { + $this->assertEquals('X-NC-Permissions', $name); + $this->assertEquals($permissions, $value); + } + }); + + $afterPut($this->request, $this->response); + } + + protected function setUp(): void { + parent::setUp(); + + $this->server = $this->createMock(Server::class); + $this->tree = $this->createMock(Tree::class); + $this->server->tree = $this->tree; + $this->logger = $this->createMock(LoggerInterface::class); + $this->plugin = new AddExtraHeadersPlugin($this->logger, false); + $this->request = $this->createMock(RequestInterface::class); + $this->response = $this->createMock(ResponseInterface::class); + } +}