[stable32] feat(files_external): convert to delegated settings

apps/files_external/ajax/applicable.php

@@ -12,7 +12,21 @@
 \OC_JSON::checkAppEnabled('files_external');
 \OC_JSON::callCheck();
 
-\OC_JSON::checkAdminUser();
+// Replaces \OC_JSON::checkAdminUser() to also allow delegated admins access.
+$currentUser = \OC::$server->getUserSession()->getUser();
+if ($currentUser === null) {
+	\OC_JSON::error(['message' => 'Not logged in']);
+	exit();
+}
+$groupManager = \OC::$server->getGroupManager();
+$authorizedGroupMapper = \OC::$server->get(\OC\Settings\AuthorizedGroupMapper::class);
+$isAdmin = $groupManager->isAdmin($currentUser->getUID());
+// A delegated admin is granted access when their group is authorized for the files_external Admin settings class.
+$isDelegated = in_array(\OCA\Files_External\Settings\Admin::class, $authorizedGroupMapper->findAllClassesForUser($currentUser), true);
+if (!$isAdmin && !$isDelegated) {
+	\OC_JSON::error(['message' => 'Not authorized']);
+	exit();
+}
 
 $pattern = '';
 $limit = null;

apps/files_external/lib/Controller/AjaxController.php

@@ -7,8 +7,10 @@
  */
 namespace OCA\Files_External\Controller;
 
+use OC\Settings\AuthorizedGroupMapper;
 use OCA\Files_External\Lib\Auth\Password\GlobalAuth;
 use OCA\Files_External\Lib\Auth\PublicKey\RSA;
+use OCA\Files_External\Settings\Admin;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
@@ -36,6 +38,7 @@ public function __construct(
 		private IUserSession $userSession,
 		private IGroupManager $groupManager,
 		private IL10N $l10n,
+		private AuthorizedGroupMapper $authorizedGroupMapper,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -86,10 +89,14 @@ public function saveGlobalCredentials($uid, $user, $password): JSONResponse {
 			], Http::STATUS_UNAUTHORIZED);
 		}
 
-		// Non-admins can only edit their own credentials
-		// Admin can edit global credentials
+		// Non-admins can only edit their own credentials.
+		// Admin or delegated admin can edit global credentials (uid === '').
+		// Cannot use #[AuthorizedAdminSetting] here because this endpoint is
+		// #[NoAdminRequired] and must also allow users to edit their own (uid !== '')
+		// credentials — the two paths share one method.
 		$allowedToEdit = $uid === ''
 			? $this->groupManager->isAdmin($currentUser->getUID())
