Skip to content
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
33 changes: 33 additions & 0 deletions frappe/model/utils/mask.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
# License: MIT. See LICENSE
from typing import Any


def mask_field_value(field, val):
Expand Down Expand Up @@ -55,6 +56,38 @@ def mask_dict_results(result, masked_fields):
return result


def mask_pluck_results(
result: list[Any],
masked_fields: list[Any],
fields: list[Any],
) -> list[Any]:
"""Mask plucked (scalar) results.

With ``pluck``, the database layer has already reduced each row to the
scalar value of the first selected field, so ``result`` is a flat list of
values rather than a list of tuples/dicts. Mask each value only when the
plucked field is itself a masked field; otherwise return it untouched.

Args:
result: Flat list of scalar values (one per row)
masked_fields: List of DocField objects with masking configuration
fields: List of field objects from the query (the plucked field is first)

Returns:
List of scalar values, with the plucked field masked when applicable
"""
if not fields:
return result

field = fields[0]
field_name = getattr(field, "alias", None) or getattr(field, "name", None)
masked_field = next((f for f in masked_fields if f.fieldname == field_name), None)
if not masked_field:
return result

return [mask_field_value(masked_field, val) for val in result]


def mask_list_results(result, masked_fields, field_index_map):
"""Mask fields in list/tuple results.

Expand Down
11 changes: 9 additions & 2 deletions frappe/query_builder/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ def mask_fields(
fields: list[Any],
result: list[dict] | list[tuple],
as_dict: bool = True,
pluck: bool = False,
) -> list[dict] | list[tuple]:
"""Mask fields in the result based on the doctype's masked fields.

Expand All @@ -92,12 +93,13 @@ def mask_fields(
fields: List of field objects from the query
result: Query results as list of dicts or tuples
as_dict: Whether results are dictionaries (True) or tuples (False)
pluck: Whether results were plucked into a flat list of scalar values

Returns:
Result with masked field values applied based on user permissions
"""
from frappe.database.query import CORE_DOCTYPES
from frappe.model.utils.mask import mask_dict_results, mask_list_results
from frappe.model.utils.mask import mask_dict_results, mask_list_results, mask_pluck_results

# We can't query meta for core doctypes here
if doctype in CORE_DOCTYPES:
Expand All @@ -108,6 +110,11 @@ def mask_fields(
if not masked_fields:
return result

# `pluck` returns a flat list of scalars, not rows, so it must be handled
# before the dict/tuple paths which assume each row is a collection.
if pluck:
return mask_pluck_results(result, masked_fields, fields)

if not as_dict:
field_index_map = {}
for idx, field in enumerate(fields):
Expand Down Expand Up @@ -135,7 +142,7 @@ def execute_query(query, *args, **kwargs):

if result and dt and fields:
as_dict = kwargs.get("as_dict", not kwargs.get("as_list", False))
result = mask_fields(dt, fields, result, as_dict=as_dict)
result = mask_fields(dt, fields, result, as_dict=as_dict, pluck=kwargs.get("pluck", False))

return result

Expand Down
55 changes: 55 additions & 0 deletions frappe/tests/test_db_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,61 @@ def test_permlevel_fields(self):
limit=1,
)

def test_get_values_pluck_with_masked_fields(self):
"""Regression for #39898.

For a non-admin user, a doctype with a masked field used to corrupt
`frappe.db.get_values(..., pluck=True)`: `mask_list_results` ran
`list(row)` on each already-plucked scalar, so a string like "P-1156"
came back as the tuple ('P', '-', '1', '1', '5', '6') (and a non-string
value such as an int raised TypeError outright).
"""
from frappe.core.doctype.doctype.test_doctype import new_doctype

frappe.delete_doc_if_exists("DocType", "Test Masked Pluck")
new_doctype(
"Test Masked Pluck",
fields=[
{"label": "Secret", "fieldname": "secret", "fieldtype": "Data", "mask": 1},
{"label": "Amount", "fieldname": "amount", "fieldtype": "Int"},
],
permissions=[
{"role": "System Manager", "read": 1, "write": 1, "create": 1, "mask": 1},
{"role": "Blogger", "read": 1},
],
).insert(ignore_permissions=True)
self.addCleanup(frappe.delete_doc_if_exists, "DocType", "Test Masked Pluck")

record = frappe.get_doc(
{"doctype": "Test Masked Pluck", "secret": "P-1156", "amount": 42}
).insert(ignore_permissions=True)

# Sanity: Administrator is never masked and gets the raw values.
self.assertEqual(
frappe.db.get_values("Test Masked Pluck", record.name, "name", pluck=True), [record.name]
)

with setup_test_user(set_user=True):
# `secret` is masked for this user, so the doctype has masked fields.
self.assertTrue(frappe.get_meta("Test Masked Pluck").get_masked_fields())

# Plucking a non-masked string field must return the scalar, not a char tuple.
self.assertEqual(
frappe.db.get_values("Test Masked Pluck", record.name, "name", pluck=True),
[record.name],
)

# Plucking a non-masked int field must not raise (used to: list(int)).
self.assertEqual(
frappe.db.get_values("Test Masked Pluck", record.name, "amount", pluck=True), [42]
)

# Plucking the masked field itself returns the masked scalar value.
self.assertEqual(
frappe.db.get_values("Test Masked Pluck", record.name, "secret", pluck=True),
["XXXXXXXX"],
)

def test_cast_name(self):
from frappe.core.doctype.doctype.test_doctype import new_doctype

Expand Down