Merge pull request #13182 from owncloud/download_versions

make versions of shared files downloadable

10 files changed, 396 insertions, 146 deletions

diff --git a/apps/files_encryption/lib/crypt.php b/apps/files_encryption/lib/crypt.php
index 38993ba65b0..cdc2df4cdd8 100644
--- a/apps/files_encryption/lib/crypt.php
+++ b/apps/files_encryption/lib/crypt.php
@@ -148,9 +148,6 @@ class Crypt {
 		// Fetch encryption metadata from end of file

 		$meta = substr($noPadding, -22);

 

-		// Fetch IV from end of file

-		$iv = substr($meta, -16);

-

 		// Fetch identifier from start of metadata

 		$identifier = substr($meta, 0, 6);

 

diff --git a/apps/files_encryption/lib/helper.php b/apps/files_encryption/lib/helper.php
index b9d45f67363..553c52e72fe 100644
--- a/apps/files_encryption/lib/helper.php
+++ b/apps/files_encryption/lib/helper.php
@@ -250,15 +250,14 @@ class Helper {
 	 * @return string e.g. turns '/admin/files/test.txt' into 'test.txt'
 	 */
 	public static function stripUserFilesPath($path) {
-		$trimmed = ltrim($path, '/');
-		$split = explode('/', $trimmed);
+		$split = self::splitPath($path);
 
 		// it is not a file relative to data/user/files
-		if (count($split) < 3 || $split[1] !== 'files') {
+		if (count($split) < 4 || $split[2] !== 'files') {
 			return false;
 		}
 
-		$sliced = array_slice($split, 2);
+		$sliced = array_slice($split, 3);
 		$relPath = implode('/', $sliced);
 
 		return $relPath;
@@ -267,7 +266,7 @@ class Helper {
 	/**
 	 * try to get the user from the path if no user is logged in
 	 * @param string $path
-	 * @return mixed user or false if we couldn't determine a user
+	 * @return string user
 	 */
 	public static function getUser($path) {
 
@@ -281,65 +280,85 @@ class Helper {
 
 		// if no user is logged in we try to access a publicly shared files.
 		// In this case we need to try to get the user from the path
+		return self::getUserFromPath($path);
+	}
 
-		$trimmed = ltrim($path, '/');
-		$split = explode('/', $trimmed);
+	/**
+	 * extract user from path
+	 *
+	 * @param string $path
+	 * @return string user id
+	 * @throws Exception\EncryptionException
+	 */
+	public static function getUserFromPath($path) {
+		$split = self::splitPath($path);
 
-		// it is not a file relative to data/user/files
-		if (count($split) < 2 || ($split[1] !== 'files' && $split[1] !== 'cache')) {
-			return false;
-		}
+		if (count($split) > 3 && (
+			$split[2] === 'files' || $split[2] === 'files_versions' || $split[2] === 'cache')) {
 
-		$user = $split[0];
+			$user = $split[1];
 
-		if (\OCP\User::userExists($user)) {
-			return $user;
+			if (\OCP\User::userExists($user)) {
+				return $user;
+			}
 		}
 
-		return false;
+		throw new Exception\EncryptionException('Could not determine user', Exception\EncryptionException::GENERIC);
 	}
 
 	/**
 	 * get path to the corresponding file in data/user/files if path points
-	 *        to a version or to a file in cache
-	 * @param string $path path to a version or a file in the trash
+	 * to a file in cache
+	 *
+	 * @param string $path path to a file in cache
 	 * @return string path to corresponding file relative to data/user/files
+	 * @throws Exception\EncryptionException
 	 */
-	public static function getPathToRealFile($path) {
-		$trimmed = ltrim($path, '/');
-		$split = explode('/', $trimmed);
-		$result = false;
-
-		if (count($split) >= 3 && ($split[1] === "files_versions" || $split[1] === 'cache')) {
-			$sliced = array_slice($split, 2);
-			$result = implode('/', $sliced);
-			if ($split[1] === "files_versions") {
-				// we skip user/files
-				$sliced = array_slice($split, 2);
-				$relPath = implode('/', $sliced);
-				//remove the last .v
-				$result = substr($relPath, 0, strrpos($relPath, '.v'));
-			}
-			if ($split[1] === "cache") {
-				// we skip /user/cache/transactionId
-				$sliced = array_slice($split, 3);
-				$result = implode('/', $sliced);
-				//prepare the folders
-				self::mkdirr($path, new \OC\Files\View('/'));
-			}
+	public static function getPathFromCachedFile($path) {
+		$split = self::splitPath($path);
+
+		if (count($split) < 5) {
+			throw new Exception\EncryptionException('no valid cache file path', Exception\EncryptionException::GENERIC);
 		}
 
-		return $result;
+		// we skip /user/cache/transactionId
+		$sliced = array_slice($split, 4);
+
+		return implode('/', $sliced);
+	}
+
+
+	/**
+	 * get path to the corresponding file in data/user/files for a version
+	 *
+	 * @param string $path path to a version
+	 * @return string path to corresponding file relative to data/user/files
+	 * @throws Exception\EncryptionException
+	 */
+	public static function getPathFromVersion($path) {
+		$split = self::splitPath($path);
+
+		if (count($split) < 4) {
+			throw new Exception\EncryptionException('no valid path to a version', Exception\EncryptionException::GENERIC);
+		}
+
+		// we skip user/files_versions
+		$sliced = array_slice($split, 3);
+		$relPath = implode('/', $sliced);
+		//remove the last .v
+		$realPath = substr($relPath, 0, strrpos($relPath, '.v'));
+
+		return $realPath;
 	}
 
 	/**
 	 * create directory recursively
+	 *
 	 * @param string $path
 	 * @param \OC\Files\View $view
 	 */
 	public static function mkdirr($path, \OC\Files\View $view) {
-		$dirname = \OC\Files\Filesystem::normalizePath(dirname($path));
-		$dirParts = explode('/', $dirname);
+		$dirParts = self::splitPath(dirname($path));
 		$dir = "";
 		foreach ($dirParts as $part) {
 			$dir = $dir . '/' . $part;
@@ -454,6 +473,32 @@ class Helper {
 	}
 
 	/**
+	 * detect file type, encryption can read/write regular files, versions
+	 * and cached files
+	 *
+	 * @param string $path
+	 * @return int
+	 * @throws Exception\EncryptionException
+	 */
+	public static function detectFileType($path) {
+	    $parts = self::splitPath($path);
+
+		if (count($parts) > 2) {
+			switch ($parts[2]) {
+				case 'files':
+					return Util::FILE_TYPE_FILE;
+				case 'files_versions':
+					return Util::FILE_TYPE_VERSION;
+				case 'cache':
+					return Util::FILE_TYPE_CACHE;
+			}
+		}
+
+		// thow exception if we couldn't detect a valid file type
+		throw new Exception\EncryptionException('Could not detect file type', Exception\EncryptionException::GENERIC);
+	}
+
+	/**
 	 * read the cipher used for encryption from the config.php
 	 *
 	 * @return string
@@ -472,5 +517,11 @@ class Helper {
 
 		return $cipher;
 	}
+
+	public static function splitPath($path) {
+		$normalized = \OC\Files\Filesystem::normalizePath($path);
+		return explode('/', $normalized);
+	}
+
 }
 
diff --git a/apps/files_encryption/lib/proxy.php b/apps/files_encryption/lib/proxy.php
index ba78c81aa35..3ee7d83f04c 100644
--- a/apps/files_encryption/lib/proxy.php
+++ b/apps/files_encryption/lib/proxy.php
@@ -56,10 +56,12 @@ class Proxy extends \OC_FileProxy {
 
 		$path = \OC\Files\Filesystem::normalizePath($path);
 
+		$parts = explode('/', $path);
+
 		// we only encrypt/decrypt files in the files and files_versions folder
 		if(
 			strpos($path, '/' . $uid . '/files/') !== 0 &&
-			strpos($path, '/' . $uid . '/files_versions/') !== 0) {
+			!($parts[2] === 'files_versions' && \OCP\User::userExists($parts[1]))) {
 
 			return true;
 		}
diff --git a/apps/files_encryption/lib/stream.php b/apps/files_encryption/lib/stream.php
index 17da4eb1cdc..1bc0d54e1bc 100644
--- a/apps/files_encryption/lib/stream.php
+++ b/apps/files_encryption/lib/stream.php
@@ -75,6 +75,8 @@ class Stream {
 	private $headerWritten = false;
 	private $containHeader = false; // the file contain a header
 	private $cipher; // cipher used for encryption/decryption
+	/** @var \OCA\Files_Encryption\Util */
+	private $util;
 
 	/**
 	 * @var \OC\Files\View
@@ -103,9 +105,7 @@ class Stream {
 		// assume that the file already exist before we decide it finally in getKey()
 		$this->newFile = false;
 
-		if (!isset($this->rootView)) {
-			$this->rootView = new \OC\Files\View('/');
-		}
+		$this->rootView = new \OC\Files\View('/');
 
 		$this->session = new Session($this->rootView);
 
@@ -116,7 +116,8 @@ class Stream {
 		}
 
 		$normalizedPath = \OC\Files\Filesystem::normalizePath(str_replace('crypt://', '', $path));
-		if ($originalFile = Helper::getPathFromTmpFile($normalizedPath)) {
+		$originalFile = Helper::getPathFromTmpFile($normalizedPath);
+		if ($originalFile) {
 			$this->rawPath = $originalFile;
 			$this->isLocalTmpFile = true;
 			$this->localTmpFile = $normalizedPath;
@@ -124,24 +125,31 @@ class Stream {
 			$this->rawPath = $normalizedPath;
 		}
 
-		$this->userId = Helper::getUser($this->rawPath);
-
-		$util = new Util($this->rootView, $this->userId);
+		$this->util = new Util($this->rootView, Helper::getUser($this->rawPath));
 
 		// get the key ID which we want to use, can be the users key or the
 		// public share key
-		$this->keyId = $util->getKeyId();
+		$this->keyId = $this->util->getKeyId();
 
-		// Strip identifier text from path, this gives us the path relative to data/<user>/files
-		$this->relPath = Helper::stripUserFilesPath($this->rawPath);
-		// if raw path doesn't point to a real file, check if it is a version or a file in the trash bin
-		if ($this->relPath === false) {
-			$this->relPath = Helper::getPathToRealFile($this->rawPath);
-		}
+		$fileType = Helper::detectFileType($this->rawPath);
 
-		if($this->relPath === false) {
-			\OCP\Util::writeLog('Encryption library', 'failed to open file "' . $this->rawPath . '" expecting a path to "files", "files_versions" or "cache"', \OCP\Util::ERROR);
-			return false;
+		switch ($fileType) {
+			case Util::FILE_TYPE_FILE:
+				$this->relPath = Helper::stripUserFilesPath($this->rawPath);
+				$this->userId = \OC::$server->getUserSession()->getUser()->getUID();
+				break;
+			case Util::FILE_TYPE_VERSION:
+				$this->relPath = Helper::getPathFromVersion($this->rawPath);
+				$this->userId = Helper::getUserFromPath($this->rawPath);
+				break;
+			case Util::FILE_TYPE_CACHE:
+				$this->relPath = Helper::getPathFromCachedFile($this->rawPath);
+				Helper::mkdirr($this->rawPath, new \OC\Files\View('/'));
+				$this->userId = \OC::$server->getUserSession()->getUser()->getUID();
+				break;
+			default:
+				\OCP\Util::writeLog('Encryption library', 'failed to open file "' . $this->rawPath . '" expecting a path to "files", "files_versions" or "cache"', \OCP\Util::ERROR);
+				return false;
 		}
 
 		// Disable fileproxies so we can get the file size and open the source file without recursive encryption
@@ -154,22 +162,12 @@ class Stream {
 			or $mode === 'wb'
 			or $mode === 'wb+'
 		) {
-
 			// We're writing a new file so start write counter with 0 bytes
 			$this->size = 0;
 			$this->unencryptedSize = 0;
-
 		} else {
-
-			if($this->privateKey === false) {
-				// if private key is not valid redirect user to a error page
-				Helper::redirectToErrorPage($this->session);
-			}
-
 			$this->size = $this->rootView->filesize($this->rawPath);
-
 			$this->readHeader();
-
 		}
 
 		if ($this->isLocalTmpFile) {
@@ -328,9 +326,10 @@ class Stream {
 
 		}
 
+		$util = new Util($this->rootView, $this->userId);
+
 		// Fetch and decrypt keyfile
 		// Fetch existing keyfile
-		$util = new Util($this->rootView, $this->userId);
 		$this->encKeyfile = Keymanager::getFileKey($this->rootView, $util, $this->relPath);
 
 		// If a keyfile already exists
@@ -614,11 +613,9 @@ class Stream {
 				// Check if OC sharing api is enabled
 				$sharingEnabled = \OCP\Share::isEnabled();
 
-				$util = new Util($this->rootView, $this->userId);
-
 				// Get all users sharing the file includes current user
-				$uniqueUserIds = $util->getSharingUsersArray($sharingEnabled, $this->relPath);
-				$checkedUserIds = $util->filterShareReadyUsers($uniqueUserIds);
+				$uniqueUserIds = $this->util->getSharingUsersArray($sharingEnabled, $this->relPath);
+				$checkedUserIds = $this->util->filterShareReadyUsers($uniqueUserIds);
 
 				// Fetch public keys for all sharing users
 				$publicKeys = Keymanager::getPublicKeys($this->rootView, $checkedUserIds['ready']);
@@ -627,10 +624,10 @@ class Stream {
 				$this->encKeyfiles = Crypt::multiKeyEncrypt($this->plainKey, $publicKeys);
 
 				// Save the new encrypted file key
-				Keymanager::setFileKey($this->rootView, $util, $this->relPath, $this->encKeyfiles['data']);
+				Keymanager::setFileKey($this->rootView, $this->util, $this->relPath, $this->encKeyfiles['data']);
 
 				// Save the sharekeys
-				Keymanager::setShareKeys($this->rootView, $util, $this->relPath, $this->encKeyfiles['keys']);
+				Keymanager::setShareKeys($this->rootView, $this->util, $this->relPath, $this->encKeyfiles['keys']);
 
 				// Re-enable proxy - our work is done
 				\OC_FileProxy::$enabled = $proxyStatus;
diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php
index 4aaf7aa2571..1b140822724 100644
--- a/apps/files_encryption/lib/util.php
+++ b/apps/files_encryption/lib/util.php
@@ -39,6 +39,10 @@ class Util {
 	const MIGRATION_IN_PROGRESS = -1; // migration is running
 	const MIGRATION_OPEN = 0;         // user still needs to be migrated
 
+	const FILE_TYPE_FILE = 0;
+	const FILE_TYPE_VERSION = 1;
+	const FILE_TYPE_CACHE = 2;
+
 	private $view; // OC\Files\View object for filesystem operations
 	private $userId; // ID of the user we use to encrypt/decrypt files
 	private $keyId; // ID of the key we want to manipulate
diff --git a/apps/files_encryption/tests/helper.php b/apps/files_encryption/tests/helper.php
index bf86860125a..62fdb80d671 100644
--- a/apps/files_encryption/tests/helper.php
+++ b/apps/files_encryption/tests/helper.php
@@ -8,10 +8,13 @@
 
 namespace OCA\Files_Encryption\Tests;
 
+use OCA\Files_Encryption;
+use OCA\Files_Encryption\Helper;
+
 /**
  * Class Helper
  */
-class Helper extends TestCase {
+class TestHelper extends TestCase {
 
 	const TEST_ENCRYPTION_HELPER_USER1 = "test-helper-user1";
 	const TEST_ENCRYPTION_HELPER_USER2 = "test-helper-user2";
@@ -30,11 +33,11 @@ class Helper extends TestCase {
 
 	public static function setupHooks() {
 		// Filesystem related hooks
-		\OCA\Files_Encryption\Helper::registerFilesystemHooks();
+		Helper::registerFilesystemHooks();
 
 		// clear and register hooks
 		\OC_FileProxy::clearProxies();
-		\OC_FileProxy::register(new \OCA\Files_Encryption\Proxy());
+		\OC_FileProxy::register(new Files_Encryption\Proxy());
 	}
 
 	public static function tearDownAfterClass() {
@@ -49,13 +52,13 @@ class Helper extends TestCase {
 		$partFilename = 'testfile.txt.part';
 		$filename = 'testfile.txt';
 
-		$this->assertTrue(\OCA\Files_Encryption\Helper::isPartialFilePath($partFilename));
+		$this->assertTrue(Helper::isPartialFilePath($partFilename));
 
-		$this->assertEquals('testfile.txt', \OCA\Files_Encryption\Helper::stripPartialFileExtension($partFilename));
+		$this->assertEquals('testfile.txt', Helper::stripPartialFileExtension($partFilename));
 
-		$this->assertFalse(\OCA\Files_Encryption\Helper::isPartialFilePath($filename));
+		$this->assertFalse(Helper::isPartialFilePath($filename));
 
-		$this->assertEquals('testfile.txt', \OCA\Files_Encryption\Helper::stripPartialFileExtension($filename));
+		$this->assertEquals('testfile.txt', Helper::stripPartialFileExtension($filename));
 	}
 
 
@@ -67,26 +70,79 @@ class Helper extends TestCase {
 		$partFilename = 'testfile.txt.ocTransferId643653835.part';
 		$filename = 'testfile.txt';
 
-		$this->assertTrue(\OCA\Files_Encryption\Helper::isPartialFilePath($partFilename));
+		$this->assertTrue(Helper::isPartialFilePath($partFilename));
+
+		$this->assertEquals('testfile.txt', Helper::stripPartialFileExtension($partFilename));
 
-		$this->assertEquals('testfile.txt', \OCA\Files_Encryption\Helper::stripPartialFileExtension($partFilename));
+		$this->assertFalse(Helper::isPartialFilePath($filename));
 
-		$this->assertFalse(\OCA\Files_Encryption\Helper::isPartialFilePath($filename));
+		$this->assertEquals('testfile.txt', Helper::stripPartialFileExtension($filename));
+	}
 
-		$this->assertEquals('testfile.txt', \OCA\Files_Encryption\Helper::stripPartialFileExtension($filename));
+	/**
+	 * @dataProvider dataVersionsPathPositive
+	 */
+	function testGetPathFromVersionPositive($path, $expected) {
+		$result = Helper::getPathFromVersion($path);
+		$this->assertSame($expected, $result);
 	}
 
-	function testGetPathToRealFile() {
+	function dataVersionsPathPositive() {
+		return array(
+			array('/user/files_versions/foo/bar/test.txt.v456756835', 'foo/bar/test.txt'),
+			array('user/files_versions/foo/bar/test.txt.v456756835', 'foo/bar/test.txt'),
+			array('user/files_versions//foo/bar/test.txt.v456756835', 'foo/bar/test.txt'),
+			array('user/files_versions/test.txt.v456756835', 'test.txt'),
+		);
+	}
+
+	/**
+	 * @dataProvider dataVersionsPathNegative
+	 * @expectedException \OCA\Files_Encryption\Exception\EncryptionException
+	 */
+	function testGetPathFromVersionNegative($path) {
+		Helper::getPathFromVersion($path);
+	}
 
-		// the relative path to /user/files/ that's what we want to get from getPathToRealFile()
-		$relativePath = "foo/bar/test.txt";
+	function dataVersionsPathNegative() {
+		return array(
+			array('/user/files_versions/'),
+			array('/user/files_versions'),
+		);
+	}
 
-		// test paths
-		$versionPath = "/user/files_versions/foo/bar/test.txt.v456756835";
-		$cachePath = "/user/cache/transferid636483/foo/bar/test.txt";
+	/**
+	 * @dataProvider dataPathsCachedFilePositive
+	 */
+	function testGetPathFromCachedFilePositive($path, $expected) {
+		$result = Helper::getPathFromCachedFile($path);
+		$this->assertEquals($expected, $result);
+	}
 
-		$this->assertEquals($relativePath, \OCA\Files_Encryption\Helper::getPathToRealFile($versionPath));
-		$this->assertEquals($relativePath, \OCA\Files_Encryption\Helper::getPathToRealFile($cachePath));
+	function dataPathsCachedFilePositive() {
+		return array(
+			array('/user/cache/transferid636483/foo/bar/test.txt', 'foo/bar/test.txt'),
+			array('/user/cache/transferid636483//test.txt', 'test.txt'),
+			array('user/cache/transferid636483//test.txt', 'test.txt'),
+		);
+	}
+
+
+	/**
+	 * @dataProvider dataPathsCachedFileNegative
+	 * @expectedException \OCA\Files_Encryption\Exception\EncryptionException
+	 */
+	function testGetPathFromCachedFileNegative($path) {
+		Helper::getPathFromCachedFile($path);
+	}
+
+	function dataPathsCachedFileNegative() {
+		return array(
+			array('/user/cache/transferid636483/'),
+			array('/user/cache/transferid636483'),
+			array('/user/cache/transferid636483//'),
+			array('/user/cache'),
+		);
 	}
 
 	function testGetUser() {
@@ -100,21 +156,167 @@ class Helper extends TestCase {
 		self::loginHelper(self::TEST_ENCRYPTION_HELPER_USER1);
 
 		// if we are logged-in every path should return the currently logged-in user
-		$this->assertEquals(self::TEST_ENCRYPTION_HELPER_USER1, \OCA\Files_Encryption\Helper::getUser($path3));
+		$this->assertEquals(self::TEST_ENCRYPTION_HELPER_USER1, Helper::getUser($path1));
+		$this->assertEquals(self::TEST_ENCRYPTION_HELPER_USER1, Helper::getUser($path2));
+		$this->assertEquals(self::TEST_ENCRYPTION_HELPER_USER1, Helper::getUser($path3));
+		$this->assertEquals(self::TEST_ENCRYPTION_HELPER_USER1, Helper::getUser($path4));
 
 		// now log out
 		self::logoutHelper();
 
 		// now we should only get the user from /user/files and user/cache paths
-		$this->assertEquals(self::TEST_ENCRYPTION_HELPER_USER1, \OCA\Files_Encryption\Helper::getUser($path1));
-		$this->assertEquals(self::TEST_ENCRYPTION_HELPER_USER1, \OCA\Files_Encryption\Helper::getUser($path2));
-
-		$this->assertFalse(\OCA\Files_Encryption\Helper::getUser($path3));
-		$this->assertFalse(\OCA\Files_Encryption\Helper::getUser($path4));
+		$this->assertEquals(self::TEST_ENCRYPTION_HELPER_USER1, Helper::getUser($path1));
+		$this->assertEquals(self::TEST_ENCRYPTION_HELPER_USER1, Helper::getUser($path2));
+
+		try {
+			$this->assertFalse(Helper::getUser($path3));
+			$this->assertFalse(true, '"OCA\Files_Encryption\Exception\EncryptionException: Could not determine user expected"');
+		} catch (Files_Encryption\Exception\EncryptionException $e) {
+			$this->assertSame('Could not determine user', $e->getMessage());
+		}
+		try {
+			$this->assertFalse(Helper::getUser($path4));
+			$this->assertFalse(true, '"OCA\Files_Encryption\Exception\EncryptionException: Could not determine user expected"');
+		} catch (Files_Encryption\Exception\EncryptionException $e) {
+			$this->assertSame('Could not determine user', $e->getMessage());
+		}
 
 		// Log-in again
 		self::loginHelper(self::TEST_ENCRYPTION_HELPER_USER1);
 		self::cleanUpUsers();
 	}
 
+	/**
+	 * @dataProvider dataStripUserFilesPath
+	 */
+	function testStripUserFilesPath($path, $expected) {
+		$result = Helper::stripUserFilesPath($path);
+		$this->assertSame($expected, $result);
+	}
+
+	function dataStripUserFilesPath() {
+		return array(
+			array('/user/files/foo.txt', 'foo.txt'),
+			array('//user/files/foo.txt', 'foo.txt'),
+			array('user//files/foo/bar.txt', 'foo/bar.txt'),
+			array('user//files/', false),
+			array('/user', false),
+			array('', false),
+		);
+	}
+
+	/**
+	 * @dataProvider dataStripUserFilesPathPositive
+	 */
+	function testGetUserFromPathPositive($path, $expected) {
+		self::setUpUsers();
+		$result = Helper::getUserFromPath($path);
+		$this->assertSame($expected, $result);
+		self::cleanUpUsers();
+	}
+
+	function dataStripUserFilesPathPositive() {
+		return array(
+			array('/' . self::TEST_ENCRYPTION_HELPER_USER1 . '/files/foo.txt', self::TEST_ENCRYPTION_HELPER_USER1),
+			array('//' . self::TEST_ENCRYPTION_HELPER_USER2 . '/files_versions/foo.txt', self::TEST_ENCRYPTION_HELPER_USER2),
+			array(self::TEST_ENCRYPTION_HELPER_USER1 . '//cache/foo/bar.txt', self::TEST_ENCRYPTION_HELPER_USER1),
+		);
+	}
+
+	/**
+	 * @dataProvider dataStripUserFilesPathNegative
+	 * @expectedException \OCA\Files_Encryption\Exception\EncryptionException
+	 */
+	function testGetUserFromPathNegative($path) {
+		Helper::getUserFromPath($path);
+	}
+
+	function dataStripUserFilesPathNegative() {
+		return array(
+			array('/unknown_user/files/foo.txt'),
+			array('/' . self::TEST_ENCRYPTION_HELPER_USER2 . '/unknown_folder/foo.txt'),
+			array('/' . self::TEST_ENCRYPTION_HELPER_USER1),
+			array(''),
+		);
+	}
+
+	/**
+	 * @dataProvider dataPaths
+	 */
+	function testMkdirr($path, $expected) {
+		self::setUpUsers();
+		Helper::mkdirr($path, new \OC\Files\View('/' . self::TEST_ENCRYPTION_HELPER_USER1 . '/files'));
+		// ignore the filename because we only check for the directories
+		$dirParts = array_slice($expected, 0, -1);
+		$expectedPath = implode('/', $dirParts);
+		$this->assertTrue(\OC\Files\Filesystem::is_dir($expectedPath));
+
+		// cleanup
+		\OC\Files\Filesystem::unlink('/' . $expected[0]);
+		self::cleanUpUsers();
+	}
+
+	/**
+	 * @dataProvider dataDetectFileTypePositive
+	 */
+	function testDetectFileTypePositive($path, $expected) {
+		$result = Helper::detectFileType($path);
+		$this->assertSame($expected, $result);
+	}
+
+	function dataDetectFileTypePositive() {
+		return array(
+			array(self::TEST_ENCRYPTION_HELPER_USER1 . '/files', Files_Encryption\Util::FILE_TYPE_FILE),
+			array(self::TEST_ENCRYPTION_HELPER_USER1 . '/files/foo/bar', Files_Encryption\Util::FILE_TYPE_FILE),
+			array('/' . self::TEST_ENCRYPTION_HELPER_USER1 . '/files/foo/bar', Files_Encryption\Util::FILE_TYPE_FILE),
+			array(self::TEST_ENCRYPTION_HELPER_USER1 . '/files_versions', Files_Encryption\Util::FILE_TYPE_VERSION),
+			array('/' . self::TEST_ENCRYPTION_HELPER_USER1 . '//files_versions/foo/bar', Files_Encryption\Util::FILE_TYPE_VERSION),
+			array('/' . self::TEST_ENCRYPTION_HELPER_USER1 . '//cache/foo/bar', Files_Encryption\Util::FILE_TYPE_CACHE),
+		);
+	}
+
+	/**
+	 * @dataProvider dataDetectFileTypeNegative
+	 * @expectedException \OCA\Files_Encryption\Exception\EncryptionException
+	 */
+	function testDetectFileTypeNegative($path) {
+		Helper::detectFileType($path);
+	}
+
+	function dataDetectFileTypeNegative() {
+		return array(
+			array('/files'),
+			array('/' . self::TEST_ENCRYPTION_HELPER_USER1 . '/unsuported_dir/foo/bar'),
+		);
+	}
+
+	/**
+	 * @dataProvider dataPaths
+	 */
+	function testSplitPath($path, $expected) {
+		$result = Helper::splitPath($path);
+		$this->compareArray($result, $expected);
+	}
+
+	function dataPaths() {
+		return array(
+			array('foo/bar/test.txt', array('', 'foo', 'bar', 'test.txt')),
+			array('/foo/bar/test.txt', array('', 'foo', 'bar', 'test.txt')),
+			array('/foo/bar//test.txt', array('', 'foo', 'bar', 'test.txt')),
+			array('//foo/bar/test.txt', array('', 'foo', 'bar', 'test.txt')),
+			array('foo', array('', 'foo')),
+			array('/foo', array('', 'foo')),
+			array('//foo', array('', 'foo')),
+		);
+	}
+
+	function compareArray($result, $expected) {
+		$this->assertSame(count($expected), count($result));
+
+		foreach ($expected as $key => $value) {
+			$this->assertArrayHasKey($key, $result);
+			$this->assertSame($value, $result[$key]);
+		}
+	}
+
 }
diff --git a/apps/files_encryption/tests/share.php b/apps/files_encryption/tests/share.php
index b6f5a1ffd20..8ecdbabed39 100755
--- a/apps/files_encryption/tests/share.php
+++ b/apps/files_encryption/tests/share.php
@@ -279,6 +279,40 @@ class Share extends TestCase {
 		}
 	}
 
+	function testDownloadVersions() {
+		// login as admin
+		self::loginHelper(self::TEST_ENCRYPTION_SHARE_USER1);
+
+		$rootView = new \OC\Files\View();
+
+		// save file twice to create a new version
+		\OC\Files\Filesystem::file_put_contents($this->filename, "revision1");
+		\OCA\Files_Versions\Storage::store($this->filename);
+		\OC\Files\Filesystem::file_put_contents($this->filename, "revision2");
+
+		// check if the owner can retrieve the correct version
+		$versions = \OCA\Files_Versions\Storage::getVersions(self::TEST_ENCRYPTION_SHARE_USER1, $this->filename);
+		$this->assertSame(1, count($versions));
+		$version = reset($versions);
+		$versionUser1 = $rootView->file_get_contents('/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files_versions/' . $this->filename . '.v' . $version['version']);
+		$this->assertSame('revision1', $versionUser1);
+
+		// share the file
+		$fileInfo = \OC\Files\Filesystem::getFileInfo($this->filename);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
+		$this->assertTrue(\OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_ENCRYPTION_SHARE_USER2, \OCP\Constants::PERMISSION_ALL));
+
+		// try to download the version as user2
+		self::loginHelper(self::TEST_ENCRYPTION_SHARE_USER2);
+		$versionUser2 = $rootView->file_get_contents('/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files_versions/' . $this->filename . '.v' . $version['version']);
+		$this->assertSame('revision1', $versionUser2);
+
+		//cleanup
+		self::loginHelper(self::TEST_ENCRYPTION_SHARE_USER1);
+		\OCP\Share::unshare('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_ENCRYPTION_SHARE_USER2);
+		\OC\Files\Filesystem::unlink($this->filename);
+	}
+
 	/**
 	 * @medium
 	 * @param bool $withTeardown
diff --git a/apps/files_encryption/tests/testcase.php b/apps/files_encryption/tests/testcase.php
index c2e5f4de8c1..35517c29d69 100644
--- a/apps/files_encryption/tests/testcase.php
+++ b/apps/files_encryption/tests/testcase.php
@@ -34,7 +34,7 @@ abstract class TestCase extends \Test\TestCase {
 		\OC_Util::tearDownFS();
 		\OC_User::setUserId('');
 		\OC\Files\Filesystem::tearDown();
-		\OC_User::setUserId($user);
+		\OC::$server->getUserSession()->setUser(new \OC\User\User($user, new \OC_User_Database()));
 		\OC_Util::setupFS($user);
 
 		if ($loadEncryption) {
diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php
index c75f406cb61..4e0b4f2d0de 100755
--- a/apps/files_encryption/tests/util.php
+++ b/apps/files_encryption/tests/util.php
@@ -575,43 +575,6 @@ class Util extends TestCase {
 	}
 
 	/**
-	 * @param string $user
-	 * @param bool $create
-	 * @param bool $password
-	 */
-	public static function loginHelper($user, $create = false, $password = false, $loadEncryption = true) {
-		if ($create) {
-			try {
-				\OC_User::createUser($user, $user);
-			} catch(\Exception $e) { // catch username is already being used from previous aborted runs
-
-			}
-		}
-
-		if ($password === false) {
-			$password = $user;
-		}
-
-		\OC_Util::tearDownFS();
-		\OC_User::setUserId('');
-		\OC\Files\Filesystem::tearDown();
-		\OC_User::setUserId($user);
-		\OC_Util::setupFS($user);
-
-		if ($loadEncryption) {
-			$params['uid'] = $user;
-			$params['password'] = $password;
-			\OCA\Files_Encryption\Hooks::login($params);
-		}
-	}
-
-	public static function logoutHelper() {
-		\OC_Util::tearDownFS();
-		\OC_User::setUserId(false);
-		\OC\Files\Filesystem::tearDown();
-	}
-
-	/**
 	 * helper function to set migration status to the right value
 	 * to be able to test the migration path
 	 *
diff --git a/apps/files_versions/lib/storage.php b/apps/files_versions/lib/storage.php
index 3d30ada863a..0fbb80f425f 100644
--- a/apps/files_versions/lib/storage.php
+++ b/apps/files_versions/lib/storage.php
@@ -139,10 +139,10 @@ class Storage {
 			\OC_FileProxy::$enabled = false;
 
 			// store a new version of a file
-			$mtime = $users_view->filemtime('files'.$filename);
-			$users_view->copy('files'.$filename, 'files_versions'.$filename.'.v'. $mtime);
+			$mtime = $users_view->filemtime('files/' . $filename);
+			$users_view->copy('files/' . $filename, 'files_versions/' . $filename . '.v' . $mtime);
 			// call getFileInfo to enforce a file cache entry for the new version
-			$users_view->getFileInfo('files_versions'.$filename.'.v'.$mtime);
+			$users_view->getFileInfo('files_versions/' . $filename . '.v' . $mtime);
 
 			// reset proxy state
 			\OC_FileProxy::$enabled = $proxyStatus;