diff options
author | Thomas Müller <thomas.mueller@tmit.eu> | 2016-02-09 13:30:22 +0100 |
---|---|---|
committer | Thomas Müller <thomas.mueller@tmit.eu> | 2016-02-09 13:30:22 +0100 |
commit | 6474866afff12ad281576438923f7d30c009db21 (patch) | |
tree | 736813f094556ec4b5f146fcc384b20fce689eb1 /apps | |
parent | 1a2d72b0ac7e43fd1780682ea469dc715e4dd0e9 (diff) | |
parent | 12afd7b0059a2afb851a2ea69cec351a8d5b9478 (diff) | |
download | nextcloud-server-6474866afff12ad281576438923f7d30c009db21.tar.gz nextcloud-server-6474866afff12ad281576438923f7d30c009db21.zip |
Merge pull request #22225 from owncloud/sharing-moar-hooks
More sharing hooks for extended auditing
Diffstat (limited to 'apps')
-rw-r--r-- | apps/files_sharing/lib/controllers/sharecontroller.php | 63 | ||||
-rw-r--r-- | apps/files_sharing/tests/controller/sharecontroller.php | 20 |
2 files changed, 76 insertions, 7 deletions
diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php index bbe68096b52..dae61a3537b 100644 --- a/apps/files_sharing/lib/controllers/sharecontroller.php +++ b/apps/files_sharing/lib/controllers/sharecontroller.php @@ -30,7 +30,6 @@ namespace OCA\Files_Sharing\Controllers; use OC; -use OC\Files\Filesystem; use OC_Files; use OC_Util; use OCP; @@ -71,7 +70,7 @@ class ShareController extends Controller { protected $logger; /** @var OCP\Activity\IManager */ protected $activityManager; - /** @var OC\Share20\Manager */ + /** @var OCP\Share\IManager */ protected $shareManager; /** @var ISession */ protected $session; @@ -88,7 +87,7 @@ class ShareController extends Controller { * @param IUserManager $userManager * @param ILogger $logger * @param OCP\Activity\IManager $activityManager - * @param \OC\Share20\Manager $shareManager + * @param \OCP\Share\IManager $shareManager * @param ISession $session * @param IPreview $previewManager * @param IRootFolder $rootFolder @@ -100,7 +99,7 @@ class ShareController extends Controller { IUserManager $userManager, ILogger $logger, \OCP\Activity\IManager $activityManager, - \OC\Share20\Manager $shareManager, + \OCP\Share\IManager $shareManager, ISession $session, IPreview $previewManager, IRootFolder $rootFolder) { @@ -177,6 +176,7 @@ class ShareController extends Controller { if ($this->shareManager->checkPassword($share, $password)) { $this->session->set('public_link_authenticated', (string)$share->getId()); } else { + $this->emitAccessShareHook($share, 403, 'Wrong password'); return false; } } else { @@ -190,6 +190,44 @@ class ShareController extends Controller { } /** + * throws hooks when a share is attempted to be accessed + * + * @param \OCP\Share\IShare|string $share the Share instance if available, + * otherwise token + * @param int $errorCode + * @param string $errorMessage + * @throws OC\HintException + * @throws OC\ServerNotAvailableException + */ + protected function emitAccessShareHook($share, $errorCode = 200, $errorMessage = '') { + $itemType = $itemSource = $uidOwner = ''; + $token = $share; + $exception = null; + if($share instanceof \OCP\Share\IShare) { + try { + $token = $share->getToken(); + $uidOwner = $share->getSharedBy(); + $itemType = $share->getNodeType(); + $itemSource = $share->getNodeId(); + } catch (\Exception $e) { + // we log what we know and pass on the exception afterwards + $exception = $e; + } + } + \OC_Hook::emit('OCP\Share', 'share_link_access', [ + 'itemType' => $itemType, + 'itemSource' => $itemSource, + 'uidOwner' => $uidOwner, + 'token' => $token, + 'errorCode' => $errorCode, + 'errorMessage' => $errorMessage, + ]); + if(!is_null($exception)) { + throw $exception; + } + } + + /** * @PublicPage * @NoCSRFRequired * @@ -205,6 +243,7 @@ class ShareController extends Controller { try { $share = $this->shareManager->getShareByToken($token); } catch (ShareNotFound $e) { + $this->emitAccessShareHook($token, 404, 'Share not found'); return new NotFoundResponse(); } @@ -215,8 +254,14 @@ class ShareController extends Controller { } // We can't get the path of a file share - if ($share->getNode() instanceof \OCP\Files\File && $path !== '') { - throw new NotFoundException(); + try { + if ($share->getNode() instanceof \OCP\Files\File && $path !== '') { + $this->emitAccessShareHook($share, 404, 'Share not found'); + throw new NotFoundException(); + } + } catch (\Exception $e) { + $this->emitAccessShareHook($share, 404, 'Share not found'); + throw $e; } $rootFolder = null; @@ -227,6 +272,7 @@ class ShareController extends Controller { try { $path = $rootFolder->get($path); } catch (\OCP\Files\NotFoundException $e) { + $this->emitAccessShareHook($share, 404, 'Share not found'); throw new NotFoundException(); } } @@ -287,6 +333,8 @@ class ShareController extends Controller { $response = new TemplateResponse($this->appName, 'public', $shareTmpl, 'base'); $response->setContentSecurityPolicy($csp); + $this->emitAccessShareHook($share); + return $response; } @@ -344,6 +392,7 @@ class ShareController extends Controller { try { $node = $node->get($path); } catch (NotFoundException $e) { + $this->emitAccessShareHook($share, 404, 'Share not found'); return new NotFoundResponse(); } } @@ -409,6 +458,8 @@ class ShareController extends Controller { setcookie('ocDownloadStarted', $downloadStartSecret, time() + 20, '/'); } + $this->emitAccessShareHook($share); + // download selected files if (!is_null($files)) { // FIXME: The exit is required here because otherwise the AppFramework is trying to add headers as well diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php index 22e15972cd6..11dc082390c 100644 --- a/apps/files_sharing/tests/controller/sharecontroller.php +++ b/apps/files_sharing/tests/controller/sharecontroller.php @@ -219,7 +219,11 @@ class ShareControllerTest extends \Test\TestCase { public function testAuthenticateInvalidPassword() { $share = \OC::$server->getShareManager()->newShare(); - $share->setId(42); + $share->setNodeId(100) + ->setNodeType('file') + ->setToken('token') + ->setSharedBy('initiator') + ->setId(42); $this->shareManager ->expects($this->once()) @@ -237,6 +241,20 @@ class ShareControllerTest extends \Test\TestCase { ->expects($this->never()) ->method('set'); + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['access'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'share_link_access', $hookListner, 'access'); + + $hookListner->expects($this->once()) + ->method('access') + ->with($this->callback(function(array $data) { + return $data['itemType'] === 'file' && + $data['itemSource'] === 100 && + $data['uidOwner'] === 'initiator' && + $data['token'] === 'token' && + $data['errorCode'] === 403 && + $data['errorMessage'] === 'Wrong password'; + })); + $response = $this->shareController->authenticate('token', 'invalidpassword'); $expectedResponse = new TemplateResponse($this->appName, 'authenticate', array('wrongpw' => true), 'guest'); $this->assertEquals($expectedResponse, $response); |