Skip to content

Commit ab120cf

Browse files
rullzerrullzer
authored
Merge pull request #21774 from nextcloud/backport/21489/stable17
[stable17] Use the correct mountpoint to calculate
2 parents 342b5bf + 50f49c2 commit ab120cf

10 files changed

Lines changed: 268 additions & 4 deletions

File tree

‎apps/files_sharing/tests/EtagPropagationTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ protected function setUpShares() {
120120
$view2 = new View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files');
121121
$view2->mkdir('/sub1/sub2');
122122
$view2->rename('/folder', '/sub1/sub2/folder');
123+
$this->loginAsUser(self::TEST_FILES_SHARING_API_USER2);
124+
123125
$insideInfo = $view2->getFileInfo('/sub1/sub2/folder/inside');
124126
$this->assertInstanceOf('\OC\Files\FileInfo', $insideInfo);
125127

‎apps/files_sharing/tests/TestCase.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,14 @@ protected function tearDown() {
124124
$qb->delete('share');
125125
$qb->execute();
126126

127+
$qb = \OC::$server->getDatabaseConnection()->getQueryBuilder();
128+
$qb->delete('mounts');
129+
$qb->execute();
130+
131+
$qb = \OC::$server->getDatabaseConnection()->getQueryBuilder();
132+
$qb->delete('filecache');
133+
$qb->execute();
134+
127135
parent::tearDown();
128136
}
129137

‎build/integration/features/sharing-v1-part2.feature‎

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,34 @@ Feature: sharing
231231
Then the OCS status code should be "404"
232232
And the HTTP status code should be "200"
233233

234+
Scenario: User is allowed to reshare file with more permissions if shares of same file to same user have them
235+
Given user "user0" exists
236+
And user "user1" exists
237+
And user "user2" exists
238+
And group "group1" exists
239+
And user "user1" belongs to group "group1"
240+
And As an "user0"
241+
And creating a share with
242+
| path | /textfile0.txt |
243+
| shareType | 1 |
244+
| shareWith | group1 |
245+
| permissions | 15 |
246+
And As an "user1"
247+
And As an "user0"
248+
And creating a share with
249+
| path | /textfile0.txt |
250+
| shareType | 0 |
251+
| shareWith | user1 |
252+
| permissions | 17 |
253+
And As an "user1"
254+
When creating a share with
255+
| path | /textfile0 (2).txt |
256+
| shareType | 0 |
257+
| shareWith | user2 |
258+
| permissions | 19 |
259+
Then the OCS status code should be "100"
260+
And the HTTP status code should be "200"
261+
234262
Scenario: User is not allowed to reshare file with more permissions
235263
As an "admin"
236264
Given user "user0" exists
@@ -251,6 +279,86 @@ Feature: sharing
251279
Then the OCS status code should be "404"
252280
And the HTTP status code should be "200"
253281

282+
Scenario: User is not allowed to reshare file with more permissions even if shares of same file to other users have them
283+
Given user "user0" exists
284+
And user "user1" exists
285+
And user "user2" exists
286+
And user "user3" exists
287+
And As an "user0"
288+
And creating a share with
289+
| path | /textfile0.txt |
290+
| shareType | 0 |
291+
| shareWith | user3 |
292+
| permissions | 15 |
293+
And As an "user3"
294+
And As an "user0"
295+
And creating a share with
296+
| path | /textfile0.txt |
297+
| shareType | 0 |
298+
| shareWith | user1 |
299+
| permissions | 17 |
300+
And As an "user1"
301+
When creating a share with
302+
| path | /textfile0 (2).txt |
303+
| shareType | 0 |
304+
| shareWith | user2 |
305+
| permissions | 19 |
306+
Then the OCS status code should be "404"
307+
And the HTTP status code should be "200"
308+
309+
Scenario: User is not allowed to reshare file with more permissions even if shares of other files from same user have them
310+
Given user "user0" exists
311+
And user "user1" exists
312+
And user "user2" exists
313+
And As an "user0"
314+
And creating a share with
315+
| path | /textfile0.txt |
316+
| shareType | 0 |
317+
| shareWith | user1 |
318+
| permissions | 15 |
319+
And As an "user1"
320+
And As an "user0"
321+
And creating a share with
322+
| path | /textfile1.txt |
323+
| shareType | 0 |
324+
| shareWith | user1 |
325+
| permissions | 17 |
326+
And As an "user1"
327+
When creating a share with
328+
| path | /textfile1 (2).txt |
329+
| shareType | 0 |
330+
| shareWith | user2 |
331+
| permissions | 19 |
332+
Then the OCS status code should be "404"
333+
And the HTTP status code should be "200"
334+
335+
Scenario: User is not allowed to reshare file with more permissions even if shares of other files from other users have them
336+
Given user "user0" exists
337+
And user "user1" exists
338+
And user "user2" exists
339+
And user "user3" exists
340+
And As an "user3"
341+
And creating a share with
342+
| path | /textfile0.txt |
343+
| shareType | 0 |
344+
| shareWith | user1 |
345+
| permissions | 15 |
346+
And As an "user1"
347+
And As an "user0"
348+
And creating a share with
349+
| path | /textfile1.txt |
350+
| shareType | 0 |
351+
| shareWith | user1 |
352+
| permissions | 17 |
353+
And As an "user1"
354+
When creating a share with
355+
| path | /textfile1 (2).txt |
356+
| shareType | 0 |
357+
| shareWith | user2 |
358+
| permissions | 19 |
359+
Then the OCS status code should be "404"
360+
And the HTTP status code should be "200"
361+
254362
Scenario: User is not allowed to reshare file with additional delete permissions
255363
As an "admin"
256364
Given user "user0" exists
@@ -456,6 +564,37 @@ Feature: sharing
456564
| permissions | 1 |
457565
Then the OCS status code should be "100"
458566

567+
Scenario: Allow reshare to exceed permissions if shares of same file to same user have them
568+
Given user "user0" exists
569+
And user "user1" exists
570+
And user "user2" exists
571+
And group "group1" exists
572+
And user "user1" belongs to group "group1"
573+
And user "user0" created a folder "/TMP"
574+
And As an "user0"
575+
And creating a share with
576+
| path | /TMP |
577+
| shareType | 1 |
578+
| shareWith | group1 |
579+
| permissions | 15 |
580+
And As an "user1"
581+
And As an "user0"
582+
And creating a share with
583+
| path | /TMP |
584+
| shareType | 0 |
585+
| shareWith | user1 |
586+
| permissions | 17 |
587+
And As an "user1"
588+
And creating a share with
589+
| path | /TMP |
590+
| shareType | 0 |
591+
| shareWith | user2 |
592+
| permissions | 17 |
593+
When Updating last share with
594+
| permissions | 31 |
595+
Then the OCS status code should be "100"
596+
And the HTTP status code should be "200"
597+
459598
Scenario: Do not allow reshare to exceed permissions
460599
Given user "user0" exists
461600
And user "user1" exists
@@ -477,6 +616,95 @@ Feature: sharing
477616
| permissions | 31 |
478617
Then the OCS status code should be "404"
479618

619+
Scenario: Do not allow reshare to exceed permissions even if shares of same file to other users have them
620+
Given user "user0" exists
621+
And user "user1" exists
622+
And user "user2" exists
623+
And user "user3" exists
624+
And user "user0" created a folder "/TMP"
625+
And As an "user0"
626+
And creating a share with
627+
| path | /TMP |
628+
| shareType | 0 |
629+
| shareWith | user3 |
630+
| permissions | 15 |
631+
And As an "user3"
632+
And As an "user0"
633+
And creating a share with
634+
| path | /TMP |
635+
| shareType | 0 |
636+
| shareWith | user1 |
637+
| permissions | 21 |
638+
And As an "user1"
639+
And creating a share with
640+
| path | /TMP |
641+
| shareType | 0 |
642+
| shareWith | user2 |
643+
| permissions | 21 |
644+
When Updating last share with
645+
| permissions | 31 |
646+
Then the OCS status code should be "404"
647+
And the HTTP status code should be "200"
648+
649+
Scenario: Do not allow reshare to exceed permissions even if shares of other files from same user have them
650+
Given user "user0" exists
651+
And user "user1" exists
652+
And user "user2" exists
653+
And As an "user0"
654+
And creating a share with
655+
| path | /FOLDER |
656+
| shareType | 0 |
657+
| shareWith | user1 |
658+
| permissions | 15 |
659+
And As an "user1"
660+
And user "user0" created a folder "/TMP"
661+
And As an "user0"
662+
And creating a share with
663+
| path | /TMP |
664+
| shareType | 0 |
665+
| shareWith | user1 |
666+
| permissions | 21 |
667+
And As an "user1"
668+
And creating a share with
669+
| path | /TMP |
670+
| shareType | 0 |
671+
| shareWith | user2 |
672+
| permissions | 21 |
673+
When Updating last share with
674+
| permissions | 31 |
675+
Then the OCS status code should be "404"
676+
And the HTTP status code should be "200"
677+
678+
Scenario: Do not allow reshare to exceed permissions even if shares of other files from other users have them
679+
Given user "user0" exists
680+
And user "user1" exists
681+
And user "user2" exists
682+
And user "user3" exists
683+
And As an "user3"
684+
And creating a share with
685+
| path | /FOLDER |
686+
| shareType | 0 |
687+
| shareWith | user1 |
688+
| permissions | 15 |
689+
And As an "user1"
690+
And user "user0" created a folder "/TMP"
691+
And As an "user0"
692+
And creating a share with
693+
| path | /TMP |
694+
| shareType | 0 |
695+
| shareWith | user1 |
696+
| permissions | 21 |
697+
And As an "user1"
698+
And creating a share with
699+
| path | /TMP |
700+
| shareType | 0 |
701+
| shareWith | user2 |
702+
| permissions | 21 |
703+
When Updating last share with
704+
| permissions | 31 |
705+
Then the OCS status code should be "404"
706+
And the HTTP status code should be "200"
707+
480708
Scenario: Do not allow sub reshare to exceed permissions
481709
Given user "user0" exists
482710
And user "user1" exists

‎lib/private/Files/Filesystem.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,10 @@ public static function initMountPoints($user = '') {
437437

438438
// home mounts are handled seperate since we need to ensure this is mounted before we call the other mount providers
439439
$homeMount = $mountConfigManager->getHomeMountForUser($userObject);
440+
if ($homeMount->getStorageRootId() === -1) {
441+
$homeMount->getStorage()->mkdir('');
442+
$homeMount->getStorage()->getScanner()->scan('');
443+
}
440444

441445
self::getMountManager()->addMount($homeMount);
442446

‎lib/private/Files/Mount/LocalHomeMountProvider.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class LocalHomeMountProvider implements IHomeMountProvider {
3535
*
3636
* @param IUser $user
3737
* @param IStorageFactory $loader
38-
* @return \OCP\Files\Mount\IMountPoint[]
38+
* @return \OCP\Files\Mount\IMountPoint|null
3939
*/
4040
public function getHomeMountForUser(IUser $user, IStorageFactory $loader) {
4141
$arguments = ['user' => $user];

‎lib/private/Files/Mount/MountPoint.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ public function getOptions() {
266266
* @return int
267267
*/
268268
public function getStorageRootId() {
269-
if (is_null($this->rootId)) {
269+
if (is_null($this->rootId) || $this->rootId === -1) {
270270
$this->rootId = (int)$this->getStorage()->getCache()->getId('');
271271
}
272272
return $this->rootId;

‎lib/private/Share20/Manager.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,13 +291,20 @@ protected function generalCreateChecks(\OCP\Share\IShare $share) {
291291

292292
$isFederatedShare = $share->getNode()->getStorage()->instanceOfStorage('\OCA\Files_Sharing\External\Storage');
293293
$permissions = 0;
294-
$mount = $share->getNode()->getMountPoint();
294+
295+
$userMounts = $userFolder->getById($share->getNode()->getId());
296+
$userMount = array_shift($userMounts);
297+
$mount = $userMount->getMountPoint();
295298
if (!$isFederatedShare && $share->getNode()->getOwner() && $share->getNode()->getOwner()->getUID() !== $share->getSharedBy()) {
296299
// When it's a reshare use the parent share permissions as maximum
297300
$userMountPointId = $mount->getStorageRootId();
298301
$userMountPoints = $userFolder->getById($userMountPointId);
299302
$userMountPoint = array_shift($userMountPoints);
300303

304+
if ($userMountPoint === null) {
305+
throw new GenericShareException('Could not get proper user mount for ' . $userMountPointId . '. Failing since else the next calls are called with null');
306+
}
307+
301308
/* Check if this is an incoming share */
302309
$incomingShares = $this->getSharedWith($share->getSharedBy(), Share::SHARE_TYPE_USER, $userMountPoint, -1, 0);
303310
$incomingShares = array_merge($incomingShares, $this->getSharedWith($share->getSharedBy(), Share::SHARE_TYPE_GROUP, $userMountPoint, -1, 0));

‎tests/lib/Share20/ManagerTest.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,7 @@ public function dataGeneralChecks() {
602602
$limitedPermssions = $this->createMock(File::class);
603603
$limitedPermssions->method('isShareable')->willReturn(true);
604604
$limitedPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ);
605+
$limitedPermssions->method('getId')->willReturn(108);
605606
$limitedPermssions->method('getPath')->willReturn('path');
606607
$limitedPermssions->method('getOwner')
607608
->willReturn($owner);
@@ -623,6 +624,7 @@ public function dataGeneralChecks() {
623624
$nonMoveableMountPermssions = $this->createMock(Folder::class);
624625
$nonMoveableMountPermssions->method('isShareable')->willReturn(true);
625626
$nonMoveableMountPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ);
627+
$nonMoveableMountPermssions->method('getId')->willReturn(108);
626628
$nonMoveableMountPermssions->method('getPath')->willReturn('path');
627629
$nonMoveableMountPermssions->method('getOwner')
628630
->willReturn($owner);
@@ -644,6 +646,7 @@ public function dataGeneralChecks() {
644646
$allPermssions = $this->createMock(Folder::class);
645647
$allPermssions->method('isShareable')->willReturn(true);
646648
$allPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_ALL);
649+
$allPermssions->method('getId')->willReturn(108);
647650
$allPermssions->method('getOwner')
648651
->willReturn($owner);
649652
$allPermssions->method('getStorage')
@@ -664,6 +667,7 @@ public function dataGeneralChecks() {
664667
$remoteFile = $this->createMock(Folder::class);
665668
$remoteFile->method('isShareable')->willReturn(true);
666669
$remoteFile->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ ^ \OCP\Constants::PERMISSION_UPDATE);
670+
$remoteFile->method('getId')->willReturn(108);
667671
$remoteFile->method('getOwner')
668672
->willReturn($owner);
669673
$remoteFile->method('getStorage')
@@ -698,6 +702,11 @@ public function testGeneralChecks($share, $exceptionMessage, $exception) {
698702
$userFolder->expects($this->any())
699703
->method('getId')
700704
->willReturn(42);
705+
// Id 108 is used in the data to refer to the node of the share.
706+
$userFolder->expects($this->any())
707+
->method('getById')
708+
->with(108)
709+
->willReturn([$share->getNode()]);
701710
$userFolder->expects($this->any())
702711
->method('getRelativePath')
703712
->willReturnArgument(0);

‎tests/lib/TestCase.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ protected function isFileLocked($view, $path, $type, $onMountPoint = false) {
441441
}
442442
}
443443

444-
private function IsDatabaseAccessAllowed() {
444+
protected function IsDatabaseAccessAllowed() {
445445
// on travis-ci.org we allow database access in any case - otherwise
446446
// this will break all apps right away
447447
if (true == getenv('TRAVIS')) {

‎tests/lib/Traits/MountProviderTrait.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ protected function registerMount($userId, $storage, $mountPoint, $arguments = nu
3333
$this->mounts[$userId] = [];
3434
}
3535
$this->mounts[$userId][] = ['storage' => $storage, 'mountPoint' => $mountPoint, 'arguments' => $arguments];
36+
37+
if ($this->IsDatabaseAccessAllowed()) {
38+
$mount = new MountPoint($storage, $mountPoint, $arguments, $this->storageFactory);
39+
$storage = $mount->getStorage();
40+
$storage->getScanner()->scan('');
41+
}
3642
}
3743

3844
protected function registerStorageWrapper($name, $wrapper) {

0 commit comments

Comments
 (0)