Skip to content

Commit

Permalink
Merge pull request #27042 from owncloud/sharing-trashfileforownerwhen…
Browse files Browse the repository at this point in the history 
…movedout

Copy file to owner's trash when recipient moves out of share
  • Loading branch information
Vincent Petry authored Feb 21, 2017
2 parents 33a7697 + 641d3dc commit 28c345d
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 11 deletions.
24 changes: 24 additions & 0 deletions apps/files_trashbin/lib/Storage.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,30 @@ public static function preRenameHook($params) {
// in cross-storage cases, a rename is a copy + unlink,
// that last unlink must not go to trash
self::$disableTrash = true;

$path1 = $params[Filesystem::signal_param_oldpath];
$path2 = $params[Filesystem::signal_param_newpath];

$view = Filesystem::getView();
$absolutePath1 = Filesystem::normalizePath($view->getAbsolutePath($path1));

$mount1 = $view->getMount($path1);
$mount2 = $view->getMount($path2);
$sourceStorage = $mount1->getStorage();
$targetStorage = $mount2->getStorage();
$sourceInternalPath = $mount1->getInternalPath($absolutePath1);
// check whether this is a cross-storage move from a *local* shared storage
if ($sourceInternalPath !== '' && $sourceStorage !== $targetStorage && $sourceStorage->instanceOfStorage('OCA\Files_Sharing\SharedStorage')) {
$ownerPath = $sourceStorage->getSourcePath($sourceInternalPath);
$owner = $sourceStorage->getOwner($sourceInternalPath);
if ($owner !== null && $owner !== '' && $ownerPath !== null && substr($ownerPath, 0, 6) === 'files/') {
// ownerPath is in the format "files/path/to/file.txt", strip "files"
$ownerPath = substr($ownerPath, 6);

// make a backup copy for the owner
\OCA\Files_Trashbin\Trashbin::copyBackupForOwner($ownerPath, $owner, time());
}
}
}

/**
Expand Down
54 changes: 43 additions & 11 deletions apps/files_trashbin/lib/Trashbin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,42 @@ private static function copyFilesToUser($sourcePath, $owner, $targetPath, $user,
$target = $user . '/files_trashbin/files/' . $targetFilename . '.d' . $timestamp;
$source = $owner . '/files_trashbin/files/' . $sourceFilename . '.d' . $timestamp;
self::copy_recursive($source, $target, $view);
}

/**
* Make a backup of a file into the trashbin for the owner
*
* @param string $ownerPath path relative to the owner's home folder and containing "files"
* @param string $owner user id of the owner
* @param int $timestamp deletion timestamp
*/
public static function copyBackupForOwner($ownerPath, $owner, $timestamp) {
self::setUpTrash($owner);

$targetFilename = basename($ownerPath);
$targetLocation = dirname($ownerPath);
$source = $owner . '/files/' . ltrim($ownerPath, '/');
$target = $owner . '/files_trashbin/files/' . $targetFilename . '.d' . $timestamp;

$view = new View('/');
self::copy_recursive($source, $target, $view);

self::retainVersions($targetFilename, $owner, $ownerPath, $timestamp, true);

if ($view->file_exists($target)) {
$query = \OC_DB::prepare("INSERT INTO `*PREFIX*files_trash` (`id`,`timestamp`,`location`,`user`) VALUES (?,?,?,?)");
$result = $query->execute([$targetFilename, $timestamp, $targetLocation, $user]);
if (!$result) {
\OCP\Util::writeLog('files_trashbin', 'trash bin database couldn\'t be updated for the files owner', \OCP\Util::ERROR);
}
self::insertTrashEntry($owner, $targetFilename, $targetLocation, $timestamp);
self::scheduleExpire($owner);
}
}

/**
*
*/
public static function insertTrashEntry($user, $targetFilename, $targetLocation, $timestamp) {
$query = \OC_DB::prepare("INSERT INTO `*PREFIX*files_trash` (`id`,`timestamp`,`location`,`user`) VALUES (?,?,?,?)");
$result = $query->execute([$targetFilename, $timestamp, $targetLocation, $user]);
if (!$result) {
\OCP\Util::writeLog('files_trashbin', 'trash bin database couldn\'t be updated for the files owner', \OCP\Util::ERROR);
}
}

