diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index 071f33998500..57e5d258399d 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -82,7 +82,7 @@ public function getMountsForUser(IUser $user, IStorageFactory $storageFactory) { $mounts = []; foreach ($superShares as $share) { try { - $mounts[] = new SharedMount( + $shareMount = new SharedMount( '\OCA\Files_Sharing\SharedStorage', $mounts, [ @@ -94,6 +94,13 @@ public function getMountsForUser(IUser $user, IStorageFactory $storageFactory) { ], $storageFactory ); + + $shareStorage = $shareMount->getStorage(); + if ($shareStorage !== null && $shareStorage->getPermissions('') !== 0) { + // permissions = 0 implies that the underlying storage + // being shared isn't valid + $mounts[] = $shareMount; + } } catch (\Exception $e) { $this->logger->logException($e); $this->logger->error('Error while trying to create shared mount'); diff --git a/apps/files_sharing/tests/MountProviderTest.php b/apps/files_sharing/tests/MountProviderTest.php index 2b390675fc99..32ad3bcd414e 100644 --- a/apps/files_sharing/tests/MountProviderTest.php +++ b/apps/files_sharing/tests/MountProviderTest.php @@ -22,6 +22,7 @@ namespace OCA\Files_Sharing\Tests; +use OC\Files\Storage\Storage; use OCA\Files_Sharing\MountProvider; use OCP\Files\Storage\IStorageFactory; use OCP\IConfig; @@ -167,6 +168,10 @@ public function testExcludeShares() { return new \OC\Share20\Share($rootFolder, $userManager); })); + $storage = $this->createMock(Storage::class); + $storage->method('getPermissions')->willReturn(1); + $this->loader->method('getInstance')->willReturn($storage); + $mounts = $this->provider->getMountsForUser($this->user, $this->loader); $this->assertCount(2, $mounts); @@ -191,6 +196,40 @@ public function testExcludeShares() { $this->assertEquals(true, $mountedShare2->getAttributes()->getAttribute('permission', 'download')); } + public function testShareFailedStorage() { + $rootFolder = $this->createMock('\OCP\Files\IRootFolder'); + $userManager = $this->createMock('\OCP\IUserManager'); + + $attr2 = [["scope" => "permission", "key" => "download", "enabled" => true]]; + $share = $this->makeMockShare(2, 100, 'user2', '/share2', 31, $attr2); + + $this->user->expects($this->any()) + ->method('getUID') + ->will($this->returnValue('user1')); + + $requiredShareTypes = [\OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_GROUP]; + $this->shareManager->expects($this->once()) + ->method('getAllSharedWith') + ->with('user1', $requiredShareTypes, null) + ->will($this->returnValue([$share])); + $this->shareManager->expects($this->never()) + ->method('getSharedWith'); + $this->shareManager->expects($this->any()) + ->method('newShare') + ->will($this->returnCallback(function () use ($rootFolder, $userManager) { + return new \OC\Share20\Share($rootFolder, $userManager); + })); + + $storage = $this->createMock(Storage::class); + $storage->method('getPermissions')->willReturn(0); + $this->loader->method('getInstance') + ->with($this->anything(), '\OCA\Files_Sharing\SharedStorage', $this->anything()) + ->willReturn($storage); + + $mounts = $this->provider->getMountsForUser($this->user, $this->loader); + $this->assertSame([], $mounts); + } + public function mergeSharesDataProvider() { // note: the user in the specs here is the shareOwner not recipient // the recipient is always "user1" @@ -373,6 +412,10 @@ public function testMergeShares($userShares, $groupShares, $expectedShares, $mov ->will($this->throwException(new \InvalidArgumentException())); } + $storage = $this->createMock(Storage::class); + $storage->method('getPermissions')->willReturn(1); + $this->loader->method('getInstance')->willReturn($storage); + $mounts = $this->provider->getMountsForUser($this->user, $this->loader); $this->assertCount(\count($expectedShares), $mounts); diff --git a/changelog/unreleased/41014 b/changelog/unreleased/41014 new file mode 100644 index 000000000000..32752d3d84dc --- /dev/null +++ b/changelog/unreleased/41014 @@ -0,0 +1,12 @@ +Bugfix: Do not mount shared storage which are failing + +Some mounts use a shared storage, which points to a different storage. +If the underlying storage is removed, the share mount was still being +present as if the underlying storage could still be accessed. This was +causing problems with the `occ files:remove-storage --show-candidates` +command because the removed storage wasn't shown as possible candidate. + +Now, that share storage won't be mounted, and the underlying storage +will be detected as a candidate to be removed with the command above. + +https://github.com/owncloud/core/pull/41014