diff --git a/frappe/change_log/current/ignore_user_permissions_if_missing.md b/frappe/change_log/current/ignore_user_permissions_if_missing.md new file mode 100644 index 0000000000..05253243e3 --- /dev/null +++ b/frappe/change_log/current/ignore_user_permissions_if_missing.md @@ -0,0 +1,3 @@ +- **Permissions:** + - If User Permissions are missing for a DocType, don't show non-matching records. + - If **Ignore User Permissions If Missing** is checked in System Settings, show records even if User Permissions are not defined. diff --git a/frappe/core/doctype/report/report.json b/frappe/core/doctype/report/report.json index 9981a99afc..562cdba6e4 100644 --- a/frappe/core/doctype/report/report.json +++ b/frappe/core/doctype/report/report.json @@ -300,7 +300,7 @@ "is_submittable": 0, "issingle": 0, "istable": 0, - "modified": "2015-02-05 05:11:44.753200", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Core", "name": "Report", @@ -368,7 +368,7 @@ }, { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 0, "delete": 0, diff --git a/frappe/core/doctype/role/role.json b/frappe/core/doctype/role/role.json index 85a026820e..fb4b299077 100644 --- a/frappe/core/doctype/role/role.json +++ b/frappe/core/doctype/role/role.json @@ -41,7 +41,7 @@ "is_submittable": 0, "issingle": 0, "istable": 0, - "modified": "2015-02-05 05:11:44.831475", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Core", "name": "Role", @@ -89,7 +89,7 @@ }, { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 0, "delete": 0, diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 6eaa88bd55..4142edac7c 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -335,6 +335,29 @@ "set_only_once": 0, "unique": 0 }, + { + "allow_on_submit": 0, + "bold": 0, + "collapsible": 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", + "fieldname": "ignore_user_permissions_if_missing", + "fieldtype": "Check", + "hidden": 0, + "ignore_user_permissions": 0, + "in_filter": 0, + "in_list_view": 0, + "label": "Ignore User Permissions If Missing", + "no_copy": 0, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "read_only": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "set_only_once": 0, + "unique": 0 + }, { "allow_on_submit": 0, "bold": 0, @@ -432,7 +455,7 @@ "is_submittable": 0, "issingle": 1, "istable": 0, - "modified": "2015-05-21 07:15:55.682132", + "modified": "2015-09-07 11:36:15.465900", "modified_by": "Administrator", "module": "Core", "name": "System Settings", diff --git a/frappe/desk/doctype/event/event.json b/frappe/desk/doctype/event/event.json index 91e80b258b..33adad16c2 100644 --- a/frappe/desk/doctype/event/event.json +++ b/frappe/desk/doctype/event/event.json @@ -635,7 +635,7 @@ "is_submittable": 0, "issingle": 0, "istable": 0, - "modified": "2015-02-20 05:08:30.153381", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Desk", "name": "Event", @@ -643,7 +643,7 @@ "permissions": [ { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 1, "delete": 0, diff --git a/frappe/desk/doctype/feed/feed.json b/frappe/desk/doctype/feed/feed.json index 08c6cd401a..47f0c873a1 100644 --- a/frappe/desk/doctype/feed/feed.json +++ b/frappe/desk/doctype/feed/feed.json @@ -145,7 +145,7 @@ "is_submittable": 0, "issingle": 0, "istable": 0, - "modified": "2015-01-07 13:40:10.882588", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Desk", "name": "Feed", @@ -173,7 +173,7 @@ }, { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 0, "delete": 0, diff --git a/frappe/desk/doctype/note/note.json b/frappe/desk/doctype/note/note.json index 62304b7e4e..bba36ed669 100644 --- a/frappe/desk/doctype/note/note.json +++ b/frappe/desk/doctype/note/note.json @@ -84,7 +84,7 @@ "is_submittable": 0, "issingle": 0, "istable": 0, - "modified": "2015-07-28 16:18:12.301520", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Desk", "name": "Note", @@ -92,7 +92,7 @@ "permissions": [ { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 1, "delete": 1, diff --git a/frappe/desk/doctype/todo/todo.json b/frappe/desk/doctype/todo/todo.json index 1454c5202e..144a4d1165 100644 --- a/frappe/desk/doctype/todo/todo.json +++ b/frappe/desk/doctype/todo/todo.json @@ -335,7 +335,7 @@ "issingle": 0, "istable": 0, "max_attachments": 0, - "modified": "2015-06-11 16:06:34.561469", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Desk", "name": "ToDo", @@ -343,7 +343,7 @@ "permissions": [ { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 1, "delete": 0, diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index 82cc238068..345c097ad2 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -32,7 +32,7 @@ def getdoc(doctype, name, user=None): run_onload(doc) if not doc.has_permission("read"): - raise frappe.PermissionError, "read" + raise frappe.PermissionError, ("read", doctype, name) # add file list get_docinfo(doc) diff --git a/frappe/email/doctype/standard_reply/standard_reply.json b/frappe/email/doctype/standard_reply/standard_reply.json index f446b97d22..049c724a9e 100644 --- a/frappe/email/doctype/standard_reply/standard_reply.json +++ b/frappe/email/doctype/standard_reply/standard_reply.json @@ -83,7 +83,7 @@ "is_submittable": 0, "issingle": 0, "istable": 0, - "modified": "2015-07-28 16:18:12.432775", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Email", "name": "Standard Reply", @@ -92,7 +92,7 @@ "permissions": [ { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 1, "delete": 0, diff --git a/frappe/geo/doctype/country/country.json b/frappe/geo/doctype/country/country.json index 8098855411..165695d7f5 100644 --- a/frappe/geo/doctype/country/country.json +++ b/frappe/geo/doctype/country/country.json @@ -105,7 +105,7 @@ "is_submittable": 0, "issingle": 0, "istable": 0, - "modified": "2015-07-28 16:18:11.855617", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Geo", "name": "Country", @@ -133,7 +133,7 @@ }, { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 0, "delete": 0, diff --git a/frappe/geo/doctype/currency/currency.json b/frappe/geo/doctype/currency/currency.json index aab3d56931..4219cb9ba9 100644 --- a/frappe/geo/doctype/currency/currency.json +++ b/frappe/geo/doctype/currency/currency.json @@ -152,7 +152,7 @@ "is_submittable": 0, "issingle": 0, "istable": 0, - "modified": "2015-07-13 05:01:14.014983", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Geo", "name": "Currency", @@ -180,7 +180,7 @@ }, { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 0, "delete": 0, diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 10f7939d3f..411d16600c 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -323,8 +323,7 @@ class DatabaseQuery(object): tuple([frappe.db.escape(s) for s in self.shared]) 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) + 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: @@ -332,13 +331,17 @@ class DatabaseQuery(object): match_conditions = [] # check in links for df in meta.get_fields_to_check_permissions(doctypes): - match_conditions.append("""(ifnull(`tab{doctype}`.`{fieldname}`, "")="" - or `tab{doctype}`.`{fieldname}` in ({values}))""".format( - doctype=self.doctype, - fieldname=df.fieldname, - values=", ".join([('"'+frappe.db.escape(v)+'"') for v in user_permissions[df.options]]) - )) - match_filters[df.options] = user_permissions[df.options] + user_permission_values = user_permissions.get(df.options, []) + + condition = 'ifnull(`tab{doctype}`.`{fieldname}`, "")=""'.format(doctype=self.doctype, fieldname=df.fieldname) + if user_permission_values: + condition += """ or `tab{doctype}`.`{fieldname}` in ({values})""".format( + doctype=self.doctype, fieldname=df.fieldname, + values=", ".join([('"'+frappe.db.escape(v)+'"') for v in user_permission_values]) + ) + match_conditions.append("({condition})".format(condition=condition)) + + match_filters[df.options] = user_permission_values if match_conditions: self.match_conditions.append(" and ".join(match_conditions)) diff --git a/frappe/patches.txt b/frappe/patches.txt index e00dfd2746..de171d4b45 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -89,3 +89,4 @@ frappe.patches.v6_0.communication_status_and_permission frappe.patches.v6_0.make_task_log_folder frappe.patches.v6_0.document_type_rename frappe.patches.v6_0.fix_ghana_currency +frappe.patches.v6_1.ignore_user_permissions_if_missing diff --git a/frappe/patches/v6_1/__init__.py b/frappe/patches/v6_1/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/patches/v6_1/ignore_user_permissions_if_missing.py b/frappe/patches/v6_1/ignore_user_permissions_if_missing.py new file mode 100644 index 0000000000..9255c79c65 --- /dev/null +++ b/frappe/patches/v6_1/ignore_user_permissions_if_missing.py @@ -0,0 +1,8 @@ +from __future__ import unicode_literals +import frappe + +def execute(): + frappe.reload_doctype("System Settings") + system_settings = frappe.get_doc("System Settings") + system_settings.ignore_user_permissions_if_missing = 1 + system_settings.save() diff --git a/frappe/permissions.py b/frappe/permissions.py index 15ffb23ef6..0122fbde98 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -166,6 +166,7 @@ def get_role_permissions(meta, user=None, verbose=False): perms = frappe._dict({ "apply_user_permissions": {}, "user_permission_doctypes": {}, "if_owner": {} }) user_roles = frappe.get_roles(user) dont_match = [] + has_a_role_with_apply_user_permissions = False for p in meta.permissions: if cint(p.permlevel)==0 and (p.role in user_roles): @@ -186,18 +187,20 @@ def get_role_permissions(meta, user=None, verbose=False): dont_match.append(ptype) if p.apply_user_permissions: + has_a_role_with_apply_user_permissions = True + if p.user_permission_doctypes: # set user_permission_doctypes in perms user_permission_doctypes = json.loads(p.user_permission_doctypes) - - if user_permission_doctypes: - # perms["user_permission_doctypes"][ptype] would be a list of list like [["User", "Blog Post"], ["User"]] - for ptype in rights: - if p.get(ptype): - perms["user_permission_doctypes"].setdefault(ptype, []).append(user_permission_doctypes) else: user_permission_doctypes = get_linked_doctypes(meta.name) + if user_permission_doctypes: + # perms["user_permission_doctypes"][ptype] would be a list of list like [["User", "Blog Post"], ["User"]] + for ptype in rights: + if p.get(ptype): + perms["user_permission_doctypes"].setdefault(ptype, []).append(user_permission_doctypes) + # if atleast one record having both Apply User Permission and If Owner unchecked is found, # don't match for those rights for ptype in rights: @@ -210,9 +213,11 @@ def get_role_permissions(meta, user=None, verbose=False): # if one row has only "Apply User Permissions" checked and another has only "If Owner" checked, # set Apply User Permissions as checked - for ptype in rights: - if perms["if_owner"].get(ptype) and perms["apply_user_permissions"].get(ptype)==0: - perms["apply_user_permissions"][ptype] = 1 + # i.e. the case when there is a role with apply_user_permissions as 1, but resultant apply_user_permissions is 0 + if has_a_role_with_apply_user_permissions: + for ptype in rights: + if perms["if_owner"].get(ptype) and perms["apply_user_permissions"].get(ptype)==0: + perms["apply_user_permissions"][ptype] = 1 # delete 0 values for key, value in perms.get("apply_user_permissions").items(): @@ -239,8 +244,8 @@ def user_has_permission(doc, verbose=True, user=None, user_permission_doctypes=N result = True for df in meta.get_fields_to_check_permissions(doctypes): - if (df.options in user_permissions and d.get(df.fieldname) - and d.get(df.fieldname) not in user_permissions[df.options]): + if (d.get(df.fieldname) + and d.get(df.fieldname) not in user_permissions.get(df.options, [])): result = False if verbose: @@ -334,14 +339,11 @@ def apply_user_permissions(doctype, ptype, user=None): def get_user_permission_doctypes(user_permission_doctypes, user_permissions): """returns a list of list like [["User", "Blog Post"], ["User"]]""" - if user_permission_doctypes: + if cint(frappe.db.get_single_value("System Settings", "ignore_user_permissions_if_missing")): # select those user permission doctypes for which user permissions exist! user_permission_doctypes = [list(set(doctypes).intersection(set(user_permissions.keys()))) for doctypes in user_permission_doctypes] - else: - user_permission_doctypes = [user_permissions.keys()] - if len(user_permission_doctypes) > 1: # OPTIMIZATION # if intersection exists, use that to reduce the amount of querying diff --git a/frappe/print/doctype/letter_head/letter_head.json b/frappe/print/doctype/letter_head/letter_head.json index 4fd3ccb49f..b70dd49587 100644 --- a/frappe/print/doctype/letter_head/letter_head.json +++ b/frappe/print/doctype/letter_head/letter_head.json @@ -116,7 +116,7 @@ "issingle": 0, "istable": 0, "max_attachments": 3, - "modified": "2015-02-05 05:11:40.906941", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Print", "name": "Letter Head", @@ -144,7 +144,7 @@ }, { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 0, "delete": 0, diff --git a/frappe/tests/test_permissions.py b/frappe/tests/test_permissions.py index 88757dd167..bffea600ec 100644 --- a/frappe/tests/test_permissions.py +++ b/frappe/tests/test_permissions.py @@ -26,6 +26,11 @@ class TestPermissions(unittest.TestCase): user = frappe.get_doc("User", "test2@example.com") user.add_roles("Blogger") + frappe.db.sql("""update `tabDocPerm` set if_owner=0 + where parent='Blog Post' and permlevel=0 and permlevel=0 and role='Blogger'""") + + self.set_ignore_user_permissions_if_missing(0) + frappe.set_user("test1@example.com") def tearDown(self): @@ -36,18 +41,28 @@ class TestPermissions(unittest.TestCase): clear_user_permissions_for_doctype("Blog Post") clear_user_permissions_for_doctype("Blogger") - frappe.db.sql("""update `tabDocPerm` set user_permission_doctypes=null + frappe.db.sql("""update `tabDocPerm` set user_permission_doctypes=null, apply_user_permissions=0 where parent='Blog Post' and permlevel=0 and apply_user_permissions=1 and `read`=1""") frappe.db.sql("""update `tabDocPerm` set if_owner=0 where parent='Blog Post' and permlevel=0 and permlevel=0 and role='Blogger'""") + self.set_ignore_user_permissions_if_missing(0) + + def set_ignore_user_permissions_if_missing(self, ignore): + ss = frappe.get_doc("System Settings") + ss.ignore_user_permissions_if_missing = 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")) def test_user_permissions_in_doc(self): + self.set_user_permission_doctypes(["Blog Category"]) + frappe.permissions.add_user_permission("Blog Category", "_Test Blog Category 1", "test2@example.com") @@ -62,6 +77,8 @@ class TestPermissions(unittest.TestCase): self.assertTrue(get_doc_permissions(post1).get("read")) def test_user_permissions_in_report(self): + self.set_user_permission_doctypes(["Blog Category"]) + frappe.permissions.add_user_permission("Blog Category", "_Test Blog Category 1", "test2@example.com") frappe.set_user("test2@example.com") @@ -78,6 +95,8 @@ class TestPermissions(unittest.TestCase): self.assertEquals(doc.get("blog_category"), "_Test Blog Category 1") def test_user_link_match_doc(self): + self.set_user_permission_doctypes(["Blogger"]) + blogger = frappe.get_doc("Blogger", "_Test Blogger 1") blogger.user = "test2@example.com" blogger.save() @@ -91,6 +110,8 @@ class TestPermissions(unittest.TestCase): self.assertFalse(post1.has_permission("read")) def test_user_link_match_report(self): + self.set_user_permission_doctypes(["Blogger"]) + blogger = frappe.get_doc("Blogger", "_Test Blogger 1") blogger.user = "test2@example.com" blogger.save() @@ -113,6 +134,8 @@ class TestPermissions(unittest.TestCase): "test2@example.com", "Blog Post", "_test-blog-post") def test_read_if_explicit_user_permissions_are_set(self): + self.set_user_permission_doctypes(["Blog Post"]) + self.test_set_user_permissions() frappe.set_user("test2@example.com") @@ -140,6 +163,8 @@ class TestPermissions(unittest.TestCase): doc = frappe.get_doc("Blog Post", "_test-blog-post-1") self.assertTrue(doc.has_permission("read")) + self.set_user_permission_doctypes(["Blog Post"]) + frappe.set_user("test1@example.com") add("test2@example.com", "Blog Post", "_test-blog-post") @@ -166,9 +191,7 @@ class TestPermissions(unittest.TestCase): frappe.set_user("test2@example.com") - frappe.db.sql("""update `tabDocPerm` set user_permission_doctypes=%s - where parent='Blog Post' and permlevel=0 and apply_user_permissions=1 - and `read`=1""", json.dumps(["Blogger"])) + self.set_user_permission_doctypes(["Blogger"]) frappe.model.meta.clear_cache("Blog Post") @@ -195,8 +218,17 @@ class TestPermissions(unittest.TestCase): frappe.model.meta.clear_cache("Blog Post") + def set_user_permission_doctypes(self, user_permission_doctypes): + frappe.db.sql("""update `tabDocPerm` set apply_user_permissions=1, + user_permission_doctypes=%s + where parent='Blog Post' and permlevel=0 + and `read`=1 and role='Blogger'""", json.dumps(user_permission_doctypes)) + + frappe.clear_cache(doctype="Blog Post") + def test_insert_if_owner_with_user_permissions(self): """If `If Owner` is checked for a Role, check if that document is allowed to be read, updated, submitted, etc. except be created, even if the document is restricted based on User Permissions.""" + self.set_user_permission_doctypes(["Blog Category"]) self.if_owner_setup() frappe.set_user("test2@example.com") @@ -227,3 +259,28 @@ class TestPermissions(unittest.TestCase): self.assertTrue(doc.has_permission("read")) self.assertTrue(doc.has_permission("write")) self.assertFalse(doc.has_permission("create")) + + def test_ignore_user_permissions_if_missing(self): + """If `Ignore User Permissions If Missing` is checked in System Settings, show records even if User Permissions are missing for a linked doctype""" + self.set_user_permission_doctypes(['Blog Category', 'Blog Post', 'Blogger']) + + frappe.set_user("Administrator") + frappe.permissions.add_user_permission("Blog Category", "_Test Blog Category", + "test2@example.com") + frappe.set_user("test2@example.com") + + doc = frappe.get_doc({ + "doctype": "Blog Post", + "blog_category": "_Test Blog Category", + "blogger": "_Test Blogger 1", + "title": "_Test Blog Post Title", + "content": "_Test Blog Post Content" + }) + + self.assertFalse(doc.has_permission("write")) + + frappe.set_user("Administrator") + self.set_ignore_user_permissions_if_missing(1) + + frappe.set_user("test2@example.com") + self.assertTrue(doc.has_permission("write")) diff --git a/frappe/website/doctype/blog_category/blog_category.json b/frappe/website/doctype/blog_category/blog_category.json index 56b9913279..373df96e26 100644 --- a/frappe/website/doctype/blog_category/blog_category.json +++ b/frappe/website/doctype/blog_category/blog_category.json @@ -126,7 +126,7 @@ "is_submittable": 0, "issingle": 0, "istable": 0, - "modified": "2015-07-28 16:18:11.486847", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Website", "name": "Blog Category", @@ -154,7 +154,7 @@ }, { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 0, "delete": 0, diff --git a/frappe/website/doctype/blog_post/blog_post.json b/frappe/website/doctype/blog_post/blog_post.json index fd2c8049f1..ab3f83e041 100644 --- a/frappe/website/doctype/blog_post/blog_post.json +++ b/frappe/website/doctype/blog_post/blog_post.json @@ -272,7 +272,7 @@ "issingle": 0, "istable": 0, "max_attachments": 5, - "modified": "2015-04-29 01:46:16.190210", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Website", "name": "Blog Post", @@ -300,7 +300,7 @@ }, { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 1, "delete": 0, diff --git a/frappe/website/doctype/blogger/blogger.json b/frappe/website/doctype/blogger/blogger.json index 5982b44155..a862f754e6 100644 --- a/frappe/website/doctype/blogger/blogger.json +++ b/frappe/website/doctype/blogger/blogger.json @@ -171,7 +171,7 @@ "issingle": 0, "istable": 0, "max_attachments": 1, - "modified": "2015-07-28 16:18:11.567110", + "modified": "2015-09-07 15:51:26", "modified_by": "Administrator", "module": "Website", "name": "Blogger", @@ -199,7 +199,7 @@ }, { "amend": 0, - "apply_user_permissions": 1, + "apply_user_permissions": 0, "cancel": 0, "create": 0, "delete": 0,