vassili/seitime-frappe
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 4

Merge pull request #12713 from rmehta/fix-reportview

Browse source 
fix(refactor): lockdown frappe.desk.reportview
This commit is contained in:
Rushabh Mehta 2021-03-31 10:50:25 +05:30 • committed by GitHub
commit ed6468d5f2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 195 additions and 124 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
frappe/__init__.py
View file

@ -1378,7 +1378,7 @@ def get_list(doctype, *args, **kwargs):
frappe.get_list("ToDo", fields="*", filters = {"description": ("like", "test%")})
"""
import frappe.model.db_query
return frappe.model.db_query.DatabaseQuery(doctype).execute(None, *args, **kwargs)
return frappe.model.db_query.DatabaseQuery(doctype).execute(*args, **kwargs)
def get_all(doctype, *args, **kwargs):
"""List database query via `frappe.model.db_query`. Will **not** check for permissions.

34
frappe/client.py
View file

@ -8,6 +8,8 @@ import frappe.model
import frappe.utils
import json, os
from frappe.utils import get_safe_filters
from frappe.desk.reportview import validate_args
from frappe.model.db_query import check_parent_permission
from six import iteritems, string_types, integer_types
@ -31,8 +33,18 @@ def get_list(doctype, fields=None, filters=None, order_by=None,
if frappe.is_table(doctype):
check_parent_permission(parent, doctype)
return frappe.get_list(doctype, fields=fields, filters=filters, order_by=order_by,
limit_start=limit_start, limit_page_length=limit_page_length, ignore_permissions=False)
args = frappe._dict(
doctype=doctype,
fields=fields,
filters=filters,
order_by=order_by,
limit_start=limit_start,
limit_page_length=limit_page_length,
)
validate_args(args)
return frappe.get_list(**args)
@frappe.whitelist()
def get_count(doctype, filters=None, debug=False, cache=False):
@ -91,12 +103,12 @@ def get_value(doctype, fieldname, filters=None, as_dict=True, debug=False, paren
if frappe.get_meta(doctype).issingle:
value = frappe.db.get_values_from_single(fields, filters, doctype, as_dict=as_dict, debug=debug)
else:
value = frappe.get_list(doctype, filters=filters, fields=fields, debug=debug, limit=1)
value = get_list(doctype, filters=filters, fields=fields, limit_page_length=1)
if as_dict:
value = value[0] if value else {}
else:
value = value[0].fieldname
value = value[0][fieldname]
return value
@ -378,18 +390,6 @@ def attach_file(filename=None, filedata=None, doctype=None, docname=None, folder
def get_hooks(hook, app_name=None):
return frappe.get_hooks(hook, app_name)
def check_parent_permission(parent, child_doctype):
if parent:
# User may pass fake parent and get the information from the child table
if child_doctype and not frappe.db.exists('DocField',
{'parent': parent, 'options': child_doctype}):
raise frappe.PermissionError
if frappe.permissions.has_permission(parent):
return
# Either parent not passed or the user doesn't have permission on parent doctype of child table!
raise frappe.PermissionError
@frappe.whitelist()
def is_document_amended(doctype, docname):
if frappe.permissions.has_permission(doctype):
@ -400,4 +400,4 @@ def is_document_amended(doctype, docname):
except frappe.db.InternalError:
pass
return False
return False

190
frappe/desk/reportview.py
View file

@ -8,6 +8,7 @@ import frappe, json
from six.moves import range
import frappe.permissions
from frappe.model.db_query import DatabaseQuery
from frappe.model import default_fields, optional_fields
from frappe import _
from six import string_types, StringIO
from frappe.core.doctype.access_log.access_log import make_access_log
@ -18,10 +19,20 @@ from frappe.utils import cstr, format_duration
@frappe.read_only()
def get():
args = get_form_params()
return compress(execute(**args), args=args)
data = compress(execute(**args), args = args)
@frappe.whitelist()
@frappe.read_only()
def get_list():
# uncompressed (refactored from frappe.model.db_query.get_list)
return execute(**get_form_params())
return data
@frappe.whitelist()
@frappe.read_only()
def get_count():
args = get_form_params()
args.fields = ['{distinct}count(name) as total_count'.format(distinct = 'distinct ' if args.distinct=='true' else '')]
return execute(**args)[0].get('total_count')
def execute(doctype, *args, **kwargs):
return DatabaseQuery(doctype).execute(*args, **kwargs)
@ -29,9 +40,136 @@ def execute(doctype, *args, **kwargs):
def get_form_params():
"""Stringify GET request parameters."""
data = frappe._dict(frappe.local.form_dict)
clean_params(data)
validate_args(data)
return data
is_report = data.get('view') == 'Report'
def validate_args(data):
parse_json(data)
setup_group_by(data)
validate_fields(data)
if data.filters:
validate_filters(data, data.filters)
if data.or_filters:
validate_filters(data, data.or_filters)
data.strict = None
return data
def validate_fields(data):
wildcard = update_wildcard_field_param(data)
for field in data.fields or []:
fieldname = extract_fieldname(field)
if is_standard(fieldname):
continue
meta, df = get_meta_and_docfield(fieldname, data)
if not df:
if wildcard:
continue
else:
raise_invalid_field(fieldname)
# remove the field from the query if the report hide flag is set and current view is Report
if df.report_hide and data.view == 'Report':
data.fields.remove(field)
continue
if df.fieldname in [_df.fieldname for _df in meta.get_high_permlevel_fields()]:
if df.get('permlevel') not in meta.get_permlevel_access(parenttype=data.doctype):
data.fields.remove(field)
def validate_filters(data, filters):
if isinstance(filters, list):
# filters as list
for condition in filters:
if len(condition)==3:
# [fieldname, condition, value]
fieldname = condition[0]
if is_standard(fieldname):
continue
meta, df = get_meta_and_docfield(fieldname, data)
if not df:
raise_invalid_field(condition[0])
else:
# [doctype, fieldname, condition, value]
fieldname = condition[1]
if is_standard(fieldname):
continue
meta = frappe.get_meta(condition[0])
if not meta.get_field(fieldname):
raise_invalid_field(fieldname)
else:
for fieldname in filters:
if is_standard(fieldname):
continue
meta, df = get_meta_and_docfield(fieldname, data)
if not df:
raise_invalid_field(fieldname)
def setup_group_by(data):
'''Add columns for aggregated values e.g. count(name)'''
if data.group_by:
if data.aggregate_function.lower() not in ('count', 'sum', 'avg'):
frappe.throw(_('Invalid aggregate function'))
if '`' in data.aggregate_on:
raise_invalid_field(data.aggregate_on)
data.fields.append('{aggregate_function}(`tab{doctype}`.`{aggregate_on}`) AS _aggregate_column'.format(**data))
if data.aggregate_on:
data.fields.append(data.aggregate_on)
data.pop('aggregate_on')
data.pop('aggregate_function')
def raise_invalid_field(fieldname):
frappe.throw(_('Field not permitted in query') + ': {0}'.format(fieldname), frappe.DataError)
def is_standard(fieldname):
if '.' in fieldname:
parenttype, fieldname = get_parenttype_and_fieldname(fieldname, None)
return fieldname in default_fields or fieldname in optional_fields
def extract_fieldname(field):
for text in (',', '/*', '#'):
if text in field:
raise_invalid_field(field)
fieldname = field
for sep in (' as ', ' AS '):
if sep in fieldname:
fieldname = fieldname.split(sep)[0]
# certain functions allowed, extract the fieldname from the function
if (fieldname.startswith('count(')
or fieldname.startswith('sum(')
or fieldname.startswith('avg(')):
if not fieldname.strip().endswith(')'):
raise_invalid_field(field)
fieldname = fieldname.split('(', 1)[1][:-1]
return fieldname
def get_meta_and_docfield(fieldname, data):
parenttype, fieldname = get_parenttype_and_fieldname(fieldname, data)
meta = frappe.get_meta(parenttype)
df = meta.get_field(fieldname)
return meta, df
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)
return True
return False
def clean_params(data):
data.pop('cmd', None)
data.pop('data', None)
data.pop('ignore_permissions', None)
@ -41,8 +179,12 @@ def get_form_params():
if "csrf_token" in data:
del data["csrf_token"]
def parse_json(data):
if isinstance(data.get("filters"), string_types):
data["filters"] = json.loads(data["filters"])
if isinstance(data.get("or_filters"), string_types):
data["or_filters"] = json.loads(data["or_filters"])
if isinstance(data.get("fields"), string_types):
data["fields"] = json.loads(data["fields"])
if isinstance(data.get("docstatus"), string_types):
@ -52,47 +194,8 @@ def get_form_params():
else:
data["save_user_settings"] = True
fields = data["fields"]
if ((isinstance(fields, string_types) and fields == "*")
or (isinstance(fields, (list, tuple)) and len(fields) == 1 and fields[0] == "*")):
parenttype = data.doctype
data["fields"] = frappe.db.get_table_columns(parenttype)
fields = data["fields"]
for field in fields:
key = field.split(" as ")[0]
if key.startswith('count('): continue
if key.startswith('sum('): continue
if key.startswith('avg('): continue
parenttype, fieldname = get_parent_dt_and_field(key, data)
if fieldname == "*":
# * inside list is not allowed with other fields
fields.remove(field)
meta = frappe.get_meta(parenttype)
df = meta.get_field(fieldname)
report_hide = df.report_hide if df else None
# remove the field from the query if the report hide flag is set and current view is Report
if report_hide and is_report:
fields.remove(field)
if df and fieldname in [df.fieldname for df in meta.get_high_permlevel_fields()]:
if df.get('permlevel') not in meta.get_permlevel_access(parenttype=data.doctype) and field in fields:
fields.remove(field)
# queries must always be server side
data.query = None
data.strict = None
return data
def get_parent_dt_and_field(field, data):
def get_parenttype_and_fieldname(field, data):
if "." in field:
parenttype, fieldname = field.split(".")[0][4:-1], field.split(".")[1].strip("`")
else:
@ -101,7 +204,6 @@ def get_parent_dt_and_field(field, data):
return parenttype, fieldname
def compress(data, args = {}):
"""separate keys and values"""
from frappe.desk.query_report import add_total_row

2
frappe/frappeclient.py
View file

@ -86,7 +86,7 @@ class FrappeClient(object):
'cmd': 'logout',
}, verify=self.verify, headers=self.headers)
def get_list(self, doctype, fields='"*"', filters=None, limit_start=0, limit_page_length=0):
def get_list(self, doctype, fields='["name"]', filters=None, limit_start=0, limit_page_length=0):
"""Returns list of records of a particular type"""
if not isinstance(fields, string_types):
fields = json.dumps(fields)

