Merge pull request #13846 from nextcloud/feature/check-if-app-exist-for-group

Check app path for enableAppForGroups

lib/private/App/AppManager.php

@@ -254,15 +254,16 @@ class AppManager implements IAppManager {
 	 *
 	 * @param string $appId
 	 * @param \OCP\IGroup[] $groups
-	 * @throws \Exception if app can't be enabled for groups
+	 * @throws \InvalidArgumentException if app can't be enabled for groups
+	 * @throws AppPathNotFoundException
 	 */
 	public function enableAppForGroups($appId, $groups) {
+		// Check if app exists
+		$this->getAppPath($appId);
+
 		$info = $this->getAppInfo($appId);
-		if (!empty($info['types'])) {
+		if (!empty($info['types']) && $this->hasProtectedAppType($info['types'])) {
-			$protectedTypes = array_intersect($this->protectedAppTypes, $info['types']);
+			throw new \InvalidArgumentException("$appId can't be enabled for groups.");
-			if (!empty($protectedTypes)) {
-				throw new \Exception("$appId can't be enabled for groups.");
-			}
 		}
 
 		$groupIds = array_map(function ($group) {

tests/lib/App/AppManagerTest.php

@@ -149,7 +149,23 @@ class AppManagerTest extends TestCase {
 			new Group('group2', array(), null)
 		);
 		$this->expectClearCache();
-		$this->manager->enableAppForGroups('test', $groups);
+
+		/** @var AppManager|\PHPUnit_Framework_MockObject_MockObject $manager */
+		$manager = $this->getMockBuilder(AppManager::class)
+			->setConstructorArgs([
+				$this->userSession, $this->appConfig, $this->groupManager, $this->cacheFactory, $this->eventDispatcher
+			])
+			->setMethods([
+				'getAppPath',
+			])
+			->getMock();
+
+		$manager->expects($this->exactly(2))
+			->method('getAppPath')
+			->with('test')
+			->willReturn('apps/test');
+
+		$manager->enableAppForGroups('test', $groups);
 		$this->assertEquals('["group1","group2"]', $this->appConfig->getValue('test', 'enabled', 'no'));
 	}
 
@@ -183,10 +199,16 @@ class AppManagerTest extends TestCase {
 				$this->userSession, $this->appConfig, $this->groupManager, $this->cacheFactory, $this->eventDispatcher
 			])
 			->setMethods([
-				'getAppInfo'
+				'getAppPath',
+				'getAppInfo',
 			])
 			->getMock();
 
+		$manager->expects($this->once())
+			->method('getAppPath')
+			->with('test')
+			->willReturn(null);
+
 		$manager->expects($this->once())
 			->method('getAppInfo')
 			->with('test')
@@ -226,10 +248,16 @@ class AppManagerTest extends TestCase {
 				$this->userSession, $this->appConfig, $this->groupManager, $this->cacheFactory, $this->eventDispatcher
 			])
 			->setMethods([
-				'getAppInfo'
+				'getAppPath',
+				'getAppInfo',
 			])
 			->getMock();
 
+		$manager->expects($this->once())
+			->method('getAppPath')
+			->with('test')
+			->willReturn(null);
+
 		$manager->expects($this->once())
 			->method('getAppInfo')
 			->with('test')