Merge pull request #8590 from nextcloud/redirect-to-download-after-share

Sharing: redirect to download after authentification if requested

5 files changed, 91 insertions, 14 deletions

diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php
index 1f8864fc5f3..f793d35e3ae 100644
--- a/apps/files_sharing/lib/Controller/ShareController.php
+++ b/apps/files_sharing/lib/Controller/ShareController.php
@@ -170,10 +170,11 @@ class ShareController extends Controller {
 	 *
 	 * Authenticates against password-protected shares
 	 * @param string $token
+	 * @param string $redirect
 	 * @param string $password
 	 * @return RedirectResponse|TemplateResponse|NotFoundResponse
 	 */
-	public function authenticate($token, $password = '') {
+	public function authenticate($token, $redirect, $password = '') {
 
 		// Check whether share exists
 		try {
@@ -184,8 +185,17 @@ class ShareController extends Controller {
 
 		$authenticate = $this->linkShareAuth($share, $password);
 
-		if($authenticate === true) {
-			return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.showShare', array('token' => $token)));
+		// if download was requested before auth, redirect to download
+		if ($authenticate === true && $redirect === 'download') {
+			return new RedirectResponse($this->urlGenerator->linkToRoute(
+				'files_sharing.sharecontroller.downloadShare',
+				array('token' => $token))
+			);
+		} else if ($authenticate === true) {
+			return new RedirectResponse($this->urlGenerator->linkToRoute(
+				'files_sharing.sharecontroller.showShare',
+				array('token' => $token))
+			);
 		}
 
 		$response = new TemplateResponse($this->appName, 'authenticate', array('wrongpw' => true), 'guest');
@@ -294,7 +304,7 @@ class ShareController extends Controller {
 		// Share is password protected - check whether the user is permitted to access the share
 		if ($share->getPassword() !== null && !$this->linkShareAuth($share)) {
 			return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate',
-				array('token' => $token)));
+				array('token' => $token, 'redirect' => 'preview')));
 		}
 
 		if (!$this->validateShare($share)) {
@@ -480,7 +490,7 @@ class ShareController extends Controller {
 		// Share is password protected - check whether the user is permitted to access the share
 		if ($share->getPassword() !== null && !$this->linkShareAuth($share)) {
 			return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate',
-				['token' => $token]));
+				['token' => $token, 'redirect' => 'download']));
 		}
 
 		$files_list = null;
diff --git a/apps/files_sharing/tests/Controller/ShareControllerTest.php b/apps/files_sharing/tests/Controller/ShareControllerTest.php
index 6dc577a354c..a977a422e7d 100644
--- a/apps/files_sharing/tests/Controller/ShareControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareControllerTest.php
@@ -218,7 +218,7 @@ class ShareControllerTest extends \Test\TestCase {
 			->with('token')
 			->will($this->throwException(new \OCP\Share\Exceptions\ShareNotFound()));
 
-		$response = $this->shareController->authenticate('token');
+		$response = $this->shareController->authenticate('token', 'preview');
 		$expectedResponse =  new NotFoundResponse();
 		$this->assertEquals($expectedResponse, $response);
 	}
@@ -249,7 +249,38 @@ class ShareControllerTest extends \Test\TestCase {
 			->with('files_sharing.sharecontroller.showShare', ['token'=>'token'])
 			->willReturn('redirect');
 
-		$response = $this->shareController->authenticate('token', 'validpassword');
+		$response = $this->shareController->authenticate('token', 'preview', 'validpassword');
+		$expectedResponse =  new RedirectResponse('redirect');
+		$this->assertEquals($expectedResponse, $response);
+	}
+
+	public function testAuthenticateValidPasswordAndDownload() {
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setId(42);
+
+		$this->shareManager
+			->expects($this->once())
+			->method('getShareByToken')
+			->with('token')
+			->willReturn($share);
+
+		$this->shareManager
+			->expects($this->once())
+			->method('checkPassword')
+			->with($share, 'validpassword')
+			->willReturn(true);
+
+		$this->session
+			->expects($this->once())
+			->method('set')
+			->with('public_link_authenticated', '42');
+
+		$this->urlGenerator->expects($this->once())
+			->method('linkToRoute')
+			->with('files_sharing.sharecontroller.downloadShare', ['token'=>'token'])
+			->willReturn('redirect');
+
+		$response = $this->shareController->authenticate('token', 'download', 'validpassword');
 		$expectedResponse =  new RedirectResponse('redirect');
 		$this->assertEquals($expectedResponse, $response);
 	}
@@ -292,7 +323,7 @@ class ShareControllerTest extends \Test\TestCase {
 					$data['errorMessage'] === 'Wrong password';
 			}));
 
-		$response = $this->shareController->authenticate('token', 'invalidpassword');
+		$response = $this->shareController->authenticate('token', 'preview', 'invalidpassword');
 		$expectedResponse =  new TemplateResponse($this->appName, 'authenticate', array('wrongpw' => true), 'guest');
 		$expectedResponse->throttle();
 		$this->assertEquals($expectedResponse, $response);
