Open fetsorn opened 3 months ago
ping @aymanbagabas, should I improve this so you can merge? make all rules "*" perhaps for consistency?
ping @aymanbagabas, should I improve this so you can merge? make all rules "*" perhaps for consistency?
Hey @fetsorn, I wonder if we can make the CORS origin field configurable. Without that, it can be a security issue for some users. https://github.com/charmbracelet/soft-serve/pull/516#discussion_r1586478681
I added three lists to the http section of yaml configuration
# The HTTP server configuration.
http:
# The address on which the HTTP server will listen.
listen_addr: ":23232"
allowed_headers:
- Content-Type
- X-Requested-With
allowed_origins:
- *
allowed_methods:
- GET
- HEAD
- POST
This looks good @fetsorn! I would move the config to http.cors.headers
etc to make it clear that these are CORS config.
I brought out the "cors" configuration struct. I believe that "allowed_headers" is more correct than just "headers" because it corresponds to gorilla's variable`handlers.AllowedHeaders" and the preflight header Access-Control-Allow-Headers.
@aymanbagabas What can I improve to bring this closer to merge?
Hey @fetsorn, could you rebase the PR and any lint issues? This will be released in v0.8.0
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 53.05%. Comparing base (
b06b555
) to head (a040d05
). Report is 57 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I needed to clone a soft-serve repo from the browser so I added cors headers.