Skip to content

Commit 5852eaa

Browse files
tanyakaprintminion-co
authored andcommitted
feat(files_external): allow delegated admins to save global credentials
Signed-off-by: Tatjana Kaschperko Lindt <kaschperko-lindt@strato.de>
1 parent 67deefe commit 5852eaa

2 files changed

Lines changed: 101 additions & 2 deletions

File tree

apps/files_external/lib/Controller/AjaxController.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@
77
*/
88
namespace OCA\Files_External\Controller;
99

10+
use OC\Settings\AuthorizedGroupMapper;
1011
use OCA\Files_External\Lib\Auth\Password\GlobalAuth;
1112
use OCA\Files_External\Lib\Auth\PublicKey\RSA;
13+
use OCA\Files_External\Settings\Admin;
1214
use OCP\AppFramework\Controller;
1315
use OCP\AppFramework\Http;
1416
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
@@ -38,6 +40,7 @@ public function __construct(
3840
private IGroupManager $groupManager,
3941
private IUserManager $userManager,
4042
private IL10N $l10n,
43+
private AuthorizedGroupMapper $authorizedGroupMapper,
4144
) {
4245
parent::__construct($appName, $request);
4346
}
@@ -112,10 +115,14 @@ public function saveGlobalCredentials($uid, $user, $password): JSONResponse {
112115
], Http::STATUS_UNAUTHORIZED);
113116
}
114117

115-
// Non-admins can only edit their own credentials
116-
// Admin can edit global credentials
118+
// Non-admins can only edit their own credentials.
119+
// Admin or delegated admin can edit global credentials (uid === '').
120+
// Cannot use #[AuthorizedAdminSetting] here because this endpoint is
121+
// #[NoAdminRequired] and must also allow users to edit their own (uid !== '')
122+
// credentials — the two paths share one method.
117123
$allowedToEdit = $uid === ''
118124
? $this->groupManager->isAdmin($currentUser->getUID())
125+
|| in_array(Admin::class, $this->authorizedGroupMapper->findAllClassesForUser($currentUser), true)
119126
: $currentUser->getUID() === $uid;
120127