@@ -323,7 +354,7 @@ class ShareControllerTest extends \Test\TestCase {
 
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRoute')
-			->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken'])
+			->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken', 'redirect' => 'preview'])
 			->willReturn('redirect');
 
 		// Test without a not existing token
@@ -505,7 +536,7 @@ class ShareControllerTest extends \Test\TestCase {
 
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRoute')
-			->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken'])
+			->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken', 'redirect' => 'download'])
 			->willReturn('redirect');
 
 		// Test with a password protected share and no authentication
@@ -533,5 +564,4 @@ class ShareControllerTest extends \Test\TestCase {
 		$expectedResponse = new DataResponse('Share is read-only');
 		$this->assertEquals($expectedResponse, $response);
 	}
-
 }
diff --git a/core/routes.php b/core/routes.php
index 97a8621fc39..d357fd45f96 100644
--- a/core/routes.php
+++ b/core/routes.php
@@ -116,7 +116,7 @@ $this->create('files_sharing.sharecontroller.showShare', '/s/{token}')->action(f
 		throw new \OC\HintException('App file sharing is not enabled');
 	}
 });
-$this->create('files_sharing.sharecontroller.authenticate', '/s/{token}/authenticate')->post()->action(function($urlParams) {
+$this->create('files_sharing.sharecontroller.authenticate', '/s/{token}/authenticate/{redirect}')->post()->action(function($urlParams) {
 	if (class_exists(\OCA\Files_Sharing\AppInfo\Application::class, false)) {
 		$app = new \OCA\Files_Sharing\AppInfo\Application($urlParams);
 		$app->dispatch('ShareController', 'authenticate');
@@ -124,7 +124,7 @@ $this->create('files_sharing.sharecontroller.authenticate', '/s/{token}/authenti
 		throw new \OC\HintException('App file sharing is not enabled');
 	}
 });
-$this->create('files_sharing.sharecontroller.showAuthenticate', '/s/{token}/authenticate')->get()->action(function($urlParams) {
+$this->create('files_sharing.sharecontroller.showAuthenticate', '/s/{token}/authenticate/{redirect}')->get()->action(function($urlParams) {
 	if (class_exists(\OCA\Files_Sharing\AppInfo\Application::class, false)) {
 		$app = new \OCA\Files_Sharing\AppInfo\Application($urlParams);
 		$app->dispatch('ShareController', 'showAuthenticate');
diff --git a/tests/acceptance/features/app-files.feature b/tests/acceptance/features/app-files.feature
index ef3d07ae499..dd5340d6374 100644
--- a/tests/acceptance/features/app-files.feature
+++ b/tests/acceptance/features/app-files.feature
@@ -71,6 +71,18 @@ Feature: app-files
     Then I see that the current page is the Authenticate page for the shared link I wrote down
     And I see that a wrong password for the shared file message is shown
 
+  Scenario: access a direct download shared link protected by password with a valid password
+    Given I act as John
+    And I am logged in
+    And I share the link for "welcome.txt" protected by the password "abcdef"
+    And I write down the shared link
+    When I act as Jane
+    And I visit the direct download shared link I wrote down
+    And I see that the current page is the Authenticate page for the direct download shared link I wrote down
+    And I authenticate with password "abcdef"
+    # download starts no page redirection
+    And I see that the current page is the Authenticate page for the direct download shared link I wrote down
+
   Scenario: show the input field for tags in the details view
     Given I am logged in
     And I open the details view for "welcome.txt"
diff --git a/tests/acceptance/features/bootstrap/FilesSharingAppContext.php b/tests/acceptance/features/bootstrap/FilesSharingAppContext.php
index 5c5d23887cd..4b7dd08c83e 100644
--- a/tests/acceptance/features/bootstrap/FilesSharingAppContext.php
+++ b/tests/acceptance/features/bootstrap/FilesSharingAppContext.php
@@ -110,6 +110,13 @@ class FilesSharingAppContext implements Context, ActorAwareInterface {
 	}
 
 	/**
+	 * @When I visit the direct download shared link I wrote down
+	 */
+	public function iVisitTheDirectDownloadSharedLinkIWroteDown() {
+		$this->actor->getSession()->visit($this->actor->getSharedNotebook()["shared link"] . "/download");
+	}
+
+	/**
 	 * @When I authenticate with password :password
 	 */
 	public function iAuthenticateWithPassword($password) {
@@ -129,7 +136,16 @@ class FilesSharingAppContext implements Context, ActorAwareInterface {
 	 */
 	public function iSeeThatTheCurrentPageIsTheAuthenticatePageForTheSharedLinkIWroteDown() {
 		PHPUnit_Framework_Assert::assertEquals(
-				$this->actor->getSharedNotebook()["shared link"] . "/authenticate",
+				$this->actor->getSharedNotebook()["shared link"] . "/authenticate/preview",
+				$this->actor->getSession()->getCurrentUrl());
+	}
+
+	/**
+	 * @Then I see that the current page is the Authenticate page for the direct download shared link I wrote down
+	 */
+	public function iSeeThatTheCurrentPageIsTheAuthenticatePageForTheDirectDownloadSharedLinkIWroteDown() {
+		PHPUnit_Framework_Assert::assertEquals(
+				$this->actor->getSharedNotebook()["shared link"] . "/authenticate/download",
 				$this->actor->getSession()->getCurrentUrl());
 	}
 
@@ -143,6 +159,15 @@ class FilesSharingAppContext implements Context, ActorAwareInterface {
 	}
 
 	/**
+	 * @Then I see that the current page is the direct download shared link I wrote down
+	 */
+	public function iSeeThatTheCurrentPageIsTheDirectDownloadSharedLinkIWroteDown() {
+		PHPUnit_Framework_Assert::assertEquals(
+				$this->actor->getSharedNotebook()["shared link"] . "/download",
+				$this->actor->getSession()->getCurrentUrl());
+	}
+
+	/**
 	 * @Then I see that a wrong password for the shared file message is shown
 	 */
 	public function iSeeThatAWrongPasswordForTheSharedFileMessageIsShown() {