Skip to content

go-chi/cors wraps one Handler call whose interesting half is already ours #280

Description

@leodip

go-chi/cors is imported by one file, core/middleware/middleware_cors.go, for one call.

What we use

return cors.Handler(cors.Options{
    AllowOriginFunc: func(r *http.Request, origin string) bool { ... },
    ...
})

The AllowOriginFunc body is entirely ours: the per-path logic (always allow /.well-known/openid-configuration and /certs, index lookup on the web_origins UNIQUE constraint from migration 000034 for /auth/token, /auth/logout and /userinfo), including the reasoning about preflights carrying neither the Authorization header nor the form body per Fetch Standard §4.8.

What the library contributes is the mechanical half: preflight detection, Vary headers, and writing the Access-Control-* response headers.

What replaces it

About 80 lines of middleware:

  • On OPTIONS with an Access-Control-Request-Method header, treat it as a preflight: run the origin check, write Access-Control-Allow-Origin, -Allow-Methods, -Allow-Headers, -Max-Age, and respond 204 without calling the next handler.
  • On any other request with an Origin header, run the origin check and write Access-Control-Allow-Origin (and -Allow-Credentials / -Expose-Headers if configured) before calling next.
  • Always write Vary: Origin on any response whose Access-Control-Allow-Origin depends on the request origin, plus Vary: Access-Control-Request-Method, Access-Control-Request-Headers on preflights.

Priority: low, and the risk is asymmetric

This one I would leave alone unless the goal is a zero-third-party middleware layer:

  • It removes one module, with no transitive tail. go-chi/cors has no dependencies.
  • CORS reads simple and is not. The Vary handling in particular is the difference between working correctly and a cache serving one origin's Access-Control-Allow-Origin to another origin, which is a security bug that no test we would think to write catches, because it lives in a cache we do not control.
  • The interesting, Goiabada-specific half is already ours and stays ours either way.

If it does go ahead, the deliverable is the test suite as much as the middleware: preflight and simple-request paths, allowed and denied origins, the Vary headers on every branch, and the /certs and discovery-endpoint always-allow cases.


Dependency reduction

Clear replacement, low risk: #268 golang-migrate, #269 gorilla/sessions, #270 gorilla/securecookie, #271 unknwon/paginater, #272 gofakeit (test-only).

Good replacement, needs care or a decision: #273 go-i18n, #274 go-simple-mail, #275 htmlsanitizer, #276 httprate, #277 go-jose, #281 mileusna/useragent.

Replacement exists but the value is low: #278 google/uuid, #279 pkg/errors, #280 go-chi/cors.

Keeping: BurntSushi/toml, the four database drivers, huandu/go-sqlbuilder, golang-jwt/v5, x/crypto, x/text, x/image, pquerna/otp, testify, goquery.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    dependenciesPull requests that update a dependency fileenhancementNew feature or requestgoPull requests that update Go code

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions