vassili/seitime-frappe
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 4

Merge pull request #1295 from anandpdoshi/apply-user-permissions-fix

Browse source 
[major fix] Restrict apply user permissions if user permissions list if empty. Fixes frappe/erpnext#3194
This commit is contained in:
Anand Doshi 2015-09-08 15:44:34 +05:30
commit de34207e22
22 changed files with 153 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
frappe/change_log/current/ignore_user_permissions_if_missing.md Normal file
View file

@ -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.

4
frappe/core/doctype/report/report.json
View file

@ -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,

4
frappe/core/doctype/role/role.json
View file

@ -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,

25
frappe/core/doctype/system_settings/system_settings.json
View file

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

4
frappe/desk/doctype/event/event.json
View file

@ -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,

4
frappe/desk/doctype/feed/feed.json
View file

@ -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,

4
frappe/desk/doctype/note/note.json
View file

@ -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,

4
frappe/desk/doctype/todo/todo.json
View file

@ -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,

2
frappe/desk/form/load.py
View file

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

4
frappe/email/doctype/standard_reply/standard_reply.json
View file

@ -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,

4
frappe/geo/doctype/country/country.json
View file

@ -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,

4
frappe/geo/doctype/currency/currency.json
View file

@ -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,

21
frappe/model/db_query.py
View file

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

1
frappe/patches.txt
View file

@ -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

0
frappe/patches/v6_1/__init__.py Normal file
View file

8
frappe/patches/v6_1/ignore_user_permissions_if_missing.py Normal file
View file

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

32
frappe/permissions.py
View file

@ -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

4
frappe/print/doctype/letter_head/letter_head.json
View file

@ -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,

65
frappe/tests/test_permissions.py
View file

@ -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"))

4
frappe/website/doctype/blog_category/blog_category.json
View file

@ -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,

4
frappe/website/doctype/blog_post/blog_post.json
View file

@ -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,

4
frappe/website/doctype/blogger/blogger.json
View file

@ -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,