Properly catch whether a share is `null`

Despite it's PHPDoc the function might return `null` which was not properly catched and thus in some situations the share was resolved to the sharing users root directory.

To test this perform the following steps:

* Share file in owncloud 7 (7.0.4.2)
* Delete the parent folder of the shared file
* The share stays is in the DB and the share via the sharelink is inaccessible. (which is good)
* Upgrade to owncloud 8 (8.0.2) (This step is crucial. The bug is not reproduceable without upgrading from 7 to 8. It seems like the old tokens are handled different than the newer ones)
* Optional Step: Logout, Reset Browser Session, etc.
* Access the share via the old share url: almost empty page, but there is a dowload button which adds a "/download" to the URL.
* Upon clicking, a download.zip is downloaded which contains EVERYTHING from the owncloud directory (of the user who shared the file)
* No exception is thrown and no error is logged.

This will add a check whether the share is a valid one and also adds unit tests to prevent further regressions in the future. Needs to be backported to ownCloud 8.

Adding a proper clean-up of the orphaned shares is out-of-scope and would probably require some kind of FK or so.

Fixes https://github.com/owncloud/core/issues/15097

1 files changed, 60 insertions, 2 deletions

diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php
index 189fb57653c..204422853d0 100644
--- a/apps/files_sharing/tests/controller/sharecontroller.php
+++ b/apps/files_sharing/tests/controller/sharecontroller.php
@@ -12,6 +12,7 @@ namespace OCA\Files_Sharing\Controllers;
 
 use OC\Files\Filesystem;
 use OCA\Files_Sharing\Application;
+use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\AppFramework\IAppContainer;
 use OCP\Files;
 use OCP\AppFramework\Http\RedirectResponse;
@@ -49,6 +50,8 @@ class ShareControllerTest extends \Test\TestCase {
 			->disableOriginalConstructor()->getMock();
 		$this->container['URLGenerator'] = $this->getMockBuilder('\OC\URLGenerator')
 			->disableOriginalConstructor()->getMock();
+		$this->container['UserManager'] = $this->getMockBuilder('\OCP\IUserManager')
+			->disableOriginalConstructor()->getMock();
 		$this->urlGenerator = $this->container['URLGenerator'];
 		$this->shareController = $this->container['ShareController'];
 
@@ -115,7 +118,7 @@ class ShareControllerTest extends \Test\TestCase {
 	public function testAuthenticate() {
 		// Test without a not existing token
 		$response = $this->shareController->authenticate('ThisTokenShouldHopefullyNeverExistSoThatTheUnitTestWillAlwaysPass :)');
-		$expectedResponse =  new TemplateResponse('core', '404', array(), 'guest');
+		$expectedResponse =  new NotFoundResponse();
 		$this->assertEquals($expectedResponse, $response);
 
 		// Test with a valid password
@@ -130,9 +133,14 @@ class ShareControllerTest extends \Test\TestCase {
 	}
 
 	public function testShowShare() {
+		$this->container['UserManager']->expects($this->exactly(2))
+			->method('userExists')
+			->with($this->user)
+			->will($this->returnValue(true));
+
 		// Test without a not existing token
 		$response = $this->shareController->showShare('ThisTokenShouldHopefullyNeverExistSoThatTheUnitTestWillAlwaysPass :)');
-		$expectedResponse =  new TemplateResponse('core', '404', array(), 'guest');
+		$expectedResponse =  new NotFoundResponse();
 		$this->assertEquals($expectedResponse, $response);
 
 		// Test with a password protected share and no authentication
@@ -176,4 +184,54 @@ class ShareControllerTest extends \Test\TestCase {
 			array('token' => $this->token)));
 		$this->assertEquals($expectedResponse, $response);
 	}
+
+	/**
+	 * @expectedException \Exception
+	 * @expectedExceptionMessage No file found belonging to file.
+	 */
+	public function testShowShareWithDeletedFile() {
+		$this->container['UserManager']->expects($this->once())
+			->method('userExists')
+			->with($this->user)
+			->will($this->returnValue(true));
+
+		$view = new View('/'. $this->user . '/files');
+		$view->unlink('file1.txt');
+		$linkItem = Share::getShareByToken($this->token, false);
+		\OC::$server->getSession()->set('public_link_authenticated', $linkItem['id']);
+		$this->shareController->showShare($this->token);
+	}
+
+	/**
+	 * @expectedException \Exception
+	 * @expectedExceptionMessage No file found belonging to file.
+	 */
+	public function testDownloadShareWithDeletedFile() {
+		$this->container['UserManager']->expects($this->once())
+			->method('userExists')
+			->with($this->user)
+			->will($this->returnValue(true));
+
+		$view = new View('/'. $this->user . '/files');
+		$view->unlink('file1.txt');
+		$linkItem = Share::getShareByToken($this->token, false);
+		\OC::$server->getSession()->set('public_link_authenticated', $linkItem['id']);
+		$this->shareController->downloadShare($this->token);
+	}
+
+	/**
+	 * @expectedException \Exception
+	 * @expectedExceptionMessage Owner of the share does not exist anymore
+	 */
+	public function testShowShareWithNotExistingUser() {
+		$this->container['UserManager']->expects($this->once())
+			->method('userExists')
+			->with($this->user)
+			->will($this->returnValue(false));
+
+		$linkItem = Share::getShareByToken($this->token, false);
+		\OC::$server->getSession()->set('public_link_authenticated', $linkItem['id']);
+		$this->shareController->showShare($this->token);
+	}
+
 }