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

[Tracker] Fix inline script execution on Checkout page according to CSP restrictions #3277

Conversation

vahonc
Copy link
Collaborator

@vahonc vahonc commented May 14, 2024

According to the CSP updates:

The default CSP configuration for payment pages for Commerce Admin and storefront areas is now restrict mode. For all other pages, the default configuration is report-only mode. In releases prior to 2.4.7, CSP was configured in report-only mode for all pages.

now an inline scripts should be wrapped using $secureRenderer variable in the .phtml templates, but for some reason this blocks the work of our Tracker script on Checkout page.

See more:

@vahonc vahonc force-pushed the 3270-elasticsuite-tracker-content-security-policy-2.11-fix branch from 67fa823 to da235f7 Compare May 27, 2024 16:43
@vahonc vahonc changed the title WIP: [Tracker] Fix inline script execution on Checkout page according to CSP restrictions [Tracker] Fix inline script execution on Checkout page according to CSP restrictions May 27, 2024
@romainruaud romainruaud merged commit 55f82f0 into Smile-SA:2.11.x Jun 4, 2024
13 checks passed
beaconUrl: '{$beaconUrl}',
telemetryUrl: '{$telemetryUrl}',
telemetryEnabled: '{$telemetryEnabled}',
sessionConfig: '{$sessionConfig}'
Copy link
Contributor

Choose a reason for hiding this comment

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

@vahonc: I believe there is a bug introduced on this line, the single quotes should not be needed around {$sessionConfig}. If the json data contains a single quote, this will break. In the old code, there also weren't single quotes surrounding this value.

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad, it looks like @romainruaud already fixed this shortly after merging this, in daff073, thanks guys!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes correct @hostep :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry guys, it's just a typo, my bad

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants