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: Allow Connection to Hubspot #633

Merged
merged 3 commits into from
Jul 6, 2023
Merged

fix: Allow Connection to Hubspot #633

merged 3 commits into from
Jul 6, 2023

Conversation

ashin-czi
Copy link
Contributor

@ashin-czi ashin-czi commented Jul 6, 2023

@ashin-czi ashin-czi requested review from tihuan and ebezzi July 6, 2023 22:54
"default-src": ["'self'"],
"connect-src": ["'self'", PLAUSIBLE_URL] + extra_connect_src,
"script-src": ["'self'", "'unsafe-eval'", PLAUSIBLE_URL] + script_hashes,
"default-src": ["'self'", HUBSPOT_FORMS_URL],
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like if we have the form URL in default src, the URL won't need to be in other directives 😄?

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/default-src

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think this worked so I'm trying to explicitly put it in the other directives in this new PR :^]

#634

Copy link
Contributor

@tihuan tihuan left a comment

Choose a reason for hiding this comment

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

LGTM! Just one non-blocking question. Thanks so much for resolving this, Andrew 🙏

@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #633 (94159bb) into main (d99f066) will not change coverage.
The diff coverage is n/a.

❗ Current head 94159bb differs from pull request most recent head b40945f. Consider uploading reports for the commit b40945f to get more accurate results

@@           Coverage Diff           @@
##             main     #633   +/-   ##
=======================================
  Coverage   77.67%   77.67%           
=======================================
  Files          88       88           
  Lines        6754     6754           
=======================================
  Hits         5246     5246           
  Misses       1508     1508           
Flag Coverage Δ
frontend 77.67% <ø> (ø)
javascript 77.67% <ø> (ø)
smokeTest ∅ <ø> (∅)
unitTest 77.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ashin-czi
Copy link
Contributor Author

LGTM! Just one non-blocking question. Thanks so much for resolving this, Andrew 🙏

So maybe something like this?

It's weird because can't really test it locally 😅

@ashin-czi ashin-czi enabled auto-merge (squash) July 6, 2023 23:27
@ashin-czi ashin-czi merged commit b8f9f5e into main Jul 6, 2023
@ashin-czi ashin-czi deleted the ashin-czi/allow-hubspot branch July 6, 2023 23:34
Bento007 added a commit that referenced this pull request Jul 11, 2023
* feat: Newsletter Banner (#601)

* feat: Newsletter Banner

* fix: Allow Connection to Hubspot (#633)

* fix: Allow Connection to Hubspot

* moving to default-src

* adding form-action

* fix: Modify CSP to Allow Hubspot (#634)

---------

Co-authored-by: Andrew Shin <[email protected]>
Bento007 added a commit that referenced this pull request Jul 13, 2023
* feat: Newsletter Banner (#601)

* feat: Newsletter Banner

* fix: Allow Connection to Hubspot (#633)

* fix: Allow Connection to Hubspot

* moving to default-src

* adding form-action

* fix: Modify CSP to Allow Hubspot (#634)

---------

Co-authored-by: Andrew Shin <[email protected]>
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