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

Remove inline handlers in cart, payment account page #2536

Merged
merged 5 commits into from
Feb 5, 2025

Conversation

rtalvarez
Copy link
Member

@rtalvarez rtalvarez commented Feb 3, 2025

What?

Per PCI, inline event handlers like onload, onclick, onsubmit, etc, are forbidden for security reasons. Instead of doing this, we should attach the relevant handlers via a trusted script (protected by nonce)

This PR removes the following inline event handlers in order to comply with PCI since they are executed on pages that fall under PCI compliance (the Cart page and the Payments Account page)

  • Gift wrapping remove anchor tag fires a confirm dialog that is currently handled inline. I've switched this to an event listener instead through cart.js, which is already protected by nonce
  • The quick search functionality has an inline event handler that is safe to remove.
  • The main bundle event handler has been removed. I've switched this to an event listener instead and protected it via nonce

Requirements

  • CHANGELOG.md entry added (required for code changes only)

Screenshots (if appropriate)

Cart Gift Wrapping Option Testing

Confirm dialog pops up

Screenshot 2025-02-03 at 10 57 10 AM

After confirm, page is reloaded and message appears

Screenshot 2025-02-03 at 10 57 18 AM

Quick search

Search correctly previews results
Screenshot 2025-02-03 at 10 57 46 AM
Search works after pressing enter
Screenshot 2025-02-03 at 10 57 54 AM

Main Bundle

The relevant handler for the bundle is attached and executed correctly

Screenshot 2025-02-03 at 11 06 21 AM

@rtalvarez rtalvarez changed the title Remove inline handler Remove inline handlers in cart, payment account page Feb 3, 2025
@rtalvarez rtalvarez marked this pull request as ready for review February 4, 2025 15:43
CHANGELOG.md Outdated Show resolved Hide resolved
@rtalvarez rtalvarez merged commit 8447dd3 into master Feb 5, 2025
2 checks passed
@rtalvarez rtalvarez deleted the remove-inline-handler branch February 5, 2025 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants