-
Notifications
You must be signed in to change notification settings - Fork 38
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
Move to Google Analytics 4 (#1371) #1496
Conversation
config/env_sample.hjson
Outdated
@@ -27,7 +27,7 @@ | |||
"REPORT_ONLY": true, | |||
# You should set your domain as the DEFAULT_SRC | |||
"DEFAULT_SRC": ["'self'","example.edu"], | |||
"SCRIPT_SRC": ["'self'", "'unsafe-inline'", "'unsafe-eval'", "www.google-analytics.com"], | |||
"SCRIPT_SRC": ["'self'", "'unsafe-inline'", "'unsafe-eval'", "www.googletagmanager.com"], |
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.
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.
I think this is fine. I feel like passing the nonce isn't super necessary here but looks like you already set it all up so should work. With us still having unsafe-inline
for the SCRIPT_SRC
that kind of makes passing a nonce unnecessary.
I'm not sure if this is the only inline script, if it is and we could remove unsafe-inline
that'd be good!
… CSP from Google docs
"OBJECT_SRC": [], | ||
"MEDIA_SRC": [], | ||
# If you're embedding in Canvas you may need to include instructure.com as FRAME_SRC | ||
"FRAME_SRC": [], | ||
"FONT_SRC": ["'self'", "fonts.gstatic.com"], | ||
"CONNECT_SRC": [], | ||
"CONNECT_SRC": ["*.google-analytics.com", "*.analytics.google.com", "*.googletagmanager.com"], | ||
"STYLE_SRC": ["'self'", "'unsafe-inline'"], |
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.
We may be able to remove unsafe-inline
from style-src
using the nonce added here.
@@ -27,14 +27,14 @@ | |||
"REPORT_ONLY": true, | |||
# You should set your domain as the DEFAULT_SRC | |||
"DEFAULT_SRC": ["'self'","example.edu"], | |||
"SCRIPT_SRC": ["'self'", "'unsafe-inline'", "'unsafe-eval'", "www.google-analytics.com"], | |||
"IMG_SRC": ["'self'", "data:", "www.google-analytics.com"], | |||
"SCRIPT_SRC": ["'self'", "'unsafe-inline'", "*.googletagmanager.com"], |
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.
Unfortunately, looks like the pylti1p3
's script
tags are preventing us from removing unsafe-inline
entirely here. It's no longer necessary for Google Analytics since we're passing the nonce.
GoogleAnalytics.initialize(gaId) | ||
GoogleAnalytics.initialize([{ | ||
trackingId: gaId, | ||
gaOptions: { nonce: cspNonce, cookieFlags: 'SameSite=None; Secure' } |
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.
The nonce
is not strictly necessary because we are using unsafe-inline
but using this now will set us up for the future if we are able to get the rest of the application to use this approach.
This PR aims to resolve #1371.
I'll have to run this in one of our test deployments to ensure it's working.
Resources