From dbf9c06b8961ea608cd8e42436b0c0151eac04fc Mon Sep 17 00:00:00 2001 From: Piotr Mrowczynski Date: Sat, 20 Jun 2020 23:19:30 +0200 Subject: [PATCH] Add new system config to enforce strict login check with user backend --- changelog/unreleased/37569 | 9 +++++++ config/config.sample.php | 8 ++++++ core/Controller/LoginController.php | 2 +- lib/private/User/Session.php | 7 +++++ tests/Core/Controller/LoginControllerTest.php | 22 ++++++++++++++-- tests/lib/User/SessionTest.php | 26 ++++++++++++++++--- 6 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 changelog/unreleased/37569 diff --git a/changelog/unreleased/37569 b/changelog/unreleased/37569 new file mode 100644 index 000000000000..ff7dbe5f56e5 --- /dev/null +++ b/changelog/unreleased/37569 @@ -0,0 +1,9 @@ +Security: Add new system config to enforce strict login check with user backend + +Adds new system config to enforce strict login check for password +in user backend, meaning only login name typed by user would be validated. +With this configuration enabled, e.g. additional check for email will +not be performed. + +https://github.com/owncloud/core/pull/37569 +https://github.com/owncloud/user_ldap/pull/581 diff --git a/config/config.sample.php b/config/config.sample.php index 94d0db236411..bebe01372e8e 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -236,6 +236,14 @@ */ 'token_auth_enforced' => false, +/** + * Enforce strict login check with user backend + * If enabled, strict login check for password in user backend will be enforced, + * meaning only the login name typed by the user would be validated. With this + * configuration enabled, e.g. an additional check for email will not be performed. + */ +'strict_login_enforced' => false, + /** * Define additional login buttons on the logon screen * Provides the ability to create additional login buttons on the logon screen, for e.g., SSO integration diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index e25f09fffb5e..ae1af96c782b 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -219,7 +219,7 @@ public function tryLogin($user, $password, $redirect_url, $timezone = null) { $originalUser = $user; // TODO: Add all the insane error handling $loginResult = $this->userSession->login($user, $password); - if ($loginResult !== true) { + if ($loginResult !== true && $this->config->getSystemValue('strict_login_enforced', false) !== true) { $users = $this->userManager->getByEmail($user); // we only allow login by email if unique if (\count($users) === 1) { diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 0f2018fd6281..dec337b1dd5f 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -360,6 +360,10 @@ public function logClientIn($user, $password, IRequest $request) { throw new PasswordLoginForbiddenException(); } if (!$this->login($user, $password)) { + if ($this->config->getSystemValue('strict_login_enforced', false) === true) { + return false; + } + $users = $this->manager->getByEmail($user); if (\count($users) === 1) { return $this->login($users[0]->getUID(), $password); @@ -397,6 +401,9 @@ protected function isTwoFactorEnforced($username) { ); $user = $this->manager->get($username); if ($user === null) { + if ($this->config->getSystemValue('strict_login_enforced', false) === true) { + return false; + } $users = $this->manager->getByEmail($username); if (empty($users)) { return false; diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index 1280bf0666a7..1c22952e86f6 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -424,11 +424,28 @@ public function testShowLoginFormForUserNamedNull() { $this->assertEquals($expectedResponse, $this->loginController->showLoginForm('0', '', '')); } - public function testLoginWithInvalidCredentials() { + public function dataLoginWithInvalidCredentials() { + return [ + [false, 1], + [true, 0], + ]; + } + + /** + * @dataProvider dataLoginWithInvalidCredentials + * @param $strictLoginCheck + * @param $expectedGetByEmailCalls + */ + public function testLoginWithInvalidCredentials($strictLoginCheck, $expectedGetByEmailCalls) { $user = 'unknown'; $password = 'secret'; $loginPageUrl = 'some url'; + $this->config->expects($this->exactly(1)) + ->method('getSystemValue') + ->with('strict_login_enforced', false) + ->willReturn($strictLoginCheck); + $this->userSession->expects($this->once()) ->method('login') ->will($this->returnValue(false)); @@ -439,7 +456,8 @@ public function testLoginWithInvalidCredentials() { $this->userSession->expects($this->never()) ->method('createSessionToken'); - $this->userManager->expects($this->any())->method('getByEmail')->willReturn([]); + $this->userManager->expects($this->exactly($expectedGetByEmailCalls)) + ->method('getByEmail')->willReturn([]); $expected = new RedirectResponse($loginPageUrl); $this->assertEquals($expected, $this->loginController->tryLogin($user, $password, '/foo')); diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index db6623e521e5..e0a676d330a2 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -439,7 +439,19 @@ public function testLogClientInNoTokenPasswordWith2fa() { $userSession->logClientIn('john', 'doe', $request); } - public function testLogClientInUnexist() { + public function dataLogClientInUnexist() { + return [ + [false, 2], + [true, 0], + ]; + } + + /** + * @dataProvider dataLogClientInUnexist + * @param $strictLoginCheck + * @param $expectedGetByEmailCalls + */ + public function testLogClientInUnexist($strictLoginCheck, $expectedGetByEmailCalls) { $manager = $this->getMockBuilder(Manager::class) ->disableOriginalConstructor() ->getMock(); @@ -457,11 +469,19 @@ public function testLogClientInUnexist() { ->method('getToken') ->with('doe') ->will($this->throwException(new InvalidTokenException())); - $this->config->expects($this->once()) + $this->config->expects($this->at(0)) ->method('getSystemValue') ->with('token_auth_enforced', false) ->will($this->returnValue(false)); - $manager->expects($this->any()) + $this->config->expects($this->at(1)) + ->method('getSystemValue') + ->with('strict_login_enforced', false) + ->willReturn($strictLoginCheck); + $this->config->expects($this->at(2)) + ->method('getSystemValue') + ->with('strict_login_enforced', false) + ->willReturn($strictLoginCheck); + $manager->expects($this->exactly($expectedGetByEmailCalls)) ->method('getByEmail') ->willReturn([]);