diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 1f8864fc5f..f793d35e3a 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 6dc577a354..a977a422e7 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 97a8621fc3..d357fd45f9 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 ef3d07ae49..dd5340d637 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 5c5d23887c..4b7dd08c83 100644 --- a/tests/acceptance/features/bootstrap/FilesSharingAppContext.php +++ b/tests/acceptance/features/bootstrap/FilesSharingAppContext.php @@ -109,6 +109,13 @@ class FilesSharingAppContext implements Context, ActorAwareInterface { $this->actor->getSession()->visit($this->actor->getSharedNotebook()["shared link"]); } + /** + * @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 */ @@ -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()); } @@ -142,6 +158,15 @@ class FilesSharingAppContext implements Context, ActorAwareInterface { $this->actor->getSession()->getCurrentUrl()); } + /** + * @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 */