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

Content Security Policy docs and better default #1504

Merged
merged 14 commits into from
Sep 16, 2022

Conversation

nelsonwittwer
Copy link
Contributor

@nelsonwittwer nelsonwittwer commented Sep 12, 2022

What this PR does

When setting the default host in the content security policy, use the application's configuration instead of a hard coded value.

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • For security fixes, the Disclosure Policy must be followed.

@nelsonwittwer nelsonwittwer changed the title Support ENV["HOST"] in Content Security Policy Use Configuration.myshopify_domain in Content Security Policy Sep 12, 2022
@nelsonwittwer nelsonwittwer changed the title Use Configuration.myshopify_domain in Content Security Policy Content Security Policy docs and better default Sep 12, 2022
Copy link
Contributor

@paulomarg paulomarg left a comment

Choose a reason for hiding this comment

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

LGTM code-wise, aside from whether we want the shop itself in the domain or not.

@nelsonwittwer nelsonwittwer merged commit 7d0e88d into main Sep 16, 2022
@nelsonwittwer nelsonwittwer deleted the nelsonwittwer/csp_host branch September 16, 2022 14:46
fabriazza pushed a commit to fabriazza/shopify_app that referenced this pull request Feb 1, 2023
* allow ENV["HOST"] in CSP

* remove test

* docs for ENV["HOST"] CSP support

* No more ENV, use configuration default instead

* remove redundant changelog

* remove current_shopify_domain support for CSP

* current_shopify_domain or myshopify_domain from config

* try without current_shopify_domain

* allow current_shopify_domain and myshopify_domain

* use singleton config for myshopify_domain

* Add link to CSP docs

* rubocop linter
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.

2 participants