Merge pull request #19577 from owncloud/share-donotreturnentrieswhenusernotingroup

Remove invalid share items from result when missing group membership

2 files changed, 43 insertions, 0 deletions

diff --git a/lib/private/share/share.php b/lib/private/share/share.php
index 42fec03d3ae..ca94c51cf76 100644
--- a/lib/private/share/share.php
+++ b/lib/private/share/share.php
@@ -1908,6 +1908,12 @@ class Share extends Constants {
 				$items = array_merge($items, $collectionItems);
 			}
 
+			// filter out invalid items, these can appear when subshare entries exist
+			// for a group in which the requested user isn't a member any more
+			$items = array_filter($items, function($item) {
+				return $item['share_type'] !== self::$shareTypeGroupUserUnique;
+			});
+
 			return self::formatResult($items, $column, $backend, $format, $parameters);
 		} elseif ($includeCollections && $collectionTypes && in_array('folder', $collectionTypes)) {
 			// FIXME: Thats a dirty hack to improve file sharing performance,
diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php
index f0dc921e969..2ca54390e65 100644
--- a/tests/lib/share/share.php
+++ b/tests/lib/share/share.php
@@ -931,6 +931,43 @@ class Test_Share extends \Test\TestCase {
 		$this->assertEmpty($expected, 'did not found all expected values');
 	}
 
+	public function testGetShareSubItemsWhenUserNotInGroup() {
+		OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1, \OCP\Constants::PERMISSION_READ);
+
+		$result = \OCP\Share::getItemsSharedWithUser('test', $this->user2);
+		$this->assertCount(1, $result);
+
+		$groupShareId = array_keys($result)[0];
+
+		// remove user from group
+		$userObject = \OC::$server->getUserManager()->get($this->user2);
+		\OC::$server->getGroupManager()->get($this->group1)->removeUser($userObject);
+
+		$result = \OCP\Share::getItemsSharedWithUser('test', $this->user2);
+		$this->assertCount(0, $result);
+
+		// test with buggy data
+		$qb = \OC::$server->getDatabaseConnection()->getQueryBuilder();
+		$qb->insert('share')
+			->values([
+				'share_type' => $qb->expr()->literal(2), // group sub-share
+				'share_with' => $qb->expr()->literal($this->user2),
+				'parent' => $qb->expr()->literal($groupShareId),
+				'uid_owner' => $qb->expr()->literal($this->user1),
+				'item_type' => $qb->expr()->literal('test'),
+				'item_source' => $qb->expr()->literal('test.txt'),
+				'item_target' => $qb->expr()->literal('test.txt'),
+				'file_target' => $qb->expr()->literal('test2.txt'),
+				'permissions' => $qb->expr()->literal(1),
+				'stime' => $qb->expr()->literal(time()),
+			])->execute();
+
+		$result = \OCP\Share::getItemsSharedWithUser('test', $this->user2);
+		$this->assertCount(0, $result);
+
+		$qb->delete('share')->execute();
+	}
+
 	public function testShareItemWithLink() {
 		OC_User::setUserId($this->user1);
 		$token = OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_LINK, null, \OCP\Constants::PERMISSION_READ);