diff --git a/frappe/core/doctype/system_settings/system_settings.js b/frappe/core/doctype/system_settings/system_settings.js index 11c01305e3..62c044121b 100644 --- a/frappe/core/doctype/system_settings/system_settings.js +++ b/frappe/core/doctype/system_settings/system_settings.js @@ -19,4 +19,10 @@ frappe.ui.form.on("System Settings", "enable_password_policy", function(frm) { } else { frm.set_value("minimum_password_score", "2"); } +}); + +frappe.ui.form.on("System Settings", "enable_two_factor_auth", function(frm) { + if(frm.doc.enable_two_factor_auth == 0){ + frm.set_value("bypass_2fa_for_retricted_ip_users", 0); + } }); \ No newline at end of file diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 42f873509a..d3c0379823 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -160,37 +160,37 @@ "unique": 0 }, { - "allow_bulk_edit": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fieldname": "is_first_startup", - "fieldtype": "Check", - "hidden": 1, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Is First Startup", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, + "allow_bulk_edit": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "fieldname": "is_first_startup", + "fieldtype": "Check", + "hidden": 1, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "label": "Is First Startup", + "length": 0, + "no_copy": 0, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 1, + "remember_last_selected_value": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "set_only_once": 0, "unique": 0 - }, + }, { - "allow_bulk_edit": 0, + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -986,8 +986,40 @@ "unique": 0 }, { - "allow_bulk_edit": 0, - "allow_on_submit": 0, + "allow_bulk_edit": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": "0", + "depends_on": "enable_two_factor_auth", + "fieldname": "bypass_2fa_for_retricted_ip_users", + "fieldtype": "Check", + "hidden": 0, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "label": "Bypass Two Factor Auth for users who login from restricted IP Address", + "length": 0, + "no_copy": 0, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "remember_last_selected_value": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "set_only_once": 0, + "unique": 0 + }, + { + "allow_bulk_edit": 0, + "allow_on_submit": 0, "bold": 0, "collapsible": 0, "columns": 0, @@ -1216,8 +1248,8 @@ "issingle": 1, "istable": 0, "max_attachments": 0, - "modified": "2017-08-31 14:53:31.065925", - "modified_by": "ewfds@wfe.ef", + "modified": "2017-08-31 14:53:31.065925", + "modified_by": "ewfds@wfe.ef", "module": "Core", "name": "System Settings", "name_case": "", diff --git a/frappe/core/doctype/system_settings/system_settings.py b/frappe/core/doctype/system_settings/system_settings.py index cd7edc6a53..ef2863fd46 100644 --- a/frappe/core/doctype/system_settings/system_settings.py +++ b/frappe/core/doctype/system_settings/system_settings.py @@ -31,6 +31,8 @@ class SystemSettings(Document): if not frappe.db.get_value('SMS Settings', None, 'sms_gateway_url'): frappe.throw(_('Please setup SMS before setting it as an authentication method, via SMS Settings')) toggle_two_factor_auth(True, roles=['All']) + else: + self.bypass_2fa_for_retricted_ip_users = 0 def on_update(self): for df in self.meta.get("fields"): @@ -59,4 +61,4 @@ def load(): return { "timezones": get_all_timezones(), "defaults": defaults - } + } \ No newline at end of file diff --git a/frappe/tests/test_twofactor.py b/frappe/tests/test_twofactor.py index bb1417ddd9..f704d2308a 100644 --- a/frappe/tests/test_twofactor.py +++ b/frappe/tests/test_twofactor.py @@ -6,9 +6,10 @@ import unittest, frappe, pyotp from werkzeug.wrappers import Request from werkzeug.test import EnvironBuilder from frappe.auth import HTTPRequest +from frappe.utils import cint from frappe.twofactor import (should_run_2fa, authenticate_for_2factor, get_cached_user_pass, two_factor_is_enabled_for_, confirm_otp_token, get_otpsecret_for_, get_verification_obj, - render_string_template) + render_string_template, two_factor_is_enabled) import time @@ -47,6 +48,43 @@ class TestTwoFactor(unittest.TestCase): self.assertTrue(frappe.cache().get('{0}{1}'.format(tmp_id,k)), '{} not available'.format(k)) + def test_two_factor_is_enabled(self): + ''' + 1. Should return true, if enabled and not bypass_2fa_for_retricted_ip_users + 2. Should return false, if not enabled + 3. Should return true, if enabled and bypass_2fa_for_retricted_ip_users and not user.restricted_ip + 4. Should return false, if enabled and bypass_2fa_for_retricted_ip_users and user.restricted_ip + ''' + + #Scenario 1 + disable_2fa() + self.assertFalse(two_factor_is_enabled(self.user)) + + #Scenario 2 + enable_2fa() + self.assertTrue(two_factor_is_enabled(self.user)) + + #Scenario 3 + enable_2fa() + user = frappe.get_doc('User', self.user) + user.restrict_ip = frappe.local.request_ip + user.save() + self.assertTrue(two_factor_is_enabled(self.user)) + + #Scenario 4 + user = frappe.get_doc('User', self.user) + user.restrict_ip = "" + user.save() + enable_2fa(1) + self.assertTrue(two_factor_is_enabled(self.user)) + + #Scenario 5 + user = frappe.get_doc('User', self.user) + user.restrict_ip = frappe.local.request_ip + user.save() + enable_2fa(1) + self.assertFalse(two_factor_is_enabled(self.user)) + def test_two_factor_is_enabled_for_user(self): '''Should return true if enabled for user.''' toggle_2fa_all_role(state=True) @@ -102,10 +140,11 @@ def create_http_request(): http_requests = HTTPRequest() return http_requests -def enable_2fa(): +def enable_2fa(bypass_two_factor_auth=0): '''Enable Two factor in system settings.''' system_settings = frappe.get_doc('System Settings') system_settings.enable_two_factor_auth = 1 + system_settings.bypass_2fa_for_retricted_ip_users = cint(bypass_two_factor_auth) system_settings.two_factor_method = 'OTP App' system_settings.save(ignore_permissions=True) frappe.db.commit() @@ -113,6 +152,7 @@ def enable_2fa(): def disable_2fa(): system_settings = frappe.get_doc('System Settings') system_settings.enable_two_factor_auth = 0 + system_settings.bypass_2fa_for_retricted_ip_users = 0 system_settings.save(ignore_permissions=True) frappe.db.commit() diff --git a/frappe/twofactor.py b/frappe/twofactor.py index 4bef5aceb7..bdfbd26a6f 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -26,6 +26,12 @@ def toggle_two_factor_auth(state, roles=[]): def two_factor_is_enabled(user=None): '''Returns True if 2FA is enabled.''' enabled = int(frappe.db.get_value('System Settings', None, 'enable_two_factor_auth') or 0) + if enabled: + bypass_two_factor_auth = int(frappe.db.get_value('System Settings', None, 'bypass_2fa_for_retricted_ip_users') or 0) + if bypass_two_factor_auth: + restrict_ip = frappe.db.get_value("User", filters={"name": user}, fieldname="restrict_ip") + if restrict_ip and bypass_two_factor_auth: + enabled = False if not user or not enabled: return enabled return two_factor_is_enabled_for_(user) @@ -85,7 +91,6 @@ def two_factor_is_enabled_for_(user): query = """select name from `tabRole` where two_factor_auth=1 and name in ({0}) limit 1""".format(', '.join('\"{}\"'.format(i) for i in roles)) - if len(frappe.db.sql(query)) > 0: return True