Apply User Permissions is being ignored for blank field values (#3496)

* issue where Apply User Permissions is being ignored for blank field values

* configurable settings and added test case

* Update system_settings.json

* save and use locally stored value for strict user permission

frappe/__init__.py

@@ -138,8 +138,7 @@ def init(site, sites_path=None, new_site=False):
 
 	local.module_app = None
 	local.app_modules = None
-	local.system_settings = None
-	local.system_country = None
+	local.system_settings = _dict()
 
 	local.user = None
 	local.user_perms = None
@@ -1364,7 +1363,7 @@ def get_active_domains():
 
 	return active_domains
 
-def get_system_country():
-	if local.system_country is None:
-		local.system_country = db.get_single_value('System Settings', 'country') or ''
-	return local.system_country
+def get_system_settings(key):
+	if not local.system_settings.has_key(key):
+		local.system_settings.update({key: db.get_single_value('System Settings', key)})
+	return local.system_settings.get(key)

frappe/contacts/doctype/contact/test_records.json

@@ -1,6 +1,7 @@
 [
  {
   "doctype": "Contact",
+  "salutation": "Mr",
   "email_id": "test_conctact@example.com",
   "first_name": "_Test Contact For _Test Customer",
   "is_primary_contact": 1,

frappe/contacts/doctype/salutation/test_records.json

@@ -0,0 +1,8 @@
+[
+ {
+  "salutation": "Mr"
+ },
+ {
+  "salutation": "Mrs"
+ }
+]

frappe/core/doctype/system_settings/system_settings.json

@@ -745,7 +745,7 @@
    "bold": 0, 
    "collapsible": 0, 
    "columns": 0, 
-   "description": "eg. If Apply User Permissions is checked for Report DocType but no User Permissions are defined for Report for a User, then all Reports are shown to that User", 
+   "description": "If Apply User Permissions is checked for Report DocType but no User Permissions are defined for Report for a User, then all Reports are shown to that User", 
    "fieldname": "ignore_user_permissions_if_missing", 
    "fieldtype": "Check", 
    "hidden": 0, 
@@ -770,6 +770,38 @@
    "set_only_once": 0, 
    "unique": 0
   }, 
+  {
+   "allow_bulk_edit": 0, 
+   "allow_on_submit": 0, 
+   "bold": 0, 
+   "collapsible": 0, 
+   "columns": 0, 
+   "default": "0", 
+   "description": "If Apply Strict User Permission is checked and User Permission is defined for a DocType for a User, then all the documents where value of the link is blank, will not be shown to that User", 
+   "fieldname": "apply_strict_user_permissions", 
+   "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": "Apply Strict User Permissions", 
+   "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, 
@@ -965,7 +997,7 @@
  "issingle": 1, 
  "istable": 0, 
  "max_attachments": 0, 
- "modified": "2017-06-12 13:05:28.924098", 
+ "modified": "2017-06-23 07:48:10.453011", 
  "modified_by": "Administrator", 
  "module": "Core", 
  "name": "System Settings", 

frappe/core/doctype/user/test_records.json

@@ -31,6 +31,13 @@
   "new_password": "Eastern_43A1W",
   "enabled": 1
  },
+ {
+  "doctype": "User",
+  "email": "test3@example.com",
+  "first_name": "_Test3",
+  "new_password": "Eastern_43A1W",
+  "enabled": 1
+ },
  {
   "doctype": "User",
   "email": "testperm@example.com",

frappe/desk/form/meta.py

@@ -65,7 +65,7 @@ class FormMeta(Meta):
 		def _get_path(fname):
 			return os.path.join(path, scrub(fname))
 
-		system_country = frappe.get_system_country()
+		system_country = frappe.get_system_settings("country")
 
 		self._add_code(_get_path(self.name + '.js'), '__js')
 		if system_country:

frappe/model/db_query.py

@@ -423,7 +423,6 @@ class DatabaseQuery(object):
 	def add_user_permissions(self, user_permissions, user_permission_doctypes=None):
 		user_permission_doctypes = frappe.permissions.get_user_permission_doctypes(user_permission_doctypes, user_permissions)
 		meta = frappe.get_meta(self.doctype)
-
 		for doctypes in user_permission_doctypes:
 			match_filters = {}
 			match_conditions = []
@@ -431,12 +430,18 @@ class DatabaseQuery(object):
 			for df in meta.get_fields_to_check_permissions(doctypes):
 				user_permission_values = user_permissions.get(df.options, [])
 
-				condition = 'ifnull(`tab{doctype}`.`{fieldname}`, "")=""'.format(doctype=self.doctype, fieldname=df.fieldname)
+				cond = 'ifnull(`tab{doctype}`.`{fieldname}`, "")=""'.format(doctype=self.doctype, fieldname=df.fieldname)
 				if user_permission_values:
-					condition += """ or `tab{doctype}`.`{fieldname}` in ({values})""".format(
+					if not cint(frappe.get_system_settings("apply_strict_user_permissions")):
+						condition = cond + " or "
+					else:
+						condition = ""
+					condition += """`tab{doctype}`.`{fieldname}` in ({values})""".format(
 						doctype=self.doctype, fieldname=df.fieldname,
-						values=", ".join([('"'+frappe.db.escape(v, percent=False)+'"') for v in user_permission_values])
-					)
+						values=", ".join([('"'+frappe.db.escape(v, percent=False)+'"') for v in user_permission_values]))
+				else:
+					condition = cond
+
 				match_conditions.append("({condition})".format(condition=condition))
 
 				match_filters[df.options] = user_permission_values

frappe/tests/test_permissions.py

@@ -15,11 +15,12 @@ from frappe.core.page.permission_manager.permission_manager import update, reset
 
 test_records = frappe.get_test_records('Blog Post')
 
-test_dependencies = ["User"]
+test_dependencies = ["User", "Contact", "Salutation"]
 
 class TestPermissions(unittest.TestCase):
 	def setUp(self):
 		frappe.clear_cache(doctype="Blog Post")
+		frappe.clear_cache(doctype="Contact")
 
 		user = frappe.get_doc("User", "test1@example.com")
 		user.add_roles("Website Manager")
@@ -27,8 +28,13 @@ class TestPermissions(unittest.TestCase):
 		user = frappe.get_doc("User", "test2@example.com")
 		user.add_roles("Blogger")
 
+		user = frappe.get_doc("User", "test3@example.com")
+		user.add_roles("Sales User")
+
 		reset('Blogger')
 		reset('Blog Post')
+		reset('Contact')
+		reset('Salutation')
 
 		self.set_ignore_user_permissions_if_missing(0)
 
@@ -41,18 +47,30 @@ class TestPermissions(unittest.TestCase):
 		clear_user_permissions_for_doctype("Blog Category")
 		clear_user_permissions_for_doctype("Blog Post")
 		clear_user_permissions_for_doctype("Blogger")
+		clear_user_permissions_for_doctype("Contact")
+		clear_user_permissions_for_doctype("Salutation")
 
 		reset('Blogger')
 		reset('Blog Post')
+		reset('Contact')
+		reset('Salutation')
 
 		self.set_ignore_user_permissions_if_missing(0)
 
-	def set_ignore_user_permissions_if_missing(self, ignore):
+	@staticmethod
+	def set_ignore_user_permissions_if_missing(ignore):
 		ss = frappe.get_doc("System Settings")
 		ss.ignore_user_permissions_if_missing = ignore
 		ss.flags.ignore_mandatory = 1
 		ss.save()
 
+	@staticmethod
+	def set_strict_user_permissions(ignore):
+		ss = frappe.get_doc("System Settings")
+		ss.apply_strict_user_permissions = ignore
+		ss.flags.ignore_mandatory = 1
+		ss.save()
+
 	def test_basic_permission(self):
 		post = frappe.get_doc("Blog Post", "-test-blog-post")
 		self.assertTrue(post.has_permission("read"))
@@ -275,6 +293,30 @@ class TestPermissions(unittest.TestCase):
 		frappe.set_user("test2@example.com")
 		self.assertTrue(doc.has_permission("write"))
 
+	def test_strict_user_permissions(self):
+		"""If `Strict User Permissions` is checked in System Settings, show records even if User Permissions are missing for a linked doctype"""
+		set_user_permission_doctypes(doctype="Contact", role="Sales User",
+			apply_user_permissions=1, user_permission_doctypes=['Salutation'])
+		set_user_permission_doctypes(doctype="Salutation", role="All",
+			apply_user_permissions=1, user_permission_doctypes=['Salutation'])
+
+		frappe.set_user("Administrator")
+		frappe.permissions.add_user_permission("Salutation", "Mr", "test3@example.com")
+		self.set_strict_user_permissions(0)
+
+		frappe.set_user("test3@example.com")
+		self.assertEquals(len(frappe.get_list("Contact")),2)
+
+		frappe.set_user("Administrator")
+		self.set_strict_user_permissions(1)
+
+		frappe.set_user("test3@example.com")
+		self.assertTrue(len(frappe.get_list("Contact")),1)
+
+		frappe.set_user("Administrator")
+		self.set_strict_user_permissions(0)
+
+
 def set_user_permission_doctypes(doctype, role, apply_user_permissions, user_permission_doctypes):
 	user_permission_doctypes = None if not user_permission_doctypes else json.dumps(user_permission_doctypes)