From 9fd4cb1b6683cdebdeaec0f744bd2ba1fb1c64e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 21 Jan 2014 10:42:47 +0100 Subject: [PATCH 1/3] adding password protection check to getShareByToken() --- apps/files_sharing/public.php | 2 +- lib/public/share.php | 36 +++++++++++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/apps/files_sharing/public.php b/apps/files_sharing/public.php index d050efd5b3..100379047d 100644 --- a/apps/files_sharing/public.php +++ b/apps/files_sharing/public.php @@ -35,7 +35,7 @@ function determineIcon($file, $sharingRoot, $sharingToken) { if (isset($_GET['t'])) { $token = $_GET['t']; - $linkItem = OCP\Share::getShareByToken($token); + $linkItem = OCP\Share::getShareByToken($token, false); if (is_array($linkItem) && isset($linkItem['uid_owner'])) { // seems to be a valid share $type = $linkItem['item_type']; diff --git a/lib/public/share.php b/lib/public/share.php index eb1dd8d1c9..4573fe8d8d 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -347,11 +347,11 @@ class Share { } /** - * Get the item shared by a token - * @param string token - * @return Item + * Based on the given token the share information will be returned - password protected shares will be verified + * @param string $token + * @return array | bool false will be returned in case the token is unknown or unauthorized */ - public static function getShareByToken($token) { + public static function getShareByToken($token, $checkPasswordProtection = true) { $query = \OC_DB::prepare('SELECT * FROM `*PREFIX*share` WHERE `token` = ?', 1); $result = $query->execute(array($token)); if (\OC_DB::isError($result)) { @@ -361,6 +361,12 @@ class Share { if (is_array($row) and self::expireItem($row)) { return false; } + + // password protected shares need to me authenticated + if ($checkPasswordProtection && !\OCP\Share::checkPasswordProtectedShare($row)) { + return false; + } + return $row; } @@ -1888,6 +1894,28 @@ class Share { } } + /** + * In case a password protected link is not yet authenticated this function will return false + * + * @param array $linkItem + * @return bool + */ + public static function checkPasswordProtectedShare(array $linkItem) { + if (!isset($linkItem['share_with'])) { + return true; + } + + if ($linkItem['share_type'] != \OCP\Share::SHARE_TYPE_LINK) { + return true; + } + + if ( \OC::$session->exists('public_link_authenticated') + && \OC::$session->get('public_link_authenticated') === $linkItem['id'] ) { + return true; + } + + return false; + } } /** From 6746ad0a7315d1ee06f7b1804b7c9457755f6648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 21 Jan 2014 10:55:10 +0100 Subject: [PATCH 2/3] in case no share is found for the given token we can return right away --- lib/public/share.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/public/share.php b/lib/public/share.php index 4573fe8d8d..ddc9e1e066 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -358,6 +358,9 @@ class Share { \OC_Log::write('OCP\Share', \OC_DB::getErrorMessage($result) . ', token=' . $token, \OC_Log::ERROR); } $row = $result->fetchRow(); + if ($row === false) { + return false; + } if (is_array($row) and self::expireItem($row)) { return false; } From a3ea5aa2ac85fea812e25e2578db4cf86a0d3418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 21 Jan 2014 12:07:08 +0100 Subject: [PATCH 3/3] fixing comment + adding unit test for checkPasswordProtectedShare --- lib/public/share.php | 8 +++++++- tests/lib/share/share.php | 42 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/lib/public/share.php b/lib/public/share.php index ddc9e1e066..f832d04a70 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -365,7 +365,7 @@ class Share { return false; } - // password protected shares need to me authenticated + // password protected shares need to be authenticated if ($checkPasswordProtection && !\OCP\Share::checkPasswordProtectedShare($row)) { return false; } @@ -1907,6 +1907,12 @@ class Share { if (!isset($linkItem['share_with'])) { return true; } + if (!isset($linkItem['share_type'])) { + return true; + } + if (!isset($linkItem['id'])) { + return true; + } if ($linkItem['share_type'] != \OCP\Share::SHARE_TYPE_LINK) { return true; diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php index 2fe2837019..d6acee6c92 100644 --- a/tests/lib/share/share.php +++ b/tests/lib/share/share.php @@ -25,6 +25,8 @@ class Test_Share extends PHPUnit_Framework_TestCase { protected $userBackend; protected $user1; protected $user2; + protected $user3; + protected $user4; protected $groupBackend; protected $group1; protected $group2; @@ -656,4 +658,44 @@ class Test_Share extends PHPUnit_Framework_TestCase { 'Failed asserting that the share of the test.txt file by user 2 has been removed.' ); } + + /** + * @dataProvider checkPasswordProtectedShareDataProvider + * @param $expected + * @param $item + */ + public function testCheckPasswordProtectedShare($expected, $item) { + \OC::$session->set('public_link_authenticated', 100); + $result = \OCP\Share::checkPasswordProtectedShare($item); + $this->assertEquals($expected, $result); + } + + function checkPasswordProtectedShareDataProvider() { + return array( + array(true, array()), + array(true, array('share_with' => null)), + array(true, array('share_with' => '')), + array(true, array('share_with' => '1234567890', 'share_type' => '1')), + array(true, array('share_with' => '1234567890', 'share_type' => 1)), + array(true, array('share_with' => '1234567890', 'share_type' => '3', 'id' => 100)), + array(true, array('share_with' => '1234567890', 'share_type' => 3, 'id' => 100)), + array(false, array('share_with' => '1234567890', 'share_type' => '3', 'id' => 101)), + array(false, array('share_with' => '1234567890', 'share_type' => 3, 'id' => 101)), + ); + + /* + if (!isset($linkItem['share_with'])) { + return true; + } + + if ($linkItem['share_type'] != \OCP\Share::SHARE_TYPE_LINK) { + return true; + } + + if ( \OC::$session->exists('public_link_authenticated') + && \OC::$session->get('public_link_authenticated') === $linkItem['id'] ) { + return true; + } + * */ + } }