make sure that we don't find the wrong shares if a user and a group have the same ID

diff --git a/lib/private/share/share.php b/lib/private/share/share.php
index f2639fd8977bfc33483de41a6566cfd94d373eff..c5f3bf69b312c1fe2012c5b1c3785f30915c30d2 100644 (file)
--- a/lib/private/share/share.php
+++ b/lib/private/share/share.php
@@ -1254,14 +1254,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;

diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php
index fbff89567d2c58dff4544dd835ef629cc1a2df2e..ff2f3c99081b7d5752e77f28637d3c57685f0659 100644 (file)
--- a/tests/lib/share/share.php
+++ b/tests/lib/share/share.php
@@ -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;
@@ -41,10 +42,12 @@ class Test_Share extends PHPUnit_Framework_TestCase {
                $this->user2 = uniqid('user2_');
                $this->user3 = uniqid('user3_');
                $this->user4 = uniqid('user4_');
+               $this->groupAndUser = uniqid('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);
@@ -52,11 +55,14 @@ class Test_Share extends PHPUnit_Framework_TestCase {
                $this->group2 = uniqid('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();
@@ -576,6 +582,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
         */