Skip to content

Commit

Permalink
fix: improved validation for address and contact
Browse files Browse the repository at this point in the history 
(cherry picked from commit 20178bd)

# Conflicts:
#	frappe/contacts/doctype/contact/contact.py
  • Loading branch information
@sagarvora @mergify
sagarvora authored and mergify[bot] committed Sep 29, 2023
1 parent 30e25e9 commit ef5709a
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
4 changes: 3 additions & 1 deletion frappe/contacts/doctype/address/address.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ def get_address_display(address_dict: dict | str | None = None) -> str | None:
return

if not isinstance(address_dict, dict):
address_dict = frappe.db.get_value("Address", address_dict, "*", as_dict=True, cache=True) or {}
address = frappe.get_cached_doc("Address", address_dict)
address.check_permission()
address_dict = address.as_dict()

name, template = get_address_templates(address_dict)

Expand Down
6 changes: 6 additions & 0 deletions frappe/contacts/doctype/contact/contact.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,13 @@ def invite_user(contact):
@frappe.whitelist()
def get_contact_details(contact):
contact = frappe.get_doc("Contact", contact)
<<<<<<< HEAD
out = {
=======
contact.check_permission()

return {
>>>>>>> 20178bd3eb (fix: improved validation for address and contact)
"contact_person": contact.get("name"),
"contact_display": " ".join(
filter(None, [contact.get("salutation"), contact.get("first_name"), contact.get("last_name")])
Expand Down

6 comments on commit ef5709a

@ophl55
Copy link

@ophl55 ophl55 commented on ef5709a Oct 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sagarvora the introduced check_permission() throws an Permission Error if users with the role "Customer" try to use the shopping cart.
Can you please check?

Here the error message:

website.js:177 Traceback (most recent call last):
File "apps/frappe/frappe/app.py", line 84, in application
init_request(request)
File "apps/frappe/frappe/app.py", line 172, in init_request
frappe.local.http_request = frappe.auth.HTTPRequest()
File "apps/frappe/frappe/auth.py", line 34, in init
self.set_session()
File "apps/frappe/frappe/auth.py", line 70, in set_session
frappe.local.login_manager = LoginManager()
File "apps/frappe/frappe/auth.py", line 115, in init
self.run_trigger("on_session_creation")
File "apps/frappe/frappe/auth.py", line 298, in run_trigger
frappe.call(frappe.get_attr(method), login_manager=self)
File "apps/frappe/frappe/init.py", line 1622, in call
return fn(*args, **newargs)
File "apps/erpnext/erpnext/e_commerce/shopping_cart/utils.py", line 32, in set_cart_count
set_cart_count()
File "apps/erpnext/erpnext/e_commerce/shopping_cart/cart.py", line 26, in set_cart_count
quotation = _get_cart_quotation()
File "apps/erpnext/erpnext/e_commerce/shopping_cart/cart.py", line 379, in _get_cart_quotation
qdoc.run_method("set_missing_values")
File "apps/frappe/frappe/model/document.py", line 917, in run_method
out = Document.hook(fn)(self, *args, **kwargs)
File "apps/frappe/frappe/model/document.py", line 1279, in composer
return composed(self, method, *args, **kwargs)
File "apps/frappe/frappe/model/document.py", line 1261, in runner
add_to_return_value(self, fn(self, *args, **kwargs))
File "apps/frappe/frappe/model/document.py", line 914, in fn
return method_object(*args, **kwargs)
File "apps/erpnext/erpnext/controllers/selling_controller.py", line 49, in set_missing_values
self.set_missing_lead_customer_details(for_validate=for_validate)
File "apps/erpnext/erpnext/controllers/selling_controller.py", line 76, in set_missing_lead_customer_details
party_details = _get_party_details(
File "apps/erpnext/erpnext/accounts/party.py", line 127, in _get_party_details
party_address, shipping_address = set_address_details(
File "apps/erpnext/erpnext/accounts/party.py", line 208, in set_address_details
party_details.address_display = get_address_display(party_details[billing_address_field])
File "apps/frappe/frappe/contacts/doctype/address/address.py", line 135, in get_address_display
address.check_permission()
File "apps/frappe/frappe/model/document.py", line 195, in check_permission
self.raise_no_permission_to(permtype)
File "apps/frappe/frappe/model/document.py", line 217, in raise_no_permission_to
raise frappe.PermissionError
frappe.exceptions.PermissionError

@sagarvora
Copy link
Collaborator Author

@sagarvora sagarvora commented on ef5709a Oct 16, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was fixed in #22604

Which version are you using? The fix seems to have been released in v14.52.

@ophl55
Copy link

@ophl55 ophl55 commented on ef5709a Oct 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on 14.52.0. Please find the bug report here: #22747

@ophl55
Copy link

@ophl55 ophl55 commented on ef5709a Oct 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for you hint. I have exported customizations and therefore maybe my code overrites the new permission for the docs.
I will check it immediately.

@ophl55
Copy link

@ophl55 ophl55 commented on ef5709a Oct 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I restored all original permissions and the error persists.

Maybe it is because the user can only see the addresses he created. But the address for the customer also could have been created by a customer service employee.
image

@ophl55
Copy link

@ophl55 ophl55 commented on ef5709a Oct 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is working, if I remove the "Only if Creator" check

Please sign in to comment.