test: add CORS middleware tests (#79) - #97
Conversation
|
Thanks for the PR here, @prince-0408! @DeleMike, would you be able to take a look? |
andrewtavis
added
the
no-changelog
|
Thanks @andrewtavis for notifying me. And thanks @prince-0408, I will look into this later today or early hours tomorrow 🤞🏾 Good PR comment! Thank you! :) |
|
reviewing... |
There is no need for this flag as we have no auth for now. Scribe-Server API is a public API.
DeleMike
left a comment
DeleMike
left a comment
There was a problem hiding this comment.
All looks great to me! :)
Great work @prince-0408
| func SetupCORS() gin.HandlerFunc { | ||
| return func(c *gin.Context) { | ||
| c.Writer.Header().Set("Access-Control-Allow-Origin", "*") | ||
| c.Writer.Header().Set("Access-Control-Allow-Credentials", "true") |
There was a problem hiding this comment.
Hi,
I removed Access-Control-Allow-Credentials. I believe it must NOT be set for now. The Fetch spec forbids pairing it with a wildcard origin, and browsers reject any credentialed request that receives both(we were initially setting both, but this PR will remove the one we don't use).
If credentials are ever needed, the wildcard above has to be replaced by a reflected, allowlisted origin first.
For now, we are a public API so I believe we should remove it for now.
|
Hi @andrewtavis , we can merge if you are satisfied :) |
andrewtavis
left a comment
andrewtavis
left a comment
There was a problem hiding this comment.
praise: Looks great to me, @prince-0408! Thanks for the review, @DeleMike 😊 Mostly going off your review as I trust that you know what we're looking for here :)
3 participants
Contributor checklist
./pre-commitexecutable as well asmake lintand have fixed all reported issuesDescription
Adds unit tests for the CORS middleware (
SetupCORS()) to ensure proper CORS headers are set across all scenarios, matching the requirements of #79:Changes:
api/middleware_test.go:GETasserts all fourAccess-Control-*headers and downstream handler execution.OPTIONSpreflight returns HTTP204without reaching the handler.400,403,404,500) preserve all four CORS headers.404and other HTTP methods (POST,PUT,DELETE).go.mod: Promotesgithub.com/stretchr/testify v1.10.0as required byTESTING.md.Verification & Coverage:
make fmt: Clean.make test: All tests pass.make lint(revive): 0 issues.api/middleware.go:SetupCORS:0.0% -> 100.0%2.3% -> 3.4%Related issue