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

fix: add localhost to asset canister's CSP #3476

Merged
merged 2 commits into from
Dec 18, 2023
Merged

Conversation

mraszyk
Copy link
Contributor

@mraszyk mraszyk commented Dec 11, 2023

This MR adds http://localhost:* as connect-src into the asset canister's CSP. This will enable browsing the asset canister at http://<canister-id>.localhost:<port> in most browsers.

@mraszyk mraszyk requested review from chenyan-dfinity and a team as code owners December 11, 2023 10:49
@smallstepman
Copy link
Contributor

I wonder if we should also add https://localhost - remember the safari issue that has been recently discussed @adamspofford-dfinity @krpeacock ?

@krpeacock
Copy link
Contributor

krpeacock commented Dec 11, 2023

@nomeata wouldn't hurt - we may add https as an option for localhost and 127.0.0.1

@smallstepman
Copy link
Contributor

extending e2e test with multiple schemas (localhost:, 127..., <can-id>.localhost:) would also strengthen the reliability of FE canister for local deployments (future work necessary for testing mainnet deployments)

@mraszyk lemme know if you want to contribute on that front as well

@mraszyk
Copy link
Contributor Author

mraszyk commented Dec 11, 2023

extending e2e test with multiple schemas (localhost:, 127..., <can-id>.localhost:) would also strengthen the reliability of FE canister for local deployments (future work necessary for testing mainnet deployments)

@mraszyk lemme know if you want to contribute on that front as well

I wouldn't mind if you could please update the e2e tests.

@mraszyk
Copy link
Contributor Author

mraszyk commented Dec 14, 2023

@smallstepman Do you want to extend e2e tests as part of this MR or do you have a separate ticket for that?

@mraszyk mraszyk enabled auto-merge (squash) December 18, 2023 20:38
@mraszyk mraszyk merged commit 1b12092 into master Dec 18, 2023
171 checks passed
@mraszyk mraszyk deleted the mraszyk/csp-localhost branch December 18, 2023 21:18
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.

3 participants