Merge pull request #48940 from nextcloud/backport/48915/stable29

[stable29] fix: encrypt and store password, decrypt and retrieve the same

3 files changed, 34 insertions, 6 deletions

diff --git a/lib/private/Authentication/LoginCredentials/Store.php b/lib/private/Authentication/LoginCredentials/Store.php
index 1e6f52ae2b9..b354611be26 100644
--- a/lib/private/Authentication/LoginCredentials/Store.php
+++ b/lib/private/Authentication/LoginCredentials/Store.php
@@ -26,6 +26,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;
@@ -33,6 +34,7 @@ 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;
@@ -47,9 +49,12 @@ class Store implements IStore {
 	/** @var IProvider|null */
 	private $tokenProvider;
 
-	public function __construct(ISession $session,
+	public function __construct(
+		ISession $session,
 		LoggerInterface $logger,
-		?IProvider $tokenProvider = null) {
+		private ICrypto $crypto,
+		?IProvider $tokenProvider = null,
+	) {
 		$this->session = $session;
 		$this->logger = $logger;
 		$this->tokenProvider = $tokenProvider;
@@ -63,6 +68,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));
 	}
 
@@ -109,6 +115,11 @@ class Store implements IStore {
 		if ($trySession && $this->session->exists('login_credentials')) {
 			/** @var array $creds */
 			$creds = json_decode($this->session->get('login_credentials'), true);
+			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
diff --git a/lib/private/Server.php b/lib/private/Server.php
index ad1f188881a..38d3f55e82e 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -524,7 +524,8 @@ class Server extends ServerContainer implements IServerContainer {
 				$tokenProvider = null;
 			}
 			$logger = $c->get(LoggerInterface::class);
-			return new Store($session, $logger, $tokenProvider);
+			$crypto = $c->get(ICrypto::class);
+			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 80d64d5466f..8c1dd2272b2 100644
--- a/tests/lib/Authentication/LoginCredentials/StoreTest.php
+++ b/tests/lib/Authentication/LoginCredentials/StoreTest.php
@@ -32,6 +32,7 @@ use OC\Authentication\Token\IProvider;
 use OC\Authentication\Token\IToken;
 use OCP\Authentication\Exceptions\CredentialsUnavailableException;
 use OCP\ISession;
+use OCP\Security\ICrypto;
 use OCP\Session\Exceptions\SessionNotAvailableException;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
@@ -46,6 +47,8 @@ class StoreTest extends TestCase {
 
 	/** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
 	private $logger;
+	/** @var ICrypto|\PHPUnit\Framework\MockObject\MockObject */
+	private $crypto;
 
 	/** @var Store */
 	private $store;
@@ -56,20 +59,24 @@ 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(ICrypto::class);
 
-		$this->store = new Store($this->session, $this->logger, $this->tokenProvider);
+		$this->store = new Store($this->session, $this->logger, $this->crypto, $this->tokenProvider);
 	}
 
 	public function testAuthenticate() {
 		$params = [
 			'run' => true,
 			'uid' => 'user123',
-			'password' => 123456,
+			'password' => '123456',
 		];
 
 		$this->session->expects($this->once())
 			->method('set')
 			->with($this->equalTo('login_credentials'), $this->equalTo(json_encode($params)));
+		$this->crypto->expects($this->once())
+			->method('encrypt')
+			->willReturn('123456');
 
 		$this->store->authenticate($params);
 	}
@@ -82,7 +89,7 @@ class StoreTest extends TestCase {
 	}
 
 	public function testGetLoginCredentialsNoTokenProvider() {
-		$this->store = new Store($this->session, $this->logger, null);
+		$this->store = new Store($this->session, $this->logger, $this->crypto, null);
 
 		$this->expectException(CredentialsUnavailableException::class);
 
@@ -156,6 +163,9 @@ class StoreTest extends TestCase {
 			->method('exists')
 			->with($this->equalTo('login_credentials'))
 			->willReturn(true);
+		$this->crypto->expects($this->once())
+			->method('decrypt')
+			->willReturn($password);
 		$this->session->expects($this->exactly(2))
 			->method('get')
 			->willReturnMap([
@@ -193,6 +203,9 @@ class StoreTest extends TestCase {
 			->method('exists')
 			->with($this->equalTo('login_credentials'))
 			->willReturn(true);
+		$this->crypto->expects($this->once())
+			->method('decrypt')
+			->willReturn($password);
 		$this->session->expects($this->exactly(2))
 			->method('get')
 			->willReturnMap([
@@ -231,6 +244,9 @@ class StoreTest extends TestCase {
 			->method('exists')
 			->with($this->equalTo('login_credentials'))
 			->willReturn(true);
+		$this->crypto->expects($this->once())
+			->method('decrypt')
+			->willReturn($password);
 		$this->session->expects($this->once())
 			->method('get')
 			->with($this->equalTo('login_credentials'))