Don't load navigation entries of restricted apps

Signed-off-by: Joas Schilling <coding@schilljs.com>

diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php
index 300c24ff940e9b0e7a86eade3e826320fc67d5bb..aff931037a36906880b7b0a696463df91b487063 100644 (file)
--- a/lib/private/NavigationManager.php
+++ b/lib/private/NavigationManager.php
@@ -228,7 +228,18 @@ class NavigationManager implements INavigationManager {
                if ($this->appManager === 'null') {
                        return;
                }
-               foreach ($this->appManager->getInstalledApps() as $app) {
+
+               if ($this->userSession->isLoggedIn()) {
+                       $apps = $this->appManager->getEnabledAppsForUser($this->userSession->getUser());
+               } else {
+                       $apps = $this->appManager->getInstalledApps();
+               }
+
+               foreach ($apps as $app) {
+                       if (!$this->userSession->isLoggedIn() && !$this->appManager->isEnabledForUser($app, $this->userSession->getUser())) {
+                               continue;
+                       }
+
                        // load plugins and collections from info.xml
                        $info = $this->appManager->getAppInfo($app);
                        if (empty($info['navigations'])) {

diff --git a/tests/lib/NavigationManagerTest.php b/tests/lib/NavigationManagerTest.php
index 0871a9a0910cbf01be8123c2dd3edaee0840e752..de432e1eaf2e027e57bcda486d7a37493e433d52 100644 (file)
--- a/tests/lib/NavigationManagerTest.php
+++ b/tests/lib/NavigationManagerTest.php
@@ -13,7 +13,9 @@
 namespace Test;
 
 use OC\App\AppManager;
+use OC\Group\Manager;
 use OC\NavigationManager;
+use OC\SubAdmin;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
@@ -46,7 +48,7 @@ class NavigationManagerTest extends TestCase {
                $this->urlGenerator = $this->createMock(IURLGenerator::class);
                $this->l10nFac = $this->createMock(IFactory::class);
                $this->userSession = $this->createMock(IUserSession::class);
-               $this->groupManager = $this->createMock(IGroupManager::class);
+               $this->groupManager = $this->createMock(Manager::class);
                $this->config = $this->createMock(IConfig::class);
                $this->navigationManager = new NavigationManager(
                        $this->appManager,
@@ -207,19 +209,26 @@ class NavigationManagerTest extends TestCase {
                        return vsprintf($text, $parameters);
                });
 
-               $this->appManager->expects($this->once())->method('getInstalledApps')->willReturn(['test']);
                $this->appManager->expects($this->once())->method('getAppInfo')->with('test')->willReturn($navigation);
-               $this->l10nFac->expects($this->exactly(count($expected) + 1))->method('get')->willReturn($l);
+               $this->l10nFac->expects($this->any())->method('get')->willReturn($l);
                $this->urlGenerator->expects($this->any())->method('imagePath')->willReturnCallback(function($appName, $file) {
                        return "/apps/$appName/img/$file";
                });
-               $this->urlGenerator->expects($this->exactly(count($expected)))->method('linkToRoute')->willReturnCallback(function() {
+               $this->urlGenerator->expects($this->any())->method('linkToRoute')->willReturnCallback(function() {
                        return "/apps/test/";
                });
                $user = $this->createMock(IUser::class);
                $user->expects($this->any())->method('getUID')->willReturn('user001');
                $this->userSession->expects($this->any())->method('getUser')->willReturn($user);
+               $this->userSession->expects($this->any())->method('isLoggedIn')->willReturn(true);
+               $this->appManager->expects($this->once())
+                       ->method('getEnabledAppsForUser')
+                       ->with($user)
+                       ->willReturn(['test']);
                $this->groupManager->expects($this->any())->method('isAdmin')->willReturn($isAdmin);
+               $subadmin = $this->createMock(SubAdmin::class);
+               $subadmin->expects($this->any())->method('isSubAdmin')->with($user)->willReturn(false);
+               $this->groupManager->expects($this->any())->method('getSubAdmin')->willReturn($subadmin);
 
                $this->navigationManager->clear();
                $entries = $this->navigationManager->getAll('all');
@@ -227,8 +236,39 @@ class NavigationManagerTest extends TestCase {
        }
 
        public function providesNavigationConfig() {
+               $apps = [
+                       [
+                               'id' => 'core_apps',
+                               'order' => 3,
+                               'href' => '/apps/test/',
+                               'icon' => '/apps/settings/img/apps.svg',
+                               'name' => 'Apps',
+                               'active' => false,
+                               'type' => 'settings',
+                       ]
+               ];
+               $defaults = [
+                       [
+                               'id' => 'settings',
+                               'order' => 1,
+                               'href' => '/apps/test/',
+                               'icon' => '/apps/settings/img/admin.svg',
+                               'name' => 'Settings',
+                               'active' => false,
+                               'type' => 'settings',
+                       ],
+                       [
+                               'id' => 'logout',
+                               'order' => 99999,
+                               'href' => null,
+                               'icon' => '/apps/core/img/actions/logout.svg',
+                               'name' => 'Log out',
+                               'active' => false,
+                               'type' => 'settings',
+                       ],
+               ];
                return [
-                       'minimalistic' => [[[
+                       'minimalistic' => [array_merge($defaults, [[
                                'id' => 'test',
                                'order' => 100,
                                'href' => '/apps/test/',
@@ -236,8 +276,8 @@ class NavigationManagerTest extends TestCase {
                                'name' => 'Test',
                                'active' => false,
                                'type' => 'link',
-                       ]], ['navigations' => [['route' => 'test.page.index', 'name' => 'Test']]]],
-                       'minimalistic-settings' => [[[
+                       ]]), ['navigations' => [['route' => 'test.page.index', 'name' => 'Test']]]],
+                       'minimalistic-settings' => [array_merge($defaults, [[
                                'id' => 'test',
                                'order' => 100,
                                'href' => '/apps/test/',
@@ -245,8 +285,8 @@ class NavigationManagerTest extends TestCase {
                                'name' => 'Test',
                                'active' => false,
                                'type' => 'settings',
-                       ]], ['navigations' => [['route' => 'test.page.index', 'name' => 'Test', 'type' => 'settings']]]],
-                       'no admin' => [[[
+                       ]]), ['navigations' => [['route' => 'test.page.index', 'name' => 'Test', 'type' => 'settings']]]],
+                       'admin' => [array_merge($apps, $defaults, [[
                                'id' => 'test',
                                'order' => 100,
                                'href' => '/apps/test/',
@@ -254,9 +294,9 @@ class NavigationManagerTest extends TestCase {
                                'name' => 'Test',
                                'active' => false,
                                'type' => 'link',
-                       ]], ['navigations' => [['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index', 'name' => 'Test']]], true],
-                       'no name' => [[], ['navigations' => [['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index']]], true],
-                       'admin' => [[], ['navigations' => [['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index', 'name' => 'Test']]]]
+                       ]]), ['navigations' => [['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index', 'name' => 'Test']]], true],
+                       'no name' => [array_merge($apps, $defaults), ['navigations' => [['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index']]], true],
+                       'no admin' => [$defaults, ['navigations' => [['@attributes' => ['role' => 'admin'], 'route' => 'test.page.index', 'name' => 'Test']]]]
                ];
        }
 }