fix(frappe client): validate http method for frappe.client api (#11228)

frappe/__init__.py

@@ -514,12 +514,15 @@ def sendmail(recipients=[], sender="", subject="No Subject", message="No Message
 whitelisted = []
 guest_methods = []
 xss_safe_methods = []
-def whitelist(allow_guest=False, xss_safe=False):
+allowed_http_methods_for_whitelisted_func = {}
+
+def whitelist(allow_guest=False, xss_safe=False, methods=None):
 	"""
 	Decorator for whitelisting a function and making it accessible via HTTP.
 	Standard request will be `/api/method/[path.to.method]`
 
 	:param allow_guest: Allow non logged-in user to access this method.
+	:param methods: Allowed http method to access the method.
 
 	Use as:
 
@@ -527,10 +530,16 @@ def whitelist(allow_guest=False, xss_safe=False):
 		def myfunc(param1, param2):
 			pass
 	"""
+
+	if not methods:
+		methods = ['GET', 'POST', 'PUT', 'DELETE']
+
 	def innerfn(fn):
-		global whitelisted, guest_methods, xss_safe_methods
+		global whitelisted, guest_methods, xss_safe_methods, allowed_http_methods_for_whitelisted_func
 		whitelisted.append(fn)
 
+		allowed_http_methods_for_whitelisted_func[fn] = methods
+
 		if allow_guest:
 			guest_methods.append(fn)
 

frappe/client.py

@@ -107,7 +107,7 @@ def get_single_value(doctype, field):
 	value = frappe.db.get_single_value(doctype, field)
 	return value
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def set_value(doctype, name, fieldname, value=None):
 	'''Set a value using get_doc, group of values
 
@@ -142,7 +142,7 @@ def set_value(doctype, name, fieldname, value=None):
 
 	return doc.as_dict()
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def insert(doc=None):
 	'''Insert a document
 
@@ -160,7 +160,7 @@ def insert(doc=None):
 		doc = frappe.get_doc(doc).insert()
 		return doc.as_dict()
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def insert_many(docs=None):
 	'''Insert multiple documents
 
@@ -186,7 +186,7 @@ def insert_many(docs=None):
 
 	return out
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def save(doc):
 	'''Update (save) an existing document
 
@@ -199,7 +199,7 @@ def save(doc):
 
 	return doc.as_dict()
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def rename_doc(doctype, old_name, new_name, merge=False):
 	'''Rename document
 
@@ -209,7 +209,7 @@ def rename_doc(doctype, old_name, new_name, merge=False):
 	new_name = frappe.rename_doc(doctype, old_name, new_name, merge=merge)
 	return new_name
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def submit(doc):
 	'''Submit a document
 
@@ -222,7 +222,7 @@ def submit(doc):
 
 	return doc.as_dict()
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def cancel(doctype, name):
 	'''Cancel a document
 
@@ -233,7 +233,7 @@ def cancel(doctype, name):
 
 	return wrapper.as_dict()
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['DELETE', 'POST'])
 def delete(doctype, name):
 	'''Delete a remote document
 
@@ -241,13 +241,13 @@ def delete(doctype, name):
 	:param name: name of the document to be deleted'''
 	frappe.delete_doc(doctype, name, ignore_missing=False)
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def set_default(key, value, parent=None):
 	"""set a user default value"""
 	frappe.db.set_default(key, value, parent or frappe.session.user)
 	frappe.clear_cache(user=frappe.session.user)
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def make_width_property_setter(doc):
 	'''Set width Property Setter
 
@@ -257,7 +257,7 @@ def make_width_property_setter(doc):
 	if doc["doctype"]=="Property Setter" and doc["property"]=="width":
 		frappe.get_doc(doc).insert(ignore_permissions = True)
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def bulk_update(docs):
 	'''Bulk update documents
 
@@ -333,7 +333,7 @@ def get_time_zone():
 	'''Returns default time zone'''
 	return {"time_zone": frappe.defaults.get_defaults().get("time_zone")}
 
-@frappe.whitelist()
+@frappe.whitelist(methods=['POST', 'PUT'])
 def attach_file(filename=None, filedata=None, doctype=None, docname=None, folder=None, decode_base64=False, is_private=None, docfield=None):
 	'''Attach a file to Document (POST)
 

frappe/handler.py

@@ -65,16 +65,21 @@ def execute_cmd(cmd, from_async=False):
 		method = method.queue
 
 	is_whitelisted(method)
+	is_valid_http_method(method)
 
 	return frappe.call(method, **frappe.form_dict)
 
+def is_valid_http_method(method):
+	http_method = frappe.local.request.method
+
+	if http_method not in frappe.allowed_http_methods_for_whitelisted_func[method]:
+		frappe.throw(_("Not permitted"), frappe.PermissionError)
 
 def is_whitelisted(method):
 	# check if whitelisted
 	if frappe.session['user'] == 'Guest':
 		if (method not in frappe.guest_methods):
-			frappe.msgprint(_("Not permitted"))
-			raise frappe.PermissionError('Not Allowed, {0}'.format(method))
+			frappe.throw(_("Not permitted"), frappe.PermissionError)
 
 		if method not in frappe.xss_safe_methods:
 			# strictly sanitize form_dict
@@ -85,8 +90,7 @@ def is_whitelisted(method):
 
 	else:
 		if not method in frappe.whitelisted:
-			frappe.msgprint(_("Not permitted"))
-			raise frappe.PermissionError('Not Allowed, {0}'.format(method))
+			frappe.throw(_("Not permitted"), frappe.PermissionError)
 
 @frappe.whitelist(allow_guest=True)
 def version():

frappe/tests/test_client.py

@@ -21,3 +21,37 @@ class TestClient(unittest.TestCase):
 
 		self.assertFalse(frappe.db.exists("ToDo", todo.name))
 		self.assertRaises(frappe.DoesNotExistError, delete, "ToDo", todo.name)
+
+	def test_http_valid_method_access(self):
+		from frappe.client import delete
+		from frappe.handler import execute_cmd
+
+		frappe.set_user("Administrator")
+
+		frappe.local.request = frappe._dict()
+		frappe.local.request.method = 'POST'
+
+		frappe.local.form_dict = frappe._dict({
+			'doc': dict(doctype='ToDo', description='Valid http method'),
+			'cmd': 'frappe.client.save'
+		})
+		todo = execute_cmd('frappe.client.save')
+
+		self.assertEqual(todo.get('description'), 'Valid http method')
+
+		delete("ToDo", todo.name)
+
+	def test_http_invalid_method_access(self):
+		from frappe.handler import execute_cmd
+
+		frappe.set_user("Administrator")
+
+		frappe.local.request = frappe._dict()
+		frappe.local.request.method = 'GET'
+
+		frappe.local.form_dict = frappe._dict({
+			'doc': dict(doctype='ToDo', description='Invalid http method'),
+			'cmd': 'frappe.client.save'
+		})
+
+		self.assertRaises(frappe.PermissionError, execute_cmd, 'frappe.client.save')