Merge pull request #21723 from owncloud/prevent-group-enable-for-apps

Prevent group enable for apps

apps/provisioning_api/appinfo/info.xml

@@ -17,10 +17,9 @@
 	<documentation>
 		<admin>admin-provisioning-api</admin>
 	</documentation>
-	<version>0.4.0</version>
+	<version>0.4.1</version>
 	<types>
-		<!-- this is used to disable the feature of enabling an app for specific groups only because this would break this app -->
-		<filesystem/>
+		<prevent_group_restriction/>
 	</types>
 	<dependencies>
 		<owncloud min-version="9.0" max-version="9.0" />

lib/private/app/appmanager.php

@@ -35,6 +35,18 @@ use OCP\IUserSession;
 
 class AppManager implements IAppManager {
 
+	/**
+	 * Apps with these types can not be enabled for certain groups only
+	 * @var string[]
+	 */
+	protected $protectedAppTypes = [
+		'filesystem',
+		'prelogin',
+		'authentication',
+		'logging',
+		'prevent_group_restriction',
+	];
+
 	/** @var \OCP\IUserSession */
 	private $userSession;
 
@@ -197,8 +209,17 @@ class AppManager implements IAppManager {
 	 *
 	 * @param string $appId
 	 * @param \OCP\IGroup[] $groups
+	 * @throws \Exception if app can't be enabled for groups
 	 */
 	public function enableAppForGroups($appId, $groups) {
+		$info = $this->getAppInfo($appId);
+		if (!empty($info['types'])) {
+			$protectedTypes = array_intersect($this->protectedAppTypes, $info['types']);
+			if (!empty($protectedTypes)) {
+				throw new \Exception("$appId can't be enabled for groups.");
+			}
+		}
+
 		$groupIds = array_map(function ($group) {
 			/** @var \OCP\IGroup $group */
 			return $group->getGID();

settings/js/apps.js

@@ -176,7 +176,8 @@ OC.Settings.Apps = OC.Settings.Apps || {
 
 		// set group select properly
 		if(OC.Settings.Apps.isType(app, 'filesystem') || OC.Settings.Apps.isType(app, 'prelogin') ||
-			OC.Settings.Apps.isType(app, 'authentication') || OC.Settings.Apps.isType(app, 'logging')) {
+			OC.Settings.Apps.isType(app, 'authentication') || OC.Settings.Apps.isType(app, 'logging') ||
+			OC.Settings.Apps.isType(app, 'prevent_group_restriction')) {
 			page.find(".groups-enable").hide();
 			page.find(".groups-enable__checkbox").attr('checked', null);
 		} else {

tests/lib/app/manager.php

@@ -13,6 +13,12 @@ use OC\Group\Group;
 use OC\User\User;
 use Test\TestCase;
 
+/**
+ * Class Manager
+ *
+ * @package Test\App
+ * @group DB
+ */
 class Manager extends TestCase {
 	/**
 	 * @return \OCP\IAppConfig | \PHPUnit_Framework_MockObject_MockObject
@@ -116,6 +122,93 @@ class Manager extends TestCase {
 		$this->assertEquals('["group1","group2"]', $this->appConfig->getValue('test', 'enabled', 'no'));
 	}
 
+	public function dataEnableAppForGroupsAllowedTypes() {
+		return [
+			[[]],
+			[[
+				'types' => [],
+			]],
+			[[
+				'types' => ['nickvergessen'],
+			]],
+		];
+	}
+
+	/**
+	 * @dataProvider dataEnableAppForGroupsAllowedTypes
+	 *
+	 * @param array $appInfo
+	 */
+	public function testEnableAppForGroupsAllowedTypes(array $appInfo) {
+		$groups = array(
+			new Group('group1', array(), null),
+			new Group('group2', array(), null)
+		);
+		$this->expectClearCache();
+
+		/** @var \OC\App\AppManager|\PHPUnit_Framework_MockObject_MockObject $manager */
+		$manager = $this->getMockBuilder('OC\App\AppManager')
+			->setConstructorArgs([
+				$this->userSession, $this->appConfig, $this->groupManager, $this->cacheFactory
+			])
+			->setMethods([
+				'getAppInfo'
+			])
+			->getMock();
+
+		$manager->expects($this->once())
+			->method('getAppInfo')
+			->with('test')
+			->willReturn($appInfo);
+
+		$manager->enableAppForGroups('test', $groups);
+		$this->assertEquals('["group1","group2"]', $this->appConfig->getValue('test', 'enabled', 'no'));
+	}
+
+	public function dataEnableAppForGroupsForbiddenTypes() {
+		return [
+			['filesystem'],
+			['prelogin'],
+			['authentication'],
+			['logging'],
+			['prevent_group_restriction'],
+		];
+	}
+
+	/**
+	 * @dataProvider dataEnableAppForGroupsForbiddenTypes
+	 *
+	 * @param string $type
+	 *
+	 * @expectedException \Exception
+	 * @expectedExceptionMessage test can't be enabled for groups.
+	 */
+	public function testEnableAppForGroupsForbiddenTypes($type) {
+		$groups = array(
+			new Group('group1', array(), null),
+			new Group('group2', array(), null)
+		);
+
+		/** @var \OC\App\AppManager|\PHPUnit_Framework_MockObject_MockObject $manager */
+		$manager = $this->getMockBuilder('OC\App\AppManager')
+			->setConstructorArgs([
+				$this->userSession, $this->appConfig, $this->groupManager, $this->cacheFactory
+			])
+			->setMethods([
+				'getAppInfo'
+			])
+			->getMock();
+
+		$manager->expects($this->once())
+			->method('getAppInfo')
+			->with('test')
+			->willReturn([
+				'types' => [$type],
+			]);
+
+		$manager->enableAppForGroups('test', $groups);
+	}
+
 	public function testIsInstalledEnabled() {
 		$this->appConfig->setValue('test', 'enabled', 'yes');
 		$this->assertTrue($this->manager->isInstalled('test'));