Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Couponcode adminhtml updated pull #447

Merged
merged 7 commits into from
Feb 1, 2020

Conversation

seansan
Copy link
Contributor

@seansan seansan commented Feb 8, 2018

Couponcode adminhtml updated pull

  • added string
  • added %s
  • added to translation files
    no squash merge here otherwise would have bundled in 1

…view)

solve longstanding order shortcoming (so actual coupon code on order view in adminhtml)
now our logistics see the description @ totals overview: but have to ask the customer: which code was it? esp. handy when using generated codes
simple fix: please improve if you think this can be done better. 
Breaks nothing, quickwin for admin users
colinmollenhour
colinmollenhour previously approved these changes Mar 9, 2018
@pocallaghan
Copy link

Whilst I haven't investigated, how the values pass through the system, both $_order->getCouponCode() and $_order->getDiscountDescription() should likely be escaped before passing them to translate. Off the top of my head, I don't think a frontend user can insert an arbitrary value, so it's probable that you'd require admin access to exploit, but it's always good practice to escape such data. It's possible that with access to a limited administrative account, privileges could be escalated through use of persistent XSS in these fields.

@seansan
Copy link
Contributor Author

seansan commented Mar 14, 2018 via email

@colinmollenhour
Copy link
Member

Good observation, @pocallaghan. PR updated. Thanks!

Copy link

@tbaden tbaden left a comment

Choose a reason for hiding this comment

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

thanks for the work 👍
I did a code review: LGTM

@tbaden
Copy link

tbaden commented Jun 1, 2019

@Flyingmana

@Flyingmana Flyingmana merged commit efd5ff2 into OpenMage:1.9.3.x Feb 1, 2020
@sreichel sreichel added Component: Sales Relates to Mage_Sales Template : admin Relates to admin template labels Jun 8, 2020
Sekiphp pushed a commit to Sekiphp/magento-lts that referenced this pull request Apr 10, 2021
* solve longstanding order shortcoming (so actual coupon code on order view)

solve longstanding order shortcoming (so actual coupon code on order view in adminhtml)
now our logistics see the description @ totals overview: but have to ask the customer: which code was it? esp. handy when using generated codes
simple fix: please improve if you think this can be done better. 
Breaks nothing, quickwin for admin users

* Sloppy missed clsoing div

* Updated string

* Add coupon code translations

* Update Mage_Adminhtml.csv

* Added coupon code

* Escape coupon code and description in template.

Co-authored-by: Colin Mollenhour <colin@mollenhour.com>
Flyingmana pushed a commit that referenced this pull request Jul 30, 2021
* solve longstanding order shortcoming (so actual coupon code on order view)

solve longstanding order shortcoming (so actual coupon code on order view in adminhtml)
now our logistics see the description @ totals overview: but have to ask the customer: which code was it? esp. handy when using generated codes
simple fix: please improve if you think this can be done better. 
Breaks nothing, quickwin for admin users

* Sloppy missed clsoing div

* Updated string

* Add coupon code translations

* Update Mage_Adminhtml.csv

* Added coupon code

* Escape coupon code and description in template.

Co-authored-by: Colin Mollenhour <colin@mollenhour.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Sales Relates to Mage_Sales enhancement Template : admin Relates to admin template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants