From 7249c179edd6ab9abf1f952f51e793551e43f337 Mon Sep 17 00:00:00 2001 From: Nikhil Kothari Date: Tue, 6 Sep 2022 13:48:00 +0530 Subject: [PATCH] feat: option to disable user pass based login (#18000) * Added checkbox to disable pass login in settings * Added user_pass disable option in Login page context * Hide user-pass fields when option disabled * Added check for social login key and LDAP * feat: Disable API based usr-pwd login * style: format with black * refactor: simpify auth validation No need for else clause * refactor: fixup sys setting json and move field * refactor: sys settings validation * refactor: simpler imports * chore: undo unintional changes * test: add test for disabled user pass Co-authored-by: Ankush Menat --- frappe/auth.py | 3 +++ .../doctype/system_settings/system_settings.json | 10 +++++++++- .../doctype/system_settings/system_settings.py | 16 ++++++++++++++++ frappe/tests/test_auth.py | 9 +++++++++ frappe/www/login.html | 11 ++++++++--- frappe/www/login.py | 4 +++- 6 files changed, 48 insertions(+), 5 deletions(-) diff --git a/frappe/auth.py b/frappe/auth.py index 7b64ecc083..7ce2b17680 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -140,6 +140,9 @@ class LoginManager: self.set_user_info() def login(self): + if frappe.get_system_settings("disable_user_pass_login"): + frappe.throw(_("Login with username and password is not allowed."), frappe.AuthenticationError) + # clear cache frappe.clear_cache(user=frappe.form_dict.get("usr")) user, pwd = get_cached_user_pass() diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index a444062b5a..0d612149a6 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -39,6 +39,7 @@ "deny_multiple_sessions", "allow_login_using_mobile_number", "allow_login_using_user_name", + "disable_user_pass_login", "allow_error_traceback", "strip_exif_metadata_from_uploaded_images", "allow_older_web_view_links", @@ -525,12 +526,19 @@ "fieldname": "email_retry_limit", "fieldtype": "Int", "label": "Email Retry Limit" + }, + { + "default": "0", + "description": "Make sure to configure a Social Login Key before disabling to prevent lockout", + "fieldname": "disable_user_pass_login", + "fieldtype": "Check", + "label": "Disable Username/Password Login" } ], "icon": "fa fa-cog", "issingle": 1, "links": [], - "modified": "2022-06-21 13:55:04.796152", + "modified": "2022-09-06 03:16:59.090906", "modified_by": "Administrator", "module": "Core", "name": "System Settings", diff --git a/frappe/core/doctype/system_settings/system_settings.py b/frappe/core/doctype/system_settings/system_settings.py index 4bd41be974..1fc27ca114 100644 --- a/frappe/core/doctype/system_settings/system_settings.py +++ b/frappe/core/doctype/system_settings/system_settings.py @@ -43,6 +43,22 @@ class SystemSettings(Document): ): frappe.flags.update_last_reset_password_date = True + self.validate_user_pass_login() + + def validate_user_pass_login(self): + if not self.disable_user_pass_login: + return + + social_login_enabled = frappe.db.exists("Social Login Key", {"enable_social_login": 1}) + ldap_enabled = frappe.db.get_single_value("LDAP Settings", "enabled") + + if not (social_login_enabled or ldap_enabled): + frappe.throw( + _( + "Please enable atleast one Social Login Key or LDAP before disabling username/password based login." + ) + ) + def on_update(self): self.set_defaults() diff --git a/frappe/tests/test_auth.py b/frappe/tests/test_auth.py index 4378d75484..403d0cf34c 100644 --- a/frappe/tests/test_auth.py +++ b/frappe/tests/test_auth.py @@ -98,6 +98,7 @@ class TestAuth(FrappeTestCase): def test_deny_multiple_login(self): self.set_system_settings("deny_multiple_sessions", 1) + self.addCleanup(self.set_system_settings, "deny_multiple_sessions", 0) first_login = FrappeClient(self.HOST_NAME, self.test_user_email, self.test_user_password) first_login.get_list("ToDo") @@ -114,6 +115,14 @@ class TestAuth(FrappeTestCase): second_login.get_list("ToDo") third_login.get_list("ToDo") + def test_disable_user_pass_login(self): + FrappeClient(self.HOST_NAME, self.test_user_email, self.test_user_password).get_list("ToDo") + self.set_system_settings("disable_user_pass_login", 1) + self.addCleanup(self.set_system_settings, "disable_user_pass_login", 0) + + with self.assertRaises(Exception): + FrappeClient(self.HOST_NAME, self.test_user_email, self.test_user_password).get_list("ToDo") + class TestLoginAttemptTracker(FrappeTestCase): def test_account_lock(self): diff --git a/frappe/www/login.html b/frappe/www/login.html index 1aaaf85656..897fdacd76 100644 --- a/frappe/www/login.html +++ b/frappe/www/login.html @@ -1,6 +1,7 @@ {% extends "templates/web.html" %} {% macro email_login_body() -%} +{% if not disable_user_pass_login %}
@@ -38,13 +39,15 @@

- {{ _("Forgot Password?") }}

+ {{ _("Forgot Password?") }} +

- +{% endif %}
+ {% if not disable_user_pass_login %} - + {% endif %} {% if ldap_settings and ldap_settings.enabled %} @@ -83,7 +86,9 @@ {{ email_login_body() }}