+				|| in_array(Admin::class, $this->authorizedGroupMapper->findAllClassesForUser($currentUser), true)
 			: $currentUser->getUID() === $uid;
 
 		if ($allowedToEdit) {

apps/files_external/lib/Controller/GlobalStoragesController.php

@@ -10,7 +10,9 @@
 use OCA\Files_External\NotFoundException;
 use OCA\Files_External\Service\BackendService;
 use OCA\Files_External\Service\GlobalStoragesService;
+use OCA\Files_External\Settings\Admin;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
 use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\IConfig;
@@ -74,6 +76,7 @@ public function __construct(
 	 *
 	 * @return DataResponse
 	 */
+	#[AuthorizedAdminSetting(settings: Admin::class)]
 	#[PasswordConfirmationRequired(strict: true)]
 	public function create(
 		$mountPoint,
@@ -129,6 +132,7 @@ public function create(
 	 *
 	 * @return DataResponse
 	 */
+	#[AuthorizedAdminSetting(settings: Admin::class)]
 	#[PasswordConfirmationRequired(strict: true)]
 	public function update(
 		$id,
@@ -179,4 +183,22 @@ public function update(
 			Http::STATUS_OK
 		);
 	}
+
+	// PHP attributes are not inherited, so these methods override the parent
+	// solely to attach #[AuthorizedAdminSetting] and expose them to delegated admins.
+	#[AuthorizedAdminSetting(settings: Admin::class)]
+	public function index() {
+		return parent::index();
+	}
+
+	#[AuthorizedAdminSetting(settings: Admin::class)]
+	public function show(int $id, $testOnly = true) {
+		return parent::show($id, $testOnly);
+	}
+
+	#[AuthorizedAdminSetting(settings: Admin::class)]
+	#[PasswordConfirmationRequired(strict: true)]
+	public function destroy(int $id) {
+		return parent::destroy($id);
+	}
 }

apps/files_external/lib/Settings/Admin.php

@@ -12,15 +12,16 @@
 use OCA\Files_External\Service\GlobalStoragesService;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\Encryption\IManager;
-use OCP\Settings\ISettings;
-
-class Admin implements ISettings {
+use OCP\IL10N;
+use OCP\Settings\IDelegatedSettings;
 
+class Admin implements IDelegatedSettings {
 	public function __construct(
 		private IManager $encryptionManager,
 		private GlobalStoragesService $globalStoragesService,
 		private BackendService $backendService,
 		private GlobalAuth $globalAuth,
+		private IL10N $l10n,
 	) {
 	}
 
@@ -60,4 +61,13 @@ public function getSection() {
 	public function getPriority() {
 		return 40;
 	}
+
+	public function getName(): string {
+		return $this->l10n->t('External storage');
+	}
+
+	public function getAuthorizedAppConfig(): array {
+		// No app config keys require delegation for external storage.
+		return [];
+	}
 }

apps/files_external/tests/Controller/AjaxControllerTest.php

@@ -7,9 +7,11 @@
  */
 namespace OCA\Files_External\Tests\Controller;
 
+use OC\Settings\AuthorizedGroupMapper;
 use OCA\Files_External\Controller\AjaxController;
 use OCA\Files_External\Lib\Auth\Password\GlobalAuth;
 use OCA\Files_External\Lib\Auth\PublicKey\RSA;
+use OCA\Files_External\Settings\Admin;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IGroupManager;
 use OCP\IL10N;
@@ -26,6 +28,7 @@ class AjaxControllerTest extends TestCase {
 	private IUserSession&MockObject $userSession;
 	private IGroupManager&MockObject $groupManager;
 	private IL10N&MockObject $l10n;
+	private AuthorizedGroupMapper&MockObject $authorizedGroupMapper;
 	private AjaxController $ajaxController;
 
 	protected function setUp(): void {
@@ -35,6 +38,7 @@ protected function setUp(): void {
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->l10n = $this->createMock(IL10N::class);
+		$this->authorizedGroupMapper = $this->createMock(AuthorizedGroupMapper::class);
 
 		$this->ajaxController = new AjaxController(
 			'files_external',
@@ -44,6 +48,7 @@ protected function setUp(): void {
 			$this->userSession,
 			$this->groupManager,
 			$this->l10n,
+			$this->authorizedGroupMapper,
 		);
 
 		$this->l10n->expects($this->any())
@@ -149,4 +154,71 @@ public function testSaveGlobalCredentialsAsNormalUserForAnotherUser(): void {
 		$this->assertSame($response->getStatus(), 403);
 		$this->assertSame('Permission denied', $response->getData()['message']);
 	}
+
+	public function testSaveGlobalCredentialsAsAdminForGlobal(): void {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('MyAdminUid');
+		$this->userSession->method('getUser')->willReturn($user);
+		$this->groupManager
+			->expects($this->once())
+			->method('isAdmin')
+			->with('MyAdminUid')
+			->willReturn(true);
+		$this->authorizedGroupMapper
+			->expects($this->never())
+			->method('findAllClassesForUser');
+		$this->globalAuth
+			->expects($this->once())
+			->method('saveAuth')
+			->with('', 'test', 'password');
+
+		$response = $this->ajaxController->saveGlobalCredentials('', 'test', 'password');
+		$this->assertSame(200, $response->getStatus());
+	}
+
+	public function testSaveGlobalCredentialsAsDelegatedAdminForGlobal(): void {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('DelegatedUid');
+		$this->userSession->method('getUser')->willReturn($user);
+		$this->groupManager
+			->expects($this->once())
+			->method('isAdmin')
+			->with('DelegatedUid')
+			->willReturn(false);
+		$this->authorizedGroupMapper
+			->expects($this->once())
+			->method('findAllClassesForUser')
+			->with($user)
+			->willReturn([Admin::class]);
+		$this->globalAuth
+			->expects($this->once())
+			->method('saveAuth')
+			->with('', 'test', 'password');
+
+		$response = $this->ajaxController->saveGlobalCredentials('', 'test', 'password');
+		$this->assertSame(200, $response->getStatus());
+	}
+
+	public function testSaveGlobalCredentialsAsNormalUserForGlobal(): void {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('NormalUid');
+		$this->userSession->method('getUser')->willReturn($user);
+		$this->groupManager
+			->expects($this->once())
+			->method('isAdmin')
+			->with('NormalUid')
+			->willReturn(false);
+		$this->authorizedGroupMapper
+			->expects($this->once())
+			->method('findAllClassesForUser')
+			->with($user)
+			->willReturn([]);
+		$this->globalAuth
+			->expects($this->never())
+			->method('saveAuth');
+
+		$response = $this->ajaxController->saveGlobalCredentials('', 'test', 'password');
+		$this->assertSame(403, $response->getStatus());
+		$this->assertSame('Permission denied', $response->getData()['message']);
+	}
 }

apps/files_external/tests/Settings/AdminTest.php

@@ -14,6 +14,7 @@
 use OCA\Files_External\Settings\Admin;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\Encryption\IManager;
+use OCP\IL10N;
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
@@ -22,6 +23,7 @@ class AdminTest extends TestCase {
 	private GlobalStoragesService&MockObject $globalStoragesService;
 	private BackendService&MockObject $backendService;
 	private GlobalAuth&MockObject $globalAuth;
+	private IL10N&MockObject $l10n;
 	private Admin $admin;
 
 	protected function setUp(): void {
@@ -30,12 +32,17 @@ protected function setUp(): void {
 		$this->globalStoragesService = $this->createMock(GlobalStoragesService::class);
 		$this->backendService = $this->createMock(BackendService::class);
 		$this->globalAuth = $this->createMock(GlobalAuth::class);
+		$this->l10n = $this->createMock(IL10N::class);
+		$this->l10n->method('t')->willReturnCallback(function ($text) {
+			return $text;
+		});
 
 		$this->admin = new Admin(
 			$this->encryptionManager,
 			$this->globalStoragesService,
 			$this->backendService,
-			$this->globalAuth
+			$this->globalAuth,
+			$this->l10n
 		);
 	}
 
@@ -91,4 +98,22 @@ public function testGetSection(): void {
 	public function testGetPriority(): void {
 		$this->assertSame(40, $this->admin->getPriority());
 	}
+
+	public function testGetName(): void {
+		$this->l10n->expects($this->once())
+			->method('t')
+			->with('External storage')
+			->willReturn('External storage');
+
+		$this->assertSame('External storage', $this->admin->getName());
+	}
+
+	public function testGetAuthorizedAppConfig(): void {
+		$this->assertSame([], $this->admin->getAuthorizedAppConfig());
+	}
+
+	public function testImplementsIDelegatedSettings(): void {
+		$this->assertInstanceOf(\OCP\Settings\IDelegatedSettings::class, $this->admin);
+		$this->assertInstanceOf(\OCP\Settings\ISettings::class, $this->admin);
+	}
 }

build/psalm-baseline.xml

@@ -1407,9 +1407,12 @@
   <file src="apps/files_external/ajax/applicable.php">
     <DeprecatedMethod>
       <code><![CDATA[\OC_JSON::callCheck()]]></code>
-      <code><![CDATA[\OC_JSON::checkAdminUser()]]></code>
       <code><![CDATA[\OC_JSON::checkAppEnabled('files_external')]]></code>
+      <code><![CDATA[\OC_JSON::error(['message' => 'Not authorized'])]]></code>
+      <code><![CDATA[\OC_JSON::error(['message' => 'Not logged in'])]]></code>
       <code><![CDATA[\OC_JSON::success($results)]]></code>
+      <code><![CDATA[getGroupManager]]></code>
+      <code><![CDATA[getUserSession]]></code>
     </DeprecatedMethod>
   </file>
   <file src="apps/files_external/ajax/oauth2.php">