Expand Down Expand Up @@ -231,7 +259,6 @@ public static function move2trash($file_path) {
$location = $path_parts['dirname'];
$timestamp = time();

// disable proxy to prevent recursive calls
$trashPath = '/files_trashbin/files/' . $filename . '.d' . $timestamp;

/** @var \OC\Files\Storage\Storage $trashStorage */
Expand Down Expand Up @@ -297,25 +324,30 @@ public static function move2trash($file_path) {
* @param string $owner owner user id
* @param string $ownerPath path relative to the owner's home storage
* @param integer $timestamp when the file was deleted
* @param bool $forceCopy true to only make a copy of the versions into the trashbin
*/
private static function retainVersions($filename, $owner, $ownerPath, $timestamp) {
private static function retainVersions($filename, $owner, $ownerPath, $timestamp, $forceCopy = false) {
if (\OCP\App::isEnabled('files_versions') && !empty($ownerPath)) {

$user = User::getUser();
$rootView = new View('/');

if ($rootView->is_dir($owner . '/files_versions/' . $ownerPath)) {
if ($owner !== $user) {
if ($owner !== $user || $forceCopy) {
self::copy_recursive($owner . '/files_versions/' . $ownerPath, $owner . '/files_trashbin/versions/' . basename($ownerPath) . '.d' . $timestamp, $rootView);
}
self::move($rootView, $owner . '/files_versions/' . $ownerPath, $user . '/files_trashbin/versions/' . $filename . '.d' . $timestamp);
if (!$forceCopy) {
self::move($rootView, $owner . '/files_versions/' . $ownerPath, $user . '/files_trashbin/versions/' . $filename . '.d' . $timestamp);
}
} else if ($versions = \OCA\Files_Versions\Storage::getVersions($owner, $ownerPath)) {

foreach ($versions as $v) {
if ($owner !== $user) {
if ($owner !== $user || $forceCopy) {
self::copy($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $owner . '/files_trashbin/versions/' . $v['name'] . '.v' . $v['version'] . '.d' . $timestamp);
}
self::move($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $user . '/files_trashbin/versions/' . $filename . '.v' . $v['version'] . '.d' . $timestamp);
if (!$forceCopy) {
self::move($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $user . '/files_trashbin/versions/' . $filename . '.v' . $v['version'] . '.d' . $timestamp);
}
}
}
}
Expand Down
80 changes: 80 additions & 0 deletions apps/files_trashbin/tests/StorageTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,14 @@ protected function setUp() {
\OC_Hook::clear();
\OCA\Files_Trashbin\Trashbin::registerHooks();

// the encryption wrapper does some twisted stuff with moveFromStorage...
// we need to register it here so that the tested behavior is closer to reality
\OC::$server->getEncryptionManager()->setupStorage();

$this->user = $this->getUniqueId('user');
\OC::$server->getUserManager()->createUser($this->user, $this->user);


// this will setup the FS
$this->loginAsUser($this->user);

Expand All @@ -82,6 +87,7 @@ protected function tearDown() {
$user = \OC::$server->getUserManager()->get($this->user);
if ($user !== null) { $user->delete(); }
\OC_Hook::clear();
\OC\Files\Filesystem::getLoader()->removeStorageWrapper('oc_encryption');
parent::tearDown();
}

Expand Down Expand Up @@ -446,6 +452,80 @@ public function testKeepFileAndVersionsWhenMovingFolderBetweenStorages() {
$this->assertEquals(0, count($results));
}

/**
* Test that the owner receives a backup of the file that was moved
* out of the shared folder
*/
public function testOwnerBackupWhenMovingFileOutOfShare() {
\OCA\Files_Versions\Hooks::connectHooks();

$this->userView->mkdir('share');
$this->userView->mkdir('share/sub');
// trigger a version (multiple would not work because of the expire logic)
$this->userView->file_put_contents('share/test.txt', 'v1');
$this->userView->file_put_contents('share/test.txt', 'v2');

$this->userView->file_put_contents('share/sub/testsub.txt', 'v1');
$this->userView->file_put_contents('share/sub/testsub.txt', 'v2');

$results = $this->rootView->getDirectoryContent($this->user . '/files_versions/share/');
$this->assertEquals(2, count($results));
$results = $this->rootView->getDirectoryContent($this->user . '/files_versions/share/sub');
$this->assertEquals(1, count($results));

$recipientUser = $this->getUniqueId('recipient_');
$user2 = \OC::$server->getUserManager()->createUser($recipientUser, $recipientUser);

$node = \OC::$server->getUserFolder($this->user)->get('share');
$share = \OC::$server->getShareManager()->newShare();
$share->setNode($node)
->setShareType(\OCP\Share::SHARE_TYPE_USER)
->setSharedBy($this->user)
->setSharedWith($recipientUser)
->setPermissions(\OCP\Constants::PERMISSION_ALL);
\OC::$server->getShareManager()->createShare($share);

$this->loginAsUser($recipientUser);

// delete as recipient
$recipientHome = \OC::$server->getUserFolder($recipientUser);

// rename received share folder to catch potential issues if using the wrong name in the code
$recipientHome->get('share')->move($recipientHome->getPath() . '/share_renamed');
$recipientHome->get('share_renamed/test.txt')->move($recipientHome->getPath() . '/test.txt');
$recipientHome->get('share_renamed/sub')->move($recipientHome->getPath() . '/sub');

$this->assertTrue($recipientHome->nodeExists('test.txt'));
$this->assertTrue($recipientHome->nodeExists('sub'));

// check if file and versions are in trashbin for owner
$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files');
$this->assertEquals(2, count($results), 'Files in owner\'s trashbin');
// grab subdir name
$subDirName = $results[0]->getName();
$name = $results[1]->getName();
$this->assertEquals('test.txt.d', substr($name, 0, strlen('test.txt.d')));

$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/versions');
$this->assertEquals(2, count($results), 'Versions in owner\'s trashbin');
// note: entry 0 is the "sub" entry for versions
$name = $results[1]->getName();
$this->assertEquals('test.txt.v', substr($name, 0, strlen('test.txt.v')));

// check if sub-file and versions are in trashbin for owner
$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/' . $subDirName);
$this->assertEquals(1, count($results), 'Subfile in owner\'s trashbin');
$this->assertEquals('testsub.txt', $results[0]->getName());

$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/versions/' . $subDirName);
$this->assertEquals(1, count($results), 'Versions in owner\'s trashbin');
$name = $results[0]->getName();
$this->assertEquals('testsub.txt.v', substr($name, 0, strlen('testsub.txt.v')));

$this->logout();
$user2->delete();
}

/**
* Delete should fail if the source file can't be deleted.
*/
Expand Down
28 changes: 28 additions & 0 deletions build/integration/features/sharing-v1.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1133,3 +1133,31 @@ Feature: sharing
Then as "user1" the file "/shared/shared_file.txt" exists
And as "user0" the file "/shared/shared_file.txt" exists

Scenario: moving file out of a share as recipient creates a backup for the owner
Given As an "admin"
And user "user0" exists
And user "user1" exists
And user "user0" created a folder "/shared"
And User "user0" moved file "/textfile0.txt" to "/shared/shared_file.txt"
And file "/shared" of user "user0" is shared with user "user1"
And User "user1" moved folder "/shared" to "/shared_renamed"
When User "user1" moved file "/shared_renamed/shared_file.txt" to "/taken_out.txt"
Then as "user1" the file "/taken_out.txt" exists
And as "user0" the file "/shared/shared_file.txt" does not exist
And as "user0" the file "/shared_file.txt" exists in trash

Scenario: moving folder out of a share as recipient creates a backup for the owner
Given As an "admin"
And user "user0" exists
And user "user1" exists
And user "user0" created a folder "/shared"
And user "user0" created a folder "/shared/sub"
And User "user0" moved file "/textfile0.txt" to "/shared/sub/shared_file.txt"
And file "/shared" of user "user0" is shared with user "user1"
And User "user1" moved folder "/shared" to "/shared_renamed"
When User "user1" moved folder "/shared_renamed/sub" to "/taken_out"
Then as "user1" the file "/taken_out" exists
And as "user0" the folder "/shared/sub" does not exist
And as "user0" the folder "/sub" exists in trash
And as "user0" the file "/sub/shared_file.txt" exists in trash

0 comments on commit 28c345d

Please sign in to comment.