Skip to content

Commit eedb711

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 19869d1 commit eedb711

File tree

11 files changed

+121
-72
lines changed

11 files changed

+121
-72
lines changed

‎custom/conf/app.example.ini

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1158,15 +1158,9 @@ LEVEL = Info
11581158
;; enable cors headers (disabled by default)
11591159
;ENABLED = false
11601160
;;
1161-
;; scheme of allowed requests
1162-
;SCHEME = http
1163-
;;
1164-
;; list of requesting domains that are allowed
1161+
;; list of requesting origins that are allowed, eg: "https://*.example.com"
11651162
;ALLOW_DOMAIN = *
11661163
;;
1167-
;; allow subdomains of headers listed above to request
1168-
;ALLOW_SUBDOMAIN = false
1169-
;;
11701164
;; list of methods allowed to request
11711165
;METHODS = GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS
11721166
;;

‎docs/content/administration/config-cheat-sheet.en-us.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,7 @@ The following configuration set `Content-Type: application/vnd.android.package-a
196196
## CORS (`cors`)
197197

198198
- `ENABLED`: **false**: enable cors headers (disabled by default)
199-
- `SCHEME`: **http**: scheme of allowed requests
200-
- `ALLOW_DOMAIN`: **\***: list of requesting domains that are allowed
201-
- `ALLOW_SUBDOMAIN`: **false**: allow subdomains of headers listed above to request
199+
- `ALLOW_DOMAIN`: **\***: list of requesting origins that are allowed, eg: "https://*.example.com"
202200
- `METHODS`: **GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS**: list of methods allowed to request
203201
- `MAX_AGE`: **10m**: max time to cache response
204202
- `ALLOW_CREDENTIALS`: **false**: allow request with credentials

‎docs/content/administration/config-cheat-sheet.zh-cn.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,7 @@ menu:
195195
## 跨域 (`cors`)
196196

197197
- `ENABLED`: **false**: 启用 CORS 头部（默认禁用）
198-
- `SCHEME`: **http**: 允许请求的协议
199198
- `ALLOW_DOMAIN`: **\***: 允许请求的域名列表
200-
- `ALLOW_SUBDOMAIN`: **false**: 允许上述列出的头部的子域名发出请求。
201199
- `METHODS`: **GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS**: 允许发起的请求方式列表
202200
- `MAX_AGE`: **10m**: 缓存响应的最大时间
203201
- `ALLOW_CREDENTIALS`: **false**: 允许带有凭据的请求

‎modules/public/public.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func FileHandlerFunc() http.HandlerFunc {
3333
assetFS := AssetFS()
3434
return func(resp http.ResponseWriter, req *http.Request) {
3535
if req.Method != "GET" && req.Method != "HEAD" {
36-
resp.WriteHeader(http.StatusNotFound)
36+
resp.WriteHeader(http.StatusBadRequest)
3737
return
3838
}
3939
handleRequest(resp, req, assetFS, req.URL.Path)

‎modules/setting/cors.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ import (
1212
// CORSConfig defines CORS settings
1313
var CORSConfig = struct {
1414
Enabled bool
15-
Scheme string
16-
AllowDomain []string
17-
AllowSubdomain bool
15+
AllowDomain []string // this option is from legacy code, it should be called "AllowedOrigins" actually
1816
Methods []string
1917
MaxAge time.Duration
2018
AllowCredentials bool

‎modules/web/route.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -136,20 +136,6 @@ func (r *Route) Get(pattern string, h ...any) {
136136
r.Methods("GET", pattern, h...)
137137
}
138138

139-
func (r *Route) Options(pattern string, h ...any) {
140-
r.Methods("OPTIONS", pattern, h...)
141-
}
142-
143-
// GetOptions delegate get and options method
144-
func (r *Route) GetOptions(pattern string, h ...any) {
145-
r.Methods("GET,OPTIONS", pattern, h...)
146-
}
147-
148-
// PostOptions delegate post and options method
149-
func (r *Route) PostOptions(pattern string, h ...any) {
150-
r.Methods("POST,OPTIONS", pattern, h...)
151-
}
152-
153139
// Head delegate head method
154140
func (r *Route) Head(pattern string, h ...any) {
155141
r.Methods("HEAD", pattern, h...)

‎routers/api/v1/api.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -822,9 +822,7 @@ func Routes() *web.Route {
822822
m.Use(securityHeaders())
823823
if setting.CORSConfig.Enabled {
824824
m.Use(cors.Handler(cors.Options{
825-
// Scheme: setting.CORSConfig.Scheme, // FIXME: the cors middleware needs scheme option
826-
AllowedOrigins: setting.CORSConfig.AllowDomain,
827-
// setting.CORSConfig.AllowSubdomain // FIXME: the cors middleware needs allowSubdomain option
825+
AllowedOrigins: setting.CORSConfig.AllowDomain,
828826
AllowedMethods: setting.CORSConfig.Methods,
829827
AllowCredentials: setting.CORSConfig.AllowCredentials,
830828
AllowedHeaders: append([]string{"Authorization", "X-Gitea-OTP"}, setting.CORSConfig.Headers...),

‎routers/web/githttp.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,16 @@ func requireSignIn(ctx *context.Context) {
2828

2929
func gitHTTPRouters(m *web.Route) {
3030
m.Group("", func() {
31-
m.PostOptions("/git-upload-pack", repo.ServiceUploadPack)
32-
m.PostOptions("/git-receive-pack", repo.ServiceReceivePack)
33-
m.GetOptions("/info/refs", repo.GetInfoRefs)
34-
m.GetOptions("/HEAD", repo.GetTextFile("HEAD"))
35-
m.GetOptions("/objects/info/alternates", repo.GetTextFile("objects/info/alternates"))
36-
m.GetOptions("/objects/info/http-alternates", repo.GetTextFile("objects/info/http-alternates"))
37-
m.GetOptions("/objects/info/packs", repo.GetInfoPacks)
38-
m.GetOptions("/objects/info/{file:[^/]*}", repo.GetTextFile(""))
39-
m.GetOptions("/objects/{head:[0-9a-f]{2}}/{hash:[0-9a-f]{38}}", repo.GetLooseObject)
40-
m.GetOptions("/objects/pack/pack-{file:[0-9a-f]{40}}.pack", repo.GetPackFile)
41-
m.GetOptions("/objects/pack/pack-{file:[0-9a-f]{40}}.idx", repo.GetIdxFile)
31+
m.Methods("POST,OPTIONS", "/git-upload-pack", repo.ServiceUploadPack)
32+
m.Methods("POST,OPTIONS", "/git-receive-pack", repo.ServiceReceivePack)
33+
m.Methods("GET,OPTIONS", "/info/refs", repo.GetInfoRefs)
34+
m.Methods("GET,OPTIONS", "/HEAD", repo.GetTextFile("HEAD"))
35+
m.Methods("GET,OPTIONS", "/objects/info/alternates", repo.GetTextFile("objects/info/alternates"))
36+
m.Methods("GET,OPTIONS", "/objects/info/http-alternates", repo.GetTextFile("objects/info/http-alternates"))
37+
m.Methods("GET,OPTIONS", "/objects/info/packs", repo.GetInfoPacks)
38+
m.Methods("GET,OPTIONS", "/objects/info/{file:[^/]*}", repo.GetTextFile(""))
39+
m.Methods("GET,OPTIONS", "/objects/{head:[0-9a-f]{2}}/{hash:[0-9a-f]{38}}", repo.GetLooseObject)
40+
m.Methods("GET,OPTIONS", "/objects/pack/pack-{file:[0-9a-f]{40}}.pack", repo.GetPackFile)
41+
m.Methods("GET,OPTIONS", "/objects/pack/pack-{file:[0-9a-f]{40}}.idx", repo.GetIdxFile)
4242
}, ignSignInAndCsrf, requireSignIn, repo.HTTPGitEnabledHandler, repo.CorsHandler(), context_service.UserAssignmentWeb())
4343
}

‎routers/web/misc/misc.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,6 @@ func DummyOK(w http.ResponseWriter, req *http.Request) {
3333
w.WriteHeader(http.StatusOK)
3434
}
3535

36-
func DummyBadRequest(w http.ResponseWriter, req *http.Request) {
37-
w.WriteHeader(http.StatusBadRequest)
38-
}
39-
4036
func RobotsTxt(w http.ResponseWriter, req *http.Request) {
4137
robotsTxt := util.FilePathJoinAbs(setting.CustomPath, "public/robots.txt")
4238
if ok, _ := util.IsExist(robotsTxt); !ok {

‎routers/web/web.go

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,11 @@ const (
6161
)
6262

6363
// CorsHandler return a http handler who set CORS options if enabled by config
64-
func CorsHandler() func(next http.Handler) http.Handler {
64+
func optionsCorsHandler() func(next http.Handler) http.Handler {
65+
var corsHandler func(next http.Handler) http.Handler
6566
if setting.CORSConfig.Enabled {
66-
return cors.Handler(cors.Options{
67-
// Scheme: setting.CORSConfig.Scheme, // FIXME: the cors middleware needs scheme option
68-
AllowedOrigins: setting.CORSConfig.AllowDomain,
69-
// setting.CORSConfig.AllowSubdomain // FIXME: the cors middleware needs allowSubdomain option
67+
corsHandler = cors.Handler(cors.Options{
68+
AllowedOrigins: setting.CORSConfig.AllowDomain,
7069
AllowedMethods: setting.CORSConfig.Methods,
7170
AllowCredentials: setting.CORSConfig.AllowCredentials,
7271
AllowedHeaders: setting.CORSConfig.Headers,
@@ -75,7 +74,22 @@ func CorsHandler() func(next http.Handler) http.Handler {
7574
}
7675

7776
return func(next http.Handler) http.Handler {
78-
return next
77+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
78+
if r.Method == http.MethodOptions {
79+
if corsHandler != nil && r.Header.Get("Access-Control-Request-Method") != "" {
80+
corsHandler(next).ServeHTTP(w, r)
81+
} else {
82+
// it should explicitly deny OPTIONS requests if CORS is disabled, to avoid the following GET/POST handler to be called by the OPTIONS request
83+
w.WriteHeader(http.StatusMethodNotAllowed)
84+
}
85+
return
86+
}
87+
if corsHandler != nil {
88+
corsHandler(next).ServeHTTP(w, r)
89+
} else {
90+
next.ServeHTTP(w, r)
91+
}
92+
})
7993
}
8094
}
8195

@@ -218,7 +232,7 @@ func Routes() *web.Route {
218232
routes := web.NewRoute()
219233

220234
routes.Head("/", misc.DummyOK) // for health check - doesn't need to be passed through gzip handler
221-
routes.Methods("GET, HEAD", "/assets/*", CorsHandler(), public.FileHandlerFunc())
235+
routes.Methods("GET, HEAD, OPTIONS", "/assets/*", optionsCorsHandler(), public.FileHandlerFunc())
222236
routes.Methods("GET, HEAD", "/avatars/*", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars))
223237
routes.Methods("GET, HEAD", "/repo-avatars/*", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars))
224238
routes.Methods("GET, HEAD", "/apple-touch-icon.png", misc.StaticRedirect("/assets/img/apple-touch-icon.png"))
@@ -458,8 +472,8 @@ func registerRoutes(m *web.Route) {
458472
m.Get("/change-password", func(ctx *context.Context) {
459473
ctx.Redirect(setting.AppSubURL + "/user/settings/account")
460474
})
461-
m.Any("/*", CorsHandler(), public.FileHandlerFunc())
462-
}, CorsHandler())
475+
m.Methods("GET, HEAD", "/*", public.FileHandlerFunc())
476+
}, optionsCorsHandler())
463477

464478
m.Group("/explore", func() {
465479
m.Get("", func(ctx *context.Context) {
@@ -532,14 +546,11 @@ func registerRoutes(m *web.Route) {
532546
// TODO manage redirection
533547
m.Post("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth)
534548
}, ignSignInAndCsrf, reqSignIn)
535-
m.Options("/login/oauth/userinfo", CorsHandler(), misc.DummyBadRequest)
536-
m.Get("/login/oauth/userinfo", ignSignInAndCsrf, auth.InfoOAuth)
537-
m.Options("/login/oauth/access_token", CorsHandler(), misc.DummyBadRequest)
538-
m.Post("/login/oauth/access_token", CorsHandler(), web.Bind(forms.AccessTokenForm{}), ignSignInAndCsrf, auth.AccessTokenOAuth)
539-
m.Options("/login/oauth/keys", CorsHandler(), misc.DummyBadRequest)
540-
m.Get("/login/oauth/keys", ignSignInAndCsrf, auth.OIDCKeys)
541-
m.Options("/login/oauth/introspect", CorsHandler(), misc.DummyBadRequest)
542-
m.Post("/login/oauth/introspect", CorsHandler(), web.Bind(forms.IntrospectTokenForm{}), ignSignInAndCsrf, auth.IntrospectOAuth)
549+
550+
m.Methods("GET, OPTIONS", "/login/oauth/userinfo", optionsCorsHandler(), ignSignInAndCsrf, auth.InfoOAuth)
551+
m.Methods("POST, OPTIONS", "/login/oauth/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), ignSignInAndCsrf, auth.AccessTokenOAuth)
552+
m.Methods("GET, OPTIONS", "/login/oauth/keys", optionsCorsHandler(), ignSignInAndCsrf, auth.OIDCKeys)
553+
m.Methods("POST, OPTIONS", "/login/oauth/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), ignSignInAndCsrf, auth.IntrospectOAuth)
543554

544555
m.Group("/user/settings", func() {
545556
m.Get("", user_setting.Profile)

‎tests/integration/cors_test.go

Lines changed: 77 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,87 @@ import (
77
"net/http"
88
"testing"
99

10+
"code.gitea.io/gitea/modules/setting"
11+
"code.gitea.io/gitea/modules/test"
12+
"code.gitea.io/gitea/routers"
1013
"code.gitea.io/gitea/tests"
1114

1215
"github.com/stretchr/testify/assert"
1316
)
1417

15-
func TestCORSNotSet(t *testing.T) {
18+
func TestCORS(t *testing.T) {
1619
defer tests.PrepareTestEnv(t)()
17-
req := NewRequest(t, "GET", "/api/v1/version")
18-
session := loginUser(t, "user2")
19-
resp := session.MakeRequest(t, req, http.StatusOK)
20-
assert.Equal(t, resp.Code, http.StatusOK)
21-
corsHeader := resp.Header().Get("Access-Control-Allow-Origin")
22-
assert.Empty(t, corsHeader, "Access-Control-Allow-Origin: generated header should match") // header not set
20+
t.Run("CORS enabled", func(t *testing.T) {
21+
defer test.MockVariableValue(&setting.CORSConfig.Enabled, true)()
22+
defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
23+
24+
t.Run("API with CORS", func(t *testing.T) {
25+
// GET api with no CORS header
26+
req := NewRequest(t, "GET", "/api/v1/version")
27+
resp := MakeRequest(t, req, http.StatusOK)
28+
assert.Empty(t, resp.Header().Get("Access-Control-Allow-Origin"))
29+
assert.Contains(t, resp.Header().Values("Vary"), "Origin")
30+
31+
// OPTIONS api for CORS
32+
req = NewRequest(t, "OPTIONS", "/api/v1/version").
33+
SetHeader("Origin", "https://example.com").
34+
SetHeader("Access-Control-Request-Method", "GET")
35+
resp = MakeRequest(t, req, http.StatusOK)
36+
assert.NotEmpty(t, resp.Header().Get("Access-Control-Allow-Origin"))
37+
assert.Contains(t, resp.Header().Values("Vary"), "Origin")
38+
})
39+
40+
t.Run("Web with CORS", func(t *testing.T) {
41+
// GET userinfo with no CORS header
42+
req := NewRequest(t, "GET", "/login/oauth/userinfo")
43+
resp := MakeRequest(t, req, http.StatusUnauthorized)
44+
assert.Empty(t, resp.Header().Get("Access-Control-Allow-Origin"))
45+
assert.Contains(t, resp.Header().Values("Vary"), "Origin")
46+
47+
// OPTIONS userinfo for CORS
48+
req = NewRequest(t, "OPTIONS", "/login/oauth/userinfo").
49+
SetHeader("Origin", "https://example.com").
50+
SetHeader("Access-Control-Request-Method", "GET")
51+
resp = MakeRequest(t, req, http.StatusOK)
52+
assert.NotEmpty(t, resp.Header().Get("Access-Control-Allow-Origin"))
53+
assert.Contains(t, resp.Header().Values("Vary"), "Origin")
54+
55+
// OPTIONS userinfo for non-CORS
56+
req = NewRequest(t, "OPTIONS", "/login/oauth/userinfo")
57+
resp = MakeRequest(t, req, http.StatusMethodNotAllowed)
58+
})
59+
})
60+
61+
t.Run("CORS disabled", func(t *testing.T) {
62+
defer test.MockVariableValue(&setting.CORSConfig.Enabled, false)()
63+
defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
64+
65+
t.Run("API without CORS", func(t *testing.T) {
66+
req := NewRequest(t, "GET", "/api/v1/version")
67+
resp := MakeRequest(t, req, http.StatusOK)
68+
assert.Empty(t, resp.Header().Get("Access-Control-Allow-Origin"))
69+
assert.Empty(t, resp.Header().Values("Vary"))
70+
71+
req = NewRequest(t, "OPTIONS", "/api/v1/version").
72+
SetHeader("Origin", "https://example.com").
73+
SetHeader("Access-Control-Request-Method", "GET")
74+
resp = MakeRequest(t, req, http.StatusMethodNotAllowed)
75+
assert.Empty(t, resp.Header().Get("Access-Control-Allow-Origin"))
76+
assert.Empty(t, resp.Header().Values("Vary"))
77+
})
78+
79+
t.Run("Web without CORS", func(t *testing.T) {
80+
req := NewRequest(t, "GET", "/login/oauth/userinfo")
81+
resp := MakeRequest(t, req, http.StatusUnauthorized)
82+
assert.Empty(t, resp.Header().Get("Access-Control-Allow-Origin"))
83+
assert.NotContains(t, resp.Header().Values("Vary"), "Origin")
84+
85+
req = NewRequest(t, "OPTIONS", "/login/oauth/userinfo").
86+
SetHeader("Origin", "https://example.com").
87+
SetHeader("Access-Control-Request-Method", "GET")
88+
resp = MakeRequest(t, req, http.StatusMethodNotAllowed)
89+
assert.Empty(t, resp.Header().Get("Access-Control-Allow-Origin"))
90+
assert.NotContains(t, resp.Header().Values("Vary"), "Origin")
91+
})
92+
})
2393
}

0 commit comments

Comments
 (0)