From 5898e87e0f69ed4a3be73cd044c19d7c4872b639 Mon Sep 17 00:00:00 2001 From: Greta Doci Date: Tue, 25 Jun 2019 15:20:06 +0200 Subject: [PATCH] Remove deleted groups from app restrictions fixes #15823 Signed-off-by: Greta Doci --- lib/base.php | 25 ++++++++++++ lib/private/App/AppManager.php | 65 +++++++++++++++++++++++++++++++- lib/public/App/IAppManager.php | 15 ++++++++ tests/lib/App/AppManagerTest.php | 36 ++++++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) diff --git a/lib/base.php b/lib/base.php index 30d57153de..7812922c16 100644 --- a/lib/base.php +++ b/lib/base.php @@ -717,6 +717,7 @@ class OC { self::registerEncryptionHooks(); self::registerAccountHooks(); self::registerResourceCollectionHooks(); + self::registerAppRestrictionsHooks(); // Make sure that the application class is not loaded before the database is setup if ($systemConfig->getValue("installed", false)) { @@ -848,6 +849,30 @@ class OC { \OCP\Util::connectHook('OC_User', 'changeUser', $hookHandler, 'changeUserHook'); } + private static function registerAppRestrictionsHooks() { + $groupManager = self::$server->query(\OCP\IGroupManager::class); + $groupManager->listen ('\OC\Group', 'postDelete', function (\OCP\IGroup $group) { + $appManager = self::$server->getAppManager(); + $apps = $appManager->getEnabledAppsForGroup($group); + foreach ($apps as $appId) { + $restrictions = $appManager->getAppRestriction($appId); + if (empty($restrictions)) { + continue; + } + $key = array_search($group->getGID(), $restrictions); + unset($restrictions[$key]); + $restrictions = array_values($restrictions); + if (empty($restrictions)) { + $appManager->disableApp($appId); + } + else{ + $appManager->enableAppForGroups($appId, $restrictions); + } + + } + }); + } + private static function registerResourceCollectionHooks() { \OC\Collaboration\Resources\Listener::register(\OC::$server->getEventDispatcher()); } diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index 7d4dbbbd34..77756999c7 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -37,6 +37,7 @@ use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use OCP\App\ManagerEvent; use OCP\ICacheFactory; +use OCP\IGroup; use OCP\IGroupManager; use OCP\IUser; use OCP\IUserSession; @@ -148,6 +149,36 @@ class AppManager implements IAppManager { return array_keys($appsForUser); } + /** + * @param \OCP\IGroup $group + * @return array + */ + public function getEnabledAppsForGroup(IGroup $group): array { + $apps = $this->getInstalledAppsValues(); + $appsForGroups = array_filter($apps, function ($enabled) use ($group) { + return $this->checkAppForGroups($enabled, $group); + }); + return array_keys($appsForGroups); + } + + /** + * @param string $appId + * @return array + */ + public function getAppRestriction(string $appId): array { + $values = $this->getInstalledAppsValues(); + + if (!isset($values[$appId])) { + return []; + } + + if ($values[$appId] === 'yes' || $values[$appId] === 'no') { + return []; + } + return json_decode($values[$appId]); + } + + /** * Check if an app is enabled for user * @@ -203,12 +234,40 @@ class AppManager implements IAppManager { } } + /** + * @param string $enabled + * @param IGroup $group + * @return bool + */ + private function checkAppForGroups(string $enabled, IGroup $group): bool { + if ($enabled === 'yes') { + return true; + } elseif ($group === null) { + return false; + } else { + if (empty($enabled)) { + return false; + } + + $groupIds = json_decode($enabled); + + if (!is_array($groupIds)) { + $jsonError = json_last_error(); + \OC::$server->getLogger()->warning('AppManger::checkAppForUser - can\'t decode group IDs: ' . print_r($enabled, true) . ' - json error code: ' . $jsonError, ['app' => 'lib']); + return false; + } + + return in_array($group->getGID(), $groupIds); + } + } + /** * Check if an app is enabled in the instance * * Notice: This actually checks if the app is enabled and not only if it is installed. * * @param string $appId + * @param \OCP\IGroup[]|String[] $groups * @return bool */ public function isInstalled($appId) { @@ -268,14 +327,18 @@ class AppManager implements IAppManager { $groupIds = array_map(function ($group) { /** @var \OCP\IGroup $group */ - return $group->getGID(); + return ($group instanceof IGroup) + ? $group->getGID() + : $group; }, $groups); + $this->installedAppsCache[$appId] = json_encode($groupIds); $this->appConfig->setValue($appId, 'enabled', json_encode($groupIds)); $this->dispatcher->dispatch(ManagerEvent::EVENT_APP_ENABLE_FOR_GROUPS, new ManagerEvent( ManagerEvent::EVENT_APP_ENABLE_FOR_GROUPS, $appId, $groups )); $this->clearAppsCache(); + } /** diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php index b0d04500f3..6213227bfd 100644 --- a/lib/public/App/IAppManager.php +++ b/lib/public/App/IAppManager.php @@ -28,6 +28,7 @@ namespace OCP\App; use OCP\IUser; +use OCP\IGroup; /** * Interface IAppManager @@ -158,4 +159,18 @@ interface IAppManager { * @since 9.0.0 */ public function getAlwaysEnabledApps(); + + /** + * @param \OCP\IGroup $group + * @return String[] + * @since 17.0.0 + */ + public function getEnabledAppsForGroup(IGroup $group): array; + + /** + * @param String $appId + * @return string[] + * @since 17.0.0 + */ + public function getAppRestriction(string $appId): array; } diff --git a/tests/lib/App/AppManagerTest.php b/tests/lib/App/AppManagerTest.php index cb94ccf44d..ba69399920 100644 --- a/tests/lib/App/AppManagerTest.php +++ b/tests/lib/App/AppManagerTest.php @@ -504,4 +504,40 @@ class AppManagerTest extends TestCase { $this->assertEquals('test1', $apps[0]['id']); $this->assertEquals('test3', $apps[1]['id']); } + + public function testGetEnabledAppsForGroup() { + $group = $this->createMock(IGroup::class); + $group->expects($this->any()) + ->method('getGID') + ->will($this->returnValue('foo')); + + $this->appConfig->setValue('test1', 'enabled', 'yes'); + $this->appConfig->setValue('test2', 'enabled', 'no'); + $this->appConfig->setValue('test3', 'enabled', '["foo"]'); + $this->appConfig->setValue('test4', 'enabled', '["asd"]'); + $enabled = [ + 'cloud_federation_api', + 'dav', + 'federatedfilesharing', + 'files', + 'lookup_server_connector', + 'oauth2', + 'provisioning_api', + 'test1', + 'test3', + 'twofactor_backupcodes', + 'workflowengine', + ]; + $this->assertEquals($enabled, $this->manager->getEnabledAppsForGroup($group)); + } + + public function testGetAppRestriction() { + $this->appConfig->setValue('test1', 'enabled', 'yes'); + $this->appConfig->setValue('test2', 'enabled', 'no'); + $this->appConfig->setValue('test3', 'enabled', '["foo"]'); + + $this->assertEquals([], $this->manager->getAppRestriction('test1')); + $this->assertEquals([], $this->manager->getAppRestriction('test2')); + $this->assertEquals(['foo'], $this->manager->getAppRestriction('test3')); + } }