diff --git a/IONOS b/IONOS index 9e0e3357464d0..0d518e210b96e 160000 --- a/IONOS +++ b/IONOS @@ -1 +1 @@ -Subproject commit 9e0e3357464d03431d9b230ace945e7ec49de3fd +Subproject commit 0d518e210b96e2796f0ae44ef89693efde70ad07 diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 8732881f9966c..2d8fcc2965dc4 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -63,6 +63,7 @@ 'OCA\\Settings\\Sections\\Personal\\Security' => $baseDir . '/../lib/Sections/Personal/Security.php', 'OCA\\Settings\\Sections\\Personal\\SyncClients' => $baseDir . '/../lib/Sections/Personal/SyncClients.php', 'OCA\\Settings\\Service\\AuthorizedGroupService' => $baseDir . '/../lib/Service/AuthorizedGroupService.php', + 'OCA\\Settings\\Service\\ConflictException' => $baseDir . '/../lib/Service/ConflictException.php', 'OCA\\Settings\\Service\\NotFoundException' => $baseDir . '/../lib/Service/NotFoundException.php', 'OCA\\Settings\\Service\\ServiceException' => $baseDir . '/../lib/Service/ServiceException.php', 'OCA\\Settings\\Settings\\Admin\\ArtificialIntelligence' => $baseDir . '/../lib/Settings/Admin/ArtificialIntelligence.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index c7358bc249df0..336d1d60cd1ec 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -78,6 +78,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\Sections\\Personal\\Security' => __DIR__ . '/..' . '/../lib/Sections/Personal/Security.php', 'OCA\\Settings\\Sections\\Personal\\SyncClients' => __DIR__ . '/..' . '/../lib/Sections/Personal/SyncClients.php', 'OCA\\Settings\\Service\\AuthorizedGroupService' => __DIR__ . '/..' . '/../lib/Service/AuthorizedGroupService.php', + 'OCA\\Settings\\Service\\ConflictException' => __DIR__ . '/..' . '/../lib/Service/ConflictException.php', 'OCA\\Settings\\Service\\NotFoundException' => __DIR__ . '/..' . '/../lib/Service/NotFoundException.php', 'OCA\\Settings\\Service\\ServiceException' => __DIR__ . '/..' . '/../lib/Service/ServiceException.php', 'OCA\\Settings\\Settings\\Admin\\ArtificialIntelligence' => __DIR__ . '/..' . '/../lib/Settings/Admin/ArtificialIntelligence.php', diff --git a/apps/settings/lib/Command/AdminDelegation/Add.php b/apps/settings/lib/Command/AdminDelegation/Add.php index 5cbef5c5d157b..d1ab66e24cf58 100644 --- a/apps/settings/lib/Command/AdminDelegation/Add.php +++ b/apps/settings/lib/Command/AdminDelegation/Add.php @@ -9,6 +9,7 @@ use OC\Core\Command\Base; use OCA\Settings\Service\AuthorizedGroupService; +use OCA\Settings\Service\ConflictException; use OCP\IGroupManager; use OCP\Settings\IDelegatedSettings; use OCP\Settings\IManager; @@ -50,7 +51,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 3; } - $this->authorizedGroupService->create($groupId, $settingClass); + try { + $this->authorizedGroupService->create($groupId, $settingClass); + } catch (ConflictException) { + $io->warning('The ' . $settingClass . ' is already delegated to ' . $groupId . '.'); + return 4; + } $io->success('Administration of ' . $settingClass . ' delegated to ' . $groupId . '.'); diff --git a/apps/settings/lib/Controller/CommonSettingsTrait.php b/apps/settings/lib/Controller/CommonSettingsTrait.php index 56760c10f8147..55d67afba7f79 100644 --- a/apps/settings/lib/Controller/CommonSettingsTrait.php +++ b/apps/settings/lib/Controller/CommonSettingsTrait.php @@ -141,8 +141,12 @@ private function getIndexResponse(string $type, string $section): TemplateRespon $user = $this->userSession->getUser(); assert($user !== null, 'No user logged in for settings'); - $this->declarativeSettingsManager->loadSchemas(); - $declarativeSettings = $this->declarativeSettingsManager->getFormsWithValues($user, $type, $section); + $declarativeSettings = []; + + if ($type === "admin" && $this->groupManager->isAdmin($user->getUID())) { + $this->declarativeSettingsManager->loadSchemas(); + $declarativeSettings = $this->declarativeSettingsManager->getFormsWithValues($user, $type, $section); + } if ($type === 'personal') { $settings = array_values($this->settingsManager->getPersonalSettings($section)); diff --git a/apps/settings/lib/Service/AuthorizedGroupService.php b/apps/settings/lib/Service/AuthorizedGroupService.php index 20966446d6187..c95b4260f796d 100644 --- a/apps/settings/lib/Service/AuthorizedGroupService.php +++ b/apps/settings/lib/Service/AuthorizedGroupService.php @@ -57,8 +57,19 @@ private function handleException(\Exception $e): void { * @param string $class * @return AuthorizedGroup * @throws Exception + * @throws ConflictException */ public function create(string $groupId, string $class): AuthorizedGroup { + // Check if the group is already assigned to this class + try { + $existing = $this->mapper->findByGroupIdAndClass($groupId, $class); + if ($existing) { + throw new ConflictException('Group is already assigned to this class'); + } + } catch (DoesNotExistException $e) { + // This is expected when no duplicate exists, continue with creation + } + $authorizedGroup = new AuthorizedGroup(); $authorizedGroup->setGroupId($groupId); $authorizedGroup->setClass($class); diff --git a/apps/settings/lib/Service/ConflictException.php b/apps/settings/lib/Service/ConflictException.php new file mode 100644 index 0000000000000..eab17d7cf0b12 --- /dev/null +++ b/apps/settings/lib/Service/ConflictException.php @@ -0,0 +1,10 @@ +settingManager = clone $settingManager; } /** @@ -117,4 +124,12 @@ public function getSection() { public function getPriority() { return 75; } + + public function getName(): string { + return $this->l10n->t('Delegation'); + } + + public function getAuthorizedAppConfig(): array { + return []; + } } diff --git a/apps/settings/tests/Command/AdminDelegation/AddTest.php b/apps/settings/tests/Command/AdminDelegation/AddTest.php new file mode 100644 index 0000000000000..a5c791ab6a552 --- /dev/null +++ b/apps/settings/tests/Command/AdminDelegation/AddTest.php @@ -0,0 +1,184 @@ +settingManager = $this->createMock(IManager::class); + $this->authorizedGroupService = $this->createMock(AuthorizedGroupService::class); + $this->groupManager = $this->createMock(IGroupManager::class); + + $this->command = new Add( + $this->settingManager, + $this->authorizedGroupService, + $this->groupManager + ); + + $this->input = $this->createMock(InputInterface::class); + $this->output = $this->createMock(OutputInterface::class); + } + + /** + * Helper method to execute the command using reflection since execute() is protected + */ + private function executeCommand(): int { + $reflection = new \ReflectionClass($this->command); + $method = $reflection->getMethod('execute'); + $method->setAccessible(true); + return $method->invokeArgs($this->command, [$this->input, $this->output]); + } + + public function testExecuteSuccessfulDelegation(): void { + $settingClass = 'OCA\\Settings\\Settings\\Admin\\Server'; + $groupId = 'testgroup'; + + // Mock valid delegated settings class + $this->input->expects($this->exactly(2)) + ->method('getArgument') + ->willReturnMap([ + ['settingClass', $settingClass], + ['groupId', $groupId] + ]); + + // Mock group exists + $this->groupManager->expects($this->once()) + ->method('groupExists') + ->with($groupId) + ->willReturn(true); + + // Mock successful creation + $authorizedGroup = new AuthorizedGroup(); + $authorizedGroup->setGroupId($groupId); + $authorizedGroup->setClass($settingClass); + + $this->authorizedGroupService->expects($this->once()) + ->method('create') + ->with($groupId, $settingClass) + ->willReturn($authorizedGroup); + + $result = $this->executeCommand(); + + $this->assertEquals(0, $result); + } + + public function testExecuteHandlesDuplicateAssignment(): void { + $settingClass = 'OCA\\Settings\\Settings\\Admin\\Server'; + $groupId = 'testgroup'; + + // Mock valid delegated settings class + $this->input->expects($this->exactly(2)) + ->method('getArgument') + ->willReturnMap([ + ['settingClass', $settingClass], + ['groupId', $groupId] + ]); + + // Mock group exists + $this->groupManager->expects($this->once()) + ->method('groupExists') + ->with($groupId) + ->willReturn(true); + + // Mock ConflictException when trying to create duplicate + $this->authorizedGroupService->expects($this->once()) + ->method('create') + ->with($groupId, $settingClass) + ->willThrowException(new ConflictException('Group is already assigned to this class')); + + $result = $this->executeCommand(); + + // Should return exit code 4 for conflict + $this->assertEquals(4, $result); + } + + public function testExecuteInvalidSettingClass(): void { + // Use a real class that exists but doesn't implement IDelegatedSettings + $settingClass = 'stdClass'; + + $this->input->expects($this->once()) + ->method('getArgument') + ->with('settingClass') + ->willReturn($settingClass); + + $result = $this->executeCommand(); + + // Should return exit code 2 for invalid setting class + $this->assertEquals(2, $result); + } + + public function testExecuteNonExistentGroup(): void { + $settingClass = 'OCA\\Settings\\Settings\\Admin\\Server'; + $groupId = 'nonexistentgroup'; + + $this->input->expects($this->exactly(2)) + ->method('getArgument') + ->willReturnMap([ + ['settingClass', $settingClass], + ['groupId', $groupId] + ]); + + // Mock group does not exist + $this->groupManager->expects($this->once()) + ->method('groupExists') + ->with($groupId) + ->willReturn(false); + + $result = $this->executeCommand(); + + // Should return exit code 3 for non-existent group + $this->assertEquals(3, $result); + } + + public function testExecuteReturnsDifferentExitCodesForDifferentErrors(): void { + // Test that duplicate assignment returns code 4 + $settingClass = 'OCA\\Settings\\Settings\\Admin\\Server'; + $groupId = 'testgroup'; + + $this->input->expects($this->exactly(2)) + ->method('getArgument') + ->willReturnMap([ + ['settingClass', $settingClass], + ['groupId', $groupId] + ]); + + $this->groupManager->expects($this->once()) + ->method('groupExists') + ->with($groupId) + ->willReturn(true); + + $this->authorizedGroupService->expects($this->once()) + ->method('create') + ->with($groupId, $settingClass) + ->willThrowException(new ConflictException('Group is already assigned to this class')); + + $result = $this->executeCommand(); + + $this->assertEquals(4, $result, 'Duplicate assignment should return exit code 4'); + } +} diff --git a/apps/settings/tests/Integration/DuplicateAssignmentIntegrationTest.php b/apps/settings/tests/Integration/DuplicateAssignmentIntegrationTest.php new file mode 100644 index 0000000000000..6e78f3bdc8907 --- /dev/null +++ b/apps/settings/tests/Integration/DuplicateAssignmentIntegrationTest.php @@ -0,0 +1,156 @@ +mapper = \OC::$server->get(AuthorizedGroupMapper::class); + $this->service = new AuthorizedGroupService($this->mapper); + } + + protected function tearDown(): void { + // Clean up any test data + try { + $allGroups = $this->mapper->findAll(); + foreach ($allGroups as $group) { + if (str_starts_with($group->getGroupId(), 'test_') || + str_starts_with($group->getClass(), 'TestClass')) { + $this->mapper->delete($group); + } + } + } catch (\Exception $e) { + // Ignore cleanup errors + } + parent::tearDown(); + } + + public function testDuplicateAssignmentPrevention(): void { + $groupId = 'test_duplicate_group'; + $class = 'TestClass\\DuplicateTest'; + + // First assignment should succeed + $result1 = $this->service->create($groupId, $class); + $this->assertInstanceOf(AuthorizedGroup::class, $result1); + $this->assertEquals($groupId, $result1->getGroupId()); + $this->assertEquals($class, $result1->getClass()); + $this->assertNotNull($result1->getId()); + + // Second assignment of same group to same class should throw ConflictException + $this->expectException(ConflictException::class); + $this->expectExceptionMessage('Group is already assigned to this class'); + + $this->service->create($groupId, $class); + } + + public function testDifferentGroupsSameClassAllowed(): void { + $groupId1 = 'test_group_1'; + $groupId2 = 'test_group_2'; + $class = 'TestClass\\MultiGroup'; + + // Both assignments should succeed + $result1 = $this->service->create($groupId1, $class); + $result2 = $this->service->create($groupId2, $class); + + $this->assertEquals($groupId1, $result1->getGroupId()); + $this->assertEquals($groupId2, $result2->getGroupId()); + $this->assertEquals($class, $result1->getClass()); + $this->assertEquals($class, $result2->getClass()); + $this->assertNotEquals($result1->getId(), $result2->getId()); + } + + public function testSameGroupDifferentClassesAllowed(): void { + $groupId = 'test_multi_class_group'; + $class1 = 'TestClass\\First'; + $class2 = 'TestClass\\Second'; + + // Both assignments should succeed + $result1 = $this->service->create($groupId, $class1); + $result2 = $this->service->create($groupId, $class2); + + $this->assertEquals($groupId, $result1->getGroupId()); + $this->assertEquals($groupId, $result2->getGroupId()); + $this->assertEquals($class1, $result1->getClass()); + $this->assertEquals($class2, $result2->getClass()); + $this->assertNotEquals($result1->getId(), $result2->getId()); + } + + public function testCreateAfterDelete(): void { + $groupId = 'test_recreate_group'; + $class = 'TestClass\\Recreate'; + + // Create initial assignment + $result1 = $this->service->create($groupId, $class); + $initialId = $result1->getId(); + + // Delete the assignment + $this->service->delete($initialId); + + // Verify it's deleted by trying to find it + $this->expectException(\OCP\AppFramework\Db\DoesNotExistException::class); + try { + $this->service->find($initialId); + } catch (\OCA\Settings\Service\NotFoundException $e) { + // Expected - now create the same assignment again, which should succeed + $result2 = $this->service->create($groupId, $class); + + $this->assertEquals($groupId, $result2->getGroupId()); + $this->assertEquals($class, $result2->getClass()); + $this->assertNotEquals($initialId, $result2->getId()); + return; + } + + $this->fail('Expected NotFoundException when finding deleted group'); + } + + /** + * Test the mapper's findByGroupIdAndClass method behavior with duplicates + */ + public function testMapperFindByGroupIdAndClassBehavior(): void { + $groupId = 'test_mapper_group'; + $class = 'TestClass\\MapperTest'; + + // Initially should throw DoesNotExistException + $this->expectException(DoesNotExistException::class); + $this->mapper->findByGroupIdAndClass($groupId, $class); + } + + /** + * Test that mapper returns existing record after creation + */ + public function testMapperFindsExistingRecord(): void { + $groupId = 'test_existing_group'; + $class = 'TestClass\\Existing'; + + // Create the record first + $created = $this->service->create($groupId, $class); + + // Now mapper should find it + $found = $this->mapper->findByGroupIdAndClass($groupId, $class); + + $this->assertEquals($created->getId(), $found->getId()); + $this->assertEquals($groupId, $found->getGroupId()); + $this->assertEquals($class, $found->getClass()); + } +} diff --git a/apps/settings/tests/Service/AuthorizedGroupServiceTest.php b/apps/settings/tests/Service/AuthorizedGroupServiceTest.php new file mode 100644 index 0000000000000..e820e589f05a1 --- /dev/null +++ b/apps/settings/tests/Service/AuthorizedGroupServiceTest.php @@ -0,0 +1,156 @@ +mapper = $this->createMock(AuthorizedGroupMapper::class); + $this->service = new AuthorizedGroupService($this->mapper); + } + + public function testCreateSuccessWhenNoDuplicateExists(): void { + $groupId = 'testgroup'; + $class = 'TestClass'; + + // Mock that no existing assignment is found (throws DoesNotExistException) + $this->mapper->expects($this->once()) + ->method('findByGroupIdAndClass') + ->with($groupId, $class) + ->willThrowException(new DoesNotExistException('Not found')); + + // Mock the successful creation + $expectedGroup = new AuthorizedGroup(); + $expectedGroup->setGroupId($groupId); + $expectedGroup->setClass($class); + $expectedGroup->setId(123); + + $this->mapper->expects($this->once()) + ->method('insert') + ->willReturn($expectedGroup); + + $result = $this->service->create($groupId, $class); + + $this->assertInstanceOf(AuthorizedGroup::class, $result); + $this->assertEquals($groupId, $result->getGroupId()); + $this->assertEquals($class, $result->getClass()); + } + + public function testCreateThrowsConflictExceptionWhenDuplicateExists(): void { + $groupId = 'testgroup'; + $class = 'TestClass'; + + // Mock that an existing assignment is found + $existingGroup = new AuthorizedGroup(); + $existingGroup->setGroupId($groupId); + $existingGroup->setClass($class); + $existingGroup->setId(42); + + $this->mapper->expects($this->once()) + ->method('findByGroupIdAndClass') + ->with($groupId, $class) + ->willReturn($existingGroup); + + // Mapper insert should never be called when duplicate exists + $this->mapper->expects($this->never()) + ->method('insert'); + + $this->expectException(ConflictException::class); + $this->expectExceptionMessage('Group is already assigned to this class'); + + $this->service->create($groupId, $class); + } + + public function testCreateAllowsDifferentGroupsSameClass(): void { + $groupId1 = 'testgroup1'; + $groupId2 = 'testgroup2'; + $class = 'TestClass'; + + // Mock that no duplicate exists for group1 + $this->mapper->expects($this->exactly(2)) + ->method('findByGroupIdAndClass') + ->willReturnCallback(function($groupId, $classArg) use ($groupId1, $groupId2, $class) { + $this->assertContains($groupId, [$groupId1, $groupId2]); + $this->assertEquals($class, $classArg); + throw new DoesNotExistException('Not found'); + }); + + $expectedGroup1 = new AuthorizedGroup(); + $expectedGroup1->setGroupId($groupId1); + $expectedGroup1->setClass($class); + $expectedGroup1->setId(123); + + $expectedGroup2 = new AuthorizedGroup(); + $expectedGroup2->setGroupId($groupId2); + $expectedGroup2->setClass($class); + $expectedGroup2->setId(124); + + $this->mapper->expects($this->exactly(2)) + ->method('insert') + ->willReturnOnConsecutiveCalls($expectedGroup1, $expectedGroup2); + + // Both creations should succeed + $result1 = $this->service->create($groupId1, $class); + $result2 = $this->service->create($groupId2, $class); + + $this->assertEquals($groupId1, $result1->getGroupId()); + $this->assertEquals($groupId2, $result2->getGroupId()); + $this->assertEquals($class, $result1->getClass()); + $this->assertEquals($class, $result2->getClass()); + } + + public function testCreateAllowsSameGroupDifferentClasses(): void { + $groupId = 'testgroup'; + $class1 = 'TestClass1'; + $class2 = 'TestClass2'; + + // Mock that no duplicate exists for either class + $this->mapper->expects($this->exactly(2)) + ->method('findByGroupIdAndClass') + ->willReturnCallback(function($groupIdArg, $class) use ($groupId, $class1, $class2) { + $this->assertEquals($groupId, $groupIdArg); + $this->assertContains($class, [$class1, $class2]); + throw new DoesNotExistException('Not found'); + }); + + $expectedGroup1 = new AuthorizedGroup(); + $expectedGroup1->setGroupId($groupId); + $expectedGroup1->setClass($class1); + $expectedGroup1->setId(123); + + $expectedGroup2 = new AuthorizedGroup(); + $expectedGroup2->setGroupId($groupId); + $expectedGroup2->setClass($class2); + $expectedGroup2->setId(124); + + $this->mapper->expects($this->exactly(2)) + ->method('insert') + ->willReturnOnConsecutiveCalls($expectedGroup1, $expectedGroup2); + + // Both creations should succeed + $result1 = $this->service->create($groupId, $class1); + $result2 = $this->service->create($groupId, $class2); + + $this->assertEquals($groupId, $result1->getGroupId()); + $this->assertEquals($groupId, $result2->getGroupId()); + $this->assertEquals($class1, $result1->getClass()); + $this->assertEquals($class2, $result2->getClass()); + } +} diff --git a/apps/settings/tests/Settings/Admin/DelegationTest.php b/apps/settings/tests/Settings/Admin/DelegationTest.php new file mode 100644 index 0000000000000..76027936d7943 --- /dev/null +++ b/apps/settings/tests/Settings/Admin/DelegationTest.php @@ -0,0 +1,151 @@ +settingManager = $this->createMock(IManager::class); + $this->initialStateService = $this->createMock(IInitialState::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->authorizedGroupService = $this->createMock(AuthorizedGroupService::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->l10n = $this->createMock(IL10N::class); + + $this->delegation = new Delegation( + $this->settingManager, + $this->initialStateService, + $this->groupManager, + $this->authorizedGroupService, + $this->urlGenerator, + $this->l10n, + ); + } + + public function testImplementsIDelegatedSettings(): void { + $this->assertInstanceOf(IDelegatedSettings::class, $this->delegation); + } + + public function testGetPriority(): void { + $this->assertEquals(75, $this->delegation->getPriority()); + } + + public function testGetName(): void { + $this->l10n->expects($this->once()) + ->method('t') + ->with('Delegation') + ->willReturn('Delegation'); + + $this->assertEquals('Delegation', $this->delegation->getName()); + } + + public function testGetAuthorizedAppConfig(): void { + $this->assertEquals([], $this->delegation->getAuthorizedAppConfig()); + } + + public function testGetSection(): void { + $this->assertEquals('admindelegation', $this->delegation->getSection()); + } + + public function testGetForm(): void { + // Mock admin sections + $this->settingManager->method('getAdminSections') + ->willReturn([]); + + // Mock group search - should filter out admin group + $adminGroup = $this->createMock(IGroup::class); + $adminGroup->method('getGID')->willReturn('admin'); + + $userGroup = $this->createMock(IGroup::class); + $userGroup->method('getGID')->willReturn('users'); + $userGroup->method('getDisplayName')->willReturn('Users'); + + $this->groupManager->method('search') + ->with('') + ->willReturn([$adminGroup, $userGroup]); + + // Mock authorized group service + $this->authorizedGroupService->method('findAll') + ->willReturn([]); + + // Mock URL generator for docs link + $this->urlGenerator->method('linkToDocs') + ->with('admin-delegation') + ->willReturn('https://docs.example.com/admin-delegation'); + + // Expect 4 calls to provideInitialState: + // 1. available-settings + // 2. available-groups + // 3. authorized-groups + // 4. authorized-settings-doc-link + $this->initialStateService->expects($this->exactly(4)) + ->method('provideInitialState') + ->withConsecutive( + ['available-settings', []], + ['available-groups', [['displayName' => 'Users', 'gid' => 'users']]], + ['authorized-groups', []], + ['authorized-settings-doc-link', 'https://docs.example.com/admin-delegation'] + ); + + $result = $this->delegation->getForm(); + + $this->assertInstanceOf(TemplateResponse::class, $result); + $this->assertEquals('settings/admin/delegation', $result->getTemplateName()); + } + + public function testGetFormFiltersAdminGroup(): void { + // Test that admin group is filtered out from available groups + $this->settingManager->method('getAdminSections')->willReturn([]); + + $adminGroup = $this->createMock(IGroup::class); + $adminGroup->method('getGID')->willReturn('admin'); + + $testGroup = $this->createMock(IGroup::class); + $testGroup->method('getGID')->willReturn('testgroup'); + $testGroup->method('getDisplayName')->willReturn('Test Group'); + + $this->groupManager->method('search') + ->willReturn([$adminGroup, $testGroup]); + + $this->authorizedGroupService->method('findAll')->willReturn([]); + $this->urlGenerator->method('linkToDocs')->willReturn(''); + + // Admin group should be filtered out, only testgroup should remain + $this->initialStateService->expects($this->exactly(4)) + ->method('provideInitialState') + ->withConsecutive( + ['available-settings', []], + ['available-groups', [['displayName' => 'Test Group', 'gid' => 'testgroup']]], + ['authorized-groups', []], + ['authorized-settings-doc-link', ''] + ); + + $this->delegation->getForm(); + } +} diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 9887c59ae724d..f055cda96fb35 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -267,25 +267,38 @@ private function init(bool $resolveClosures = true): void { 'name' => $l->t('Apps'), ]); - // Personal settings - $this->add([ - 'type' => 'settings', - 'id' => 'settings', - 'order' => 3, - 'href' => $this->urlGenerator->linkToRoute('settings.PersonalSettings.index'), - 'name' => $l->t('Personal settings'), - 'icon' => $this->urlGenerator->imagePath('settings', 'personal.svg'), - ]); - - // Admin settings - $this->add([ - 'type' => 'settings', - 'id' => 'admin_settings', - 'order' => 4, - 'href' => $this->urlGenerator->linkToRoute('settings.AdminSettings.index', ['section' => 'overview']), - 'name' => $l->t('Administration settings'), - 'icon' => $this->urlGenerator->imagePath('settings', 'admin.svg'), - ]); + $hasDelegatedSettings = $this->config->getSystemValueBool('settings.only-delegated-settings'); + + if ($hasDelegatedSettings) { + $this->add([ + 'type' => 'settings', + 'id' => 'settings', + 'order' => 3, + 'href' => $this->urlGenerator->linkToRoute('settings.PersonalSettings.index'), + 'name' => $l->t('Settings'), + 'icon' => $this->urlGenerator->imagePath('settings', 'admin.svg'), + ]); + } else { + // Personal settings + $this->add([ + 'type' => 'settings', + 'id' => 'settings', + 'order' => 3, + 'href' => $this->urlGenerator->linkToRoute('settings.PersonalSettings.index'), + 'name' => $l->t('Personal settings'), + 'icon' => $this->urlGenerator->imagePath('settings', 'personal.svg'), + ]); + + // Admin settings + $this->add([ + 'type' => 'settings', + 'id' => 'admin_settings', + 'order' => 4, + 'href' => $this->urlGenerator->linkToRoute('settings.AdminSettings.index', ['section' => 'overview']), + 'name' => $l->t('Administration settings'), + 'icon' => $this->urlGenerator->imagePath('settings', 'admin.svg'), + ]); + } } else { // Personal settings $this->add([ diff --git a/lib/private/Settings/DeclarativeManager.php b/lib/private/Settings/DeclarativeManager.php index dea0c678f2046..389837f50e9d1 100644 --- a/lib/private/Settings/DeclarativeManager.php +++ b/lib/private/Settings/DeclarativeManager.php @@ -132,6 +132,8 @@ public function getFormsWithValues(IUser $user, ?string $type, ?string $section) $isAdmin = $this->groupManager->isAdmin($user->getUID()); $forms = []; + $onlyDelegatedSettings = $this->config->getSystemValueBool('settings.only-delegated-settings'); + foreach ($this->appSchemas as $app => $schemas) { foreach ($schemas as $schema) { if ($type !== null && $schema['section_type'] !== $type) { @@ -145,6 +147,10 @@ public function getFormsWithValues(IUser $user, ?string $type, ?string $section) continue; } + if ($schema['section_type'] === 'admin' && $onlyDelegatedSettings) { + continue; + } + $s = $schema; $s['app'] = $app; diff --git a/lib/private/Settings/Manager.php b/lib/private/Settings/Manager.php index c96c04f34fff4..2504978ed8b01 100644 --- a/lib/private/Settings/Manager.php +++ b/lib/private/Settings/Manager.php @@ -9,6 +9,7 @@ use Closure; use OCP\AppFramework\QueryException; use OCP\Group\ISubAdmin; +use OCP\IConfig; use OCP\IGroupManager; use OCP\IL10N; use OCP\IServerContainer; @@ -54,6 +55,7 @@ public function __construct( AuthorizedGroupMapper $mapper, IGroupManager $groupManager, ISubAdmin $subAdmin, + private IConfig $config, ) { $this->log = $log; $this->l10nFactory = $l10nFactory; @@ -303,6 +305,11 @@ public function getPersonalSettings(string $section): array { */ public function getAllowedAdminSettings(string $section, IUser $user): array { $isAdmin = $this->groupManager->isAdmin($user->getUID()); + + if ($this->config->getSystemValueBool('settings.only-delegated-settings')) { + $isAdmin = false; + } + if ($isAdmin) { $appSettings = $this->getSettings('admin', $section); } else { diff --git a/tests/lib/NavigationManagerTest.php b/tests/lib/NavigationManagerTest.php index bcd63bb3af973..77328b8e43cc8 100644 --- a/tests/lib/NavigationManagerTest.php +++ b/tests/lib/NavigationManagerTest.php @@ -832,4 +832,154 @@ public function testDefaultEntryUpdated(): void { $this->assertEquals(false, $this->navigationManager->get('app3')['default']); $this->assertEquals(true, $this->navigationManager->get('app4')['default']); } + + /** + * Test navigation entries when delegated settings are enabled + */ + public function testNavigationWithDelegatedSettingsEnabled(): void { + $l = $this->createMock(IL10N::class); + $l->expects($this->any())->method('t')->willReturnCallback(function ($text, $parameters = []) { + return vsprintf($text, $parameters); + }); + + $this->config->method('getUserValue')->willReturnArgument(3); + $this->config->method('getSystemValueBool') + ->with('settings.only-delegated-settings') + ->willReturn(true); + + $this->appManager->expects($this->any()) + ->method('isEnabledForUser') + ->with('theming') + ->willReturn(true); + $this->l10nFac->expects($this->any())->method('get')->willReturn($l); + $this->urlGenerator->expects($this->any())->method('linkToRoute')->willReturnCallback(function ($route) { + if ($route === 'core.login.logout') { + return 'https://example.com/logout'; + } + if ($route === 'settings.PersonalSettings.index') { + return '/settings/personal'; + } + return '/apps/test/'; + }); + $this->urlGenerator->expects($this->any())->method('imagePath')->willReturnCallback(function ($appName, $file) { + return "/apps/$appName/img/$file"; + }); + + $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->any()) + ->method('getEnabledAppsForUser') + ->with($user) + ->willReturn([]); + $this->groupManager->expects($this->any())->method('isAdmin')->willReturn(true); + $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(); + $this->dispatcher->expects($this->once()) + ->method('dispatchTyped') + ->willReturnCallback(function ($event) { + $this->assertInstanceOf(LoadAdditionalEntriesEvent::class, $event); + }); + + $entries = $this->navigationManager->getAll('all'); + + // When delegated settings are enabled, there should be a single "Settings" entry + // with admin icon instead of separate personal and admin settings + $settingsEntries = array_filter($entries, function ($entry) { + return $entry['type'] === 'settings' && $entry['id'] === 'settings'; + }); + + $this->assertCount(1, $settingsEntries); + $settingsEntry = array_values($settingsEntries)[0]; + $this->assertEquals('Settings', $settingsEntry['name']); + $this->assertEquals('/apps/settings/img/admin.svg', $settingsEntry['icon']); + $this->assertEquals('/settings/personal', $settingsEntry['href']); + + // There should be no separate admin_settings entry + $adminSettingsEntries = array_filter($entries, function ($entry) { + return $entry['id'] === 'admin_settings'; + }); + $this->assertEmpty($adminSettingsEntries); + } + + /** + * Test navigation entries when delegated settings are disabled (default behavior) + */ + public function testNavigationWithDelegatedSettingsDisabled(): void { + $l = $this->createMock(IL10N::class); + $l->expects($this->any())->method('t')->willReturnCallback(function ($text, $parameters = []) { + return vsprintf($text, $parameters); + }); + + $this->config->method('getUserValue')->willReturnArgument(3); + $this->config->method('getSystemValueBool') + ->with('settings.only-delegated-settings') + ->willReturn(false); + + $this->appManager->expects($this->any()) + ->method('isEnabledForUser') + ->with('theming') + ->willReturn(true); + $this->l10nFac->expects($this->any())->method('get')->willReturn($l); + $this->urlGenerator->expects($this->any())->method('linkToRoute')->willReturnCallback(function ($route) { + if ($route === 'core.login.logout') { + return 'https://example.com/logout'; + } + if ($route === 'settings.PersonalSettings.index') { + return '/settings/personal'; + } + if ($route === 'settings.AdminSettings.index') { + return '/settings/admin'; + } + return '/apps/test/'; + }); + $this->urlGenerator->expects($this->any())->method('imagePath')->willReturnCallback(function ($appName, $file) { + return "/apps/$appName/img/$file"; + }); + + $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->any()) + ->method('getEnabledAppsForUser') + ->with($user) + ->willReturn([]); + $this->groupManager->expects($this->any())->method('isAdmin')->willReturn(true); + $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(); + $this->dispatcher->expects($this->once()) + ->method('dispatchTyped') + ->willReturnCallback(function ($event) { + $this->assertInstanceOf(LoadAdditionalEntriesEvent::class, $event); + }); + + $entries = $this->navigationManager->getAll('all'); + + // When delegated settings are disabled, there should be both personal and admin settings entries + $personalSettingsEntries = array_filter($entries, function ($entry) { + return $entry['type'] === 'settings' && $entry['id'] === 'settings'; + }); + $adminSettingsEntries = array_filter($entries, function ($entry) { + return $entry['type'] === 'settings' && $entry['id'] === 'admin_settings'; + }); + + $this->assertCount(1, $personalSettingsEntries); + $this->assertCount(1, $adminSettingsEntries); + + $personalEntry = array_values($personalSettingsEntries)[0]; + $this->assertEquals('Personal settings', $personalEntry['name']); + $this->assertEquals('/apps/settings/img/personal.svg', $personalEntry['icon']); + + $adminEntry = array_values($adminSettingsEntries)[0]; + $this->assertEquals('Administration settings', $adminEntry['name']); + $this->assertEquals('/apps/settings/img/admin.svg', $adminEntry['icon']); + } } diff --git a/tests/lib/Settings/ManagerTest.php b/tests/lib/Settings/ManagerTest.php index 90c195e6bd05d..e2a1fc78c946a 100644 --- a/tests/lib/Settings/ManagerTest.php +++ b/tests/lib/Settings/ManagerTest.php @@ -9,11 +9,13 @@ use OC\Settings\AuthorizedGroupMapper; use OC\Settings\Manager; use OCP\Group\ISubAdmin; +use OCP\IConfig; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IL10N; use OCP\IServerContainer; use OCP\IURLGenerator; +use OCP\IUser; use OCP\L10N\IFactory; use OCP\Settings\ISettings; use OCP\Settings\ISubAdminSettings; @@ -40,6 +42,8 @@ class ManagerTest extends TestCase { private $groupManager; /** @var ISubAdmin|MockObject */ private $subAdmin; + /** @var IConfig|MockObject */ + private $config; protected function setUp(): void { parent::setUp(); @@ -52,6 +56,7 @@ protected function setUp(): void { $this->mapper = $this->createMock(AuthorizedGroupMapper::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->subAdmin = $this->createMock(ISubAdmin::class); + $this->config = $this->createMock(IConfig::class); $this->manager = new Manager( $this->logger, @@ -61,6 +66,7 @@ protected function setUp(): void { $this->mapper, $this->groupManager, $this->subAdmin, + $this->config, ); } @@ -223,4 +229,44 @@ public function testSameSectionAsPersonalAndAdmin(): void { 55 => [$section], ], $this->manager->getAdminSections()); } + + public function testGetAllowedAdminSettingsWithDelegatedSettings(): void { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('testUser'); + + $this->groupManager->method('isAdmin')->with('testUser')->willReturn(true); + $this->config->method('getSystemValueBool')->with('settings.only-delegated-settings')->willReturn(true); + + $section = $this->createMock(ISettings::class); + $section->method('getPriority')->willReturn(13); + $section->method('getSection')->willReturn('sharing'); + + $this->container->method('get')->with('myAdminClass')->willReturn($section); + $this->manager->registerSetting('admin', 'myAdminClass'); + + // When delegated settings are enabled, even admins should be treated as non-admin + $settings = $this->manager->getAllowedAdminSettings('sharing', $user); + + $this->assertEquals([], $settings); + } + + public function testGetAllowedAdminSettingsWithoutDelegatedSettings(): void { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('testUser'); + + $this->groupManager->method('isAdmin')->with('testUser')->willReturn(true); + $this->config->method('getSystemValueBool')->with('settings.only-delegated-settings')->willReturn(false); + + $section = $this->createMock(ISettings::class); + $section->method('getPriority')->willReturn(13); + $section->method('getSection')->willReturn('sharing'); + + $this->container->method('get')->with('myAdminClass')->willReturn($section); + $this->manager->registerSetting('admin', 'myAdminClass'); + + // When delegated settings are disabled, admins should see admin settings + $settings = $this->manager->getAllowedAdminSettings('sharing', $user); + + $this->assertEquals([13 => [$section]], $settings); + } }