Skip to content

Commit b23c2ca

Browse files
jancborchardtbackportbot[bot]
authored andcommitted
fix(core): hide personal settings of apps not enabled for the user
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Jan C. Borchardt <925062+jancborchardt@users.noreply.github.com>
1 parent 0418a83 commit b23c2ca

4 files changed

Lines changed: 120 additions & 10 deletions

File tree

lib/private/App/AppManager.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -521,32 +521,32 @@ public function loadApp(string $app): void {
521521
$settingsManager = Server::get(ISettingsManager::class);
522522
if (!empty($info['settings']['admin'])) {
523523
foreach ($info['settings']['admin'] as $setting) {
524-
$settingsManager->registerSetting('admin', $setting);
524+
$settingsManager->registerSetting('admin', $setting, $app);
525525
}
526526
}
527527
if (!empty($info['settings']['admin-section'])) {
528528
foreach ($info['settings']['admin-section'] as $section) {
529-
$settingsManager->registerSection('admin', $section);
529+
$settingsManager->registerSection('admin', $section, $app);
530530
}
531531
}
532532
if (!empty($info['settings']['personal'])) {
533533
foreach ($info['settings']['personal'] as $setting) {
534-
$settingsManager->registerSetting('personal', $setting);
534+
$settingsManager->registerSetting('personal', $setting, $app);
535535
}
536536
}
537537
if (!empty($info['settings']['personal-section'])) {
538538
foreach ($info['settings']['personal-section'] as $section) {
539-
$settingsManager->registerSection('personal', $section);
539+
$settingsManager->registerSection('personal', $section, $app);
540540
}
541541
}
542542
if (!empty($info['settings']['admin-delegation'])) {
543543
foreach ($info['settings']['admin-delegation'] as $setting) {
544-
$settingsManager->registerSetting(ISettingsManager::SETTINGS_DELEGATION, $setting);
544+
$settingsManager->registerSetting(ISettingsManager::SETTINGS_DELEGATION, $setting, $app);
545545
}
546546
}
547547
if (!empty($info['settings']['admin-delegation-section'])) {
548548
foreach ($info['settings']['admin-delegation-section'] as $section) {
549-
$settingsManager->registerSection(ISettingsManager::SETTINGS_DELEGATION, $section);
549+
$settingsManager->registerSection(ISettingsManager::SETTINGS_DELEGATION, $section, $app);
550550
}
551551
}
552552
}

lib/private/Settings/Manager.php

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace OC\Settings;
99

