[FW][FIX] sale_quotation_builder: checkout product with blockquote description#122154
Closed
fw-bot wants to merge 1 commit into
Closed
[FW][FIX] sale_quotation_builder: checkout product with blockquote description#122154fw-bot wants to merge 1 commit into
fw-bot wants to merge 1 commit into
Conversation
Current behaviour: If we add a blockquote in the `website_description` of a product on the e-shop, we cannot checkout the product. Silent HTTP 400 error code, due to an exception raised by https://github.com/odoo/odoo/blob/bf772181933ce5334da35c8368455963b2478399/odoo/fields.py#L1987-L1993 Expected behaviour: You should be able to checkout products even if they have blockquote in their `website_description`. Steps to reproduce: - Install eCommerce, sale_quotation_builder (issue is present only after installing sale_quotation_builder) - On a product, with the website editor, add a `blockquote` to the description of the product > Save - In a private browser window, as public user, visit the product on the e-shop and try to checkout with it. - Observe there is no visible error, and we do not proceed in the checkout process. Reason for the problem: The exception mentioned above is triggered when there is a difference between the html content that is saved in the DB and after sanitization, meaning that someone with escalated privilege saved the HTML content by overriding the sanitization with `sanitize_overridable`. In our use case the only diff is the presence of the attribute `data-o-mail-quote-node` which is removed after the sanitization. Fix: This issue can be resolved two ways: 1) Adding `data-o-mail-quote-node` to the list of save attributes, meaning it will not be removed during the sanitization process. Since this is an attribute that we add on `<blockquote>` nodes, it can be considered safe, just like `data-o-mail-quote`. 2) Remove the attribute sanitization of the `website_description`, just like it is done in the website_sale module. Since the `website_description` and `quotation_description` are both computed from one-another, they should have the same sanitization level. I am implementing both solutions, 1) because adding the attribute to the safe list seems safe in general, and may prevent future issues of this sort. 2) because it is the root cause of the issue, since the bug is present only after installation of the `sale_quotation_builder` module. Affected versions: - 16.0 - saas-16.1 - saas-16.2 - master opw-3297237 X-original-commit: 2302214
Contributor
Contributor
Author
|
@pivi-odoo @xmo-odoo this PR targets master and is the last of the forward-port chain containing:
To merge the full chain, say
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port |
Contributor
|
@fw-bot r+ |
Contributor
|
@pivi-odoo @xmo-odoo staging failed: ci/runbot on 9ee0749355ed1494e0a53b54aa34da01a71da286 (view more at https://runbot.odoo.com/runbot/build/40379721) |
Contributor
|
@robodoo retry |
robodoo
pushed a commit
that referenced
this pull request
May 24, 2023
Current behaviour: If we add a blockquote in the `website_description` of a product on the e-shop, we cannot checkout the product. Silent HTTP 400 error code, due to an exception raised by https://github.com/odoo/odoo/blob/bf772181933ce5334da35c8368455963b2478399/odoo/fields.py#L1987-L1993 Expected behaviour: You should be able to checkout products even if they have blockquote in their `website_description`. Steps to reproduce: - Install eCommerce, sale_quotation_builder (issue is present only after installing sale_quotation_builder) - On a product, with the website editor, add a `blockquote` to the description of the product > Save - In a private browser window, as public user, visit the product on the e-shop and try to checkout with it. - Observe there is no visible error, and we do not proceed in the checkout process. Reason for the problem: The exception mentioned above is triggered when there is a difference between the html content that is saved in the DB and after sanitization, meaning that someone with escalated privilege saved the HTML content by overriding the sanitization with `sanitize_overridable`. In our use case the only diff is the presence of the attribute `data-o-mail-quote-node` which is removed after the sanitization. Fix: This issue can be resolved two ways: 1) Adding `data-o-mail-quote-node` to the list of save attributes, meaning it will not be removed during the sanitization process. Since this is an attribute that we add on `<blockquote>` nodes, it can be considered safe, just like `data-o-mail-quote`. 2) Remove the attribute sanitization of the `website_description`, just like it is done in the website_sale module. Since the `website_description` and `quotation_description` are both computed from one-another, they should have the same sanitization level. I am implementing both solutions, 1) because adding the attribute to the safe list seems safe in general, and may prevent future issues of this sort. 2) because it is the root cause of the issue, since the bug is present only after installation of the `sale_quotation_builder` module. Affected versions: - 16.0 - saas-16.1 - saas-16.2 - master opw-3297237 closes #122154 X-original-commit: 2302214 Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com> Signed-off-by: Piryns Victor (pivi) <pivi@odoo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Current behaviour
If we add a blockquote in the
website_descriptionof a product on the e-shop, we cannot checkout the product. Silent HTTP 400 error code, due to an exception raised byodoo/odoo/fields.py
Lines 1987 to 1993 in bf77218
Expected behaviour
You should be able to checkout products even if they have blockquote in their
website_description.Steps to reproduce
blockquoteto the description of the product > SaveReason for the problem
The exception mentioned above is triggered when there is a difference between the html content that is saved in the DB and after sanitization, meaning that someone with escalated privilege saved the HTML content by overriding the sanitization with
sanitize_overridable. In our use case the only diff is the presence of the attributedata-o-mail-quote-nodewhich is removed after the sanitization.Fix
This issue can be resolved two ways:
data-o-mail-quote-nodeto the list of save attributes,meaning it will not be removed during the sanitization process.
Since this is an attribute that we add on
<blockquote>nodes,it can be considered safe, just like
data-o-mail-quote.website_description,just like it is done in the website_sale module.
Since the
website_descriptionandquotation_descriptionare bothcomputed from one-another, they should have the same sanitization
level.
I am implementing both solutions, 1) because adding the attribute to the safe list seems safe in general, and may prevent future issues of this sort. 2) because it is the root cause of the issue, since the bug is present only after installation of the
sale_quotation_buildermodule.Affected versions
opw-3297237
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr
Forward-Port-Of: #120764