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

[CORL-2872]: Add cors to single comment embed oembed endpoint #4297

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

kabeaty
Copy link
Contributor

@kabeaty kabeaty commented Jul 18, 2023

What does this PR do?

Adds cors to the oembed endpoint for single comment embeds.

(Permissions are still determined via site permitted domains, this just ensures that Access-Control-Allow-Origin header for the permitted requesting origin is included if those checks are passed.)

These changes will impact:

  • commenters
  • moderators
  • admins
  • developers

What changes to the GraphQL/Database Schema does this PR introduce?

none

Does this PR introduce any new environment variables or feature flags?

no

If any indexes were added, were they added to INDEXES.md?

n/a

How do I test this PR?

You can uncomment the example of the Oembed API at the bottom of commentEmbed.html (you'll need to edit to add a comment id where noted). Inspect in the network tab of dev tools and see that the response still works as expected.

Also copy over to a test site and embed there (update fetch url and comment ids as needed). See that the comment embed response now includes the Access-Control-Allow-Origin header set to the correct origin for the permitted request. Remove your test site from the permitted sites list. See that now you get a 401 Unauthorized error as expected.

Where any tests migrated to React Testing Library?

How do we deploy this PR?

@netlify
Copy link

netlify bot commented Jul 18, 2023

Deploy Preview for gallant-galileo-14878c canceled.

Name Link
🔨 Latest commit d9cef98
🔍 Latest deploy log https://app.netlify.com/sites/gallant-galileo-14878c/deploys/64b7e08aed5a3b0008fe1a4a

@kabeaty kabeaty changed the title Add cors to single comment embed oembed endpoint CORL-2872]: Add cors to single comment embed oembed endpoint Jul 18, 2023
@kabeaty kabeaty changed the title CORL-2872]: Add cors to single comment embed oembed endpoint [CORL-2872]: Add cors to single comment embed oembed endpoint Jul 19, 2023
@nick-funk
Copy link
Contributor

This looks good, approved! Thank you for limiting the cors() with an allow list handler, much appreciated!

@tessalt tessalt merged commit da6cfaf into develop Jul 19, 2023
7 checks passed
@tessalt tessalt deleted the fix/comment-oembed-cors branch July 19, 2023 18:28
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