Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Check permission_type in get_permitted_fieldnames [v13] (backport #20905) #20955

Merged
merged 4 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions frappe/desk/reportview.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import frappe.permissions
from frappe import _
from frappe.core.doctype.access_log.access_log import make_access_log
from frappe.model import default_fields, optional_fields
from frappe.model import default_fields, get_permitted_fields, optional_fields
from frappe.model.base_document import get_controller
from frappe.model.db_query import DatabaseQuery
from frappe.utils import cstr, format_duration
Expand Down Expand Up @@ -209,7 +209,10 @@ def update_wildcard_field_param(data):
if (isinstance(data.fields, string_types) and data.fields == "*") or (
isinstance(data.fields, (list, tuple)) and len(data.fields) == 1 and data.fields[0] == "*"
):
data.fields = frappe.db.get_table_columns(data.doctype)
if frappe.get_system_settings("apply_perm_level_on_api_calls"):
data.fields = get_permitted_fields(data.doctype, parenttype=data.parenttype)
else:
data.fields = frappe.db.get_table_columns(data.doctype)
return True

return False
Expand Down
15 changes: 13 additions & 2 deletions frappe/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,10 @@ def delete_fields(args_dict, delete=0):


def get_permitted_fields(
doctype: str, parenttype: Optional[str] = None, user: Optional[str] = None
doctype: str,
parenttype: Optional[str] = None,
user: Optional[str] = None,
permission_type: Optional[str] = None,
) -> List[str]:
meta = frappe.get_meta(doctype)
valid_columns = meta.get_valid_columns()
Expand All @@ -195,9 +198,17 @@ def get_permitted_fields(
if set(valid_columns).issubset(default_fields):
return valid_columns

permitted_fields = meta.get_permitted_fieldnames(parenttype=parenttype, user=user)
if permission_type is None:
permission_type = "select" if frappe.only_has_select_perm(doctype, user=user) else "read"

permitted_fields = meta.get_permitted_fieldnames(
parenttype=parenttype, user=user, permission_type=permission_type
)

if permitted_fields:
if permission_type == "select":
return permitted_fields

meta_fields = meta.default_fields.copy()
optional_meta_fields = [x for x in optional_fields if x in valid_columns]

Expand Down
8 changes: 6 additions & 2 deletions frappe/model/db_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ def __init__(self, doctype, user=None):
self.ignore_ifnull = False
self.flags = frappe._dict()
self.reference_doctype = None
self.permission_map = {}

def execute(
self,
Expand Down Expand Up @@ -98,7 +99,6 @@ def execute(
and not frappe.has_permission(self.doctype, "select", user=user)
and not frappe.has_permission(self.doctype, "read", user=user)
):

frappe.flags.error_message = _("Insufficient Permission for {0}").format(
frappe.bold(self.doctype)
)
Expand Down Expand Up @@ -421,6 +421,7 @@ def append_table(self, table_name):
if (not self.flags.ignore_permissions) and (not frappe.has_permission(doctype, ptype=ptype)):
frappe.flags.error_message = _("Insufficient Permission for {0}").format(frappe.bold(doctype))
raise frappe.PermissionError(doctype)
self.permission_map[doctype] = ptype

def set_field_tables(self):
"""If there are more than one table, the fieldname must not be ambiguous.
Expand Down Expand Up @@ -525,7 +526,10 @@ def apply_fieldlevel_read_permissions(self):
return

asterisk_fields = []
permitted_fields = get_permitted_fields(doctype=self.doctype)
permitted_fields = get_permitted_fields(
doctype=self.doctype,
permission_type=self.permission_map.get(self.doctype),
)

for i, field in enumerate(self.fields):
if "distinct" in field.lower():
Expand Down
12 changes: 10 additions & 2 deletions frappe/model/meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ def get_high_permlevel_fields(self):

return self.high_permlevel_fields

def get_permitted_fieldnames(self, parenttype=None, *, user=None):
def get_permitted_fieldnames(self, parenttype=None, *, user=None, permission_type="read"):
"""Build list of `fieldname` with read perm level and all the higher perm levels defined.
Note: If permissions are not defined for DocType, return all the fields with value.
"""
Expand All @@ -505,10 +505,18 @@ def get_permitted_fieldnames(self, parenttype=None, *, user=None):
if self.istable and not parenttype:
return permitted_fieldnames

if not permission_type:
permission_type = "select" if frappe.only_has_select_perm(self.name, user=user) else "read"

if permission_type == "select":
return self.get_search_fields()

if not self.get_permissions(parenttype=parenttype):
return self.get_fieldnames_with_value()

permlevel_access = set(self.get_permlevel_access("read", parenttype, user=user))
permlevel_access = set(
self.get_permlevel_access(permission_type=permission_type, parenttype=parenttype, user=user)
)

for df in self.get_fieldnames_with_value(with_field_meta=True):
if df.permlevel in permlevel_access:
Expand Down
7 changes: 7 additions & 0 deletions frappe/tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
update_permission_property,
)
from frappe.test_runner import make_test_records_for_doctype
from frappe.tests.test_db_query import enable_permlevel_restrictions

test_dependencies = ["Blogger", "Blog Post", "User", "Contact", "Salutation"]

Expand Down Expand Up @@ -89,6 +90,12 @@ def test_select_permission(self):
self.assertFalse(post.has_permission("read"))
self.assertRaises(frappe.PermissionError, post.save)

with enable_permlevel_restrictions():
permitted_record = frappe.get_list("Blog Post", fields="*", limit=1)[0]
full_record = frappe.get_all("Blog Post", fields="*", limit=1)[0]
self.assertNotEqual(permitted_record, full_record)
self.assertTrue(set(post.meta.get_search_fields()).issubset(set(permitted_record)))

def test_user_permissions_in_doc(self):
add_user_permission("Blog Category", "-test-blog-category-1", "test2@example.com")

Expand Down
4 changes: 2 additions & 2 deletions frappe/workflow/doctype/workflow/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,15 @@ def set_active(self):

@frappe.whitelist()
def get_fieldnames_for(doctype):
frappe.has_permission(doctype=doctype, ptype='read', throw=True)
frappe.has_permission(doctype=doctype, ptype="read", throw=True)
return [
f.fieldname for f in frappe.get_meta(doctype).fields if f.fieldname not in no_value_fields
]


@frappe.whitelist()
def get_workflow_state_count(doctype, workflow_state_field, states):
frappe.has_permission(doctype=doctype, ptype='read', throw=True)
frappe.has_permission(doctype=doctype, ptype="read", throw=True)
states = frappe.parse_json(states)
result = frappe.get_all(
doctype,
Expand Down
Loading