From 30341d5d53f25407b1d6d06442e4a09a5ef291a2 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 24 Feb 2021 19:54:01 +0530 Subject: [PATCH 1/5] fix: frappe.new_doc won't set default in presence of User Permissions --- frappe/model/create_new.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/model/create_new.py b/frappe/model/create_new.py index e0087a9e40..b1210cb189 100644 --- a/frappe/model/create_new.py +++ b/frappe/model/create_new.py @@ -61,6 +61,9 @@ def set_user_and_static_default_values(doc): doctype_user_permissions = user_permissions.get(df.options, []) # Allowed records for the reference doctype (link field) along with default doc allowed_records, default_doc = filter_allowed_docs_for_doctype(doctype_user_permissions, df.parent, with_default_doc=True) + allowed_records += filter_allowed_docs_for_doctype(doctype_user_permissions, df.options, with_default_doc=False) + + allowed_records = list(set(allowed_records)) user_default_value = get_user_default_value(df, defaults, doctype_user_permissions, allowed_records, default_doc) if user_default_value is not None: From c5cc262ead3465ef7ebe1b500f6cdf6b678cef72 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 24 Feb 2021 21:08:33 +0530 Subject: [PATCH 2/5] fix: No user Permissions against new doc was not considered - the new doc could not have any User Permissions on it - No restrictions, and hence default must be set --- frappe/model/create_new.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/frappe/model/create_new.py b/frappe/model/create_new.py index b1210cb189..c20c7b289d 100644 --- a/frappe/model/create_new.py +++ b/frappe/model/create_new.py @@ -60,10 +60,8 @@ def set_user_and_static_default_values(doc): # user permissions for link options doctype_user_permissions = user_permissions.get(df.options, []) # Allowed records for the reference doctype (link field) along with default doc - allowed_records, default_doc = filter_allowed_docs_for_doctype(doctype_user_permissions, df.parent, with_default_doc=True) - allowed_records += filter_allowed_docs_for_doctype(doctype_user_permissions, df.options, with_default_doc=False) - - allowed_records = list(set(allowed_records)) + allowed_records, default_doc = filter_allowed_docs_for_doctype(doctype_user_permissions, + df.parent, with_default_doc=True) user_default_value = get_user_default_value(df, defaults, doctype_user_permissions, allowed_records, default_doc) if user_default_value is not None: @@ -86,11 +84,17 @@ def get_user_default_value(df, defaults, doctype_user_permissions, allowed_recor # 2 - Look in user defaults user_default = defaults.get(df.fieldname) - is_allowed_user_default = user_default and (not user_permissions_exist(df, doctype_user_permissions) - or user_default in allowed_records) + + allowed_by_user_permission = True + if user_permissions_exist(df, doctype_user_permissions) and allowed_records: + # If allowed records is empty, that means there is *no* User Permission that + # is applicable to this new doctype. + + # If not, check if this field value is allowed via User Permissions. + allowed_by_user_permission = user_default in allowed_records # is this user default also allowed as per user permissions? - if is_allowed_user_default: + if user_default and allowed_by_user_permission: return user_default def get_static_default_value(df, doctype_user_permissions, allowed_records): From 46ff9c6102774521ff4bff032745f350c1428728 Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 12 Mar 2021 12:40:22 +0530 Subject: [PATCH 3/5] fix: Fixed for field default values too + Added tests - made fix for field default values too along with user default values - Commonified validation of default value via User Perm - Added tests for both cases --- .../user_permission/test_user_permission.py | 83 ++++++++++++++++++- frappe/model/create_new.py | 26 ++++-- 2 files changed, 99 insertions(+), 10 deletions(-) diff --git a/frappe/core/doctype/user_permission/test_user_permission.py b/frappe/core/doctype/user_permission/test_user_permission.py index 7e0b4a49c6..a1eb34d1e8 100644 --- a/frappe/core/doctype/user_permission/test_user_permission.py +++ b/frappe/core/doctype/user_permission/test_user_permission.py @@ -2,7 +2,7 @@ # Copyright (c) 2017, Frappe Technologies and Contributors # See license.txt from __future__ import unicode_literals -from frappe.core.doctype.user_permission.user_permission import add_user_permissions +from frappe.core.doctype.user_permission.user_permission import add_user_permissions, remove_applicable from frappe.permissions import has_user_permission import frappe @@ -17,6 +17,7 @@ class TestUserPermission(unittest.TestCase): 'nested_doc_user@example.com')""") frappe.delete_doc_if_exists("DocType", "Person") frappe.db.sql_ddl("DROP TABLE IF EXISTS `tabPerson`") + frappe.db.sql_ddl("DROP TABLE IF EXISTS `tabDoc A`") def test_default_user_permission_validation(self): user = create_user('test_default_permission@example.com') @@ -153,6 +154,86 @@ class TestUserPermission(unittest.TestCase): self.assertTrue(has_user_permission(frappe.get_doc("Person", parent_record.name), user.name)) self.assertFalse(has_user_permission(frappe.get_doc("Person", child_record.name), user.name)) + def test_user_perm_on_new_doc_with_field_default(self): + """Test User Perm impact on frappe.new_doc. with *field* default value""" + from frappe.core.doctype.doctype.test_doctype import new_doctype + + frappe.set_user('Administrator') + user = create_user("new_doc_test@example.com", "Blogger") + + # make a doctype "Doc A" with 'doctype' link field and default value ToDo + if not frappe.db.exists("DocType", "Doc A"): + doc = new_doctype("Doc A", + fields=[ + { + "label": "DocType", + "fieldname": "doc", + "fieldtype": "Link", + "options": "DocType", + "default": "ToDo" + } + ], unique=0) + doc.insert() + + # make User Perm on DocType 'ToDo' in Assignment Rule (unrelated doctype) + add_user_permissions(get_params(user, "DocType", "ToDo", applicable=["Assignment Rule"])) + frappe.set_user("new_doc_test@example.com") + + new_doc = frappe.new_doc("Doc A") + + # User perm is created on ToDo but for doctype Assignment Rule only + # it should not have impact on Doc A + self.assertEquals(new_doc.doc, "ToDo") + + frappe.set_user('Administrator') + remove_applicable(["Assignment Rule"], "new_doc_test@example.com", "DocType", "ToDo") + + def test_user_perm_on_new_doc_with_user_default(self): + """Test User Perm impact on frappe.new_doc. with *user* default value""" + from frappe.core.doctype.session_default_settings.session_default_settings import (clear_session_defaults, + set_session_default_values) + + frappe.set_user('Administrator') + user = create_user("user_default_test@example.com", "Blogger") + + # make a doctype "Doc A" with 'doctype' link field + if not frappe.db.exists("DocType", "Doc A"): + doc = new_doctype("Doc A", + fields=[ + { + "label": "DocType", + "fieldname": "doc", + "fieldtype": "Link", + "options": "DocType", + } + ], unique=0) + doc.insert() + + # create a 'DocType' session default field + if not frappe.db.exists("Session Default", {"ref_doctype": "DocType"}): + settings = frappe.get_single('Session Default Settings') + settings.append("session_defaults", { + "ref_doctype": "DocType" + }) + settings.save() + + # make User Perm on DocType 'ToDo' in Assignment Rule (unrelated doctype) + add_user_permissions(get_params(user, "DocType", "ToDo", applicable=["Assignment Rule"])) + + # User default Doctype value is ToDo via Session Defaults + frappe.set_user("user_default_test@example.com") + set_session_default_values({"doctype": "ToDo"}) + + new_doc = frappe.new_doc("Doc A") + + # User perm is created on ToDo but for doctype Assignment Rule only + # it should not have impact on WorkFlow + self.assertEquals(new_doc.doc, "ToDo") + + frappe.set_user('Administrator') + clear_session_defaults() + remove_applicable(["Assignment Rule"], "user_default_test@example.com", "DocType", "ToDo") + def create_user(email, role="System Manager"): ''' create user with role system manager ''' if frappe.db.exists('User', email): diff --git a/frappe/model/create_new.py b/frappe/model/create_new.py index c20c7b289d..dc4fd97e4c 100644 --- a/frappe/model/create_new.py +++ b/frappe/model/create_new.py @@ -85,13 +85,8 @@ def get_user_default_value(df, defaults, doctype_user_permissions, allowed_recor # 2 - Look in user defaults user_default = defaults.get(df.fieldname) - allowed_by_user_permission = True - if user_permissions_exist(df, doctype_user_permissions) and allowed_records: - # If allowed records is empty, that means there is *no* User Permission that - # is applicable to this new doctype. - - # If not, check if this field value is allowed via User Permissions. - allowed_by_user_permission = user_default in allowed_records + allowed_by_user_permission = validate_value_via_user_permissions(df, doctype_user_permissions, + allowed_records, user_default=user_default) # is this user default also allowed as per user permissions? if user_default and allowed_by_user_permission: @@ -108,8 +103,8 @@ def get_static_default_value(df, doctype_user_permissions, allowed_records): elif not cstr(df.default).startswith(":"): # a simple default value - is_allowed_default_value = (not user_permissions_exist(df, doctype_user_permissions) - or (df.default in allowed_records)) + is_allowed_default_value = validate_value_via_user_permissions(df, doctype_user_permissions, + allowed_records) if df.fieldtype!="Link" or df.options=="User" or is_allowed_default_value: return df.default @@ -117,6 +112,19 @@ def get_static_default_value(df, doctype_user_permissions, allowed_records): elif (df.fieldtype == "Select" and df.options and df.options not in ("[Select]", "Loading...")): return df.options.split("\n")[0] +def validate_value_via_user_permissions(df, doctype_user_permissions, allowed_records, user_default=None): + is_valid = True + # If User Permission exists and allowed records is empty, + # that means there are User Perms, but none applicable to this new doctype. + + if user_permissions_exist(df, doctype_user_permissions) and allowed_records: + # If allowed records is not empty, + # check if this field value is allowed via User Permissions applied to this doctype. + value = user_default if user_default else df.default + is_valid = value in allowed_records + + return is_valid + def set_dynamic_default_values(doc, parent_doc, parentfield): # these values should not be cached user_permissions = get_user_permissions() From 3564eadc61e31cf4055571278f465fa8d9d4398e Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 12 Mar 2021 12:57:27 +0530 Subject: [PATCH 4/5] fix: Sider (make reused import common) --- frappe/core/doctype/user_permission/test_user_permission.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/core/doctype/user_permission/test_user_permission.py b/frappe/core/doctype/user_permission/test_user_permission.py index a1eb34d1e8..1740d1b1e1 100644 --- a/frappe/core/doctype/user_permission/test_user_permission.py +++ b/frappe/core/doctype/user_permission/test_user_permission.py @@ -4,6 +4,7 @@ from __future__ import unicode_literals from frappe.core.doctype.user_permission.user_permission import add_user_permissions, remove_applicable from frappe.permissions import has_user_permission +from frappe.core.doctype.doctype.test_doctype import new_doctype import frappe import unittest @@ -156,8 +157,6 @@ class TestUserPermission(unittest.TestCase): def test_user_perm_on_new_doc_with_field_default(self): """Test User Perm impact on frappe.new_doc. with *field* default value""" - from frappe.core.doctype.doctype.test_doctype import new_doctype - frappe.set_user('Administrator') user = create_user("new_doc_test@example.com", "Blogger") From 7e2b671fa368aea12684044baf5a38d79f6a5299 Mon Sep 17 00:00:00 2001 From: Marica Date: Tue, 16 Mar 2021 11:39:22 +0530 Subject: [PATCH 5/5] fix: Comment in test --- frappe/core/doctype/user_permission/test_user_permission.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/user_permission/test_user_permission.py b/frappe/core/doctype/user_permission/test_user_permission.py index 1740d1b1e1..8319b0a270 100644 --- a/frappe/core/doctype/user_permission/test_user_permission.py +++ b/frappe/core/doctype/user_permission/test_user_permission.py @@ -226,7 +226,7 @@ class TestUserPermission(unittest.TestCase): new_doc = frappe.new_doc("Doc A") # User perm is created on ToDo but for doctype Assignment Rule only - # it should not have impact on WorkFlow + # it should not have impact on Doc A self.assertEquals(new_doc.doc, "ToDo") frappe.set_user('Administrator')