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

[Access] Allow all origins by default on websockets connections #4954

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

peterargue
Copy link
Contributor

@peterargue peterargue commented Nov 8, 2023

Closes: #4952

This PR updates the websockets Origin header handling to allow any origin. The default behavior requires that either the Origin header is not set, or that it matches the host's hostname. This is inconsistent with the REST API, and breaks websockets for browser based clients unless the AN's hostname matches the URL.

@codecov-commenter
Copy link

codecov-commenter commented Nov 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (417ec4a) 55.74% compared to head (dd74b1e) 55.75%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4954      +/-   ##
==========================================
+ Coverage   55.74%   55.75%   +0.01%     
==========================================
  Files         959      959              
  Lines       89245    89249       +4     
==========================================
+ Hits        49748    49761      +13     
+ Misses      35752    35740      -12     
- Partials     3745     3748       +3     
Flag Coverage Δ
unittests 55.75% <100.00%> (+0.01%) ⬆️

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

Files Coverage Δ
engine/access/rest/routes/websocket_handler.go 65.77% <100.00%> (+0.74%) ⬆️

... and 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@peterargue peterargue added this pull request to the merge queue Nov 8, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 8, 2023
@peterargue peterargue added this pull request to the merge queue Nov 8, 2023
@peterargue peterargue removed this pull request from the merge queue due to a manual request Nov 8, 2023
@peterargue peterargue added this pull request to the merge queue Nov 9, 2023
Merged via the queue into master with commit 58099aa Nov 9, 2023
36 checks passed
@peterargue peterargue deleted the petera/4952-fix-websockets-cors branch November 9, 2023 04:51
peterargue added a commit that referenced this pull request Nov 14, 2023
[Access] Allow all origins by default on websockets connections
This pull request was closed.
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.

[Access] Websocket event streaming does not work with Origin header present
4 participants