Add new system config to enforce strict login check with user backend

owncloud · Jun 22, 2020 · 4ebbebd · 4ebbebd
1 parent 4116040
commit 4ebbebd
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 6 deletions.
diff --git a/changelog/unreleased/37569 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
@@ -236,6 +236,15 @@
  */
 'token_auth_enforced' => false,
 
+/**
+ * Add new system config to enforce strict login check with user backend
+ * If enabled, strict login check for password
+in user backend will be enforced, meaning only login name typed by user would
+ * be validated. With this configuration enabled, e.g. 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
@@ -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
@@ -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
@@ -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
@@ -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([]);