-
Notifications
You must be signed in to change notification settings - Fork 113
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
Updated content security policy to allow script and style from google analytics #2649
Updated content security policy to allow script and style from google analytics #2649
Conversation
👷 Deploy Preview for chef-supermarket processing.
|
ad hoc build has passed |
ga('create', '<%= ENV["GOOGLE_ANALYTICS_ID"] %>', 'auto'); | ||
ga('require', 'linkid', 'linkid.js'); | ||
ga('require', 'displayfeatures'); | ||
ga('send', 'pageview'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This snippet doesn't seem to be working. Got the reference to the new script from this link: https://developers.google.com/analytics/devguides/collection/gtagjs
@RajeshPaul38 Can we have screenshots with the PR to make sure that google analytics are working fine with this change ? |
Sure, let me attach |
Here is how the data is getting reflected in google analytics. @msys-sgarg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
@RajeshPaul38 I hope we took a look into the script injection possibilities with this change ? |
d1b4c6e
to
de39c3f
Compare
@msys-sgarg thanks for pointing out the possible chances of XSS attack. I've tried to mitigate it as much as possible by disabling inline scripts and only keeping the inline css enabled. I've used a CSP mechanism called nonce which ensures only the inline scripts that is signed with a valid nonce, will be allowed by the CSP. This nonce is a hash that gets generated uniquely for every request. Hence the chances of XSS attack is zero as no one else will know what will be the nonce hash for a certain request. Hope this is a satisfactory resolution for the issues pointed out during the demo. If yes pls approve. |
Can we get this merged with priority @saghoshprogress as this is a customer bug? |
Ready for merge @saghoshprogress |
@RajeshPaul38 Deploy preview check failed. please take a look! |
a580fa5
to
c42e169
Compare
f24ae79
to
c42e169
Compare
I've tried debugging the issue with netlify build. But seems like an issue in the build process, not related to this PR. Pls approve. |
… analytics scripts Signed-off-by: Rajesh Paul <rajesh.paul@progress.com>
Signed-off-by: Rajesh Paul <rajesh.paul@progress.com>
Signed-off-by: Rajesh Paul <rajesh.paul@progress.com>
Signed-off-by: Rajesh Paul <rajesh.paul@progress.com>
…nline scripts Signed-off-by: Rajesh Paul <rajesh.paul@progress.com>
Signed-off-by: Rajesh Paul <rajesh.paul@progress.com>
c42e169
to
058fccd
Compare
Signed-off-by: Rajesh Paul rajesh.paul@progress.com
Description
[Please describe what this change achieves]
Issues Resolved
[List any existing issues this PR resolves, or any Discourse or
StackOverflow discussions that are relevant]
Check List