Remove deleted groups from app restrictions fixes #15823

Signed-off-by: Greta Doci <gretadoci@gmail.com>

diff --git a/lib/base.php b/lib/base.php
index 30d57153de986751ce86345da4f7cda65884d165..7812922c168d0b8c52b0f5c0133df9e7c894d616 100644 (file)
--- 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 7d4dbbbd34eeb4d3dd3b4d97083f3abe3a934bf7..77756999c793c698671cf90749ec48d5ee3fd57c 100644 (file)
--- 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 b0d04500f35f3e48e35d0e395e27cc3e1b4a0a4d..6213227bfd1e50a208d0000b3e39c48eff6aa54b 100644 (file)
--- 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 cb94ccf44d09af46fcd0d8639236440e6fc7105b..ba693999205aa009f8befc14fe93bd567cf05e69 100644 (file)
--- 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'));
+       }
 }