Respect disabled group sharing in sharee endpoint

* Fix tests

3 files changed, 69 insertions, 35 deletions

diff --git a/apps/files_sharing/api/sharees.php b/apps/files_sharing/api/sharees.php
index 718be4dece9..4e005c5e26c 100644
--- a/apps/files_sharing/api/sharees.php
+++ b/apps/files_sharing/api/sharees.php
@@ -62,6 +62,9 @@ class Sharees {
 	/** @var ILogger */
 	protected $logger;
 
+	/** @var \OCP\Share\IManager */
+	protected $shareManager;
+
 	/** @var bool */
 	protected $shareWithGroupOnly = false;
 
@@ -97,6 +100,7 @@ class Sharees {
 	 * @param IURLGenerator $urlGenerator
 	 * @param IRequest $request
 	 * @param ILogger $logger
+	 * @param \OCP\Share\IManager $shareManager
 	 */
 	public function __construct(IGroupManager $groupManager,
 								IUserManager $userManager,
@@ -105,7 +109,8 @@ class Sharees {
 								IUserSession $userSession,
 								IURLGenerator $urlGenerator,
 								IRequest $request,
-								ILogger $logger) {
+								ILogger $logger,
+								\OCP\Share\IManager $shareManager) {
 		$this->groupManager = $groupManager;
 		$this->userManager = $userManager;
 		$this->contactsManager = $contactsManager;
@@ -114,6 +119,7 @@ class Sharees {
 		$this->urlGenerator = $urlGenerator;
 		$this->request = $request;
 		$this->logger = $logger;
+		$this->shareManager = $shareManager;
 	}
 
 	/**
@@ -411,9 +417,14 @@ class Sharees {
 
 		$shareTypes = [
 			Share::SHARE_TYPE_USER,
-			Share::SHARE_TYPE_GROUP,
-			Share::SHARE_TYPE_REMOTE,
 		];
+
+		if ($this->shareManager->allowGroupSharing()) {
+			$shareTypes[] = Share::SHARE_TYPE_GROUP;
+		}
+
+		$shareTypes[] = Share::SHARE_TYPE_REMOTE;
+
 		if (isset($_GET['shareType']) && is_array($_GET['shareType'])) {
 			$shareTypes = array_intersect($shareTypes, $_GET['shareType']);
 			sort($shareTypes);
diff --git a/apps/files_sharing/appinfo/routes.php b/apps/files_sharing/appinfo/routes.php
index 961206079be..80632f0fedf 100644
--- a/apps/files_sharing/appinfo/routes.php
+++ b/apps/files_sharing/appinfo/routes.php
@@ -126,7 +126,8 @@ $sharees = new \OCA\Files_Sharing\API\Sharees(\OC::$server->getGroupManager(),
                                               \OC::$server->getUserSession(),
                                               \OC::$server->getURLGenerator(),
                                               \OC::$server->getRequest(),
-                                              \OC::$server->getLogger());
+                                              \OC::$server->getLogger(),
+                                              \OC::$server->getShareManager());
 
 API::register('get',
 		'/apps/files_sharing/api/v1/sharees',
diff --git a/apps/files_sharing/tests/api/shareestest.php b/apps/files_sharing/tests/api/shareestest.php
index 87d143d0853..cda41f55183 100644
--- a/apps/files_sharing/tests/api/shareestest.php
+++ b/apps/files_sharing/tests/api/shareestest.php
@@ -55,6 +55,9 @@ class ShareesTest extends TestCase {
 	/** @var \OCP\IRequest|\PHPUnit_Framework_MockObject_MockObject */
 	protected $request;
 
+	/** @var \OCP\Share\IManager|\PHPUnit_Framework_MockObject_MockObject */
+	protected $shareManager;
+
 	protected function setUp() {
 		parent::setUp();
 
@@ -78,6 +81,10 @@ class ShareesTest extends TestCase {
 			->disableOriginalConstructor()
 			->getMock();
 
+		$this->shareManager = $this->getMockBuilder('OCP\Share\IManager')
+			->disableOriginalConstructor()
+			->getMock();
+
 		$this->sharees = new Sharees(
 			$this->groupManager,
 			$this->userManager,
@@ -86,7 +93,8 @@ class ShareesTest extends TestCase {
 			$this->session,
 			$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
 			$this->request,
-			$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock()
+			$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock(),
+			$this->shareManager
 		);
 	}
 
@@ -966,89 +974,95 @@ class ShareesTest extends TestCase {
 		$allTypes = [Share::SHARE_TYPE_USER, Share::SHARE_TYPE_GROUP, Share::SHARE_TYPE_REMOTE];
 
 		return [
-			[[], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
+			[[], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
 
 			// Test itemType
 			[[
 				'search' => '',
-			], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
+			], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
 			[[
 				'search' => 'foobar',
-			], '', 'yes', true, 'foobar', null, $allTypes, 1, 200, false, true],
+			], '', 'yes', true, 'foobar', null, $allTypes, 1, 200, false, true, true],
 			[[
 				'search' => 0,
-			], '', 'yes', true, '0', null, $allTypes, 1, 200, false, true],
+			], '', 'yes', true, '0', null, $allTypes, 1, 200, false, true, true],
 
 			// Test itemType
 			[[
 				'itemType' => '',
-			], '', 'yes', true, '', '', $allTypes, 1, 200, false, true],
+			], '', 'yes', true, '', '', $allTypes, 1, 200, false, true, true],
 			[[
 				'itemType' => 'folder',
-			], '', 'yes', true, '', 'folder', $allTypes, 1, 200, false, true],
+			], '', 'yes', true, '', 'folder', $allTypes, 1, 200, false, true, true],
 			[[
 				'itemType' => 0,
-			], '', 'yes', true, '', '0', $allTypes, 1, 200, false, true],
+			], '', 'yes', true, '', '0', $allTypes, 1, 200, false, true, true],
 
 			// Test shareType
 			[[
-			], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
+			], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
 			[[
 				'shareType' => 0,
-			], '', 'yes', true, '', null, [0], 1, 200, false, true],
+			], '', 'yes', true, '', null, [0], 1, 200, false, true, true],
 			[[
 				'shareType' => '0',
-			], '', 'yes', true, '', null, [0], 1, 200, false, true],
+			], '', 'yes', true, '', null, [0], 1, 200, false, true, true],
 			[[
 				'shareType' => 1,
-			], '', 'yes', true, '', null, [1], 1, 200, false, true],
+			], '', 'yes', true, '', null, [1], 1, 200, false, true, true],
 			[[
 				'shareType' => 12,
-			], '', 'yes', true, '', null, [], 1, 200, false, true],
+			], '', 'yes', true, '', null, [], 1, 200, false, true, true],
 			[[
 				'shareType' => 'foobar',
-			], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
+			], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
 			[[
 				'shareType' => [0, 1, 2],
-			], '', 'yes', true, '', null, [0, 1], 1, 200, false, true],
+			], '', 'yes', true, '', null, [0, 1], 1, 200, false, true, true],
 			[[
 				'shareType' => [0, 1],
-			], '', 'yes', true, '', null, [0, 1], 1, 200, false, true],
+			], '', 'yes', true, '', null, [0, 1], 1, 200, false, true, true],
 			[[
 				'shareType' => $allTypes,
-			], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
+			], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
 			[[
 				'shareType' => $allTypes,
-			], '', 'yes', false, '', null, [0, 1], 1, 200, false, true],
+			], '', 'yes', false, '', null, [0, 1], 1, 200, false, true, true],
+			[[
+				'shareType' => $allTypes,
+			], '', 'yes', true, '', null, [0, 6], 1, 200, false, true, false],
+			[[
+				'shareType' => $allTypes,
+			], '', 'yes', false, '', null, [0], 1, 200, false, true, false],
 
 			// Test pagination
 			[[
 				'page' => 1,
-			], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
+			], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
 			[[
 				'page' => 10,
-			], '', 'yes', true, '', null, $allTypes, 10, 200, false, true],
+			], '', 'yes', true, '', null, $allTypes, 10, 200, false, true, true],
 
 			// Test perPage
 			[[
 				'perPage' => 1,
-			], '', 'yes', true, '', null, $allTypes, 1, 1, false, true],
+			], '', 'yes', true, '', null, $allTypes, 1, 1, false, true, true],
 			[[
 				'perPage' => 10,
-			], '', 'yes', true, '', null, $allTypes, 1, 10, false, true],
+			], '', 'yes', true, '', null, $allTypes, 1, 10, false, true, true],
 
 			// Test $shareWithGroupOnly setting
