Skip to content

Commit 3311526

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #51395 from nextcloud/backport/51320/stable31
[stable31] fix(external_storage): fix settings save
2 parents 1d58ce8 + 57313e3 commit 3311526

File tree

7 files changed

+77
-35
lines changed

7 files changed

+77
-35
lines changed

‎apps/files_external/lib/Controller/AjaxController.php

+20-9
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@
99
use OCA\Files_External\Lib\Auth\Password\GlobalAuth;
1010
use OCA\Files_External\Lib\Auth\PublicKey\RSA;
1111
use OCP\AppFramework\Controller;
12+
use OCP\AppFramework\Http;
1213
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
1314
use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
1415
use OCP\AppFramework\Http\JSONResponse;
1516
use OCP\IGroupManager;
17+
use OCP\IL10N;
1618
use OCP\IRequest;
1719
use OCP\IUserSession;
1820

@@ -32,6 +34,7 @@ public function __construct(
3234
private GlobalAuth $globalAuth,
3335
private IUserSession $userSession,
3436
private IGroupManager $groupManager,
37+
private IL10N $l10n,
3538
) {
3639
parent::__construct($appName, $request);
3740
}
@@ -56,27 +59,30 @@ private function generateSshKeys($keyLength) {
5659
#[NoAdminRequired]
5760
public function getSshKeys($keyLength = 1024) {
5861
$key = $this->generateSshKeys($keyLength);
59-
return new JSONResponse(
60-
['data' => [
62+
return new JSONResponse([
63+
'data' => [
6164
'private_key' => $key['privatekey'],
6265
'public_key' => $key['publickey']
6366
],
64-
'status' => 'success'
65-
]);
67+
'status' => 'success',
68+
]);
6669
}
6770

6871
/**
6972
* @param string $uid
7073
* @param string $user
7174
* @param string $password
72-
* @return bool
75+
* @return JSONResponse
7376
*/
7477
#[NoAdminRequired]
7578
#[PasswordConfirmationRequired(strict: true)]
76-
public function saveGlobalCredentials($uid, $user, $password) {
79+
public function saveGlobalCredentials($uid, $user, $password): JSONResponse {
7780
$currentUser = $this->userSession->getUser();
7881
if ($currentUser === null) {
79-
return false;
82+
return new JSONResponse([
83+
'status' => 'error',
84+
'message' => $this->l10n->t('You are not logged in'),
85+
], Http::STATUS_UNAUTHORIZED);
8086
}
8187

8288
// Non-admins can only edit their own credentials
@@ -87,9 +93,14 @@ public function saveGlobalCredentials($uid, $user, $password) {
8793

8894
if ($allowedToEdit) {
8995
$this->globalAuth->saveAuth($uid, $user, $password);
90-
return true;
96+
return new JSONResponse([
97+
'status' => 'success',
98+
]);
9199
}
92100

93-
return false;
101+
return new JSONResponse([
102+
'status' => 'success',
103+
'message' => $this->l10n->t('Permission denied'),
104+
], Http::STATUS_FORBIDDEN);
94105
}
95106
}

‎apps/files_external/src/settings.js

+26-15
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44
* SPDX-License-Identifier: AGPL-3.0-or-later
55
*/
66

7-
import axios from '@nextcloud/axios'
8-
import { t } from '@nextcloud/l10n'
97
import { addPasswordConfirmationInterceptors, PwdConfirmationMode } from '@nextcloud/password-confirmation'
8+
import { generateUrl } from '@nextcloud/router'
9+
import { showError } from '@nextcloud/dialogs'
10+
import { t } from '@nextcloud/l10n'
11+
import axios, { isAxiosError } from '@nextcloud/axios'
1012

1113
import jQuery from 'jquery'
1214

@@ -1522,21 +1524,30 @@ window.addEventListener('DOMContentLoaded', function() {
15221524
const uid = $form.find('[name=uid]').val()
15231525
const user = $form.find('[name=username]').val()
15241526
const password = $form.find('[name=password]').val()
1525-
await axios.request({
1526-
method: 'POST',
1527-
data: JSON.stringify({
1528-
uid,
1529-
user,
1530-
password,
1531-
}),
1532-
url: OC.generateUrl('apps/files_external/globalcredentials'),
1533-
confirmPassword: PwdConfirmationMode.Strict,
1534-
})
15351527

1536-
$submit.val(t('files_external', 'Saved'))
1537-
setTimeout(function() {
1528+
try {
1529+
await axios.request({
1530+
method: 'POST',
1531+
data: {
1532+
uid,
1533+
user,
1534+
password,
1535+
},
1536+
url: generateUrl('apps/files_external/globalcredentials'),
1537+
confirmPassword: PwdConfirmationMode.Strict,
1538+
})
1539+
1540+
$submit.val(t('files_external', 'Saved'))
1541+
setTimeout(function() {
1542+
$submit.val(t('files_external', 'Save'))
1543+
}, 2500)
1544+
} catch (error) {
15381545
$submit.val(t('files_external', 'Save'))
1539-
}, 2500)
1546+
if (isAxiosError(error)) {
1547+
const message = error.response?.data?.message || t('files_external', 'Failed to save global credentials')
1548+
showError(t('files_external', 'Failed to save global credentials: {message}', { message }))
1549+
}
1550+
}
15401551

15411552
return false
15421553
})

‎apps/files_external/tests/Controller/AjaxControllerTest.php

+25-5
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OCA\Files_External\Lib\Auth\PublicKey\RSA;
1111
use OCP\AppFramework\Http\JSONResponse;
1212
use OCP\IGroupManager;
13+
use OCP\IL10N;
1314
use OCP\IRequest;
1415
use OCP\IUser;
1516
use OCP\IUserSession;
@@ -28,6 +29,8 @@ class AjaxControllerTest extends TestCase {
2829
private $groupManager;
2930
/** @var AjaxController */
3031
private $ajaxController;
32+
/** @var IL10N */
33+
private $l10n;
3134

3235
protected function setUp(): void {
3336
$this->request = $this->createMock(IRequest::class);
@@ -39,16 +42,27 @@ protected function setUp(): void {
3942
->getMock();
4043
$this->userSession = $this->createMock(IUserSession::class);
4144
$this->groupManager = $this->createMock(IGroupManager::class);
45+
$this->l10n = $this->createMock(IL10N::class);
4246

4347
$this->ajaxController = new AjaxController(
4448
'files_external',
4549
$this->request,
4650
$this->rsa,
4751
$this->globalAuth,
4852
$this->userSession,
49-
$this->groupManager
53+
$this->groupManager,
54+
$this->l10n,
5055
);
5156

57+
$this->l10n->expects($this->any())
58+
->method('t')
59+
->willReturnCallback(function ($string, $args) {
60+
if (!is_array($args)) {
61+
$args = [$args];
62+
}
63+
return vsprintf($string, $args);
64+
});
65+
5266
parent::setUp();
5367
}
5468

@@ -87,7 +101,9 @@ public function testSaveGlobalCredentialsAsAdminForAnotherUser(): void {
87101
->expects($this->never())
88102
->method('saveAuth');
89103

90-
$this->assertSame(false, $this->ajaxController->saveGlobalCredentials('UidOfTestUser', 'test', 'password'));
104+
$response = $this->ajaxController->saveGlobalCredentials('UidOfTestUser', 'test', 'password');
105+
$this->assertSame($response->getStatus(), 403);
106+
$this->assertSame('Permission denied', $response->getData()['message']);
91107
}
92108

93109
public function testSaveGlobalCredentialsAsAdminForSelf(): void {
@@ -105,7 +121,8 @@ public function testSaveGlobalCredentialsAsAdminForSelf(): void {
105121
->method('saveAuth')
106122
->with('MyAdminUid', 'test', 'password');
107123

108-
$this->assertSame(true, $this->ajaxController->saveGlobalCredentials('MyAdminUid', 'test', 'password'));
124+
$response = $this->ajaxController->saveGlobalCredentials('MyAdminUid', 'test', 'password');
125+
$this->assertSame($response->getStatus(), 200);
109126
}
110127

111128
public function testSaveGlobalCredentialsAsNormalUserForSelf(): void {
@@ -120,7 +137,8 @@ public function testSaveGlobalCredentialsAsNormalUserForSelf(): void {
120137
->method('saveAuth')
121138
->with('MyUserUid', 'test', 'password');
122139

123-
$this->assertSame(true, $this->ajaxController->saveGlobalCredentials('MyUserUid', 'test', 'password'));
140+
$response = $this->ajaxController->saveGlobalCredentials('MyUserUid', 'test', 'password');
141+
$this->assertSame($response->getStatus(), 200);
124142
}
125143

126144
public function testSaveGlobalCredentialsAsNormalUserForAnotherUser(): void {
@@ -135,6 +153,8 @@ public function testSaveGlobalCredentialsAsNormalUserForAnotherUser(): void {
135153
->expects($this->never())
136154
->method('saveAuth');
137155

138-
$this->assertSame(false, $this->ajaxController->saveGlobalCredentials('AnotherUserUid', 'test', 'password'));
156+
$response = $this->ajaxController->saveGlobalCredentials('AnotherUserUid', 'test', 'password');
157+
$this->assertSame($response->getStatus(), 403);
158+
$this->assertSame('Permission denied', $response->getData()['message']);
139159
}
140160
}

‎dist/core-common.js

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/core-common.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/files_external-settings.js

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/files_external-settings.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)