1010
use Closure;
11+
use OCP\App\IAppManager;
1112
use OCP\AppFramework\QueryException;
1213
use OCP\Group\ISubAdmin;
1314
use OCP\IGroupManager;
@@ -38,6 +39,9 @@ class Manager implements IManager {
3839
/** @var array<self::SETTINGS_*, array<string, list<ISettings>>> */
3940
protected array $settings = [];
4041

42+
/** @var array<class-string<ISettings|IIconSection>, string> App each class was registered by */
43+
protected array $appIds = [];
44+
4145
public function __construct(
4246
private LoggerInterface $log,
4347
private IFactory $l10nFactory,
@@ -46,19 +50,23 @@ public function __construct(
4650
private AuthorizedGroupMapper $mapper,
4751
private IGroupManager $groupManager,
4852
private ISubAdmin $subAdmin,
53+
private IAppManager $appManager,
4954
) {
5055
}
5156

5257
/**
5358
* @inheritdoc
5459
*/
5560
#[\Override]
56-
public function registerSection(string $type, string $section) {
61+
public function registerSection(string $type, string $section, ?string $appId = null) {
5762
if (!isset($this->sectionClasses[$type])) {
5863
$this->sectionClasses[$type] = [];
5964
}
6065

6166
$this->sectionClasses[$type][] = $section;
67+
if ($appId !== null) {
68+
$this->appIds[$section] = $appId;
69+
}
6270
}
6371

6472
/**
@@ -76,6 +84,11 @@ protected function getSections(string $type): array {
7684
}
7785

7886
foreach (array_unique($this->sectionClasses[$type]) as $index => $class) {
87+
if ($type === self::SETTINGS_PERSONAL && !$this->isAvailableToCurrentUser($class)) {
88+
unset($this->sectionClasses[$type][$index]);
89+
continue;
90+
}
91+
7992
try {
8093
/** @var IIconSection $section */
8194
$section = $this->container->get($class);
@@ -122,8 +135,28 @@ protected function isKnownDuplicateSectionId(string $sectionID): bool {
122135
* @inheritdoc
123136
*/
124137
#[\Override]
125-
public function registerSetting(string $type, string $setting) {
138+
public function registerSetting(string $type, string $setting, ?string $appId = null) {
126139
$this->settingClasses[$setting] = $type;
140+
if ($appId !== null) {
141+
$this->appIds[$setting] = $appId;
142+
}
143+
}
144+
145+
/**
146+
* Apps can be limited to some groups, but their settings are registered for
147+
* every user. So check the app of a setting or section is available to the
148+
* current user before showing it.
149+
*
150+
* @param class-string<ISettings|IIconSection> $class
151+
*/
152+
protected function isAvailableToCurrentUser(string $class): bool {
153+
$appId = $this->appIds[$class] ?? null;
154+
if ($appId === null) {
155+
// Not registered by an app, e.g. a built-in setting.
156+
return true;
157+
}
158+
159+
return $this->appManager->isEnabledForUser($appId);
127160
}
128161

129162
/**
@@ -145,6 +178,11 @@ protected function getSettings(string $type, string $section, ?Closure $filter =
145178
continue;
146179
}
147180

181+
if ($type === self::SETTINGS_PERSONAL && !$this->isAvailableToCurrentUser($class)) {
182+
unset($this->settingClasses[$class]);
183+
continue;
184+
}
185+
148186
try {
149187
/** @var ISettings $setting */
150188
$setting = $this->container->get($class);

lib/public/Settings/IManager.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,20 @@ interface IManager {
5656
/**
5757
* @psalm-param self::SETTINGS_* $type
5858
* @param class-string<IIconSection> $section
59+
* @param ?string $appId app the section belongs to, so personal sections of
60+
* apps not enabled for the user can be hidden (since 35.0.0)
5961
* @since 14.0.0
6062
*/
61-
public function registerSection(string $type, string $section);
63+
public function registerSection(string $type, string $section, ?string $appId = null);
6264

6365
/**
6466
* @psalm-param self::SETTINGS_* $type
6567
* @param class-string<ISettings> $setting
68+
* @param ?string $appId app the setting belongs to, so personal settings of
69+
* apps not enabled for the user can be hidden (since 35.0.0)
6670
* @since 14.0.0
6771
*/
68-
public function registerSetting(string $type, string $setting);
72+
public function registerSetting(string $type, string $setting, ?string $appId = null);
6973

7074
/**
7175
* returns a list of the admin sections

tests/lib/Settings/ManagerTest.php

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OC\Settings\AuthorizedGroupMapper;
1111
use OC\Settings\Manager;
1212
use OCA\WorkflowEngine\Settings\Section;
13+
use OCP\App\IAppManager;
1314
use OCP\Group\ISubAdmin;
1415
use OCP\IGroupManager;
1516
use OCP\IL10N;
@@ -32,6 +33,7 @@ class ManagerTest extends TestCase {
3233
private AuthorizedGroupMapper&MockObject $mapper;
3334
private IGroupManager&MockObject $groupManager;
3435
private ISubAdmin&MockObject $subAdmin;
36+
private IAppManager&MockObject $appManager;
3537

3638
private Manager $manager;
3739

@@ -47,6 +49,7 @@ protected function setUp(): void {
4749
$this->mapper = $this->createMock(AuthorizedGroupMapper::class);
4850
$this->groupManager = $this->createMock(IGroupManager::class);
4951
$this->subAdmin = $this->createMock(ISubAdmin::class);
52+
$this->appManager = $this->createMock(IAppManager::class);
5053

5154
$this->manager = new Manager(
5255
$this->logger,
@@ -56,6 +59,7 @@ protected function setUp(): void {
5659
$this->mapper,
5760
$this->groupManager,
5861
$this->subAdmin,
62+
$this->appManager,
5963
);
6064
}
6165

@@ -186,6 +190,70 @@ public function testGetPersonalSettings(): void {
186190
], $settings);
187191
}
188192

193+
public function testGetPersonalSettingsHidesSettingsOfAppsNotEnabledForUser(): void {
194+
$visible = $this->createMock(ISettings::class);
195+
$visible->method('getPriority')
196+
->willReturn(16);
197+
$visible->method('getSection')
198+
->willReturn('security');
199+
200+
$this->manager->registerSetting('personal', 'visibleClass', 'enabled_app');
201+
$this->manager->registerSetting('personal', 'hiddenClass', 'restricted_app');
202+
203+
$this->appManager->method('isEnabledForUser')
204+
->willReturnCallback(static fn (string $appId): bool => $appId === 'enabled_app');
205+
206+
// The settings of the app the user has no access to are never instantiated.
207+
$this->container->expects($this->once())
208+
->method('get')
209+
->with('visibleClass')
210+
->willReturn($visible);
211+
212+
$this->assertEquals([
213+
16 => [$visible],
214+
], $this->manager->getPersonalSettings('security'));
215+
}
216+
217+
public function testGetPersonalSectionsHidesSectionsOfAppsNotEnabledForUser(): void {
218+
$this->l10nFactory->method('get')
219+
->with('lib')
220+
->willReturn($this->l10n);
221+
$this->l10n->method('t')
222+
->willReturnArgument(0);
223+
224+
$this->manager->registerSection('personal', Section::class, 'restricted_app');
225+
226+
$this->appManager->method('isEnabledForUser')
227+
->with('restricted_app')
228+
->willReturn(false);
229+
230+
$this->container->expects($this->never())
231+
->method('get');
232+
233+
$this->assertEquals([], $this->manager->getPersonalSections());
234+
}
235+
236+
public function testGetAdminSettingsAreNotHiddenForAppsNotEnabledForUser(): void {
237+
// Admins configure apps they are not a member of themselves.
238+
$setting = $this->createMock(ISettings::class);
239+
$setting->method('getPriority')
240+
->willReturn(13);
241+
$setting->method('getSection')
242+
->willReturn('sharing');
243+
244+
$this->manager->registerSetting('admin', 'myAdminClass', 'restricted_app');
245+
246+
$this->appManager->expects($this->never())
247+
->method('isEnabledForUser');
248+
$this->container->method('get')
249+
->with('myAdminClass')
250+
->willReturn($setting);
251+
252+
$this->assertEquals([
253+
13 => [$setting],
254+
], $this->manager->getAdminSettings('sharing'));
255+
}
256+
189257
public function testSameSectionAsPersonalAndAdmin(): void {
190258
$this->l10nFactory
191259
->expects($this->once())

0 commit comments

Comments
 (0)