Merge pull request #18231 from nextcloud/fix/shared_version_restore

Fix restoring shared versions

3 files changed, 64 insertions, 15 deletions

diff --git a/apps/files_versions/lib/Storage.php b/apps/files_versions/lib/Storage.php
index c63a3dba403..8df98d087d8 100644
--- a/apps/files_versions/lib/Storage.php
+++ b/apps/files_versions/lib/Storage.php
@@ -52,6 +52,7 @@ use OCA\Files_Versions\Command\Expire;
 use OCA\Files_Versions\Events\CreateVersionEvent;
 use OCA\Files_Versions\Versions\IVersionManager;
 use OCP\Files\NotFoundException;
+use OCP\IUser;
 use OCP\Lock\ILockingProvider;
 use OCP\User;
 
@@ -327,20 +328,16 @@ class Storage {
 	 * @param int $revision revision timestamp
 	 * @return bool
 	 */
-	public static function rollback($file, $revision) {
+	public static function rollback(string $file, int $revision, IUser $user) {
 
 		// add expected leading slash
-		$file = '/' . ltrim($file, '/');
-		list($uid, $filename) = self::getUidAndFilename($file);
-		if ($uid === null || trim($filename, '/') === '') {
-			return false;
-		}
+		$filename = '/' . ltrim($file, '/');
 
 		// Fetch the userfolder to trigger view hooks
-		$userFolder = \OC::$server->getUserFolder($uid);
+		$userFolder = \OC::$server->getUserFolder($user->getUID());
 
-		$users_view = new View('/'.$uid);
-		$files_view = new View('/'. User::getUser().'/files');
+		$users_view = new View('/'.$user->getUID());
+		$files_view = new View('/'. $user->getUID().'/files');
 
 		$versionCreated = false;
 
@@ -376,7 +373,7 @@ class Storage {
 		// rollback
 		if (self::copyFileContents($users_view, $fileToRestore, 'files' . $filename)) {
 			$files_view->touch($file, $revision);
-			Storage::scheduleExpire($uid, $file);
+			Storage::scheduleExpire($user->getUID(), $file);
 
 			$node = $userFolder->get($file);
 
diff --git a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php
index b441a16ca82..db8530d1303 100644
--- a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php
+++ b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php
@@ -97,7 +97,7 @@ class LegacyVersionsBackend implements IVersionBackend {
 	}
 
 	public function rollback(IVersion $version) {
-		return Storage::rollback($version->getVersionPath(), $version->getRevisionId());
+		return Storage::rollback($version->getVersionPath(), $version->getRevisionId(), $version->getUser());
 	}
 
 	private function getVersionFolder(IUser $user): Folder {
diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php
index bda9b3f1440..9bee9e0abc2 100644
--- a/apps/files_versions/tests/VersioningTest.php
+++ b/apps/files_versions/tests/VersioningTest.php
@@ -37,6 +37,7 @@ require_once __DIR__ . '/../appinfo/app.php';
 
 use OC\Files\Storage\Temporary;
 use OCP\IConfig;
+use OCP\IUser;
 use OCP\Share\IShare;
 
 /**
@@ -55,6 +56,8 @@ class VersioningTest extends \Test\TestCase {
 	 * @var \OC\Files\View
 	 */
 	private $rootView;
+	private $user1;
+	private $user2;
 
 	public static function setUpBeforeClass(): void {
 		parent::setUpBeforeClass();
@@ -102,6 +105,13 @@ class VersioningTest extends \Test\TestCase {
 		if (!$this->rootView->file_exists(self::USERS_VERSIONS_ROOT)) {
 			$this->rootView->mkdir(self::USERS_VERSIONS_ROOT);
 		}
+
+		$this->user1 = $this->createMock(IUser::class);
+		$this->user1->method('getUID')
+			->willReturn(self::TEST_VERSIONS_USER);
+		$this->user2 = $this->createMock(IUser::class);
+		$this->user2->method('getUID')
+			->willReturn(self::TEST_VERSIONS_USER2);
 	}
 
 	protected function tearDown(): void {
@@ -130,7 +140,7 @@ class VersioningTest extends \Test\TestCase {
 		$startTime = 5000000;
 
 		$testClass = new VersionStorageToTest();
-		list($deleted, $size) = $testClass->callProtectedGetExpireList($startTime, $versions);
+		[$deleted, $size] = $testClass->callProtectedGetExpireList($startTime, $versions);
 
 		// we should have deleted 16 files each of the size 1
 		$this->assertEquals($sizeOfAllDeletedFiles, $size);
@@ -670,7 +680,7 @@ class VersioningTest extends \Test\TestCase {
 
 		$firstVersion = current($versions);
 
-		$this->assertFalse(\OCA\Files_Versions\Storage::rollback('folder/test.txt', $firstVersion['version']), 'Revert did not happen');
+		$this->assertFalse(\OCA\Files_Versions\Storage::rollback('folder/test.txt', $firstVersion['version'], $this->user2), 'Revert did not happen');
 
 		$this->loginAsUser(self::TEST_VERSIONS_USER);
 
@@ -678,6 +688,48 @@ class VersioningTest extends \Test\TestCase {
 		$this->assertEquals('test file', $file->getContent(), 'File content has not changed');
 	}
 
+	public function testRestoreMovedShare() {
+		$this->loginAsUser(self::TEST_VERSIONS_USER);
+
+		$userHome = \OC::$server->getUserFolder(self::TEST_VERSIONS_USER);
+		$node = $userHome->newFolder('folder');
+		$file = $node->newFile('test.txt');
+
+		$userHome2 = \OC::$server->getUserFolder(self::TEST_VERSIONS_USER2);
+		$userHome2->newFolder('subfolder');
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setNode($node)
+			->setShareType(\OCP\Share::SHARE_TYPE_USER)
+			->setSharedBy(self::TEST_VERSIONS_USER)
+			->setSharedWith(self::TEST_VERSIONS_USER2)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL);
+		$share = \OC::$server->getShareManager()->createShare($share);
+		$shareManager = \OC::$server->getShareManager();
+		$shareManager->acceptShare($share, self::TEST_VERSIONS_USER2);
+
+		$share->setTarget("subfolder/folder");
+		$shareManager->moveShare($share, self::TEST_VERSIONS_USER2);
+
+		$versions = $this->createAndCheckVersions(
+			\OC\Files\Filesystem::getView(),
+			'folder/test.txt'
+		);
+
+		$file->putContent('test file');
+
+		$this->loginAsUser(self::TEST_VERSIONS_USER2);
+
+		$firstVersion = current($versions);
+
+		$this->assertTrue(\OCA\Files_Versions\Storage::rollback('folder/test.txt', $firstVersion['version'], $this->user1));
+
+		$this->loginAsUser(self::TEST_VERSIONS_USER);
+
+		\OC::$server->getShareManager()->deleteShare($share);
+		$this->assertEquals('version 2', $file->getContent(), 'File content has not changed');
+	}
+
 	/**
 	 * @param string $hookName name of hook called
 	 * @param string $params variable to receive parameters provided by hook
@@ -738,7 +790,7 @@ class VersioningTest extends \Test\TestCase {
 		$params = array();
 		$this->connectMockHooks('rollback', $params);
 
-		$this->assertTrue(\OCA\Files_Versions\Storage::rollback('sub/test.txt', $t2));
+		$this->assertTrue(\OCA\Files_Versions\Storage::rollback('sub/test.txt', $t2, $this->user1));
 		$expectedParams = array(
 			'path' => '/sub/test.txt',
 		);
@@ -873,7 +925,7 @@ class VersioningTest extends \Test\TestCase {
 		$this->loginAsUser(self::TEST_VERSIONS_USER);
 
 		// need to scan for the versions
-		list($rootStorage,) = $this->rootView->resolvePath(self::TEST_VERSIONS_USER . '/files_versions');
+		[$rootStorage,] = $this->rootView->resolvePath(self::TEST_VERSIONS_USER . '/files_versions');
 		$rootStorage->getScanner()->scan('files_versions');
 
 		$versions = \OCA\Files_Versions\Storage::getVersions(