Don't load navigation entries of restricted apps

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

2 files changed, 64 insertions, 13 deletions

diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php
index 16552005931..b854b44b340 100644
--- a/lib/private/NavigationManager.php
+++ b/lib/private/NavigationManager.php
@@ -216,7 +216,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']]]]
 		];
 	}
 }