extend admin-delegation

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',

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',

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 . '.');
 

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));

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);

apps/settings/lib/Service/ConflictException.php

@@ -0,0 +1,10 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OCA\Settings\Service;
+
+class ConflictException extends ServiceException {
+}

apps/settings/lib/Settings/Admin/Delegation.php

@@ -11,19 +11,26 @@
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\IGroupManager;
+use OCP\IL10N;
 use OCP\IURLGenerator;
 use OCP\Settings\IDelegatedSettings;
 use OCP\Settings\IManager;
 use OCP\Settings\ISettings;
 
-class Delegation implements ISettings {
+class Delegation implements IDelegatedSettings {
 	public function __construct(
 		private IManager $settingManager,
 		private IInitialState $initialStateService,
 		private IGroupManager $groupManager,
 		private AuthorizedGroupService $authorizedGroupService,
 		private IURLGenerator $urlGenerator,
+		private IL10N $l10n,
 	) {
+		// Settings manager is cloned in order to preserve the filtered state.
+		// Prevent initSettingState to reload already filtered delegated states of settingManager for current user.
+		// Fixes rendering of delegated sections in apps/settings/templates/settings/frame.php
+		// While browsing to /settings/admin/admindelegation
+		$this->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 [];
+	}
 }

apps/settings/tests/Command/AdminDelegation/AddTest.php

@@ -0,0 +1,184 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Settings\Tests\Command\AdminDelegation;
+
+use OC\Settings\AuthorizedGroup;
+use OCA\Settings\Command\AdminDelegation\Add;
+use OCA\Settings\Service\AuthorizedGroupService;
+use OCA\Settings\Service\ConflictException;
+use OCP\IGroupManager;
+use OCP\Settings\IManager;
+use PHPUnit\Framework\MockObject\MockObject;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Test\TestCase;
+
+class AddTest extends TestCase {
+
+	private IManager&MockObject $settingManager;
+	private AuthorizedGroupService&MockObject $authorizedGroupService;
+	private IGroupManager&MockObject $groupManager;
+	private Add $command;
+	private InputInterface&MockObject $input;
+	private OutputInterface&MockObject $output;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->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');
+	}
+}