-			[[], 'no', 'yes',  true, '', null, $allTypes, 1, 200, false, true],
-			[[], 'yes', 'yes', true, '', null, $allTypes, 1, 200, true, true],
+			[[], 'no', 'yes',  true, '', null, $allTypes, 1, 200, false, true, true],
+			[[], 'yes', 'yes', true, '', null, $allTypes, 1, 200, true, true, true],
 
 			// Test $shareeEnumeration setting
-			[[], 'no', 'yes',  true, '', null, $allTypes, 1, 200, false, true],
-			[[], 'no', 'no', true, '', null, $allTypes, 1, 200, false, false],
+			[[], 'no', 'yes',  true, '', null, $allTypes, 1, 200, false, true, true],
+			[[], 'no', 'no', true, '', null, $allTypes, 1, 200, false, false, true],
 
 			// Test keep case for search
 			[[
 				'search' => 'foo@example.com/ownCloud',
-			], '', 'yes', true, 'foo@example.com/ownCloud', null, $allTypes, 1, 200, false, true],
+			], '', 'yes', true, 'foo@example.com/ownCloud', null, $allTypes, 1, 200, false, true, true],
 		];
 	}
 
@@ -1066,8 +1080,9 @@ class ShareesTest extends TestCase {
 	 * @param int $perPage
 	 * @param bool $shareWithGroupOnly
 	 * @param bool $shareeEnumeration
+	 * @param bool $allowGroupSharing
 	 */
-	public function testSearch($getData, $apiSetting, $enumSetting, $remoteSharingEnabled, $search, $itemType, $shareTypes, $page, $perPage, $shareWithGroupOnly, $shareeEnumeration) {
+	public function testSearch($getData, $apiSetting, $enumSetting, $remoteSharingEnabled, $search, $itemType, $shareTypes, $page, $perPage, $shareWithGroupOnly, $shareeEnumeration, $allowGroupSharing) {
 		$oldGet = $_GET;
 		$_GET = $getData;
 
@@ -1082,6 +1097,10 @@ class ShareesTest extends TestCase {
 				['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', $enumSetting],
 			]);
 
+		$this->shareManager->expects($this->once())
+			->method('allowGroupSharing')
+			->willReturn($allowGroupSharing);
+
 		$sharees = $this->getMockBuilder('\OCA\Files_Sharing\API\Sharees')
 			->setConstructorArgs([
 				$this->groupManager,
@@ -1091,7 +1110,8 @@ class ShareesTest extends TestCase {
 				$this->session,
 				$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
 				$this->getMockBuilder('OCP\IRequest')->disableOriginalConstructor()->getMock(),
-				$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock()
+				$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock(),
+				$this->shareManager
 			])
 			->setMethods(array('searchSharees', 'isRemoteSharingAllowed'))
 			->getMock();
@@ -1175,7 +1195,8 @@ class ShareesTest extends TestCase {
 				$this->session,
 				$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
 				$this->getMockBuilder('OCP\IRequest')->disableOriginalConstructor()->getMock(),
-				$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock()
+				$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock(),
+				$this->shareManager
 			])
 			->setMethods(array('searchSharees', 'isRemoteSharingAllowed'))
 			->getMock();
@@ -1327,7 +1348,8 @@ class ShareesTest extends TestCase {
 				$this->session,
 				$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
 				$this->getMockBuilder('OCP\IRequest')->disableOriginalConstructor()->getMock(),
-				$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock()
+				$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock(),
+				$this->shareManager
 			])
 			->setMethods(array('getShareesForShareIds', 'getUsers', 'getGroups', 'getRemote'))
 			->getMock();