diff options
18 files changed, 514 insertions, 58 deletions
diff --git a/apps/dav/appinfo/v2/publicremote.php b/apps/dav/appinfo/v2/publicremote.php index 501f9188c1f..62bc75a1d3b 100644 --- a/apps/dav/appinfo/v2/publicremote.php +++ b/apps/dav/appinfo/v2/publicremote.php @@ -29,6 +29,10 @@ * along with this program. If not, see <http://www.gnu.org/licenses/> * */ + +use OCP\EventDispatcher\IEventDispatcher; +use Psr\Log\LoggerInterface; + // load needed apps $RUNTIME_APPTYPES = ['filesystem', 'authentication', 'logging']; @@ -42,7 +46,8 @@ $authBackend = new OCA\DAV\Connector\Sabre\PublicAuth( \OC::$server->getRequest(), \OC::$server->getShareManager(), \OC::$server->getSession(), - \OC::$server->getBruteForceThrottler() + \OC::$server->getBruteForceThrottler(), + \OC::$server->query(LoggerInterface::class) ); $authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); @@ -55,7 +60,7 @@ $serverFactory = new OCA\DAV\Connector\Sabre\ServerFactory( \OC::$server->getTagManager(), \OC::$server->getRequest(), \OC::$server->getPreviewManager(), - \OC::$server->getEventDispatcher(), + \OC::$server->query(IEventDispatcher::class), \OC::$server->getL10N('dav') ); @@ -65,6 +70,7 @@ $linkCheckPlugin = new \OCA\DAV\Files\Sharing\PublicLinkCheckPlugin(); $filesDropPlugin = new \OCA\DAV\Files\Sharing\FilesDropPlugin(); // Define root url with /public.php/dav/files/TOKEN +// $baseuri is defined in public.php preg_match('/(^files\/\w+)/i', substr($requestUri, strlen($baseuri)), $match); $baseuri = $baseuri . $match[0]; diff --git a/apps/dav/lib/Connector/Sabre/PublicAuth.php b/apps/dav/lib/Connector/Sabre/PublicAuth.php index 5c71d2b47dc..d5b3d41e1ef 100644 --- a/apps/dav/lib/Connector/Sabre/PublicAuth.php +++ b/apps/dav/lib/Connector/Sabre/PublicAuth.php @@ -57,21 +57,23 @@ class PublicAuth extends AbstractBasic { private const BRUTEFORCE_ACTION = 'public_dav_auth'; public const DAV_AUTHENTICATED = 'public_link_authenticated'; - private IShare $share; + private ?IShare $share = null; private IManager $shareManager; private ISession $session; private IRequest $request; private IThrottler $throttler; - + private LoggerInterface $logger; public function __construct(IRequest $request, - IManager $shareManager, - ISession $session, - IThrottler $throttler) { + IManager $shareManager, + ISession $session, + IThrottler $throttler, + LoggerInterface $logger) { $this->request = $request; $this->shareManager = $shareManager; $this->session = $session; $this->throttler = $throttler; + $this->logger = $logger; // setup realm $defaults = new \OCP\Defaults(); @@ -108,7 +110,7 @@ class PublicAuth extends AbstractBasic { } catch (\Exception $e) { $class = get_class($e); $msg = $e->getMessage(); - \OC::$server->get(LoggerInterface::class)->error($e->getMessage(), ['exception' => $e]); + $this->logger->error($e->getMessage(), ['exception' => $e]); throw new ServiceUnavailable("$class: $msg"); } } @@ -118,8 +120,8 @@ class PublicAuth extends AbstractBasic { * @return string * @throws NotFound */ - private function getToken(): string { - $path = $this->request->getPathInfo(); + private function getToken(): string { + $path = $this->request->getPathInfo() ?: ''; // ['', 'dav', 'files', 'token'] $splittedPath = explode('/', $path); @@ -140,6 +142,7 @@ class PublicAuth extends AbstractBasic { $token = $this->getToken(); try { + /** @var IShare $share */ $share = $this->shareManager->getShareByToken($token); } catch (ShareNotFound $e) { $this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress()); @@ -176,7 +179,7 @@ class PublicAuth extends AbstractBasic { * @return bool * @throws NotAuthenticated */ - protected function validateUserPass($username, $password): bool { + protected function validateUserPass($username, $password) { $this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), self::BRUTEFORCE_ACTION); $token = $this->getToken(); @@ -203,6 +206,11 @@ class PublicAuth extends AbstractBasic { } return true; } + + if ($this->session->exists(PublicAuth::DAV_AUTHENTICATED) + && $this->session->get(PublicAuth::DAV_AUTHENTICATED) === $share->getId()) { + return true; + } if (in_array('XMLHttpRequest', explode(',', $this->request->getHeader('X-Requested-With')))) { // do not re-authenticate over ajax, use dummy auth name to prevent browser popup @@ -219,9 +227,9 @@ class PublicAuth extends AbstractBasic { $this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress()); return false; - } else { - return true; } + + return true; } public function getShare(): IShare { diff --git a/apps/dav/tests/unit/Connector/PublicAuthTest.php b/apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php index 4a2ebb496b0..1c83beb8389 100644 --- a/apps/dav/tests/unit/Connector/PublicAuthTest.php +++ b/apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php @@ -34,13 +34,13 @@ use OCP\Share\IManager; use OCP\Share\IShare; /** - * Class PublicAuthTest + * Class LegacyPublicAuthTest * * @group DB * * @package OCA\DAV\Tests\unit\Connector */ -class PublicAuthTest extends \Test\TestCase { +class LegacyPublicAuthTest extends \Test\TestCase { /** @var ISession|\PHPUnit\Framework\MockObject\MockObject */ private $session; @@ -48,7 +48,7 @@ class PublicAuthTest extends \Test\TestCase { private $request; /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ private $shareManager; - /** @var \OCA\DAV\Connector\PublicAuth */ + /** @var \OCA\DAV\Connector\LegacyPublicAuth */ private $auth; /** @var IThrottler|\PHPUnit\Framework\MockObject\MockObject */ private $throttler; @@ -72,7 +72,7 @@ class PublicAuthTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); - $this->auth = new \OCA\DAV\Connector\PublicAuth( + $this->auth = new \OCA\DAV\Connector\LegacyPublicAuth( $this->request, $this->shareManager, $this->session, @@ -195,7 +195,7 @@ class PublicAuthTest extends \Test\TestCase { $this->assertTrue($result); } - public function testSharePasswordLinkValidSession(): void { + public function testInvalidSharePasswordLinkValidSession(): void { $share = $this->getMockBuilder(IShare::class) ->disableOriginalConstructor() ->getMock(); diff --git a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php new file mode 100644 index 00000000000..34990550bb0 --- /dev/null +++ b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php @@ -0,0 +1,433 @@ +<?php +/** + * @copyright Copyright (c) 2016, ownCloud, Inc. + * + * @author Bjoern Schiessle <bjoern@schiessle.org> + * @author Joas Schilling <coding@schilljs.com> + * @author Lukas Reschke <lukas@statuscode.ch> + * @author Morris Jobke <hey@morrisjobke.de> + * @author Roeland Jago Douma <roeland@famdouma.nl> + * @author Thomas Müller <thomas.mueller@tmit.eu> + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see <http://www.gnu.org/licenses/> + * + */ +namespace OCA\DAV\Tests\unit\Connector; + +use OCP\IRequest; +use OCP\ISession; +use OCP\Security\Bruteforce\IThrottler; +use OCP\Share\Exceptions\ShareNotFound; +use OCP\Share\IManager; +use OCP\Share\IShare; +use Psr\Log\LoggerInterface; + +/** + * Class PublicAuthTest + * + * @group DB + * + * @package OCA\DAV\Tests\unit\Connector + */ +class PublicAuthTest extends \Test\TestCase { + + /** @var ISession|MockObject */ + private $session; + /** @var IRequest|MockObject */ + private $request; + /** @var IManager|MockObject */ + private $shareManager; + /** @var PublicAuth */ + private $auth; + /** @var IThrottler|MockObject */ + private $throttler; + /** @var LoggerInterface|MockObject */ + private $logger; + + /** @var string */ + private $oldUser; + + protected function setUp(): void { + parent::setUp(); + + $this->session = $this->getMockBuilder(ISession::class) + ->disableOriginalConstructor() + ->getMock(); + $this->request = $this->getMockBuilder(IRequest::class) + ->disableOriginalConstructor() + ->getMock(); + $this->shareManager = $this->getMockBuilder(IManager::class) + ->disableOriginalConstructor() + ->getMock(); + $this->throttler = $this->getMockBuilder(IThrottler::class) + ->disableOriginalConstructor() + ->getMock(); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->auth = new \OCA\DAV\Connector\Sabre\PublicAuth( + $this->request, + $this->shareManager, + $this->session, + $this->throttler, + $this->logger, + ); + + // Store current user + $this->oldUser = \OC_User::getUser(); + } + + protected function tearDown(): void { + \OC_User::setIncognitoMode(false); + + // Set old user + \OC_User::setUserId($this->oldUser); + \OC_Util::setupFS($this->oldUser); + + parent::tearDown(); + } + + public function testGetToken(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $result = $this->invokePrivate($this->auth, 'getToken'); + + $this->assertSame('GX9HSGQrGE', $result); + } + + public function testGetTokenInvalid(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files'); + + $this->expectException(\Sabre\DAV\Exception\NotFound::class); + $this->invokePrivate($this->auth, 'getToken'); + } + + public function testCheckTokenValidShare(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn(null); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $result = $this->invokePrivate($this->auth, 'checkToken'); + $this->assertSame([true, 'principals/GX9HSGQrGE'], $result); + } + + public function testCheckTokenInvalidShare(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->will($this->throwException(new ShareNotFound())); + + $this->expectException(\Sabre\DAV\Exception\NotFound::class); + $this->invokePrivate($this->auth, 'checkToken'); + } + + public function testCheckTokenAlreadyAuthenticated(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getShareType')->willReturn(42); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); + $this->session->method('get')->with('public_link_authenticated')->willReturn('42'); + + $result = $this->invokePrivate($this->auth, 'checkToken'); + $this->assertSame([true, 'principals/GX9HSGQrGE'], $result); + } + + public function testCheckTokenPasswordNotAuthenticated(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn('password'); + $share->method('getShareType')->willReturn(42); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(false); + + $this->expectException(\Sabre\DAV\Exception\NotAuthenticated::class); + $this->invokePrivate($this->auth, 'checkToken'); + } + + public function testCheckTokenPasswordAuthenticatedWrongShare(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn('password'); + $share->method('getShareType')->willReturn(42); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(false); + $this->session->method('get')->with('public_link_authenticated')->willReturn('43'); + + $this->expectException(\Sabre\DAV\Exception\NotAuthenticated::class); + $this->invokePrivate($this->auth, 'checkToken'); + } + + public function testNoShare(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willThrowException(new ShareNotFound()); + + $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); + + $this->assertFalse($result); + } + + public function testShareNoPassword(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn(null); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); + + $this->assertTrue($result); + } + + public function testSharePasswordFancyShareType(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn('password'); + $share->method('getShareType')->willReturn(42); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); + + $this->assertFalse($result); + } + + + public function testSharePasswordRemote(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn('password'); + $share->method('getShareType')->willReturn(IShare::TYPE_REMOTE); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); + + $this->assertTrue($result); + } + + public function testSharePasswordLinkValidPassword(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn('password'); + $share->method('getShareType')->willReturn(IShare::TYPE_LINK); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $this->shareManager->expects($this->once()) + ->method('checkPassword')->with( + $this->equalTo($share), + $this->equalTo('password') + )->willReturn(true); + + $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); + + $this->assertTrue($result); + } + + public function testSharePasswordMailValidPassword(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn('password'); + $share->method('getShareType')->willReturn(IShare::TYPE_EMAIL); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $this->shareManager->expects($this->once()) + ->method('checkPassword')->with( + $this->equalTo($share), + $this->equalTo('password') + )->willReturn(true); + + $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); + + $this->assertTrue($result); + } + + public function testInvalidSharePasswordLinkValidSession(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn('password'); + $share->method('getShareType')->willReturn(IShare::TYPE_LINK); + $share->method('getId')->willReturn('42'); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $this->shareManager->expects($this->once()) + ->method('checkPassword') + ->with( + $this->equalTo($share), + $this->equalTo('password') + )->willReturn(false); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); + $this->session->method('get')->with('public_link_authenticated')->willReturn('42'); + + $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); + + $this->assertTrue($result); + } + + public function testSharePasswordLinkInvalidSession(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn('password'); + $share->method('getShareType')->willReturn(IShare::TYPE_LINK); + $share->method('getId')->willReturn('42'); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $this->shareManager->expects($this->once()) + ->method('checkPassword') + ->with( + $this->equalTo($share), + $this->equalTo('password') + )->willReturn(false); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); + $this->session->method('get')->with('public_link_authenticated')->willReturn('43'); + + $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); + + $this->assertFalse($result); + } + + + public function testSharePasswordMailInvalidSession(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + + $share = $this->getMockBuilder(IShare::class) + ->disableOriginalConstructor() + ->getMock(); + $share->method('getPassword')->willReturn('password'); + $share->method('getShareType')->willReturn(IShare::TYPE_EMAIL); + $share->method('getId')->willReturn('42'); + + $this->shareManager->expects($this->once()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $this->shareManager->expects($this->once()) + ->method('checkPassword') + ->with( + $this->equalTo($share), + $this->equalTo('password') + )->willReturn(false); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); + $this->session->method('get')->with('public_link_authenticated')->willReturn('43'); + + $result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); + + $this->assertFalse($result); + } +} diff --git a/apps/files_sharing/js/files_drop.js b/apps/files_sharing/js/files_drop.js index ffbe8284433..5215cfc5b54 100644 --- a/apps/files_sharing/js/files_drop.js +++ b/apps/files_sharing/js/files_drop.js @@ -24,10 +24,9 @@ var filesClient = new OC.Files.Client({ host: OC.getHost(), port: OC.getPort(), - userName: $('#sharingToken').val(), // note: password not be required, the endpoint // will recognize previous validation from the session - root: OC.getRootPath() + '/public.php/webdav', + root: OC.getRootPath() + '/public.php/dav/files/' + $('#sharingToken').val() + '/', useHTTPS: OC.getProtocol() === 'https' }); @@ -45,7 +44,7 @@ return false; } var base = OC.getProtocol() + '://' + OC.getHost(); - data.url = base + OC.getRootPath() + '/public.php/webdav/' + encodeURI(name); + data.url = base + OC.getRootPath() + '/public.php/dav/files/' + $('#sharingToken').val() + '/' + encodeURI(name); data.multipart = false; @@ -53,14 +52,6 @@ data.headers = {}; } - var userName = filesClient.getUserName(); - var password = filesClient.getPassword(); - if (userName) { - // copy username/password from DAV client - data.headers['Authorization'] = - 'Basic ' + btoa(userName + ':' + (password || '')); - } - $('#drop-upload-done-indicator').addClass('hidden'); $('#drop-upload-progress-indicator').removeClass('hidden'); diff --git a/apps/files_sharing/js/public.js b/apps/files_sharing/js/public.js index 154b970c5be..d26cecd02ee 100644 --- a/apps/files_sharing/js/public.js +++ b/apps/files_sharing/js/public.js @@ -69,10 +69,9 @@ OCA.Sharing.PublicApp = { var filesClient = new OC.Files.Client({ host: OC.getHost(), port: OC.getPort(), - userName: token, // note: password not be required, the endpoint // will recognize previous validation from the session - root: OC.getRootPath() + '/public.php/webdav', + root: OC.getRootPath() + '/public.php/dav/files/' + token + '/', useHTTPS: OC.getProtocol() === 'https' }); @@ -167,11 +166,10 @@ OCA.Sharing.PublicApp = { return; } // Undocumented Url to public WebDAV endpoint - var url = parent.location.protocol + '//' + location.host + OC.linkTo('', 'public.php/webdav'); + var url = parent.location.protocol + '//' + location.host + OC.linkTo('', 'public.php/dav/files/'+ token); $.ajax({ url: url, headers: { - Authorization: 'Basic ' + btoa(token + ':'), Range: 'bytes=0-10000' } }).then(function (data) { @@ -247,7 +245,9 @@ OCA.Sharing.PublicApp = { // also add auth in URL due to POST workaround base = OC.getProtocol() + '://' + token + '@' + OC.getHost() + (OC.getPort() ? ':' + OC.getPort() : ''); } - return base + OC.getRootPath() + '/public.php/webdav' + encodedPath; + + // encodedPath starts with a leading slash + return base + OC.getRootPath() + '/public.php/dav/files/' + token + encodedPath; }; this.fileList.getAjaxUrl = function (action, params) { diff --git a/apps/files_sharing/lib/Controller/PublicPreviewController.php b/apps/files_sharing/lib/Controller/PublicPreviewController.php index 54a1a321665..c59baac73c4 100644 --- a/apps/files_sharing/lib/Controller/PublicPreviewController.php +++ b/apps/files_sharing/lib/Controller/PublicPreviewController.php @@ -61,7 +61,7 @@ class PublicPreviewController extends PublicShareController { $this->previewManager = $previewManager; } - protected function getPasswordHash(): string { + protected function getPasswordHash(): ?string { return $this->share->getPassword(); } diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index bd9cdffa83d..eebf964c8f9 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -204,7 +204,7 @@ class ShareController extends AuthPublicShareController { return $this->shareManager->checkPassword($this->share, $password); } - protected function getPasswordHash(): string { + protected function getPasswordHash(): ?string { return $this->share->getPassword(); } diff --git a/apps/files_sharing/lib/ResponseDefinitions.php b/apps/files_sharing/lib/ResponseDefinitions.php index aa1ee004971..313cf82bd51 100644 --- a/apps/files_sharing/lib/ResponseDefinitions.php +++ b/apps/files_sharing/lib/ResponseDefinitions.php @@ -50,18 +50,18 @@ namespace OCA\Files_Sharing; * mimetype: string, * note: string, * parent: null, - * password?: string, + * password?: null|string, * password_expiration_time?: ?string, * path: ?string, * permissions: int, * send_password_by_talk?: bool, * share_type: int, - * share_with?: string, + * share_with?: null|string, * share_with_avatar?: string, * share_with_displayname?: string, * share_with_displayname_unique?: ?string, * share_with_link?: string, - * status?: array{clearAt?: int|null, icon?: ?string, message?: ?string, status?: string}, + * status?: array{clearAt: int|null, icon: ?string, message: ?string, status: string}, * stime: int, * storage: int, * storage_id: string, diff --git a/apps/files_sharing/openapi.json b/apps/files_sharing/openapi.json index bc8128df0d2..abbc3d250a6 100644 --- a/apps/files_sharing/openapi.json +++ b/apps/files_sharing/openapi.json @@ -598,7 +598,8 @@ "nullable": true }, "password": { - "type": "string" + "type": "string", + "nullable": true }, "password_expiration_time": { "type": "string", @@ -620,7 +621,8 @@ "format": "int64" }, "share_with": { - "type": "string" + "type": "string", + "nullable": true }, "share_with_avatar": { "type": "string" @@ -637,6 +639,12 @@ }, "status": { "type": "object", + "required": [ + "clearAt", + "icon", + "message", + "status" + ], "properties": { "clearAt": { "type": "integer", diff --git a/apps/files_sharing/tests/js/fileDropSpec.js b/apps/files_sharing/tests/js/fileDropSpec.js index efe87802d1c..18eee661e7f 100644 --- a/apps/files_sharing/tests/js/fileDropSpec.js +++ b/apps/files_sharing/tests/js/fileDropSpec.js @@ -91,8 +91,7 @@ describe("files Drop tests", function() { OCA.FilesSharingDrop.addFileToUpload('',data); expect(data.submit.calledOnce).toEqual(true); - expect(data.url).toContain("/public.php/webdav/" + encodeURI(testFile.name)); - expect(data.headers['Authorization']).toEqual('Basic ' + btoa(sharingToken+":")); + expect(data.url).toContain("/public.php/dav/files/" + sharingToken + '/' + encodeURI(testFile.name)); }); } }); diff --git a/apps/files_sharing/tests/js/publicAppSpec.js b/apps/files_sharing/tests/js/publicAppSpec.js index 59ac4bd7bbd..229e57ac4ed 100644 --- a/apps/files_sharing/tests/js/publicAppSpec.js +++ b/apps/files_sharing/tests/js/publicAppSpec.js @@ -107,8 +107,7 @@ describe('OCA.Sharing.PublicApp tests', function() { App.initialize($('#preview')); expect(fakeServer.requests.length).toEqual(1); expect(fakeServer.requests[0].method).toEqual('PROPFIND'); - expect(fakeServer.requests[0].url).toEqual('https://example.com:9876/owncloud/public.php/webdav/subdir'); - expect(fakeServer.requests[0].requestHeaders.Authorization).toEqual('Basic c2g0dG9rOm51bGw='); + expect(fakeServer.requests[0].url).toEqual('https://example.com:9876/owncloud/public.php/dav/files/sh4tok/subdir'); uploaderDetectStub.restore(); }); @@ -149,11 +148,11 @@ describe('OCA.Sharing.PublicApp tests', function() { }); it('returns correct upload URL', function() { expect(fileList.getUploadUrl('some file.txt')) - .toEqual('/owncloud/public.php/webdav/subdir/some%20file.txt'); + .toEqual('/owncloud/public.php/dav/files/sh4tok/subdir/some%20file.txt'); }); it('returns correct upload URL with specified dir', function() { expect(fileList.getUploadUrl('some file.txt', 'sub')) - .toEqual('/owncloud/public.php/webdav/sub/some%20file.txt'); + .toEqual('/owncloud/public.php/dav/files/sh4tok/sub/some%20file.txt'); }); }); }); diff --git a/build/integration/features/bootstrap/FilesDropContext.php b/build/integration/features/bootstrap/FilesDropContext.php index a5d4dad14e3..22eaa639030 100644 --- a/build/integration/features/bootstrap/FilesDropContext.php +++ b/build/integration/features/bootstrap/FilesDropContext.php @@ -45,9 +45,8 @@ class FilesDropContext implements Context, SnippetAcceptingContext { } $base = substr($this->baseUrl, 0, -4); - $fullUrl = $base . '/public.php/webdav' . $path; + $fullUrl = $base . "/public.php/dav/files/$token/$path"; - $options['auth'] = [$token, '']; $options['headers'] = [ 'X-REQUESTED-WITH' => 'XMLHttpRequest' ]; @@ -73,9 +72,8 @@ class FilesDropContext implements Context, SnippetAcceptingContext { } $base = substr($this->baseUrl, 0, -4); - $fullUrl = $base . '/public.php/webdav/' . $folder; + $fullUrl = $base . "/public.php/dav/files/$token/$folder"; - $options['auth'] = [$token, '']; $options['headers'] = [ 'X-REQUESTED-WITH' => 'XMLHttpRequest' ]; diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index a38ef392e61..7993d502707 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -187,8 +187,8 @@ trait Sharing { $token = $this->lastShareData->data->token; } - $fullUrl = substr($this->baseUrl, 0, -4) . "public.php/webdav"; - $this->checkDownload($fullUrl, [$token, $password], 'text/plain'); + $fullUrl = substr($this->baseUrl, 0, -4) . "public.php/dav/files/$token/"; + $this->checkDownload($fullUrl, ['', $password], 'text/plain'); } private function checkDownload($url, $auth = null, $mimeType = null) { diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index c239461b788..b919976e790 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -169,11 +169,10 @@ trait WebDav { */ public function downloadPublicFileWithRange($range) { $token = $this->lastShareData->data->token; - $fullUrl = substr($this->baseUrl, 0, -4) . "public.php/webdav"; + $fullUrl = substr($this->baseUrl, 0, -4) . "public.php/dav/files/$token"; $client = new GClient(); $options = []; - $options['auth'] = [$token, ""]; $options['headers'] = [ 'Range' => $range ]; @@ -187,7 +186,7 @@ trait WebDav { */ public function downloadPublicFileInsideAFolderWithRange($path, $range) { $token = $this->lastShareData->data->token; - $fullUrl = substr($this->baseUrl, 0, -4) . "public.php/webdav" . "$path"; + $fullUrl = substr($this->baseUrl, 0, -4) . "public.php/dav/files/$token/$path"; $client = new GClient(); $options = [ @@ -195,7 +194,6 @@ trait WebDav { 'Range' => $range ] ]; - $options['auth'] = [$token, ""]; $this->response = $client->request("GET", $fullUrl, $options); } diff --git a/build/integration/features/maintenance-mode.feature b/build/integration/features/maintenance-mode.feature index 56d3b9c0fb6..e6637a5edff 100644 --- a/build/integration/features/maintenance-mode.feature +++ b/build/integration/features/maintenance-mode.feature @@ -39,3 +39,9 @@ Feature: maintenance-mode Then the HTTP status code should be "503" Then Maintenance mode is disabled And the command was successful + + Scenario: Accessing /public.php/dav with maintenance mode enabled + When requesting "/public.php/dav" with "GET" + Then the HTTP status code should be "503" + Then Maintenance mode is disabled + And the command was successful diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index ba529091482..0ba7f3163dd 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -105,6 +105,16 @@ <code>$baseuri</code> </UndefinedGlobalVariable> </file> + <file src="apps/dav/appinfo/v2/publicremote.php"> + <InternalMethod> + <code>\OC\Files\Filesystem::logWarningWhenAddingStorageWrapper($previousLog)</code> + <code>\OC\Files\Filesystem::logWarningWhenAddingStorageWrapper(false)</code> + </InternalMethod> + <UndefinedGlobalVariable> + <code>$baseuri</code> + <code>$baseuri</code> + </UndefinedGlobalVariable> + </file> <file src="apps/dav/lib/AppInfo/Application.php"> <InvalidArgument> <code>CalendarDeletionDefaultUpdaterListener::class</code> diff --git a/lib/public/AppFramework/PublicShareController.php b/lib/public/AppFramework/PublicShareController.php index 7c3725c9290..cbcb9343198 100644 --- a/lib/public/AppFramework/PublicShareController.php +++ b/lib/public/AppFramework/PublicShareController.php @@ -86,7 +86,7 @@ abstract class PublicShareController extends Controller { * * @since 14.0.0 */ - abstract protected function getPasswordHash(): string; + abstract protected function getPasswordHash(): ?string; /** * Is the provided token a valid token |