ext/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-10-29 22:57:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #61551 from aerele/fix/currency-exchange-provider-validation

Browse Source 
fix(accounts): validate currency exchange provider requests and responses
This commit is contained in:
mergify[bot]
2026-10-26 10:21:29 +00:00
committed by GitHub
parent b4e37a0e74 06431307f0
commit 64b7380b65
8 changed files with 176 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
erpnext/accounts/doctype/currency_exchange_settings/currency_exchange_settings.py
+15 -3
View File
@@ -1,6 +1,8 @@
# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors
# For license information, please see license.txt
from math import isfinite
import frappe
import requests
from frappe import _
@@ -41,6 +43,14 @@ class CurrencyExchangeSettings(Document):
response, value = self.validate_parameters()
self.validate_result(response, value)
def on_update(self):
def clear_rates():
frappe.cache.delete_keys("currency_exchange_rate_")
clear_rates()
# Concurrent lookups can cache old provider rates before this save commits.
frappe.db.after_commit.add(clear_rates)
def set_parameters_and_result(self):
if self.service_provider == "exchangerate.host":
if not self.access_key:
@@ -88,7 +98,9 @@ class CurrencyExchangeSettings(Document):
api_url = self.api_endpoint.format(transaction_date=nowdate(), to_currency="INR", from_currency="USD")
try:
response = requests.get(api_url, params=params)
response = requests.get(
api_url, params=params, timeout=frappe.conf.get("currency_exchange_request_timeout") or 10
)
except requests.exceptions.RequestException as e:
frappe.throw(_("Error: {0}").format(str(e)))
@@ -105,8 +117,8 @@ class CurrencyExchangeSettings(Document):
]
except Exception:
frappe.throw(_("Invalid result key. Response:") + " " + response.text)
if not isinstance(value, int | float):
frappe.throw(_("Returned exchange rate is neither integer not float."))
if isinstance(value, bool) or not isinstance(value, int | float) or not isfinite(value) or value <= 0:
frappe.throw(_("Returned exchange rate must be a positive finite number."))
self.url = response.url
erpnext/accounts/doctype/currency_exchange_settings/test_currency_exchange_settings.py
+101 -4
View File
@@ -1,9 +1,106 @@
# Copyright (c) 2021, Wahni Green Technologies Pvt. Ltd. and contributors
# For license information, please see license.txt
# import frappe
from fnmatch import fnmatch
from unittest.mock import Mock, patch
from erpnext.tests.utils import ERPNextTestSuite
import frappe
from frappe.tests import UnitTestCase
from frappe.utils import CallbackManager
from frappe.utils.redis_wrapper import RedisWrapper
from erpnext.accounts.doctype.currency_exchange_settings.currency_exchange_settings import (
CurrencyExchangeSettings,
)
from erpnext.setup.utils import get_exchange_rate
class TestCurrencyExchangeSettings(ERPNextTestSuite):
pass
class TestCurrencyExchangeSettings(UnitTestCase):
def test_clear_provider_rates_after_commit(self):
with patch(
"erpnext.accounts.doctype.currency_exchange_settings.currency_exchange_settings.frappe"
) as mock_frappe:
mock_frappe.db.after_commit = CallbackManager()
rates = {"currency_exchange_rate_USD_INR": 80}
mock_frappe.cache.delete_keys.side_effect = lambda *args, **kwargs: rates.clear()
CurrencyExchangeSettings.on_update(None)
self.assertEqual(rates, {})
mock_frappe.cache.delete_keys.assert_called_once_with("currency_exchange_rate_")
# A concurrent lookup still sees the old settings until the save commits.
rates["currency_exchange_rate_USD_INR"] = 80
mock_frappe.db.after_commit.run()
self.assertEqual(rates, {})
self.assertEqual(mock_frappe.cache.delete_keys.call_count, 2)
mock_frappe.cache.delete_keys.assert_called_with("currency_exchange_rate_")
def test_website_user_can_use_local_rates_but_not_request_provider(self):
with (
patch("erpnext.setup.utils.frappe") as mock_frappe,
patch("erpnext.setup.utils.is_system_user", return_value=False),
patch("requests.get") as request,
):
mock_frappe.PermissionError = frappe.PermissionError
mock_frappe.throw.side_effect = frappe.PermissionError
mock_frappe.get_cached_doc.return_value = frappe._dict(allow_stale=1)
mock_frappe.get_single_value.return_value = 0
mock_frappe.get_all.return_value = [frappe._dict(exchange_rate=80)]
self.assertEqual(get_exchange_rate("USD", "INR", "2026-01-01"), 80)
self.assertEqual(get_exchange_rate("USD", "USD", "2026-01-01"), 1)
mock_frappe.get_all.return_value = []
mock_frappe.cache.return_value.get_value.return_value = 81
self.assertEqual(get_exchange_rate("USD", "INR", "2026-01-01"), 81)
mock_frappe.cache.return_value.get_value.return_value = None
with self.assertRaises(frappe.PermissionError):
get_exchange_rate("USD", "INR", "2026-01-01")
request.assert_not_called()
def test_provider_cache_is_isolated_between_sites(self):
cache = RedisWrapper()
rates = {}
settings = frappe._dict(
allow_stale=1,
api_endpoint="https://example.com/rates",
req_params=[],
result_key=[],
)
with (
patch.object(frappe.local, "conf", frappe._dict(db_name="site_one")),
patch.object(frappe.local, "cache", {}),
patch.object(frappe, "cache", cache),
patch.object(frappe, "db", Mock()) as database,
patch.object(frappe, "get_all", return_value=[]),
patch.object(frappe, "get_single_value", return_value=0),
patch.object(frappe, "get_cached_doc", return_value=settings),
patch("erpnext.setup.utils.is_system_user", return_value=True),
patch("requests.get") as request,
patch.object(cache, "get", side_effect=rates.get),
patch.object(cache, "set") as store,
patch.object(
cache, "keys", side_effect=lambda pattern: [key for key in rates if fnmatch(key, pattern)]
),
patch.object(cache, "unlink", side_effect=lambda *keys: [rates.pop(key, None) for key in keys]),
):
store.side_effect = lambda name, value, ex: rates.update({name: value})
request.return_value.json.side_effect = [80, 90]
self.assertEqual(get_exchange_rate("USD", "INR", "2026-01-01"), 80)
self.assertEqual(store.call_args.kwargs["ex"], 21600)
frappe.local.conf.db_name = "site_two"
self.assertEqual(get_exchange_rate("USD", "INR", "2026-01-01"), 90)
self.assertEqual(request.call_count, 2)
frappe.local.conf.db_name = "site_one"
self.assertEqual(get_exchange_rate("USD", "INR", "2026-01-01"), 80)
database.after_commit = CallbackManager()
CurrencyExchangeSettings.on_update(None)
database.after_commit.run()
self.assertIsNone(cache.get_value("currency_exchange_rate_2026-01-01:USD:INR"))
frappe.local.conf.db_name = "site_two"
self.assertEqual(get_exchange_rate("USD", "INR", "2026-01-01"), 90)
self.assertEqual(request.call_count, 2)
erpnext/accounts/doctype/pricing_rule/test_pricing_rule.py
+14 -14
View File
@@ -43,10 +43,10 @@ class TestPricingRule(ERPNextTestSuite):
"item_code": "_Test Item",
"company": "_Test Company",
"price_list": "_Test Price List",
"currency": "_Test Currency",
"currency": "INR",
"doctype": "Sales Order",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"order_type": "Sales",
"customer": "_Test Customer",
@@ -174,10 +174,10 @@ class TestPricingRule(ERPNextTestSuite):
"item_code": "_Test FG Item 2",
"company": "_Test Company",
"price_list": "_Test Price List",
"currency": "_Test Currency",
"currency": "INR",
"doctype": "Sales Order",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"order_type": "Sales",
"customer": "_Test Customer",
@@ -222,10 +222,10 @@ class TestPricingRule(ERPNextTestSuite):
"item_group": "Products",
"company": "_Test Company",
"price_list": "_Test Price List",
"currency": "_Test Currency",
"currency": "INR",
"doctype": "Sales Order",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"order_type": "Sales",
"customer": "_Test Customer",
@@ -262,10 +262,10 @@ class TestPricingRule(ERPNextTestSuite):
"item_code": "_Test Item",
"company": "_Test Company",
"price_list": "_Test Price List",
"currency": "_Test Currency",
"currency": "INR",
"doctype": "Sales Order",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"order_type": "Sales",
"customer": "_Test Customer",
@@ -329,10 +329,10 @@ class TestPricingRule(ERPNextTestSuite):
"item_code": "Test Variant PRT",
"company": "_Test Company",
"price_list": "_Test Price List",
"currency": "_Test Currency",
"currency": "INR",
"doctype": "Sales Order",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"order_type": "Sales",
"customer": "_Test Customer",
@@ -634,10 +634,10 @@ class TestPricingRule(ERPNextTestSuite):
"item_code": "_Test Item",
"company": "_Test Company",
"price_list": "_Test Price List",
"currency": "_Test Currency",
"currency": "INR",
"doctype": "Sales Invoice",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"order_type": "Sales",
"customer": "_Test Customer",
@@ -1050,7 +1050,7 @@ class TestPricingRule(ERPNextTestSuite):
"currency": "USD",
"doctype": "Sales Invoice",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"order_type": "Sales",
"customer": "_Test Customer",
@@ -1070,7 +1070,7 @@ class TestPricingRule(ERPNextTestSuite):
"currency": "INR",
"doctype": "Sales Invoice",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"order_type": "Sales",
"customer": "_Test Customer",
erpnext/selling/doctype/quotation/test_quotation.py
+1 -1
View File
@@ -1460,7 +1460,7 @@ class TestQuotation(ERPNextTestSuite):
cache = frappe.cache()
key = "currency_exchange_rate_{}:{}:{}".format("2026-01-01", "QAR", "INR")
value = cache.get(key)
value = cache.get_value(key)
expected_rate = flt(value) / 3.64
self.assertEqual(
erpnext/setup/doctype/currency_exchange/test_currency_exchange.py
+1 -1
View File
@@ -89,7 +89,7 @@ class TestCurrencyExchange(ERPNextTestSuite):
cache = frappe.cache()
for date in test_exchange_values.keys():
key = "currency_exchange_rate_{}:{}:{}".format(date, "USD", "INR")
cache.delete(key)
cache.delete_value(key)
def test_exchange_rate(self, mock_get):
frappe.db.set_single_value("Accounts Settings", "allow_stale", 1)
erpnext/setup/utils.py
+37 -6
View File
@@ -1,9 +1,13 @@
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
# License: GNU General Public License v3. See license.txt
from math import isfinite
from urllib.parse import quote
import frappe
from frappe import _
from frappe.utils import add_days, flt, get_datetime_str, nowdate
from frappe.permissions import is_system_user
from frappe.utils import add_days, flt, get_datetime_str, getdate, nowdate
from frappe.utils.data import DateTimeLikeObject
from frappe.utils.nestedset import get_root_of
@@ -68,11 +72,17 @@ def get_exchange_rate(
if not (from_currency and to_currency):
# manqala 19/09/2016: Should this be an empty return or should it throw and exception?
return
for currency in (from_currency, to_currency):
if not isinstance(currency, str) or not frappe.db.exists("Currency", currency):
frappe.throw(_("Invalid currency: {0}").format(currency))
if from_currency == to_currency:
return 1
transaction_date = getdate(transaction_date)
if not transaction_date:
transaction_date = nowdate()
frappe.throw(_("Invalid transaction date"))
transaction_date = transaction_date.isoformat()
currency_settings = frappe.get_cached_doc("Accounts Settings")
allow_stale_rates = currency_settings.get("allow_stale")
@@ -117,9 +127,15 @@ def get_exchange_rate(
try:
cache = frappe.cache()
key = f"currency_exchange_rate_{transaction_date}:{from_currency}:{to_currency}"
value = cache.get(key)
value = cache.get_value(key)
if not value:
if not is_system_user():
frappe.throw(
_("Only system users can fetch exchange rates from an external provider."),
frappe.PermissionError,
)
import requests
settings = frappe.get_cached_doc("Currency Exchange Settings")
@@ -135,16 +151,29 @@ def get_exchange_rate(
params = {}
for row in settings.req_params:
params[row.key] = format_ces_api(row.value, req_params)
response = requests.get(format_ces_api(settings.api_endpoint, req_params), params=params)
response = requests.get(
format_ces_api(
settings.api_endpoint, {key: quote(value, safe="") for key, value in req_params.items()}
),
params=params,
timeout=frappe.conf.get("currency_exchange_request_timeout") or 10,
)
# expire in 6 hours
response.raise_for_status()
value = response.json()
for res_key in settings.result_key:
value = value[format_ces_api(str(res_key.key), req_params)]
cache.setex(name=key, time=21600, value=flt(value))
if isinstance(value, bool):
raise ValueError("Invalid exchange rate")
value = float(value)
if not isfinite(value) or value <= 0:
raise ValueError("Exchange rate must be a positive finite number")
cache.set_value(key, value, expires_in_sec=21600)
# Support multiple pegged currencies
value = flt(value)
value = float(value)
if not isfinite(value) or value <= 0:
raise ValueError("Exchange rate must be a positive finite number")
if currency_settings.allow_pegged_currencies_exchange_rates and to_currency in pegged_currencies:
value *= flt(pegged_currencies[to_currency]["ratio"])
@@ -152,6 +181,8 @@ def get_exchange_rate(
value /= flt(pegged_currencies[from_currency]["ratio"])
return flt(value)
except frappe.PermissionError:
raise
except Exception:
frappe.log_error("Unable to fetch exchange rate")
frappe.msgprint(
erpnext/stock/doctype/batch/test_batch.py
+1 -1
View File
@@ -746,7 +746,7 @@ class TestBatch(ERPNextTestSuite):
"currency": currency,
"doctype": "Sales Invoice",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"customer": "_Test Customer",
"name": None,
erpnext/stock/doctype/item/test_item.py
+6 -6
View File
@@ -325,10 +325,10 @@ class TestItem(ERPNextTestSuite):
"tax_category": data["tax_category"],
"company": "_Test Company",
"price_list": "_Test Price List",
"currency": "_Test Currency",
"currency": "INR",
"doctype": "Sales Order",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"order_type": "Sales",
"customer": "_Test Customer",
@@ -374,10 +374,10 @@ class TestItem(ERPNextTestSuite):
"item_code": "Test Item With Defaults",
"company": "_Test Company",
"price_list": "_Test Price List",
"currency": "_Test Currency",
"currency": "INR",
"doctype": "Sales Invoice",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"customer": "_Test Customer",
}
@@ -399,10 +399,10 @@ class TestItem(ERPNextTestSuite):
"item_code": "Test Item With Defaults",
"company": "_Test Company",
"price_list": "_Test Price List",
"currency": "_Test Currency",
"currency": "INR",
"doctype": "Purchase Invoice",
"conversion_rate": 1,
"price_list_currency": "_Test Currency",
"price_list_currency": "INR",
"plc_conversion_rate": 1,
"supplier": "_Test Supplier",
}