fix: Login with username fails (#12658)

With the recent refactoring we missed to include username check in auth
flow. Added test cases along with the fix.

frappe/core/doctype/user/user.py

@@ -534,30 +534,36 @@ class User(Document):
 	@classmethod
 	def find_by_credentials(cls, user_name: str, password: str, validate_password: bool = True):
 		"""Find the user by credentials.
+
+		This is a login utility that needs to check login related system settings while finding the user.
+		1. Find user by email ID by default
+		2. If allow_login_using_mobile_number is set, you can use mobile number while finding the user.
+		3. If allow_login_using_user_name is set, you can use username while finding the user.
 		"""
+
 		login_with_mobile = cint(frappe.db.get_value("System Settings", "System Settings", "allow_login_using_mobile_number"))
+		login_with_username = cint(frappe.db.get_value("System Settings", "System Settings", "allow_login_using_user_name"))
 
-		user = None
+		or_filters = [{"name": user_name}]
 		if login_with_mobile:
-			filter = {"mobile_no": user_name}
-			user = frappe.db.get_value("User", filters=filter, fieldname=['name', 'enabled'], as_dict=True)
-		if not user:
-			filter = {"name": user_name}
-			user = frappe.db.get_value("User", filters=filter, fieldname=['name', 'enabled'], as_dict=True)
+			or_filters.append({"mobile_no": user_name})
+		if login_with_username:
+			or_filters.append({"username": user_name})
 
-		if not user:
+		users = frappe.db.get_all('User', fields=['name', 'enabled'], or_filters=or_filters, limit=1)
+		if not users:
 			return
 
+		user = users[0]
 		user['is_authenticated'] = True
 		if validate_password:
 			try:
-				check_password(user_name, password)
+				check_password(user['name'], password)
 			except frappe.AuthenticationError:
 				user['is_authenticated'] = False
 
 		return user
 
-
 @frappe.whitelist()
 def get_timezones():
 	import pytz

frappe/tests/test_auth.py

@@ -7,21 +7,85 @@ import unittest
 
 import frappe
 from frappe.auth import LoginAttemptTracker
-from frappe.frappeclient import FrappeClient
-
+from frappe.frappeclient import FrappeClient, AuthError
 
 class TestAuth(unittest.TestCase):
-	def test_admin_login(self):
-		# Make sure that authentication works when allow_login_using_mobile_number is set to 0
-		frappe.db.set_value("System Settings", "System Settings", "allow_login_using_mobile_number", 0)
+	def __init__(self, *args, **kwargs):
+		super(TestAuth, self).__init__(*args, **kwargs)
+		self.test_user_email = 'test@test.com'
+		self.test_user_name = 'test_user'
+		self.test_user_mobile = '+911234567890'
+		self.test_user_password = 'pwd_012'
+
+	def setUp(self):
+		self.tearDown()
+
+		self.add_user(self.test_user_email, self.test_user_password,
+			username=self.test_user_name, mobile_no=self.test_user_mobile)
+
+	def tearDown(self):
+		frappe.delete_doc('User', self.test_user_email, force=True)
+
+	def add_user(self, email, password, username=None, mobile_no=None):
+		first_name = email.split('@', 1)[0]
+		user = frappe.get_doc(
+				dict(doctype='User', email=email, first_name=first_name, username=username, mobile_no=mobile_no)
+			).insert()
+		user.new_password = password
+		user.save()
 		frappe.db.commit()
-		FrappeClient(frappe.get_site_config().host_name, "Administrator", "admin", verify=False)
 
-		# Make sure that authentication works when allow_login_using_mobile_number is set to 1
-		frappe.db.set_value("System Settings", "System Settings", "allow_login_using_mobile_number", 1)
+	def set_system_settings(self, k, v):
+		frappe.db.set_value("System Settings", "System Settings", k, v)
 		frappe.db.commit()
-		FrappeClient(frappe.get_site_config().host_name, "Administrator", "admin", verify=False)
 
+	def test_allow_login_using_mobile(self):
+		self.set_system_settings('allow_login_using_mobile_number', 1)
+		self.set_system_settings('allow_login_using_user_name', 0)
+
+		# Login by both email and mobile should work
+		FrappeClient(frappe.get_site_config().host_name, self.test_user_mobile, self.test_user_password)
+		FrappeClient(frappe.get_site_config().host_name, self.test_user_email, self.test_user_password)
+
+		# login by username should fail
+		with self.assertRaises(AuthError):
+			FrappeClient(frappe.get_site_config().host_name, self.test_user_name, self.test_user_password)
+
+	def test_allow_login_using_only_email(self):
+		self.set_system_settings('allow_login_using_mobile_number', 0)
+		self.set_system_settings('allow_login_using_user_name', 0)
+
+		# Login by mobile number should fail
+		with self.assertRaises(AuthError):
+			FrappeClient(frappe.get_site_config().host_name, self.test_user_mobile, self.test_user_password)
+
+		# login by username should fail
+		with self.assertRaises(AuthError):
+			FrappeClient(frappe.get_site_config().host_name, self.test_user_name, self.test_user_password)
+
+		# Login by email should work
+		FrappeClient(frappe.get_site_config().host_name, self.test_user_email, self.test_user_password)
+
+	def test_allow_login_using_username(self):
+		self.set_system_settings('allow_login_using_mobile_number', 0)
+		self.set_system_settings('allow_login_using_user_name', 1)
+
+		# Mobile login should fail
+		with self.assertRaises(AuthError):
+			FrappeClient(frappe.get_site_config().host_name, self.test_user_mobile, self.test_user_password)
+
+		# Both email and username logins should work
+		FrappeClient(frappe.get_site_config().host_name, self.test_user_email, self.test_user_password)
+		FrappeClient(frappe.get_site_config().host_name, self.test_user_name, self.test_user_password)
+
+	def test_allow_login_using_username_and_mobile(self):
+		self.set_system_settings('allow_login_using_mobile_number', 1)
+		self.set_system_settings('allow_login_using_user_name', 1)
+
+		# Both email and username and mobile logins should work
+		FrappeClient(frappe.get_site_config().host_name, self.test_user_mobile, self.test_user_password)
+		FrappeClient(frappe.get_site_config().host_name, self.test_user_email, self.test_user_password)
+		FrappeClient(frappe.get_site_config().host_name, self.test_user_name, self.test_user_password)
 
 class TestLoginAttemptTracker(unittest.TestCase):
 	def test_account_lock(self):