diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index 1f91825733713..4746b32aaf9f1 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -521,32 +521,32 @@ public function loadApp(string $app): void { $settingsManager = Server::get(ISettingsManager::class); if (!empty($info['settings']['admin'])) { foreach ($info['settings']['admin'] as $setting) { - $settingsManager->registerSetting('admin', $setting); + $settingsManager->registerSetting('admin', $setting, $app); } } if (!empty($info['settings']['admin-section'])) { foreach ($info['settings']['admin-section'] as $section) { - $settingsManager->registerSection('admin', $section); + $settingsManager->registerSection('admin', $section, $app); } } if (!empty($info['settings']['personal'])) { foreach ($info['settings']['personal'] as $setting) { - $settingsManager->registerSetting('personal', $setting); + $settingsManager->registerSetting('personal', $setting, $app); } } if (!empty($info['settings']['personal-section'])) { foreach ($info['settings']['personal-section'] as $section) { - $settingsManager->registerSection('personal', $section); + $settingsManager->registerSection('personal', $section, $app); } } if (!empty($info['settings']['admin-delegation'])) { foreach ($info['settings']['admin-delegation'] as $setting) { - $settingsManager->registerSetting(ISettingsManager::SETTINGS_DELEGATION, $setting); + $settingsManager->registerSetting(ISettingsManager::SETTINGS_DELEGATION, $setting, $app); } } if (!empty($info['settings']['admin-delegation-section'])) { foreach ($info['settings']['admin-delegation-section'] as $section) { - $settingsManager->registerSection(ISettingsManager::SETTINGS_DELEGATION, $section); + $settingsManager->registerSection(ISettingsManager::SETTINGS_DELEGATION, $section, $app); } } } diff --git a/lib/private/Settings/Manager.php b/lib/private/Settings/Manager.php index 34c097b9f3063..27922ae93688c 100644 --- a/lib/private/Settings/Manager.php +++ b/lib/private/Settings/Manager.php @@ -8,6 +8,7 @@ namespace OC\Settings; use Closure; +use OCP\App\IAppManager; use OCP\AppFramework\QueryException; use OCP\Group\ISubAdmin; use OCP\IGroupManager; @@ -38,6 +39,9 @@ class Manager implements IManager { /** @var array>> */ protected array $settings = []; + /** @var array, string> App each class was registered by */ + protected array $appIds = []; + public function __construct( private LoggerInterface $log, private IFactory $l10nFactory, @@ -46,6 +50,7 @@ public function __construct( private AuthorizedGroupMapper $mapper, private IGroupManager $groupManager, private ISubAdmin $subAdmin, + private IAppManager $appManager, ) { } @@ -53,12 +58,15 @@ public function __construct( * @inheritdoc */ #[\Override] - public function registerSection(string $type, string $section) { + public function registerSection(string $type, string $section, ?string $appId = null) { if (!isset($this->sectionClasses[$type])) { $this->sectionClasses[$type] = []; } $this->sectionClasses[$type][] = $section; + if ($appId !== null) { + $this->appIds[$section] = $appId; + } } /** @@ -76,6 +84,11 @@ protected function getSections(string $type): array { } foreach (array_unique($this->sectionClasses[$type]) as $index => $class) { + if ($type === self::SETTINGS_PERSONAL && !$this->isAvailableToCurrentUser($class)) { + unset($this->sectionClasses[$type][$index]); + continue; + } + try { /** @var IIconSection $section */ $section = $this->container->get($class); @@ -122,8 +135,28 @@ protected function isKnownDuplicateSectionId(string $sectionID): bool { * @inheritdoc */ #[\Override] - public function registerSetting(string $type, string $setting) { + public function registerSetting(string $type, string $setting, ?string $appId = null) { $this->settingClasses[$setting] = $type; + if ($appId !== null) { + $this->appIds[$setting] = $appId; + } + } + + /** + * Apps can be limited to some groups, but their settings are registered for + * every user. So check the app of a setting or section is available to the + * current user before showing it. + * + * @param class-string $class + */ + protected function isAvailableToCurrentUser(string $class): bool { + $appId = $this->appIds[$class] ?? null; + if ($appId === null) { + // Not registered by an app, e.g. a built-in setting. + return true; + } + + return $this->appManager->isEnabledForUser($appId); } /** @@ -145,6 +178,11 @@ protected function getSettings(string $type, string $section, ?Closure $filter = continue; } + if ($type === self::SETTINGS_PERSONAL && !$this->isAvailableToCurrentUser($class)) { + unset($this->settingClasses[$class]); + continue; + } + try { /** @var ISettings $setting */ $setting = $this->container->get($class); diff --git a/lib/public/Settings/IManager.php b/lib/public/Settings/IManager.php index 1a22705d3e702..b3b90ef9d76af 100644 --- a/lib/public/Settings/IManager.php +++ b/lib/public/Settings/IManager.php @@ -56,16 +56,20 @@ interface IManager { /** * @psalm-param self::SETTINGS_* $type * @param class-string $section + * @param ?string $appId app the section belongs to, so personal sections of + * apps not enabled for the user can be hidden (since 35.0.0) * @since 14.0.0 */ - public function registerSection(string $type, string $section); + public function registerSection(string $type, string $section, ?string $appId = null); /** * @psalm-param self::SETTINGS_* $type * @param class-string $setting + * @param ?string $appId app the setting belongs to, so personal settings of + * apps not enabled for the user can be hidden (since 35.0.0) * @since 14.0.0 */ - public function registerSetting(string $type, string $setting); + public function registerSetting(string $type, string $setting, ?string $appId = null); /** * returns a list of the admin sections diff --git a/tests/lib/Settings/ManagerTest.php b/tests/lib/Settings/ManagerTest.php index 2fe7838e71bf6..6f2f33008a054 100644 --- a/tests/lib/Settings/ManagerTest.php +++ b/tests/lib/Settings/ManagerTest.php @@ -10,6 +10,7 @@ use OC\Settings\AuthorizedGroupMapper; use OC\Settings\Manager; use OCA\WorkflowEngine\Settings\Section; +use OCP\App\IAppManager; use OCP\Group\ISubAdmin; use OCP\IGroupManager; use OCP\IL10N; @@ -32,6 +33,7 @@ class ManagerTest extends TestCase { private AuthorizedGroupMapper&MockObject $mapper; private IGroupManager&MockObject $groupManager; private ISubAdmin&MockObject $subAdmin; + private IAppManager&MockObject $appManager; private Manager $manager; @@ -47,6 +49,7 @@ protected function setUp(): void { $this->mapper = $this->createMock(AuthorizedGroupMapper::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->subAdmin = $this->createMock(ISubAdmin::class); + $this->appManager = $this->createMock(IAppManager::class); $this->manager = new Manager( $this->logger, @@ -56,6 +59,7 @@ protected function setUp(): void { $this->mapper, $this->groupManager, $this->subAdmin, + $this->appManager, ); } @@ -186,6 +190,70 @@ public function testGetPersonalSettings(): void { ], $settings); } + public function testGetPersonalSettingsHidesSettingsOfAppsNotEnabledForUser(): void { + $visible = $this->createMock(ISettings::class); + $visible->method('getPriority') + ->willReturn(16); + $visible->method('getSection') + ->willReturn('security'); + + $this->manager->registerSetting('personal', 'visibleClass', 'enabled_app'); + $this->manager->registerSetting('personal', 'hiddenClass', 'restricted_app'); + + $this->appManager->method('isEnabledForUser') + ->willReturnCallback(static fn (string $appId): bool => $appId === 'enabled_app'); + + // The settings of the app the user has no access to are never instantiated. + $this->container->expects($this->once()) + ->method('get') + ->with('visibleClass') + ->willReturn($visible); + + $this->assertEquals([ + 16 => [$visible], + ], $this->manager->getPersonalSettings('security')); + } + + public function testGetPersonalSectionsHidesSectionsOfAppsNotEnabledForUser(): void { + $this->l10nFactory->method('get') + ->with('lib') + ->willReturn($this->l10n); + $this->l10n->method('t') + ->willReturnArgument(0); + + $this->manager->registerSection('personal', Section::class, 'restricted_app'); + + $this->appManager->method('isEnabledForUser') + ->with('restricted_app') + ->willReturn(false); + + $this->container->expects($this->never()) + ->method('get'); + + $this->assertEquals([], $this->manager->getPersonalSections()); + } + + public function testGetAdminSettingsAreNotHiddenForAppsNotEnabledForUser(): void { + // Admins configure apps they are not a member of themselves. + $setting = $this->createMock(ISettings::class); + $setting->method('getPriority') + ->willReturn(13); + $setting->method('getSection') + ->willReturn('sharing'); + + $this->manager->registerSetting('admin', 'myAdminClass', 'restricted_app'); + + $this->appManager->expects($this->never()) + ->method('isEnabledForUser'); + $this->container->method('get') + ->with('myAdminClass') + ->willReturn($setting); + + $this->assertEquals([ + 13 => [$setting], + ], $this->manager->getAdminSettings('sharing')); + } + public function testSameSectionAsPersonalAndAdmin(): void { $this->l10nFactory ->expects($this->once())