Skip to content

Conversation

@lforst
Copy link
Contributor

@lforst lforst commented Jun 16, 2023

Apparently some cors policy headers like Cross-Origin-Opener-Policy: same-origin and Cross-Origin-Embedder-Policy: require-corp cause requests to our user report dialog template to fail.

This PR adds a Access-Control-Allow-Origin: * header to the response to always allow access to the resource, regardless of the policy. This should be safe since the resource is not behind auth or anything user-specific.

Would love input from some more CORS-savvy people though.

Fixes #49535

@lforst lforst requested review from JoshFerge and oioki June 16, 2023 12:17
@lforst lforst self-assigned this Jun 16, 2023
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 16, 2023
Copy link
Member

@JoshFerge JoshFerge left a comment

Choose a reason for hiding this comment

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

could add a test to tests/sentry/web/frontend/test_error_page_embed.py to confirm behavior but looks good to me!

@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #51138 (f3eff30) into master (bbea2aa) will increase coverage by 2.18%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #51138      +/-   ##
==========================================
+ Coverage   79.04%   81.23%   +2.18%     
==========================================
  Files        4886     4895       +9     
  Lines      204852   205385     +533     
  Branches    11123    11123              
==========================================
+ Hits       161929   166847    +4918     
+ Misses      42677    38292    -4385     
  Partials      246      246              
Impacted Files Coverage Δ
src/sentry/web/frontend/error_page_embed.py 96.58% <100.00%> (+0.05%) ⬆️

... and 518 files with indirect coverage changes

@lforst lforst merged commit ab32854 into master Jun 21, 2023
@lforst lforst deleted the lforst-user-feedback-dialog-cors branch June 21, 2023 11:50
@github-actions github-actions bot locked and limited conversation to collaborators Jul 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allows cors on ingest.sentry.io

3 participants