From bd5350c7a1ddb403823e18ce71c7b7ab088742ca Mon Sep 17 00:00:00 2001 From: Tatjana Kaschperko Lindt Date: Mon, 16 Mar 2026 12:21:32 +0100 Subject: [PATCH 1/4] feat(files_external): convert to delegated settings Signed-off-by: Tatjana Kaschperko Lindt --- apps/files_external/lib/Settings/Admin.php | 15 +++++++++++++-- apps/files_external/tests/Settings/AdminTest.php | 11 +++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/apps/files_external/lib/Settings/Admin.php b/apps/files_external/lib/Settings/Admin.php index c3f8e175c72f5..4a44a03707f68 100644 --- a/apps/files_external/lib/Settings/Admin.php +++ b/apps/files_external/lib/Settings/Admin.php @@ -13,10 +13,11 @@ use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; use OCP\Encryption\IManager; +use OCP\IL10N; use OCP\IURLGenerator; -use OCP\Settings\ISettings; +use OCP\Settings\IDelegatedSettings; -class Admin implements ISettings { +class Admin implements IDelegatedSettings { use CommonSettingsTrait; public function __construct( @@ -26,6 +27,7 @@ public function __construct( private GlobalAuth $globalAuth, private IInitialState $initialState, private IURLGenerator $urlGenerator, + private IL10N $l10n, ) { $this->visibility = BackendService::VISIBILITY_ADMIN; } @@ -65,4 +67,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 []; + } } diff --git a/apps/files_external/tests/Settings/AdminTest.php b/apps/files_external/tests/Settings/AdminTest.php index b8524b5fc2a68..7ff3dcf2321e9 100644 --- a/apps/files_external/tests/Settings/AdminTest.php +++ b/apps/files_external/tests/Settings/AdminTest.php @@ -12,10 +12,10 @@ use OCA\Files_External\Service\BackendService; use OCA\Files_External\Service\GlobalStoragesService; use OCA\Files_External\Settings\Admin; -use OCP\App\IAppManager; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; use OCP\Encryption\IManager; +use OCP\IL10N; use OCP\IURLGenerator; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -27,7 +27,7 @@ class AdminTest extends TestCase { private GlobalAuth&MockObject $globalAuth; private IInitialState&MockObject $initialState; private IURLGenerator&MockObject $urlGenerator; - private IAppManager&MockObject $appManager; + private IL10N&MockObject $l10n; private Admin $admin; protected function setUp(): void { @@ -38,7 +38,10 @@ protected function setUp(): void { $this->globalAuth = $this->createMock(GlobalAuth::class); $this->initialState = $this->createMock(IInitialState::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); - $this->appManager = $this->createMock(IAppManager::class); + $this->l10n = $this->createMock(IL10N::class); + $this->l10n->method('t')->willReturnCallback(function ($text) { + return $text; + }); $this->admin = new Admin( $this->encryptionManager, @@ -47,7 +50,7 @@ protected function setUp(): void { $this->globalAuth, $this->initialState, $this->urlGenerator, - $this->appManager, + $this->l10n, ); } From f6f4bce01e84040b3b47e609e03ea34f3f8297d8 Mon Sep 17 00:00:00 2001 From: Tatjana Kaschperko Lindt Date: Mon, 30 Mar 2026 12:25:54 +0200 Subject: [PATCH 2/4] feat(files_external): add #[AuthorizedAdminSetting] to GlobalStoragesController Signed-off-by: Tatjana Kaschperko Lindt --- .../Controller/GlobalStoragesController.php | 22 +++++++++++++++++++ .../tests/Settings/AdminTest.php | 18 +++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/apps/files_external/lib/Controller/GlobalStoragesController.php b/apps/files_external/lib/Controller/GlobalStoragesController.php index 3d78fe9a49a31..287c00e179b4f 100644 --- a/apps/files_external/lib/Controller/GlobalStoragesController.php +++ b/apps/files_external/lib/Controller/GlobalStoragesController.php @@ -9,7 +9,9 @@ use OCA\Files_External\NotFoundException; 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; @@ -60,6 +62,7 @@ public function __construct( * @param ?array $applicableGroups groups for which to mount the storage * @param ?int $priority priority */ + #[AuthorizedAdminSetting(settings: Admin::class)] #[PasswordConfirmationRequired(strict: true)] public function create( string $mountPoint, @@ -123,6 +126,7 @@ public function create( * @param ?array $applicableGroups groups for which to mount the storage * @param ?int $priority priority */ + #[AuthorizedAdminSetting(settings: Admin::class)] #[PasswordConfirmationRequired(strict: true)] public function update( int $id, @@ -173,4 +177,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); + } } diff --git a/apps/files_external/tests/Settings/AdminTest.php b/apps/files_external/tests/Settings/AdminTest.php index 7ff3dcf2321e9..d4ac8419024ce 100644 --- a/apps/files_external/tests/Settings/AdminTest.php +++ b/apps/files_external/tests/Settings/AdminTest.php @@ -126,4 +126,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); + } } From 281f0e7367f66a33a0f80cd4a210f9aa79310442 Mon Sep 17 00:00:00 2001 From: Tatjana Kaschperko Lindt Date: Mon, 30 Mar 2026 12:54:06 +0200 Subject: [PATCH 3/4] feat(files_external): allow delegated admins to save global credentials Signed-off-by: Tatjana Kaschperko Lindt --- .../lib/Controller/AjaxController.php | 11 ++- .../tests/Controller/AjaxControllerTest.php | 92 +++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/apps/files_external/lib/Controller/AjaxController.php b/apps/files_external/lib/Controller/AjaxController.php index 3d1d61254dd86..72518106530bc 100644 --- a/apps/files_external/lib/Controller/AjaxController.php +++ b/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; @@ -38,6 +40,7 @@ public function __construct( private IGroupManager $groupManager, private IUserManager $userManager, private IL10N $l10n, + private AuthorizedGroupMapper $authorizedGroupMapper, ) { parent::__construct($appName, $request); } @@ -112,10 +115,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) { diff --git a/apps/files_external/tests/Controller/AjaxControllerTest.php b/apps/files_external/tests/Controller/AjaxControllerTest.php index 144564df61dc3..7fd5255e93a9d 100644 --- a/apps/files_external/tests/Controller/AjaxControllerTest.php +++ b/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; @@ -28,6 +30,7 @@ class AjaxControllerTest extends TestCase { private IGroupManager&MockObject $groupManager; private IUserManager&MockObject $userManager; private IL10N&MockObject $l10n; + private AuthorizedGroupMapper&MockObject $authorizedGroupMapper; private AjaxController $ajaxController; protected function setUp(): void { @@ -38,6 +41,7 @@ protected function setUp(): void { $this->groupManager = $this->createMock(IGroupManager::class); $this->userManager = $this->createMock(IUserManager::class); $this->l10n = $this->createMock(IL10N::class); + $this->authorizedGroupMapper = $this->createMock(AuthorizedGroupMapper::class); $this->ajaxController = new AjaxController( 'files_external', @@ -48,6 +52,7 @@ protected function setUp(): void { $this->groupManager, $this->userManager, $this->l10n, + $this->authorizedGroupMapper, ); $this->l10n->expects($this->any()) @@ -153,4 +158,91 @@ 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 testSaveGlobalCredentialsAsDelegatedAdminForAnotherUser(): void { + // Delegated admins may only set global (uid='') credentials, not impersonate other users. + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('DelegatedUid'); + $this->userSession->method('getUser')->willReturn($user); + $this->groupManager + ->expects($this->never()) + ->method('isAdmin'); + $this->authorizedGroupMapper + ->expects($this->never()) + ->method('findAllClassesForUser'); + $this->globalAuth + ->expects($this->never()) + ->method('saveAuth'); + + $response = $this->ajaxController->saveGlobalCredentials('OtherUserUid', 'test', 'password'); + $this->assertSame(403, $response->getStatus()); + $this->assertSame('Permission denied', $response->getData()['message']); + } + + 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']); + } } From f2fdcae49458ea11dcf68d05b43f753bfb753859 Mon Sep 17 00:00:00 2001 From: Tatjana Kaschperko Lindt Date: Wed, 1 Apr 2026 12:08:30 +0200 Subject: [PATCH 4/4] feat(files_external): allow delegated admins to search applicable users/groups Signed-off-by: Tatjana Kaschperko Lindt --- .../lib/Controller/AjaxController.php | 2 + .../tests/Controller/AjaxControllerTest.php | 45 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/apps/files_external/lib/Controller/AjaxController.php b/apps/files_external/lib/Controller/AjaxController.php index 72518106530bc..6ec60406d0c0c 100644 --- a/apps/files_external/lib/Controller/AjaxController.php +++ b/apps/files_external/lib/Controller/AjaxController.php @@ -13,6 +13,7 @@ use OCA\Files_External\Settings\Admin; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired; use OCP\AppFramework\Http\JSONResponse; @@ -54,6 +55,7 @@ public function __construct( * @param int|null $offset The offset from which to start returning results * @return JSONResponse */ + #[AuthorizedAdminSetting(settings: Admin::class)] public function getApplicableEntities(string $pattern = '', ?int $limit = null, ?int $offset = null): JSONResponse { $groups = []; foreach ($this->groupManager->search($pattern, $limit, $offset) as $group) { diff --git a/apps/files_external/tests/Controller/AjaxControllerTest.php b/apps/files_external/tests/Controller/AjaxControllerTest.php index 7fd5255e93a9d..5a2e0981ebc72 100644 --- a/apps/files_external/tests/Controller/AjaxControllerTest.php +++ b/apps/files_external/tests/Controller/AjaxControllerTest.php @@ -13,6 +13,7 @@ use OCA\Files_External\Lib\Auth\PublicKey\RSA; use OCA\Files_External\Settings\Admin; use OCP\AppFramework\Http\JSONResponse; +use OCP\IGroup; use OCP\IGroupManager; use OCP\IL10N; use OCP\IRequest; @@ -67,6 +68,50 @@ protected function setUp(): void { parent::setUp(); } + public function testGetApplicableEntitiesReturnsGroupsAndUsers(): void { + $group = $this->createMock(IGroup::class); + $group->method('getGID')->willReturn('group1'); + $group->method('getDisplayName')->willReturn('Group One'); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('user1'); + $user->method('getDisplayName')->willReturn('User One'); + + $this->groupManager + ->expects($this->once()) + ->method('search') + ->with('test', 10, 0) + ->willReturn([$group]); + $this->userManager + ->expects($this->once()) + ->method('searchDisplayName') + ->with('test', 10, 0) + ->willReturn([$user]); + + $response = $this->ajaxController->getApplicableEntities('test', 10, 0); + $this->assertSame(200, $response->getStatus()); + $this->assertSame(['group1' => 'Group One'], $response->getData()['groups']); + $this->assertSame(['user1' => 'User One'], $response->getData()['users']); + } + + public function testGetApplicableEntitiesWithNoResults(): void { + $this->groupManager + ->expects($this->once()) + ->method('search') + ->with('', null, null) + ->willReturn([]); + $this->userManager + ->expects($this->once()) + ->method('searchDisplayName') + ->with('', null, null) + ->willReturn([]); + + $response = $this->ajaxController->getApplicableEntities(); + $this->assertSame(200, $response->getStatus()); + $this->assertSame([], $response->getData()['groups']); + $this->assertSame([], $response->getData()['users']); + } + public function testGetSshKeys(): void { $this->rsa ->expects($this->once())