Skip to content

Commit

Permalink
fix(security): validate web form permissions correctly (backport #19088
Browse files Browse the repository at this point in the history
…) (#19108)

* fix: use permtype from passed arguments in has_web_form_permission when applying document permissions

(cherry picked from commit 856d7a9)

* fix: use webform doctype rather than allowing user to pass any doctype

(cherry picked from commit 8e0c4ce)

* chore: consider docname via data

(cherry picked from commit d7f4540)

# Conflicts:
#	frappe/public/js/frappe/web_form/web_form.js
#	frappe/website/doctype/web_form/web_form.py

* chore: remove docname param

* chore: remove docname property from args

Co-authored-by: phot0n <ritwikpuri5678@gmail.com>
  • Loading branch information
mergify[bot] and phot0n committed Dec 5, 2022
1 parent 851a803 commit 553408e
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
3 changes: 1 addition & 2 deletions frappe/public/js/frappe/web_form/web_form.js
Expand Up @@ -326,8 +326,7 @@ export default class WebForm extends frappe.ui.FieldGroup {
args: {
data: this.doc,
web_form: this.name,
docname: this.doc.name,
for_payment
for_payment,
},
callback: response => {
// Check for any exception in response
Expand Down
21 changes: 11 additions & 10 deletions frappe/website/doctype/web_form/web_form.py
Expand Up @@ -390,7 +390,7 @@ def has_web_form_permission(self, doctype, name, ptype="read"):
return False

if self.apply_document_permissions:
return frappe.get_doc(doctype, name).has_permission()
return frappe.get_doc(doctype, name).has_permission(permtype=ptype)

# owner matches
elif frappe.db.get_value(doctype, name, "owner") == frappe.session.user:
Expand All @@ -413,7 +413,7 @@ def get_web_form_module(doc):

@frappe.whitelist(allow_guest=True)
@rate_limit(key="web_form", limit=5, seconds=60, methods=["POST"])
def accept(web_form, data, docname=None, for_payment=False):
def accept(web_form, data, for_payment=False):
"""Save the web form"""
data = frappe._dict(json.loads(data))
for_payment = frappe.parse_json(for_payment)
Expand All @@ -422,19 +422,20 @@ def accept(web_form, data, docname=None, for_payment=False):
files_to_delete = []

web_form = frappe.get_doc("Web Form", web_form)
doctype = web_form.doc_type

if data.name and not web_form.allow_edit:
frappe.throw(_("You are not allowed to update this Web Form Document"))

frappe.flags.in_web_form = True
meta = frappe.get_meta(data.doctype)
meta = frappe.get_meta(doctype)

if docname:
if data.name:
# update
doc = frappe.get_doc(data.doctype, docname)
doc = frappe.get_doc(doctype, data.name)
else:
# insert
doc = frappe.new_doc(data.doctype)
doc = frappe.new_doc(doctype)

# set values
for field in web_form.web_form_fields:
Expand All @@ -459,7 +460,7 @@ def accept(web_form, data, docname=None, for_payment=False):
doc.run_method("validate_payment")

if doc.name:
if web_form.has_web_form_permission(doc.doctype, doc.name, "write"):
if web_form.has_web_form_permission(doctype, doc.name, "write"):
doc.save(ignore_permissions=True)
else:
# only if permissions are present
Expand All @@ -481,15 +482,15 @@ def accept(web_form, data, docname=None, for_payment=False):

# remove earlier attached file (if exists)
if doc.get(fieldname):
remove_file_by_url(doc.get(fieldname), doctype=doc.doctype, name=doc.name)
remove_file_by_url(doc.get(fieldname), doctype=doctype, name=doc.name)

# save new file
filename, dataurl = filedata.split(",", 1)
_file = frappe.get_doc(
{
"doctype": "File",
"file_name": filename,
"attached_to_doctype": doc.doctype,
"attached_to_doctype": doctype,
"attached_to_name": doc.name,
"content": dataurl,
"decode": True,
Expand All @@ -505,7 +506,7 @@ def accept(web_form, data, docname=None, for_payment=False):
if files_to_delete:
for f in files_to_delete:
if f:
remove_file_by_url(f, doctype=doc.doctype, name=doc.name)
remove_file_by_url(f, doctype=doctype, name=doc.name)

frappe.flags.web_form_doc = doc

Expand Down

0 comments on commit 553408e

Please sign in to comment.