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

Sharing: redirect to download after authentification if requested
This commit is contained in:
Roeland Jago Douma 2018-03-05 11:11:43 +01:00 committed by GitHub
commit 05ef2d70e7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 91 additions and 14 deletions

View File

@ -170,10 +170,11 @@ class ShareController extends Controller {
* *
* Authenticates against password-protected shares * Authenticates against password-protected shares
* @param string $token * @param string $token
* @param string $redirect
* @param string $password * @param string $password
* @return RedirectResponse|TemplateResponse|NotFoundResponse * @return RedirectResponse|TemplateResponse|NotFoundResponse
*/ */
public function authenticate($token, $password = '') { public function authenticate($token, $redirect, $password = '') {
// Check whether share exists // Check whether share exists
try { try {
@ -184,8 +185,17 @@ class ShareController extends Controller {
$authenticate = $this->linkShareAuth($share, $password); $authenticate = $this->linkShareAuth($share, $password);
if($authenticate === true) { // if download was requested before auth, redirect to download
return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.showShare', array('token' => $token))); 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'); $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 // Share is password protected - check whether the user is permitted to access the share
if ($share->getPassword() !== null && !$this->linkShareAuth($share)) { if ($share->getPassword() !== null && !$this->linkShareAuth($share)) {
return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate', return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate',
array('token' => $token))); array('token' => $token, 'redirect' => 'preview')));
} }
if (!$this->validateShare($share)) { 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 // Share is password protected - check whether the user is permitted to access the share
if ($share->getPassword() !== null && !$this->linkShareAuth($share)) { if ($share->getPassword() !== null && !$this->linkShareAuth($share)) {
return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate', return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate',
['token' => $token])); ['token' => $token, 'redirect' => 'download']));
} }
$files_list = null; $files_list = null;

View File

@ -218,7 +218,7 @@ class ShareControllerTest extends \Test\TestCase {
->with('token') ->with('token')
->will($this->throwException(new \OCP\Share\Exceptions\ShareNotFound())); ->will($this->throwException(new \OCP\Share\Exceptions\ShareNotFound()));
$response = $this->shareController->authenticate('token'); $response = $this->shareController->authenticate('token', 'preview');
$expectedResponse = new NotFoundResponse(); $expectedResponse = new NotFoundResponse();
$this->assertEquals($expectedResponse, $response); $this->assertEquals($expectedResponse, $response);
} }
@ -249,7 +249,38 @@ class ShareControllerTest extends \Test\TestCase {
->with('files_sharing.sharecontroller.showShare', ['token'=>'token']) ->with('files_sharing.sharecontroller.showShare', ['token'=>'token'])
->willReturn('redirect'); ->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'); $expectedResponse = new RedirectResponse('redirect');
$this->assertEquals($expectedResponse, $response); $this->assertEquals($expectedResponse, $response);
} }
@ -292,7 +323,7 @@ class ShareControllerTest extends \Test\TestCase {
$data['errorMessage'] === 'Wrong password'; $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 = new TemplateResponse($this->appName, 'authenticate', array('wrongpw' => true), 'guest');
$expectedResponse->throttle(); $expectedResponse->throttle();
$this->assertEquals($expectedResponse, $response); $this->assertEquals($expectedResponse, $response);
@ -323,7 +354,7 @@ class ShareControllerTest extends \Test\TestCase {
$this->urlGenerator->expects($this->once()) $this->urlGenerator->expects($this->once())
->method('linkToRoute') ->method('linkToRoute')
->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken']) ->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken', 'redirect' => 'preview'])
->willReturn('redirect'); ->willReturn('redirect');
// Test without a not existing token // Test without a not existing token
@ -505,7 +536,7 @@ class ShareControllerTest extends \Test\TestCase {
$this->urlGenerator->expects($this->once()) $this->urlGenerator->expects($this->once())
->method('linkToRoute') ->method('linkToRoute')
->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken']) ->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken', 'redirect' => 'download'])
->willReturn('redirect'); ->willReturn('redirect');
// Test with a password protected share and no authentication // 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'); $expectedResponse = new DataResponse('Share is read-only');
$this->assertEquals($expectedResponse, $response); $this->assertEquals($expectedResponse, $response);
} }
} }

View File

@ -116,7 +116,7 @@ $this->create('files_sharing.sharecontroller.showShare', '/s/{token}')->action(f
throw new \OC\HintException('App file sharing is not enabled'); 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)) { if (class_exists(\OCA\Files_Sharing\AppInfo\Application::class, false)) {
$app = new \OCA\Files_Sharing\AppInfo\Application($urlParams); $app = new \OCA\Files_Sharing\AppInfo\Application($urlParams);
$app->dispatch('ShareController', 'authenticate'); $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'); 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)) { if (class_exists(\OCA\Files_Sharing\AppInfo\Application::class, false)) {
$app = new \OCA\Files_Sharing\AppInfo\Application($urlParams); $app = new \OCA\Files_Sharing\AppInfo\Application($urlParams);
$app->dispatch('ShareController', 'showAuthenticate'); $app->dispatch('ShareController', 'showAuthenticate');

View File

@ -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 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 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 Scenario: show the input field for tags in the details view
Given I am logged in Given I am logged in
And I open the details view for "welcome.txt" And I open the details view for "welcome.txt"

View File

@ -109,6 +109,13 @@ class FilesSharingAppContext implements Context, ActorAwareInterface {
$this->actor->getSession()->visit($this->actor->getSharedNotebook()["shared link"]); $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 * @When I authenticate with password :password
*/ */
@ -129,7 +136,16 @@ class FilesSharingAppContext implements Context, ActorAwareInterface {
*/ */
public function iSeeThatTheCurrentPageIsTheAuthenticatePageForTheSharedLinkIWroteDown() { public function iSeeThatTheCurrentPageIsTheAuthenticatePageForTheSharedLinkIWroteDown() {
PHPUnit_Framework_Assert::assertEquals( 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()); $this->actor->getSession()->getCurrentUrl());
} }
@ -142,6 +158,15 @@ class FilesSharingAppContext implements Context, ActorAwareInterface {
$this->actor->getSession()->getCurrentUrl()); $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 * @Then I see that a wrong password for the shared file message is shown
*/ */