From 505dfd65fd3520aaf95add30ef680723ddcd4dbd Mon Sep 17 00:00:00 2001 From: yemkareems Date: Mon, 28 Oct 2024 11:22:36 +0530 Subject: fix: encrypt and store password, decrypt and retrieve the same Signed-off-by: yemkareems --- lib/private/Authentication/LoginCredentials/Store.php | 10 +++++++++- lib/private/Server.php | 3 ++- 2 files changed, 11 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/private/Authentication/LoginCredentials/Store.php b/lib/private/Authentication/LoginCredentials/Store.php index bd39dd11460..8e31d7e23ca 100644 --- a/lib/private/Authentication/LoginCredentials/Store.php +++ b/lib/private/Authentication/LoginCredentials/Store.php @@ -10,6 +10,7 @@ namespace OC\Authentication\LoginCredentials; use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Token\IProvider; +use OC\Security\Crypto; use OCP\Authentication\Exceptions\CredentialsUnavailableException; use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\LoginCredentials\ICredentials; @@ -29,12 +30,17 @@ class Store implements IStore { /** @var IProvider|null */ private $tokenProvider; + /** @var Crypto|null */ + private $crypto; + public function __construct(ISession $session, LoggerInterface $logger, - ?IProvider $tokenProvider = null) { + ?IProvider $tokenProvider = null, + ?Crypto $crypto = null) { $this->session = $session; $this->logger = $logger; $this->tokenProvider = $tokenProvider; + $this->crypto = $crypto; Util::connectHook('OC_User', 'post_login', $this, 'authenticate'); } @@ -45,6 +51,7 @@ class Store implements IStore { * @param array $params */ public function authenticate(array $params) { + $params['password'] = $this->crypto->encrypt((string)$params['password']); $this->session->set('login_credentials', json_encode($params)); } @@ -91,6 +98,7 @@ class Store implements IStore { if ($trySession && $this->session->exists('login_credentials')) { /** @var array $creds */ $creds = json_decode($this->session->get('login_credentials'), true); + $creds['password'] = $this->crypto->decrypt($creds['password']); return new Credentials( $creds['uid'], $creds['loginName'] ?? $this->session->get('loginname') ?? $creds['uid'], // Pre 20 didn't have a loginName property, hence fall back to the session value and then to the UID diff --git a/lib/private/Server.php b/lib/private/Server.php index 0016e2bbb7a..be3f0bb9823 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -451,7 +451,8 @@ class Server extends ServerContainer implements IServerContainer { $tokenProvider = null; } $logger = $c->get(LoggerInterface::class); - return new Store($session, $logger, $tokenProvider); + $crypto = $c->get(Crypto::class); + return new Store($session, $logger, $tokenProvider, $crypto); }); $this->registerAlias(IStore::class, Store::class); $this->registerAlias(IProvider::class, Authentication\Token\Manager::class); -- cgit v1.2.3 From a74ef8237da49b75a930ca335713eef0347c9d51 Mon Sep 17 00:00:00 2001 From: yemkareems Date: Mon, 28 Oct 2024 15:04:11 +0530 Subject: fix: crypto type made not nullable and tests run using ICrypto Signed-off-by: yemkareems --- lib/private/Authentication/LoginCredentials/Store.php | 6 +++--- lib/private/Server.php | 2 +- tests/lib/Authentication/LoginCredentials/StoreTest.php | 9 ++++----- 3 files changed, 8 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/private/Authentication/LoginCredentials/Store.php b/lib/private/Authentication/LoginCredentials/Store.php index 8e31d7e23ca..210d1cc6463 100644 --- a/lib/private/Authentication/LoginCredentials/Store.php +++ b/lib/private/Authentication/LoginCredentials/Store.php @@ -30,13 +30,13 @@ class Store implements IStore { /** @var IProvider|null */ private $tokenProvider; - /** @var Crypto|null */ + /** @var Crypto */ private $crypto; public function __construct(ISession $session, LoggerInterface $logger, - ?IProvider $tokenProvider = null, - ?Crypto $crypto = null) { + Crypto $crypto, + ?IProvider $tokenProvider = null) { $this->session = $session; $this->logger = $logger; $this->tokenProvider = $tokenProvider; diff --git a/lib/private/Server.php b/lib/private/Server.php index be3f0bb9823..fb8c0aa7eda 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -452,7 +452,7 @@ class Server extends ServerContainer implements IServerContainer { } $logger = $c->get(LoggerInterface::class); $crypto = $c->get(Crypto::class); - return new Store($session, $logger, $tokenProvider, $crypto); + return new Store($session, $logger, $crypto, $tokenProvider); }); $this->registerAlias(IStore::class, Store::class); $this->registerAlias(IProvider::class, Authentication\Token\Manager::class); diff --git a/tests/lib/Authentication/LoginCredentials/StoreTest.php b/tests/lib/Authentication/LoginCredentials/StoreTest.php index 0a9a133746e..c58bb09faaa 100644 --- a/tests/lib/Authentication/LoginCredentials/StoreTest.php +++ b/tests/lib/Authentication/LoginCredentials/StoreTest.php @@ -13,7 +13,6 @@ use OC\Authentication\LoginCredentials\Credentials; use OC\Authentication\LoginCredentials\Store; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; -use OC\Security\Crypto; use OCP\Authentication\Exceptions\CredentialsUnavailableException; use OCP\ISession; use OCP\Security\ICrypto; @@ -43,9 +42,9 @@ class StoreTest extends TestCase { $this->session = $this->createMock(ISession::class); $this->tokenProvider = $this->createMock(IProvider::class); $this->logger = $this->createMock(LoggerInterface::class); - $this->crypto = $this->createMock(Crypto::class); + $this->crypto = $this->createMock(ICrypto::class); - $this->store = new Store($this->session, $this->logger, $this->tokenProvider, $this->crypto); + $this->store = new Store($this->session, $this->logger, $this->crypto, $this->tokenProvider); } public function testAuthenticate(): void { @@ -60,7 +59,7 @@ class StoreTest extends TestCase { ->with($this->equalTo('login_credentials'), $this->equalTo(json_encode($params))); $this->crypto->expects($this->once()) ->method('encrypt') - ->willReturn($params['password']); + ->willReturn('123456'); $this->store->authenticate($params); } @@ -73,7 +72,7 @@ class StoreTest extends TestCase { } public function testGetLoginCredentialsNoTokenProvider(): void { - $this->store = new Store($this->session, $this->logger, null, $this->crypto); + $this->store = new Store($this->session, $this->logger, $this->crypto, null); $this->expectException(CredentialsUnavailableException::class); -- cgit v1.2.3 From 79b11227495dda19e838a418641f95c658c4d241 Mon Sep 17 00:00:00 2001 From: yemkareems Date: Mon, 28 Oct 2024 15:49:05 +0530 Subject: fix: use Icrypto in place of Cypto Signed-off-by: yemkareems --- lib/private/Authentication/LoginCredentials/Store.php | 6 +++--- lib/private/Server.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/private/Authentication/LoginCredentials/Store.php b/lib/private/Authentication/LoginCredentials/Store.php index 210d1cc6463..927c1c29f63 100644 --- a/lib/private/Authentication/LoginCredentials/Store.php +++ b/lib/private/Authentication/LoginCredentials/Store.php @@ -10,12 +10,12 @@ namespace OC\Authentication\LoginCredentials; use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Token\IProvider; -use OC\Security\Crypto; use OCP\Authentication\Exceptions\CredentialsUnavailableException; use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\LoginCredentials\ICredentials; use OCP\Authentication\LoginCredentials\IStore; use OCP\ISession; +use OCP\Security\ICrypto; use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\Util; use Psr\Log\LoggerInterface; @@ -30,12 +30,12 @@ class Store implements IStore { /** @var IProvider|null */ private $tokenProvider; - /** @var Crypto */ + /** @var ICrypto */ private $crypto; public function __construct(ISession $session, LoggerInterface $logger, - Crypto $crypto, + ICrypto $crypto, ?IProvider $tokenProvider = null) { $this->session = $session; $this->logger = $logger; diff --git a/lib/private/Server.php b/lib/private/Server.php index fb8c0aa7eda..27a5f2662f8 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -451,7 +451,7 @@ class Server extends ServerContainer implements IServerContainer { $tokenProvider = null; } $logger = $c->get(LoggerInterface::class); - $crypto = $c->get(Crypto::class); + $crypto = $c->get(ICrypto::class); return new Store($session, $logger, $crypto, $tokenProvider); }); $this->registerAlias(IStore::class, Store::class); -- cgit v1.2.3 From 3fd16de636e6ee32de9bc8c808a9c6eda2523493 Mon Sep 17 00:00:00 2001 From: yemkareems Date: Mon, 28 Oct 2024 16:32:57 +0530 Subject: fix: crypto made inline for constructor and decrypt error handled in exception Signed-off-by: yemkareems --- lib/private/Authentication/LoginCredentials/Store.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/private/Authentication/LoginCredentials/Store.php b/lib/private/Authentication/LoginCredentials/Store.php index 927c1c29f63..3330eb90230 100644 --- a/lib/private/Authentication/LoginCredentials/Store.php +++ b/lib/private/Authentication/LoginCredentials/Store.php @@ -8,6 +8,7 @@ declare(strict_types=1); */ namespace OC\Authentication\LoginCredentials; +use Exception; use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Token\IProvider; use OCP\Authentication\Exceptions\CredentialsUnavailableException; @@ -30,17 +31,13 @@ class Store implements IStore { /** @var IProvider|null */ private $tokenProvider; - /** @var ICrypto */ - private $crypto; - public function __construct(ISession $session, LoggerInterface $logger, - ICrypto $crypto, + private readonly ICrypto $crypto, ?IProvider $tokenProvider = null) { $this->session = $session; $this->logger = $logger; $this->tokenProvider = $tokenProvider; - $this->crypto = $crypto; Util::connectHook('OC_User', 'post_login', $this, 'authenticate'); } @@ -98,7 +95,11 @@ class Store implements IStore { if ($trySession && $this->session->exists('login_credentials')) { /** @var array $creds */ $creds = json_decode($this->session->get('login_credentials'), true); - $creds['password'] = $this->crypto->decrypt($creds['password']); + try { + $creds['password'] = $this->crypto->decrypt($creds['password']); + } catch (Exception $e) { + //decryption failed, continue with old password as it is + } return new Credentials( $creds['uid'], $creds['loginName'] ?? $this->session->get('loginname') ?? $creds['uid'], // Pre 20 didn't have a loginName property, hence fall back to the session value and then to the UID -- cgit v1.2.3 From 34b07ace9579db1ce04feebd1c15f626f5503ee3 Mon Sep 17 00:00:00 2001 From: yemkareems Date: Mon, 28 Oct 2024 16:43:24 +0530 Subject: fix: crypto made inline for constructor and decrypt error handled in exception Signed-off-by: yemkareems --- lib/private/Authentication/LoginCredentials/Store.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/private/Authentication/LoginCredentials/Store.php b/lib/private/Authentication/LoginCredentials/Store.php index 3330eb90230..b6f22ce345f 100644 --- a/lib/private/Authentication/LoginCredentials/Store.php +++ b/lib/private/Authentication/LoginCredentials/Store.php @@ -31,10 +31,12 @@ class Store implements IStore { /** @var IProvider|null */ private $tokenProvider; - public function __construct(ISession $session, + public function __construct( + ISession $session, LoggerInterface $logger, private readonly ICrypto $crypto, - ?IProvider $tokenProvider = null) { + ?IProvider $tokenProvider = null, + ) { $this->session = $session; $this->logger = $logger; $this->tokenProvider = $tokenProvider; -- cgit v1.2.3