Merge pull request #23736 from owncloud/move_public_webdav_auth_to_share_manager

Move public webdav auth to share manager

5 files changed, 252 insertions, 51 deletions

diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php
index 558a8238666..b26e9ebe7c8 100644
--- a/apps/dav/appinfo/v1/publicwebdav.php
+++ b/apps/dav/appinfo/v1/publicwebdav.php
@@ -32,7 +32,11 @@ OC_App::loadApps($RUNTIME_APPTYPES);
 OC_Util::obEnd();
 
 // Backends
-$authBackend = new OCA\DAV\Connector\PublicAuth(\OC::$server->getConfig(), \OC::$server->getRequest());
+$authBackend = new OCA\DAV\Connector\PublicAuth(
+	\OC::$server->getRequest(),
+	\OC::$server->getShareManager(),
+	\OC::$server->getSession()
+);
 
 $serverFactory = new OCA\DAV\Connector\Sabre\ServerFactory(
 	\OC::$server->getConfig(),
@@ -56,10 +60,9 @@ $server = $serverFactory->createServer($baseuri, $requestUri, $authBackend, func
 	}
 
 	$share = $authBackend->getShare();
-	$rootShare = \OCP\Share::resolveReShare($share);
-	$owner = $rootShare['uid_owner'];
-	$isWritable = $share['permissions'] & (\OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_CREATE);
-	$fileId = $share['file_source'];
+	$owner = $share->getShareOwner();
+	$isWritable = $share->getPermissions() & (\OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_CREATE);
+	$fileId = $share->getNodeId();
 
 	if (!$isWritable) {
 		\OC\Files\Filesystem::addStorageWrapper('readonly', function ($mountPoint, $storage) {
diff --git a/apps/dav/lib/connector/publicauth.php b/apps/dav/lib/connector/publicauth.php
index 0d75a4db493..3aa58cda244 100644
--- a/apps/dav/lib/connector/publicauth.php
+++ b/apps/dav/lib/connector/publicauth.php
@@ -26,31 +26,41 @@
 
 namespace OCA\DAV\Connector;
 
-use OCP\IConfig;
 use OCP\IRequest;
+use OCP\ISession;
+use OCP\Share\Exceptions\ShareNotFound;
+use OCP\Share\IManager;
 
+/**
+ * Class PublicAuth
+ *
+ * @package OCA\DAV\Connector
+ */
 class PublicAuth extends \Sabre\DAV\Auth\Backend\AbstractBasic {
 
-	/**
-	 * @var \OCP\IConfig
-	 */
-	private $config;
-
+	/** @var \OCP\Share\IShare */
 	private $share;
 
-	/**
-	 * @var IRequest
-	 */
+	/** @var IManager */
+	private $shareManager;
+
+	/** @var ISession */
+	private $session;
+
+	/** @var IRequest */
 	private $request;
 
 	/**
-	 * @param \OCP\IConfig $config
 	 * @param IRequest $request
+	 * @param IManager $shareManager
+	 * @param ISession $session
 	 */
-	public function __construct(IConfig $config,
-								IRequest $request) {
-		$this->config = $config;
+	public function __construct(IRequest $request,
+								IManager $shareManager,
+								ISession $session) {
 		$this->request = $request;
+		$this->shareManager = $shareManager;
+		$this->session = $session;
 	}
 
 	/**
@@ -66,42 +76,23 @@ class PublicAuth extends \Sabre\DAV\Auth\Backend\AbstractBasic {
 	 * @throws \Sabre\DAV\Exception\NotAuthenticated
 	 */
 	protected function validateUserPass($username, $password) {
-		$linkItem = \OCP\Share::getShareByToken($username, false);
-		\OC_User::setIncognitoMode(true);
-		$this->share = $linkItem;
-		if (!$linkItem) {
+		try {
+			$share = $this->shareManager->getShareByToken($username);
+		} catch (ShareNotFound $e) {
 			return false;
 		}
 
-		if ((int)$linkItem['share_type'] === \OCP\Share::SHARE_TYPE_LINK &&
-			$this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes') !== 'yes') {
-			$this->share['permissions'] &= ~(\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE);
-		}
+		$this->share = $share;
 
-		// check if the share is password protected
-		if (isset($linkItem['share_with'])) {
-			if ($linkItem['share_type'] == \OCP\Share::SHARE_TYPE_LINK) {
-				// Check Password
-				$newHash = '';
-				if(\OC::$server->getHasher()->verify($password, $linkItem['share_with'], $newHash)) {
-					/**
-					 * FIXME: Migrate old hashes to new hash format
-					 * Due to the fact that there is no reasonable functionality to update the password
-					 * of an existing share no migration is yet performed there.
-					 * The only possibility is to update the existing share which will result in a new
-					 * share ID and is a major hack.
-					 *
-					 * In the future the migration should be performed once there is a proper method
-					 * to update the share's password. (for example `$share->updatePassword($password)`
-					 *
-					 * @link https://github.com/owncloud/core/issues/10671
-					 */
-					if(!empty($newHash)) {
+		\OC_User::setIncognitoMode(true);
 
-					}
+		// check if the share is password protected
+		if ($share->getPassword() !== null) {
+			if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
+				if ($this->shareManager->checkPassword($share, $password)) {
 					return true;
-				} else if (\OC::$server->getSession()->exists('public_link_authenticated')
-					&& \OC::$server->getSession()->get('public_link_authenticated') === $linkItem['id']) {
+				} else if ($this->session->exists('public_link_authenticated')
+					&& $this->session->get('public_link_authenticated') === $share->getId()) {
 					return true;
 				} else {
 					if (in_array('XMLHttpRequest', explode(',', $this->request->getHeader('X-Requested-With')))) {
@@ -112,7 +103,7 @@ class PublicAuth extends \Sabre\DAV\Auth\Backend\AbstractBasic {
 					}
 					return false;
 				}
-			} else if ($linkItem['share_type'] == \OCP\Share::SHARE_TYPE_REMOTE) {
+			} else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_REMOTE) {
 				return true;
 			} else {
 				return false;
@@ -123,7 +114,7 @@ class PublicAuth extends \Sabre\DAV\Auth\Backend\AbstractBasic {
 	}
 
 	/**
-	 * @return array
+	 * @return \OCP\Share\IShare
 	 */
 	public function getShare() {
 		return $this->share;
diff --git a/apps/dav/tests/unit/connector/publicauth.php b/apps/dav/tests/unit/connector/publicauth.php
new file mode 100644
index 00000000000..5f46651d372
--- /dev/null
+++ b/apps/dav/tests/unit/connector/publicauth.php
@@ -0,0 +1,170 @@
+<?php
+
+namespace OCA\DAV\Tests\Unit\Connector;
+
+use OCP\IRequest;
+use OCP\ISession;
+use OCP\Share\Exceptions\ShareNotFound;
+use OCP\Share\IManager;
+
+class PublicAuth extends \Test\TestCase {
+
+	/** @var ISession|\PHPUnit_Framework_MockObject_MockObject */
+	private $session;
+	/** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
+	private $request;
+	/** @var IManager|\PHPUnit_Framework_MockObject_MockObject */
+	private $shareManager;
+	/** @var \OCA\DAV\Connector\PublicAuth */
+	private $auth;
+
+	/** @var string */
+	private $oldUser;
+
+	protected function setUp() {
+		parent::setUp();
+
+		$this->session = $this->getMock('\OCP\ISession');
+		$this->request = $this->getMock('\OCP\IRequest');
+		$this->shareManager = $this->getMock('\OCP\Share\IManager');
+
+		$this->auth = new \OCA\DAV\Connector\PublicAuth(
+			$this->request,
+			$this->shareManager,
+			$this->session
+		);
+
+		// Store current user
+		$this->oldUser = \OC_User::getUser();
+	}
+
+	protected function tearDown() {
+		\OC_User::setIncognitoMode(false);
+
+		// Set old user
+		\OC_User::setUserId($this->oldUser);
+		\OC_Util::setupFS($this->oldUser);
+
+		parent::tearDown();
+	}
+
+	public function testNoShare() {
+		$this->shareManager->expects($this->once())
+			->method('getShareByToken')
+			->willThrowException(new ShareNotFound());
+
+		$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);
+
+		$this->assertFalse($result);
+	}
+
+	public function testShareNoPassword() {
+		$share = $this->getMock('OCP\Share\IShare');
+		$share->method('getPassword')->willReturn(null);
+
+		$this->shareManager->expects($this->once())
+			->method('getShareByToken')
+			->willReturn($share);
+
+		$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);
+
+		$this->assertTrue($result);
+	}
+
+	public function testSharePasswordFancyShareType() {
+		$share = $this->getMock('OCP\Share\IShare');
+		$share->method('getPassword')->willReturn('password');
+		$share->method('getShareType')->willReturn(42);
+
+		$this->shareManager->expects($this->once())
+			->method('getShareByToken')
+			->willReturn($share);
+
+		$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);
+
+		$this->assertFalse($result);
+	}
+
+
+	public function testSharePasswordRemote() {
+		$share = $this->getMock('OCP\Share\IShare');
+		$share->method('getPassword')->willReturn('password');
+		$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_REMOTE);
+
+		$this->shareManager->expects($this->once())
+			->method('getShareByToken')
+			->willReturn($share);
+
+		$result = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']);
+
+		$this->assertTrue($result);
+	}
+
+	public function testSharePasswordLinkValidPassword() {
+		$share = $this->getMock('OCP\Share\IShare');
+		$share->method('getPassword')->willReturn('password');
+		$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK);
+
+		$this->shareManager->expects($this->once())
+			->method('getShareByToken')
+			->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 testSharePasswordLinkValidSession() {
+		$share = $this->getMock('OCP\Share\IShare');
+		$share->method('getPassword')->willReturn('password');
+		$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK);
+		$share->method('getId')->willReturn('42');
+
+		$this->shareManager->expects($this->once())
+			->method('getShareByToken')
+			->willReturn($share);
+
+		$this->shareManager->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() {
+		$share = $this->getMock('OCP\Share\IShare');
+		$share->method('getPassword')->willReturn('password');
+		$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK);
+		$share->method('getId')->willReturn('42');
+
+		$this->shareManager->expects($this->once())
+			->method('getShareByToken')
+			->willReturn($share);
+
+		$this->shareManager->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/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index 6c665f7e133..be7257de36d 100644
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -976,7 +976,17 @@ class Manager implements IManager {
 	public function getShareByToken($token) {
 		$provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_LINK);
 
-		$share = $provider->getShareByToken($token);
+		try {
+			$share = $provider->getShareByToken($token);
+		} catch (ShareNotFound $e) {
+			//Ignore
+		}
+
+		// If it is not a link share try to fetch a federated share by token
+		if ($share === null) {
+			$provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_REMOTE);
+			$share = $provider->getShareByToken($token);
+		}
 
 		if ($share->getExpirationDate() !== null &&
 			$share->getExpirationDate() <= new \DateTime()) {
@@ -984,6 +994,14 @@ class Manager implements IManager {
 			throw new ShareNotFound();
 		}
 
+		/*
+		 * Reduce the permissions for link shares if public upload is not enabled
+		 */
+		if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK &&
+			!$this->shareApiLinkAllowPublicUpload()) {
+			$share->setPermissions($share->getPermissions() & ~(\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE));
+		}
+
 		return $share;
 	}
 
diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php
index 029c8cd8543..d8eb3e0a31e 100644
--- a/tests/lib/share20/managertest.php
+++ b/tests/lib/share20/managertest.php
@@ -2084,6 +2084,25 @@ class ManagerTest extends \Test\TestCase {
 		$this->assertSame($share, $res);
 	}
 
+	public function testGetShareByTokenPublicSharingDisabled() {
+		$share = $this->manager->newShare();
+		$share->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPermissions(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE);
+
+		$this->config->method('getAppValue')->will($this->returnValueMap([
+			['core', 'shareapi_allow_public_upload', 'yes', 'no'],
+		]));
+
+		$this->defaultProvider->expects($this->once())
+			->method('getShareByToken')
+			->willReturn('validToken')
+			->willReturn($share);
+
+		$res = $this->manager->getShareByToken('validToken');
+
+		$this->assertSame(\OCP\Constants::PERMISSION_READ, $res->getPermissions());
+	}
+
 	public function testCheckPasswordNoLinkShare() {
 		$share = $this->getMock('\OCP\Share\IShare');
 		$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER);