Merge pull request #671 from owncloud/stable8.2-tokens-are-too-loosev8.2.7RC1v8.2.6RC1v8.2.6

[Stable8.2] Tokens are too loose

6 files changed, 99 insertions, 8 deletions

diff --git a/.travis.yml b/.travis.yml
index 6c0b9344..9a28ca86 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -82,7 +82,7 @@ before_script:
 
 script:
   # Test lint
-  - php vendor/bin/parallel-lint --exclude travis --exclude vendor . vendor/composer vendor/symfony/yaml vendor/autoload.php
+  - php vendor/bin/parallel-lint --exclude vendor/composer/autoload_static.php --exclude travis --exclude vendor . vendor/composer vendor/symfony/yaml vendor/autoload.php
 
   # Acceptance tests, using Firefox, without code coverage
   - sh -c "if [ '$TRAVIS_PHP_VERSION' != 'hhvm' ] && [ '$TRAVIS_PHP_VERSION' != '7' ]; then php vendor/bin/codecept run acceptance --env firefox; fi;"
diff --git a/environment/environment.php b/environment/environment.php
index 69600d32..cf4b6daa 100644
--- a/environment/environment.php
+++ b/environment/environment.php
@@ -59,6 +59,10 @@ class Environment {
 	 */
 	private $sharedNodeId;
 	/**
+	 * @var File|Folder
+	 */
+	private $sharedNode;
+	/**
 	 * @var IRootFolder
 	 */
 	private $rootFolder;
@@ -125,6 +129,8 @@ class Environment {
 
 		// This is actually the node ID
 		$this->sharedNodeId = $linkItem['file_source'];
+		$this->sharedNode =
+			$this->getResourceFromFolderAndId($this->userFolder, $this->sharedNodeId);
 		$this->fromRootToFolder = $this->buildFromRootToFolder($this->sharedNodeId);
 
 		$this->folderName = $linkItem['file_target'];
@@ -144,6 +150,15 @@ class Environment {
 	}
 
 	/**
+	 * Returns true if the environment has been setup using a token
+	 *
+	 * @return bool
+	 */
+	public function isTokenBasedEnv() {
+		return !empty($this->sharedNodeId);
+	}
+
+	/**
 	 * Returns the Node based on a path starting from the virtual root
 	 *
 	 * @param string $subPath
@@ -196,12 +211,17 @@ class Environment {
 	 * @throws NotFoundEnvException
 	 */
 	public function getResourceFromId($resourceId) {
-		$resourcesArray = $this->userFolder->getById($resourceId);
-		if ($resourcesArray[0] === null) {
-			throw new NotFoundEnvException('Could not locate file linked to ID: ' . $resourceId);
+		if ($this->isTokenBasedEnv()) {
+			if ($this->sharedNode->getType() === 'dir') {
+				$resource = $this->getResourceFromFolderAndId($this->sharedNode, $resourceId);
+			} else {
+				$resource = $this->sharedNode;
+			}
+		} else {
+			$resource = $this->getResourceFromFolderAndId($this->userFolder, $resourceId);
 		}
 
-		return $resourcesArray[0];
+		return $resource;
 	}
 
 	/**
@@ -221,7 +241,7 @@ class Environment {
 	 */
 	public function getVirtualRootFolder() {
 		$rootFolder = $this->userFolder;
-		if (!empty($this->sharedNodeId)) {
+		if ($this->isTokenBasedEnv()) {
 			$node = $this->getSharedNode();
 			$nodeType = $node->getType();
 			if ($nodeType === 'dir') {
@@ -329,6 +349,25 @@ class Environment {
 	}
 
 	/**
+	 * Returns the resource found in a specific folder and identified by the given ID
+	 *
+	 * @param Folder $folder
+	 * @param int $resourceId
+	 *
+	 * @return Node
+	 * @throws NotFoundEnvException
+	 */
+	private function getResourceFromFolderAndId($folder, $resourceId) {
+		$resourcesArray = $folder->getById($resourceId);
+
+		if ($resourcesArray[0] === null) {
+			throw new NotFoundEnvException('Could not locate node linked to ID: ' . $resourceId);
+		}
+
+		return $resourcesArray[0];
+	}
+
+	/**
 	 * Returns the path from the shared folder to the root folder in the original
 	 * owner's filesystem: /userId/files/parent_folder/shared_folder
 	 *
diff --git a/tests/_support/Helper/Api.php b/tests/_support/Helper/Api.php
index 9e8cda69..96f01f14 100644
--- a/tests/_support/Helper/Api.php
+++ b/tests/_support/Helper/Api.php
@@ -55,6 +55,16 @@ class Api extends \Codeception\Module {
 	 * @return mixed
 	 * @throws \Codeception\Exception\ModuleException
 	 */
+	public function getPrivateFile() {
+		$privateFile['file'] = $this->getModule('\Helper\DataSetup')->privateFile;
+
+		return $privateFile;
+	}
+
+	/**
+	 * @return mixed
+	 * @throws \Codeception\Exception\ModuleException
+	 */
 	public function getSharedFolder() {
 		$sharedFolder['folder'] = $this->getModule('\Helper\DataSetup')->sharedFolder;
 		$sharedFolder['token'] = $this->getModule('\Helper\DataSetup')->sharedFolderToken;
diff --git a/tests/_support/Helper/DataSetup.php b/tests/_support/Helper/DataSetup.php
index a8800b1b..b1e10cf9 100644
--- a/tests/_support/Helper/DataSetup.php
+++ b/tests/_support/Helper/DataSetup.php
@@ -101,6 +101,10 @@ class DataSetup extends \Codeception\Module {
 	public $sharedFileName = 'testimage-wide.png';
 	/** @var string */
 	public $sharedFileToken;
+	/** @var File */
+	public $privateFile;
+	/** @var string */
+	public $privateFileName = 'font.ttf';
 
 	/** @var IAppContainer */
 	private $container;
@@ -346,6 +350,9 @@ class DataSetup extends \Codeception\Module {
 				if ($value === $this->sharedFileName) {
 					$this->sharedFile = $file;
 				}
+				if ($value === $this->privateFileName) {
+					$this->privateFile = $file;
+				}
 			}
 		}
 	}
diff --git a/tests/_support/Step/Api/TokenUser.php b/tests/_support/Step/Api/TokenUser.php
index 8be4eaf8..7e638233 100644
--- a/tests/_support/Step/Api/TokenUser.php
+++ b/tests/_support/Step/Api/TokenUser.php
@@ -32,6 +32,20 @@ class TokenUser extends \ApiTester {
 	}
 
 	/**
+	 * Retrieves the shared file and its token
+	 */
+	public function getPrivateFileInformation() {
+		$I = $this;
+		$privateFile = $I->getPrivateFile();
+
+		$fileMetaData = [
+			'fileId' => $privateFile['file']->getId(),
+		];
+
+		return $fileMetaData;
+	}
+
+	/**
 	 * Retrieves the shared folder and its token
 	 */
 	public function getSharedFolderInformation() {
diff --git a/tests/api/DownloadWithTokenCest.php b/tests/api/DownloadWithTokenCest.php
index 610bd1f9..7d87b627 100644
--- a/tests/api/DownloadWithTokenCest.php
+++ b/tests/api/DownloadWithTokenCest.php
@@ -77,9 +77,9 @@ class DownloadWithTokenCest {
 	 *
 	 * @param \Step\Api\TokenUser $I
 	 */
-	public function TryTodownloadFileWithoutAToken(\Step\Api\TokenUser $I) {
+	public function TryToDownloadFileWithoutAToken(\Step\Api\TokenUser $I) {
 		$I->am('a thief');
-		$I->wantTo('steal all the files I can get my hands on');
+		$I->wantTo('steal all the files I can get my hands on without a token');
 
 		$fileMetaData = $I->getSharedFileInformation();
 		$params = [
@@ -90,4 +90,25 @@ class DownloadWithTokenCest {
 		$I->seeResponseCodeIs(404);
 		$I->seeHttpHeader('Content-type', 'text/html; charset=UTF-8');
 	}
+
+	/**
+	 * This is to make sure we get the file we're supposed to even in case the wrong fileId is used
+	 * with a token
+	 *
+	 * @param \Step\Api\TokenUser $I
+	 */
+	public function TryToDownloadWrongFileUsingToken(\Step\Api\TokenUser $I) {
+		$I->am('a thief');
+		$I->wantTo('steal files I\'m not allowed to access using this token');
+
+		$fileMetaData = $I->getSharedFileInformation();
+		$privateFileMetaData = $I->getSharedFileInformation();
+		$params = [
+			'fileId' => $privateFileMetaData['fileId'],
+			'token'  => $fileMetaData['token']
+		];
+		$I->haveHttpHeader('Accept', $this->browserHeader);
+		$I->sendGET(GalleryApp::$URL . '/files.public/download/{fileId}', $params);
+		$I->downloadAFile($fileMetaData);
+	}
 }