Enhancement to allow User to login from any IP if two factor auth is enabled (#5209)
* Enhancement to allow login from any IP if two factor auth is enabled * Resolve Conflicts * optimize code
This commit is contained in:
parent
6a4a246ad7
commit
bcaabe5163
8 changed files with 143 additions and 28 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}).insert(ignore_permissions=True)
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
return otp.now()
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
frappe.db.set_value('System Settings', None, 'enable_two_factor_auth', 0)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue