diff --git a/frappe/auth.py b/frappe/auth.py index 26ff573330..f0ce61f160 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -237,15 +237,22 @@ class LoginManager: def validate_ip_address(self): """check if IP Address is valid""" - ip_list = frappe.db.get_value('User', self.user, 'restrict_ip', ignore=True) + user = frappe.get_doc("User", self.user) + ip_list = user.get_restricted_ip_list() if not ip_list: return - ip_list = ip_list.replace(",", "\n").split('\n') - ip_list = [i.strip() for i in ip_list] - + bypass_restrict_ip_check = 0 + # check if two factor auth is enabled + enabled = int(frappe.get_system_settings('enable_two_factor_auth') or 0) + if enabled: + #check if bypass restrict ip is enabled for all users + bypass_restrict_ip_check = int(frappe.get_system_settings('bypass_restrict_ip_check_if_2fa_enabled') or 0) + if not bypass_restrict_ip_check: + #check if bypass restrict ip is enabled for login user + bypass_restrict_ip_check = int(frappe.db.get_value('User', self.user, 'bypass_restrict_ip_check_if_2fa_enabled') or 0) for ip in ip_list: - if frappe.local.request_ip.startswith(ip): + if frappe.local.request_ip.startswith(ip) or bypass_restrict_ip_check: return frappe.throw(_("Not allowed from this IP Address"), frappe.AuthenticationError) diff --git a/frappe/core/doctype/system_settings/system_settings.js b/frappe/core/doctype/system_settings/system_settings.js index 62c044121b..aa60ea7e95 100644 --- a/frappe/core/doctype/system_settings/system_settings.js +++ b/frappe/core/doctype/system_settings/system_settings.js @@ -24,5 +24,6 @@ frappe.ui.form.on("System Settings", "enable_password_policy", function(frm) { 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); + frm.set_value("bypass_restrict_ip_check_if_2fa_enabled", 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 4d7d1d69da..d70bdd413a 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -995,6 +995,7 @@ "columns": 0, "default": "0", "depends_on": "enable_two_factor_auth", + "description": "If enabled, users who login from Restricted IP Address, won't be prompted for Two Factor Auth", "fieldname": "bypass_2fa_for_retricted_ip_users", "fieldtype": "Check", "hidden": 0, @@ -1019,6 +1020,38 @@ "set_only_once": 0, "unique": 0 }, + { + "allow_bulk_edit": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "depends_on": "enable_two_factor_auth", + "description": "If enabled, all users can login from any IP Address using Two Factor Auth. This can also be set only for specific user(s) in User Page", + "fieldname": "bypass_restrict_ip_check_if_2fa_enabled", + "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 restricted IP Address check If Two Factor Auth Enabled", + "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, @@ -1341,7 +1374,7 @@ "issingle": 1, "istable": 0, "max_attachments": 0, - "modified": "2018-01-16 07:19:46.261765", + "modified": "2018-05-23 16:45:46.261765", "modified_by": "Administrator", "module": "Core", "name": "System Settings", @@ -1376,4 +1409,4 @@ "sort_order": "ASC", "track_changes": 1, "track_seen": 0 -} \ No newline at end of file +} diff --git a/frappe/core/doctype/system_settings/system_settings.py b/frappe/core/doctype/system_settings/system_settings.py index bd35edb880..47a9226582 100644 --- a/frappe/core/doctype/system_settings/system_settings.py +++ b/frappe/core/doctype/system_settings/system_settings.py @@ -33,6 +33,7 @@ class SystemSettings(Document): toggle_two_factor_auth(True, roles=['All']) else: self.bypass_2fa_for_retricted_ip_users = 0 + self.bypass_restrict_ip_check_if_2fa_enabled = 0 def on_update(self): for df in self.meta.get("fields"): @@ -61,4 +62,4 @@ def load(): return { "timezones": get_all_timezones(), "defaults": defaults - } \ No newline at end of file + } diff --git a/frappe/core/doctype/user/user.json b/frappe/core/doctype/user/user.json index ae8ab5d9f5..00099c0b61 100644 --- a/frappe/core/doctype/user/user.json +++ b/frappe/core/doctype/user/user.json @@ -1661,6 +1661,38 @@ "set_only_once": 0, "unique": 0 }, + { + "allow_bulk_edit": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "depends_on": "eval:doc.restrict_ip && doc.restrict_ip.length", + "description": "If enabled, user can login from any IP Address using Two Factor Auth, this can also be set for all users in System Settings", + "fieldname": "bypass_restrict_ip_check_if_2fa_enabled", + "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 restricted IP Address check If Two Factor Auth Enabled", + "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, @@ -1890,7 +1922,7 @@ "istable": 0, "max_attachments": 5, "menu_index": 0, - "modified": "2018-04-04 10:59:52.839252", + "modified": "2018-05-23 16:59:52.839252", "modified_by": "Administrator", "module": "Core", "name": "User", @@ -1946,4 +1978,4 @@ "title_field": "full_name", "track_changes": 1, "track_seen": 0 -} \ No newline at end of file +} diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 7861a70f23..c5e73b52fe 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -516,6 +516,15 @@ class User(Document): self.append("social_logins", social_logins) + def get_restricted_ip_list(self): + if not self.restrict_ip: + return + + ip_list = self.restrict_ip.replace(",", "\n").split('\n') + ip_list = [i.strip() for i in ip_list] + + return ip_list + @frappe.whitelist() def get_timezones(): import pytz @@ -1048,4 +1057,4 @@ def create_contact(user): "gender": user.gender, "phone": user.phone, "mobile_no": user.mobile_no - }).insert(ignore_permissions=True) \ No newline at end of file + }).insert(ignore_permissions=True) diff --git a/frappe/tests/test_twofactor.py b/frappe/tests/test_twofactor.py index f704d2308a..b5eccec81d 100644 --- a/frappe/tests/test_twofactor.py +++ b/frappe/tests/test_twofactor.py @@ -52,38 +52,39 @@ class TestTwoFactor(unittest.TestCase): ''' 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 + 3. Should return true, if enabled and not bypass_2fa_for_retricted_ip_users and ip in restrict_ip + 4. Should return true, if enabled and bypass_2fa_for_retricted_ip_users and not restrict_ip + 5. Should return false, if enabled and bypass_2fa_for_retricted_ip_users and ip in restrict_ip ''' #Scenario 1 - disable_2fa() - self.assertFalse(two_factor_is_enabled(self.user)) + enable_2fa() + self.assertTrue(should_run_2fa(self.user)) #Scenario 2 - enable_2fa() - self.assertTrue(two_factor_is_enabled(self.user)) + disable_2fa() + self.assertFalse(should_run_2fa(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)) + self.assertTrue(should_run_2fa(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)) + self.assertTrue(should_run_2fa(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)) + self.assertFalse(should_run_2fa(self.user)) def test_two_factor_is_enabled_for_user(self): '''Should return true if enabled for user.''' @@ -126,6 +127,32 @@ class TestTwoFactor(unittest.TestCase): _str = render_string_template(_str,args) self.assertEqual(_str,'Verification Code from Frappe Technologies') + def test_bypass_restict_ip(self): + ''' + 1. Raise error if user not login from one of the restrict_ip, Bypass restrict ip check disabled by default + 2. Bypass restrict ip check enabled in System Settings + 3. Bypass restrict ip check enabled for User + ''' + + #1 + user = frappe.get_doc('User', self.user) + user.restrict_ip = "192.168.255.254" #Dummy IP + user.save() + enable_2fa(bypass_restrict_ip_check=0) + with self.assertRaises(frappe.AuthenticationError): + self.login_manager.validate_ip_address() + + #2 + enable_2fa(bypass_restrict_ip_check=1) + self.assertIsNone(self.login_manager.validate_ip_address()) + + #3 + user = frappe.get_doc('User', self.user) + user.bypass_restrict_ip_check_if_2fa_enabled = 1 + user.save() + enable_2fa() + self.assertIsNone(self.login_manager.validate_ip_address()) + def set_request(**kwargs): builder = EnvironBuilder(**kwargs) frappe.local.request = Request(builder.get_environ()) @@ -140,11 +167,12 @@ def create_http_request(): http_requests = HTTPRequest() return http_requests -def enable_2fa(bypass_two_factor_auth=0): +def enable_2fa(bypass_two_factor_auth=0, bypass_restrict_ip_check=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.bypass_restrict_ip_check_if_2fa_enabled = cint(bypass_restrict_ip_check) system_settings.two_factor_method = 'OTP App' system_settings.save(ignore_permissions=True) frappe.db.commit() @@ -152,7 +180,6 @@ def enable_2fa(bypass_two_factor_auth=0): 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() @@ -169,4 +196,4 @@ def toggle_2fa_all_role(state=None): def get_otp(user): otp_secret = get_otpsecret_for_(user) otp = pyotp.TOTP(otp_secret) - return otp.now() \ No newline at end of file + return otp.now() diff --git a/frappe/twofactor.py b/frappe/twofactor.py index b890a35831..931f8c3d6c 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -29,9 +29,14 @@ def two_factor_is_enabled(user=None): 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 + user_doc = frappe.get_doc("User", user) + restrict_ip_list = user_doc.get_restricted_ip_list() #can be None or one or more than one ip address + if restrict_ip_list: + for ip in restrict_ip_list: + if frappe.local.request_ip.startswith(ip): + enabled = False + break + if not user or not enabled: return enabled return two_factor_is_enabled_for_(user) @@ -385,4 +390,4 @@ def should_remove_barcode_image(barcode): return False def disable(): - frappe.db.set_value('System Settings', None, 'enable_two_factor_auth', 0) \ No newline at end of file + frappe.db.set_value('System Settings', None, 'enable_two_factor_auth', 0)