Proper message shown when private links accessed

When user tries to access private links which are not accessible, then proper message is delivered instead of Internal server error message. So is the case when user is logged in and tries to access private links not accessible. Signed-off-by: Sujith H <[email protected]>
owncloud · Jul 24, 2017 · a6ff748 · a6ff748
1 parent 26631fd
commit a6ff748
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 4 deletions.
diff --git a/apps/files/lib/Controller/ViewController.php b/apps/files/lib/Controller/ViewController.php
@@ -281,9 +281,12 @@ public function showFile($fileId) {
 		$params = [];
 
 		if (empty($files) && $this->appManager->isEnabledForUser('files_trashbin')) {
-			$baseFolder = $this->rootFolder->get($uid . '/files_trashbin/files/');
-			$files = $baseFolder->getById($fileId);
-			$params['view'] = 'trashbin';
+			// Access files_trashbin if it exists
+			if ( $this->rootFolder->nodeExists($uid . '/files_trashbin/files/')) {
+				$baseFolder = $this->rootFolder->get($uid . '/files_trashbin/files/');
+				$files = $baseFolder->getById($fileId);
+				$params['view'] = 'trashbin';
+			}
 		}
 
 		if (!empty($files)) {
@@ -299,6 +302,13 @@ public function showFile($fileId) {
 			}
 			return new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', $params));
 		}
+
+		if ( $this->userSession->isLoggedIn() and empty($files)) {
+			$l = \OC::$server->getL10N("core");
+			$param["error"] = $l->t("You don't have permissions to access this file/folder - Please contact the owner to share it with you.");
+			return new TemplateResponse("core", 'error', ["errors" => [$param]], 'guest');
+		}
+
 		throw new \OCP\Files\NotFoundException();
 	}
 }
diff --git a/apps/files/tests/Controller/ViewControllerTest.php b/apps/files/tests/Controller/ViewControllerTest.php
@@ -428,6 +428,10 @@ public function testShowFileRouteWithTrashedFile($useShowFile) {
 			->with('files_trashbin')
 			->will($this->returnValue(true));
 
+		$this->rootFolder->expects($this->once())
+			->method('nodeExists')
+			->will($this->returnValue(true));
+
 		$parentNode = $this->createMock('\OCP\Files\Folder');
 		$parentNode->expects($this->once())
 			->method('getPath')
@@ -440,7 +444,8 @@ public function testShowFileRouteWithTrashedFile($useShowFile) {
 			->method('get')
 			->with('testuser1/files/')
 			->will($this->returnValue($baseFolderFiles));
-		$this->rootFolder->expects($this->at(1))
+		//The index is pointing to 2, because nodeExists internally calls get method.
+		$this->rootFolder->expects($this->at(2))
 			->method('get')
 			->with('testuser1/files_trashbin/files/')
 			->will($this->returnValue($baseFolderTrash));

diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php
@@ -166,6 +166,21 @@ public function showLoginForm($user, $redirect_url, $remember_login) {
 			$parameters['user_autofocus'] = true;
 		}
 
+		/**
+		 * If redirect_url is not empty and remember_login is null and
+		 * user not logged in and check if the string
+		 * webroot+"/index.php/f/" is in redirect_url then
+		 * user is trying to access files for which he needs to login.
+		 */
+
+		if ((!empty($redirect_url)) and ($remember_login === null) and
+			($this->userSession->isLoggedIn() === false) and
+			strpos(urldecode($this->urlGenerator->getAbsoluteURL(urldecode($redirect_url))),
+				urldecode($this->urlGenerator->getAbsoluteURL('/index.php/f/'))) !== false) {
+
+			$parameters['accessLink'] = true;
+		}
+
 		return new TemplateResponse(
 			$this->appName, 'login', $parameters, 'guest'
 		);

diff --git a/core/templates/login.php b/core/templates/login.php
@@ -66,6 +66,11 @@
 				<?php p($l->t('Wrong password.')); ?>
 			</p>
 		<?php } ?>
+		<?php if (!empty($_['accessLink'])) { ?>
+			<p class="warning">
+				<?php p($l->t("You are trying to access a private link. Please log in first.")) ?>
+			</p>
+		<?php } ?>
 		<?php if ($_['rememberLoginAllowed'] === true) : ?>
 		<div class="remember-login-container">
 			<?php if ($_['rememberLoginState'] === 0) { ?>

diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php
@@ -137,6 +137,23 @@ public function testShowLoginFormForLoggedInUsers() {
 		$this->assertEquals($expectedResponse, $this->loginController->showLoginForm('', '', ''));
 	}
 
+	public function testResponseForNotLoggedinUser() {
+		$params = [
+			'messages' => Array (),
+			'loginName' => '',
+			'user_autofocus' => true,
+			'redirect_url' => '%2Findex.php%2Ff%2F17',
+			'canResetPassword' => true,
+			'resetPasswordLink' => null,
+			'alt_login' => Array (),
+			'rememberLoginAllowed' => false,
+			'rememberLoginState' => 0
+		];
+
+		$expectedResponse = new TemplateResponse('core', 'login', $params, 'guest');
+		$this->assertEquals($expectedResponse, $this->loginController->showLoginForm('', '%2Findex.php%2Ff%2F17', ''));
+	}
+
 	public function testShowLoginFormWithErrorsInSession() {
 		$this->userSession
 			->expects($this->once())