Merge pull request #51320 from nextcloud/fix/storage-settings

fix(external_storage): fix settings save

apps/files_external/lib/Controller/AjaxController.php

@@ -9,10 +9,12 @@ namespace OCA\Files_External\Controller;
 use OCA\Files_External\Lib\Auth\Password\GlobalAuth;
 use OCA\Files_External\Lib\Auth\PublicKey\RSA;
 use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IGroupManager;
+use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IUserSession;
 
@@ -32,6 +34,7 @@ class AjaxController extends Controller {
 		private GlobalAuth $globalAuth,
 		private IUserSession $userSession,
 		private IGroupManager $groupManager,
+		private IL10N $l10n,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -56,27 +59,30 @@ class AjaxController extends Controller {
 	#[NoAdminRequired]
 	public function getSshKeys($keyLength = 1024) {
 		$key = $this->generateSshKeys($keyLength);
-		return new JSONResponse(
-			['data' => [
+		return new JSONResponse([
+			'data' => [
 				'private_key' => $key['privatekey'],
 				'public_key' => $key['publickey']
 			],
-				'status' => 'success'
-			]);
+			'status' => 'success',
+		]);
 	}
 
 	/**
 	 * @param string $uid
 	 * @param string $user
 	 * @param string $password
-	 * @return bool
+	 * @return JSONResponse
 	 */
 	#[NoAdminRequired]
 	#[PasswordConfirmationRequired(strict: true)]
-	public function saveGlobalCredentials($uid, $user, $password) {
+	public function saveGlobalCredentials($uid, $user, $password): JSONResponse {
 		$currentUser = $this->userSession->getUser();
 		if ($currentUser === null) {
-			return false;
+			return new JSONResponse([
+				'status' => 'error',
+				'message' => $this->l10n->t('You are not logged in'),
+			], Http::STATUS_UNAUTHORIZED);
 		}
 
 		// Non-admins can only edit their own credentials
@@ -87,9 +93,14 @@ class AjaxController extends Controller {
 
 		if ($allowedToEdit) {
 			$this->globalAuth->saveAuth($uid, $user, $password);
-			return true;
+			return new JSONResponse([
+				'status' => 'success',
+			]);
 		}
 
-		return false;
+		return new JSONResponse([
+			'status' => 'success',
+			'message' => $this->l10n->t('Permission denied'),
+		], Http::STATUS_FORBIDDEN);
 	}
 }

apps/files_external/src/settings.js

@@ -4,9 +4,11 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-import axios from '@nextcloud/axios'
-import { t } from '@nextcloud/l10n'
 import { addPasswordConfirmationInterceptors, PwdConfirmationMode } from '@nextcloud/password-confirmation'
+import { generateUrl } from '@nextcloud/router'
+import { showError } from '@nextcloud/dialogs'
+import { t } from '@nextcloud/l10n'
+import axios, { isAxiosError } from '@nextcloud/axios'
 
 import jQuery from 'jquery'
 
@@ -1522,21 +1524,30 @@ window.addEventListener('DOMContentLoaded', function() {
 		const uid = $form.find('[name=uid]').val()
 		const user = $form.find('[name=username]').val()
 		const password = $form.find('[name=password]').val()
-		await axios.request({
-			method: 'POST',
-			data: JSON.stringify({
-				uid,
-				user,
-				password,
-			}),
-			url: OC.generateUrl('apps/files_external/globalcredentials'),
-			confirmPassword: PwdConfirmationMode.Strict,
-		})
 
-		$submit.val(t('files_external', 'Saved'))
-		setTimeout(function() {
+		try {
+			await axios.request({
+				method: 'POST',
+				data: {
+					uid,
+					user,
+					password,
+				},
+				url: generateUrl('apps/files_external/globalcredentials'),
+				confirmPassword: PwdConfirmationMode.Strict,
+			})
+
+			$submit.val(t('files_external', 'Saved'))
+			setTimeout(function() {
+				$submit.val(t('files_external', 'Save'))
+			}, 2500)
+		} catch (error) {
 			$submit.val(t('files_external', 'Save'))
-		}, 2500)
+			if (isAxiosError(error)) {
+				const message = error.response?.data?.message || t('files_external', 'Failed to save global credentials')
+				showError(t('files_external', 'Failed to save global credentials: {message}', { message }))
+			}
+		}
 
 		return false
 	})

apps/files_external/tests/Controller/AjaxControllerTest.php

@@ -10,6 +10,7 @@ use OCA\Files_External\Lib\Auth\Password\GlobalAuth;
 use OCA\Files_External\Lib\Auth\PublicKey\RSA;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IGroupManager;
+use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IUser;
 use OCP\IUserSession;
@@ -28,6 +29,8 @@ class AjaxControllerTest extends TestCase {
 	private $groupManager;
 	/** @var AjaxController */
 	private $ajaxController;
+	/** @var IL10N */
+	private $l10n;
 
 	protected function setUp(): void {
 		$this->request = $this->createMock(IRequest::class);
@@ -39,6 +42,7 @@ class AjaxControllerTest extends TestCase {
 			->getMock();
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->groupManager = $this->createMock(IGroupManager::class);
+		$this->l10n = $this->createMock(IL10N::class);
 
 		$this->ajaxController = new AjaxController(
 			'files_external',
@@ -46,9 +50,19 @@ class AjaxControllerTest extends TestCase {
 			$this->rsa,
 			$this->globalAuth,
 			$this->userSession,
-			$this->groupManager
+			$this->groupManager,
+			$this->l10n,
 		);
 
+		$this->l10n->expects($this->any())
+			->method('t')
+			->willReturnCallback(function ($string, $args) {
+				if (!is_array($args)) {
+					$args = [$args];
+				}
+				return vsprintf($string, $args);
+			});
+
 		parent::setUp();
 	}
 
@@ -87,7 +101,9 @@ class AjaxControllerTest extends TestCase {
 			->expects($this->never())
 			->method('saveAuth');
 
-		$this->assertSame(false, $this->ajaxController->saveGlobalCredentials('UidOfTestUser', 'test', 'password'));
+		$response = $this->ajaxController->saveGlobalCredentials('UidOfTestUser', 'test', 'password');
+		$this->assertSame($response->getStatus(), 403);
+		$this->assertSame('Permission denied', $response->getData()['message']);
 	}
 
 	public function testSaveGlobalCredentialsAsAdminForSelf(): void {
@@ -105,7 +121,8 @@ class AjaxControllerTest extends TestCase {
 			->method('saveAuth')
 			->with('MyAdminUid', 'test', 'password');
 
-		$this->assertSame(true, $this->ajaxController->saveGlobalCredentials('MyAdminUid', 'test', 'password'));
+		$response = $this->ajaxController->saveGlobalCredentials('MyAdminUid', 'test', 'password');
+		$this->assertSame($response->getStatus(), 200);
 	}
 
 	public function testSaveGlobalCredentialsAsNormalUserForSelf(): void {
@@ -120,7 +137,8 @@ class AjaxControllerTest extends TestCase {
 			->method('saveAuth')
 			->with('MyUserUid', 'test', 'password');
 
-		$this->assertSame(true, $this->ajaxController->saveGlobalCredentials('MyUserUid', 'test', 'password'));
+		$response = $this->ajaxController->saveGlobalCredentials('MyUserUid', 'test', 'password');
+		$this->assertSame($response->getStatus(), 200);
 	}
 
 	public function testSaveGlobalCredentialsAsNormalUserForAnotherUser(): void {
@@ -135,6 +153,8 @@ class AjaxControllerTest extends TestCase {
 			->expects($this->never())
 			->method('saveAuth');
 
-		$this->assertSame(false, $this->ajaxController->saveGlobalCredentials('AnotherUserUid', 'test', 'password'));
+		$response = $this->ajaxController->saveGlobalCredentials('AnotherUserUid', 'test', 'password');
+		$this->assertSame($response->getStatus(), 403);
+		$this->assertSame('Permission denied', $response->getData()['message']);
 	}
 }