From fec8ad8441484bc4da70729ea911d420fc8fb7a6 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Thu, 13 Apr 2023 18:05:37 +0200 Subject: Do not override stored credentials when login in with SAML When login in with SAML, the password from `$event->getPassword()` is `null`. This PR makes sure that this `null` value won't be used to override the stored password even though it is different. This PR also allow for the password and user to be updated even though they were not set before. Signed-off-by: Louis Chemineau --- apps/files_external/lib/Listener/StorePasswordListener.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'apps') diff --git a/apps/files_external/lib/Listener/StorePasswordListener.php b/apps/files_external/lib/Listener/StorePasswordListener.php index 66232a78a93..f5820eff52c 100644 --- a/apps/files_external/lib/Listener/StorePasswordListener.php +++ b/apps/files_external/lib/Listener/StorePasswordListener.php @@ -59,12 +59,12 @@ class StorePasswordListener implements IEventListener { $newCredentials = $storedCredentials; $shouldUpdate = false; - if (isset($storedCredentials['password']) && $storedCredentials['password'] !== $event->getPassword()) { + if (($storedCredentials['password'] ?? null) !== $event->getPassword() && $event->getPassword() !== null) { $shouldUpdate = true; $newCredentials['password'] = $event->getPassword(); } - if (isset($storedCredentials['user']) && $event instanceof UserLoggedInEvent && $storedCredentials['user'] !== $event->getLoginName()) { + if ($event instanceof UserLoggedInEvent && ($storedCredentials['user'] ?? null) !== $event->getLoginName()) { $shouldUpdate = true; $newCredentials['user'] = $event->getLoginName(); } -- cgit v1.2.3 From d5e7682b6b4897412a970f745da4859ead28896e Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Wed, 19 Apr 2023 11:53:34 +0200 Subject: Test StorePasswordListener Signed-off-by: Louis Chemineau --- .../tests/Listener/StorePasswordListenerTest.php | 175 +++++++++++++++++++++ 1 file changed, 175 insertions(+) create mode 100644 apps/files_external/tests/Listener/StorePasswordListenerTest.php (limited to 'apps') diff --git a/apps/files_external/tests/Listener/StorePasswordListenerTest.php b/apps/files_external/tests/Listener/StorePasswordListenerTest.php new file mode 100644 index 00000000000..fd7e147aebf --- /dev/null +++ b/apps/files_external/tests/Listener/StorePasswordListenerTest.php @@ -0,0 +1,175 @@ + + * + * @author Louis Chmn + * + * @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\Tests\Listener; + +use OCA\Files_External\Lib\Auth\Password\LoginCredentials; +use OCA\Files_External\Listener\StorePasswordListener; +use OCP\IUser; +use OCP\Security\ICredentialsManager; +use OCP\User\Events\PasswordUpdatedEvent; +use OCP\User\Events\UserLoggedInEvent; +use Test\TestCase; +use PHPUnit\Framework\MockObject\MockObject; + +/** + * @group DB + */ +class StorePasswordListenerTest extends TestCase { + /** @var MockObject|IUser */ + protected $mockedUser; + + protected function setUp(): void { + parent::setUp(); + $this->mockedUser = $this->createMock(IUser::class); + $this->mockedUser + ->expects($this->any()) + ->method('getUID') + ->willReturn('test'); + } + + /** + * @param array|false|null $initialCredentials + * @param UserLoggedInEvent|PasswordUpdatedEvent $event + * @param array|null $expectedCredentials + */ + public function getMockedCredentialManager($initialCredentials, $event, $expectedCredentials) { + $mockedCredentialsManager = $this->createMock(ICredentialsManager::class); + + if ($initialCredentials !== null) { + $mockedCredentialsManager + ->expects($this->once()) + ->method('retrieve') + ->with( + $this->equalTo('test'), + $this->equalTo(LoginCredentials::CREDENTIALS_IDENTIFIER), + ) + ->willReturn($initialCredentials); + } else { + $mockedCredentialsManager + ->expects($this->never()) + ->method('retrieve'); + } + + if ($expectedCredentials !== null) { + $mockedCredentialsManager + ->expects($this->once()) + ->method('store') + ->with( + $this->equalTo('test'), + $this->equalTo(LoginCredentials::CREDENTIALS_IDENTIFIER), + $this->equalTo($expectedCredentials), + ); + } else { + $mockedCredentialsManager + ->expects($this->never()) + ->method('store'); + } + + $storePasswordListener = new StorePasswordListener($mockedCredentialsManager); + $storePasswordListener->handle($event); + } + + public function testClassicLoginSameCredentials() { + $this->getMockedCredentialManager( + [ + 'user' => 'test', + 'password' => 'password', + ], + new UserLoggedInEvent($this->mockedUser, 'test', 'password', false), + null, + ); + } + + public function testClassicLoginNewPassword() { + $this->getMockedCredentialManager( + [ + 'user' => 'test', + 'password' => 'password', + ], + new UserLoggedInEvent($this->mockedUser, 'test', 'password2', false), + [ + 'user' => 'test', + 'password' => 'password2', + ], + ); + } + + public function testClassicLoginNewUser() { + $this->getMockedCredentialManager( + [ + 'user' => 'test', + 'password' => 'password', + ], + new UserLoggedInEvent($this->mockedUser, 'test2', 'password', false), + [ + 'user' => 'test2', + 'password' => 'password', + ], + ); + } + + public function testSSOLogin() { + $this->getMockedCredentialManager( + [ + 'user' => 'test', + 'password' => 'password', + ], + new UserLoggedInEvent($this->mockedUser, 'test', null, false), + null, + ); + } + + public function testPasswordUpdated() { + $this->getMockedCredentialManager( + [ + 'user' => 'test', + 'password' => 'password', + ], + new PasswordUpdatedEvent($this->mockedUser, 'password2'), + [ + 'user' => 'test', + 'password' => 'password2', + ], + ); + } + + public function testUserLoginWithToken() { + $this->getMockedCredentialManager( + null, + new UserLoggedInEvent($this->mockedUser, 'test', 'password', true), + null, + ); + } + + public function testNoInitialCredentials() { + $this->getMockedCredentialManager( + false, + new PasswordUpdatedEvent($this->mockedUser, 'test', 'password'), + null, + ); + } +} -- cgit v1.2.3