Merge pull request #4023 from schilgod/develop

Bypass 2FA if user login from restricted IP Address

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);
+	}
 });

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": "", 

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
-	}
+	}

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()
 

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