From 69d76499694608f66a21754164ac135ffa901121 Mon Sep 17 00:00:00 2001 From: Saif Ur Rehman Date: Sat, 28 Dec 2019 16:49:54 +0500 Subject: [PATCH 1/6] fix(get_permlevel_access): Lazy load handle different permission types --- frappe/model/document.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index c58e09ef5a..0539f8806d 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -577,14 +577,17 @@ class Document(BaseDocument): def get_permlevel_access(self, permission_type='write'): if not hasattr(self, "_has_access_to"): + self._has_access_to = frappe._dict() + + if not self._has_access_to.get(permission_type): + self._has_access_to[permission_type] = [] roles = frappe.get_roles() - self._has_access_to = [] for perm in self.get_permissions(): if perm.role in roles and perm.permlevel > 0 and perm.get(permission_type): - if perm.permlevel not in self._has_access_to: - self._has_access_to.append(perm.permlevel) + if perm.permlevel not in self._has_access_to[permission_type]: + self._has_access_to[permission_type].append(perm.permlevel) - return self._has_access_to + return self._has_access_to[permission_type] def has_permlevel_access_to(self, fieldname, df=None, permission_type='read'): if not df: From dd72d5e022cbcaeac9273b44b3d2e8a9fd56269c Mon Sep 17 00:00:00 2001 From: Saif Ur Rehman Date: Sat, 28 Dec 2019 16:58:12 +0500 Subject: [PATCH 2/6] chore: No need for frappe._dict() --- frappe/model/document.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 0539f8806d..9a6baf63c4 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -577,7 +577,7 @@ class Document(BaseDocument): def get_permlevel_access(self, permission_type='write'): if not hasattr(self, "_has_access_to"): - self._has_access_to = frappe._dict() + self._has_access_to = {} if not self._has_access_to.get(permission_type): self._has_access_to[permission_type] = [] From c9adf965eb6b637e45f53addc650859a50e2603e Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 21 Jan 2020 15:41:03 +0530 Subject: [PATCH 3/6] test: Fix fieldlevel permission test --- frappe/tests/test_form_load.py | 95 ++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 44 deletions(-) diff --git a/frappe/tests/test_form_load.py b/frappe/tests/test_form_load.py index 95dc19c0f1..e934aed222 100644 --- a/frappe/tests/test_form_load.py +++ b/frappe/tests/test_form_load.py @@ -4,8 +4,10 @@ from __future__ import unicode_literals import frappe, unittest from frappe.desk.form.load import getdoctype, getdoc -from frappe.core.page.permission_manager.permission_manager import update, reset +from frappe.core.page.permission_manager.permission_manager import update, reset, add +from frappe.custom.doctype.property_setter.property_setter import make_property_setter +test_dependencies = ['Blog Category', 'Blogger'] class TestFormLoad(unittest.TestCase): def test_load(self): @@ -20,56 +22,61 @@ class TestFormLoad(unittest.TestCase): self.assertTrue(meta.get("__calendar_js")) def test_fieldlevel_permissions_in_load(self): + blog = frappe.get_doc({ + "doctype": "Blog Post", + "blog_category": "_Test Blog Category 1", + "blog_intro": "Test Blog Intro", + "blogger": "_Test Blogger 1", + "content": "Test Blog Content", + "doctype": "Blog Post", + "title": "_Test Blog Post {}".format(frappe.utils.now()), + "published": 0 + }) + + blog.insert() + user = frappe.get_doc('User', 'test@example.com') - user.remove_roles('Website Manager') + user.remove_roles('Website Manager', 'System Manager') user.add_roles('Blogger') - reset('Blog Post') - frappe.db.set_value('DocField', { - 'fieldname': 'published', - 'parent': 'Blog Post' - }, 'permlevel', 1) - - update('Blog Post', 'Website Manager', 0, 'permlevel', 1) + make_property_setter('Blog Post', 'published', 'permlevel', 1, 'Int') + add('Blog Post', 'Website Manager', 1) + update('Blog Post', 'Website Manager', 1, 'write', 1) frappe.set_user(user.name) - # print frappe.as_json(get_valid_perms('Blog Post')) + blog_doc = get_blog(blog.name) - frappe.clear_cache(doctype='Blog Post') + self.assertEqual(blog_doc.name, blog.name) + # since published field has higher permlevel + self.assertEqual(blog_doc.published, None) - blog = frappe.db.get_value('Blog Post', {'title': '_Test Blog Post'}) - - getdoc('Blog Post', blog) - - checked = False - - for doc in frappe.response.docs: - if doc.name == blog: - self.assertEqual(doc.published, None) - checked = True - - self.assertTrue(checked, True) - - frappe.db.set_value('DocField', { - 'fieldname': 'published', - 'parent': 'Blog Post' - }, 'permlevel', 0) - - reset('Blog Post') - - frappe.clear_cache(doctype='Blog Post') - - frappe.response.docs = [] - getdoc('Blog Post', blog) - - checked = False - - for doc in frappe.response.docs: - if doc.name == blog: - self.assertEqual(doc.published, 1) - checked = True - - self.assertTrue(checked, True) + # this will be ignored because user does not + # have write access on `published` field (or on permlevel 1 fields) + blog_doc.published = 1 + blog_doc.save() + # since published field has higher permlevel + self.assertEqual(blog_doc.published, 0) frappe.set_user('Administrator') + user.add_roles('Website Manager') + frappe.cache().hdel("roles", user.name) + frappe.set_user(user.name) + + doc = frappe.get_doc('Blog Post', blog.name) + doc.published = 1 + doc.save() + + blog_doc = get_blog(blog.name) + # now user should be allowed to read field with higher permlevel + # (after adding Website Manager role) + self.assertEqual(blog_doc.published, 1) + + frappe.set_user('Administrator') + + +def get_blog(blog_name): + getdoc('Blog Post', blog_name) + doc = frappe.response.docs[0] + frappe.response.docs = [] + return doc \ No newline at end of file From 5cbe2170e83bca2c18c4c07c9d954da8221f31bd Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 21 Jan 2020 15:55:19 +0530 Subject: [PATCH 4/6] test: Reset Blog Post doc perms --- frappe/tests/test_form_load.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/test_form_load.py b/frappe/tests/test_form_load.py index e934aed222..34df5bc882 100644 --- a/frappe/tests/test_form_load.py +++ b/frappe/tests/test_form_load.py @@ -40,6 +40,7 @@ class TestFormLoad(unittest.TestCase): user.add_roles('Blogger') make_property_setter('Blog Post', 'published', 'permlevel', 1, 'Int') + reset('Blog Post') add('Blog Post', 'Website Manager', 1) update('Blog Post', 'Website Manager', 1, 'write', 1) @@ -60,7 +61,6 @@ class TestFormLoad(unittest.TestCase): frappe.set_user('Administrator') user.add_roles('Website Manager') - frappe.cache().hdel("roles", user.name) frappe.set_user(user.name) doc = frappe.get_doc('Blog Post', blog.name) From 02453fa7e070a4787b70441cb73a34580830c4da Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 21 Jan 2020 19:03:40 +0530 Subject: [PATCH 5/6] style: Fix codacy --- frappe/tests/test_form_load.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/tests/test_form_load.py b/frappe/tests/test_form_load.py index 34df5bc882..5c9abd04f6 100644 --- a/frappe/tests/test_form_load.py +++ b/frappe/tests/test_form_load.py @@ -28,7 +28,6 @@ class TestFormLoad(unittest.TestCase): "blog_intro": "Test Blog Intro", "blogger": "_Test Blogger 1", "content": "Test Blog Content", - "doctype": "Blog Post", "title": "_Test Blog Post {}".format(frappe.utils.now()), "published": 0 }) @@ -76,7 +75,7 @@ class TestFormLoad(unittest.TestCase): def get_blog(blog_name): + frappe.response.docs = [] getdoc('Blog Post', blog_name) doc = frappe.response.docs[0] - frappe.response.docs = [] return doc \ No newline at end of file From 5eaaaf03dbdfde95b376098f9202fc5d4a97280b Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 22 Jan 2020 11:03:47 +0530 Subject: [PATCH 6/6] test: Fix user permission test --- frappe/tests/test_form_load.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frappe/tests/test_form_load.py b/frappe/tests/test_form_load.py index 5c9abd04f6..fc53e21fef 100644 --- a/frappe/tests/test_form_load.py +++ b/frappe/tests/test_form_load.py @@ -35,7 +35,9 @@ class TestFormLoad(unittest.TestCase): blog.insert() user = frappe.get_doc('User', 'test@example.com') - user.remove_roles('Website Manager', 'System Manager') + + user_roles = frappe.get_roles() + user.remove_roles(*user_roles) user.add_roles('Blogger') make_property_setter('Blog Post', 'published', 'permlevel', 1, 'Int') @@ -73,6 +75,9 @@ class TestFormLoad(unittest.TestCase): frappe.set_user('Administrator') + # reset user roles + user.remove_roles('Blogger', 'Website Manager') + user.add_roles(*user_roles) def get_blog(blog_name): frappe.response.docs = []