diff --git a/erpnext/accounts/custom/address.py b/erpnext/accounts/custom/address.py index 1c657b9a7a3..29454ecac1d 100644 --- a/erpnext/accounts/custom/address.py +++ b/erpnext/accounts/custom/address.py @@ -55,6 +55,18 @@ class ERPNextAddress(Address): @frappe.whitelist() def get_shipping_address(company: str, address: str | None = None): + # `company` is caller supplied and this returns that company's own registered address with every + # field. `select` rather than `read` on Company: Delivery, Maintenance, Purchase Manager and + # Stock Manager all fill in transactions that ask for this while holding no Company `read` row. + frappe.has_permission("Company", ptype="select", throw=True) + + # and scope it to the caller's own Company restrictions, which costs nobody who has none + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "Address") + if allowed_companies and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + filters = [ ["Dynamic Link", "link_doctype", "=", "Company"], ["Dynamic Link", "link_name", "=", company], diff --git a/erpnext/accounts/dashboard_chart_source/account_balance_timeline/account_balance_timeline.py b/erpnext/accounts/dashboard_chart_source/account_balance_timeline/account_balance_timeline.py index 0a49280e5ae..08f8ccac157 100644 --- a/erpnext/accounts/dashboard_chart_source/account_balance_timeline/account_balance_timeline.py +++ b/erpnext/accounts/dashboard_chart_source/account_balance_timeline/account_balance_timeline.py @@ -24,7 +24,7 @@ def get( heatmap_year: str | None = None, ): if chart_name: - chart = frappe.get_doc("Dashboard Chart", chart_name) + chart = frappe.get_doc("Dashboard Chart", chart_name, check_permission="read") else: chart = frappe._dict(frappe.parse_json(chart)) timespan = chart.timespan @@ -46,6 +46,9 @@ def get( if not account: frappe.throw(_("Account filter not set!")) + # authorise the account itself, as get_balance_on() does; doc= brings User Permissions with it + frappe.has_permission("Account", doc=account, throw=True) + if not to_date: to_date = nowdate() if not from_date: diff --git a/erpnext/accounts/doctype/account/account.py b/erpnext/accounts/doctype/account/account.py index 7ea8935e584..34852ea976d 100644 --- a/erpnext/accounts/doctype/account/account.py +++ b/erpnext/accounts/doctype/account/account.py @@ -503,24 +503,21 @@ class Account(NestedSet): @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_parent_account(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): - Account = frappe.qb.DocType("Account") - - search_field_obj = getattr(Account, searchfield) - - query = ( - frappe.qb.from_(Account) - .select(Account.name) - .where(Account.is_group == 1) - .where(Account.docstatus != 2) - .where(Account.company == filters["company"]) - .where(search_field_obj.like(f"%{txt}%")) - .order_by(Account.name) - .limit(page_len) - .offset(start) + return frappe.get_list( + "Account", + filters=[ + ["is_group", "=", 1], + ["docstatus", "!=", 2], + ["company", "=", filters["company"]], + [searchfield, "like", f"%{txt}%"], + ], + fields=["name"], + order_by="name", + limit_start=start, + limit_page_length=page_len, + as_list=True, ) - return query.run(as_list=1) - def get_account_currency(account): """Helper function to get account currency""" diff --git a/erpnext/accounts/doctype/accounting_dimension/accounting_dimension.py b/erpnext/accounts/doctype/accounting_dimension/accounting_dimension.py index f5b1867fbf9..fa04f2dfc9e 100644 --- a/erpnext/accounts/doctype/accounting_dimension/accounting_dimension.py +++ b/erpnext/accounts/doctype/accounting_dimension/accounting_dimension.py @@ -223,8 +223,11 @@ def delete_accounting_dimension(doc): frappe.clear_cache(doctype=doctype) -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def disable_dimension(doc: str): + # toggle_disabling rewrites a Custom Field site-wide, so demand the write that configures dimensions + frappe.has_permission("Accounting Dimension", "write", throw=True) + if frappe.in_test: toggle_disabling(doc=doc) else: diff --git a/erpnext/accounts/doctype/bank_guarantee/bank_guarantee.py b/erpnext/accounts/doctype/bank_guarantee/bank_guarantee.py index 4326c404fd5..efb674ae4b3 100644 --- a/erpnext/accounts/doctype/bank_guarantee/bank_guarantee.py +++ b/erpnext/accounts/doctype/bank_guarantee/bank_guarantee.py @@ -60,6 +60,9 @@ def get_voucher_details(bank_guarantee_type: str, reference_name: str): if not isinstance(reference_name, str): raise TypeError("reference_name must be a string") + # the form is the boundary, not the referenced order: an order guard would break one of the two roles + frappe.has_permission("Bank Guarantee", throw=True) + fields_to_fetch = ["grand_total"] if bank_guarantee_type == "Receiving": @@ -70,4 +73,14 @@ def get_voucher_details(bank_guarantee_type: str, reference_name: str): doctype = "Purchase Order" fields_to_fetch.append("supplier") + # and scope the referenced order to the caller's own Company restrictions, which costs nobody + # who has none + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "Bank Guarantee") + if allowed_companies: + company = frappe.db.get_value(doctype, reference_name, "company") + if company and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + return frappe.db.get_value(doctype, reference_name, fields_to_fetch, as_dict=True) diff --git a/erpnext/accounts/doctype/bank_statement_import/bank_statement_import.py b/erpnext/accounts/doctype/bank_statement_import/bank_statement_import.py index de477326e78..010e9dab8c6 100644 --- a/erpnext/accounts/doctype/bank_statement_import/bank_statement_import.py +++ b/erpnext/accounts/doctype/bank_statement_import/bank_statement_import.py @@ -437,6 +437,11 @@ def get_import_logs(docname: str): @frappe.whitelist() def upload_bank_statement(**args): + # The only caller is the Bank Reconciliation Tool's "Upload Bank Statement" button, whose + # callback routes straight into a new Bank Statement Import form — so `create` is exactly the + # right to require, and both doctypes are System Manager only, which makes it loser-free. + frappe.has_permission("Bank Statement Import", "create", throw=True) + args = frappe._dict(args) bsi = frappe.new_doc("Bank Statement Import") diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction_upload.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction_upload.py index 813f4ad3589..2f281774213 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction_upload.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction_upload.py @@ -11,6 +11,11 @@ from frappe.utils.dateutils import parse_date @frappe.whitelist() def upload_bank_statement(): + # Parsing a statement is the first step of creating Bank Transactions from it, so that is the + # right to require. Both functions in this file are reached only over HTTP — nothing in the tree + # calls either — so there is no caller to break. + frappe.has_permission("Bank Transaction", "create", throw=True) + if getattr(frappe, "uploaded_file", None): with open(frappe.uploaded_file, "rb") as upfile: fcontent = upfile.read() @@ -36,6 +41,12 @@ def upload_bank_statement(): @frappe.whitelist(methods=["POST"]) def create_bank_entries(columns: str, data: str | list, bank_account: str): + # insert()/submit() below already enforce this per document, but only after the per-row loop has + # read the Bank Account and its Bank mapping and written an Error Log for every rejected row — + # so check once up front rather than failing row by row. + frappe.has_permission("Bank Transaction", "create", throw=True) + frappe.has_permission("Bank Account", doc=bank_account, throw=True) + header_map = get_header_mapping(columns, bank_account) success = 0 diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index f78c78697a3..5068ed49499 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -1023,6 +1023,11 @@ def get_default_bank_cash_account( ) -> dict: from erpnext.accounts.doctype.sales_invoice.sales_invoice import get_bank_cash_account + # the company is the scope being authorised, and doc= brings User Permissions to bear. `select`, + # not `read`: this also runs server-side from get_payment_entry, and Auditor/HR User/Desk User + # hold only the select row on Company + frappe.has_permission("Company", ptype="select", doc=company, throw=True) + if mode_of_payment: account = get_bank_cash_account(mode_of_payment, company).get("account") @@ -1051,6 +1056,11 @@ def get_default_bank_cash_account( account = account_list[0].name if account: + # `account` may be named by the caller outright, so authorise the account actually being + # described. get_balance_on() checks this too, but only on the branch that reads a balance, + # and `fetch_balance` is a caller-supplied argument. + frappe.has_permission("Account", doc=account, throw=True) + account_details = frappe.get_cached_value( "Account", account, ["account_currency", "account_type"], as_dict=1 ) @@ -1080,30 +1090,40 @@ def get_against_jv( if not frappe.db.has_column("Journal Entry", searchfield): return [] - JournalEntry = frappe.qb.DocType("Journal Entry") - JournalEntryAccount = frappe.qb.DocType("Journal Entry Account") + account = filters.get("account") + party = filters.get("party") - query = ( - frappe.qb.from_(JournalEntry) - .join(JournalEntryAccount) - .on(JournalEntryAccount.parent == JournalEntry.name) - .select(JournalEntry.name, JournalEntry.posting_date, JournalEntry.remark) - .where(JournalEntryAccount.account == filters.get("account")) - .where(JournalEntryAccount.reference_type.isnull() | (JournalEntryAccount.reference_type == "")) - .where(JournalEntry.docstatus == 1) - .where(JournalEntry[searchfield].like(f"%{txt}%")) - .orderby(JournalEntry.name, order=frappe.qb.desc) - .limit(page_len) - .offset(start) + # each names one value. A list would be read as a filter operator below and widen the search + # past what the caller named. + for value in (account, party): + if value and not isinstance(value, str): + frappe.throw(_("Invalid filter"), frappe.PermissionError) + + # get_list applies the permission query conditions; the child-table filter resolves the check to `read` + je_filters = [ + ["docstatus", "=", 1], + [searchfield, "like", f"%{txt}%"], + ["Journal Entry Account", "account", "=", account], + ["Journal Entry Account", "reference_type", "is", "not set"], + ] + je_filters.append( + ["Journal Entry Account", "party", "=", party] + if party + else ["Journal Entry Account", "party", "is", "not set"] ) - party = filters.get("party") - if party: - query = query.where(JournalEntryAccount.party == party) - else: - query = query.where(JournalEntryAccount.party.isnull() | (JournalEntryAccount.party == "")) - - return query.run() + return frappe.get_list( + "Journal Entry", + filters=je_filters, + fields=["name", "posting_date", "remark"], + order_by="name desc", + limit_start=start, + limit_page_length=page_len, + as_list=True, + # one row per entry, not per matching account row. group_by rather than distinct: frappe + # drops ORDER BY from a distinct query on postgres, which would lose the ordering above. + group_by="name", + ) @frappe.whitelist() diff --git a/erpnext/accounts/doctype/loyalty_program/loyalty_program.py b/erpnext/accounts/doctype/loyalty_program/loyalty_program.py index ef418c8ed1d..cba09a7ad2c 100644 --- a/erpnext/accounts/doctype/loyalty_program/loyalty_program.py +++ b/erpnext/accounts/doctype/loyalty_program/loyalty_program.py @@ -129,6 +129,11 @@ def get_loyalty_program_details( silent: bool = False, include_expired_entry: bool = False, ): + # Same guard as get_loyalty_program_details_with_points above: the customer is what the caller + # is entitled to, not the programme. A check on Loyalty Program itself would be read-only to + # System Manager and would deny every role that actually fills in the two calling forms. + frappe.has_permission("Customer", doc=customer, throw=True) + lp_details = frappe._dict() if not loyalty_program: @@ -150,6 +155,13 @@ def get_loyalty_program_details( @frappe.whitelist() def get_redeemption_factor(loyalty_program: str | None = None, customer: str | None = None): + # both call sites send only `loyalty_program`, so the calling form is the boundary; the customer branch stays guarded + if not (frappe.has_permission("Sales Invoice") or frappe.has_permission("POS Invoice")): + frappe.throw(_("Not permitted"), frappe.PermissionError) + + if customer: + frappe.has_permission("Customer", doc=customer, throw=True) + customer_loyalty_program = None if not loyalty_program: customer_loyalty_program = frappe.db.get_value("Customer", customer, "loyalty_program") diff --git a/erpnext/accounts/doctype/payment_order/payment_order.py b/erpnext/accounts/doctype/payment_order/payment_order.py index 7bc7ae1010b..898d829f0f8 100644 --- a/erpnext/accounts/doctype/payment_order/payment_order.py +++ b/erpnext/accounts/doctype/payment_order/payment_order.py @@ -57,9 +57,27 @@ class PaymentOrder(Document): frappe.db.set_value(self.payment_order_type, d.get(ref_doc_field), ref_field, status) +def _readable_payment_order(filters: dict) -> str | None: + """Authorise the parent before reading its rows. + + A child table carries no permissions of its own, so a read of it has to be authorised on the + Payment Order the rows belong to. + """ + parent = filters.get("parent") + if not parent or not frappe.db.exists("Payment Order", parent): + return None + + ptype = "select" if frappe.only_has_select_perm("Payment Order") else "read" + frappe.has_permission("Payment Order", ptype, doc=parent, throw=True) + return parent + + @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_mop_query(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): + if not _readable_payment_order(filters): + return [] + return frappe.get_all( "Payment Order Reference", filters={"parent": filters.get("parent"), "mode_of_payment": ["like", f"%{txt}%"]}, @@ -74,6 +92,9 @@ def get_mop_query(doctype: str, txt: str, searchfield: str, start: int, page_len @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_supplier_query(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): + if not _readable_payment_order(filters): + return [] + return frappe.get_all( "Payment Order Reference", filters={ diff --git a/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py b/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py index 68d7fc0150d..614180fa1ba 100644 --- a/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py +++ b/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py @@ -256,6 +256,13 @@ class POSClosingEntry(StatusUpdater): @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_cashiers(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): + pos_profile = filters.get("parent") + if not pos_profile or not frappe.db.exists("POS Profile", pos_profile): + return [] + + ptype = "select" if frappe.only_has_select_perm("POS Profile") else "read" + frappe.has_permission("POS Profile", ptype, doc=pos_profile, throw=True) + cashiers_list = frappe.get_all("POS Profile User", filters=filters, fields=["user"], as_list=1) return [c for c in cashiers_list] diff --git a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py index d6591e8b563..767ff126b68 100644 --- a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py +++ b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py @@ -909,6 +909,30 @@ class POSInvoice(SalesInvoice): @frappe.whitelist() def get_stock_availability(item_code: str | None, warehouse: str): + # The POS Profile is what entitles a caller to POS stock figures, and it is the only boundary + # that fits: `Item` read and `Bin` read both exclude Accounts Manager, `Item` select is granted + # to every desk user by `Desk User`, and `POS Invoice` read is granted to `All`. + frappe.has_permission("POS Profile", throw=True) + + # and keep a company-restricted caller inside their own companies, which costs nobody who has + # no Company User Permission + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "POS Profile") + if allowed_companies: + company = frappe.db.get_value("Warehouse", warehouse, "company") + if company and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + + # the caller picks the warehouse when allow_warehouse_change is set, and the company check above + # does not narrow within a company; costs nobody who has no Warehouse User Permission + from frappe.permissions import get_allowed_docs_for_doctype, get_user_permissions + + if warehouse_permissions := get_user_permissions(frappe.session.user).get("Warehouse"): + allowed_warehouses = get_allowed_docs_for_doctype(warehouse_permissions, "POS Invoice") + if allowed_warehouses and warehouse not in allowed_warehouses: + frappe.throw(_("Not permitted for {0}").format(warehouse), frappe.PermissionError) + if frappe.db.get_value("Item", item_code, "is_stock_item"): is_stock_item = True bin_qty = get_bin_qty(item_code, warehouse) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index fd43f5b58d5..58a3f94b368 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -12,6 +12,29 @@ from frappe import _, throw from frappe.model.document import Document from frappe.utils import cint, flt +# the transactions the pricing engine is called for, from transaction.js and the POS +PRICING_TRANSACTION_DOCTYPES = frozenset( + { + "Quotation", + "Sales Order", + "Delivery Note", + "Sales Invoice", + "POS Invoice", + "Supplier Quotation", + "Purchase Order", + "Purchase Receipt", + "Purchase Invoice", + "Material Request", + # these three also extend a controller that calls the pricing engine: BOM and BOM Creator + # through TransactionController, Request for Quotation through BuyingController + "BOM", + "BOM Creator", + "Request for Quotation", + # no client sends this one, but set_transaction_type below still branches on it + "Opportunity", + } +) + apply_on_dict = {"Item Code": "items", "Item Group": "item_groups", "Brand": "brands"} other_fields = ["other_item_code", "other_item_group", "other_brand"] @@ -366,6 +389,30 @@ def apply_pricing_rule(args: str | dict, doc: str | dict | Document | None = Non args = frappe._dict(args) + # `args` is caller supplied, and what comes back is pricing: matched Pricing Rules, discounts + # and rates. The transaction being priced is what decides who may price it, so authorise that + # — and the document itself where the caller named an existing one, so User Permissions apply. + # an allow-list, not just a type check: `doctype` is caller-chosen, and any doctype the caller can + # read would otherwise satisfy has_permission below while the pricing engine still ran + transaction_doctype = args.get("doctype") + if transaction_doctype not in PRICING_TRANSACTION_DOCTYPES: + frappe.throw(_("Invalid doctype"), frappe.PermissionError) + + transaction_name = args.get("name") + if not isinstance(transaction_name, str) or not frappe.db.exists(transaction_doctype, transaction_name): + transaction_name = None + + frappe.has_permission(transaction_doctype, doc=transaction_name, throw=True) + + # scope by the caller's own Company restrictions, not a Company read: several roles that fill these forms hold none + company = args.get("company") + if company: + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, transaction_doctype) + if allowed_companies and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + set_transaction_type(args) # list of dictionaries @@ -725,14 +772,18 @@ def set_transaction_type(pricing_ctx: frappe._dict) -> None: @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_item_uoms(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): - items = [filters.get("value")] - if filters.get("apply_on") != "Item Code": - field = frappe.scrub(filters.get("apply_on")) - items = [d.name for d in frappe.db.get_all("Item", filters={field: filters.get("value")})] + if filters.get("apply_on") == "Item Code": + item_filters = [["name", "=", filters.get("value")]] + else: + item_filters = [[frappe.scrub(filters.get("apply_on")), "=", filters.get("value")]] + + items = frappe.get_list("Item", filters=item_filters, pluck="name") + if not items: + return [] return frappe.get_all( "UOM Conversion Detail", - filters={"parent": ("in", items), "uom": ("like", f"{txt}%")}, + filters={"parent": ("in", items), "parenttype": "Item", "uom": ("like", f"{txt}%")}, fields=["uom"], as_list=1, distinct=True, diff --git a/erpnext/accounts/doctype/process_period_closing_voucher/process_period_closing_voucher.py b/erpnext/accounts/doctype/process_period_closing_voucher/process_period_closing_voucher.py index fda57a87086..34be6040ae5 100644 --- a/erpnext/accounts/doctype/process_period_closing_voucher/process_period_closing_voucher.py +++ b/erpnext/accounts/doctype/process_period_closing_voucher/process_period_closing_voucher.py @@ -133,14 +133,17 @@ def initialize_parallel_threads(docname: str): frappe.db.set_value("Process Period Closing Voucher", docname, "status", "Completed") -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def start_pcv_processing(docname: str): + # checked before the status is read, not inside the branch: otherwise an unentitled caller + # learns the document's status from whether this returns or throws + frappe.has_permission("Process Period Closing Voucher", "write", doc=docname, throw=True) + if frappe.db.get_value("Process Period Closing Voucher", docname, "status") in ["Queued", "Running"]: - frappe.has_permission("Process Period Closing Voucher", "write", doc=docname, throw=True) initialize_parallel_threads(docname) -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def pause_pcv_processing(docname: str): frappe.has_permission("Process Period Closing Voucher", ptype="write", doc=docname, throw=True) @@ -157,7 +160,7 @@ def pause_pcv_processing(docname: str): qb.update(ppcvd).set(ppcvd.status, "Paused").where(ppcvd.name.isin(queued_dates)).run() -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def cancel_pcv_processing(docname: str): frappe.has_permission("Process Period Closing Voucher", ptype="cancel", doc=docname, throw=True) @@ -173,7 +176,7 @@ def cancel_pcv_processing(docname: str): qb.update(ppcvd).set(ppcvd.status, "Cancelled").where(ppcvd.name.isin(queued_dates)).run() -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def resume_pcv_processing(docname: str): frappe.has_permission("Process Period Closing Voucher", ptype="write", doc=docname, throw=True) @@ -258,8 +261,11 @@ def get_gle_for_closing_account(pcv, dimension_balance, dimensions): return gl_entry -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def schedule_next_date(docname: str): + # marks a row Running and enqueues a long job, so it needs the same write check as the sibling controls + frappe.has_permission("Process Period Closing Voucher", ptype="write", doc=docname, throw=True) + timeout = frappe.db.get_single_value("Accounts Settings", "pcv_job_timeout") or 3600 ppcvd = qb.DocType("Process Period Closing Voucher Detail") diff --git a/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py index fcb3a02633d..4406857087c 100644 --- a/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py +++ b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py @@ -514,6 +514,11 @@ def validate_docs_for_voucher_types(doc_voucher_types): def get_repost_allowed_types( doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict ): + # Reached only from this doctype's own form (repost_accounting_ledger.js:8), and Repost + # Accounting Ledger is System-Manager-only, so the form is the boundary. `filters` is passed + # straight to db.get_all, which is why reaching it needs to be gated rather than merely typed. + frappe.has_permission("Repost Accounting Ledger", throw=True) + if txt: filters.update({"document_type": ("like", f"%{txt}%")}) diff --git a/erpnext/accounts/doctype/sales_invoice/mapper.py b/erpnext/accounts/doctype/sales_invoice/mapper.py index 888bc7b0b60..bdd5df0c3f5 100644 --- a/erpnext/accounts/doctype/sales_invoice/mapper.py +++ b/erpnext/accounts/doctype/sales_invoice/mapper.py @@ -376,9 +376,26 @@ def _apply_sales_party_details(target_doc, source_doc, details): @frappe.whitelist() def get_received_items(reference_name: str, doctype: str, reference_fieldname: str): - reference_field = "inter_company_invoice_reference" - if doctype == "Purchase Order": - reference_field = "inter_company_order_reference" + # The only two targets this resolves a reference field for. Stating them rejects a caller + # supplied doctype that would otherwise be filtered on a column it does not have. + reference_fields = { + "Purchase Invoice": ("inter_company_invoice_reference", "Sales Invoice"), + "Purchase Order": ("inter_company_order_reference", "Sales Order"), + } + if doctype not in reference_fields: + frappe.throw(_("Invalid doctype {0}").format(doctype), frappe.PermissionError) + + reference_field, source_doctype = reference_fields[doctype] + + # `reference_name` is the caller's own document. The targets belong to the counterpart company + # and the caller legitimately may not be able to read them, so the source is what decides + # access here rather than the doctype being counted. doc= brings User Permissions in. + frappe.has_permission(source_doctype, doc=reference_name, throw=True) + + # `reference_fieldname` is selected as a column below and its value becomes the result key, + # so an unchecked one returns any field of the item table to the caller. + if not frappe.get_meta(doctype + " Item").has_field(reference_fieldname): + frappe.throw(_("Invalid field {0}").format(reference_fieldname), frappe.PermissionError) filters = { reference_field: reference_name, diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 4eb27964e8f..f01882cdd45 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -52,7 +52,7 @@ from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import create_stock_reconciliation, ) from erpnext.stock.get_item_details import get_item_tax_map -from erpnext.stock.utils import get_incoming_rate, get_stock_balance +from erpnext.stock.utils import _get_incoming_rate, get_stock_balance from erpnext.tests.utils import ERPNextTestSuite @@ -3356,7 +3356,7 @@ class TestSalesInvoice(ERPNextTestSuite): rate = 0.0 for d in si.get("items"): - rate = get_incoming_rate( + rate = _get_incoming_rate( { "item_code": d.item_code, "warehouse": d.warehouse, diff --git a/erpnext/accounts/doctype/tax_rule/tax_rule.py b/erpnext/accounts/doctype/tax_rule/tax_rule.py index fb91fef654e..80532a6cc8d 100644 --- a/erpnext/accounts/doctype/tax_rule/tax_rule.py +++ b/erpnext/accounts/doctype/tax_rule/tax_rule.py @@ -145,6 +145,12 @@ def get_party_details(party: str | None, party_type: str, args: dict | None = No out = {} billing_address, shipping_address = None, None if args: + # each of these names a single Address. A dict is read as a filter instead, and `get_doc` + # would resolve it to whichever Address happens to match, so only a plain name is accepted + for fieldname in ("billing_address", "shipping_address"): + if args.get(fieldname) and not isinstance(args.get(fieldname), str): + frappe.throw(_("Invalid address"), frappe.PermissionError) + if args.get("billing_address"): billing_address = frappe.get_doc("Address", args.get("billing_address")) if args.get("shipping_address"): diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index 087eaa5bef1..ea42589bbd1 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -16,7 +16,7 @@ from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import ( ) from erpnext.accounts.report.financial_statements import get_cost_centers_with_children from erpnext.stock.report.stock_ledger.stock_ledger import get_item_group_condition -from erpnext.stock.utils import get_incoming_rate +from erpnext.stock.utils import _get_incoming_rate def execute(filters=None): @@ -969,7 +969,7 @@ class GrossProfitGenerator: if row.serial_and_batch_bundle: args.update({"serial_and_batch_bundle": row.serial_and_batch_bundle}) - average_buying_rate = get_incoming_rate(args) + average_buying_rate = _get_incoming_rate(args) self.average_buying_rate[key] = flt(average_buying_rate) return self.average_buying_rate[key] diff --git a/erpnext/accounts/services/taxes.py b/erpnext/accounts/services/taxes.py index ebccf91f0a3..7700fd33d61 100644 --- a/erpnext/accounts/services/taxes.py +++ b/erpnext/accounts/services/taxes.py @@ -181,6 +181,27 @@ class TaxService: return amount, base_amount +# the only doctypes a `taxes_and_charges` Link points at; `master_doctype` is caller-supplied and reaches get_doc() +TAX_MASTER_DOCTYPES = ("Sales Taxes and Charges Template", "Purchase Taxes and Charges Template") + + +def validate_tax_master(master_doctype: str, master_name: str | None = None) -> None: + if master_doctype not in TAX_MASTER_DOCTYPES: + frappe.throw(_("Invalid tax master doctype"), frappe.PermissionError) + + if not master_name: + return + + # keep a company-restricted caller inside their own companies; this does NOT authorise the template itself + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, master_doctype) + if allowed_companies: + company = frappe.db.get_value(master_doctype, master_name, "company") + if company and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + + @frappe.whitelist() def get_tax_rate(account_head: str) -> dict: return frappe.get_cached_value("Account", account_head, ["tax_rate", "account_name"], as_dict=True) @@ -193,6 +214,8 @@ def get_default_taxes_and_charges( if not company: return {} + validate_tax_master(master_doctype, tax_template) + if tax_template and company: tax_template_company = frappe.get_cached_value(master_doctype, tax_template, "company") if tax_template_company == company: @@ -210,6 +233,9 @@ def get_default_taxes_and_charges( def get_taxes_and_charges(master_doctype: str, master_name: str | None = None) -> list | None: if not master_name: return + + validate_tax_master(master_doctype, master_name) + from frappe.model import child_table_fields, default_fields tax_master = frappe.get_doc(master_doctype, master_name) diff --git a/erpnext/assets/doctype/asset/asset.py b/erpnext/assets/doctype/asset/asset.py index 0f8566e3a28..0e6caea7b1f 100644 --- a/erpnext/assets/doctype/asset/asset.py +++ b/erpnext/assets/doctype/asset/asset.py @@ -1171,15 +1171,36 @@ def get_asset_value_after_depreciation( asset_name: str, finance_book: str | None = None, ): + # one of the three calling forms is the boundary; Asset itself excludes the roles holding Asset Value Adjustment write + if not any( + frappe.has_permission(dt, "write") + for dt in ("Asset Value Adjustment", "Asset Capitalization", "Asset Repair") + ): + frappe.throw(_("Not permitted"), frappe.PermissionError) + asset = frappe.get_doc("Asset", asset_name) + + _check_asset_company(asset.company) + if not asset.calculate_depreciation: return flt(asset.value_after_depreciation) return asset.get_value_after_depreciation(finance_book) +def _check_asset_company(company: str | None) -> None: + """Keep a company-restricted caller inside their own companies; a no-op for everyone else.""" + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "Asset") + if allowed_companies and company and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + + @frappe.whitelist() def has_active_capitalization(asset: str): + frappe.has_permission("Asset", doc=asset, throw=True) + active_capitalizations = frappe.db.count( "Asset Capitalization", filters={"target_asset": asset, "docstatus": 1} ) @@ -1192,7 +1213,21 @@ def get_values_from_purchase_doc( item_code: str, doctype: str, ): + # `doctype` is caller-supplied and reaches frappe.get_doc() as the doctype itself, so without + # this list any document with an `items` table could be read for its valuation rates. The two + # values below are the only ones this function handles — see the branches further down. + if doctype not in ("Purchase Receipt", "Purchase Invoice"): + frappe.throw(_("Invalid document type"), frappe.PermissionError) + + # The caller is filling in an Asset (asset.js:794), and the Asset form is the boundary: Quality + # Manager writes Assets but holds read on neither Purchase Receipt nor Purchase Invoice, so the + # purchase document cannot be it. + frappe.has_permission("Asset", "write", throw=True) + purchase_doc = frappe.get_doc(doctype, purchase_doc_name) + + _check_asset_company(purchase_doc.company) + matching_items = [item for item in purchase_doc.items if item.item_code == item_code] if not matching_items: diff --git a/erpnext/assets/doctype/asset_capitalization/asset_capitalization.py b/erpnext/assets/doctype/asset_capitalization/asset_capitalization.py index 19532b8bbe5..4684b524bb3 100644 --- a/erpnext/assets/doctype/asset_capitalization/asset_capitalization.py +++ b/erpnext/assets/doctype/asset_capitalization/asset_capitalization.py @@ -29,7 +29,7 @@ from erpnext.stock.get_item_details import ( get_item_warehouse_, ) from erpnext.stock.stock_ledger import get_previous_sle -from erpnext.stock.utils import get_incoming_rate +from erpnext.stock.utils import _get_incoming_rate, check_warehouse_company force_fields = [ "target_item_name", @@ -191,7 +191,7 @@ class AssetCapitalization(StockController): cumulative_qty += flt(d.stock_qty) args = self.get_args_for_incoming_rate(d) args["qty"] = -1 * cumulative_qty - cumulative_rate = flt(get_incoming_rate(args, raise_error_if_no_rate=False)) + cumulative_rate = flt(_get_incoming_rate(args, raise_error_if_no_rate=False)) cumulative_value = cumulative_rate * cumulative_qty row_value = cumulative_value - prev_cumulative_value @@ -326,6 +326,8 @@ class AssetCapitalization(StockController): @frappe.whitelist() def set_warehouse_details(self): + self.check_permission("write") + for d in self.get("stock_items"): if d.item_code and d.warehouse: args = self.get_args_for_incoming_rate(d) @@ -336,6 +338,8 @@ class AssetCapitalization(StockController): @frappe.whitelist() def set_asset_values(self): + self.check_permission("write") + for d in self.get("asset_items"): if d.asset: finance_book = d.get("finance_book") or self.get("finance_book") @@ -511,8 +515,24 @@ class AssetCapitalization(StockController): ) +def check_capitalization_access(company: str | None = None) -> None: + """Every lookup in this file feeds the Asset Capitalization form, so that form is the boundary.""" + frappe.has_permission("Asset Capitalization", throw=True) + + if not isinstance(company, str) or not company: + return + + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "Asset Capitalization") + if allowed_companies and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + + @frappe.whitelist() def get_target_item_details(item_code: str | None = None, company: str | None = None): + check_capitalization_access(company) + out = frappe._dict() # Get Item Details @@ -539,6 +559,8 @@ def get_target_item_details(item_code: str | None = None, company: str | None = @frappe.whitelist() def get_target_asset_details(asset: str | None = None, company: str | None = None): + check_capitalization_access(company) + out = frappe._dict() # Get Asset Details @@ -624,10 +646,12 @@ def get_warehouse_details(ctx: ItemDetailsCtx) -> frappe._dict: frappe.has_permission("Item", doc=ctx.item_code, throw=True) frappe.has_permission("Warehouse", doc=ctx.warehouse, throw=True) frappe.has_permission("Stock Ledger Entry", throw=True) + # inherited from get_incoming_rate before the split; _get_incoming_rate does not scope + check_warehouse_company(ctx.warehouse) out = frappe._dict( { "actual_qty": get_previous_sle(ctx).get("qty_after_transaction") or 0, - "valuation_rate": get_incoming_rate(ctx, raise_error_if_no_rate=False), + "valuation_rate": _get_incoming_rate(ctx, raise_error_if_no_rate=False), } ) return out @@ -636,6 +660,8 @@ def get_warehouse_details(ctx: ItemDetailsCtx) -> frappe._dict: @frappe.whitelist() @erpnext.normalize_ctx_input(ItemDetailsCtx) def get_consumed_asset_details(ctx: ItemDetailsCtx) -> frappe._dict: + check_capitalization_access(ctx.get("company")) + out = frappe._dict() asset_details = frappe._dict() @@ -682,6 +708,8 @@ def get_consumed_asset_details(ctx: ItemDetailsCtx) -> frappe._dict: @frappe.whitelist() @erpnext.normalize_ctx_input(ItemDetailsCtx) def get_service_item_details(ctx: ItemDetailsCtx) -> frappe._dict: + check_capitalization_access(ctx.get("company")) + out = frappe._dict() item = frappe._dict() @@ -706,6 +734,8 @@ def get_service_item_details(ctx: ItemDetailsCtx) -> frappe._dict: def get_items_tagged_to_wip_composite_asset(params: dict | str): params = frappe.parse_json(params) + check_capitalization_access(params.get("company") if isinstance(params, dict | frappe._dict) else None) + fields = [ "item_code", "item_name", diff --git a/erpnext/assets/doctype/asset_repair/asset_repair.py b/erpnext/assets/doctype/asset_repair/asset_repair.py index cba62683c8e..e90a2d62327 100644 --- a/erpnext/assets/doctype/asset_repair/asset_repair.py +++ b/erpnext/assets/doctype/asset_repair/asset_repair.py @@ -351,6 +351,20 @@ class AssetRepair(AccountsController): add_asset_activity(self.asset, subject) +def check_asset_repair_access(company: str | None = None) -> None: + """Both pickers below sit on the Asset Repair form, so that form is the boundary, not Purchase Invoice.""" + frappe.has_permission("Asset Repair", throw=True) + + if not isinstance(company, str) or not company: + return + + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "Asset Repair") + if allowed_companies and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + + @frappe.whitelist() def get_downtime(failure_date: DateTimeLikeObject, completion_date: DateTimeLikeObject): downtime = time_diff_in_hours(completion_date, failure_date) @@ -371,6 +385,8 @@ def get_purchase_invoice( Get Purchase Invoices that have expense accounts for non-stock items. Only returns invoices with at least one non-stock, non-fixed-asset item with an expense account. """ + check_asset_repair_access(filters.get("company") if isinstance(filters, dict) else None) + pi = DocType("Purchase Invoice") pi_item = DocType("Purchase Invoice Item") item = DocType("Item") @@ -413,6 +429,8 @@ def get_expense_accounts( Get expense accounts for non-stock (service) items from the purchase invoice. Used as a query function for link fields. """ + check_asset_repair_access() + purchase_invoice = filters.get("purchase_invoice") if not purchase_invoice: return [] diff --git a/erpnext/buying/doctype/purchase_order/purchase_order.py b/erpnext/buying/doctype/purchase_order/purchase_order.py index 658113a597e..b1430423924 100644 --- a/erpnext/buying/doctype/purchase_order/purchase_order.py +++ b/erpnext/buying/doctype/purchase_order/purchase_order.py @@ -611,14 +611,19 @@ def item_last_purchase_rate(name, conversion_rate, item_code, conversion_factor= return item_last_purchase_rate -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def close_or_unclose_purchase_orders(names: str | list, status: str): - if not frappe.has_permission("Purchase Order", "write"): - frappe.throw(_("Not permitted"), frappe.PermissionError) + frappe.has_permission("Purchase Order", "write", throw=True) names = frappe.parse_json(names) for name in names: - po = frappe.get_lazy_doc("Purchase Order", name) + if not isinstance(name, str): + frappe.throw(_("Invalid name"), frappe.PermissionError) + + # the check above is doctype level and never consults User Permissions, so on its own it + # lets a caller restricted to one company close another company's orders. Checking each + # document is what scopes it, and matches what update_status() below already does. + po = frappe.get_lazy_doc("Purchase Order", name, check_permission="submit") if po.docstatus == 1: if status == "Closed": if po.status not in ("Cancelled", "Closed") and ( @@ -649,7 +654,7 @@ def get_list_context(context=None): return list_context -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def update_status(status: str, name: str): po = frappe.get_lazy_doc("Purchase Order", name, check_permission="submit") po.update_status(status) diff --git a/erpnext/buying/doctype/request_for_quotation/request_for_quotation.py b/erpnext/buying/doctype/request_for_quotation/request_for_quotation.py index 7818e4cc2fc..02650930c5b 100644 --- a/erpnext/buying/doctype/request_for_quotation/request_for_quotation.py +++ b/erpnext/buying/doctype/request_for_quotation/request_for_quotation.py @@ -8,7 +8,6 @@ from frappe.contacts.doctype.contact.contact import get_full_name from frappe.core.doctype.communication.email import make from frappe.desk.form.load import get_attachments from frappe.model.document import Document -from frappe.query_builder import Order from frappe.utils import get_url from frappe.utils.print_format import download_pdf from frappe.utils.user import get_user_fullname @@ -481,32 +480,34 @@ def get_supplier_tag(): def get_rfq_containing_supplier( doctype: str | None, txt: str, searchfield: str | None, start: int, page_len: int, filters: dict ): - rfq = frappe.qb.DocType("Request for Quotation") - rfq_supplier = frappe.qb.DocType("Request for Quotation Supplier") + rfq_filters = [ + ["docstatus", "=", 1], + ["company", "=", filters.get("company")], + ] - query = ( - frappe.qb.from_(rfq) - .from_(rfq_supplier) - .select(rfq.name) - .distinct() - .select(rfq.transaction_date, rfq.company) - .where( - (rfq.name == rfq_supplier.parent) - & (rfq_supplier.supplier == filters.get("supplier")) - & (rfq.docstatus == 1) - & (rfq.company == filters.get("company")) + if frappe.has_permission("Request for Quotation", "read"): + rfq_filters.append(["Request for Quotation Supplier", "supplier", "=", filters.get("supplier")]) + else: + parents = frappe.get_all( + "Request for Quotation Supplier", + filters={"supplier": filters.get("supplier"), "parenttype": "Request for Quotation"}, + pluck="parent", + distinct=True, ) - .orderby(rfq.transaction_date, order=Order.asc) - .limit(page_len) - .offset(start) - ) + rfq_filters.append(["name", "in", parents or [""]]) if txt: - query = query.where(rfq.name.like(f"%%{txt}%%")) + rfq_filters.append(["name", "like", f"%{txt}%"]) if filters.get("transaction_date"): - query = query.where(rfq.transaction_date == filters.get("transaction_date")) + rfq_filters.append(["transaction_date", "=", filters.get("transaction_date")]) - rfq_data = query.run(as_dict=1) - - return rfq_data + return frappe.get_list( + "Request for Quotation", + filters=rfq_filters, + fields=["name", "transaction_date", "company"], + group_by="name", + order_by="transaction_date asc", + limit_start=start, + limit_page_length=page_len, + ) diff --git a/erpnext/buying/doctype/supplier/supplier.py b/erpnext/buying/doctype/supplier/supplier.py index 121dd8df9c8..21467a0e2c3 100644 --- a/erpnext/buying/doctype/supplier/supplier.py +++ b/erpnext/buying/doctype/supplier/supplier.py @@ -242,6 +242,16 @@ def get_supplier_primary( ): supplier = filters.get("supplier") type = filters.get("type") + + # `type` is caller-supplied and was interpolated straight into qb.DocType(), so any doctype on + # the site could be joined to Dynamic Link and read. The two pickers that call this + # (supplier.js:51,61) send only these two values. + if type not in ("Contact", "Address"): + frappe.throw(_("Invalid type"), frappe.PermissionError) + + # authorise the party, not Contact/Address: the `if_owner` row on Address would empty the picker rather than error + frappe.has_permission("Supplier", doc=supplier, throw=True) + type_doctype = frappe.qb.DocType(type) dynamic_link = frappe.qb.DocType("Dynamic Link") diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index f9d84d118d8..4379c65e038 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -23,7 +23,7 @@ from erpnext.stock.get_item_details import ( get_conversion_factor, get_item_defaults, ) -from erpnext.stock.utils import get_incoming_rate +from erpnext.stock.utils import _get_incoming_rate class QtyMismatchError(ValidationError): @@ -180,7 +180,7 @@ class BuyingController(SubcontractingController): for row in self.items: if row.rate <= 0: # override the rate with valuation rate - row.rate = get_incoming_rate( + row.rate = _get_incoming_rate( { "item_code": row.item_code, "warehouse": row.warehouse, @@ -664,7 +664,7 @@ class BuyingController(SubcontractingController): if not posting_time: posting_time = nowtime() - outgoing_rate = get_incoming_rate( + outgoing_rate = _get_incoming_rate( { "item_code": d.item_code, "warehouse": d.get("from_warehouse"), diff --git a/erpnext/controllers/item_variant.py b/erpnext/controllers/item_variant.py index 3c4f4cb5dfd..d1be9bbe06a 100644 --- a/erpnext/controllers/item_variant.py +++ b/erpnext/controllers/item_variant.py @@ -40,6 +40,11 @@ def get_variant( :param item: Template Item :param args: A dictionary with "Attribute" as key and "Attribute Value" as value """ + # The client callers are the Item form (item.js:1144, 1483), so the template Item is the boundary + # and `read` is loser-free: the roles that cannot read Item cannot open that form either. The two + # server-side callers (item.py:1083 on Item save, item_variant.py:393) already hold the template. + frappe.has_permission("Item", doc=template, throw=True) + item_template = frappe.get_doc("Item", template) if item_template.variant_based_on == "Manufacturer" and manufacturer: @@ -318,6 +323,12 @@ def find_variant(template, args, variant_item_code=None): @frappe.whitelist() def create_variant(item: str, args: dict | str, use_template_image: bool = False): + # Same right its sibling enqueue_multiple_variant_creation already requires — this builds an + # Item the caller is about to insert (item.js:1511) — plus record-level read on the template + # it copies from. + frappe.has_permission("Item", ptype="create", throw=True) + frappe.has_permission("Item", doc=item, throw=True) + use_template_image = frappe.parse_json(use_template_image) args = frappe.parse_json(args) @@ -342,7 +353,7 @@ def create_variant(item: str, args: dict | str, use_template_image: bool = False return variant -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def enqueue_multiple_variant_creation(item: str, args: dict | str, use_template_image: bool = False): use_template_image = frappe.parse_json(use_template_image) # There can be innumerable attribute combinations, enqueue @@ -539,6 +550,10 @@ def make_variant_item_code(template_item_code, template_item_name, variant): @frappe.whitelist() def create_variant_doc_for_quick_entry(template: str, args: dict | str): + # Delegates to get_variant and create_variant below, which carry their own checks; this one + # fails fast rather than relying on that delegation. + frappe.has_permission("Item", doc=template, throw=True) + variant_based_on = frappe.db.get_value("Item", template, "variant_based_on") args = frappe.parse_json(args) if variant_based_on == "Manufacturer": diff --git a/erpnext/controllers/queries.py b/erpnext/controllers/queries.py index 8d72144925f..f93f5814cd0 100644 --- a/erpnext/controllers/queries.py +++ b/erpnext/controllers/queries.py @@ -482,49 +482,71 @@ def get_project_name( doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict | None = None ): proj = qb.DocType("Project") - qb_filter_and_conditions = [] - qb_filter_or_conditions = [] + meta = frappe.get_meta(doctype) + + list_filters = [["status", "not in", ["Completed", "Cancelled", "On hold"]]] if filters: if filters.get("customer"): - qb_filter_and_conditions.append( - (proj.customer == filters.get("customer")) | (proj.customer.isnull()) | (proj.customer == "") - ) + # an `in` containing "" renders as `ifnull(customer,'') in (...)`: this customer, or none + list_filters.append(["customer", "in", [filters.get("customer"), ""]]) if filters.get("company"): - qb_filter_and_conditions.append(proj.company == filters.get("company")) - - qb_filter_and_conditions.append(proj.status.notin(["Completed", "Cancelled", "On hold"])) - - q = qb.from_(proj) - - fields = get_fields(doctype, ["name", "project_name"]) - for x in fields: - q = q.select(proj[x]) + list_filters.append(["company", "=", filters.get("company")]) # don't consider 'customer' and 'status' fields for pattern search, as they must be exactly matched + # permlevel fields go too: get_list refuses to filter on one, which would fail the whole call searchfields = [ - x for x in frappe.get_meta(doctype).get_search_fields() if x not in ["customer", "status"] + x + for x in meta.get_search_fields() + if x not in ["customer", "status"] and not (meta.get_field(x) and meta.get_field(x).permlevel) ] - # pattern search - if txt: - for x in searchfields: - qb_filter_or_conditions.append(proj[x].like(f"%{txt}%")) + fields = get_fields(doctype, ["name", "project_name"]) - q = q.where(Criterion.all(qb_filter_and_conditions)).where(Criterion.any(qb_filter_or_conditions)) - - # ordering - if txt: - # project_name containing search string 'txt' will be given higher precedence - q = q.orderby( - Case() - .when( - Locate(Lower(txt), Lower(proj.project_name)) > 0, - Locate(Lower(txt), Lower(proj.project_name)), - ) - .else_(99999) + # get_list applies the doctype check and the caller's record-level conditions + if not txt: + # no search term means no relevance ordering, so the whole query is expressible here and + # stays paginated in SQL rather than materialising every permitted name + return frappe.get_list( + "Project", + filters=list_filters, + fields=fields, + order_by="idx desc, name", + limit_start=start, + limit_page_length=page_len, + as_list=True, ) + + # with a search term, resolve the (already LIKE-narrowed) permitted names and rank them below: + # the relevance ordering is a CASE expression, which `order_by` rejects + permitted = frappe.get_list( + "Project", + filters=list_filters, + or_filters=[[x, "like", f"%{txt}%"] for x in searchfields], + pluck="name", + order_by="", + limit_page_length=0, + ) + + if not permitted: + return [] + + q = ( + frappe.qb.from_(proj) + .select(*[proj[fieldname] for fieldname in fields]) + .where(proj.name.isin(permitted)) + ) + + # project_name containing search string 'txt' will be given higher precedence + q = q.orderby( + Case() + .when( + Locate(Lower(txt), Lower(proj.project_name)) > 0, + Locate(Lower(txt), Lower(proj.project_name)), + ) + .else_(99999) + ) q = q.orderby(proj.idx, order=Order.desc).orderby(proj.name) if page_len: @@ -798,28 +820,33 @@ def get_account_list( @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_blanket_orders(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): - bo = frappe.qb.DocType("Blanket Order") - bo_item = frappe.qb.DocType("Blanket Order Item") + bo_filters = [ + ["docstatus", "=", 1], + ["blanket_order_type", "=", filters.get("blanket_order_type")], + ["company", "=", filters.get("company")], + ] - query = ( - frappe.qb.from_(bo) - .from_(bo_item) - .select(bo.name) - .distinct() - .select(bo.blanket_order_type, bo.to_date) - .where( - (bo_item.parent == bo.name) - & (bo_item.item_code == filters.get("item")) - & (bo.blanket_order_type == filters.get("blanket_order_type")) - & (bo.company == filters.get("company")) - & (bo.docstatus == 1) + if frappe.has_permission("Blanket Order", "read"): + bo_filters.append(["Blanket Order Item", "item_code", "=", filters.get("item")]) + else: + parents = frappe.get_all( + "Blanket Order Item", + filters={"item_code": filters.get("item"), "parenttype": "Blanket Order"}, + pluck="parent", + distinct=True, ) - ) + bo_filters.append(["name", "in", parents or [""]]) if currency := filters.get("currency"): - query = query.where(bo.currency == currency) + bo_filters.append(["currency", "=", currency]) - return query.run() + return frappe.get_list( + "Blanket Order", + filters=bo_filters, + fields=["name", "blanket_order_type", "to_date"], + group_by="name", + as_list=True, + ) @frappe.whitelist() @@ -1015,21 +1042,22 @@ def get_doctype_wise_filters(filters): @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_batch_numbers(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): - batch = frappe.qb.DocType("Batch") - query = ( - frappe.qb.from_(batch) - .select(batch.batch_id) - .where( - (batch.disabled == 0) - & (batch.expiry_date.isnull() | (batch.expiry_date >= today())) - & batch.name.like(f"%{txt}%") - ) - ) + # get_list applies the select check and the caller's record-level conditions together + batch_filters = [["disabled", "=", 0], ["name", "like", f"%{txt}%"]] if filters and filters.get("item"): - query = query.where(batch.item == filters.get("item")) + batch_filters.append(["item", "=", filters.get("item")]) - return query.orderby(batch.batch_id).limit(page_len).offset(start).run() + return frappe.get_list( + "Batch", + filters=batch_filters, + or_filters=[["expiry_date", "is", "not set"], ["expiry_date", ">=", today()]], + fields=["batch_id"], + order_by="batch_id", + limit_start=start, + limit_page_length=page_len, + as_list=True, + ) @frappe.whitelist() @@ -1056,41 +1084,71 @@ def item_manufacturer_query( @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_purchase_receipts(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): - pr = frappe.qb.DocType("Purchase Receipt") - pr_item = frappe.qb.DocType("Purchase Receipt Item") - query = ( - frappe.qb.from_(pr) - .inner_join(pr_item) - .on(pr_item.parent == pr.name) - .select(pr.name) - .distinct() # one row per receipt, not per matching item line - .where((pr.docstatus == 1) & pr.name.like(f"%{txt}%")) - ) + pr_filters = [["docstatus", "=", 1], ["name", "like", f"%{txt}%"]] if filters and filters.get("item_code"): - query = query.where(pr_item.item_code == filters.get("item_code")) + if frappe.has_permission("Purchase Receipt", "read"): + # one indexed join, deduped by group_by below + pr_filters.append(["Purchase Receipt Item", "item_code", "=", filters.get("item_code")]) + else: + # a select-only caller may use this picker but may not filter on a child table, so resolve + # the parents separately rather than losing the filter to a PermissionError + parents = frappe.get_all( + "Purchase Receipt Item", + filters={"item_code": filters.get("item_code"), "parenttype": "Purchase Receipt"}, + pluck="parent", + distinct=True, + ) + pr_filters.append(["name", "in", parents or [""]]) - return query.orderby(pr.name).limit(page_len).offset(start).run() + # get_list applies the select check and the caller's record-level conditions together. + # group_by, not distinct: it dedupes the child join just the same, and frappe drops ORDER BY + # from a distinct query on Postgres + return frappe.get_list( + "Purchase Receipt", + filters=pr_filters, + fields=["name"], + group_by="name", + order_by="name", + limit_start=start, + limit_page_length=page_len, + as_list=True, + ) @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_purchase_invoices(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): - pi = frappe.qb.DocType("Purchase Invoice") - pi_item = frappe.qb.DocType("Purchase Invoice Item") - query = ( - frappe.qb.from_(pi) - .inner_join(pi_item) - .on(pi_item.parent == pi.name) - .select(pi.name) - .distinct() # one row per invoice, not per matching item line - .where((pi.docstatus == 1) & pi.name.like(f"%{txt}%")) - ) + pi_filters = [["docstatus", "=", 1], ["name", "like", f"%{txt}%"]] if filters and filters.get("item_code"): - query = query.where(pi_item.item_code == filters.get("item_code")) + if frappe.has_permission("Purchase Invoice", "read"): + # one indexed join, deduped by group_by below + pi_filters.append(["Purchase Invoice Item", "item_code", "=", filters.get("item_code")]) + else: + # a select-only caller may use this picker but may not filter on a child table, so resolve + # the parents separately rather than losing the filter to a PermissionError + parents = frappe.get_all( + "Purchase Invoice Item", + filters={"item_code": filters.get("item_code"), "parenttype": "Purchase Invoice"}, + pluck="parent", + distinct=True, + ) + pi_filters.append(["name", "in", parents or [""]]) - return query.orderby(pi.name).limit(page_len).offset(start).run() + # get_list applies the select check and the caller's record-level conditions together. + # group_by, not distinct: it dedupes the child join just the same, and frappe drops ORDER BY + # from a distinct query on Postgres + return frappe.get_list( + "Purchase Invoice", + filters=pi_filters, + fields=["name"], + group_by="name", + order_by="name", + limit_start=start, + limit_page_length=page_len, + as_list=True, + ) @frappe.whitelist() @@ -1177,9 +1235,30 @@ def get_payment_terms_for_references( ): terms = [] if filters: + reference = filters.get("reference") + if not reference: + return terms + + # only a plain name names one document: a filter operator (["like", "%"], ["!=", ""]) would + # widen this past the document the caller named, and past the one being authorised below + if not isinstance(reference, str): + frappe.throw(_("Invalid reference"), frappe.PermissionError) + + # Payment Schedule is a child table and carries no permissions of its own, so the + # document the schedule belongs to is what decides access to these rows + # prefer the caller's own reference type; the lookup below cannot tell two parents of + # different types apart when they share a name + parenttype = filters.get("reference_doctype") or frappe.db.get_value( + "Payment Schedule", {"parent": reference}, "parenttype" + ) + if not parenttype: + return terms + + frappe.has_permission(parenttype, doc=reference, throw=True) + terms = frappe.db.get_all( "Payment Schedule", - filters={"parent": filters.get("reference")}, + filters={"parent": reference, "parenttype": parenttype}, fields=["payment_term"], limit=page_len, as_list=1, @@ -1192,6 +1271,31 @@ def get_payment_terms_for_references( def get_filtered_child_rows( doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict ): + parent = filters.get("parent") if filters else None + + # a plain name, never a filter operator: ["like", "%"] here would span parents, and only one + # of them would be the document authorised below + if not parent or not isinstance(parent, str): + frappe.throw(_("Parent document is required to search child rows"), frappe.PermissionError) + + # `doctype` is caller supplied, so it has to be a child table before it is worth checking: + # any other doctype would put the caller's filters on a table this query never meant to read + if not frappe.get_meta(doctype).istable: + frappe.throw(_("{0} is not a child table").format(doctype), frappe.PermissionError) + + # child tables carry no permissions of their own, so the document the rows hang off is what + # decides access. Read the parent type off the rows rather than off `filters`, so that the + # document being authorised is always the one being returned. + parenttype = frappe.db.get_value(doctype, {"parent": parent}, "parenttype") + + if not parenttype or not frappe.db.exists(parenttype, parent): + return [] + + frappe.has_permission(doctype, parent_doctype=parenttype, throw=True) + + # and on the parent record itself, so that User Permissions still apply + frappe.has_permission(parenttype, doc=parent, throw=True) + table = frappe.qb.DocType(doctype) query = ( frappe.get_query(table, filters=filters) @@ -1217,7 +1321,11 @@ def get_filtered_child_rows( @frappe.validate_and_sanitize_search_inputs def get_item_uom_query(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): if frappe.get_single_value("Stock Settings", "allow_uom_with_conversion_rate_defined_in_item"): - query_filters = {"parent": filters.get("item_code")} + item_code = filters.get("item_code") + if not item_code or not frappe.get_list("Item", filters=[["name", "=", item_code]], pluck="name"): + return [] + + query_filters = {"parent": item_code, "parenttype": "Item"} if txt: query_filters["uom"] = ["like", f"%{txt}%"] @@ -1232,7 +1340,7 @@ def get_item_uom_query(doctype: str, txt: str, searchfield: str, start: int, pag as_list=1, ) - return frappe.get_all( + return frappe.get_list( "UOM", filters={"name": ["like", f"%{txt}%"], "enabled": 1}, fields=["name"], diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py index e095a1501a4..7945170cf54 100644 --- a/erpnext/controllers/sales_and_purchase_return.py +++ b/erpnext/controllers/sales_and_purchase_return.py @@ -12,7 +12,7 @@ from frappe.utils import cint, flt, format_datetime, get_datetime import erpnext from erpnext.stock.serial_batch_bundle import get_batches_from_bundle -from erpnext.stock.utils import get_combine_datetime, get_incoming_rate, get_valuation_method, getdate +from erpnext.stock.utils import _get_incoming_rate, get_combine_datetime, get_valuation_method, getdate class StockOverReturnError(frappe.ValidationError): @@ -821,7 +821,7 @@ def get_rate_for_return( rate = frappe.db.get_value(f"{voucher_type} Item", voucher_detail_no, "incoming_rate") if rate is None and sle: - rate = get_incoming_rate( + rate = _get_incoming_rate( { "item_code": sle.item_code, "warehouse": sle.warehouse, @@ -1309,14 +1309,38 @@ def get_available_serial_nos(serial_nos, warehouse): ) +# the only doctypes these endpoints are called for; both reach get_value()/get_all() as the doctype itself +RETURNABLE_INVOICE_DOCTYPES = ("Sales Invoice", "POS Invoice") + + @frappe.whitelist() def get_payment_data(invoice: str): + # `invoice` may be either a Sales Invoice or a POS Invoice — both share the Sales Invoice + # Payment child table — so resolve which one it is before authorising rather than guessing. + parenttype = frappe.db.get_value("Sales Invoice Payment", {"parent": invoice}, "parenttype") + if not parenttype: + return [] + + if parenttype not in RETURNABLE_INVOICE_DOCTYPES: + frappe.throw(_("Invalid document type"), frappe.PermissionError) + + frappe.has_permission(parenttype, doc=invoice, throw=True) + payment = frappe.db.get_all("Sales Invoice Payment", {"parent": invoice}, ["mode_of_payment", "amount"]) return payment +def validate_returnable_invoice(doctype: str, invoice: str) -> None: + if doctype not in RETURNABLE_INVOICE_DOCTYPES: + frappe.throw(_("Invalid document type"), frappe.PermissionError) + + frappe.has_permission(doctype, doc=invoice, throw=True) + + @frappe.whitelist() def get_invoice_item_returned_qty(doctype: str, invoice: str, customer: str, item_row_name: str): + validate_returnable_invoice(doctype, invoice) + is_return, docstatus = frappe.db.get_value(doctype, invoice, ["is_return", "docstatus"]) if not is_return and docstatus == 1: return get_returned_qty_map_for_row(invoice, customer, item_row_name, doctype) @@ -1324,6 +1348,8 @@ def get_invoice_item_returned_qty(doctype: str, invoice: str, customer: str, ite @frappe.whitelist() def is_invoice_returnable(doctype: str, invoice: str): + validate_returnable_invoice(doctype, invoice) + is_return, docstatus, customer = frappe.db.get_value( doctype, invoice, ["is_return", "docstatus", "customer"] ) diff --git a/erpnext/controllers/selling_controller.py b/erpnext/controllers/selling_controller.py index 02670dd39de..4e1d455ce85 100644 --- a/erpnext/controllers/selling_controller.py +++ b/erpnext/controllers/selling_controller.py @@ -13,7 +13,7 @@ from erpnext.controllers.sales_and_purchase_return import get_rate_for_return, i from erpnext.controllers.stock_controller import StockController from erpnext.stock.doctype.item.item import set_item_default from erpnext.stock.get_item_details import get_bin_details, get_conversion_factor -from erpnext.stock.utils import get_combine_datetime, get_incoming_rate, get_valuation_method +from erpnext.stock.utils import _get_incoming_rate, get_combine_datetime, get_valuation_method class SellingController(StockController): @@ -588,7 +588,7 @@ class SellingController(StockController): and self.get("is_return") ) ): - d.incoming_rate = get_incoming_rate( + d.incoming_rate = _get_incoming_rate( { "item_code": d.item_code, "warehouse": d.warehouse, diff --git a/erpnext/controllers/subcontracting_controller.py b/erpnext/controllers/subcontracting_controller.py index a78b45f4d86..674fe812cd1 100644 --- a/erpnext/controllers/subcontracting_controller.py +++ b/erpnext/controllers/subcontracting_controller.py @@ -21,7 +21,7 @@ from erpnext.stock.doctype.serial_and_batch_bundle.serial_and_batch_bundle impor ) from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos from erpnext.stock.serial_batch_bundle import SerialBatchCreation, get_serial_nos_from_bundle -from erpnext.stock.utils import get_incoming_rate +from erpnext.stock.utils import _get_incoming_rate class SubcontractingController(StockController): @@ -89,7 +89,7 @@ class SubcontractingController(StockController): } ) - rate = get_incoming_rate(kwargs) + rate = _get_incoming_rate(kwargs) precision = frappe.get_precision("Subcontracting Receipt Supplied Item", "rate") if flt(rate, precision) != flt(row.rate, precision): row.rate = rate @@ -844,7 +844,7 @@ class SubcontractingController(StockController): args["batch_no"] = rm_obj.batch_no args["serial_no"] = rm_obj.serial_no - rm_obj.rate = get_incoming_rate(args) + rm_obj.rate = _get_incoming_rate(args) def __set_batch_nos(self, bom_item, item_row, rm_obj, qty): key = (rm_obj.rm_item_code, item_row.item_code, item_row.get(self.subcontract_data.order_field)) diff --git a/erpnext/crm/doctype/lead/mapper.py b/erpnext/crm/doctype/lead/mapper.py index 9ef3e2da0e5..582f3f77377 100644 --- a/erpnext/crm/doctype/lead/mapper.py +++ b/erpnext/crm/doctype/lead/mapper.py @@ -114,6 +114,16 @@ def make_quotation(source_name: str, target_doc: str | dict | Document | None = def make_lead_from_communication(communication: str, ignore_communication_links: bool = False): """raise a issue from email""" + # `communication` is caller supplied and nothing here checked it. Communication grants read to + # `All` only for the owner (if_owner) and carries a has_permission hook, so doc= is what decides + # access; the desk button only appears on an email the caller already has open. + frappe.has_permission("Communication", doc=communication, throw=True) + + # both paths below end in a Lead. The insert path checks `create` on its own, but the path that + # reuses an existing Lead required nothing, so it returned a Lead's name and linked the email + # for callers with no access to Leads at all. + frappe.has_permission("Lead", ptype="create", throw=True) + doc = frappe.get_doc("Communication", communication) lead_name = None if doc.sender: diff --git a/erpnext/crm/doctype/opportunity/mapper.py b/erpnext/crm/doctype/opportunity/mapper.py index 342b0e9a362..17497fad129 100644 --- a/erpnext/crm/doctype/opportunity/mapper.py +++ b/erpnext/crm/doctype/opportunity/mapper.py @@ -130,8 +130,15 @@ def make_opportunity_from_communication( ): from erpnext.crm.doctype.lead.mapper import make_lead_from_communication + # `communication` is caller supplied and nothing checked it. Communication grants read to `All` + # only for the owner (if_owner) and carries a has_permission hook, so doc= is what decides + # access; the desk button only appears on an email the caller already has open. + frappe.has_permission("Communication", doc=communication, throw=True) + doc = frappe.get_doc("Communication", communication) + # make_lead_from_communication() carries its own check, but it is skipped entirely when the + # email already references a Lead, so this cannot rely on it. lead = doc.reference_name if doc.reference_doctype == "Lead" else None if not lead: lead = make_lead_from_communication(communication, ignore_communication_links=True) diff --git a/erpnext/crm/utils.py b/erpnext/crm/utils.py index 1a0ccbba265..02762cc1279 100644 --- a/erpnext/crm/utils.py +++ b/erpnext/crm/utils.py @@ -150,6 +150,11 @@ def link_open_events(ref_doctype, ref_docname, doc): @frappe.whitelist() def get_open_activities(ref_doctype: str, ref_docname: str): + # both arguments are caller supplied and nothing below checked them: the ToDo and Event rows are + # read with get_all, so the referenced document is what decides who may see its activities. + # doc= applies User Permissions; the desk only asks this for a form the caller has open. + frappe.has_permission(ref_doctype, doc=ref_docname, throw=True) + tasks = get_open_todos(ref_doctype, ref_docname) events = get_open_events(ref_doctype, ref_docname) tasks_history = get_closed_todos(ref_doctype, ref_docname) diff --git a/erpnext/edi/doctype/code_list/code_list_import.py b/erpnext/edi/doctype/code_list/code_list_import.py index 0b8ec588b30..ac9ed51c909 100644 --- a/erpnext/edi/doctype/code_list/code_list_import.py +++ b/erpnext/edi/doctype/code_list/code_list_import.py @@ -20,8 +20,11 @@ class CodeListSelectionMismatchError(Exception): pass -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def import_genericode(): + # check before save(), which only runs after the XML is fetched and parsed; denies exactly who save() would, sooner + frappe.has_permission("Code List", "create", throw=True) + try: content, file_name = get_uploaded_genericode_file() @@ -149,7 +152,7 @@ def parse_genericode_content(content: bytes): return etree.fromstring(content, parser=parser) -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def process_genericode_import( code_list_name: str, file_name: str, @@ -160,6 +163,11 @@ def process_genericode_import( ): from erpnext.edi.doctype.common_code.common_code import import_genericode + # Same reasoning as above: common_code.save() enforces this per document, but only after the + # file has been read and its XML parsed and queried. + frappe.has_permission("Common Code", "create", throw=True) + frappe.has_permission("Code List", doc=code_list_name, throw=True) + column_map = {"code": code_column, "title": title_column, "description": description_column} return import_genericode( diff --git a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py index a88340e6cee..473eb8c9245 100644 --- a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py +++ b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py @@ -37,8 +37,12 @@ class PlaidSettings(Document): return plaid.get_link_token() -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def get_plaid_configuration(): + # Returns plaid_env and a freshly minted Plaid link_token. Plaid Settings is a System-Manager-only + # single doctype and every caller reaches this from its own form, so that is the boundary. + frappe.has_permission("Plaid Settings", throw=True) + if frappe.db.get_single_value("Plaid Settings", "enabled"): plaid_settings = frappe.get_single("Plaid Settings") return { @@ -52,6 +56,8 @@ def get_plaid_configuration(): @frappe.whitelist(methods=["POST"]) def add_institution(token: str, response: str | dict): + frappe.has_permission("Plaid Settings", throw=True) + response = frappe.parse_json(response) plaid = PlaidConnector() @@ -81,6 +87,8 @@ def add_institution(token: str, response: str | dict): @frappe.whitelist(methods=["POST"]) def add_bank_accounts(response: str | dict, bank: str | dict, company: str): + frappe.has_permission("Plaid Settings", throw=True) + response = frappe.parse_json(response) bank = frappe.parse_json(bank) result = [] @@ -328,8 +336,10 @@ def automatic_synchronization(): enqueue_synchronization() -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def enqueue_synchronization(): + frappe.has_permission("Plaid Settings", throw=True) + plaid_accounts = frappe.get_all( "Bank Account", filters={"integration_id": ["!=", ""]}, fields=["name", "bank"] ) @@ -342,8 +352,12 @@ def enqueue_synchronization(): ) -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def get_link_token_for_update(access_token: str): + # `access_token` is caller-supplied and is used to mint a link token at Plaid, so this creates + # state at the provider even though it writes nothing here. + frappe.has_permission("Plaid Settings", throw=True) + plaid = PlaidConnector(access_token) return plaid.get_link_token(update_mode=True) @@ -362,8 +376,10 @@ def get_company(bank_account_name): frappe.throw(_("Could not detect the Company for updating Bank Accounts")) -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def update_bank_account_ids(response: str | dict): + frappe.has_permission("Plaid Settings", throw=True) + data = frappe.parse_json(response) institution_name = data["institution"]["name"] bank = frappe.get_doc("Bank", institution_name).as_dict() diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 9e5ac40e502..0cdc29b35fa 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -15,7 +15,7 @@ from frappe.website.website_generator import WebsiteGenerator import erpnext from erpnext.setup.utils import get_exchange_rate -from erpnext.stock.doctype.item.item import get_item_details +from erpnext.stock.doctype.item.item import _get_item_details from erpnext.stock.get_item_details import get_conversion_factor, get_price_list_rate form_grid_templates = {"items": "templates/form_grid/item_grid.html"} @@ -564,7 +564,7 @@ class BOM(WebsiteGenerator): self.manage_default_bom() def get_item_det(self, item_code): - item = get_item_details(item_code) + item = _get_item_details(item_code) if not item: frappe.throw(_("Item: {0} does not exist in the system").format(item_code)) @@ -972,7 +972,7 @@ class BOM(WebsiteGenerator): def _add_raw_material_row(self, operation_row_id, row): row = parse_json(row) - row.update(get_item_details(row.get("item_code"))) + row.update(_get_item_details(row.get("item_code"))) row.operation_row_id = operation_row_id item_row = self.get_item_data(row.item_code, operation_row_id) diff --git a/erpnext/manufacturing/doctype/bom/mapper.py b/erpnext/manufacturing/doctype/bom/mapper.py index e99e994f355..fea01196db1 100644 --- a/erpnext/manufacturing/doctype/bom/mapper.py +++ b/erpnext/manufacturing/doctype/bom/mapper.py @@ -14,7 +14,7 @@ from frappe.query_builder import Field from frappe.query_builder.functions import IfNull from frappe.utils import today -from erpnext.stock.doctype.item.item import get_item_details +from erpnext.stock.doctype.item.item import _get_item_details _BOM_DIFF_IDENTIFIERS = { "operations": "operation", @@ -195,7 +195,7 @@ def make_variant_bom( def _postprocess_variant_bom(source, doc, item, variant_items, source_name): from erpnext.manufacturing.doctype.work_order.work_order import add_variant_item - item_data = get_item_details(item) + item_data = _get_item_details(item) doc.item = item doc.quantity = 1 doc.update( diff --git a/erpnext/manufacturing/doctype/production_plan/services/material_request.py b/erpnext/manufacturing/doctype/production_plan/services/material_request.py index b3094c3fa11..e44b3d0e790 100644 --- a/erpnext/manufacturing/doctype/production_plan/services/material_request.py +++ b/erpnext/manufacturing/doctype/production_plan/services/material_request.py @@ -152,6 +152,7 @@ def get_items_for_material_requests( frappe.has_permission("Production Plan", "read", throw=True) doc = _normalize_mr_doc(doc) + _authorize_mr_request(doc, warehouses) _validate_group_warehouse_target(doc) warehouses = _filter_warehouses(doc, warehouses, get_parent_warehouse_data) doc["mr_items"] = [] @@ -180,6 +181,56 @@ def _normalize_mr_doc(doc): return doc +def _authorize_mr_request(doc, warehouses=None): + """Scope a caller-supplied plan to what the caller may see; `doc` is often unsaved, so check only a real name.""" + name = doc.get("name") + if isinstance(name, str) and frappe.db.exists("Production Plan", name): + frappe.has_permission("Production Plan", doc=name, throw=True) + + # Every value below arrives through frappe.parse_json, so container elements are untyped: a dict + # in any of these reaches frappe.db.get_value() in its *name* position and becomes a filter. + for row in _iter_mr_rows(doc): + for fieldname in ("item_code", "warehouse", "bom_no", "sales_order", "uom", "purchase_uom"): + value = row.get(fieldname) + if value is not None and not isinstance(value, str): + frappe.throw(_("Invalid {0}").format(fieldname), frappe.PermissionError) + + # The warehouse — not `company` — is what selects whose stock figures come back, so that is what + # a Company User Permission has to be applied to. Costs nobody who holds no such permission. + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "Production Plan") + if not allowed_companies: + return + + for warehouse in _iter_mr_warehouses(doc, warehouses): + company = frappe.db.get_value("Warehouse", warehouse, "company") + if company and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + + +def _iter_mr_rows(doc): + for key in ("po_items", "items", "sub_assembly_items"): + for row in doc.get(key) or []: + if isinstance(row, dict): + yield row + + +def _iter_mr_warehouses(doc, warehouses): + seen = set() + for value in (doc.get("for_warehouse"), doc.get("warehouse")): + if isinstance(value, str) and value: + seen.add(value) + for row in _iter_mr_rows(doc): + value = row.get("warehouse") + if isinstance(value, str) and value: + seen.add(value) + for value in get_warehouse_list(warehouses) if warehouses else []: + if isinstance(value, str) and value: + seen.add(value) + return seen + + def _validate_group_warehouse_target(doc): # the group only scopes availability; raw materials still need a concrete # receiving warehouse, so for_warehouse is required once we generate items. diff --git a/erpnext/manufacturing/doctype/routing/routing.py b/erpnext/manufacturing/doctype/routing/routing.py index 6edf8d28760..e3655d2330d 100644 --- a/erpnext/manufacturing/doctype/routing/routing.py +++ b/erpnext/manufacturing/doctype/routing/routing.py @@ -63,8 +63,25 @@ def get_operations(doctype: str, txt: str, searchfield: str, start: int, page_le if txt: query_filters = {"operation": ["like", f"%{txt}%"]} - if filters.get("routing"): - query_filters["parent"] = filters.get("routing") + if routing := filters.get("routing"): + if not frappe.db.exists("Routing", routing): + return [] + + ptype = "select" if frappe.only_has_select_perm("Routing") else "read" + frappe.has_permission("Routing", ptype, doc=routing, throw=True) + query_filters["parent"] = routing + query_filters["parenttype"] = "Routing" + else: + parents = [] + for parenttype in ("Routing", "BOM"): + ptype = "select" if frappe.only_has_select_perm(parenttype) else "read" + if frappe.has_permission(parenttype, ptype): + parents += frappe.get_list(parenttype, pluck="name") + + if not parents: + return [] + + query_filters["parent"] = ["in", parents] return frappe.get_all( "BOM Operation", diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index cad669ee10e..22dbd798781 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -1087,6 +1087,14 @@ class WorkOrder(Document): @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_bom_operations(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): + parent = filters.get("parent") + parenttype = filters.get("parenttype") or "BOM" + if not parent or not frappe.db.exists(parenttype, parent): + return [] + + ptype = "select" if frappe.only_has_select_perm(parenttype) else "read" + frappe.has_permission(parenttype, ptype, doc=parent, throw=True) + if txt: filters["operation"] = ("like", "%%%s%%" % txt) @@ -1132,14 +1140,15 @@ def get_default_warehouse(company: str): } -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def stop_unstop(work_order: str, status: str): """Called from client side on Stop/Unstop event""" - if not frappe.has_permission("Work Order", "write"): - frappe.throw(_("Not permitted"), frappe.PermissionError) + frappe.has_permission("Work Order", "write", throw=True) - pro_order = frappe.get_doc("Work Order", work_order) + # the check above is doctype level and never consults User Permissions, so on its own it lets + # a caller restricted to one company stop another company's orders + pro_order = frappe.get_doc("Work Order", work_order, check_permission="write") if pro_order.status == "Closed": frappe.throw(_("Closed Work Order can not be stopped or Re-opened")) @@ -1170,12 +1179,12 @@ def query_sales_order(doctype: str, txt: str, searchfield: str, start: int, page ) -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def close_work_order(work_order: str, status: str): - if not frappe.has_permission("Work Order", "write"): - frappe.throw(_("Not permitted"), frappe.PermissionError) + frappe.has_permission("Work Order", "write", throw=True) - work_order = frappe.get_doc("Work Order", work_order) + # doctype level above, record level here — see stop_unstop() + work_order = frappe.get_doc("Work Order", work_order, check_permission="write") if work_order.get("operations"): job_cards = frappe.get_list( "Job Card", diff --git a/erpnext/manufacturing/doctype/workstation/workstation.py b/erpnext/manufacturing/doctype/workstation/workstation.py index 88e8ac179b8..eb2ddbd2c89 100644 --- a/erpnext/manufacturing/doctype/workstation/workstation.py +++ b/erpnext/manufacturing/doctype/workstation/workstation.py @@ -418,34 +418,36 @@ def get_workstations(**kwargs): frappe.has_permission("Workstation", "read", throw=True) kwargs = frappe._dict(kwargs) - _workstation = frappe.qb.DocType("Workstation") - query = ( - frappe.qb.from_(_workstation) - .select( - _workstation.name, - _workstation.description, - _workstation.status, - _workstation.on_status_image, - _workstation.off_status_image, - ) - .orderby(_workstation.creation, _workstation.workstation_type, _workstation.name) - .where((_workstation.plant_floor == kwargs.plant_floor) & (_workstation.disabled == 0)) - ) + if not kwargs.plant_floor: + # The query this replaced compared `plant_floor` against the argument with `=`, which no + # row satisfies when it is empty; `get_list` would read the same filter as `IS NULL` and + # start returning floor-less workstations. Keep the original contract. + return [] + + # A list of filters, not a dict: `workstation` and `workstation_name` both constrain `name` + # and a dict would silently drop the first of them. + filters = [["plant_floor", "=", kwargs.plant_floor], ["disabled", "=", 0]] if kwargs.workstation: - query = query.where(_workstation.name == kwargs.workstation) + filters.append(["name", "=", kwargs.workstation]) if kwargs.workstation_type: - query = query.where(_workstation.workstation_type == kwargs.workstation_type) + filters.append(["workstation_type", "=", kwargs.workstation_type]) if kwargs.workstation_status: - query = query.where(_workstation.status == kwargs.workstation_status) + filters.append(["status", "=", kwargs.workstation_status]) if kwargs.workstation_name: - query = query.where(_workstation.name == kwargs.workstation_name) + filters.append(["name", "=", kwargs.workstation_name]) - data = query.run(as_dict=True) + # get_list, not get_all: it applies the caller's User Permissions to rows the doctype check does not scope + data = frappe.get_list( + "Workstation", + filters=filters, + fields=["name", "description", "status", "on_status_image", "off_status_image"], + order_by="creation, workstation_type, name", + ) color_map = get_color_map() diff --git a/erpnext/regional/italy/utils.py b/erpnext/regional/italy/utils.py index 1cbede63ef1..1ed215a09c6 100644 --- a/erpnext/regional/italy/utils.py +++ b/erpnext/regional/italy/utils.py @@ -34,7 +34,8 @@ def update_itemised_tax_data(doc): def export_invoices(filters: str | None = None): frappe.has_permission("Sales Invoice", throw=True) - invoices = frappe.get_all( + # get_list, not get_all: what leaves here is a zip of e-invoice attachments, so the rows must be scoped too + invoices = frappe.get_list( "Sales Invoice", filters=get_conditions(filters), fields=["name", "company_tax_id"] ) diff --git a/erpnext/selling/doctype/customer/customer.py b/erpnext/selling/doctype/customer/customer.py index e014490eda0..4bb317cf8ea 100644 --- a/erpnext/selling/doctype/customer/customer.py +++ b/erpnext/selling/doctype/customer/customer.py @@ -843,6 +843,16 @@ def get_credit_limit(customer, company): def get_customer_primary(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): customer = filters.get("customer") type = filters.get("type") + + # `type` is caller-supplied and was interpolated straight into qb.DocType(), so any doctype on + # the site could be joined to Dynamic Link and read. The two pickers that call this + # (customer.js:84,94) send only these two values. + if type not in ("Contact", "Address"): + frappe.throw(_("Invalid type"), frappe.PermissionError) + + # authorise the party, not Contact/Address: the `if_owner` row on Address would empty the picker rather than error + frappe.has_permission("Customer", doc=customer, throw=True) + type_doctype = qb.DocType(type) dlink = qb.DocType("Dynamic Link") diff --git a/erpnext/selling/doctype/product_bundle/product_bundle.py b/erpnext/selling/doctype/product_bundle/product_bundle.py index c51c0ec5967..808786b7827 100644 --- a/erpnext/selling/doctype/product_bundle/product_bundle.py +++ b/erpnext/selling/doctype/product_bundle/product_bundle.py @@ -233,16 +233,14 @@ def get_new_item_code(doctype: str, txt: str, searchfield: str, start: int, page searchfield = searchfield.split(",") searchfield.append("name") - item = frappe.qb.DocType("Item") - query = ( - frappe.qb.from_(item) - .select(item.name, item.item_name) - .where((item.is_stock_item == 0) & (item.is_fixed_asset == 0)) - .limit(page_len) - .offset(start) + # get_list applies Item's permission conditions and User Permissions, as item_query() does + return frappe.get_list( + "Item", + filters=[["is_stock_item", "=", 0], ["is_fixed_asset", "=", 0]], + or_filters=[[fieldname, "like", f"%{txt}%"] for fieldname in searchfield] if searchfield else None, + fields=["name", "item_name"], + order_by="", # the query this replaced had no ORDER BY; suppress the injected default + limit_start=start, + limit_page_length=page_len, + as_list=True, ) - - if searchfield: - query = query.where(Criterion.any([item[fieldname].like(f"%{txt}%") for fieldname in searchfield])) - - return query.run() diff --git a/erpnext/selling/doctype/proforma_invoice/proforma_invoice.py b/erpnext/selling/doctype/proforma_invoice/proforma_invoice.py index e8452293cfb..4524adde978 100644 --- a/erpnext/selling/doctype/proforma_invoice/proforma_invoice.py +++ b/erpnext/selling/doctype/proforma_invoice/proforma_invoice.py @@ -98,7 +98,9 @@ class ProformaInvoice(Document): @frappe.whitelist() def get_sales_order_items(sales_order: str) -> list[dict]: """Sales Order lines (with already-proformed totals) to drive the create-proforma dialog.""" - sales_order_doc = frappe.get_doc("Sales Order", sales_order) + # this returns line rates and amounts for a caller-named order, so the order itself is what + # decides access. check_permission applies User Permissions, which a doctype check would not. + sales_order_doc = frappe.get_doc("Sales Order", sales_order, check_permission="read") proformed = get_proformed_totals(sales_order) return [ { diff --git a/erpnext/selling/doctype/sales_order/sales_order.py b/erpnext/selling/doctype/sales_order/sales_order.py index 144d2ff40e5..4f434cddd43 100755 --- a/erpnext/selling/doctype/sales_order/sales_order.py +++ b/erpnext/selling/doctype/sales_order/sales_order.py @@ -790,14 +790,19 @@ def is_enable_cutoff_date_on_bulk_delivery_note_creation(): return frappe.get_single_value("Selling Settings", "enable_cutoff_date_on_bulk_delivery_note_creation") -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def close_or_unclose_sales_orders(names: str | list, status: str): - if not frappe.has_permission("Sales Order", "write"): - frappe.throw(_("Not permitted"), frappe.PermissionError) + frappe.has_permission("Sales Order", "write", throw=True) names = frappe.parse_json(names) for name in names: - so = frappe.get_lazy_doc("Sales Order", name) + if not isinstance(name, str): + frappe.throw(_("Invalid name"), frappe.PermissionError) + + # the check above is doctype level and never consults User Permissions, so on its own it + # lets a caller restricted to one company close another company's orders. Checking each + # document is what scopes it, and matches what update_status() below already does. + so = frappe.get_lazy_doc("Sales Order", name, check_permission="submit") if so.docstatus == 1: if status == "Closed": if so.status not in ("Cancelled", "Closed") and ( @@ -857,7 +862,7 @@ def get_events(start: str, end: str, filters: str | dict | None = None): return data -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def update_status(status: str, name: str): so = frappe.get_doc("Sales Order", name, check_permission="submit") so.update_status(status) diff --git a/erpnext/selling/page/point_of_sale/point_of_sale.py b/erpnext/selling/page/point_of_sale/point_of_sale.py index 39621ff9fb0..383c520d734 100644 --- a/erpnext/selling/page/point_of_sale/point_of_sale.py +++ b/erpnext/selling/page/point_of_sale/point_of_sale.py @@ -121,8 +121,20 @@ def filter_result_items(result, pos_profile): result["items"] = [item for item in result.get("items") if item.get("item_group") in pos_item_groups] +def check_pos_profile_access(pos_profile: str | None) -> None: + """The POS Profile is what entitles a caller to POS data — see the Bin/Item analysis on + pos_invoice.get_stock_availability. Record-level when a profile is named, so a Company User + Permission applies too.""" + if isinstance(pos_profile, str) and pos_profile: + frappe.has_permission("POS Profile", doc=pos_profile, throw=True) + else: + frappe.has_permission("POS Profile", throw=True) + + @frappe.whitelist() def get_parent_item_group(pos_profile: str): + check_pos_profile_access(pos_profile) + item_groups = get_item_groups(pos_profile) if not item_groups: @@ -140,6 +152,8 @@ def get_items( pos_profile: str, search_term: str = "", ): + check_pos_profile_access(pos_profile) + warehouse, hide_unavailable_items = frappe.db.get_value( "POS Profile", pos_profile, ["warehouse", "hide_unavailable_items"] ) @@ -272,6 +286,9 @@ def get_items( @frappe.whitelist() def search_for_serial_or_batch_or_barcode_number(search_value: str) -> dict[str, str | None]: + # POS-page wrapper around scan_barcode; the page's entitlement is the POS Profile. + frappe.has_permission("POS Profile", throw=True) + return scan_barcode(search_value) @@ -316,6 +333,7 @@ def get_item_group_condition(pos_profile, item=None): @frappe.validate_and_sanitize_search_inputs def item_group_query(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): pos_profile = filters.get("pos_profile") + check_pos_profile_access(pos_profile) item_filters = [["name", "like", f"%{txt}%"]] if pos_profile: @@ -323,7 +341,8 @@ def item_group_query(doctype: str, txt: str, searchfield: str, start: int, page_ if item_groups: item_filters.append(["name", "in", item_groups]) - return frappe.get_all( + # get_list, not get_all: it adds the caller's Item Group User Permissions; a Desk User select row keeps everyone in + return frappe.get_list( "Item Group", filters=item_filters, fields=["name"], @@ -337,6 +356,10 @@ def item_group_query(doctype: str, txt: str, searchfield: str, start: int, page_ @frappe.whitelist() def check_opening_entry(user: str): + # `user` was caller input, so anyone could enumerate another's open POS sessions; this is a POS Opening Entry question + if user != frappe.session.user: + frappe.has_permission("POS Opening Entry", throw=True) + open_vouchers = frappe.db.get_all( "POS Opening Entry", filters={"user": user, "pos_closing_entry": ["in", ["", None]], "docstatus": 1}, @@ -349,6 +372,10 @@ def check_opening_entry(user: str): @frappe.whitelist(methods=["POST"]) def create_opening_voucher(pos_profile: str, company: str, balance_details: str | list): + # submit() enforces POS Opening Entry rights per document, but only after the profile and company + # have been accepted from the caller — check the profile the session is being opened against. + check_pos_profile_access(pos_profile) + balance_details = frappe.parse_json(balance_details) new_pos_opening = frappe.get_doc( @@ -521,6 +548,8 @@ def set_customer_info(fieldname: str, customer: str, value: str = ""): @frappe.whitelist() def get_pos_profile_data(pos_profile: str): + check_pos_profile_access(pos_profile) + pos_profile = frappe.get_doc("POS Profile", pos_profile) pos_profile = pos_profile.as_dict() diff --git a/erpnext/setup/doctype/company/company.py b/erpnext/setup/doctype/company/company.py index 607a9784fb3..815d5dbdf2e 100644 --- a/erpnext/setup/doctype/company/company.py +++ b/erpnext/setup/doctype/company/company.py @@ -1082,7 +1082,10 @@ def get_children(doctype: str, parent: str | None = None, company: str | None = filters = {"parent_company": parent} if parent else {"parent_company": ["is", "not set"]} - return frappe.get_all( + # get_list, not get_all: it applies the caller's Company permission and their Company User + # Permissions, so a restricted user sees only their own companies. Matches the sibling tree + # source in accounts/utils.py, which already uses get_list. + return frappe.get_list( "Company", filters=filters, fields=["name as value", "is_group as expandable"], @@ -1096,6 +1099,11 @@ def add_node(): args = frappe.form_dict args = make_tree_args(**args) + # This is the Company tree's "add node" action; `args` comes straight from form_dict, so without + # this the caller chooses the doctype that gets created. insert() would still check permissions + # on whatever they named, but nothing else here is meant to build anything but a Company. + args.doctype = "Company" + if args.parent_company == "All Companies": args.parent_company = None @@ -1165,6 +1173,22 @@ def get_default_company_address( sort_key: Literal["is_shipping_address", "is_primary_address"] = "is_primary_address", existing_address: str | None = None, ): + # `Literal` is NOT enforced by typing_validations — measured, sort_key="name" was accepted — and + # addr[sort_key] is a column reference, so check it here. + if sort_key not in ("is_shipping_address", "is_primary_address"): + frappe.throw(_("Invalid sort key"), frappe.PermissionError) + + # Same boundary as accounts/custom/address.py::get_shipping_address: `select` denies the portal + # identities and costs none of the twelve transaction-writing roles, and the company scoping is + # what actually closes the cross-company read. + frappe.has_permission("Company", ptype="select", throw=True) + + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "Address") + if allowed_companies and name not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(name), frappe.PermissionError) + addr = frappe.qb.DocType("Address") dl = frappe.qb.DocType("Dynamic Link") out = ( diff --git a/erpnext/setup/doctype/department/department.py b/erpnext/setup/doctype/department/department.py index 6eda9e3510d..351421d01fd 100644 --- a/erpnext/setup/doctype/department/department.py +++ b/erpnext/setup/doctype/department/department.py @@ -89,10 +89,15 @@ def get_children( else: filters["parent_department"] = parent - if frappe.db.has_column(doctype, "disabled") and not include_disabled: + # `doctype` is caller-supplied and only ever reaches has_column here; the query below is fixed to + # Department, so pin it rather than letting a caller probe another table's columns. + if frappe.db.has_column("Department", "disabled") and not include_disabled: filters["disabled"] = False - return frappe.get_all("Department", fields=fields, filters=filters, order_by="name") + # get_list, not get_all: it applies the caller's Department permission and their User + # Permissions. Department carries no `if_owner` row, so this does not silently empty the tree — + # the same check made before swapping the call in setup/doctype/company/company.py. + return frappe.get_list("Department", fields=fields, filters=filters, order_by="name") @frappe.whitelist(methods=["POST"]) @@ -102,6 +107,11 @@ def add_node(): args = frappe.form_dict args = make_tree_args(**args) + # `args` comes straight from form_dict, so without this the caller chooses the doctype that gets + # created. insert() would still check permissions on whatever they named, but the Department + # tree's add-node action is not meant to build anything else. + args.doctype = "Department" + if args.parent_department == args.company: args.parent_department = None diff --git a/erpnext/stock/doctype/delivery_trip/delivery_trip.py b/erpnext/stock/doctype/delivery_trip/delivery_trip.py index b9eefdff786..e264127fde8 100644 --- a/erpnext/stock/doctype/delivery_trip/delivery_trip.py +++ b/erpnext/stock/doctype/delivery_trip/delivery_trip.py @@ -311,6 +311,9 @@ class DeliveryTrip(Document): @frappe.whitelist() def get_contact_and_address(name: str): + # `select`, not `read`: three of the four roles that run Delivery Trips hold no Customer read row + frappe.has_permission("Customer", ptype="select", doc=name, throw=True) + out = frappe._dict() get_default_contact(out, name) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 47d0ca2e82b..b059f45b608 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -1495,11 +1495,36 @@ def set_item_default(item_code, company, fieldname, value): @frappe.whitelist() def get_item_details(item_code: str, company: str | None = None): + # The whitelisted entry point authorises; _get_item_details is the in-process helper that does + # not. Deliberately NOT an `ignore_permissions` argument on this function: it is whitelisted, so + # a caller could pass it and skip the check. + return _get_item_details(item_code, company, ignore_permissions=False) + + +def _get_item_details(item_code: str, company: str | None = None, ignore_permissions: bool = True): + doc = frappe.get_cached_doc("Item", item_code) + if not ignore_permissions: + # the whole Item document is returned below, so the record itself has to be authorised. This + # is the check stock/get_item_details.py already makes before returning details for a + # transaction. + doc.check_permission() + out = frappe._dict() if company: + if not ignore_permissions: + # `company` is caller supplied and scopes the Item Defaults returned alongside the item. + # Checked through the caller's own Company restrictions rather than a permission on + # Company, so a caller with no Company restriction is unaffected. + from erpnext.stock.doctype.company_restriction.company_restriction import ( + get_allowed_companies, + ) + + allowed_companies = get_allowed_companies(frappe.session.user, "Item") + if allowed_companies and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + out = get_item_defaults(item_code, company) or frappe._dict() - doc = frappe.get_cached_doc("Item", item_code) out.update(doc.as_dict()) return out @@ -1633,32 +1658,28 @@ ITEM_PRICES_LIMIT = 10 @frappe.whitelist() def get_item_prices(item_code: str): """Fetch valid item prices for the item prices tab.""" - if not frappe.has_permission("Item Price", "read"): - frappe.throw(_("Not permitted"), frappe.PermissionError) + frappe.has_permission("Item Price", "read", throw=True) today = getdate() - ItemPrice = frappe.qb.DocType("Item Price") - - prices = ( - frappe.qb.from_(ItemPrice) - .select( - ItemPrice.name, - ItemPrice.price_list, - ItemPrice.price_list_rate, - ItemPrice.currency, - ItemPrice.uom, - ItemPrice.customer, - ItemPrice.supplier, - ItemPrice.buying, - ItemPrice.selling, - ItemPrice.valid_upto, - ) - .where(ItemPrice.item_code == item_code) - .where(ItemPrice.docstatus != 2) - .where((ItemPrice.valid_upto.isnull()) | (ItemPrice.valid_upto >= today)) - .orderby(ItemPrice.price_list) - .limit(ITEM_PRICES_LIMIT + 1) - .run(as_dict=True) + # get_list, not get_all: otherwise a caller restricted to one Price List sees every party's negotiated rate + prices = frappe.get_list( + "Item Price", + filters={"item_code": item_code, "docstatus": ["!=", 2]}, + or_filters=[["valid_upto", "is", "not set"], ["valid_upto", ">=", today]], + fields=[ + "name", + "price_list", + "price_list_rate", + "currency", + "uom", + "customer", + "supplier", + "buying", + "selling", + "valid_upto", + ], + order_by="price_list", + limit=ITEM_PRICES_LIMIT + 1, ) return { @@ -1675,8 +1696,7 @@ def make_opening_stock_entry( valuation_rate: float, warehouse: str | None = None, ): - if not frappe.has_permission("Item", "write", item_code): - frappe.throw(_("Not permitted"), frappe.PermissionError) + frappe.has_permission("Item", "write", item_code, throw=True) item = frappe.get_doc("Item", item_code) diff --git a/erpnext/stock/doctype/item_standard_cost/item_standard_cost.py b/erpnext/stock/doctype/item_standard_cost/item_standard_cost.py index c1a7a35506b..558aa55bd72 100644 --- a/erpnext/stock/doctype/item_standard_cost/item_standard_cost.py +++ b/erpnext/stock/doctype/item_standard_cost/item_standard_cost.py @@ -385,8 +385,19 @@ def get_standard_cost_items( 'Standard Cost' — i.e. the item is explicitly Standard Cost, or it has no valuation method of its own and the applicable default (Company, else Stock Settings) is Standard Cost. This mirrors get_valuation_method, so every shown item also passes validate_item.""" + # the form is the boundary, not Item: Accounts Manager writes this doctype and holds no Item read or select + frappe.has_permission("Item Standard Cost", throw=True) + company = (filters or {}).get("company") if company: + # `company` is caller supplied and selects whose default valuation method is applied, so a + # caller restricted to particular companies must not ask about the others + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "Item Standard Cost") + if allowed_companies and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + default_method = frappe.get_cached_value("Company", company, "valuation_method") else: default_method = frappe.db.get_single_value("Stock Settings", "valuation_method") diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py index e502ceb5c82..81a7bd815d0 100644 --- a/erpnext/stock/doctype/material_request/material_request.py +++ b/erpnext/stock/doctype/material_request/material_request.py @@ -11,8 +11,7 @@ import frappe import frappe.defaults from frappe import _, msgprint from frappe.model.document import Document -from frappe.query_builder import Order -from frappe.query_builder.functions import Min, Sum +from frappe.query_builder.functions import Sum from frappe.utils import cint, flt, get_datetime, get_link_to_form, getdate, new_line_sep, nowdate from erpnext.buying.utils import check_on_hold_or_closed_status, validate_for_items @@ -575,38 +574,41 @@ def get_material_requests_based_on_supplier( if not supplier_items: frappe.throw(_("{0} is not the default supplier for any items.").format(supplier)) - mr = frappe.qb.DocType("Material Request") - mr_item = frappe.qb.DocType("Material Request Item") + mr_filters = [ + ["material_request_type", "=", "Purchase"], + ["per_ordered", "<", 99.99], + ["docstatus", "=", 1], + ["status", "!=", "Stopped"], + ["company", "=", filters.get("company")], + ] - query = ( - frappe.qb.from_(mr) - .from_(mr_item) - .select(mr.name, mr.transaction_date, mr.company) - .where( - (mr.name == mr_item.parent) - & (mr_item.item_code.isin(supplier_items)) - & (mr.material_request_type == "Purchase") - & (mr.per_ordered < 99.99) - & (mr.docstatus == 1) - & (mr.status != "Stopped") - & (mr.company == filters.get("company")) + if frappe.has_permission("Material Request", "read"): + mr_filters.append(["Material Request Item", "item_code", "in", supplier_items]) + else: + parents = frappe.get_all( + "Material Request Item", + filters={"item_code": ("in", supplier_items), "parenttype": "Material Request"}, + pluck="parent", + distinct=True, ) - .groupby(mr.name, mr.transaction_date, mr.company) - .orderby(Min(mr_item.item_code), order=Order.asc) - .limit(cint(page_len)) - .offset(cint(start)) - ) + mr_filters.append(["name", "in", parents or [""]]) if txt: - query = query.where(mr.name.like(f"%%{txt}%%")) + mr_filters.append(["name", "like", f"%{txt}%"]) if filters.get("transaction_date"): date = filters.get("transaction_date")[1] - query = query.where(mr.transaction_date[date[0] : date[1]]) + mr_filters.append(["transaction_date", "between", [date[0], date[1]]]) - material_requests = query.run(as_dict=True) - - return material_requests + return frappe.get_list( + "Material Request", + filters=mr_filters, + fields=["name", "transaction_date", "company"], + group_by="name", + order_by="name", + limit_start=cint(start), + limit_page_length=cint(page_len), + ) @frappe.whitelist(methods=["POST"]) diff --git a/erpnext/stock/doctype/pick_list/pick_list.py b/erpnext/stock/doctype/pick_list/pick_list.py index 9bbf001c385..3c8cde3e58f 100644 --- a/erpnext/stock/doctype/pick_list/pick_list.py +++ b/erpnext/stock/doctype/pick_list/pick_list.py @@ -544,9 +544,10 @@ class PickList(TransactionBase): work_order = frappe.get_doc("Work Order", self.work_order) RequiredItemsService(work_order).update_picked_qty_for_required_items() - @frappe.whitelist() + @frappe.whitelist(methods=["POST"]) def create_stock_reservation_entries(self, notify: bool = True) -> None: """Creates Stock Reservation Entries for Sales Order Items against Pick List.""" + self.check_permission("write") so_items_details_map = {} for location in self.locations: @@ -571,9 +572,10 @@ class PickList(TransactionBase): notify=notify, ) - @frappe.whitelist() + @frappe.whitelist(methods=["POST"]) def cancel_stock_reservation_entries(self, notify: bool = True) -> None: """Cancel Stock Reservation Entries for Sales Order Items created against Pick List.""" + self.check_permission("write") from erpnext.stock.doctype.stock_reservation_entry.stock_reservation_entry import ( cancel_stock_reservation_entries, @@ -596,8 +598,16 @@ class PickList(TransactionBase): ).format(row.item_code, row.sales_order) ) - @frappe.whitelist() + @frappe.whitelist(methods=["POST"]) def set_item_locations(self, save: bool = False): + # gate the allocation up front rather than letting save() catch it afterwards — but only for a + # document that already exists: before_save and the SO/WO/MR mappers reach this on an unsaved + # list, where there is no record to authorise and insert() checks `create` anyway. + # Test the record, not is_new(): that reads `__islocal`, which arrives in the client's own + # JSON through run_doc_method, so a caller can skip the check by setting it on a saved list. + if self.name and frappe.db.exists("Pick List", self.name): + self.check_permission("write") + self.validate_for_qty() items = self.aggregate_item_qty() @@ -1637,6 +1647,18 @@ def get_available_item_locations_for_other_item( return item_locations +def check_pick_list_company(company: str | None) -> None: + """Keep a company-restricted caller inside their own companies; a no-op for everyone else.""" + if not isinstance(company, str) or not company: + return + + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "Pick List") + if allowed_companies and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + + @frappe.whitelist() def get_pending_work_orders( doctype: Any, @@ -1647,6 +1669,10 @@ def get_pending_work_orders( filters: dict, as_dict: bool = False, ): + # same guard as the sibling get_pick_list_query; a Work Order guard would lose Stock and Manufacturing Manager + frappe.has_permission("Pick List", throw=True) + check_pick_list_company(filters.get("company") if isinstance(filters, dict) else None) + wo = frappe.qb.DocType("Work Order") return ( frappe.qb.from_(wo) @@ -1673,6 +1699,9 @@ def get_pending_work_orders( def get_item_details( item_code: str, uom: str | None = None, warehouse: str | None = None, company: str | None = None ): + frappe.has_permission("Pick List", throw=True) + check_pick_list_company(company) + details = frappe.db.get_value("Item", item_code, "stock_uom", as_dict=1) details.uom = uom or details.stock_uom if uom: diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index c74261b1100..377815c4677 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -554,6 +554,15 @@ def update_regional_gl_entries(gl_list, doc): @frappe.whitelist() def make_lcv(doctype: str, docname: str): + # `doctype` is caller-supplied and reaches get_value() as the doctype; only these two carry the fields read below + if doctype not in ("Purchase Receipt", "Purchase Invoice"): + frappe.throw(_("Invalid document type"), frappe.PermissionError) + + # Authorise the source document, not the Landed Cost Voucher: LCV create is held by Stock Manager + # alone here, while the roles that actually press this button are the ones who can read the + # receipt or invoice they are pressing it on. + frappe.has_permission(doctype, doc=docname, throw=True) + landed_cost_voucher = frappe.new_doc("Landed Cost Voucher") details = frappe.db.get_value(doctype, docname, ["supplier", "company", "base_grand_total"], as_dict=1) diff --git a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py index 0b97c4cd2b8..bfff1284247 100644 --- a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py @@ -1536,7 +1536,7 @@ class TestPurchaseReceipt(ERPNextTestSuite): from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( create_stock_reconciliation, ) - from erpnext.stock.get_item_details import get_valuation_rate + from erpnext.stock.get_item_details import _get_valuation_rate from erpnext.stock.utils import get_stock_balance prepare_data_for_internal_transfer() @@ -1553,7 +1553,7 @@ class TestPurchaseReceipt(ERPNextTestSuite): ) if ( - get_valuation_rate( + _get_valuation_rate( pr1.items[0].item_code, "_Test Company with perpetual inventory", warehouse="Stores - TCP1" ) != 50 diff --git a/erpnext/stock/doctype/quality_inspection/quality_inspection.py b/erpnext/stock/doctype/quality_inspection/quality_inspection.py index 88a6e573608..a729ecb9629 100644 --- a/erpnext/stock/doctype/quality_inspection/quality_inspection.py +++ b/erpnext/stock/doctype/quality_inspection/quality_inspection.py @@ -520,7 +520,7 @@ def item_query(doctype: Any, txt: str | None, searchfield: Any, start: int, page def quality_inspection_query( doctype: Any, txt: str | None, searchfield: Any, start: int, page_len: int, filters: dict ): - return frappe.get_all( + return frappe.get_list( "Quality Inspection", limit_start=start, limit_page_length=page_len, diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py index 495858f2c16..dce4944a780 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -234,6 +234,8 @@ class RepostItemValuation(Document): @frappe.whitelist() def set_company(self): + self.check_permission("write") + if self.based_on == "Transaction": self.company = frappe.get_cached_value(self.voucher_type, self.voucher_no, "company") elif self.warehouse: @@ -388,8 +390,13 @@ class RepostItemValuation(Document): doc.update_stock_ledger(allow_negative_stock=True) -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def bulk_restart_reposting(names: str | list): + # restart_reposting() below checks write per document and lets the error propagate, so the + # authorisation is already correct — but it only fires after each document has been loaded and + # its status read. Gate first; this denies exactly who the per-document check would. + frappe.has_permission("Repost Item Valuation", "write", throw=True) + names = frappe.parse_json(names) for name in names: doc = frappe.get_doc("Repost Item Valuation", name) @@ -955,9 +962,11 @@ def in_configured_timeslot(repost_settings=None, current_time=None): return now_time >= start_time or now_time <= end_time -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def execute_repost_item_valuation(): """Execute repost item valuation via scheduler.""" + # Force-enqueues the site-wide reposting job, so it needs the same right as restarting one. + frappe.has_permission("Repost Item Valuation", "write", throw=True) method = "erpnext.stock.doctype.repost_item_valuation.repost_item_valuation.repost_entries" if frappe.db.get_single_value("Stock Reposting Settings", "enable_parallel_reposting"): diff --git a/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py b/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py index 4e1d728ab03..4357e220cdf 100644 --- a/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py +++ b/erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py @@ -1920,6 +1920,8 @@ def download_blank_csv_template(content: str | list): @frappe.whitelist() def upload_csv_file(item_code: str, file_path: str): + frappe.has_permission("Item", ptype="select", throw=True) + serial_nos, batch_nos = [], [] serial_nos, batch_nos = get_serial_batch_from_csv(item_code, file_path) @@ -1938,9 +1940,11 @@ def get_serial_batch_from_csv(item_code, file_path): if not file_path: return serial_nos, batch_nos - try: - file = frappe.get_doc("File", {"file_url": file_path}) - except frappe.DoesNotExistError: + from frappe.core.doctype.file.utils import find_file_by_url + + # look the file up through find_file_by_url, which returns it only when the caller may download it + file = find_file_by_url(file_path) + if not file: frappe.msgprint( _("File '{0}' not found").format(frappe.bold(file_path)), alert=True, @@ -2035,6 +2039,8 @@ def get_serial_batch_from_data(item_code, kwargs): @frappe.whitelist() def create_serial_nos(item_code: str, serial_nos: list | str): + frappe.has_permission("Item", ptype="select", throw=True) + serial_nos = get_serial_batch_from_data( item_code, { @@ -2158,7 +2164,8 @@ def item_query( if txt: item_filters["name"] = ("like", f"%{txt}%") - return frappe.get_all( + # get_list, not get_all, so Item permissions apply; `select` is what keeps the roles that work bundles usable + return frappe.get_list( "Item", filters=item_filters, or_filters={"has_serial_no": 1, "has_batch_no": 1}, @@ -3697,6 +3704,8 @@ def get_batch_no_from_serial_no(serial_no: str): def is_serial_batch_no_exists( item_code: str, type_of_transaction: str, serial_no: str | None = None, batch_no: str | None = None ): + frappe.has_permission("Item", ptype="select", throw=True) + if serial_no and not frappe.db.exists("Serial No", serial_no): if type_of_transaction != "Inward": frappe.throw(_("Serial No {0} does not exist").format(serial_no)) diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index 3fad9e50e54..ced816e68ed 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -34,7 +34,7 @@ from erpnext.stock.get_item_details import ( get_default_cost_center, ) from erpnext.stock.stock_ledger import get_previous_sle, get_valuation_rate -from erpnext.stock.utils import get_combine_datetime, get_incoming_rate +from erpnext.stock.utils import _get_incoming_rate, check_warehouse_company, get_combine_datetime from .services.disassemble import DisassembleStockEntry from .services.manufacturing import ( @@ -768,7 +768,7 @@ class StockEntry(StockController, SubcontractingInwardController): if d.s_warehouse: if reset_outgoing_rate: args = self.get_args_for_incoming_rate(d) - rate = get_incoming_rate(args, raise_error_if_no_rate) + rate = _get_incoming_rate(args, raise_error_if_no_rate) if rate >= 0: d.basic_rate = rate @@ -1960,6 +1960,13 @@ def get_warehouse_details(args: str | dict): args = frappe._dict(args) + # Restored explicitly: both checks were inherited from get_incoming_rate until it was split into + # a guarded whitelist wrapper and the unguarded _get_incoming_rate this now calls. + # `select`, not `read`: this is reached from stock_entry.js:740, and the desk roles that open + # that form clear select through the Desk User row while holding no Item read of their own. + frappe.has_permission("Item", ptype="select", throw=True) + check_warehouse_company(args.get("warehouse")) + ret = {} if args.warehouse and args.item_code: args.update( @@ -1970,6 +1977,6 @@ def get_warehouse_details(args: str | dict): ) ret = { "actual_qty": get_previous_sle(args).get("qty_after_transaction") or 0, - "basic_rate": get_incoming_rate(args), + "basic_rate": _get_incoming_rate(args), } return ret diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py index 7388769a6c5..d62ff30af49 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py @@ -21,7 +21,12 @@ from erpnext.stock.doctype.serial_and_batch_bundle.serial_and_batch_bundle impor ) from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos from erpnext.stock.doctype.stock_reconciliation_item.stock_reconciliation_item import StockReconciliationItem -from erpnext.stock.utils import get_incoming_rate, get_stock_balance, get_valuation_method +from erpnext.stock.utils import ( + _get_incoming_rate, + check_warehouse_company, + get_stock_balance, + get_valuation_method, +) class OpeningEntryAccountError(frappe.ValidationError): @@ -1561,7 +1566,10 @@ def get_stock_balance_for( serial_nos = "\n".join(d.serial_no for d in serial_no_details if d.batch_no == batch_no) if row and row.use_serial_batch_fields and row.batch_no and (qty or row.current_qty): - rate = get_incoming_rate( + # inherited from get_incoming_rate before the split; scoped here rather than at the top + # of the function so the guard covers exactly the path it covered before + check_warehouse_company(row.warehouse) + rate = _get_incoming_rate( frappe._dict( { "item_code": row.item_code, diff --git a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py index fa8c9e67d6b..22e40389c6c 100644 --- a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py @@ -25,8 +25,8 @@ from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse from erpnext.stock.stock_ledger import get_previous_sle, update_entries_after from erpnext.stock.tests.test_utils import StockTestMixin from erpnext.stock.utils import ( + _get_incoming_rate, get_combine_datetime, - get_incoming_rate, get_stock_balance, get_stock_value_on, get_valuation_method, @@ -178,7 +178,7 @@ class TestStockReconciliation(ERPNextTestSuite, StockTestMixin): "serial_and_batch_bundle": sr.items[0].serial_and_batch_bundle, } - valuation_rate = get_incoming_rate(args) + valuation_rate = _get_incoming_rate(args) self.assertEqual(valuation_rate, 200) to_delete_records.append(sr.name) @@ -200,7 +200,7 @@ class TestStockReconciliation(ERPNextTestSuite, StockTestMixin): "serial_and_batch_bundle": sr.items[0].serial_and_batch_bundle, } - valuation_rate = get_incoming_rate(args) + valuation_rate = _get_incoming_rate(args) self.assertEqual(valuation_rate, 300) to_delete_records.append(sr.name) @@ -251,7 +251,7 @@ class TestStockReconciliation(ERPNextTestSuite, StockTestMixin): "serial_and_batch_bundle": sr1.items[0].serial_and_batch_bundle, } - valuation_rate = get_incoming_rate(args) + valuation_rate = _get_incoming_rate(args) self.assertEqual(valuation_rate, 300) to_delete_records.append(sr1.name) diff --git a/erpnext/stock/doctype/warehouse/warehouse.py b/erpnext/stock/doctype/warehouse/warehouse.py index a3d73173659..b97dc23170a 100644 --- a/erpnext/stock/doctype/warehouse/warehouse.py +++ b/erpnext/stock/doctype/warehouse/warehouse.py @@ -223,11 +223,18 @@ def add_node(): frappe.get_doc(args).insert() -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def convert_to_group_or_ledger(docname: str | None = None): if not docname: docname = frappe.form_dict.docname - return frappe.get_doc("Warehouse", docname).convert_to_group_or_ledger() + + # Converting a warehouse between group and ledger restructures the tree, so it needs write on + # the warehouse being converted. `Warehouse` write is held by Item Manager alone, which is also + # who can open the form this button sits on (warehouse.js:104). + warehouse = frappe.get_doc("Warehouse", docname) + warehouse.check_permission("write") + + return warehouse.convert_to_group_or_ledger() @request_cache @@ -310,20 +317,23 @@ def apply_warehouse_filter(query, sle, filters): def get_warehouses_for_reorder( doctype: str, txt: Any, searchfield: Any, start: int, page_len: int, filters: dict ): + # Reached from the Item form's reorder table (item.js:774); `read` on Warehouse is the target + # right and costs none of the roles that can edit an Item. + frappe.has_permission("Warehouse", throw=True) + filters = frappe._dict(filters or {}) if filters.warehouse and not frappe.db.exists("Warehouse", filters.warehouse): frappe.throw(_("Warehouse {0} does not exist").format(filters.warehouse)) - doctype = frappe.qb.DocType("Warehouse") - - warehouses = ( - frappe.qb.from_(doctype) - .select(doctype.name) - .where(doctype.disabled == 0) - .where((doctype.is_group == 1) | (doctype.name == filters.warehouse)) - .orderby(doctype.name) - .run(as_list=True) + # get_list, not get_all: it scopes the rows the doctype check does not; `as_list` keeps the tuples the picker expects + warehouses = frappe.get_list( + "Warehouse", + filters={"disabled": 0}, + or_filters=[["is_group", "=", 1], ["name", "=", filters.warehouse]], + fields=["name"], + order_by="name", + as_list=True, ) return warehouses diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py index 8d52b77ace1..25f8144155d 100644 --- a/erpnext/stock/get_item_details.py +++ b/erpnext/stock/get_item_details.py @@ -283,7 +283,7 @@ def set_valuation_rate(out: frappe._dict, ctx: frappe._dict): for bundle_item in bundled_items.items: valuation_rate += flt( - get_valuation_rate(bundle_item.item_code, ctx.company, out.get("warehouse")).get( + _get_valuation_rate(bundle_item.item_code, ctx.company, out.get("warehouse")).get( "valuation_rate" ) * bundle_item.qty @@ -292,7 +292,7 @@ def set_valuation_rate(out: frappe._dict, ctx: frappe._dict): out.update({"valuation_rate": valuation_rate}) else: - out.update(get_valuation_rate(ctx.item_code, ctx.company, out.get("warehouse"))) + out.update(_get_valuation_rate(ctx.item_code, ctx.company, out.get("warehouse"))) def update_stock(ctx, out, doc=None): @@ -1625,6 +1625,10 @@ def get_conversion_factor(item_code: str | None, uom: str): @frappe.whitelist() def get_projected_qty(item_code: str, warehouse: str): + # record-level read on the item, matching what get_item_details() in this file already does. + # Nothing in the tree calls this, so there is no caller whose roles constrain the choice. + frappe.has_permission("Item", doc=item_code, throw=True) + return { "projected_qty": frappe.db.get_value( "Bin", {"item_code": item_code, "warehouse": warehouse}, "projected_qty" @@ -1636,6 +1640,9 @@ def get_projected_qty(item_code: str, warehouse: str): def get_bin_details( item_code: str, warehouse: str | None, company: str | None = None, include_child_warehouses: bool = False ): + # `select`, not `read`: the selling/buying rows and SellingController reach this with no Item read row + frappe.has_permission("Item", ptype="select", throw=True) + bin_details = {"projected_qty": 0, "actual_qty": 0, "reserved_qty": 0} if warehouse: @@ -1817,6 +1824,15 @@ def get_default_bom(item_code: str | None = None): @frappe.whitelist() def get_valuation_rate(item_code: str, company: str, warehouse: str | None = None): + """Whitelisted entry point: authorise the item, then return its cost price.""" + frappe.has_permission("Item", doc=item_code, throw=True) + + return _get_valuation_rate(item_code, company, warehouse) + + +def _get_valuation_rate(item_code: str, company: str, warehouse: str | None = None): + # no guard here: set_valuation_rate calls this for the item AND for every Product Bundle + # component, and a caller entitled to the bundle is not necessarily entitled to each component if frappe.get_cached_value("Warehouse", warehouse, "is_group"): return {"valuation_rate": 0.0} diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index ffd1d674b10..0dfbf41e4fa 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -95,6 +95,22 @@ def get_stock_value_on( return query.run(as_list=True)[0][0] +def check_warehouse_company(warehouse: str | None) -> None: + """Keep a company-restricted caller inside their own companies; a no-op for everyone else.""" + if not isinstance(warehouse, str) or not warehouse: + return + + from erpnext.stock.doctype.company_restriction.company_restriction import get_allowed_companies + + allowed_companies = get_allowed_companies(frappe.session.user, "Item") + if not allowed_companies: + return + + company = frappe.db.get_value("Warehouse", warehouse, "company") + if company and company not in allowed_companies: + frappe.throw(_("Not permitted for {0}").format(company), frappe.PermissionError) + + @frappe.whitelist() def get_stock_balance( item_code: str, @@ -178,6 +194,12 @@ def get_serial_nos_data(serial_nos): @frappe.whitelist() def get_latest_stock_qty(item_code: str, warehouse: str | None = None): + # Same guard as get_stock_balance above, which returns the same Bin quantity from the same file. + # Loser-free for the only caller: work_order.js:797, and Work Order write is held by + # Manufacturing User, who holds Item read. (Manufacturing Manager holds neither.) + frappe.has_permission("Item", "read", throw=True) + check_warehouse_company(warehouse) + bin_dt = frappe.qb.DocType("Bin") query = frappe.qb.from_(bin_dt).select(Sum(bin_dt.actual_qty)).where(bin_dt.item_code == item_code) @@ -268,6 +290,22 @@ def _create_bin(item_code, warehouse): @frappe.whitelist() def get_incoming_rate(args: dict | str, raise_error_if_no_rate: bool = True, fallbacks: bool = True): + """Whitelisted entry point: authorise the caller, then compute the rate.""" + args = frappe.parse_json(args) + + # `select`, not `read`: this is reached from transaction.js:1069 on every sales and buying form, + # and Accounts Manager — who writes Sales Invoice and Purchase Invoice — holds no Item read + frappe.has_permission("Item", ptype="select", throw=True) + # only on this path: in-process callers legitimately price a warehouse the caller is not scoped + # to — Delivery Note submit, Stock Entry transfer, Subcontracting Receipt and the Product Bundle + # set_valuation_rate loop all raised "Not permitted for ..." for an entitled Company-restricted + # identity while this guard sat on the shared function + check_warehouse_company(args.get("warehouse") if isinstance(args, dict | frappe._dict) else None) + + return _get_incoming_rate(args, raise_error_if_no_rate, fallbacks) + + +def _get_incoming_rate(args: dict | str, raise_error_if_no_rate: bool = True, fallbacks: bool = True): """Get Incoming Rate based on valuation method""" from erpnext.stock.stock_ledger import get_previous_sle, get_valuation_rate @@ -598,6 +636,17 @@ def check_pending_reposting(posting_date: str, company: str | None = None, throw @frappe.whitelist() def scan_barcode(search_value: str, ctx: dict | str | None = None) -> BarcodeScanResult: + # Reached from barcode_scanner.js on every form with a scan field, so `select` for the same + # reason as get_incoming_rate: Accounts Manager scans on invoices and holds no Item read. + frappe.has_permission("Item", ptype="select", throw=True) + + def authorised(data: BarcodeScanResult) -> BarcodeScanResult: + # the check above is doctype level; the scan resolves to one Item and that is what the + # caller receives, so authorise the resolved row before returning it + if data and data.get("item_code"): + frappe.has_permission("Item", ptype="select", doc=data.get("item_code"), throw=True) + return data + def set_cache(data: BarcodeScanResult): frappe.cache().set_value(f"erpnext:barcode_scan:{search_value}", data, expires_in_sec=120) _update_item_info(data, ctx) @@ -614,7 +663,7 @@ def scan_barcode(search_value: str, ctx: dict | str | None = None) -> BarcodeSca ctx = frappe._dict() if scan_data := get_cache(): - return scan_data + return authorised(scan_data) # search barcode no barcode_data = frappe.db.get_value( @@ -625,7 +674,7 @@ def scan_barcode(search_value: str, ctx: dict | str | None = None) -> BarcodeSca ) if barcode_data: set_cache(barcode_data) - return barcode_data + return authorised(barcode_data) # search serial no serial_no_data = frappe.db.get_value( @@ -636,7 +685,7 @@ def scan_barcode(search_value: str, ctx: dict | str | None = None) -> BarcodeSca ) if serial_no_data: set_cache(serial_no_data) - return serial_no_data + return authorised(serial_no_data) # search batch no batch_no_data = frappe.db.get_value( @@ -654,7 +703,7 @@ def scan_barcode(search_value: str, ctx: dict | str | None = None) -> BarcodeSca ) set_cache(batch_no_data) - return batch_no_data + return authorised(batch_no_data) warehouse = frappe.get_cached_value("Warehouse", search_value, ("name", "disabled"), as_dict=True) if warehouse and not warehouse.disabled: diff --git a/erpnext/support/doctype/issue/issue.py b/erpnext/support/doctype/issue/issue.py index 77a84f2901a..8a21702b082 100644 --- a/erpnext/support/doctype/issue/issue.py +++ b/erpnext/support/doctype/issue/issue.py @@ -260,13 +260,16 @@ def get_issue_list(doctype, txt, filters, limit_start, limit_page_length=20, ord ) -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def set_multiple_status(names: str | list, status: str): for name in frappe.parse_json(names): + if not isinstance(name, str): + frappe.throw(_("Invalid name"), frappe.PermissionError) + set_status(name, status) -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def set_status(name: str, status: str): frappe.has_permission("Issue", "write", name, throw=True) frappe.db.set_value("Issue", name, "status", status) @@ -321,6 +324,11 @@ def make_task(source_name: str, target_doc: str | dict | Document | None = None) def make_issue_from_communication(communication: str, ignore_communication_links: bool = False): """raise a issue from email""" + # `communication` is caller supplied and nothing checked it. Communication grants read to `All` + # only for the owner (if_owner) and carries a has_permission hook, so doc= is what decides + # access; the desk button only appears on an email the caller already has open. + frappe.has_permission("Communication", doc=communication, throw=True) + doc = frappe.get_doc("Communication", communication) issue = frappe.get_doc( { diff --git a/erpnext/utilities/bulk_transaction.py b/erpnext/utilities/bulk_transaction.py index d1e383cda9d..46342c685b7 100644 --- a/erpnext/utilities/bulk_transaction.py +++ b/erpnext/utilities/bulk_transaction.py @@ -6,7 +6,7 @@ from frappe import _ from frappe.utils import get_link_to_form, today -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def transaction_processing( data: str | list, from_doctype: str, to_doctype: str, args: str | frappe._dict | None = None ): @@ -21,6 +21,16 @@ def transaction_processing( deserialized_data = [d for d in deserialized_data if d.get("status") not in ("On Hold", "Closed")] + # The checks above are doctype level and never consult User Permissions, so on their own they + # let a caller convert documents they cannot read — a company-restricted user could turn another + # company's orders into invoices. Each source document is checked before anything is enqueued. + for row in deserialized_data: + source_name = row.get("name") + if not source_name or not isinstance(source_name, str): + frappe.throw(_("Invalid name"), frappe.PermissionError) + + frappe.has_permission(from_doctype, "read", source_name, throw=True) + length_of_data = len(deserialized_data) skipped_msg = "" @@ -50,7 +60,7 @@ def transaction_processing( ) -@frappe.whitelist() +@frappe.whitelist(methods=["POST"]) def retry(date: str | None = None): frappe.only_for("System Manager") if not date: diff --git a/erpnext/utilities/doctype/rename_tool/rename_tool.py b/erpnext/utilities/doctype/rename_tool/rename_tool.py index 972c87deaac..7bdb3414481 100644 --- a/erpnext/utilities/doctype/rename_tool/rename_tool.py +++ b/erpnext/utilities/doctype/rename_tool/rename_tool.py @@ -29,6 +29,10 @@ class RenameTool(Document): @frappe.whitelist() @deprecated def get_doctypes(): + # The Rename Tool page is System-Manager-only, and its sibling upload() below already checks + # before doing anything; this listed every renameable doctype on the site to any caller. + frappe.has_permission("Rename Tool", throw=True) + return frappe.get_all( "DocType", filters={"allow_rename": 1, "module": ["!=", "Core"]}, order_by="name", pluck="name" ) diff --git a/erpnext/utilities/transaction_base.py b/erpnext/utilities/transaction_base.py index e926e3713c1..6851daad20f 100644 --- a/erpnext/utilities/transaction_base.py +++ b/erpnext/utilities/transaction_base.py @@ -10,7 +10,7 @@ from frappe.utils import cint, flt, get_time, now_datetime from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import get_dimensions from erpnext.controllers.status_updater import StatusUpdater from erpnext.stock.get_item_details import NOT_APPLICABLE_TAX, get_item_details -from erpnext.stock.utils import get_incoming_rate +from erpnext.stock.utils import _get_incoming_rate class UOMMustBeIntegerError(frappe.ValidationError): @@ -435,7 +435,7 @@ class TransactionBase(StatusUpdater): } ) - rate = get_incoming_rate(args=args) + rate = _get_incoming_rate(args=args) item_obj.rate = rate * item_obj.conversion_factor else: self.set_rate_based_on_price_list(item_obj, item_details)