From 1d628d0a13999c9c444fa9f5d2e2014e76d73237 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Fri, 12 Feb 2021 21:38:29 +0530 Subject: [PATCH 01/16] fix: All todos not accessable to all users --- frappe/__init__.py | 5 +++++ frappe/desk/doctype/todo/todo.py | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 9b3ffc4662..e366c7dc67 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -459,6 +459,11 @@ def get_roles(username=None): import frappe.permissions return frappe.permissions.get_roles(username or local.session.user) +def get_doctype_roles(doctype): + """Returns roles of doctype.""" + meta = get_meta(doctype) + return [d.role for d in meta.get("permissions")] + def get_request_header(key, default=None): """Return HTTP request header. diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index 804174b56b..ab45baca9a 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -85,14 +85,14 @@ class ToDo(Document): else: raise -# NOTE: todo is viewable if either owner or assigned_to or System Manager in roles +# NOTE: todo is viewable if either owner or assigned_to or any Todo doctype role in user's roles def on_doctype_update(): frappe.db.add_index("ToDo", ["reference_type", "reference_name"]) def get_permission_query_conditions(user): if not user: user = frappe.session.user - if "System Manager" in frappe.get_roles(user): + if any(check in frappe.get_doctype_roles('Todo') for check in frappe.get_roles(user)): return None else: return """(`tabToDo`.owner = {user} or `tabToDo`.assigned_by = {user})"""\ From adf76a7ec931906bac8a7c6c6cf135ebb6aba79b Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 15 Feb 2021 11:53:20 +0530 Subject: [PATCH 02/16] fix: Also updated has_permission --- frappe/desk/doctype/todo/todo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index ab45baca9a..cd61b3203d 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -99,7 +99,7 @@ def get_permission_query_conditions(user): .format(user=frappe.db.escape(user)) def has_permission(doc, user): - if "System Manager" in frappe.get_roles(user): + if any(check in frappe.get_doctype_roles('Todo') for check in frappe.get_roles(user)): return True else: return doc.owner==user or doc.assigned_by==user From 0f1eb5c6b211855246749fc0c9ede2ed5d197d7b Mon Sep 17 00:00:00 2001 From: shariquerik Date: Thu, 18 Feb 2021 10:26:41 +0530 Subject: [PATCH 03/16] fix: added test case --- frappe/desk/doctype/todo/test_todo.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index d8ecdffb1e..c0d47cac0f 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -5,8 +5,10 @@ from __future__ import unicode_literals import frappe import unittest +from frappe.model.db_query import DatabaseQuery # test_records = frappe.get_test_records('ToDo') +test_user_records = frappe.get_test_records('User') class TestToDo(unittest.TestCase): def test_delete(self): @@ -47,6 +49,19 @@ class TestToDo(unittest.TestCase): self.assertEqual(todo.assigned_by_full_name, frappe.db.get_value('User', todo.assigned_by, 'full_name')) + def test_access(self): + insert_test_records() + + frappe.set_user('testperm@example.com') + test_user_data = DatabaseQuery('ToDo').execute() + + frappe.set_user('Administrator') + admin_data = DatabaseQuery('ToDo').execute() + + self.assertEqual(test_user_data, admin_data) + + frappe.db.rollback() + def test_fetch_if_empty(self): frappe.db.sql('delete from tabToDo') @@ -74,3 +89,15 @@ def test_fetch_if_empty(self): self.assertEqual(todo.assigned_by_full_name, frappe.db.get_value('User', todo.assigned_by, 'full_name')) + +def insert_test_records(): + create_new_todo('Test1', 'Administrator') + create_new_todo('Test2', 'testperm@example.com') + +def create_new_todo(description, assigned_by): + todo = { + 'doctype': 'ToDo', + 'description': description, + 'assigned_by': assigned_by + } + frappe.get_doc(todo).insert() From b7833ebb923685299bfdc75c1d7b94442c71bcb7 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Fri, 19 Feb 2021 12:24:01 +0530 Subject: [PATCH 04/16] fix: updated access logic and test case --- frappe/desk/doctype/todo/test_todo.py | 2 +- frappe/desk/doctype/todo/todo.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index c0d47cac0f..dc86b3424c 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -58,7 +58,7 @@ class TestToDo(unittest.TestCase): frappe.set_user('Administrator') admin_data = DatabaseQuery('ToDo').execute() - self.assertEqual(test_user_data, admin_data) + self.assertNotEqual(test_user_data, admin_data) frappe.db.rollback() diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index cd61b3203d..d62b2d6cef 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -92,14 +92,20 @@ def on_doctype_update(): def get_permission_query_conditions(user): if not user: user = frappe.session.user - if any(check in frappe.get_doctype_roles('Todo') for check in frappe.get_roles(user)): + todo_roles = frappe.get_doctype_roles('Todo') + if 'All' in todo_roles: todo_roles.remove('All') + + if any(check in todo_roles for check in frappe.get_roles(user)): return None else: return """(`tabToDo`.owner = {user} or `tabToDo`.assigned_by = {user})"""\ .format(user=frappe.db.escape(user)) def has_permission(doc, user): - if any(check in frappe.get_doctype_roles('Todo') for check in frappe.get_roles(user)): + todo_roles = frappe.get_doctype_roles('Todo') + if 'All' in todo_roles: todo_roles.remove('All') + + if any(check in todo_roles for check in frappe.get_roles(user)): return True else: return doc.owner==user or doc.assigned_by==user From e5c1682856e82cb5547a3781ad77c16da4d710ad Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Mon, 8 Mar 2021 13:28:53 +0530 Subject: [PATCH 05/16] fix: Apply suggestions from code review Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/__init__.py | 4 ++-- frappe/desk/doctype/todo/todo.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index e366c7dc67..8d53143b8c 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -459,8 +459,8 @@ def get_roles(username=None): import frappe.permissions return frappe.permissions.get_roles(username or local.session.user) -def get_doctype_roles(doctype): - """Returns roles of doctype.""" +def get_doctype_roles(doctype, access_type="read"): + """Returns a list of roles that are allowed to access passed doctype.""" meta = get_meta(doctype) return [d.role for d in meta.get("permissions")] diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index d62b2d6cef..d54df74819 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -85,14 +85,14 @@ class ToDo(Document): else: raise -# NOTE: todo is viewable if either owner or assigned_to or any Todo doctype role in user's roles +# NOTE: todo is viewable if a user is an owner, or set as assigned_to value, or has any role that is allowed to access ToDo doctype. def on_doctype_update(): frappe.db.add_index("ToDo", ["reference_type", "reference_name"]) def get_permission_query_conditions(user): if not user: user = frappe.session.user - todo_roles = frappe.get_doctype_roles('Todo') + todo_roles = frappe.get_doctype_roles('ToDo') if 'All' in todo_roles: todo_roles.remove('All') if any(check in todo_roles for check in frappe.get_roles(user)): From 4caac4f90d501ee50704f43dbf8633ccd1bccc14 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 8 Mar 2021 13:34:06 +0530 Subject: [PATCH 06/16] fix: updated get_doctype_roles logic based on suggestion --- frappe/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 8d53143b8c..2ab788d5fe 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -462,7 +462,7 @@ def get_roles(username=None): def get_doctype_roles(doctype, access_type="read"): """Returns a list of roles that are allowed to access passed doctype.""" meta = get_meta(doctype) - return [d.role for d in meta.get("permissions")] + return [d.role for d in meta.get("permissions") if d.get(access_type)] def get_request_header(key, default=None): """Return HTTP request header. From c8ed2a1d660e20eae7086289ea87e41d48bc2cb7 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Wed, 10 Mar 2021 12:08:37 +0530 Subject: [PATCH 07/16] fix: Typo --- frappe/desk/doctype/todo/todo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index d54df74819..4c100cdccb 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -102,7 +102,7 @@ def get_permission_query_conditions(user): .format(user=frappe.db.escape(user)) def has_permission(doc, user): - todo_roles = frappe.get_doctype_roles('Todo') + todo_roles = frappe.get_doctype_roles('ToDo') if 'All' in todo_roles: todo_roles.remove('All') if any(check in todo_roles for check in frappe.get_roles(user)): From 08a58dc0c7ddac08cb39ea3626b0724a5a11736f Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Wed, 10 Mar 2021 19:16:25 +0530 Subject: [PATCH 08/16] fix: Apply suggestions from code review Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/desk/doctype/todo/todo.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index 4c100cdccb..b2c46cf2fb 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -101,8 +101,9 @@ def get_permission_query_conditions(user): return """(`tabToDo`.owner = {user} or `tabToDo`.assigned_by = {user})"""\ .format(user=frappe.db.escape(user)) -def has_permission(doc, user): - todo_roles = frappe.get_doctype_roles('ToDo') +def has_permission(doc, ptype="read", user=None): + user = user or frappe.session.user + todo_roles = frappe.get_doctype_roles('ToDo', ptype) if 'All' in todo_roles: todo_roles.remove('All') if any(check in todo_roles for check in frappe.get_roles(user)): From 5e1a3a5d104d0177733e00118009ec7fe81217c7 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 15 Mar 2021 17:05:02 +0530 Subject: [PATCH 09/16] fix: updated test case --- frappe/core/doctype/user/test_records.json | 7 +++++++ frappe/desk/doctype/todo/test_todo.py | 21 ++++++++++++++------- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/frappe/core/doctype/user/test_records.json b/frappe/core/doctype/user/test_records.json index 93fcca5517..f9033d4660 100644 --- a/frappe/core/doctype/user/test_records.json +++ b/frappe/core/doctype/user/test_records.json @@ -38,6 +38,13 @@ "new_password": "Eastern_43A1W", "enabled": 1 }, + { + "doctype": "User", + "email": "test4@example.com", + "first_name": "_Test4", + "new_password": "Eastern_43A1W", + "enabled": 1 + }, { "doctype": "User", "email": "testperm@example.com", diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index dc86b3424c..685456392b 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -50,14 +50,25 @@ class TestToDo(unittest.TestCase): frappe.db.get_value('User', todo.assigned_by, 'full_name')) def test_access(self): - insert_test_records() + todo1 = create_new_todo('Test1', 'Administrator') + todo2 = create_new_todo('Test2', 'test4@example.com') - frappe.set_user('testperm@example.com') + frappe.set_user('test4@example.com') test_user_data = DatabaseQuery('ToDo').execute() + self.assertFalse(todo1.has_permission("read")) + self.assertFalse(todo1.has_permission("write")) + self.assertTrue(todo2.has_permission("read")) + self.assertTrue(todo2.has_permission("write")) + frappe.set_user('Administrator') admin_data = DatabaseQuery('ToDo').execute() + self.assertTrue(todo1.has_permission("read")) + self.assertTrue(todo1.has_permission("write")) + self.assertTrue(todo2.has_permission("read")) + self.assertTrue(todo2.has_permission("write")) + self.assertNotEqual(test_user_data, admin_data) frappe.db.rollback() @@ -90,14 +101,10 @@ def test_fetch_if_empty(self): self.assertEqual(todo.assigned_by_full_name, frappe.db.get_value('User', todo.assigned_by, 'full_name')) -def insert_test_records(): - create_new_todo('Test1', 'Administrator') - create_new_todo('Test2', 'testperm@example.com') - def create_new_todo(description, assigned_by): todo = { 'doctype': 'ToDo', 'description': description, 'assigned_by': assigned_by } - frappe.get_doc(todo).insert() + return frappe.get_doc(todo).insert() From a2e0a0cd7ea488fef81aca6584488d9d72051842 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 16 Mar 2021 15:25:23 +0530 Subject: [PATCH 10/16] fix: added test case to check read access to ToDo documents via role permissions --- frappe/desk/doctype/todo/test_todo.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index 685456392b..89c4909ead 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -6,6 +6,7 @@ from __future__ import unicode_literals import frappe import unittest from frappe.model.db_query import DatabaseQuery +from frappe.permissions import get_doc_permissions # test_records = frappe.get_test_records('ToDo') test_user_records = frappe.get_test_records('User') @@ -73,6 +74,30 @@ class TestToDo(unittest.TestCase): frappe.db.rollback() + def test_doc_read_access(self): + todo1 = create_new_todo('Test1', 'Administrator') + todo2 = create_new_todo('Test2', 'test4@example.com') + + # user without role permission to read ToDo's + frappe.set_user('test4@example.com') + user_todo1_permission = get_doc_permissions(todo1) + user_todo2_permission = get_doc_permissions(todo2) + self.assertFalse(user_todo1_permission.get("read")) + self.assertTrue(user_todo2_permission.get("read")) + + # user with role permission to read ToDo's + frappe.set_user('test@example.com') + user_todo1_permission = get_doc_permissions(todo1) + user_todo2_permission = get_doc_permissions(todo2) + self.assertTrue(user_todo1_permission.get("read")) + self.assertTrue(user_todo2_permission.get("read")) + + frappe.set_user('Administrator') + admin_todo1_permission = get_doc_permissions(todo1) + admin_todo2_permission = get_doc_permissions(todo2) + self.assertTrue(admin_todo1_permission.get("read")) + self.assertTrue(admin_todo2_permission.get("read")) + def test_fetch_if_empty(self): frappe.db.sql('delete from tabToDo') From 3e61163c804337fa2de3e985710a2852cfd5190f Mon Sep 17 00:00:00 2001 From: shariquerik Date: Fri, 19 Mar 2021 19:25:25 +0530 Subject: [PATCH 11/16] fix: updated test cases --- frappe/desk/doctype/todo/test_todo.py | 67 ++++++++++++++------------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index 89c4909ead..bb70a5ac23 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -50,53 +50,56 @@ class TestToDo(unittest.TestCase): self.assertEqual(todo.assigned_by_full_name, frappe.db.get_value('User', todo.assigned_by, 'full_name')) - def test_access(self): - todo1 = create_new_todo('Test1', 'Administrator') - todo2 = create_new_todo('Test2', 'test4@example.com') + def test_todo_list_access(self): + todo1 = create_new_todo('Test1', 'testperm@example.com') frappe.set_user('test4@example.com') + todo2 = create_new_todo('Test2', 'test4@example.com') test_user_data = DatabaseQuery('ToDo').execute() - self.assertFalse(todo1.has_permission("read")) - self.assertFalse(todo1.has_permission("write")) - self.assertTrue(todo2.has_permission("read")) - self.assertTrue(todo2.has_permission("write")) + frappe.set_user('testperm@example.com') + system_manager_data = DatabaseQuery('ToDo').execute() - frappe.set_user('Administrator') - admin_data = DatabaseQuery('ToDo').execute() - - self.assertTrue(todo1.has_permission("read")) - self.assertTrue(todo1.has_permission("write")) - self.assertTrue(todo2.has_permission("read")) - self.assertTrue(todo2.has_permission("write")) - - self.assertNotEqual(test_user_data, admin_data) + self.assertNotEqual(test_user_data, system_manager_data) frappe.db.rollback() def test_doc_read_access(self): - todo1 = create_new_todo('Test1', 'Administrator') + #owner and assigned_by is testperm + todo1 = create_new_todo('Test1', 'testperm@example.com') + test_user = frappe.get_doc('User', 'test4@example.com') + + #owner is testperm, but assigned_by is test1 todo2 = create_new_todo('Test2', 'test4@example.com') - # user without role permission to read ToDo's frappe.set_user('test4@example.com') - user_todo1_permission = get_doc_permissions(todo1) - user_todo2_permission = get_doc_permissions(todo2) - self.assertFalse(user_todo1_permission.get("read")) - self.assertTrue(user_todo2_permission.get("read")) + #owner and assigned_by is test1 + todo3 = create_new_todo('Test3', 'test4@example.com') + + # user without any role to read or write todo document + self.assertFalse(todo1.has_permission("read")) + self.assertFalse(todo1.has_permission("write")) - # user with role permission to read ToDo's - frappe.set_user('test@example.com') - user_todo1_permission = get_doc_permissions(todo1) - user_todo2_permission = get_doc_permissions(todo2) - self.assertTrue(user_todo1_permission.get("read")) - self.assertTrue(user_todo2_permission.get("read")) + # user without any role but he/she is assigned_by of that todo document + self.assertTrue(todo2.has_permission("read")) + self.assertTrue(todo2.has_permission("write")) + + # user is the owner and assigned_by of the todo document + self.assertTrue(todo3.has_permission("read")) + self.assertTrue(todo3.has_permission("write")) frappe.set_user('Administrator') - admin_todo1_permission = get_doc_permissions(todo1) - admin_todo2_permission = get_doc_permissions(todo2) - self.assertTrue(admin_todo1_permission.get("read")) - self.assertTrue(admin_todo2_permission.get("read")) + + test_user.add_roles('Blogger') + add_permission('ToDo', 'Blogger') + + frappe.set_user('test4@example.com') + + # user with only read access to todo document, not an owner or assigned_by + self.assertTrue(todo1.has_permission("read")) + self.assertFalse(todo1.has_permission("write")) + + frappe.db.rollback() def test_fetch_if_empty(self): frappe.db.sql('delete from tabToDo') From b16e45040fe8ef4652eaf1febec82fece44bcc1a Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Fri, 19 Mar 2021 19:29:54 +0530 Subject: [PATCH 12/16] fix: minor fix --- frappe/desk/doctype/todo/test_todo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index bb70a5ac23..a1a57a6caf 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -6,7 +6,7 @@ from __future__ import unicode_literals import frappe import unittest from frappe.model.db_query import DatabaseQuery -from frappe.permissions import get_doc_permissions +from frappe.permissions import get_doc_permissions, add_permission # test_records = frappe.get_test_records('ToDo') test_user_records = frappe.get_test_records('User') From 3d379990c0c6126f793be7c006752a3ca8cc640f Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 23 Mar 2021 10:12:49 +0530 Subject: [PATCH 13/16] fix: sider + updated test case --- frappe/desk/doctype/todo/test_todo.py | 6 ++++-- frappe/desk/doctype/todo/todo.py | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index a1a57a6caf..6547d12df4 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -51,10 +51,10 @@ class TestToDo(unittest.TestCase): frappe.db.get_value('User', todo.assigned_by, 'full_name')) def test_todo_list_access(self): - todo1 = create_new_todo('Test1', 'testperm@example.com') + create_new_todo('Test1', 'testperm@example.com') frappe.set_user('test4@example.com') - todo2 = create_new_todo('Test2', 'test4@example.com') + create_new_todo('Test2', 'test4@example.com') test_user_data = DatabaseQuery('ToDo').execute() frappe.set_user('testperm@example.com') @@ -62,6 +62,7 @@ class TestToDo(unittest.TestCase): self.assertNotEqual(test_user_data, system_manager_data) + frappe.set_user('Administrator') frappe.db.rollback() def test_doc_read_access(self): @@ -99,6 +100,7 @@ class TestToDo(unittest.TestCase): self.assertTrue(todo1.has_permission("read")) self.assertFalse(todo1.has_permission("write")) + frappe.set_user('Administrator') frappe.db.rollback() def test_fetch_if_empty(self): diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index b2c46cf2fb..5d4fa75b65 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -93,7 +93,8 @@ def get_permission_query_conditions(user): if not user: user = frappe.session.user todo_roles = frappe.get_doctype_roles('ToDo') - if 'All' in todo_roles: todo_roles.remove('All') + if 'All' in todo_roles: + todo_roles.remove('All') if any(check in todo_roles for check in frappe.get_roles(user)): return None @@ -104,7 +105,8 @@ def get_permission_query_conditions(user): def has_permission(doc, ptype="read", user=None): user = user or frappe.session.user todo_roles = frappe.get_doctype_roles('ToDo', ptype) - if 'All' in todo_roles: todo_roles.remove('All') + if 'All' in todo_roles: + todo_roles.remove('All') if any(check in todo_roles for check in frappe.get_roles(user)): return True From af810e8b6cc002072f4062a462d47038deab2608 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 23 Mar 2021 11:43:58 +0530 Subject: [PATCH 14/16] fix: sider + updated test case --- frappe/desk/doctype/todo/test_todo.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index 6547d12df4..4e5a4e576b 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -6,7 +6,8 @@ from __future__ import unicode_literals import frappe import unittest from frappe.model.db_query import DatabaseQuery -from frappe.permissions import get_doc_permissions, add_permission +from frappe.permissions import add_permission, reset_perms +from frappe.core.doctype.doctype.doctype import clear_permissions_cache # test_records = frappe.get_test_records('ToDo') test_user_records = frappe.get_test_records('User') @@ -62,7 +63,6 @@ class TestToDo(unittest.TestCase): self.assertNotEqual(test_user_data, system_manager_data) - frappe.set_user('Administrator') frappe.db.rollback() def test_doc_read_access(self): @@ -70,11 +70,11 @@ class TestToDo(unittest.TestCase): todo1 = create_new_todo('Test1', 'testperm@example.com') test_user = frappe.get_doc('User', 'test4@example.com') - #owner is testperm, but assigned_by is test1 + #owner is testperm, but assigned_by is test4 todo2 = create_new_todo('Test2', 'test4@example.com') frappe.set_user('test4@example.com') - #owner and assigned_by is test1 + #owner and assigned_by is test4 todo3 = create_new_todo('Test3', 'test4@example.com') # user without any role to read or write todo document @@ -101,6 +101,9 @@ class TestToDo(unittest.TestCase): self.assertFalse(todo1.has_permission("write")) frappe.set_user('Administrator') + test_user.remove_roles('Blogger') + reset_perms('ToDo') + clear_permissions_cache('ToDo') frappe.db.rollback() def test_fetch_if_empty(self): From 7aa062873194f4e4372e100aafbfd12a7eef4625 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Wed, 24 Mar 2021 16:41:01 +0530 Subject: [PATCH 15/16] refactor: relocated get_doctype_roles function --- frappe/__init__.py | 5 ----- frappe/desk/doctype/todo/todo.py | 4 ++-- frappe/permissions.py | 5 +++++ 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 853a8d34f7..871d1b9e92 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -466,11 +466,6 @@ def get_roles(username=None): import frappe.permissions return frappe.permissions.get_roles(username or local.session.user) -def get_doctype_roles(doctype, access_type="read"): - """Returns a list of roles that are allowed to access passed doctype.""" - meta = get_meta(doctype) - return [d.role for d in meta.get("permissions") if d.get(access_type)] - def get_request_header(key, default=None): """Return HTTP request header. diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index 5d4fa75b65..a766375fde 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -92,7 +92,7 @@ def on_doctype_update(): def get_permission_query_conditions(user): if not user: user = frappe.session.user - todo_roles = frappe.get_doctype_roles('ToDo') + todo_roles = frappe.permissions.get_doctype_roles('ToDo') if 'All' in todo_roles: todo_roles.remove('All') @@ -104,7 +104,7 @@ def get_permission_query_conditions(user): def has_permission(doc, ptype="read", user=None): user = user or frappe.session.user - todo_roles = frappe.get_doctype_roles('ToDo', ptype) + todo_roles = frappe.permissions.get_doctype_roles('ToDo', ptype) if 'All' in todo_roles: todo_roles.remove('All') diff --git a/frappe/permissions.py b/frappe/permissions.py index abb1f6653a..e597402c38 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -362,6 +362,11 @@ def get_roles(user=None, with_standard=True): return roles +def get_doctype_roles(doctype, access_type="read"): + """Returns a list of roles that are allowed to access passed doctype.""" + meta = frappe.get_meta(doctype) + return [d.role for d in meta.get("permissions") if d.get(access_type)] + def get_perms_for(roles, perm_doctype='DocPerm'): '''Get perms for given roles''' filters = { From 700b91834784ea462581588567a4523157bc79d6 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Thu, 25 Mar 2021 13:41:46 +0530 Subject: [PATCH 16/16] test: test case fix --- frappe/desk/doctype/todo/test_todo.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index 4e5a4e576b..b767fd4aef 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -63,6 +63,7 @@ class TestToDo(unittest.TestCase): self.assertNotEqual(test_user_data, system_manager_data) + frappe.set_user('Administrator') frappe.db.rollback() def test_doc_read_access(self):