Merge pull request #51392 from nextcloud/backport/51320/stable29

[stable29] fix(external_storage): fix settings save

Author: AndyScherzinger

apps/files_external/lib/Controller/AjaxController.php

@@ -31,10 +31,12 @@
 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;
 
@@ -47,6 +49,8 @@ class AjaxController extends Controller {
 	private $userSession;
 	/** @var IGroupManager */
 	private $groupManager;
+	/** @var IL10N */
+	private $l10n;
 
 	/**
 	 * @param string $appName
@@ -61,12 +65,15 @@ public function __construct($appName,
 		RSA $rsaMechanism,
 		GlobalAuth $globalAuth,
 		IUserSession $userSession,
-		IGroupManager $groupManager) {
+		IGroupManager $groupManager,
+		IL10N $l10n,
+	) {
 		parent::__construct($appName, $request);
 		$this->rsaMechanism = $rsaMechanism;
 		$this->globalAuth = $globalAuth;
 		$this->userSession = $userSession;
 		$this->groupManager = $groupManager;
+		$this->l10n = $l10n;
 	}
 
 	/**
@@ -89,27 +96,30 @@ private function generateSshKeys($keyLength) {
 	 */
 	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
@@ -120,9 +130,14 @@ public function saveGlobalCredentials($uid, $user, $password) {
 
 		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

@@ -8,9 +8,11 @@
  *
  */
 
-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'
 
@@ -1526,21 +1528,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

@@ -29,6 +29,7 @@
 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;
@@ -47,6 +48,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);
@@ -58,16 +61,27 @@ protected function setUp(): void {
 			->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',
 			$this->request,
 			$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();
 	}
 
@@ -106,7 +120,9 @@ public function testSaveGlobalCredentialsAsAdminForAnotherUser() {
 			->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() {
@@ -124,7 +140,8 @@ public function testSaveGlobalCredentialsAsAdminForSelf() {
 			->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() {
@@ -139,7 +156,8 @@ public function testSaveGlobalCredentialsAsNormalUserForSelf() {
 			->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() {
@@ -154,6 +172,8 @@ public function testSaveGlobalCredentialsAsNormalUserForAnotherUser() {
 			->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']);
 	}
 }