Skip to content

Commit d21b74d

Browse files
elzodyelzody
authored
Merge pull request #5422 from nextcloud/backport/5420/stable33
[stable33] fix: non-admins cannot upload/delete system config settings
2 parents 3546b00 + cd28335 commit d21b74d

4 files changed

Lines changed: 141 additions & 3 deletions

File tree

‎lib/Controller/WopiController.php‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -442,13 +442,19 @@ public function getSettings(string $type, string $access_token): JSONResponse {
442442
public function uploadSettingsFile(string $fileId, string $access_token): JSONResponse {
443443
try {
444444
$wopi = $this->wopiMapper->getWopiForToken($access_token);
445-
446445
$userId = $wopi->getEditorUid();
447446

448447
if (empty($userId)) {
449448
throw new \Exception('UserID is empty');
450449
}
451450

451+
$isUserAdmin = $this->groupManager->isAdmin($userId);
452+
// Use the fileId as a file path URL (e.g., "/settings/systemconfig/wordbook/en_US%20(1).dic")
453+
$settingsUrl = new SettingsUrl($fileId);
454+
if ($settingsUrl->isSystemConfig() && !$isUserAdmin) {
455+
throw new NotPermittedException();
456+
}
457+
452458
$content = fopen('php://input', 'rb');
453459
if (!$content) {
454460
throw new \Exception('Failed to read input stream.');
@@ -457,8 +463,7 @@ public function uploadSettingsFile(string $fileId, string $access_token): JSONRe
457463
$fileContent = stream_get_contents($content);
458464
fclose($content);
459465

460-
// Use the fileId as a file path URL (e.g., "/settings/systemconfig/wordbook/en_US%20(1).dic")
461-
$settingsUrl = new SettingsUrl($fileId);
466+
462467
$result = $this->settingsService->uploadFile($settingsUrl, $fileContent, $userId);
463468

464469
return new JSONResponse([
@@ -470,6 +475,8 @@ public function uploadSettingsFile(string $fileId, string $access_token): JSONRe
470475
} catch (UnknownTokenException $e) {
471476
$this->logger->debug($e->getMessage(), ['exception' => $e]);
472477
return new JSONResponse(['error' => 'Invalid token'], Http::STATUS_FORBIDDEN);
478+
} catch (NotPermittedException $e) {
479+
return new JSONResponse(['error' => 'Not permitted'], Http::STATUS_FORBIDDEN);
473480
} catch (\Exception $e) {
474481
$this->logger->error($e->getMessage(), ['exception' => $e]);
475482
return new JSONResponse(['error' => $e->getMessage()], Http::STATUS_INTERNAL_SERVER_ERROR);
@@ -493,6 +500,11 @@ public function deleteSettingsFile(string $fileId, string $access_token): JSONRe
493500
$category = $settingsUrl->getCategory();
494501
$fileName = $settingsUrl->getFileName();
495502
$userId = $wopi->getEditorUid();
503+
$isUserAdmin = $this->groupManager->isAdmin($userId);
504+
505+
if ($settingsUrl->isSystemConfig() && !$isUserAdmin) {
506+
throw new NotPermittedException();
507+
}
496508

497509
$this->settingsService->deleteSettingsFile($type, $category, $fileName, $userId);
498510

‎lib/WOPI/SettingsUrl.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,13 @@ public function getFileName(): string {
100100
public function getRawUrl(): string {
101101
return $this->rawUrl;
102102
}
103+
104+
/**
105+
* Determines if this Settings URL leads to a system config file
106+
*
107+
* @return bool
108+
*/
109+
public function isSystemConfig(): bool {
110+
return $this->getType() === 'systemconfig';
111+
}
103112
}

‎tests/features/admin-settings.feature‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,20 @@ Background:
1010
Scenario: Admin can retrieve admin settings
1111
When the admin settings are requested by an admin
1212
Then the admin settings are returned
13+
14+
Scenario: Normal user cannot upload a system config file
15+
When a user uploads a system configuration file
16+
Then the system configuration upload is forbidden
17+
18+
Scenario: Admin can upload a system config file
19+
When an admin uploads a system configuration file
20+
Then the system configuration upload is allowed
21+
22+
Scenario: Normal user cannot delete a system config file
23+
When a user deletes a system configuration file
24+
Then the system configuration deletion is forbidden
25+
26+
Scenario: Admin can delete a system config file
27+
When an admin deletes a system configuration file
28+
Then the system configuration deletion is allowed
29+

‎tests/features/bootstrap/SettingsContext.php‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,104 @@ public function adminSettingsRequestReturned() {
7878
$this->httpResponse->getBody()->getContents(),
7979
);
8080
}
81+
82+
#[When('a user uploads a system configuration file')]
83+
public function userUploadsSystemConfigFile() {
84+
$this->serverContext->actingAsUser('user1');
85+
86+
$settingsAccessToken = $this->getSettingsAccessToken('user');
87+
$postOptions = [
88+
'query' => [
89+
'access_token' => $settingsAccessToken,
90+
'fileId' => '/settings/systemconfig/wordbook/poc.dic',
91+
],
92+
'body' => 'fake dictionary',
93+
];
94+
95+
$options = array_merge($this->serverContext->getWebOptions(), $postOptions);
96+
97+
$this->httpResponse = $this->http->post('wopi/settings/upload', $options);
98+
}
99+
100+
#[When('an admin uploads a system configuration file')]
101+
public function adminUploadsSystemConfigFile() {
102+
$this->serverContext->actAsAdmin(function () {
103+
$settingsAccessToken = $this->getSettingsAccessToken('admin');
104+
$postOptions = [
105+
'query' => [
106+
'access_token' => $settingsAccessToken,
107+
'fileId' => '/settings/systemconfig/wordbook/poc.dic',
108+
],
109+
'body' => 'fake dictionary',
110+
];
111+
112+
$options = array_merge($this->serverContext->getWebOptions(), $postOptions);
113+
114+
$this->httpResponse = $this->http->post('wopi/settings/upload', $options);
115+
});
116+
}
117+
118+
#[When('a user deletes a system configuration file')]
119+
public function userDeletesSystemConfigFile() {
120+
$this->serverContext->actingAsUser('user1');
121+
122+
$settingsAccessToken = $this->getSettingsAccessToken('user');
123+
$deleteOptions = [
124+
'query' => [
125+
'access_token' => $settingsAccessToken,
126+
'fileId' => '/settings/systemconfig/wordbook/poc.dic',
127+
],
128+
];
129+
130+
$options = array_merge($this->serverContext->getWebOptions(), $deleteOptions);
131+
132+
$this->httpResponse = $this->http->delete('wopi/settings', $options);
133+
}
134+
135+
#[When('an admin deletes a system configuration file')]
136+
public function adminDeletesSystemConfigFile() {
137+
$this->serverContext->actAsAdmin(function () {
138+
$settingsAccessToken = $this->getSettingsAccessToken('admin');
139+
$deleteOptions = [
140+
'query' => [
141+
'access_token' => $settingsAccessToken,
142+
'fileId' => '/settings/systemconfig/wordbook/poc.dic',
143+
],
144+
];
145+
146+
$options = array_merge($this->serverContext->getWebOptions(), $deleteOptions);
147+
148+
$this->httpResponse = $this->http->delete('wopi/settings', $options);
149+
});
150+
}
151+
152+
#[Then('the system configuration upload is forbidden')]
153+
public function systemConfigUploadForbidden() {
154+
Assert::assertEquals(403, $this->httpResponse->getStatusCode());
155+
}
156+
157+
#[Then('the system configuration upload is allowed')]
158+
public function systemConfigUploadAllowed() {
159+
Assert::assertEquals(200, $this->httpResponse->getStatusCode());
160+
}
161+
162+
#[Then('the system configuration deletion is forbidden')]
163+
public function systemConfigDeletionForbidden() {
164+
Assert::assertEquals(403, $this->httpResponse->getStatusCode());
165+
}
166+
167+
#[Then('the system configuration deletion is allowed')]
168+
public function systemConfigDeletionAllowed() {
169+
Assert::assertEquals(200, $this->httpResponse->getStatusCode());
170+
}
171+
172+
private function getSettingsAccessToken(string $type) {
173+
$options = $this->serverContext->getWebOptions();
174+
175+
$response = $this->http->get("settings/generateToken/$type", $options);
176+
$responseBody = $response->getBody()->getContents();
177+
$responseJson = json_decode($responseBody, true);
178+
179+
return $responseJson['token'];
180+
}
81181
}

0 commit comments

Comments
 (0)