fix(Share20\Manager): Ensure node is still accessible when checking share

Signed-off-by: provokateurin <kate@provokateurin.de>

lib/private/Share20/Manager.php

@@ -1488,6 +1488,15 @@ class Manager implements IManager {
 			$this->deleteShare($share);
 			throw new ShareNotFound($this->l->t('The requested share does not exist anymore'));
 		}
+
+		try {
+			$share->getNode();
+			// Ignore share, file is still accessible
+		} catch (NotFoundException) {
+			// Access lost, but maybe only temporarily, so don't delete the share right away
+			throw new ShareNotFound($this->l->t('The requested share does not exist anymore'));
+		}
+
 		if ($this->config->getAppValue('files_sharing', 'hide_disabled_user_shares', 'no') === 'yes') {
 			$uids = array_unique([$share->getShareOwner(),$share->getSharedBy()]);
 			foreach ($uids as $uid) {

tests/lib/Share20/ManagerTest.php

@@ -25,6 +25,7 @@ use OCP\Files\Mount\IMountManager;
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Mount\IShareOwnerlessMount;
 use OCP\Files\Node;
+use OCP\Files\NotFoundException;
 use OCP\Files\Storage\IStorage;
 use OCP\HintException;
 use OCP\IConfig;
@@ -667,6 +668,24 @@ class ManagerTest extends \Test\TestCase {
 	}
 
 
+	public function testGetShareByIdNodeAccessible(): void {
+		$share = $this->createMock(IShare::class);
+		$share
+			->expects($this->once())
+			->method('getNode')
+			->willThrowException(new NotFoundException());
+
+		$this->defaultProvider
+			->expects($this->once())
+			->method('getShareById')
+			->with(42)
+			->willReturn($share);
+
+		$this->expectException(ShareNotFound::class);
+		$this->manager->getShareById('default:42');
+	}
+
+
 	public function testGetExpiredShareById(): void {
 		$this->expectException(\OCP\Share\Exceptions\ShareNotFound::class);
 
@@ -2875,10 +2894,11 @@ class ManagerTest extends \Test\TestCase {
 	}
 
 	public function testGetSharesBy(): void {
-		$share = $this->manager->newShare();
-
 		$node = $this->createMock(Folder::class);
 
+		$share = $this->manager->newShare();
+		$share->setNode($node);
+
 		$this->defaultProvider->expects($this->once())
 			->method('getSharesBy')
 			->with(
@@ -2930,6 +2950,8 @@ class ManagerTest extends \Test\TestCase {
 	 * deleted (as they are evaluated). but share 8 should still be there.
 	 */
 	public function testGetSharesByExpiredLinkShares(): void {
+		$node = $this->createMock(File::class);
+
 		$manager = $this->createManagerMock()
 			->setMethods(['deleteShare'])
 			->getMock();
@@ -2943,6 +2965,7 @@ class ManagerTest extends \Test\TestCase {
 		for ($i = 0; $i < 8; $i++) {
 			$share = $this->manager->newShare();
 			$share->setId($i);
+			$share->setNode($node);
 			$shares[] = $share;
 		}
 
@@ -2963,8 +2986,6 @@ class ManagerTest extends \Test\TestCase {
 			$shares2[] = clone $shares[$i];
 		}
 
-		$node = $this->createMock(File::class);
-
 		/*
 		 * Simulate the getSharesBy call.
 		 */
@@ -3126,8 +3147,10 @@ class ManagerTest extends \Test\TestCase {
 		$date = new \DateTime();
 		$date->setTime(0, 0, 0);
 		$date->add(new \DateInterval('P2D'));
+		$node = $this->createMock(File::class);
 		$share = $this->manager->newShare();
 		$share->setExpirationDate($date);
+		$share->setNode($node);
 		$share->setShareOwner('owner');
 		$share->setSharedBy('sharedBy');
 
@@ -3205,8 +3228,10 @@ class ManagerTest extends \Test\TestCase {
 		$date = new \DateTime();
 		$date->setTime(0, 0, 0);
 		$date->add(new \DateInterval('P2D'));
+		$node = $this->createMock(Folder::class);
 		$share = $this->manager->newShare();
 		$share->setExpirationDate($date);
+		$share->setNode($node);
 
 		$this->defaultProvider->expects($this->once())
 			->method('getShareByToken')