Refactor CORS handler (#28587)
The CORS code has been unmaintained for long time, and the behavior is not correct. This PR tries to improve it. The key point is written as comment in code. And add more tests. Fix #28515 Fix #27642 Fix #17098
This commit is contained in:
parent
d0f24ff4ca
commit
b41925cee3
11 changed files with 131 additions and 78 deletions
|
@ -822,9 +822,7 @@ func Routes() *web.Route {
|
|||
m.Use(securityHeaders())
|
||||
if setting.CORSConfig.Enabled {
|
||||
m.Use(cors.Handler(cors.Options{
|
||||
// Scheme: setting.CORSConfig.Scheme, // FIXME: the cors middleware needs scheme option
|
||||
AllowedOrigins: setting.CORSConfig.AllowDomain,
|
||||
// setting.CORSConfig.AllowSubdomain // FIXME: the cors middleware needs allowSubdomain option
|
||||
AllowedOrigins: setting.CORSConfig.AllowDomain,
|
||||
AllowedMethods: setting.CORSConfig.Methods,
|
||||
AllowCredentials: setting.CORSConfig.AllowCredentials,
|
||||
AllowedHeaders: append([]string{"Authorization", "X-Gitea-OTP"}, setting.CORSConfig.Headers...),
|
||||
|
|
|
@ -28,16 +28,16 @@ func requireSignIn(ctx *context.Context) {
|
|||
|
||||
func gitHTTPRouters(m *web.Route) {
|
||||
m.Group("", func() {
|
||||
m.PostOptions("/git-upload-pack", repo.ServiceUploadPack)
|
||||
m.PostOptions("/git-receive-pack", repo.ServiceReceivePack)
|
||||
m.GetOptions("/info/refs", repo.GetInfoRefs)
|
||||
m.GetOptions("/HEAD", repo.GetTextFile("HEAD"))
|
||||
m.GetOptions("/objects/info/alternates", repo.GetTextFile("objects/info/alternates"))
|
||||
m.GetOptions("/objects/info/http-alternates", repo.GetTextFile("objects/info/http-alternates"))
|
||||
m.GetOptions("/objects/info/packs", repo.GetInfoPacks)
|
||||
m.GetOptions("/objects/info/{file:[^/]*}", repo.GetTextFile(""))
|
||||
m.GetOptions("/objects/{head:[0-9a-f]{2}}/{hash:[0-9a-f]{38}}", repo.GetLooseObject)
|
||||
m.GetOptions("/objects/pack/pack-{file:[0-9a-f]{40}}.pack", repo.GetPackFile)
|
||||
m.GetOptions("/objects/pack/pack-{file:[0-9a-f]{40}}.idx", repo.GetIdxFile)
|
||||
m.Methods("POST,OPTIONS", "/git-upload-pack", repo.ServiceUploadPack)
|
||||
m.Methods("POST,OPTIONS", "/git-receive-pack", repo.ServiceReceivePack)
|
||||
m.Methods("GET,OPTIONS", "/info/refs", repo.GetInfoRefs)
|
||||
m.Methods("GET,OPTIONS", "/HEAD", repo.GetTextFile("HEAD"))
|
||||
m.Methods("GET,OPTIONS", "/objects/info/alternates", repo.GetTextFile("objects/info/alternates"))
|
||||
m.Methods("GET,OPTIONS", "/objects/info/http-alternates", repo.GetTextFile("objects/info/http-alternates"))
|
||||
m.Methods("GET,OPTIONS", "/objects/info/packs", repo.GetInfoPacks)
|
||||
m.Methods("GET,OPTIONS", "/objects/info/{file:[^/]*}", repo.GetTextFile(""))
|
||||
m.Methods("GET,OPTIONS", "/objects/{head:[0-9a-f]{2}}/{hash:[0-9a-f]{38}}", repo.GetLooseObject)
|
||||
m.Methods("GET,OPTIONS", "/objects/pack/pack-{file:[0-9a-f]{40}}.pack", repo.GetPackFile)
|
||||
m.Methods("GET,OPTIONS", "/objects/pack/pack-{file:[0-9a-f]{40}}.idx", repo.GetIdxFile)
|
||||
}, ignSignInAndCsrf, requireSignIn, repo.HTTPGitEnabledHandler, repo.CorsHandler(), context_service.UserAssignmentWeb())
|
||||
}
|
||||
|
|
|
@ -33,10 +33,6 @@ func DummyOK(w http.ResponseWriter, req *http.Request) {
|
|||
w.WriteHeader(http.StatusOK)
|
||||
}
|
||||
|
||||
func DummyBadRequest(w http.ResponseWriter, req *http.Request) {
|
||||
w.WriteHeader(http.StatusBadRequest)
|
||||
}
|
||||
|
||||
func RobotsTxt(w http.ResponseWriter, req *http.Request) {
|
||||
robotsTxt := util.FilePathJoinAbs(setting.CustomPath, "public/robots.txt")
|
||||
if ok, _ := util.IsExist(robotsTxt); !ok {
|
||||
|
|
|
@ -60,13 +60,12 @@ const (
|
|||
GzipMinSize = 1400
|
||||
)
|
||||
|
||||
// CorsHandler return a http handler who set CORS options if enabled by config
|
||||
func CorsHandler() func(next http.Handler) http.Handler {
|
||||
// optionsCorsHandler return a http handler which sets CORS options if enabled by config, it blocks non-CORS OPTIONS requests.
|
||||
func optionsCorsHandler() func(next http.Handler) http.Handler {
|
||||
var corsHandler func(next http.Handler) http.Handler
|
||||
if setting.CORSConfig.Enabled {
|
||||
return cors.Handler(cors.Options{
|
||||
// Scheme: setting.CORSConfig.Scheme, // FIXME: the cors middleware needs scheme option
|
||||
AllowedOrigins: setting.CORSConfig.AllowDomain,
|
||||
// setting.CORSConfig.AllowSubdomain // FIXME: the cors middleware needs allowSubdomain option
|
||||
corsHandler = cors.Handler(cors.Options{
|
||||
AllowedOrigins: setting.CORSConfig.AllowDomain,
|
||||
AllowedMethods: setting.CORSConfig.Methods,
|
||||
AllowCredentials: setting.CORSConfig.AllowCredentials,
|
||||
AllowedHeaders: setting.CORSConfig.Headers,
|
||||
|
@ -75,7 +74,23 @@ func CorsHandler() func(next http.Handler) http.Handler {
|
|||
}
|
||||
|
||||
return func(next http.Handler) http.Handler {
|
||||
return next
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method == http.MethodOptions {
|
||||
if corsHandler != nil && r.Header.Get("Access-Control-Request-Method") != "" {
|
||||
corsHandler(next).ServeHTTP(w, r)
|
||||
} else {
|
||||
// it should explicitly deny OPTIONS requests if CORS handler is not executed, to avoid the next GET/POST handler being incorrectly called by the OPTIONS request
|
||||
w.WriteHeader(http.StatusMethodNotAllowed)
|
||||
}
|
||||
return
|
||||
}
|
||||
// for non-OPTIONS requests, call the CORS handler to add some related headers like "Vary"
|
||||
if corsHandler != nil {
|
||||
corsHandler(next).ServeHTTP(w, r)
|
||||
} else {
|
||||
next.ServeHTTP(w, r)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -218,7 +233,7 @@ func Routes() *web.Route {
|
|||
routes := web.NewRoute()
|
||||
|
||||
routes.Head("/", misc.DummyOK) // for health check - doesn't need to be passed through gzip handler
|
||||
routes.Methods("GET, HEAD", "/assets/*", CorsHandler(), public.FileHandlerFunc())
|
||||
routes.Methods("GET, HEAD, OPTIONS", "/assets/*", optionsCorsHandler(), public.FileHandlerFunc())
|
||||
routes.Methods("GET, HEAD", "/avatars/*", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars))
|
||||
routes.Methods("GET, HEAD", "/repo-avatars/*", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars))
|
||||
routes.Methods("GET, HEAD", "/apple-touch-icon.png", misc.StaticRedirect("/assets/img/apple-touch-icon.png"))
|
||||
|
@ -458,8 +473,8 @@ func registerRoutes(m *web.Route) {
|
|||
m.Get("/change-password", func(ctx *context.Context) {
|
||||
ctx.Redirect(setting.AppSubURL + "/user/settings/account")
|
||||
})
|
||||
m.Any("/*", CorsHandler(), public.FileHandlerFunc())
|
||||
}, CorsHandler())
|
||||
m.Methods("GET, HEAD", "/*", public.FileHandlerFunc())
|
||||
}, optionsCorsHandler())
|
||||
|
||||
m.Group("/explore", func() {
|
||||
m.Get("", func(ctx *context.Context) {
|
||||
|
@ -532,14 +547,11 @@ func registerRoutes(m *web.Route) {
|
|||
// TODO manage redirection
|
||||
m.Post("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth)
|
||||
}, ignSignInAndCsrf, reqSignIn)
|
||||
m.Options("/login/oauth/userinfo", CorsHandler(), misc.DummyBadRequest)
|
||||
m.Get("/login/oauth/userinfo", ignSignInAndCsrf, auth.InfoOAuth)
|
||||
m.Options("/login/oauth/access_token", CorsHandler(), misc.DummyBadRequest)
|
||||
m.Post("/login/oauth/access_token", CorsHandler(), web.Bind(forms.AccessTokenForm{}), ignSignInAndCsrf, auth.AccessTokenOAuth)
|
||||
m.Options("/login/oauth/keys", CorsHandler(), misc.DummyBadRequest)
|
||||
m.Get("/login/oauth/keys", ignSignInAndCsrf, auth.OIDCKeys)
|
||||
m.Options("/login/oauth/introspect", CorsHandler(), misc.DummyBadRequest)
|
||||
m.Post("/login/oauth/introspect", CorsHandler(), web.Bind(forms.IntrospectTokenForm{}), ignSignInAndCsrf, auth.IntrospectOAuth)
|
||||
|
||||
m.Methods("GET, OPTIONS", "/login/oauth/userinfo", optionsCorsHandler(), ignSignInAndCsrf, auth.InfoOAuth)
|
||||
m.Methods("POST, OPTIONS", "/login/oauth/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), ignSignInAndCsrf, auth.AccessTokenOAuth)
|
||||
m.Methods("GET, OPTIONS", "/login/oauth/keys", optionsCorsHandler(), ignSignInAndCsrf, auth.OIDCKeys)
|
||||
m.Methods("POST, OPTIONS", "/login/oauth/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), ignSignInAndCsrf, auth.IntrospectOAuth)
|
||||
|
||||
m.Group("/user/settings", func() {
|
||||
m.Get("", user_setting.Profile)
|
||||
|
@ -770,7 +782,7 @@ func registerRoutes(m *web.Route) {
|
|||
|
||||
m.Group("", func() {
|
||||
m.Get("/{username}", user.UsernameSubRoute)
|
||||
m.Get("/attachments/{uuid}", repo.GetAttachment)
|
||||
m.Methods("GET, OPTIONS", "/attachments/{uuid}", optionsCorsHandler(), repo.GetAttachment)
|
||||
}, ignSignIn)
|
||||
|
||||
m.Post("/{username}", reqSignIn, context_service.UserAssignmentWeb(), user.Action)
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue