Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Oct 28, 2024
15 changes: 13 additions & 2 deletions lib/private/Authentication/LoginCredentials/Store.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@
*/
namespace OC\Authentication\LoginCredentials;

use Exception;
use OC\Authentication\Exceptions\PasswordlessTokenException;
use OC\Authentication\Token\IProvider;
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;
Expand All @@ -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;
Expand All @@ -63,6 +68,7 @@ public function __construct(ISession $session,
* @param array $params
*/
public function authenticate(array $params) {
$params['password'] = $this->crypto->encrypt((string)$params['password']);
Fixed Show fixed Hide fixed
$this->session->set('login_credentials', json_encode($params));
}

Expand Down Expand Up @@ -109,6 +115,11 @@ public function getLoginCredentials(): ICredentials {
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']);
Fixed Show fixed Hide fixed
} 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
Expand Down
3 changes: 2 additions & 1 deletion lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,8 @@ public function __construct($webRoot, \OC\Config $config) {
$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);
Fixed Show fixed Hide fixed
});
$this->registerAlias(IStore::class, Store::class);
$this->registerAlias(IProvider::class, Authentication\Token\Manager::class);
Expand Down
22 changes: 19 additions & 3 deletions tests/lib/Authentication/LoginCredentials/StoreTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
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;
Expand All @@ -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;
Expand All @@ -56,20 +59,24 @@ protected function setUp(): void {
$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);
}
Expand All @@ -82,7 +89,7 @@ public function testSetSession() {
}

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);

Expand Down Expand Up @@ -156,6 +163,9 @@ public function testGetLoginCredentialsPartialCredentialsAndSessionName() {
->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([
Expand Down Expand Up @@ -193,6 +203,9 @@ public function testGetLoginCredentialsPartialCredentials() {
->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([
Expand Down Expand Up @@ -231,6 +244,9 @@ public function testGetLoginCredentialsInvalidTokenLoginCredentials() {
->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'))
Expand Down
Loading