diff options
author | Louis Chemineau <louis@chmn.me> | 2024-11-27 17:01:25 +0100 |
---|---|---|
committer | Louis Chemineau <louis@chmn.me> | 2024-11-28 11:01:54 +0100 |
commit | a2f2f7ce93e66f97827229de1d2b4cb233976096 (patch) | |
tree | 7febee48e19b5997ccb5b29a5a0c58c46ed4cbcf | |
parent | ec26cd7b6c7a031921c3f3ea97ecb2bd1981dc00 (diff) | |
download | nextcloud-server-a2f2f7ce93e66f97827229de1d2b4cb233976096.tar.gz nextcloud-server-a2f2f7ce93e66f97827229de1d2b4cb233976096.zip |
feat: Use inline password confirmation in external storage settings
Signed-off-by: Louis Chemineau <louis@chmn.me>
-rw-r--r-- | apps/files_external/lib/Controller/AjaxController.php | 2 | ||||
-rw-r--r-- | apps/files_external/lib/Controller/GlobalStoragesController.php | 4 | ||||
-rw-r--r-- | apps/files_external/lib/Controller/StoragesController.php | 2 | ||||
-rw-r--r-- | apps/files_external/lib/Controller/UserGlobalStoragesController.php | 2 | ||||
-rw-r--r-- | apps/files_external/lib/Controller/UserStoragesController.php | 6 | ||||
-rw-r--r-- | apps/files_external/src/settings.js (renamed from apps/files_external/js/settings.js) | 104 | ||||
-rw-r--r-- | apps/files_external/templates/settings.php | 6 | ||||
-rw-r--r-- | lib/private/AppFramework/DependencyInjection/DIContainer.php | 3 | ||||
-rw-r--r-- | lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php | 123 | ||||
-rw-r--r-- | lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php | 17 | ||||
-rw-r--r-- | webpack.modules.js | 1 |
11 files changed, 135 insertions, 135 deletions
diff --git a/apps/files_external/lib/Controller/AjaxController.php b/apps/files_external/lib/Controller/AjaxController.php index 3934b12c7fa..3332044c948 100644 --- a/apps/files_external/lib/Controller/AjaxController.php +++ b/apps/files_external/lib/Controller/AjaxController.php @@ -72,7 +72,7 @@ class AjaxController extends Controller { * @return bool */ #[NoAdminRequired] - #[PasswordConfirmationRequired] + #[PasswordConfirmationRequired(strict: true)] public function saveGlobalCredentials($uid, $user, $password) { $currentUser = $this->userSession->getUser(); if ($currentUser === null) { diff --git a/apps/files_external/lib/Controller/GlobalStoragesController.php b/apps/files_external/lib/Controller/GlobalStoragesController.php index 85d765d268b..466c4f6f551 100644 --- a/apps/files_external/lib/Controller/GlobalStoragesController.php +++ b/apps/files_external/lib/Controller/GlobalStoragesController.php @@ -70,7 +70,7 @@ class GlobalStoragesController extends StoragesController { * * @return DataResponse */ - #[PasswordConfirmationRequired] + #[PasswordConfirmationRequired(strict: true)] public function create( $mountPoint, $backend, @@ -136,7 +136,7 @@ class GlobalStoragesController extends StoragesController { * * @return DataResponse */ - #[PasswordConfirmationRequired] + #[PasswordConfirmationRequired(strict: true)] public function update( $id, $mountPoint, diff --git a/apps/files_external/lib/Controller/StoragesController.php b/apps/files_external/lib/Controller/StoragesController.php index 858143e5056..72bcbd48a4c 100644 --- a/apps/files_external/lib/Controller/StoragesController.php +++ b/apps/files_external/lib/Controller/StoragesController.php @@ -302,7 +302,7 @@ abstract class StoragesController extends Controller { * * @return DataResponse */ - #[PasswordConfirmationRequired] + #[PasswordConfirmationRequired(strict: true)] public function destroy(int $id) { try { $this->service->removeStorage($id); diff --git a/apps/files_external/lib/Controller/UserGlobalStoragesController.php b/apps/files_external/lib/Controller/UserGlobalStoragesController.php index 355df7fb726..a7c4fc61997 100644 --- a/apps/files_external/lib/Controller/UserGlobalStoragesController.php +++ b/apps/files_external/lib/Controller/UserGlobalStoragesController.php @@ -137,7 +137,7 @@ class UserGlobalStoragesController extends StoragesController { * @return DataResponse */ #[NoAdminRequired] - #[PasswordConfirmationRequired] + #[PasswordConfirmationRequired(strict: true)] public function update( $id, $backendOptions, diff --git a/apps/files_external/lib/Controller/UserStoragesController.php b/apps/files_external/lib/Controller/UserStoragesController.php index 05984ad1a88..6d797b5818d 100644 --- a/apps/files_external/lib/Controller/UserStoragesController.php +++ b/apps/files_external/lib/Controller/UserStoragesController.php @@ -100,7 +100,7 @@ class UserStoragesController extends StoragesController { * @return DataResponse */ #[NoAdminRequired] - #[PasswordConfirmationRequired] + #[PasswordConfirmationRequired(strict: true)] public function create( $mountPoint, $backend, @@ -156,7 +156,7 @@ class UserStoragesController extends StoragesController { * @return DataResponse */ #[NoAdminRequired] - #[PasswordConfirmationRequired] + #[PasswordConfirmationRequired(strict: true)] public function update( $id, $mountPoint, @@ -208,7 +208,7 @@ class UserStoragesController extends StoragesController { * {@inheritdoc} */ #[NoAdminRequired] - #[PasswordConfirmationRequired] + #[PasswordConfirmationRequired(strict: true)] public function destroy(int $id) { return parent::destroy($id); } diff --git a/apps/files_external/js/settings.js b/apps/files_external/src/settings.js index 61f1696545a..1dac6e80eee 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/src/settings.js @@ -4,7 +4,11 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -(function(){ +import axios from '@nextcloud/axios' +import { t } from '@nextcloud/l10n' +import { addPasswordConfirmationInterceptors, PwdConfirmationMode } from '@nextcloud/password-confirmation' + +addPasswordConfirmationInterceptors(axios) /** * Returns the selection of applicable users in the given configuration row @@ -274,7 +278,7 @@ StorageConfig.prototype = { url = OC.generateUrl(this._url + '/{id}', {id: this.id}); } - window.OC.PasswordConfirmation.requirePasswordConfirmation(() => this._save(method, url, options), options.error); + this._save(method, url, options); }, /** @@ -283,22 +287,20 @@ StorageConfig.prototype = { * @param {string} url * @param {{success: Function, error: Function}} options */ - _save: function(method, url, options) { - self = this; - - $.ajax({ - type: method, - url: url, - contentType: 'application/json', - data: JSON.stringify(this.getData()), - success: function(result) { - self.id = result.id; - if (_.isFunction(options.success)) { - options.success(result); - } - }, - error: options.error - }); + _save: async function(method, url, options) { + try { + const response = await axios.request({ + confirmPassword: PwdConfirmationMode.Strict, + method: method, + url: url, + data: this.getData(), + }) + const result = response.data + this.id = result.id + options.success(result) + } catch (error) { + options.error(error) + } }, /** @@ -351,7 +353,7 @@ StorageConfig.prototype = { * @param {Function} [options.success] success callback * @param {Function} [options.error] error callback */ - destroy: function(options) { + destroy: async function(options) { if (!_.isNumber(this.id)) { // the storage hasn't even been created => success if (_.isFunction(options.success)) { @@ -360,20 +362,16 @@ StorageConfig.prototype = { return; } - window.OC.PasswordConfirmation.requirePasswordConfirmation(() => this._destroy(options), options.error) - }, - - /** - * Private implementation of the DELETE method called after password confirmation - * @param {{ success: Function, error: Function }} options - */ - _destroy: function(options) { - $.ajax({ - type: 'DELETE', - url: OC.generateUrl(this._url + '/{id}', {id: this.id}), - success: options.success, - error: options.error - }); + try { + await axios.request({ + method: 'DELETE', + url: OC.generateUrl(this._url + '/{id}', {id: this.id}), + confirmPassword: PwdConfirmationMode.Strict, + }) + options.success() + } catch (e) { + options.error(e) + } }, /** @@ -1486,38 +1484,32 @@ window.addEventListener('DOMContentLoaded', function() { $allowUserMounting.trigger('change'); } - }); + }) + + $('#global_credentials').on('submit', async function (event) { + event.preventDefault(); + var $form = $(this); + var $submit = $form.find('[type=submit]'); + $submit.val(t('files_external', 'Saving …')); - function _submitCredentials(success) { var uid = $form.find('[name=uid]').val(); var user = $form.find('[name=username]').val(); var password = $form.find('[name=password]').val(); - $.ajax({ - type: 'POST', - contentType: 'application/json', + await axios.request({ + method: 'POST', data: JSON.stringify({ uid, user, password, }), - url: OC.generateUrl('apps/files_external/globalcredentials'), - dataType: 'json', - success, - }); - } - - $('#global_credentials').on('submit', function() { - var $form = $(this); - var $submit = $form.find('[type=submit]'); - $submit.val(t('files_external', 'Saving …')); + url: OC.generateUrl('apps/files_external/globalcredentials'), + confirmPassword: PwdConfirmationMode.Strict, + }) - window.OC.PasswordConfirmation - .requirePasswordConfirmation(() => _submitCredentials(function() { - $submit.val(t('files_external', 'Saved')); - setTimeout(function(){ - $submit.val(t('files_external', 'Save')); - }, 2500); - })); + $submit.val(t('files_external', 'Saved')); + setTimeout(function(){ + $submit.val(t('files_external', 'Save')); + }, 2500); return false; }); @@ -1547,5 +1539,3 @@ OCA.Files_External.Settings = OCA.Files_External.Settings || {}; OCA.Files_External.Settings.GlobalStorageConfig = GlobalStorageConfig; OCA.Files_External.Settings.UserStorageConfig = UserStorageConfig; OCA.Files_External.Settings.MountConfigListView = MountConfigListView; - -})(); diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php index 0ebb5800d29..c5af1bddaa7 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -22,10 +22,8 @@ $l->t('Never'); $l->t('Once every direct access'); $l->t('Read only'); -script('files_external', [ - 'settings', - 'templates' -]); +\OCP\Util::addScript('files_external', 'settings'); +\OCP\Util::addScript('files_external', 'templates'); style('files_external', 'settings'); // load custom JS diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index a6a3aec9489..2083a0e7195 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -23,6 +23,7 @@ use OC\Diagnostics\EventLogger; use OC\Log\PsrLoggerAdapter; use OC\ServerContainer; use OC\Settings\AuthorizedGroupMapper; +use OC\User\Manager as UserManager; use OCA\WorkflowEngine\Manager; use OCP\AppFramework\Http\IOutput; use OCP\AppFramework\IAppContainer; @@ -251,6 +252,8 @@ class DIContainer extends SimpleContainer implements IAppContainer { $c->get(ITimeFactory::class), $c->get(\OC\Authentication\Token\IProvider::class), $c->get(LoggerInterface::class), + $c->get(IRequest::class), + $c->get(UserManager::class), ) ); $dispatcher->registerMiddleware( diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index f5416ff652a..2e3ed02ed34 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -8,6 +8,7 @@ namespace OC\AppFramework\Middleware\Security; use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException; use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Authentication\Token\IProvider; +use OC\User\Manager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired; use OCP\AppFramework\Middleware; @@ -16,6 +17,7 @@ use OCP\Authentication\Exceptions\ExpiredTokenException; use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\Exceptions\WipeTokenException; use OCP\Authentication\Token\IToken; +use OCP\IRequest; use OCP\ISession; use OCP\IUserSession; use OCP\Session\Exceptions\SessionNotAvailableException; @@ -24,76 +26,67 @@ use Psr\Log\LoggerInterface; use ReflectionMethod; class PasswordConfirmationMiddleware extends Middleware { - /** @var ControllerMethodReflector */ - private $reflector; - /** @var ISession */ - private $session; - /** @var IUserSession */ - private $userSession; - /** @var ITimeFactory */ - private $timeFactory; - /** @var array */ - private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true]; - private IProvider $tokenProvider; + private array $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true]; - /** - * PasswordConfirmationMiddleware constructor. - * - * @param ControllerMethodReflector $reflector - * @param ISession $session - * @param IUserSession $userSession - * @param ITimeFactory $timeFactory - */ public function __construct( - ControllerMethodReflector $reflector, - ISession $session, - IUserSession $userSession, - ITimeFactory $timeFactory, - IProvider $tokenProvider, + private ControllerMethodReflector $reflector, + private ISession $session, + private IUserSession $userSession, + private ITimeFactory $timeFactory, + private IProvider $tokenProvider, private readonly LoggerInterface $logger, + private readonly IRequest $request, + private readonly Manager $userManager, ) { - $this->reflector = $reflector; - $this->session = $session; - $this->userSession = $userSession; - $this->timeFactory = $timeFactory; - $this->tokenProvider = $tokenProvider; } /** - * @param Controller $controller - * @param string $methodName * @throws NotConfirmedException */ - public function beforeController($controller, $methodName) { + public function beforeController(Controller $controller, string $methodName) { $reflectionMethod = new ReflectionMethod($controller, $methodName); - if ($this->hasAnnotationOrAttribute($reflectionMethod, 'PasswordConfirmationRequired', PasswordConfirmationRequired::class)) { - $user = $this->userSession->getUser(); - $backendClassName = ''; - if ($user !== null) { - $backend = $user->getBackend(); - if ($backend instanceof IPasswordConfirmationBackend) { - if (!$backend->canConfirmPassword($user->getUID())) { - return; - } - } + if (!$this->needsPasswordConfirmation($reflectionMethod)) { + return; + } - $backendClassName = $user->getBackendClassName(); + $user = $this->userSession->getUser(); + $backendClassName = ''; + if ($user !== null) { + $backend = $user->getBackend(); + if ($backend instanceof IPasswordConfirmationBackend) { + if (!$backend->canConfirmPassword($user->getUID())) { + return; + } } - try { - $sessionId = $this->session->getId(); - $token = $this->tokenProvider->getToken($sessionId); - } catch (SessionNotAvailableException|InvalidTokenException|WipeTokenException|ExpiredTokenException) { - // States we do not deal with here. - return; - } - $scope = $token->getScopeAsArray(); - if (isset($scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION]) && $scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION] === true) { - // Users logging in from SSO backends cannot confirm their password by design - return; + $backendClassName = $user->getBackendClassName(); + } + + try { + $sessionId = $this->session->getId(); + $token = $this->tokenProvider->getToken($sessionId); + } catch (SessionNotAvailableException|InvalidTokenException|WipeTokenException|ExpiredTokenException) { + // States we do not deal with here. + return; + } + + $scope = $token->getScopeAsArray(); + if (isset($scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION]) && $scope[IToken::SCOPE_SKIP_PASSWORD_VALIDATION] === true) { + // Users logging in from SSO backends cannot confirm their password by design + return; + } + + if ($this->isPasswordConfirmationStrict($reflectionMethod)) { + $authHeader = $this->request->getHeader('Authorization'); + [, $password] = explode(':', base64_decode(substr($authHeader, 6)), 2); + $loginResult = $this->userManager->checkPassword($user->getUid(), $password); + if ($loginResult === false) { + throw new NotConfirmedException(); } + $this->session->set('last-password-confirm', $this->timeFactory->getTime()); + } else { $lastConfirm = (int)$this->session->get('last-password-confirm'); // TODO: confirm excludedUserBackEnds can go away and remove it if (!isset($this->excludedUserBackEnds[$backendClassName]) && $lastConfirm < ($this->timeFactory->getTime() - (30 * 60 + 15))) { // allow 15 seconds delay @@ -102,24 +95,22 @@ class PasswordConfirmationMiddleware extends Middleware { } } - /** - * @template T - * - * @param ReflectionMethod $reflectionMethod - * @param string $annotationName - * @param class-string<T> $attributeClass - * @return boolean - */ - protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, string $annotationName, string $attributeClass): bool { - if (!empty($reflectionMethod->getAttributes($attributeClass))) { + private function needsPasswordConfirmation(ReflectionMethod $reflectionMethod): bool { + $attributes = $reflectionMethod->getAttributes(PasswordConfirmationRequired::class); + if (!empty($attributes)) { return true; } - if ($this->reflector->hasAnnotation($annotationName)) { - $this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead'); + if ($this->reflector->hasAnnotation('PasswordConfirmationRequired')) { + $this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . 'PasswordConfirmationRequired' . ' annotation and should use the #[PasswordConfirmationRequired] attribute instead'); return true; } return false; } + + private function isPasswordConfirmationStrict(ReflectionMethod $reflectionMethod): bool { + $attributes = $reflectionMethod->getAttributes(PasswordConfirmationRequired::class); + return !empty($attributes) && ($attributes[0]->newInstance()->getStrict()); + } } diff --git a/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php b/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php index 0f0f4b38040..c41e5aa2445 100644 --- a/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php +++ b/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php @@ -18,4 +18,21 @@ use Attribute; */ #[Attribute] class PasswordConfirmationRequired { + /** + * @param bool $strict - Whether password confirmation needs to happen in the request. + * + * @since 31.0.0 + */ + public function __construct( + protected bool $strict = false, + ) { + } + + /** + * @since 31.0.0 + */ + public function getStrict(): bool { + return $this->strict; + } + } diff --git a/webpack.modules.js b/webpack.modules.js index 77275c45eea..8dbffd169e2 100644 --- a/webpack.modules.js +++ b/webpack.modules.js @@ -44,6 +44,7 @@ module.exports = { }, files_external: { init: path.join(__dirname, 'apps/files_external/src', 'init.ts'), + settings: path.join(__dirname, 'apps/files_external/src', 'settings.js'), }, files_reminders: { init: path.join(__dirname, 'apps/files_reminders/src', 'init.ts'), |