Skip to content

Commit a372544

Browse files
wxiaoguangAbdulrhmnGhanem
authored andcommitted
Refactor CSRF protection modules, make sure CSRF tokens can be up-to-date. (go-gitea#19337)
Do a refactoring to the CSRF related code, remove most unnecessary functions. Parse the generated token's issue time, regenerate the token every a few minutes.
1 parent 9a6790b commit a372544

File tree

10 files changed

+166
-192
lines changed

10 files changed

+166
-192
lines changed

integrations/csrf_test.go

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright 2017 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package integrations
6+
7+
import (
8+
"net/http"
9+
"strings"
10+
"testing"
11+
12+
"code.gitea.io/gitea/models/unittest"
13+
user_model "code.gitea.io/gitea/models/user"
14+
"code.gitea.io/gitea/modules/setting"
15+
16+
"github.com/stretchr/testify/assert"
17+
)
18+
19+
func TestCsrfProtection(t *testing.T) {
20+
defer prepareTestEnv(t)()
21+
22+
// test web form csrf via form
23+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User)
24+
session := loginUser(t, user.Name)
25+
req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
26+
"_csrf": "fake_csrf",
27+
})
28+
session.MakeRequest(t, req, http.StatusSeeOther)
29+
30+
resp := session.MakeRequest(t, req, http.StatusSeeOther)
31+
loc := resp.Header().Get("Location")
32+
assert.Equal(t, setting.AppSubURL+"/", loc)
33+
resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
34+
htmlDoc := NewHTMLParser(t, resp.Body)
35+
assert.Equal(t, "Bad Request: invalid CSRF token",
36+
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
37+
)
38+
39+
// test web form csrf via header. TODO: should use an UI api to test
40+
req = NewRequest(t, "POST", "/user/settings")
41+
req.Header.Add("X-Csrf-Token", "fake_csrf")
42+
session.MakeRequest(t, req, http.StatusSeeOther)
43+
44+
resp = session.MakeRequest(t, req, http.StatusSeeOther)
45+
loc = resp.Header().Get("Location")
46+
assert.Equal(t, setting.AppSubURL+"/", loc)
47+
resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
48+
htmlDoc = NewHTMLParser(t, resp.Body)
49+
assert.Equal(t, "Bad Request: invalid CSRF token",
50+
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
51+
)
52+
}

integrations/repo_topic_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"testing"
1111

1212
api "code.gitea.io/gitea/modules/structs"
13+
1314
"github.com/stretchr/testify/assert"
1415
)
1516

modules/context/auth.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func Toggle(options *ToggleOptions) func(ctx *Context) {
6363
}
6464

6565
if !options.SignOutRequired && !options.DisableCSRF && ctx.Req.Method == "POST" {
66-
Validate(ctx, ctx.csrf)
66+
ctx.csrf.Validate(ctx)
6767
if ctx.Written() {
6868
return
6969
}

modules/context/context.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ type Context struct {
5959
Render Render
6060
translation.Locale
6161
Cache cache.Cache
62-
csrf CSRF
62+
csrf CSRFProtector
6363
Flash *middleware.Flash
6464
Session session.Store
6565

@@ -666,7 +666,9 @@ func Auth(authMethod auth.Method) func(*Context) {
666666
func Contexter() func(next http.Handler) http.Handler {
667667
rnd := templates.HTMLRenderer()
668668
csrfOpts := getCsrfOpts()
669-
669+
if !setting.IsProd {
670+
CsrfTokenRegenerationInterval = 5 * time.Second // in dev, re-generate the tokens more aggressively for debug purpose
671+
}
670672
return func(next http.Handler) http.Handler {
671673
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
672674
locale := middleware.Locale(resp, req)
@@ -697,7 +699,7 @@ func Contexter() func(next http.Handler) http.Handler {
697699
ctx.Data["Context"] = &ctx
698700

699701
ctx.Req = WithContext(req, &ctx)
700-
ctx.csrf = Csrfer(csrfOpts, &ctx)
702+
ctx.csrf = PrepareCSRFProtector(csrfOpts, &ctx)
701703

702704
// Get flash.
703705
flashCookie := ctx.GetCookie("macaron_flash")
@@ -755,7 +757,7 @@ func Contexter() func(next http.Handler) http.Handler {
755757

756758
ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
757759

758-
ctx.Data["CsrfToken"] = html.EscapeString(ctx.csrf.GetToken())
760+
ctx.Data["CsrfToken"] = ctx.csrf.GetToken()
759761
ctx.Data["CsrfTokenHtml"] = template.HTML(`<input type="hidden" name="_csrf" value="` + ctx.Data["CsrfToken"].(string) + `">`)
760762

761763
// FIXME: do we really always need these setting? There should be someway to have to avoid having to always set these

0 commit comments

Comments
 (0)