diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index 30d78ebd23..a843892773 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -179,6 +179,27 @@ trait BasicStructure { } } + public function sendingToWithDirectUrl($verb, $url, $body) { + $fullUrl = substr($this->baseUrl, 0, -5) . $url; + $client = new Client(); + $options = []; + if ($this->currentUser === 'admin') { + $options['auth'] = $this->adminUser; + } else { + $options['auth'] = [$this->currentUser, $this->regularUser]; + } + if ($body instanceof \Behat\Gherkin\Node\TableNode) { + $fd = $body->getRowsHash(); + $options['body'] = $fd; + } + + try { + $this->response = $client->send($client->createRequest($verb, $fullUrl, $options)); + } catch (\GuzzleHttp\Exception\ClientException $ex) { + $this->response = $ex->getResponse(); + } + } + public function isExpectedUrl($possibleUrl, $finalPart){ $baseUrlChopped = substr($this->baseUrl, 0, -4); $endCharacter = strlen($baseUrlChopped) + strlen($finalPart); @@ -320,6 +341,16 @@ trait BasicStructure { fclose($file); } + /** + * @When User :user empties trashbin + * @param string $user + */ + public function emptyTrashbin($user) { + $body = new \Behat\Gherkin\Node\TableNode([['allfiles', 'true'], ['dir', '%2F']]); + $this->sendingToWithDirectUrl('POST', "/index.php/apps/files_trashbin/ajax/delete.php", $body); + $this->theHTTPStatusCodeShouldBe('200'); + } + /** * @BeforeSuite */ diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index ad29f28e10..cdb1fc3fdf 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -455,11 +455,12 @@ trait WebDav { } /** - * @When User :user deletes file :file + * @When /^User "([^"]*)" deletes (file|folder) "([^"]*)"$/ * @param string $user + * @param string $type * @param string $file */ - public function userDeletesFile($user, $file) { + public function userDeletesFile($user, $type, $file) { try { $this->response = $this->makeDavRequest($user, 'DELETE', $file, []); } catch (\GuzzleHttp\Exception\ServerException $e) { diff --git a/build/integration/features/sharing-v1.feature b/build/integration/features/sharing-v1.feature index 3c769fba3d..ab1b9c63cc 100644 --- a/build/integration/features/sharing-v1.feature +++ b/build/integration/features/sharing-v1.feature @@ -920,3 +920,21 @@ Feature: sharing # |{http://owncloud.org/ns}permissions| # And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK" # And as "user1" the folder "merge-test-outside-groups-renamebeforesecondshare" does not exist + + Scenario: Empting trashbin + Given As an "admin" + And user "user0" exists + And User "user0" deletes file "/textfile0.txt" + When User "user0" empties trashbin + Then the HTTP status code should be "200" + + Scenario: orphaned shares + Given As an "admin" + And user "user0" exists + And user "user1" exists + And user "user0" created a folder "/common" + And user "user0" created a folder "/common/sub" + And file "/common/sub" of user "user0" is shared with user "user1" + And User "user0" deletes folder "/common" + When User "user0" empties trashbin + Then as "user1" the folder "sub" does not exist diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index f33c297b02..d6afcf9991 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -582,6 +582,25 @@ class DefaultShareProvider implements IShareProvider { return $shares; } + /** + * Returns whether the given database result can be interpreted as + * a share with accessible file (not trashed, not deleted) + */ + private function isAccessibleResult($data) { + // exclude shares leading to deleted file entries + if ($data['fileid'] === null) { + return false; + } + + // exclude shares leading to trashbin on home storages + $pathSections = explode('/', $data['path'], 2); + // FIXME: would not detect rare md5'd home storage case properly + if ($pathSections[0] !== 'files' && explode(':', $data['storage_string_id'], 2)[0] === 'home') { + return false; + } + return true; + } + /** * @inheritdoc */ @@ -592,11 +611,14 @@ class DefaultShareProvider implements IShareProvider { if ($shareType === \OCP\Share::SHARE_TYPE_USER) { //Get shares directly with this user $qb = $this->dbConn->getQueryBuilder(); - $qb->select('*') - ->from('share'); + $qb->select('s.*', 'f.fileid', 'f.path') + ->selectAlias('st.id', 'storage_string_id') + ->from('share', 's') + ->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid')) + ->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id')); // Order by id - $qb->orderBy('id'); + $qb->orderBy('s.id'); // Set limit and offset if ($limit !== -1) { @@ -619,7 +641,9 @@ class DefaultShareProvider implements IShareProvider { $cursor = $qb->execute(); while($data = $cursor->fetch()) { - $shares[] = $this->createShare($data); + if ($this->isAccessibleResult($data)) { + $shares[] = $this->createShare($data); + } } $cursor->closeCursor(); @@ -640,9 +664,12 @@ class DefaultShareProvider implements IShareProvider { } $qb = $this->dbConn->getQueryBuilder(); - $qb->select('*') - ->from('share') - ->orderBy('id') + $qb->select('s.*', 'f.fileid', 'f.path') + ->selectAlias('st.id', 'storage_string_id') + ->from('share', 's') + ->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid')) + ->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id')) + ->orderBy('s.id') ->setFirstResult(0); if ($limit !== -1) { @@ -672,7 +699,10 @@ class DefaultShareProvider implements IShareProvider { $offset--; continue; } - $shares2[] = $this->createShare($data); + + if ($this->isAccessibleResult($data)) { + $shares2[] = $this->createShare($data); + } } $cursor->closeCursor(); } diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index ae9ad47b9a..2fe5046083 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -77,6 +77,8 @@ class DefaultShareProviderTest extends \Test\TestCase { public function tearDown() { $this->dbConn->getQueryBuilder()->delete('share')->execute(); + $this->dbConn->getQueryBuilder()->delete('filecache')->execute(); + $this->dbConn->getQueryBuilder()->delete('storages')->execute(); } /** @@ -781,7 +783,47 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->provider->getShareByToken('invalidtoken'); } - public function testGetSharedWithUser() { + private function createTestStorageEntry($storageStringId) { + $qb = $this->dbConn->getQueryBuilder(); + $qb->insert('storages') + ->values([ + 'id' => $qb->expr()->literal($storageStringId), + ]); + $this->assertEquals(1, $qb->execute()); + return $qb->getLastInsertId(); + } + + private function createTestFileEntry($path, $storage = 1) { + $qb = $this->dbConn->getQueryBuilder(); + $qb->insert('filecache') + ->values([ + 'storage' => $qb->expr()->literal($storage), + 'path' => $qb->expr()->literal($path), + 'path_hash' => $qb->expr()->literal(md5($path)), + 'name' => $qb->expr()->literal(basename($path)), + ]); + $this->assertEquals(1, $qb->execute()); + return $qb->getLastInsertId(); + } + + public function storageAndFileNameProvider() { + return [ + // regular file on regular storage + ['home::shareOwner', 'files/test.txt', 'files/test2.txt'], + // regular file on external storage + ['smb::whatever', 'files/test.txt', 'files/test2.txt'], + // regular file on external storage in trashbin-like folder, + ['smb::whatever', 'files_trashbin/files/test.txt', 'files_trashbin/files/test2.txt'], + ]; + } + + /** + * @dataProvider storageAndFileNameProvider + */ + public function testGetSharedWithUser($storageStringId, $fileName1, $fileName2) { + $storageId = $this->createTestStorageEntry($storageStringId); + $fileId = $this->createTestFileEntry($fileName1, $storageId); + $fileId2 = $this->createTestFileEntry($fileName2, $storageId); $qb = $this->dbConn->getQueryBuilder(); $qb->insert('share') ->values([ @@ -790,7 +832,7 @@ class DefaultShareProviderTest extends \Test\TestCase { 'uid_owner' => $qb->expr()->literal('shareOwner'), 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), - 'file_source' => $qb->expr()->literal(42), + 'file_source' => $qb->expr()->literal($fileId), 'file_target' => $qb->expr()->literal('myTarget'), 'permissions' => $qb->expr()->literal(13), ]); @@ -805,7 +847,7 @@ class DefaultShareProviderTest extends \Test\TestCase { 'uid_owner' => $qb->expr()->literal('shareOwner2'), 'uid_initiator' => $qb->expr()->literal('sharedBy2'), 'item_type' => $qb->expr()->literal('file2'), - 'file_source' => $qb->expr()->literal(43), + 'file_source' => $qb->expr()->literal($fileId2), 'file_target' => $qb->expr()->literal('myTarget2'), 'permissions' => $qb->expr()->literal(14), ]); @@ -813,7 +855,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $file = $this->createMock(File::class); $this->rootFolder->method('getUserFolder')->with('shareOwner')->will($this->returnSelf()); - $this->rootFolder->method('getById')->with(42)->willReturn([$file]); + $this->rootFolder->method('getById')->with($fileId)->willReturn([$file]); $share = $this->provider->getSharedWith('sharedWith', \OCP\Share::SHARE_TYPE_USER, null, 1 , 0); $this->assertCount(1, $share); @@ -826,7 +868,13 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals(\OCP\Share::SHARE_TYPE_USER, $share->getShareType()); } - public function testGetSharedWithGroup() { + /** + * @dataProvider storageAndFileNameProvider + */ + public function testGetSharedWithGroup($storageStringId, $fileName1, $fileName2) { + $storageId = $this->createTestStorageEntry($storageStringId); + $fileId = $this->createTestFileEntry($fileName1, $storageId); + $fileId2 = $this->createTestFileEntry($fileName2, $storageId); $qb = $this->dbConn->getQueryBuilder(); $qb->insert('share') ->values([ @@ -835,7 +883,7 @@ class DefaultShareProviderTest extends \Test\TestCase { 'uid_owner' => $qb->expr()->literal('shareOwner2'), 'uid_initiator' => $qb->expr()->literal('sharedBy2'), 'item_type' => $qb->expr()->literal('file'), - 'file_source' => $qb->expr()->literal(43), + 'file_source' => $qb->expr()->literal($fileId2), 'file_target' => $qb->expr()->literal('myTarget2'), 'permissions' => $qb->expr()->literal(14), ]); @@ -849,7 +897,7 @@ class DefaultShareProviderTest extends \Test\TestCase { 'uid_owner' => $qb->expr()->literal('shareOwner'), 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), - 'file_source' => $qb->expr()->literal(42), + 'file_source' => $qb->expr()->literal($fileId), 'file_target' => $qb->expr()->literal('myTarget'), 'permissions' => $qb->expr()->literal(13), ]); @@ -884,7 +932,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $file = $this->createMock(File::class); $this->rootFolder->method('getUserFolder')->with('shareOwner')->will($this->returnSelf()); - $this->rootFolder->method('getById')->with(42)->willReturn([$file]); + $this->rootFolder->method('getById')->with($fileId)->willReturn([$file]); $share = $this->provider->getSharedWith('sharedWith', \OCP\Share::SHARE_TYPE_GROUP, null, 20 , 1); $this->assertCount(1, $share); @@ -897,7 +945,12 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals(\OCP\Share::SHARE_TYPE_GROUP, $share->getShareType()); } - public function testGetSharedWithGroupUserModified() { + /** + * @dataProvider storageAndFileNameProvider + */ + public function testGetSharedWithGroupUserModified($storageStringId, $fileName1, $fileName2) { + $storageId = $this->createTestStorageEntry($storageStringId); + $fileId = $this->createTestFileEntry($fileName1, $storageId); $qb = $this->dbConn->getQueryBuilder(); $qb->insert('share') ->values([ @@ -906,7 +959,7 @@ class DefaultShareProviderTest extends \Test\TestCase { 'uid_owner' => $qb->expr()->literal('shareOwner'), 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), - 'file_source' => $qb->expr()->literal(42), + 'file_source' => $qb->expr()->literal($fileId), 'file_target' => $qb->expr()->literal('myTarget'), 'permissions' => $qb->expr()->literal(13), ]); @@ -924,7 +977,7 @@ class DefaultShareProviderTest extends \Test\TestCase { 'uid_owner' => $qb->expr()->literal('shareOwner'), 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), - 'file_source' => $qb->expr()->literal(42), + 'file_source' => $qb->expr()->literal($fileId), 'file_target' => $qb->expr()->literal('wrongTarget'), 'permissions' => $qb->expr()->literal(31), 'parent' => $qb->expr()->literal($id), @@ -942,7 +995,7 @@ class DefaultShareProviderTest extends \Test\TestCase { 'uid_owner' => $qb->expr()->literal('shareOwner'), 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), - 'file_source' => $qb->expr()->literal(42), + 'file_source' => $qb->expr()->literal($fileId), 'file_target' => $qb->expr()->literal('userTarget'), 'permissions' => $qb->expr()->literal(0), 'parent' => $qb->expr()->literal($id), @@ -970,7 +1023,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $file = $this->createMock(File::class); $this->rootFolder->method('getUserFolder')->with('shareOwner')->will($this->returnSelf()); - $this->rootFolder->method('getById')->with(42)->willReturn([$file]); + $this->rootFolder->method('getById')->with($fileId)->willReturn([$file]); $share = $this->provider->getSharedWith('user', \OCP\Share::SHARE_TYPE_GROUP, null, -1, 0); $this->assertCount(1, $share); @@ -985,11 +1038,17 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertSame('userTarget', $share->getTarget()); } - public function testGetSharedWithUserWithNode() { + /** + * @dataProvider storageAndFileNameProvider + */ + public function testGetSharedWithUserWithNode($storageStringId, $fileName1, $fileName2) { + $storageId = $this->createTestStorageEntry($storageStringId); + $fileId = $this->createTestFileEntry($fileName1, $storageId); + $fileId2 = $this->createTestFileEntry($fileName2, $storageId); $this->addShareToDB(\OCP\Share::SHARE_TYPE_USER, 'user0', 'user1', 'user1', - 'file', 42, 'myTarget', 31, null, null, null); + 'file', $fileId, 'myTarget', 31, null, null, null); $id = $this->addShareToDB(\OCP\Share::SHARE_TYPE_USER, 'user0', 'user1', 'user1', - 'file', 43, 'myTarget', 31, null, null, null); + 'file', $fileId2, 'myTarget', 31, null, null, null); $user0 = $this->createMock(IUser::class); $user0->method('getUID')->willReturn('user0'); @@ -1002,9 +1061,10 @@ class DefaultShareProviderTest extends \Test\TestCase { ]); $file = $this->createMock(File::class); - $file->method('getId')->willReturn(43); + $file->method('getId')->willReturn($fileId2); + $this->rootFolder->method('getUserFolder')->with('user1')->will($this->returnSelf()); - $this->rootFolder->method('getById')->with(43)->willReturn([$file]); + $this->rootFolder->method('getById')->with($fileId2)->willReturn([$file]); $share = $this->provider->getSharedWith('user0', \OCP\Share::SHARE_TYPE_USER, $file, -1, 0); $this->assertCount(1, $share); @@ -1018,11 +1078,17 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals(\OCP\Share::SHARE_TYPE_USER, $share->getShareType()); } - public function testGetSharedWithGroupWithNode() { + /** + * @dataProvider storageAndFileNameProvider + */ + public function testGetSharedWithGroupWithNode($storageStringId, $fileName1, $fileName2) { + $storageId = $this->createTestStorageEntry($storageStringId); + $fileId = $this->createTestFileEntry($fileName1, $storageId); + $fileId2 = $this->createTestFileEntry($fileName2, $storageId); $this->addShareToDB(\OCP\Share::SHARE_TYPE_GROUP, 'group0', 'user1', 'user1', - 'file', 42, 'myTarget', 31, null, null, null); + 'file', $fileId, 'myTarget', 31, null, null, null); $id = $this->addShareToDB(\OCP\Share::SHARE_TYPE_GROUP, 'group0', 'user1', 'user1', - 'file', 43, 'myTarget', 31, null, null, null); + 'file', $fileId2, 'myTarget', 31, null, null, null); $user0 = $this->createMock(IUser::class); $user0->method('getUID')->willReturn('user0'); @@ -1041,9 +1107,9 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->groupManager->method('getUserGroups')->with($user0)->willReturn([$group0]); $node = $this->createMock(Folder::class); - $node->method('getId')->willReturn(43); + $node->method('getId')->willReturn($fileId2); $this->rootFolder->method('getUserFolder')->with('user1')->will($this->returnSelf()); - $this->rootFolder->method('getById')->with(43)->willReturn([$node]); + $this->rootFolder->method('getById')->with($fileId2)->willReturn([$node]); $share = $this->provider->getSharedWith('user0', \OCP\Share::SHARE_TYPE_GROUP, $node, -1, 0); $this->assertCount(1, $share); @@ -1057,6 +1123,75 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals(\OCP\Share::SHARE_TYPE_GROUP, $share->getShareType()); } + public function shareTypesProvider() { + return [ + [\OCP\Share::SHARE_TYPE_USER, false], + [\OCP\Share::SHARE_TYPE_GROUP, false], + [\OCP\Share::SHARE_TYPE_USER, true], + [\OCP\Share::SHARE_TYPE_GROUP, true], + ]; + } + + /** + * @dataProvider shareTypesProvider + */ + public function testGetSharedWithWithDeletedFile($shareType, $trashed) { + if ($trashed) { + // exists in database but is in trash + $storageId = $this->createTestStorageEntry('home::shareOwner'); + $deletedFileId = $this->createTestFileEntry('files_trashbin/files/test.txt.d1465553223', $storageId); + } else { + // fileid that doesn't exist in the database + $deletedFileId = 123; + } + $qb = $this->dbConn->getQueryBuilder(); + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal($shareType), + 'share_with' => $qb->expr()->literal('sharedWith'), + 'uid_owner' => $qb->expr()->literal('shareOwner'), + 'uid_initiator' => $qb->expr()->literal('sharedBy'), + 'item_type' => $qb->expr()->literal('file'), + 'file_source' => $qb->expr()->literal($deletedFileId), + 'file_target' => $qb->expr()->literal('myTarget'), + 'permissions' => $qb->expr()->literal(13), + ]); + $this->assertEquals(1, $qb->execute()); + + $file = $this->getMock('\OCP\Files\File'); + $this->rootFolder->method('getUserFolder')->with('shareOwner')->will($this->returnSelf()); + $this->rootFolder->method('getById')->with($deletedFileId)->willReturn([$file]); + + $groups = []; + foreach(range(0, 100) as $i) { + $group = $this->getMock('\OCP\IGroup'); + $group->method('getGID')->willReturn('group'.$i); + $groups[] = $group; + } + + $group = $this->getMock('\OCP\IGroup'); + $group->method('getGID')->willReturn('sharedWith'); + $groups[] = $group; + + $user = $this->getMock('\OCP\IUser'); + $user->method('getUID')->willReturn('sharedWith'); + $owner = $this->getMock('\OCP\IUser'); + $owner->method('getUID')->willReturn('shareOwner'); + $initiator = $this->getMock('\OCP\IUser'); + $initiator->method('getUID')->willReturn('sharedBy'); + + $this->userManager->method('get')->willReturnMap([ + ['sharedWith', $user], + ['shareOwner', $owner], + ['sharedBy', $initiator], + ]); + $this->groupManager->method('getUserGroups')->with($user)->willReturn($groups); + $this->groupManager->method('get')->with('sharedWith')->willReturn($group); + + $share = $this->provider->getSharedWith('sharedWith', $shareType, null, 1 , 0); + $this->assertCount(0, $share); + } + public function testGetSharesBy() { $qb = $this->dbConn->getQueryBuilder(); $qb->insert('share')