From eae6ab29ec1814932e8bada5478638edfa6829ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 21 Jul 2021 14:23:49 +0200 Subject: [PATCH] Make sure that the dav propfind plugins always use the proper user id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For old android versions it could happen that the requests are performed with a login name instead of the actual user id, so before this change the property methods used the wrong value for fetching their information Signed-off-by: Julius Härtl --- apps/dav/lib/Connector/Sabre/FilesPlugin.php | 26 ++++++++++++++++--- .../dav/lib/Connector/Sabre/ServerFactory.php | 1 + apps/dav/lib/Server.php | 1 + .../unit/Connector/Sabre/FilesPluginTest.php | 26 +++++++++---------- .../Connector/Sabre/FilesReportPluginTest.php | 7 +++-- 5 files changed, 41 insertions(+), 20 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index 2a1412a8d97d3..3f2ae0f35eccb 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -40,6 +40,7 @@ use OCP\IConfig; use OCP\IPreview; use OCP\IRequest; +use OCP\IUserSession; use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\Exception\NotFound; use Sabre\DAV\IFile; @@ -88,6 +89,11 @@ class FilesPlugin extends ServerPlugin { */ private $tree; + /** + * @var IUserSession + */ + private $userSession; + /** * Whether this is public webdav. * If true, some returned information will be stripped off. @@ -128,11 +134,13 @@ public function __construct(Tree $tree, IConfig $config, IRequest $request, IPreview $previewManager, + IUserSession $userSession, $isPublic = false, $downloadAttachment = true) { $this->tree = $tree; $this->config = $config; $this->request = $request; + $this->userSession = $userSession; $this->isPublic = $isPublic; $this->downloadAttachment = $downloadAttachment; $this->previewManager = $previewManager; @@ -322,14 +330,22 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node) }); $propFind->handle(self::SHARE_PERMISSIONS_PROPERTYNAME, function () use ($node, $httpRequest) { + $user = $this->userSession->getUser(); + if ($user === null) { + return null; + } return $node->getSharePermissions( - $httpRequest->getRawServerValue('PHP_AUTH_USER') + $user->getUID() ); }); $propFind->handle(self::OCM_SHARE_PERMISSIONS_PROPERTYNAME, function () use ($node, $httpRequest) { + $user = $this->userSession->getUser(); + if ($user === null) { + return null; + } $ncPermissions = $node->getSharePermissions( - $httpRequest->getRawServerValue('PHP_AUTH_USER') + $user->getUID() ); $ocmPermissions = $this->ncPermissions2ocmPermissions($ncPermissions); return json_encode($ocmPermissions); @@ -367,8 +383,12 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node) }); $propFind->handle(self::SHARE_NOTE, function () use ($node, $httpRequest) { + $user = $this->userSession->getUser(); + if ($user === null) { + return null; + } return $node->getNoteFromShare( - $httpRequest->getRawServerValue('PHP_AUTH_USER') + $user->getUID() ); }); } diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 5a8b109cd381e..7be24014881c9 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -171,6 +171,7 @@ public function createServer($baseUri, $this->config, $this->request, $this->previewManager, + $this->userSession, false, !$this->config->getSystemValue('debug', false) ) diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 9193da49b48cc..f22cf888e1cbf 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -238,6 +238,7 @@ public function __construct(IRequest $request, $baseUri) { \OC::$server->getConfig(), $this->request, \OC::$server->getPreviewManager(), + \OC::$server->getUserSession(), false, !\OC::$server->getConfig()->getSystemValue('debug', false) ) diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php index 9f254822f3257..52fcab8a301bc 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php @@ -41,6 +41,8 @@ use OCP\IConfig; use OCP\IPreview; use OCP\IRequest; +use OCP\IUserSession; +use PHPUnit\Framework\MockObject\MockObject; use Sabre\DAV\PropFind; use Sabre\DAV\PropPatch; use Sabre\DAV\Server; @@ -99,30 +101,27 @@ class FilesPluginTest extends TestCase { */ private $previewManager; + /** @var IUserSession|MockObject */ + private $userSession; + protected function setUp(): void { parent::setUp(); - $this->server = $this->getMockBuilder(Server::class) - ->disableOriginalConstructor() - ->getMock(); - $this->tree = $this->getMockBuilder(Tree::class) - ->disableOriginalConstructor() - ->getMock(); + $this->server = $this->createMock(Server::class); + $this->tree = $this->createMock(Tree::class); $this->config = $this->createMock(IConfig::class); $this->config->expects($this->any())->method('getSystemValue') ->with($this->equalTo('data-fingerprint'), $this->equalTo('')) ->willReturn('my_fingerprint'); - $this->request = $this->getMockBuilder(IRequest::class) - ->disableOriginalConstructor() - ->getMock(); - $this->previewManager = $this->getMockBuilder(IPreview::class) - ->disableOriginalConstructor() - ->getMock(); + $this->request = $this->createMock(IRequest::class); + $this->previewManager = $this->createMock(IPreview::class); + $this->userSession = $this->createMock(IUserSession::class); $this->plugin = new FilesPlugin( $this->tree, $this->config, $this->request, - $this->previewManager + $this->previewManager, + $this->userSession ); $response = $this->getMockBuilder(ResponseInterface::class) @@ -264,6 +263,7 @@ public function testGetPublicPermissions() { ->disableOriginalConstructor() ->getMock(), $this->previewManager, + $this->userSession, 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 8f59b5c791b09..1d1329bbb3c8f 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php @@ -406,10 +406,9 @@ public function testPrepareResponses() { new \OCA\DAV\Connector\Sabre\FilesPlugin( $this->tree, $config, - $this->getMockBuilder(IRequest::class) - ->disableOriginalConstructor() - ->getMock(), - $this->previewManager + $this->createMock(IRequest::class), + $this->previewManager, + $this->createMock(IUserSession::class) ) ); $this->plugin->initialize($this->server);