From e40d21673ea4dfc000ce81839e88a0e53ddd8baa Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 14 Jan 2016 16:44:59 +0100 Subject: [PATCH 1/9] [Share 2.0] Add fetching link shares to share manager --- lib/private/share20/defaultshareprovider.php | 35 ++++++++++++++--- lib/private/share20/ishareprovider.php | 8 ++-- lib/private/share20/manager.php | 40 ++++++++++++++++++-- 3 files changed, 71 insertions(+), 12 deletions(-) diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php index a5afe9ff06b..d47919d21a3 100644 --- a/lib/private/share20/defaultshareprovider.php +++ b/lib/private/share20/defaultshareprovider.php @@ -284,7 +284,11 @@ class DefaultShareProvider implements IShareProvider { throw new ShareNotFound(); } - $share = $this->createShare($data); + try { + $share = $this->createShare($data); + } catch (InvalidShare $e) { + throw new ShareNotFound(); + } return $share; } @@ -328,13 +332,34 @@ class DefaultShareProvider implements IShareProvider { } /** - * Get a share by token and if present verify the password + * Get a share by token * * @param string $token - * @param string $password - * @param Share + * @return IShare + * @throws ShareNotFound */ - public function getShareByToken($token, $password = null) { + public function getShareByToken($token) { + $qb = $this->dbConn->getQueryBuilder(); + + $cursor = $qb->select('*') + ->from('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_LINK))) + ->andWhere($qb->expr()->eq('token', $qb->createNamedParameter($token))) + ->execute(); + + $data = $cursor->fetch(); + + if ($data === false) { + throw new ShareNotFound(); + } + + try { + $share = $this->createShare($data); + } catch (InvalidShare $e) { + throw new ShareNotFound(); + } + + return $share; } /** diff --git a/lib/private/share20/ishareprovider.php b/lib/private/share20/ishareprovider.php index d1f82557a5f..81770a45874 100644 --- a/lib/private/share20/ishareprovider.php +++ b/lib/private/share20/ishareprovider.php @@ -103,11 +103,11 @@ interface IShareProvider { public function getSharedWithMe(IUser $user, $shareType = null); /** - * Get a share by token and if present verify the password + * Get a share by token * * @param string $token - * @param string $password - * @param Share + * @return IShare + * @throws ShareNotFound */ - public function getShareByToken($token, $password = null); + public function getShareByToken($token); } diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index 035026b47ea..2be8fb5174d 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -665,13 +665,47 @@ class Manager { * Get the share by token possible with password * * @param string $token - * @param string $password - * * @return Share * * @throws ShareNotFound */ - public function getShareByToken($token, $password=null) { + public function getShareByToken($token) { + $provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_LINK); + + $share = $provider->getShareByToken($token); + + //TODO check if share expired + + return $share; + } + + /** + * Verify the password of a public share + * + * @param IShare $share + * @param string $password + * @return bool + */ + public function checkPassword(IShare $share, $password) { + if ($share->getShareType() !== \OCP\Share::SHARE_TYPE_LINK) { + //TODO maybe exception? + return false; + } + + if ($password === null || $share->getPassword() === null) { + return false; + } + + $newHash = ''; + if (!$this->hasher->verify($password, $share->getPassword(), $newHash)) { + return false; + } + + if (!empty($newHash)) { + //TODO update hash! + } + + return true; } /** From 0ebc92c44ab6332e74e88fb3a32e860139417677 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 15 Jan 2016 08:11:11 +0100 Subject: [PATCH 2/9] [Share 2.0] Move showing link share to sharing 2.0 --- apps/files_sharing/appinfo/application.php | 5 +- .../lib/controllers/sharecontroller.php | 121 +++++++++++++----- apps/files_sharing/templates/public.php | 2 +- 3 files changed, 91 insertions(+), 37 deletions(-) diff --git a/apps/files_sharing/appinfo/application.php b/apps/files_sharing/appinfo/application.php index 4f47027953a..8f953b236c4 100644 --- a/apps/files_sharing/appinfo/application.php +++ b/apps/files_sharing/appinfo/application.php @@ -53,7 +53,10 @@ class Application extends App { $c->query('URLGenerator'), $c->query('UserManager'), $server->getLogger(), - $server->getActivityManager() + $server->getActivityManager(), + $server->getShareManager(), + $server->getSession(), + $server->getPreviewManager() ); }); $container->registerService('ExternalSharesController', function (SimpleContainer $c) { diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php index 63ab49b4df3..bf4f297260d 100644 --- a/apps/files_sharing/lib/controllers/sharecontroller.php +++ b/apps/files_sharing/lib/controllers/sharecontroller.php @@ -45,10 +45,10 @@ use OC\URLGenerator; use OC\AppConfig; use OCP\ILogger; use OCA\Files_Sharing\Helper; -use OCP\User; use OCP\Util; use OCA\Files_Sharing\Activity; use \OCP\Files\NotFoundException; +use \OC\Share20\IShare; /** * Class ShareController @@ -71,6 +71,12 @@ class ShareController extends Controller { protected $logger; /** @var OCP\Activity\IManager */ protected $activityManager; + /** @var OC\Share20\Manager */ + protected $shareManager; + /** @var \OCP\ISession */ + protected $session; + /** @var \OCP\IPreview */ + protected $previewManager; /** * @param string $appName @@ -82,6 +88,9 @@ class ShareController extends Controller { * @param OCP\IUserManager $userManager * @param ILogger $logger * @param OCP\Activity\IManager $activityManager + * @param \OC\Share20\Manager $shareManager + * @param \OCP\ISession $session + * @param \OCP\IPreview $previewManager */ public function __construct($appName, IRequest $request, @@ -91,7 +100,10 @@ class ShareController extends Controller { URLGenerator $urlGenerator, OCP\IUserManager $userManager, ILogger $logger, - OCP\Activity\IManager $activityManager) { + OCP\Activity\IManager $activityManager, + \OC\Share20\Manager $shareManager, + \OCP\ISession $session, + \OCP\IPreview $previewManager) { parent::__construct($appName, $request); $this->userSession = $userSession; @@ -101,6 +113,9 @@ class ShareController extends Controller { $this->userManager = $userManager; $this->logger = $logger; $this->activityManager = $activityManager; + $this->shareManager = $shareManager; + $this->session = $session; + $this->previewManager = $previewManager; } /** @@ -111,9 +126,9 @@ class ShareController extends Controller { * @return TemplateResponse|RedirectResponse */ public function showAuthenticate($token) { - $linkItem = Share::getShareByToken($token, false); + $share = $this->shareManager->getShareByToken($token); - if(Helper::authenticate($linkItem)) { + if($this->linkShareAuth($share)) { return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.showShare', array('token' => $token))); } @@ -130,12 +145,15 @@ class ShareController extends Controller { * @return RedirectResponse|TemplateResponse */ public function authenticate($token, $password = '') { - $linkItem = Share::getShareByToken($token, false); - if($linkItem === false) { + + // Check whether share exists + try { + $share = $this->shareManager->getShareByToken($token); + } catch (\OC\Share20\Exception\ShareNotFound $e) { return new NotFoundResponse(); } - $authenticate = Helper::authenticate($linkItem, $password); + $authenticate = $this->linkShareAuth($share, $password); if($authenticate === true) { return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.showShare', array('token' => $token))); @@ -144,6 +162,34 @@ class ShareController extends Controller { return new TemplateResponse($this->appName, 'authenticate', array('wrongpw' => true), 'guest'); } + /** + * Authenticate a link item with the given password. + * Or use the session if no password is provided. + * + * This is a modified version of Helper::authenticate + * TODO: Try to merge back eventually with Helper::authenticate + * + * @param IShare $share + * @param string|null $password + * @return bool + */ + private function linkShareAuth(IShare $share, $password = null) { + if ($password !== null) { + if ($this->shareManager->checkPassword($share, $password)) { + $this->session->set('public_link_authenticated', (string)$share->getId()); + } else { + return false; + } + } else { + // not authenticated ? + if ( ! $this->session->exists('public_link_authenticated') + || $this->session->get('public_link_authenticated') !== (string)$share->getId()) { + return false; + } + } + return true; + } + /** * @PublicPage * @NoCSRFRequired @@ -157,54 +203,59 @@ class ShareController extends Controller { \OC_User::setIncognitoMode(true); // Check whether share exists - $linkItem = Share::getShareByToken($token, false); - if($linkItem === false) { + try { + $share = $this->shareManager->getShareByToken($token); + } catch (\OC\Share20\Exception\ShareNotFound $e) { return new NotFoundResponse(); } - $shareOwner = $linkItem['uid_owner']; - $originalSharePath = $this->getPath($token); - // Share is password protected - check whether the user is permitted to access the share - if (isset($linkItem['share_with']) && !Helper::authenticate($linkItem)) { + if ($share->getPassword() !== null && !$this->linkShareAuth($share)) { return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate', array('token' => $token))); } - if (Filesystem::isReadable($originalSharePath . $path)) { - $getPath = Filesystem::normalizePath($path); - $originalSharePath .= $path; - } else { + // We can't get the path of a file share + if ($share->getPath() instanceof \OCP\Files\File && $path !== '') { throw new NotFoundException(); } - $file = basename($originalSharePath); + $rootFolder = null; + if ($share->getPath() instanceof \OCP\Files\Folder) { + /** @var \OCP\Files\Folder $rootFolder */ + $rootFolder = $share->getPath(); + + try { + $path = $rootFolder->get($path); + } catch (\OCP\Files\NotFoundException $e) { + throw new NotFoundException(); + } + } $shareTmpl = []; - $shareTmpl['displayName'] = User::getDisplayName($shareOwner); - $shareTmpl['owner'] = $shareOwner; - $shareTmpl['filename'] = $file; - $shareTmpl['directory_path'] = $linkItem['file_target']; - $shareTmpl['mimetype'] = Filesystem::getMimeType($originalSharePath); - $shareTmpl['previewSupported'] = \OC::$server->getPreviewManager()->isMimeSupported($shareTmpl['mimetype']); - $shareTmpl['dirToken'] = $linkItem['token']; + $shareTmpl['displayName'] = $share->getShareOwner()->getDisplayName(); + $shareTmpl['owner'] = $share->getShareOwner()->getUID(); + $shareTmpl['filename'] = $share->getPath()->getName(); + $shareTmpl['directory_path'] = $share->getTarget(); + $shareTmpl['mimetype'] = $share->getPath()->getMimetype(); + $shareTmpl['previewSupported'] = $this->previewManager->isMimeSupported($share->getPath()->getMimetype()); + $shareTmpl['dirToken'] = $token; $shareTmpl['sharingToken'] = $token; $shareTmpl['server2serversharing'] = Helper::isOutgoingServer2serverShareEnabled(); - $shareTmpl['protected'] = isset($linkItem['share_with']) ? 'true' : 'false'; + $shareTmpl['protected'] = $share->getPassword() !== null ? 'true' : 'false'; $shareTmpl['dir'] = ''; - $nonHumanFileSize = \OC\Files\Filesystem::filesize($originalSharePath); - $shareTmpl['nonHumanFileSize'] = $nonHumanFileSize; - $shareTmpl['fileSize'] = \OCP\Util::humanFileSize($nonHumanFileSize); + $shareTmpl['nonHumanFileSize'] = $share->getPath()->getSize(); + $shareTmpl['fileSize'] = \OCP\Util::humanFileSize($share->getPath()->getSize()); // Show file list - if (Filesystem::is_dir($originalSharePath)) { - $shareTmpl['dir'] = $getPath; - $maxUploadFilesize = Util::maxUploadFilesize($originalSharePath); - $freeSpace = Util::freeSpace($originalSharePath); + if ($share->getPath() instanceof \OCP\Files\Folder) { + $shareTmpl['dir'] = $rootFolder->getRelativePath($path->getPath()); + $maxUploadFilesize = Util::maxUploadFilesize($share->getPath()->getPath()); + $freeSpace = Util::freeSpace($share->getPath()->getPath()); $uploadLimit = Util::uploadLimit(); $folder = new Template('files', 'list', ''); - $folder->assign('dir', $getPath); - $folder->assign('dirToken', $linkItem['token']); + $folder->assign('dir', $rootFolder->getRelativePath($path->getPath())); + $folder->assign('dirToken', $token); $folder->assign('permissions', \OCP\Constants::PERMISSION_READ); $folder->assign('isPublic', true); $folder->assign('publicUploadEnabled', 'no'); diff --git a/apps/files_sharing/templates/public.php b/apps/files_sharing/templates/public.php index aa1f926ea35..e6c4f57009f 100644 --- a/apps/files_sharing/templates/public.php +++ b/apps/files_sharing/templates/public.php @@ -26,7 +26,7 @@ $thumbSize = 1024; ?> - +
From 717697313b07106fd7941b83ff0271272c2c7791 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 15 Jan 2016 09:41:51 +0100 Subject: [PATCH 3/9] [Share 2.0] Move tests over the sharemanager Nice side effect... pure unit tests! --- apps/files_sharing/appinfo/application.php | 12 +- .../lib/controllers/sharecontroller.php | 51 ++-- .../tests/controller/sharecontroller.php | 250 ++++++++++++------ 3 files changed, 199 insertions(+), 114 deletions(-) diff --git a/apps/files_sharing/appinfo/application.php b/apps/files_sharing/appinfo/application.php index 8f953b236c4..5fa400db7d2 100644 --- a/apps/files_sharing/appinfo/application.php +++ b/apps/files_sharing/appinfo/application.php @@ -47,15 +47,13 @@ class Application extends App { return new ShareController( $c->query('AppName'), $c->query('Request'), - $c->query('UserSession'), - $server->getAppConfig(), $server->getConfig(), $c->query('URLGenerator'), $c->query('UserManager'), $server->getLogger(), $server->getActivityManager(), - $server->getShareManager(), - $server->getSession(), + $c->query('ShareManager'), + $c->query('Session'), $server->getPreviewManager() ); }); @@ -71,6 +69,12 @@ class Application extends App { /** * Core class wrappers */ + $container->registerService('Session', function(SimpleContainer $c) use ($server) { + return $server->getSession(); + }); + $container->registerService('ShareManager', function(SimpleContainer $c) use ($server) { + return $server->getShareManager(); + }); $container->registerService('UserSession', function (SimpleContainer $c) use ($server) { return $server->getUserSession(); }); diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php index bf4f297260d..f701a66dd0f 100644 --- a/apps/files_sharing/lib/controllers/sharecontroller.php +++ b/apps/files_sharing/lib/controllers/sharecontroller.php @@ -41,9 +41,12 @@ use OCP\IRequest; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\NotFoundResponse; -use OC\URLGenerator; -use OC\AppConfig; +use OCP\IURLGenerator; +use OCP\IConfig; use OCP\ILogger; +use OCP\IUserManager; +use OCP\ISession; +use OCP\IPreview; use OCA\Files_Sharing\Helper; use OCP\Util; use OCA\Files_Sharing\Activity; @@ -57,57 +60,47 @@ use \OC\Share20\IShare; */ class ShareController extends Controller { - /** @var \OC\User\Session */ - protected $userSession; - /** @var \OC\AppConfig */ - protected $appConfig; - /** @var \OCP\IConfig */ + /** @var IConfig */ protected $config; - /** @var \OC\URLGenerator */ + /** @var IURLGenerator */ protected $urlGenerator; - /** @var \OC\User\Manager */ + /** @var IUserManager */ protected $userManager; - /** @var \OCP\ILogger */ + /** @var ILogger */ protected $logger; /** @var OCP\Activity\IManager */ protected $activityManager; /** @var OC\Share20\Manager */ protected $shareManager; - /** @var \OCP\ISession */ + /** @var ISession */ protected $session; - /** @var \OCP\IPreview */ + /** @var IPreview */ protected $previewManager; /** * @param string $appName * @param IRequest $request - * @param OC\User\Session $userSession - * @param AppConfig $appConfig - * @param OCP\IConfig $config - * @param URLGenerator $urlGenerator - * @param OCP\IUserManager $userManager + * @param IConfig $config + * @param IURLGenerator $urlGenerator + * @param IUserManager $userManager * @param ILogger $logger * @param OCP\Activity\IManager $activityManager * @param \OC\Share20\Manager $shareManager - * @param \OCP\ISession $session - * @param \OCP\IPreview $previewManager + * @param ISession $session + * @param IPreview $previewManager */ public function __construct($appName, IRequest $request, - OC\User\Session $userSession, - AppConfig $appConfig, - OCP\IConfig $config, - URLGenerator $urlGenerator, - OCP\IUserManager $userManager, + IConfig $config, + IURLGenerator $urlGenerator, + IUserManager $userManager, ILogger $logger, - OCP\Activity\IManager $activityManager, + \OCP\Activity\IManager $activityManager, \OC\Share20\Manager $shareManager, - \OCP\ISession $session, - \OCP\IPreview $previewManager) { + ISession $session, + IPreview $previewManager) { parent::__construct($appName, $request); - $this->userSession = $userSession; - $this->appConfig = $appConfig; $this->config = $config; $this->urlGenerator = $urlGenerator; $this->userManager = $userManager; diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php index 914c98d9470..29dfbd1e8b1 100644 --- a/apps/files_sharing/tests/controller/sharecontroller.php +++ b/apps/files_sharing/tests/controller/sharecontroller.php @@ -30,15 +30,17 @@ namespace OCA\Files_Sharing\Controllers; use OC\Files\Filesystem; +use OC\Share20\Exception\ShareNotFound; use OCA\Files_Sharing\AppInfo\Application; use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\IAppContainer; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\ISession; use OCP\Security\ISecureRandom; use OC\Files\View; use OCP\Share; -use OC\URLGenerator; +use OCP\IURLGenerator; /** * @group DB @@ -57,21 +59,26 @@ class ShareControllerTest extends \Test\TestCase { private $oldUser; /** @var ShareController */ private $shareController; - /** @var URLGenerator */ + /** @var IURLGenerator */ private $urlGenerator; + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject */ + private $session; + /** @var \OC\Share20\Manager | \PHPUnit_Framework_MockObject_MockObject */ + private $shareManager; protected function setUp() { $app = new Application(); $this->container = $app->getContainer(); - $this->container['Config'] = $this->getMockBuilder('\OCP\IConfig') - ->disableOriginalConstructor()->getMock(); + $this->container['Config'] = $this->getMock('\OCP\IConfig'); $this->container['AppName'] = 'files_sharing'; - $this->container['UserSession'] = $this->getMockBuilder('\OC\User\Session') - ->disableOriginalConstructor()->getMock(); - $this->container['URLGenerator'] = $this->getMockBuilder('\OC\URLGenerator') - ->disableOriginalConstructor()->getMock(); - $this->container['UserManager'] = $this->getMockBuilder('\OCP\IUserManager') + $this->container['URLGenerator'] = $this->getMock('\OCP\IURLGenerator'); + $this->container['UserManager'] = $this->getMock('\OCP\IUserManager'); + $this->container['ShareManager'] = $this->getMockBuilder('\OC\Share20\Manager') ->disableOriginalConstructor()->getMock(); + $this->container['Session'] = $this->getMock('\OCP\ISession'); + + $this->session = $this->container['Session']; + $this->shareManager = $this->container['ShareManager']; $this->urlGenerator = $this->container['URLGenerator']; $this->shareController = $this->container['ShareController']; @@ -112,72 +119,184 @@ class ShareControllerTest extends \Test\TestCase { \OC_Util::setupFS($this->oldUser); } - public function testShowAuthenticate() { - $linkItem = \OCP\Share::getShareByToken($this->token, false); + public function testShowAuthenticateNotAuthenticated() { + $share = $this->getMock('\OC\Share20\IShare'); - // Test without being authenticated - $response = $this->shareController->showAuthenticate($this->token); - $expectedResponse = new TemplateResponse($this->container['AppName'], 'authenticate', array(), 'guest'); - $this->assertEquals($expectedResponse, $response); + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); - // Test with being authenticated for another file - \OC::$server->getSession()->set('public_link_authenticated', $linkItem['id']-1); - $response = $this->shareController->showAuthenticate($this->token); - $expectedResponse = new TemplateResponse($this->container['AppName'], 'authenticate', array(), 'guest'); - $this->assertEquals($expectedResponse, $response); - - // Test with being authenticated for the correct file - \OC::$server->getSession()->set('public_link_authenticated', $linkItem['id']); - $response = $this->shareController->showAuthenticate($this->token); - $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.showShare', array('token' => $this->token))); + $response = $this->shareController->showAuthenticate('token'); + $expectedResponse = new TemplateResponse($this->container['AppName'], 'authenticate', [], 'guest'); $this->assertEquals($expectedResponse, $response); } - public function testAuthenticate() { - // Test without a not existing token - $response = $this->shareController->authenticate('ThisTokenShouldHopefullyNeverExistSoThatTheUnitTestWillAlwaysPass :)'); + public function testShowAuthenticateAuthenticatedForDifferentShare() { + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getId')->willReturn(1); + + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); + $this->session->method('get')->with('public_link_authenticated')->willReturn('2'); + + $response = $this->shareController->showAuthenticate('token'); + $expectedResponse = new TemplateResponse($this->container['AppName'], 'authenticate', [], 'guest'); + $this->assertEquals($expectedResponse, $response); + } + + public function testShowAuthenticateCorrectShare() { + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getId')->willReturn(1); + + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); + $this->session->method('get')->with('public_link_authenticated')->willReturn('1'); + + $response = $this->shareController->showAuthenticate('token'); + $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.showShare', array('token' => 'token'))); + $this->assertEquals($expectedResponse, $response); + } + + public function testAutehnticateInvalidToken() { + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->will($this->throwException(new \OC\Share20\Exception\ShareNotFound())); + + $response = $this->shareController->authenticate('token'); $expectedResponse = new NotFoundResponse(); $this->assertEquals($expectedResponse, $response); + } - // Test with a valid password - $response = $this->shareController->authenticate($this->token, 'IAmPasswordProtected!'); - $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.showShare', array('token' => $this->token))); + public function testAuthenticateValidPassword() { + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getId')->willReturn(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'); + + $response = $this->shareController->authenticate('token', 'validpassword'); + $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.showShare', array('token' => 'token'))); $this->assertEquals($expectedResponse, $response); + } - // Test with a invalid password - $response = $this->shareController->authenticate($this->token, 'WrongPw!'); + public function testAuthenticateInvalidPassword() { + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getId')->willReturn(42); + + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + + $this->shareManager + ->expects($this->once()) + ->method('checkPassword') + ->with($share, 'invalidpassword') + ->willReturn(false); + + $this->session + ->expects($this->never()) + ->method('set'); + + $response = $this->shareController->authenticate('token', 'invalidpassword'); $expectedResponse = new TemplateResponse($this->container['AppName'], 'authenticate', array('wrongpw' => true), 'guest'); $this->assertEquals($expectedResponse, $response); } - public function testShowShare() { - $this->container['UserManager']->expects($this->exactly(2)) - ->method('userExists') - ->with($this->user) - ->will($this->returnValue(true)); + public function testShowShareInvalidToken() { + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('invalidtoken') + ->will($this->throwException(new ShareNotFound())); // Test without a not existing token - $response = $this->shareController->showShare('ThisTokenShouldHopefullyNeverExistSoThatTheUnitTestWillAlwaysPass :)'); + $response = $this->shareController->showShare('invalidtoken'); $expectedResponse = new NotFoundResponse(); $this->assertEquals($expectedResponse, $response); + } - // Test with a password protected share and no authentication - $response = $this->shareController->showShare($this->token); - $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate', array('token' => $this->token))); + public function testShowShareNotAuthenticated() { + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getPassword')->willReturn('password'); + + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('validtoken') + ->willReturn($share); + + // Test without a not existing token + $response = $this->shareController->showShare('validtoken'); + $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate', array('token' => 'validtoken'))); $this->assertEquals($expectedResponse, $response); + } - // Test with password protected share and authentication - $linkItem = Share::getShareByToken($this->token, false); - \OC::$server->getSession()->set('public_link_authenticated', $linkItem['id']); - $response = $this->shareController->showShare($this->token); + + public function testShowShare() { + $owner = $this->getMock('OCP\IUser'); + $owner->method('getDisplayName')->willReturn('ownerDisplay'); + $owner->method('getUID')->willReturn('ownerUID'); + + $file = $this->getMock('OCP\Files\File'); + $file->method('getName')->willReturn('file1.txt'); + $file->method('getMimetype')->willReturn('text/plain'); + $file->method('getSize')->willReturn(33); + + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getId')->willReturn('42'); + $share->method('getPassword')->willReturn('password'); + $share->method('getShareOwner')->willReturn($owner); + $share->method('getPath')->willReturn($file); + $share->method('getTarget')->willReturn('/file1.txt'); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); + $this->session->method('get')->with('public_link_authenticated')->willReturn('42'); + + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + + $response = $this->shareController->showShare('token'); $sharedTmplParams = array( - 'displayName' => $this->user, - 'owner' => $this->user, + 'displayName' => 'ownerDisplay', + 'owner' => 'ownerUID', 'filename' => 'file1.txt', 'directory_path' => '/file1.txt', 'mimetype' => 'text/plain', - 'dirToken' => $this->token, - 'sharingToken' => $this->token, + 'dirToken' => 'token', + 'sharingToken' => 'token', 'server2serversharing' => true, 'protected' => 'true', 'dir' => '', @@ -205,22 +324,6 @@ class ShareControllerTest extends \Test\TestCase { $this->assertEquals($expectedResponse, $response); } - /** - * @expectedException \OCP\Files\NotFoundException - */ - 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 \OCP\Files\NotFoundException */ @@ -237,19 +340,4 @@ class ShareControllerTest extends \Test\TestCase { $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); - } - } From 8734ebe505d621c355da0495e29692c56ca45226 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 15 Jan 2016 11:49:50 +0100 Subject: [PATCH 4/9] [Share 2.0] Make link share download use share manager --- apps/files_sharing/appinfo/application.php | 6 +- .../lib/controllers/sharecontroller.php | 116 ++++++++++-------- .../tests/controller/sharecontroller.php | 29 ++--- 3 files changed, 79 insertions(+), 72 deletions(-) diff --git a/apps/files_sharing/appinfo/application.php b/apps/files_sharing/appinfo/application.php index 5fa400db7d2..3971b04097f 100644 --- a/apps/files_sharing/appinfo/application.php +++ b/apps/files_sharing/appinfo/application.php @@ -54,7 +54,8 @@ class Application extends App { $server->getActivityManager(), $c->query('ShareManager'), $c->query('Session'), - $server->getPreviewManager() + $server->getPreviewManager(), + $c->query('RootFolder') ); }); $container->registerService('ExternalSharesController', function (SimpleContainer $c) { @@ -69,6 +70,9 @@ class Application extends App { /** * Core class wrappers */ + $container->registerService('RootFolder', function(SimpleContainer $c) use ($server) { + return $server->getRootFolder(); + }); $container->registerService('Session', function(SimpleContainer $c) use ($server) { return $server->getSession(); }); diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php index f701a66dd0f..2342a092c64 100644 --- a/apps/files_sharing/lib/controllers/sharecontroller.php +++ b/apps/files_sharing/lib/controllers/sharecontroller.php @@ -52,6 +52,7 @@ use OCP\Util; use OCA\Files_Sharing\Activity; use \OCP\Files\NotFoundException; use \OC\Share20\IShare; +use OCP\Files\IRootFolder; /** * Class ShareController @@ -76,6 +77,8 @@ class ShareController extends Controller { protected $session; /** @var IPreview */ protected $previewManager; + /** @var IRootFolder */ + protected $rootFolder; /** * @param string $appName @@ -98,7 +101,8 @@ class ShareController extends Controller { \OCP\Activity\IManager $activityManager, \OC\Share20\Manager $shareManager, ISession $session, - IPreview $previewManager) { + IPreview $previewManager, + IRootFolder $rootFolder) { parent::__construct($appName, $request); $this->config = $config; @@ -109,6 +113,7 @@ class ShareController extends Controller { $this->shareManager = $shareManager; $this->session = $session; $this->previewManager = $previewManager; + $this->rootFolder = $rootFolder; } /** @@ -286,14 +291,12 @@ class ShareController extends Controller { public function downloadShare($token, $files = null, $path = '', $downloadStartSecret = '') { \OC_User::setIncognitoMode(true); - $linkItem = OCP\Share::getShareByToken($token, false); + $share = $this->shareManager->getShareByToken($token); // Share is password protected - check whether the user is permitted to access the share - if (isset($linkItem['share_with'])) { - if(!Helper::authenticate($linkItem)) { - return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate', - array('token' => $token))); - } + if ($share->getPassword() !== null && !$this->linkShareAuth($share)) { + return new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate', + ['token' => $token])); } $files_list = null; @@ -301,41 +304,74 @@ class ShareController extends Controller { $files_list = json_decode($files); // in case we get only a single file if ($files_list === null) { - $files_list = array($files); + $files_list = [$files]; } } - $originalSharePath = self::getPath($token); + $userFolder = $this->rootFolder->getUserFolder($share->getShareOwner()->getUID()); + $originalSharePath = $userFolder->getRelativePath($share->getPath()->getPath()); - // Create the activities - if (isset($originalSharePath) && Filesystem::isReadable($originalSharePath . $path)) { - $originalSharePath = Filesystem::normalizePath($originalSharePath . $path); - $isDir = \OC\Files\Filesystem::is_dir($originalSharePath); + // Single file share + if ($share->getPath() instanceof \OCP\Files\File) { + $this->activityManager->publishActivity( + 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED, [$originalSharePath], '', [], + $originalSharePath, '', $share->getShareOwner()->getUID(), Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM + ); + } + // Directory share + else { + /** @var \OCP\Files\Folder $node */ + $node = $share->getPath(); - $activities = []; - if (!$isDir) { - // Single file public share - $activities[$originalSharePath] = Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED; + // Try to get the path + if ($path !== '') { + try { + $node = $node->get($path); + } catch (NotFoundException $e) { + return new NotFoundResponse(); + } + } + + $originalSharePath = $userFolder->getRelativePath($node->getPath()); + + if ($node instanceof \OCP\Files\File) { + // Single file download + $this->activityManager->publishActivity( + 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED, [$originalSharePath], '', [], + $originalSharePath, '', $share->getShareOwner()->getUID(), Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM + ); } else if (!empty($files_list)) { - // Only some files are downloaded + /** @var \OCP\Files\Folder $node */ + + // Subset of files is downloaded foreach ($files_list as $file) { - $filePath = Filesystem::normalizePath($originalSharePath . '/' . $file); - $isDir = \OC\Files\Filesystem::is_dir($filePath); - $activities[$filePath] = ($isDir) ? Activity::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED : Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED; + $subNode = $node->get($file); + $nodePath = $userFolder->getRelativePath($node->getPath()); + if ($subNode instanceof \OCP\Files\File) { + $this->activityManager->publishActivity( + 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED, [$nodePath], '', [], + $nodePath, '', $share->getShareOwner()->getUID(), Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM + ); + } else { + $this->activityManager->publishActivity( + 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED, [$nodePath], '', [], + $nodePath, '', $share->getShareOwner()->getUID(), Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM + ); + } } } else { // The folder is downloaded - $activities[$originalSharePath] = Activity::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED; - } - - foreach ($activities as $filePath => $subject) { $this->activityManager->publishActivity( - 'files_sharing', $subject, array($filePath), '', array(), - $filePath, '', $linkItem['uid_owner'], Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM + 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED, [$originalSharePath], '', [], + $originalSharePath, '', $share->getShareOwner()->getUID(), Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM ); } } + /* FIXME: We should do this all nicely in OCP */ + OC_Util::tearDownFS(); + OC_Util::setupFS($share->getShareOwner()->getUID()); + /** * this sets a cookie to be able to recognize the start of the download * the content must not be longer than 32 characters and must only contain @@ -362,30 +398,4 @@ class ShareController extends Controller { exit(); } } - - /** - * @param string $token - * @return string Resolved file path of the token - * @throws NotFoundException In case share could not get properly resolved - */ - private function getPath($token) { - $linkItem = Share::getShareByToken($token, false); - if (is_array($linkItem) && isset($linkItem['uid_owner'])) { - // seems to be a valid share - $rootLinkItem = Share::resolveReShare($linkItem); - if (isset($rootLinkItem['uid_owner'])) { - if(!$this->userManager->userExists($rootLinkItem['uid_owner'])) { - throw new NotFoundException('Owner of the share does not exist anymore'); - } - OC_Util::tearDownFS(); - OC_Util::setupFS($rootLinkItem['uid_owner']); - $path = Filesystem::getPath($linkItem['file_source']); - if(Filesystem::isReadable($path)) { - return $path; - } - } - } - - throw new NotFoundException('No file found belonging to file.'); - } } diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php index 29dfbd1e8b1..710537fe8cb 100644 --- a/apps/files_sharing/tests/controller/sharecontroller.php +++ b/apps/files_sharing/tests/controller/sharecontroller.php @@ -317,27 +317,20 @@ class ShareControllerTest extends \Test\TestCase { } public function testDownloadShare() { + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getPassword')->willReturn('password'); + + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('validtoken') + ->willReturn($share); + // Test with a password protected share and no authentication - $response = $this->shareController->downloadShare($this->token); + $response = $this->shareController->downloadShare('validtoken'); $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate', - array('token' => $this->token))); + ['token' => 'validtoken'])); $this->assertEquals($expectedResponse, $response); } - /** - * @expectedException \OCP\Files\NotFoundException - */ - 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); - } - } From 894a88ca516c69304d902bb6a98c5bcdb15becea Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 19 Jan 2016 10:17:29 +0100 Subject: [PATCH 5/9] [Share 2.0] Make public link work without view --- .../lib/controllers/sharecontroller.php | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php index 2342a092c64..f717dadf576 100644 --- a/apps/files_sharing/lib/controllers/sharecontroller.php +++ b/apps/files_sharing/lib/controllers/sharecontroller.php @@ -248,9 +248,20 @@ class ShareController extends Controller { // Show file list if ($share->getPath() instanceof \OCP\Files\Folder) { $shareTmpl['dir'] = $rootFolder->getRelativePath($path->getPath()); - $maxUploadFilesize = Util::maxUploadFilesize($share->getPath()->getPath()); - $freeSpace = Util::freeSpace($share->getPath()->getPath()); + + /* + * The OC_Util methods require a view. This just uses the node API + */ + $freeSpace = $share->getPath()->getStorage()->free_space($share->getPath()->getInternalPath()); + if ($freeSpace !== \OCP\Files\FileInfo::SPACE_UNKNOWN) { + $freeSpace = max($freeSpace, 0); + } else { + $freeSpace = (INF > 0) ? INF: PHP_INT_MAX; // work around https://bugs.php.net/bug.php?id=69188 + } + $uploadLimit = Util::uploadLimit(); + $maxUploadFilesize = min($freeSpace, $uploadLimit); + $folder = new Template('files', 'list', ''); $folder->assign('dir', $rootFolder->getRelativePath($path->getPath())); $folder->assign('dirToken', $token); From 9467859e42638476006004cfc0020b3247f51724 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 20 Jan 2016 09:04:22 +0100 Subject: [PATCH 6/9] Get correct path for activity downloads --- apps/files_sharing/lib/controllers/sharecontroller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php index f717dadf576..2ed5db5f00f 100644 --- a/apps/files_sharing/lib/controllers/sharecontroller.php +++ b/apps/files_sharing/lib/controllers/sharecontroller.php @@ -357,7 +357,7 @@ class ShareController extends Controller { // Subset of files is downloaded foreach ($files_list as $file) { $subNode = $node->get($file); - $nodePath = $userFolder->getRelativePath($node->getPath()); + $nodePath = $userFolder->getRelativePath($subNode->getPath()); if ($subNode instanceof \OCP\Files\File) { $this->activityManager->publishActivity( 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED, [$nodePath], '', [], From 67f5216160e9045a7fcbb152eda34fbf5e0833d4 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 20 Jan 2016 09:39:05 +0100 Subject: [PATCH 7/9] Do not use deprected activities API --- .../lib/controllers/sharecontroller.php | 54 +++++++++++-------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php index 2ed5db5f00f..19517a9012e 100644 --- a/apps/files_sharing/lib/controllers/sharecontroller.php +++ b/apps/files_sharing/lib/controllers/sharecontroller.php @@ -324,10 +324,14 @@ class ShareController extends Controller { // Single file share if ($share->getPath() instanceof \OCP\Files\File) { - $this->activityManager->publishActivity( - 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED, [$originalSharePath], '', [], - $originalSharePath, '', $share->getShareOwner()->getUID(), Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM - ); + // Single file download + $event = $this->activityManager->generateEvent(); + $event->setApp('files_sharing') + ->setType(Activity::TYPE_PUBLIC_LINKS) + ->setSubject(Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED, [$userFolder->getRelativePath($share->getPath()->getPath())]) + ->setAffectedUser($share->getShareOwner()->getUID()) + ->setObject('files', $share->getPath()->getId(), $userFolder->getRelativePath($share->getPath()->getPath())); + $this->activityManager->publish($event); } // Directory share else { @@ -347,35 +351,43 @@ class ShareController extends Controller { if ($node instanceof \OCP\Files\File) { // Single file download - $this->activityManager->publishActivity( - 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED, [$originalSharePath], '', [], - $originalSharePath, '', $share->getShareOwner()->getUID(), Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM - ); + $event = $this->activityManager->generateEvent(); + $event->setApp('files_sharing') + ->setType(Activity::TYPE_PUBLIC_LINKS) + ->setSubject(Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED, [$userFolder->getRelativePath($node->getPath())]) + ->setAffectedUser($share->getShareOwner()->getUID()) + ->setObject('files', $node->getId(), $userFolder->getRelativePath($node->getPath())); + $this->activityManager->publish($event); } else if (!empty($files_list)) { /** @var \OCP\Files\Folder $node */ // Subset of files is downloaded foreach ($files_list as $file) { $subNode = $node->get($file); - $nodePath = $userFolder->getRelativePath($subNode->getPath()); + + $event = $this->activityManager->generateEvent(); + $event->setApp('files_sharing') + ->setType(Activity::TYPE_PUBLIC_LINKS) + ->setAffectedUser($share->getShareOwner()->getUID()) + ->setObject('files', $subNode->getId(), $userFolder->getRelativePath($subNode->getPath())); + if ($subNode instanceof \OCP\Files\File) { - $this->activityManager->publishActivity( - 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED, [$nodePath], '', [], - $nodePath, '', $share->getShareOwner()->getUID(), Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM - ); + $event->setSubject(Activity::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED, [$userFolder->getRelativePath($subNode->getPath())]); } else { - $this->activityManager->publishActivity( - 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED, [$nodePath], '', [], - $nodePath, '', $share->getShareOwner()->getUID(), Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM - ); + $event->setSubject(Activity::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED, [$userFolder->getRelativePath($subNode->getPath())]); } + + $this->activityManager->publish($event); } } else { // The folder is downloaded - $this->activityManager->publishActivity( - 'files_sharing', Activity::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED, [$originalSharePath], '', [], - $originalSharePath, '', $share->getShareOwner()->getUID(), Activity::TYPE_PUBLIC_LINKS, Activity::PRIORITY_MEDIUM - ); + $event = $this->activityManager->generateEvent(); + $event->setApp('files_sharing') + ->setType(Activity::TYPE_PUBLIC_LINKS) + ->setSubject(Activity::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED, [$userFolder->getRelativePath($node->getPath())]) + ->setAffectedUser($share->getShareOwner()->getUID()) + ->setObject('files', $node->getId(), $userFolder->getRelativePath($node->getPath())); + $this->activityManager->publish($event); } } From 18421e7e686a4ed90f276925322b5d168f347e30 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 20 Jan 2016 10:14:03 +0100 Subject: [PATCH 8/9] Directly get from the server container * Updated unit tests --- apps/files_sharing/appinfo/application.php | 28 +---- .../lib/controllers/sharecontroller.php | 1 + .../tests/controller/sharecontroller.php | 111 +++++++++++------- 3 files changed, 75 insertions(+), 65 deletions(-) diff --git a/apps/files_sharing/appinfo/application.php b/apps/files_sharing/appinfo/application.php index 3971b04097f..e4a2262fece 100644 --- a/apps/files_sharing/appinfo/application.php +++ b/apps/files_sharing/appinfo/application.php @@ -48,14 +48,14 @@ class Application extends App { $c->query('AppName'), $c->query('Request'), $server->getConfig(), - $c->query('URLGenerator'), - $c->query('UserManager'), + $server->getURLGenerator(), + $server->getUserManager(), $server->getLogger(), $server->getActivityManager(), - $c->query('ShareManager'), - $c->query('Session'), + $server->getShareManager(), + $server->getSession(), $server->getPreviewManager(), - $c->query('RootFolder') + $server->getRootFolder() ); }); $container->registerService('ExternalSharesController', function (SimpleContainer $c) { @@ -70,24 +70,6 @@ class Application extends App { /** * Core class wrappers */ - $container->registerService('RootFolder', function(SimpleContainer $c) use ($server) { - return $server->getRootFolder(); - }); - $container->registerService('Session', function(SimpleContainer $c) use ($server) { - return $server->getSession(); - }); - $container->registerService('ShareManager', function(SimpleContainer $c) use ($server) { - return $server->getShareManager(); - }); - $container->registerService('UserSession', function (SimpleContainer $c) use ($server) { - return $server->getUserSession(); - }); - $container->registerService('URLGenerator', function (SimpleContainer $c) use ($server) { - return $server->getUrlGenerator(); - }); - $container->registerService('UserManager', function (SimpleContainer $c) use ($server) { - return $server->getUserManager(); - }); $container->registerService('HttpClientService', function (SimpleContainer $c) use ($server) { return $server->getHTTPClientService(); }); diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php index 19517a9012e..6f9a8d742cc 100644 --- a/apps/files_sharing/lib/controllers/sharecontroller.php +++ b/apps/files_sharing/lib/controllers/sharecontroller.php @@ -91,6 +91,7 @@ class ShareController extends Controller { * @param \OC\Share20\Manager $shareManager * @param ISession $session * @param IPreview $previewManager + * @param IRootFolder $rootFolder */ public function __construct($appName, IRequest $request, diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php index 710537fe8cb..43db17f5abc 100644 --- a/apps/files_sharing/tests/controller/sharecontroller.php +++ b/apps/files_sharing/tests/controller/sharecontroller.php @@ -31,15 +31,11 @@ namespace OCA\Files_Sharing\Controllers; use OC\Files\Filesystem; use OC\Share20\Exception\ShareNotFound; -use OCA\Files_Sharing\AppInfo\Application; use OCP\AppFramework\Http\NotFoundResponse; -use OCP\AppFramework\IAppContainer; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\TemplateResponse; use OCP\ISession; use OCP\Security\ISecureRandom; -use OC\Files\View; -use OCP\Share; use OCP\IURLGenerator; /** @@ -49,38 +45,49 @@ use OCP\IURLGenerator; */ class ShareControllerTest extends \Test\TestCase { - /** @var IAppContainer */ - private $container; /** @var string */ private $user; /** @var string */ - private $token; - /** @var string */ private $oldUser; + + /** @var string */ + private $appName = 'files_sharing'; /** @var ShareController */ private $shareController; - /** @var IURLGenerator */ + /** @var IURLGenerator | \PHPUnit_Framework_MockObject_MockObject */ private $urlGenerator; /** @var ISession | \PHPUnit_Framework_MockObject_MockObject */ private $session; + /** @var \OCP\IPreview | \PHPUnit_Framework_MockObject_MockObject */ + private $previewManager; + /** @var \OCP\IConfig | \PHPUnit_Framework_MockObject_MockObject */ + private $config; /** @var \OC\Share20\Manager | \PHPUnit_Framework_MockObject_MockObject */ private $shareManager; protected function setUp() { - $app = new Application(); - $this->container = $app->getContainer(); - $this->container['Config'] = $this->getMock('\OCP\IConfig'); - $this->container['AppName'] = 'files_sharing'; - $this->container['URLGenerator'] = $this->getMock('\OCP\IURLGenerator'); - $this->container['UserManager'] = $this->getMock('\OCP\IUserManager'); - $this->container['ShareManager'] = $this->getMockBuilder('\OC\Share20\Manager') - ->disableOriginalConstructor()->getMock(); - $this->container['Session'] = $this->getMock('\OCP\ISession'); + $this->appName = 'files_sharing'; + + $this->shareManager = $this->getMockBuilder('\OC\Share20\Manager')->disableOriginalConstructor()->getMock(); + $this->urlGenerator = $this->getMock('\OCP\IURLGenerator'); + $this->session = $this->getMock('\OCP\ISession'); + $this->previewManager = $this->getMock('\OCP\IPreview'); + $this->config = $this->getMock('\OCP\IConfig'); + + $this->shareController = new \OCA\Files_Sharing\Controllers\ShareController( + $this->appName, + $this->getMock('\OCP\IRequest'), + $this->config, + $this->urlGenerator, + $this->getMock('\OCP\IUserManager'), + $this->getMock('\OCP\ILogger'), + $this->getMock('\OCP\Activity\IManager'), + $this->shareManager, + $this->session, + $this->previewManager, + $this->getMock('\OCP\Files\IRootFolder') + ); - $this->session = $this->container['Session']; - $this->shareManager = $this->container['ShareManager']; - $this->urlGenerator = $this->container['URLGenerator']; - $this->shareController = $this->container['ShareController']; // Store current user $this->oldUser = \OC_User::getUser(); @@ -91,17 +98,6 @@ class ShareControllerTest extends \Test\TestCase { \OC::$server->getUserManager()->createUser($this->user, $this->user); \OC_Util::tearDownFS(); $this->loginAsUser($this->user); - - // Create a dummy shared file - $view = new View('/'. $this->user . '/files'); - $view->file_put_contents('file1.txt', 'I am such an awesome shared file!'); - $this->token = \OCP\Share::shareItem( - Filesystem::getFileInfo('file1.txt')->getType(), - Filesystem::getFileInfo('file1.txt')->getId(), - \OCP\Share::SHARE_TYPE_LINK, - 'IAmPasswordProtected!', - 1 - ); } protected function tearDown() { @@ -129,7 +125,7 @@ class ShareControllerTest extends \Test\TestCase { ->willReturn($share); $response = $this->shareController->showAuthenticate('token'); - $expectedResponse = new TemplateResponse($this->container['AppName'], 'authenticate', [], 'guest'); + $expectedResponse = new TemplateResponse($this->appName, 'authenticate', [], 'guest'); $this->assertEquals($expectedResponse, $response); } @@ -147,7 +143,7 @@ class ShareControllerTest extends \Test\TestCase { $this->session->method('get')->with('public_link_authenticated')->willReturn('2'); $response = $this->shareController->showAuthenticate('token'); - $expectedResponse = new TemplateResponse($this->container['AppName'], 'authenticate', [], 'guest'); + $expectedResponse = new TemplateResponse($this->appName, 'authenticate', [], 'guest'); $this->assertEquals($expectedResponse, $response); } @@ -164,8 +160,13 @@ class ShareControllerTest extends \Test\TestCase { $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); $this->session->method('get')->with('public_link_authenticated')->willReturn('1'); + $this->urlGenerator->expects($this->once()) + ->method('linkToRoute') + ->with('files_sharing.sharecontroller.showShare', ['token' => 'token']) + ->willReturn('redirect'); + $response = $this->shareController->showAuthenticate('token'); - $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.showShare', array('token' => 'token'))); + $expectedResponse = new RedirectResponse('redirect'); $this->assertEquals($expectedResponse, $response); } @@ -202,8 +203,13 @@ class ShareControllerTest extends \Test\TestCase { ->method('set') ->with('public_link_authenticated', '42'); + $this->urlGenerator->expects($this->once()) + ->method('linkToRoute') + ->with('files_sharing.sharecontroller.showShare', ['token'=>'token']) + ->willReturn('redirect'); + $response = $this->shareController->authenticate('token', 'validpassword'); - $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.showShare', array('token' => 'token'))); + $expectedResponse = new RedirectResponse('redirect'); $this->assertEquals($expectedResponse, $response); } @@ -228,7 +234,7 @@ class ShareControllerTest extends \Test\TestCase { ->method('set'); $response = $this->shareController->authenticate('token', 'invalidpassword'); - $expectedResponse = new TemplateResponse($this->container['AppName'], 'authenticate', array('wrongpw' => true), 'guest'); + $expectedResponse = new TemplateResponse($this->appName, 'authenticate', array('wrongpw' => true), 'guest'); $this->assertEquals($expectedResponse, $response); } @@ -255,9 +261,14 @@ class ShareControllerTest extends \Test\TestCase { ->with('validtoken') ->willReturn($share); + $this->urlGenerator->expects($this->once()) + ->method('linkToRoute') + ->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken']) + ->willReturn('redirect'); + // Test without a not existing token $response = $this->shareController->showShare('validtoken'); - $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate', array('token' => 'validtoken'))); + $expectedResponse = new RedirectResponse('redirect'); $this->assertEquals($expectedResponse, $response); } @@ -282,6 +293,18 @@ class ShareControllerTest extends \Test\TestCase { $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); $this->session->method('get')->with('public_link_authenticated')->willReturn('42'); + $this->previewManager->method('isMimeSupported')->with('text/plain')->willReturn(true); + + $this->config->method('getSystemValue') + ->willReturnMap( + [ + ['max_filesize_animated_gifs_public_sharing', 10, 10], + ['enable_previews', true, true], + ] + ); + $shareTmpl['maxSizeAnimateGif'] = $this->config->getSystemValue('max_filesize_animated_gifs_public_sharing', 10); + $shareTmpl['previewEnabled'] = $this->config->getSystemValue('enable_previews', true); + $this->shareManager ->expects($this->once()) ->method('getShareByToken') @@ -310,7 +333,7 @@ class ShareControllerTest extends \Test\TestCase { $csp = new \OCP\AppFramework\Http\ContentSecurityPolicy(); $csp->addAllowedFrameDomain('\'self\''); - $expectedResponse = new TemplateResponse($this->container['AppName'], 'public', $sharedTmplParams, 'base'); + $expectedResponse = new TemplateResponse($this->appName, 'public', $sharedTmplParams, 'base'); $expectedResponse->setContentSecurityPolicy($csp); $this->assertEquals($expectedResponse, $response); @@ -326,10 +349,14 @@ class ShareControllerTest extends \Test\TestCase { ->with('validtoken') ->willReturn($share); + $this->urlGenerator->expects($this->once()) + ->method('linkToRoute') + ->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken']) + ->willReturn('redirect'); + // Test with a password protected share and no authentication $response = $this->shareController->downloadShare('validtoken'); - $expectedResponse = new RedirectResponse($this->urlGenerator->linkToRoute('files_sharing.sharecontroller.authenticate', - ['token' => 'validtoken'])); + $expectedResponse = new RedirectResponse('redirect'); $this->assertEquals($expectedResponse, $response); } From 88bc8634d2076b7392c9ec214e414c558a6584d6 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 20 Jan 2016 21:56:55 +0100 Subject: [PATCH 9/9] Add Unit tests --- .../lib/share20/defaultshareprovidertest.php | 50 ++++++++++++++ tests/lib/share20/managertest.php | 65 +++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/tests/lib/share20/defaultshareprovidertest.php b/tests/lib/share20/defaultshareprovidertest.php index 4db6b2b14e7..039692772a0 100644 --- a/tests/lib/share20/defaultshareprovidertest.php +++ b/tests/lib/share20/defaultshareprovidertest.php @@ -721,4 +721,54 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertSame('token', $share2->getToken()); $this->assertEquals($expireDate, $share2->getExpirationDate()); } + + public function testGetShareByToken() { + $qb = $this->dbConn->getQueryBuilder(); + + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_LINK), + 'share_with' => $qb->expr()->literal('password'), + 'uid_owner' => $qb->expr()->literal('shareOwner'), + 'uid_initiator' => $qb->expr()->literal('sharedBy'), + 'item_type' => $qb->expr()->literal('file'), + 'file_source' => $qb->expr()->literal(42), + 'file_target' => $qb->expr()->literal('myTarget'), + 'permissions' => $qb->expr()->literal(13), + 'token' => $qb->expr()->literal('secrettoken'), + ]); + $qb->execute(); + $id = $qb->getLastInsertId(); + + $owner = $this->getMock('\OCP\IUser'); + $owner->method('getUID')->willReturn('shareOwner'); + $initiator = $this->getMock('\OCP\IUser'); + $initiator->method('getUID')->willReturn('sharedBy'); + + $this->userManager->method('get') + ->will($this->returnValueMap([ + ['sharedBy', $initiator], + ['shareOwner', $owner], + ])); + + $file = $this->getMock('\OCP\Files\File'); + + $this->rootFolder->method('getUserFolder')->with('shareOwner')->will($this->returnSelf()); + $this->rootFolder->method('getById')->with(42)->willReturn([$file]); + + $share = $this->provider->getShareByToken('secrettoken'); + $this->assertEquals($id, $share->getId()); + $this->assertSame($owner, $share->getShareOwner()); + $this->assertSame($initiator, $share->getSharedBy()); + $this->assertSame('secrettoken', $share->getToken()); + $this->assertSame('password', $share->getPassword()); + $this->assertSame(null, $share->getSharedWith()); + } + + /** + * @expectedException \OC\Share20\Exception\ShareNotFound + */ + public function testGetShareByTokenNotFound() { + $this->provider->getShareByToken('invalidtoken'); + } } diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php index 92ebe1951b5..28303d3152f 100644 --- a/tests/lib/share20/managertest.php +++ b/tests/lib/share20/managertest.php @@ -1478,6 +1478,71 @@ class ManagerTest extends \Test\TestCase { $manager->createShare($share); } + + public function testGetShareByToken() { + $factory = $this->getMock('\OC\Share20\IProviderFactory'); + + $manager = new Manager( + $this->logger, + $this->config, + $this->secureRandom, + $this->hasher, + $this->mountManager, + $this->groupManager, + $this->l, + $factory + ); + + $share = $this->getMock('\OC\Share20\IShare'); + + $factory->expects($this->once()) + ->method('getProviderForType') + ->with(\OCP\Share::SHARE_TYPE_LINK) + ->willReturn($this->defaultProvider); + + $this->defaultProvider->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + + $ret = $manager->getShareByToken('token'); + $this->assertSame($share, $ret); + } + + public function testCheckPasswordNoLinkShare() { + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER); + $this->assertFalse($this->manager->checkPassword($share, 'password')); + } + + public function testCheckPasswordNoPassword() { + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK); + $this->assertFalse($this->manager->checkPassword($share, 'password')); + + $share->method('getPassword')->willReturn('password'); + $this->assertFalse($this->manager->checkPassword($share, null)); + } + + public function testCheckPasswordInvalidPassword() { + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK); + $share->method('getPassword')->willReturn('password'); + + $this->hasher->method('verify')->with('invalidpassword', 'password', '')->willReturn(false); + + $this->assertFalse($this->manager->checkPassword($share, 'invalidpassword')); + } + + public function testCheckPasswordValidPassword() { + $share = $this->getMock('\OC\Share20\IShare'); + $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK); + $share->method('getPassword')->willReturn('passwordHash'); + + $this->hasher->method('verify')->with('password', 'passwordHash', '')->willReturn(true); + + $this->assertTrue($this->manager->checkPassword($share, 'password')); + } } class DummyPassword {