From 7ad0f66fc437a63d9b4deac3cd74814882acd6f0 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 8 May 2020 16:38:13 +0200 Subject: [PATCH 1/4] Only save login credentials in database once there is an external storage that needs it Signed-off-by: Robin Appelman --- .../Lib/Auth/Password/LoginCredentials.php | 52 +++++++++++-------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php b/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php index 9f41697452..b8dace8bf5 100644 --- a/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php +++ b/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php @@ -27,6 +27,8 @@ namespace OCA\Files_External\Lib\Auth\Password; use OCA\Files_External\Lib\Auth\AuthMechanism; use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; use OCA\Files_External\Lib\StorageConfig; +use OCP\Authentication\Exceptions\CredentialsUnavailableException; +use OCP\Authentication\LoginCredentials\IStore as CredentialsStore; use OCP\IL10N; use OCP\ISession; use OCP\IUser; @@ -44,45 +46,49 @@ class LoginCredentials extends AuthMechanism { /** @var ICredentialsManager */ protected $credentialsManager; - public function __construct(IL10N $l, ISession $session, ICredentialsManager $credentialsManager) { + /** @var CredentialsStore */ + private $credentialsStore; + + public function __construct(IL10N $l, ISession $session, ICredentialsManager $credentialsManager, CredentialsStore $credentialsStore) { $this->session = $session; $this->credentialsManager = $credentialsManager; + $this->credentialsStore = $credentialsStore; $this ->setIdentifier('password::logincredentials') ->setScheme(self::SCHEME_PASSWORD) ->setText($l->t('Log-in credentials, save in database')) ->addParameters([ - ]) - ; - - \OCP\Util::connectHook('OC_User', 'post_login', $this, 'authenticate'); + ]); } - /** - * Hook listener on post login - * - * @param array $params - */ - public function authenticate(array $params) { - $userId = $params['uid']; - $credentials = [ - 'user' => $this->session->get('loginname'), - 'password' => $params['password'] - ]; - $this->credentialsManager->store($userId, self::CREDENTIALS_IDENTIFIER, $credentials); + private function getCredentials(IUser $user): array { + $credentials = $this->credentialsManager->retrieve($user->getUID(), self::CREDENTIALS_IDENTIFIER); + + if (is_null($credentials)) { + // nothing saved in db, try to get it from the session and save it + try { + $sessionCredentials = $this->credentialsStore->getLoginCredentials(); + + $credentials = [ + 'user' => $sessionCredentials->getLoginName(), + 'password' => $sessionCredentials->getPassword() + ]; + + $this->credentialsManager->store($user->getUID(), self::CREDENTIALS_IDENTIFIER, $credentials); + } catch (CredentialsUnavailableException $e) { + throw new InsufficientDataForMeaningfulAnswerException('No login credentials saved'); + } + } + + return $credentials; } public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) { if (!isset($user)) { throw new InsufficientDataForMeaningfulAnswerException('No login credentials saved'); } - $uid = $user->getUID(); - $credentials = $this->credentialsManager->retrieve($uid, self::CREDENTIALS_IDENTIFIER); - - if (!isset($credentials)) { - throw new InsufficientDataForMeaningfulAnswerException('No login credentials saved'); - } + $credentials = $this->getCredentials($user); $storage->setBackendOption('user', $credentials['user']); $storage->setBackendOption('password', $credentials['password']); From c864e5dfc245e1de1bed6b73e11e5b3a4cdf65b7 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 15 May 2020 17:09:57 +0200 Subject: [PATCH 2/4] remove saved credentails if the user no longer has any storage configured using them Signed-off-by: Robin Appelman --- apps/files_external/appinfo/info.xml | 4 ++ .../lib/BackgroundJob/CredentialsCleanup.php | 69 +++++++++++++++++++ .../lib/Service/UserGlobalStoragesService.php | 13 ++-- 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 apps/files_external/lib/BackgroundJob/CredentialsCleanup.php diff --git a/apps/files_external/appinfo/info.xml b/apps/files_external/appinfo/info.xml index c2ac25bcea..03a8845d3d 100644 --- a/apps/files_external/appinfo/info.xml +++ b/apps/files_external/appinfo/info.xml @@ -31,6 +31,10 @@ External storage can be configured using the GUI or at the command line. This se + + OCA\Files_External\BackgroundJob\CredentialsCleanup + + OCA\Files_External\Command\ListCommand OCA\Files_External\Command\Config diff --git a/apps/files_external/lib/BackgroundJob/CredentialsCleanup.php b/apps/files_external/lib/BackgroundJob/CredentialsCleanup.php new file mode 100644 index 0000000000..80cb51f157 --- /dev/null +++ b/apps/files_external/lib/BackgroundJob/CredentialsCleanup.php @@ -0,0 +1,69 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Files_External\BackgroundJob; + +use OCA\Files_External\Lib\Auth\Password\LoginCredentials; +use OCA\Files_External\Lib\StorageConfig; +use OCA\Files_External\Service\UserGlobalStoragesService; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\TimedJob; +use OCP\Security\ICredentialsManager; +use OCP\IUser; +use OCP\IUserManager; + +class CredentialsCleanup extends TimedJob { + private $credentialsManager; + private $userGlobalStoragesService; + private $userManager; + + public function __construct( + ITimeFactory $time, + ICredentialsManager $credentialsManager, + UserGlobalStoragesService $userGlobalStoragesService, + IUserManager $userManager + ) { + parent::__construct($time); + + $this->credentialsManager = $credentialsManager; + $this->userGlobalStoragesService = $userGlobalStoragesService; + $this->userManager = $userManager; + + // run every day + $this->setInterval(24 * 60 * 60); + } + + protected function run($argument) { + $this->userManager->callForSeenUsers(function (IUser $user) { + $storages = $this->userGlobalStoragesService->getAllStoragesForUser($user); + + $usesLoginCredentials = array_reduce($storages, function (bool $uses, StorageConfig $storage) { + return $uses || $storage->getAuthMechanism() instanceof LoginCredentials; + }, false); + + if (!$usesLoginCredentials) { + $this->credentialsManager->delete($user->getUID(), LoginCredentials::CREDENTIALS_IDENTIFIER); + } + }); + } +} diff --git a/apps/files_external/lib/Service/UserGlobalStoragesService.php b/apps/files_external/lib/Service/UserGlobalStoragesService.php index 7b9af77323..b8ea137428 100644 --- a/apps/files_external/lib/Service/UserGlobalStoragesService.php +++ b/apps/files_external/lib/Service/UserGlobalStoragesService.php @@ -27,6 +27,7 @@ namespace OCA\Files_External\Service; use OCA\Files_External\Lib\StorageConfig; use OCP\Files\Config\IUserMountCache; use OCP\IGroupManager; +use OCP\IUser; use OCP\IUserSession; /** @@ -177,14 +178,18 @@ class UserGlobalStoragesService extends GlobalStoragesService { /** * Gets all storages for the user, admin, personal, global, etc * + * @param IUser|null $user user to get the storages for, if not set the currently logged in user will be used * @return StorageConfig[] array of storage configs */ - public function getAllStoragesForUser() { - if (is_null($this->getUser())) { + public function getAllStoragesForUser(IUser $user = null) { + if (is_null($user)) { + $user = $this->getUser(); + } + if (is_null($user)) { return []; } - $groupIds = $this->groupManager->getUserGroupIds($this->getUser()); - $mounts = $this->dbConfig->getMountsForUser($this->getUser()->getUID(), $groupIds); + $groupIds = $this->groupManager->getUserGroupIds($user); + $mounts = $this->dbConfig->getMountsForUser($user->getUID(), $groupIds); $configs = array_map([$this, 'getStorageConfigFromDBMount'], $mounts); $configs = array_filter($configs, function ($config) { return $config instanceof StorageConfig; From d1bd3ba594e2ee949a29a0ce53aaa6ba78aaf354 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 20 May 2020 15:54:16 +0200 Subject: [PATCH 3/4] update saved credentials on password change Signed-off-by: Robin Appelman --- .../Lib/Auth/Password/LoginCredentials.php | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php b/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php index b8dace8bf5..5ea4a69820 100644 --- a/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php +++ b/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php @@ -29,10 +29,13 @@ use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; use OCA\Files_External\Lib\StorageConfig; use OCP\Authentication\Exceptions\CredentialsUnavailableException; use OCP\Authentication\LoginCredentials\IStore as CredentialsStore; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IL10N; use OCP\ISession; use OCP\IUser; use OCP\Security\ICredentialsManager; +use OCP\User\Events\PasswordUpdatedEvent; +use OCP\User\Events\UserLoggedInEvent; /** * Username and password from login credentials, saved in DB @@ -49,7 +52,7 @@ class LoginCredentials extends AuthMechanism { /** @var CredentialsStore */ private $credentialsStore; - public function __construct(IL10N $l, ISession $session, ICredentialsManager $credentialsManager, CredentialsStore $credentialsStore) { + public function __construct(IL10N $l, ISession $session, ICredentialsManager $credentialsManager, CredentialsStore $credentialsStore, IEventDispatcher $eventDispatcher) { $this->session = $session; $this->credentialsManager = $credentialsManager; $this->credentialsStore = $credentialsStore; @@ -60,6 +63,29 @@ class LoginCredentials extends AuthMechanism { ->setText($l->t('Log-in credentials, save in database')) ->addParameters([ ]); + + $eventDispatcher->addListener(UserLoggedInEvent::class, [$this, 'updateCredentials']); + $eventDispatcher->addListener(PasswordUpdatedEvent::class, [$this, 'updateCredentials']); + } + + /** + * @param UserLoggedInEvent | PasswordUpdatedEvent $event + */ + public function updateCredentials($event) { + if ($event instanceof UserLoggedInEvent && $event->isTokenLogin()) { + return; + } + + $stored = $this->credentialsManager->retrieve($event->getUser()->getUID(), self::CREDENTIALS_IDENTIFIER); + + if ($stored && $stored['password'] != $event->getPassword()) { + $credentials = [ + 'user' => $stored['user'], + 'password' => $event->getPassword() + ]; + + $this->credentialsManager->store($event->getUser()->getUID(), self::CREDENTIALS_IDENTIFIER, $credentials); + } } private function getCredentials(IUser $user): array { From 0b0cc48c8830b71e9ed237c1866163bb85c5d379 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 30 Jul 2020 11:53:09 +0200 Subject: [PATCH 4/4] Move event listener to dedicated class Signed-off-by: Morris Jobke --- .../Lib/Auth/Password/LoginCredentials.php | 25 +------- .../lib/Listener/StorePasswordListener.php | 64 +++++++++++++++++++ 2 files changed, 67 insertions(+), 22 deletions(-) create mode 100644 apps/files_external/lib/Listener/StorePasswordListener.php diff --git a/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php b/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php index 5ea4a69820..982ef57f3e 100644 --- a/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php +++ b/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php @@ -27,6 +27,7 @@ namespace OCA\Files_External\Lib\Auth\Password; use OCA\Files_External\Lib\Auth\AuthMechanism; use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; use OCA\Files_External\Lib\StorageConfig; +use OCA\Files_External\Listener\StorePasswordListener; use OCP\Authentication\Exceptions\CredentialsUnavailableException; use OCP\Authentication\LoginCredentials\IStore as CredentialsStore; use OCP\EventDispatcher\IEventDispatcher; @@ -64,28 +65,8 @@ class LoginCredentials extends AuthMechanism { ->addParameters([ ]); - $eventDispatcher->addListener(UserLoggedInEvent::class, [$this, 'updateCredentials']); - $eventDispatcher->addListener(PasswordUpdatedEvent::class, [$this, 'updateCredentials']); - } - - /** - * @param UserLoggedInEvent | PasswordUpdatedEvent $event - */ - public function updateCredentials($event) { - if ($event instanceof UserLoggedInEvent && $event->isTokenLogin()) { - return; - } - - $stored = $this->credentialsManager->retrieve($event->getUser()->getUID(), self::CREDENTIALS_IDENTIFIER); - - if ($stored && $stored['password'] != $event->getPassword()) { - $credentials = [ - 'user' => $stored['user'], - 'password' => $event->getPassword() - ]; - - $this->credentialsManager->store($event->getUser()->getUID(), self::CREDENTIALS_IDENTIFIER, $credentials); - } + $eventDispatcher->addServiceListener(UserLoggedInEvent::class, StorePasswordListener::class); + $eventDispatcher->addServiceListener(PasswordUpdatedEvent::class, StorePasswordListener::class); } private function getCredentials(IUser $user): array { diff --git a/apps/files_external/lib/Listener/StorePasswordListener.php b/apps/files_external/lib/Listener/StorePasswordListener.php new file mode 100644 index 0000000000..3212f2a48c --- /dev/null +++ b/apps/files_external/lib/Listener/StorePasswordListener.php @@ -0,0 +1,64 @@ + + * + * @author Morris Jobke + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Files_External\Listener; + +use OCA\Files_External\Lib\Auth\Password\LoginCredentials; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\Security\ICredentialsManager; +use OCP\User\Events\PasswordUpdatedEvent; +use OCP\User\Events\UserLoggedInEvent; + +class StorePasswordListener implements IEventListener { + /** @var ICredentialsManager */ + private $credentialsManager; + + public function __construct(ICredentialsManager $credentialsManager) { + $this->credentialsManager = $credentialsManager; + } + + public function handle(Event $event): void { + if (!$event instanceof UserLoggedInEvent && !$event instanceof PasswordUpdatedEvent) { + return; + } + + if ($event instanceof UserLoggedInEvent && $event->isTokenLogin()) { + return; + } + + $stored = $this->credentialsManager->retrieve($event->getUser()->getUID(), LoginCredentials::CREDENTIALS_IDENTIFIER); + + if ($stored && $stored['password'] !== $event->getPassword()) { + $credentials = [ + 'user' => $stored['user'], + 'password' => $event->getPassword() + ]; + + $this->credentialsManager->store($event->getUser()->getUID(), LoginCredentials::CREDENTIALS_IDENTIFIER, $credentials); + } + } +}