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

add more sources to allow list #1206

Merged
merged 1 commit into from
Jul 5, 2022

Conversation

alisaduncan
Copy link
Member

I think this is how it works??

@netlify
Copy link

netlify bot commented Jul 5, 2022

Deploy Preview for okta-blog ready!

Name Link
🔨 Latest commit 2c4cfbb
🔍 Latest deploy log https://app.netlify.com/sites/okta-blog/deploys/62c480ba7b947d0009542f0a
😎 Deploy Preview https://deploy-preview-1206--okta-blog.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@alisaduncan alisaduncan requested a review from bdemers July 5, 2022 18:23
Copy link
Contributor

@bdemers bdemers left a comment

Choose a reason for hiding this comment

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

I'd say ship it, but we should open an issue to address a few potential issues with this header in general.

  • It's a long string and hard to diff.
  • Individual posts shouldn't need to globally change the policy.
  • Having individual posts copy/edit the CSP is likely error prone.

For the first one, switching to a netlify.toml might work as we could do something like:

[[headers]]
  for = "/*"
  [headers.values]
	Content-Security-Policy = '''
	upgrade-insecure-requests; 
        default-src 'self' https://app.netlify.com https://devforum.okta.com ...'''

NOTE: Per this issue this cannot be done with a _headers as that would result in an extra comma getting added to the header and making it invalid.

Longer-term maybe we could write a Jekyll plugin to write the _headers file, and have it add additional entries based on a front matter property, e.g. csp-img-src: [https://coolpics.example.com]

Again, all this might be overcomplicating the issue, and those improvements probably shouldn't block your post

@alisaduncan alisaduncan merged commit d5e87c3 into oktadev:main Jul 5, 2022
@alisaduncan alisaduncan deleted the update-csp-allowlist branch July 5, 2022 19:35
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.

None yet

2 participants