From 20091a21c93ee03b7d19b186dcf49f32f6919dfa Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 5 Jul 2017 13:52:51 +0200 Subject: [PATCH] Don't load navigation entries of restricted apps Signed-off-by: Joas Schilling --- lib/private/NavigationManager.php | 13 +++++- tests/lib/NavigationManagerTest.php | 64 +++++++++++++++++++++++------ 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 300c24ff940..aff931037a3 100644 --- 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 0871a9a0910..de432e1eaf2 100644 --- 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']]]] ]; } } -- 2.39.5