56
frappe/model/db_query.py
View file

@ -14,7 +14,6 @@ import frappe.permissions
from datetime import datetime
import frappe, json, copy, re
from frappe.model import optional_fields
from frappe.client import check_parent_permission
from frappe.model.utils.user_settings import get_user_settings, update_user_settings
from frappe.utils import flt, cint, get_time, make_filter_tuple, get_filter, add_to_date, cstr, get_timespan_date_range
from frappe.model.meta import get_table_columns
@ -32,7 +31,7 @@ class DatabaseQuery(object):
self.flags = frappe._dict()
self.reference_doctype = None
def execute(self, query=None, fields=None, filters=None, or_filters=None,
def execute(self, fields=None, filters=None, or_filters=None,
docstatus=None, group_by=None, order_by=None, limit_start=False,
limit_page_length=None, as_list=False, with_childnames=False, debug=False,
ignore_permissions=False, user=None, with_comment_count=False,
@ -104,12 +103,9 @@ class DatabaseQuery(object):
# no table & ignore_ddl, return
if not self.columns: return []
if query:
result = self.run_custom_query(query)
else:
result = self.build_and_run()
if return_query:
return result
result = self.build_and_run()
if return_query:
return result
if with_comment_count and not as_list and self.doctype:
self.add_comment_count(result)
@ -707,12 +703,6 @@ class DatabaseQuery(object):
return " and ".join(conditions) if conditions else ""
def run_custom_query(self, query):
if '%(key)s' in query:
query = query.replace('%(key)s', '`name`')
return frappe.db.sql(query, as_dict = (not self.as_list))
def set_order_by(self, args):
meta = frappe.get_meta(self.doctype)
@ -754,7 +744,7 @@ class DatabaseQuery(object):
return
_lower = parameters.lower()
if 'select' in _lower and ' from ' in _lower:
if 'select' in _lower and 'from' in _lower:
frappe.throw(_('Cannot use sub-query in order by'))
if re.compile(r".*[^a-z0-9-_ ,`'\"\.\(\)].*").match(_lower):
@ -795,6 +785,18 @@ class DatabaseQuery(object):
update_user_settings(self.doctype, user_settings)
def check_parent_permission(parent, child_doctype):
if parent:
# User may pass fake parent and get the information from the child table
if child_doctype and not frappe.db.exists('DocField',
{'parent': parent, 'options': child_doctype}):
raise frappe.PermissionError
if frappe.permissions.has_permission(parent):
return
# Either parent not passed or the user doesn't have permission on parent doctype of child table!
raise frappe.PermissionError
def get_order_by(doctype, meta):
order_by = ""
@ -819,30 +821,6 @@ def get_order_by(doctype, meta):
return order_by
@frappe.whitelist()
def get_list(doctype, *args, **kwargs):
'''wrapper for DatabaseQuery'''
kwargs.pop('cmd', None)
kwargs.pop('ignore_permissions', None)
kwargs.pop('data', None)
kwargs.pop('strict', None)
kwargs.pop('user', None)
# If doctype is child table
if frappe.is_table(doctype):
# Example frappe.db.get_list('Purchase Receipt Item', {'parent': 'Purchase Receipt'})
# Here purchase receipt is the parent doctype of the child doctype Purchase Receipt Item
if not kwargs.get('parent'):
frappe.flags.error_message = _('Parent is required to get child table data')
raise frappe.PermissionError(doctype)
check_parent_permission(kwargs.get('parent'), doctype)
del kwargs['parent']
return DatabaseQuery(doctype).execute(None, *args, **kwargs)
def is_parent_only_filter(doctype, filters):
#check if filters contains only parent doctype
only_parent_doctype = True

16
frappe/public/js/frappe/db.js
View file

@ -15,7 +15,7 @@ frappe.db = {
}
return new Promise ((resolve) => {
frappe.call({
method: 'frappe.model.db_query.get_list',
method: 'frappe.desk.reportview.get_list',
args: args,
type: 'GET',
callback: function(r) {
@ -92,25 +92,25 @@ frappe.db = {
},
count: function(doctype, args={}) {
let filters = args.filters || {};
const with_child_table_filter = Array.isArray(filters) && filters.some(filter => {
// has a filter with childtable?
const distinct = Array.isArray(filters) && filters.some(filter => {
return filter[0] !== doctype;
});
const fields = [
// cannot break this line as it adds extra \n's and \t's which breaks the query
`count(${with_child_table_filter ? 'distinct': ''} ${frappe.model.get_full_column_name('name', doctype)}) AS total_count`
];
const fields = [];
return frappe.call({
type: 'GET',
method: 'frappe.desk.reportview.get',
method: 'frappe.desk.reportview.get_count',
args: {
doctype,
filters,
fields,
distinct,
}
}).then(r => {
return r.message.values[0][0];
return r.message.values;
});
},
get_link_options(doctype, txt = '', filters={}) {

16
frappe/public/js/frappe/ui/group_by/group_by.js
View file

@ -286,15 +286,6 @@ frappe.ui.GroupBy = class {
set_args(args) {
if (this.aggregate_function && this.group_by) {
let aggregate_column, aggregate_on_field;
if (this.aggregate_function === 'count') {
aggregate_column = 'count(`tab' + this.doctype + '`.`name`)';
} else {
aggregate_column = `${this.aggregate_function}(${this.aggregate_on})`;
aggregate_on_field = this.aggregate_on;
}
this.report_view.group_by = this.group_by;
this.report_view.sort_by = '_aggregate_column';
this.report_view.sort_order = 'desc';
@ -316,17 +307,14 @@ frappe.ui.GroupBy = class {
'_aggregate_column',
this.aggregate_on_doctype || this.doctype,
]);
args.fields.push(aggregate_column + ' as _aggregate_column');
if (aggregate_on_field) {
args.fields.push(aggregate_on_field);
}
// setup columns in datatable
this.report_view.setup_columns();
Object.assign(args, {
with_comment_count: false,
aggregate_on: this.aggregate_on || 'name',
aggregate_function: this.aggregate_function || 'count',
group_by: this.report_view.group_by || null,
order_by: '_aggregate_column desc',
});

3
frappe/tests/test_db_query.py
View file

@ -17,6 +17,9 @@ from frappe.utils.testutils import add_custom_field, clear_custom_fields
test_dependencies = ['User', 'Blog Post', 'Blog Category', 'Blogger']
class TestReportview(unittest.TestCase):
def setUp(self):
frappe.set_user("Administrator")
def test_basic(self):
self.assertTrue({"name":"DocType"} in DatabaseQuery("DocType").execute(limit_page_length=None))