From 55882c75cbf38285bc4be37e0c92c6c04ef4e1f2 Mon Sep 17 00:00:00 2001
From: Nick Liu <deltik@gmx.com>
Date: Mon, 24 Aug 2020 23:40:25 -0500
Subject: [PATCH 1/2] Do not populate e_user_model as a logged in user if login
 failed

Fixes: #4236
---
 e107_handlers/login.php                    |  6 +++---
 e107_tests/tests/unit/e_user_modelTest.php | 12 ++++++++++--
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/e107_handlers/login.php b/e107_handlers/login.php
index 4c77fc212..b33b7017a 100644
--- a/e107_handlers/login.php
+++ b/e107_handlers/login.php
@@ -342,8 +342,8 @@ class userlogin
 	 * Note: PASSWORD IS NOT VERIFIED BY THIS ROUTINE
 	 * @param string $username - as entered
 	 * @param boolean $forceLogin - TRUE if login is being forced from clicking signup link; normally FALSE
-	 * @return TRUE if name exists, and $this->userData array set up
-	 *		   otherwise FALSE
+	 * @return boolean TRUE if name exists, and $this->userData array set up
+	 *		           FALSE otherwise
 	 */
 	protected function lookupUser($username, $forceLogin)
 	{
@@ -540,7 +540,7 @@ class userlogin
 		global $pref, $sql;
 
 		$doCheck = FALSE;			// Flag set if need to ban check
-
+		$this->userData = array();
 
 		switch($reason)
 		{
diff --git a/e107_tests/tests/unit/e_user_modelTest.php b/e107_tests/tests/unit/e_user_modelTest.php
index fba624ced..e613a4fcf 100644
--- a/e107_tests/tests/unit/e_user_modelTest.php
+++ b/e107_tests/tests/unit/e_user_modelTest.php
@@ -377,7 +377,15 @@
 
 		}
 */
+		/**
+		 * @see https://github.com/e107inc/e107/issues/4236
+		 */
+		public function testUserLoginWrongCredentialsNotUser()
+		{
+			$user = e107::getUser();
+			$user->login("e107", "DefinitelyTheWrongPassword");
 
-
-
+			$this->assertFalse($user->isUser());
+			$this->assertEmpty($user->getData());
+		}
 	}

From 4fbf4a93ca30c43dc952046becaff89fd808dd26 Mon Sep 17 00:00:00 2001
From: Nick Liu <deltik@gmx.com>
Date: Tue, 25 Aug 2020 00:12:40 -0500
Subject: [PATCH 2/2] Do not trigger user_login event if authentication fails

Related: #4236
---
 e107_handlers/login.php                    |  2 +-
 e107_handlers/user_model.php               |  8 ++++----
 e107_tests/tests/unit/e_user_modelTest.php | 20 ++++++++++++++++++++
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/e107_handlers/login.php b/e107_handlers/login.php
index b33b7017a..18eb56798 100644
--- a/e107_handlers/login.php
+++ b/e107_handlers/login.php
@@ -625,7 +625,7 @@ class userlogin
 			return $message;
 		}
 
-		define('LOGINMESSAGE', $message);
+		defined('LOGINMESSAGE') or define('LOGINMESSAGE', $message);
 
 	//	$sql->update('online', 'user_active = 0 WHERE user_ip = "'.$this->userIP.'" LIMIT 1');
 
diff --git a/e107_handlers/user_model.php b/e107_handlers/user_model.php
index f9443d86f..9912dcca7 100644
--- a/e107_handlers/user_model.php
+++ b/e107_handlers/user_model.php
@@ -1592,12 +1592,12 @@ class e_user extends e_user_model
 		if($this->isUser()) return false;
 
 		$userlogin = new userlogin();
-		$userlogin->login($uname, $upass_plain, $uauto, $uchallange, $noredirect);
-		
+		$loginSuccess = $userlogin->login($uname, $upass_plain, $uauto, $uchallange, $noredirect);
+
 		$userdata  = $userlogin->getUserData(); 
-		
 		$this->setSessionData(true)->setData($userdata);
-		
+		if ($loginSuccess === false) return false;
+
 		e107::getEvent()->trigger('user_login', $userdata); 	
 
 		return $this->isUser();
diff --git a/e107_tests/tests/unit/e_user_modelTest.php b/e107_tests/tests/unit/e_user_modelTest.php
index e613a4fcf..21ba46aa7 100644
--- a/e107_tests/tests/unit/e_user_modelTest.php
+++ b/e107_tests/tests/unit/e_user_modelTest.php
@@ -388,4 +388,24 @@
 			$this->assertFalse($user->isUser());
 			$this->assertEmpty($user->getData());
 		}
+
+		public function testUserLoginFailureDoesNotTriggerUserLoginEvent()
+		{
+			$originalEventHandler = e107::getRegistry('core/e107/singleton/e107_event');
+			$mockEventHandler = $this->createMock(e107_event::class);
+			$mockEventHandler->expects($this->never())->method('trigger');
+			e107::setRegistry('core/e107/singleton/e107_event', $mockEventHandler);
+
+			try
+			{
+				$user = e107::getUser();
+				$user->login("e107", "DefinitelyTheWrongPassword");
+
+				e107::getEvent();
+			}
+			finally
+			{
+				e107::setRegistry('core/e107/singleton/e107_event', $originalEventHandler);
+			}
+		}
 	}