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 d8ecdffb1e..b767fd4aef 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -5,8 +5,12 @@ from __future__ import unicode_literals import frappe import unittest +from frappe.model.db_query import DatabaseQuery +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') class TestToDo(unittest.TestCase): def test_delete(self): @@ -47,6 +51,62 @@ class TestToDo(unittest.TestCase): self.assertEqual(todo.assigned_by_full_name, frappe.db.get_value('User', todo.assigned_by, 'full_name')) + def test_todo_list_access(self): + create_new_todo('Test1', 'testperm@example.com') + + frappe.set_user('test4@example.com') + create_new_todo('Test2', 'test4@example.com') + test_user_data = DatabaseQuery('ToDo').execute() + + frappe.set_user('testperm@example.com') + system_manager_data = DatabaseQuery('ToDo').execute() + + self.assertNotEqual(test_user_data, system_manager_data) + + frappe.set_user('Administrator') + frappe.db.rollback() + + def test_doc_read_access(self): + #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 test4 + todo2 = create_new_todo('Test2', 'test4@example.com') + + frappe.set_user('test4@example.com') + #owner and assigned_by is test4 + 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 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') + + 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.set_user('Administrator') + test_user.remove_roles('Blogger') + reset_perms('ToDo') + clear_permissions_cache('ToDo') + frappe.db.rollback() + def test_fetch_if_empty(self): frappe.db.sql('delete from tabToDo') @@ -74,3 +134,11 @@ def test_fetch_if_empty(self): self.assertEqual(todo.assigned_by_full_name, frappe.db.get_value('User', todo.assigned_by, 'full_name')) + +def create_new_todo(description, assigned_by): + todo = { + 'doctype': 'ToDo', + 'description': description, + 'assigned_by': assigned_by + } + return frappe.get_doc(todo).insert() diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index 804174b56b..a766375fde 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -85,21 +85,30 @@ 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 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 - if "System Manager" in frappe.get_roles(user): + todo_roles = frappe.permissions.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 "System Manager" in frappe.get_roles(user): +def has_permission(doc, ptype="read", user=None): + user = user or frappe.session.user + todo_roles = frappe.permissions.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)): return True else: return doc.owner==user or doc.assigned_by==user 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 = {