mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-21 10:19:57 +00:00
fix: check company permission before reading sample retention stock
validate_sample_quantity and move_sample_to_retention_warehouse are whitelisted and take company from the caller, which selects whose retention warehouse gets read. The retained batch qty then reaches the return value and the max-retained warning, so an authenticated user could probe another company's stock with a known item and batch. Gate the shared company -> warehouse resolution on read permission for the Company, which respects User Permissions. validate_sample_quantity only grew a company argument in this branch; move_sample_to_retention_warehouse already took one, so this closes that path too.
This commit is contained in:
@@ -1286,6 +1286,9 @@ def validate_sample_quantity(
|
||||
|
||||
|
||||
def get_sample_retention_warehouse(company: str) -> str:
|
||||
# `company` arrives from whitelisted callers, so it decides which company's stock gets read.
|
||||
frappe.has_permission("Company", "read", company, throw=True)
|
||||
|
||||
warehouse = frappe.get_cached_value("Company", company, "sample_retention_warehouse")
|
||||
if not warehouse:
|
||||
frappe.throw(
|
||||
|
||||
@@ -3260,6 +3260,40 @@ class TestStockEntryCoverage(ERPNextTestSuite):
|
||||
"_Sample Batch",
|
||||
)
|
||||
|
||||
def test_sample_retention_warehouse_denied_for_other_company(self):
|
||||
"""`company` comes from whitelisted callers, so it must not read another company's stock."""
|
||||
from erpnext.stock.doctype.stock_entry.services.manufacturing import (
|
||||
get_sample_retention_warehouse,
|
||||
)
|
||||
|
||||
frappe.db.set_value(
|
||||
"Company", "_Test Company", "sample_retention_warehouse", "_Test Warehouse 1 - _TC"
|
||||
)
|
||||
|
||||
user = "test_sample_retention_perm@example.com"
|
||||
if not frappe.db.exists("User", user):
|
||||
frappe.get_doc(
|
||||
{
|
||||
"doctype": "User",
|
||||
"email": user,
|
||||
"first_name": "Sample Retention",
|
||||
"send_welcome_email": 0,
|
||||
"roles": [{"role": "Stock User"}],
|
||||
}
|
||||
).insert(ignore_permissions=True)
|
||||
|
||||
frappe.get_doc(
|
||||
{
|
||||
"doctype": "User Permission",
|
||||
"user": user,
|
||||
"allow": "Company",
|
||||
"for_value": "_Test Company 1",
|
||||
}
|
||||
).insert(ignore_permissions=True)
|
||||
|
||||
with self.set_user(user):
|
||||
self.assertRaises(frappe.PermissionError, get_sample_retention_warehouse, "_Test Company")
|
||||
|
||||
# ── get_expired_batches ────────────────────────────────────────────────────
|
||||
|
||||
def test_get_expired_batches_includes_expired_batch(self):
|
||||
|
||||
Reference in New Issue
Block a user