121128
if ($allowedToEdit) {

apps/files_external/tests/Controller/AjaxControllerTest.php

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77
*/
88
namespace OCA\Files_External\Tests\Controller;
99

10+
use OC\Settings\AuthorizedGroupMapper;
1011
use OCA\Files_External\Controller\AjaxController;
1112
use OCA\Files_External\Lib\Auth\Password\GlobalAuth;
1213
use OCA\Files_External\Lib\Auth\PublicKey\RSA;
14+
use OCA\Files_External\Settings\Admin;
1315
use OCP\AppFramework\Http\JSONResponse;
1416
use OCP\IGroupManager;
1517
use OCP\IL10N;
@@ -28,6 +30,7 @@ class AjaxControllerTest extends TestCase {
2830
private IGroupManager&MockObject $groupManager;
2931
private IUserManager&MockObject $userManager;
3032
private IL10N&MockObject $l10n;
33+
private AuthorizedGroupMapper&MockObject $authorizedGroupMapper;
3134
private AjaxController $ajaxController;
3235

3336
protected function setUp(): void {
@@ -38,6 +41,7 @@ protected function setUp(): void {
3841
$this->groupManager = $this->createMock(IGroupManager::class);
3942
$this->userManager = $this->createMock(IUserManager::class);
4043
$this->l10n = $this->createMock(IL10N::class);
44+
$this->authorizedGroupMapper = $this->createMock(AuthorizedGroupMapper::class);
4145

4246
$this->ajaxController = new AjaxController(
4347
'files_external',
@@ -48,6 +52,7 @@ protected function setUp(): void {
4852
$this->groupManager,
4953
$this->userManager,
5054
$this->l10n,
55+
$this->authorizedGroupMapper,
5156
);
5257

5358
$this->l10n->expects($this->any())
@@ -153,4 +158,91 @@ public function testSaveGlobalCredentialsAsNormalUserForAnotherUser(): void {
153158
$this->assertSame($response->getStatus(), 403);
154159
$this->assertSame('Permission denied', $response->getData()['message']);
155160
}
161+
162+
public function testSaveGlobalCredentialsAsAdminForGlobal(): void {
163+
$user = $this->createMock(IUser::class);
164+
$user->method('getUID')->willReturn('MyAdminUid');
165+
$this->userSession->method('getUser')->willReturn($user);
166+
$this->groupManager
167+
->expects($this->once())
168+
->method('isAdmin')
169+
->with('MyAdminUid')
170+
->willReturn(true);
171+
$this->authorizedGroupMapper
172+
->expects($this->never())
173+
->method('findAllClassesForUser');
174+
$this->globalAuth
175+
->expects($this->once())
176+
->method('saveAuth')
177+
->with('', 'test', 'password');
178+
179+
$response = $this->ajaxController->saveGlobalCredentials('', 'test', 'password');
180+
$this->assertSame(200, $response->getStatus());
181+
}
182+
183+
public function testSaveGlobalCredentialsAsDelegatedAdminForGlobal(): void {
184+
$user = $this->createMock(IUser::class);
185+
$user->method('getUID')->willReturn('DelegatedUid');
186+
$this->userSession->method('getUser')->willReturn($user);
187+
$this->groupManager
188+
->expects($this->once())
189+
->method('isAdmin')
190+
->with('DelegatedUid')
191+
->willReturn(false);
192+
$this->authorizedGroupMapper
193+
->expects($this->once())
194+
->method('findAllClassesForUser')
195+
->with($user)
196+
->willReturn([Admin::class]);
197+
$this->globalAuth
198+
->expects($this->once())
199+
->method('saveAuth')
200+
->with('', 'test', 'password');
201+
202+
$response = $this->ajaxController->saveGlobalCredentials('', 'test', 'password');
203+
$this->assertSame(200, $response->getStatus());
204+
}
205+
206+
public function testSaveGlobalCredentialsAsDelegatedAdminForAnotherUser(): void {
207+
// Delegated admins may only set global (uid='') credentials, not impersonate other users.
208+
$user = $this->createMock(IUser::class);
209+
$user->method('getUID')->willReturn('DelegatedUid');
210+
$this->userSession->method('getUser')->willReturn($user);
211+
$this->groupManager
212+
->expects($this->never())
213+
->method('isAdmin');
214+
$this->authorizedGroupMapper
215+
->expects($this->never())
216+
->method('findAllClassesForUser');
217+
$this->globalAuth
218+
->expects($this->never())
219+
->method('saveAuth');
220+
221+
$response = $this->ajaxController->saveGlobalCredentials('OtherUserUid', 'test', 'password');
222+
$this->assertSame(403, $response->getStatus());
223+
$this->assertSame('Permission denied', $response->getData()['message']);
224+
}
225+
226+
public function testSaveGlobalCredentialsAsNormalUserForGlobal(): void {
227+
$user = $this->createMock(IUser::class);
228+
$user->method('getUID')->willReturn('NormalUid');
229+
$this->userSession->method('getUser')->willReturn($user);
230+
$this->groupManager
231+
->expects($this->once())
232+
->method('isAdmin')
233+
->with('NormalUid')
234+
->willReturn(false);
235+
$this->authorizedGroupMapper
236+
->expects($this->once())
237+
->method('findAllClassesForUser')
238+
->with($user)
239+
->willReturn([]);
240+
$this->globalAuth
241+
->expects($this->never())
242+
->method('saveAuth');
243+
244+
$response = $this->ajaxController->saveGlobalCredentials('', 'test', 'password');
245+
$this->assertSame(403, $response->getStatus());
246+
$this->assertSame('Permission denied', $response->getData()['message']);
247+
}
156248
}

0 commit comments

Comments
 (0)