Skip to content

GitLab

Commit 106d0f9c authored by Björn Schießle's avatar Björn Schießle
Browse files

Merge pull request #12224 from owncloud/fix_12211 

make sure that we don't find the wrong shares if a user and a group have the same ID
parents 30978800 ea4eedd3
Hide whitespace changes
Inline Side-by-side
lib/private/share/share.php
View file @ 106d0f9c
......@@ -288,9 +288,10 @@ class Share extends \OC\Share\Constants {
* @param string $itemType
* @param string $itemSource
* @param string $user User user to whom the item was shared
* @param int $shareType only look for a specific share type
* @return array Return list of items with file_target, permissions and expiration
*/
public static function getItemSharedWithUser($itemType, $itemSource, $user) {
public static function getItemSharedWithUser($itemType, $itemSource, $user, $shareType = null) {
$shares = array();
$fileDependend = false;
......@@ -314,6 +315,11 @@ class Share extends \OC\Share\Constants {
$arguments[] = $user;
}
if ($shareType !== null) {
$where .= ' AND `share_type` = ? ';
$arguments[] = $shareType;
}
$query = \OC_DB::prepare('SELECT ' . $select . ' FROM `*PREFIX*share` '. $where);
$result = \OC_DB::executeAudited($query, $arguments);
......@@ -697,7 +703,7 @@ class Share extends \OC\Share\Constants {
// check if it is a valid itemType
self::getBackend($itemType);
$items = self::getItemSharedWithUser($itemType, $itemSource, $shareWith);
$items = self::getItemSharedWithUser($itemType, $itemSource, $shareWith, $shareType);
$toDelete = array();
$newParent = null;
......@@ -1308,14 +1314,18 @@ class Share extends \OC\Share\Constants {
if (isset($shareType)) {
// Include all user and group items
if ($shareType == self::$shareTypeUserAndGroups && isset($shareWith)) {
$where .= ' AND `share_type` IN (?,?,?)';
$where .= ' AND ((`share_type` in (?, ?) AND `share_with` = ?) ';
$queryArgs[] = self::SHARE_TYPE_USER;
$queryArgs[] = self::SHARE_TYPE_GROUP;
$queryArgs[] = self::$shareTypeGroupUserUnique;
$userAndGroups = array_merge(array($shareWith), \OC_Group::getUserGroups($shareWith));
$placeholders = join(',', array_fill(0, count($userAndGroups), '?'));
$where .= ' AND `share_with` IN ('.$placeholders.')';
$queryArgs = array_merge($queryArgs, $userAndGroups);
$queryArgs[] = $shareWith;
$groups = \OC_Group::getUserGroups($shareWith);
if (!empty($groups)) {
$placeholders = join(',', array_fill(0, count($groups), '?'));
$where .= ' OR (`share_type` = ? AND `share_with` IN ('.$placeholders.')) ';
$queryArgs[] = self::SHARE_TYPE_GROUP;
$queryArgs = array_merge($queryArgs, $groups);
}
$where .= ')';
// Don't include own group shares
$where .= ' AND `uid_owner` != ?';
$queryArgs[] = $shareWith;
......@@ -1506,8 +1516,11 @@ class Share extends \OC\Share\Constants {
$row['permissions'] &= ~\OCP\PERMISSION_SHARE;
}
// Add display names to result
if ( isset($row['share_with']) && $row['share_with'] != '') {
if ( isset($row['share_with']) && $row['share_with'] != '' &&
isset($row['share_with']) && $row['share_type'] === self::SHARE_TYPE_USER) {
$row['share_with_displayname'] = \OCP\User::getDisplayName($row['share_with']);
} else {
$row['share_with_displayname'] = $row['share_with'];
}
if ( isset($row['uid_owner']) && $row['uid_owner'] != '') {
$row['displayname_owner'] = \OCP\User::getDisplayName($row['uid_owner']);
......
tests/lib/share/share.php
View file @ 106d0f9c
......@@ -19,7 +19,7 @@
* License along with this library. If not, see <http://www.gnu.org/licenses/>.
*/
class Test_Share extends PHPUnit_Framework_TestCase {
class Test_Share extends Test\TestCase {
protected $itemType;
protected $userBackend;
......@@ -27,6 +27,7 @@ class Test_Share extends PHPUnit_Framework_TestCase {
protected $user2;
protected $user3;
protected $user4;
protected $groupAndUser;
protected $groupBackend;
protected $group1;
protected $group2;
......@@ -34,29 +35,35 @@ class Test_Share extends PHPUnit_Framework_TestCase {
protected $dateInFuture;
protected $dateInPast;
public function setUp() {
protected function setUp() {
parent::setUp();
OC_User::clearBackends();
OC_User::useBackend('dummy');
$this->user1 = uniqid('user1_');
$this->user2 = uniqid('user2_');
$this->user3 = uniqid('user3_');
$this->user4 = uniqid('user4_');
$this->user1 = $this->getUniqueID('user1_');
$this->user2 = $this->getUniqueID('user2_');
$this->user3 = $this->getUniqueID('user3_');
$this->user4 = $this->getUniqueID('user4_');
$this->groupAndUser = $this->getUniqueID('groupAndUser_');
OC_User::createUser($this->user1, 'pass');
OC_User::createUser($this->user2, 'pass');
OC_User::createUser($this->user3, 'pass');
OC_User::createUser($this->user4, 'pass');
OC_User::createUser($this->groupAndUser, 'pass');
OC_User::setUserId($this->user1);
OC_Group::clearBackends();
OC_Group::useBackend(new OC_Group_Dummy);
$this->group1 = uniqid('group1_');
$this->group2 = uniqid('group2_');
$this->group1 = $this->getUniqueID('group1_');
$this->group2 = $this->getUniqueID('group2_');
OC_Group::createGroup($this->group1);
OC_Group::createGroup($this->group2);
OC_Group::createGroup($this->groupAndUser);
OC_Group::addToGroup($this->user1, $this->group1);
OC_Group::addToGroup($this->user2, $this->group1);
OC_Group::addToGroup($this->user3, $this->group1);
OC_Group::addToGroup($this->user2, $this->group2);
OC_Group::addToGroup($this->user4, $this->group2);
OC_Group::addToGroup($this->user2, $this->groupAndUser);
OC_Group::addToGroup($this->user3, $this->groupAndUser);
OCP\Share::registerBackend('test', 'Test_Share_Backend');
OC_Hook::clear('OCP\\Share');
OC::registerShareHooks();
......@@ -70,10 +77,11 @@ class Test_Share extends PHPUnit_Framework_TestCase {
$this->dateInFuture = date($dateFormat, $now + 20 * 60);
}
public function tearDown() {
protected function tearDown() {
$query = OC_DB::prepare('DELETE FROM `*PREFIX*share` WHERE `item_type` = ?');
$query->execute(array('test'));
OC_Appconfig::setValue('core', 'shareapi_allow_resharing', $this->resharing);
parent::tearDown();
}
public function testShareInvalidShareType() {
......@@ -600,6 +608,41 @@ class Test_Share extends PHPUnit_Framework_TestCase {
$this->assertEquals(array(), OCP\Share::getItemsShared('test'));
}
public function testShareWithGroupAndUserBothHaveTheSameId() {
$this->shareUserTestFileWithUser($this->user1, $this->groupAndUser);
OC_User::setUserId($this->groupAndUser);
$this->assertEquals(array('test.txt'), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_SOURCE),
'"groupAndUser"-User does not see the file but it was shared with him');
OC_User::setUserId($this->user2);
$this->assertEquals(array(), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_SOURCE),
'User2 sees test.txt but it was only shared with the user "groupAndUser" and not with group');
OC_User::setUserId($this->user1);
$this->assertTrue(OCP\Share::unshareAll('test', 'test.txt'));
$this->assertTrue(
OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->groupAndUser, OCP\PERMISSION_READ),
'Failed asserting that user 1 successfully shared text.txt with group 1.'
);
OC_User::setUserId($this->groupAndUser);
$this->assertEquals(array(), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_SOURCE),
'"groupAndUser"-User sees test.txt but it was only shared with the group "groupAndUser" and not with the user');
OC_User::setUserId($this->user2);
$this->assertEquals(array('test.txt'), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_SOURCE),
'User2 does not see test.txt but it was shared with the group "groupAndUser"');
OC_User::setUserId($this->user1);
$this->assertTrue(OCP\Share::unshareAll('test', 'test.txt'));
}
/**
* @param boolean|string $token
*/
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment