Skip to content

Commit

Permalink
Refactor CSRF protection modules, make sure CSRF tokens can be up-to-…
Browse files Browse the repository at this point in the history
…date. (#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.
  • Loading branch information
wxiaoguang committed Apr 8, 2022
1 parent 3c3d498 commit 84ceaa9
Show file tree
Hide file tree
Showing 10 changed files with 166 additions and 192 deletions.
52 changes: 52 additions & 0 deletions integrations/csrf_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright 2017 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package integrations

import (
"net/http"
"strings"
"testing"

"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting"

"github.com/stretchr/testify/assert"
)

func TestCsrfProtection(t *testing.T) {
defer prepareTestEnv(t)()

// test web form csrf via form
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User)
session := loginUser(t, user.Name)
req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
"_csrf": "fake_csrf",
})
session.MakeRequest(t, req, http.StatusSeeOther)

resp := session.MakeRequest(t, req, http.StatusSeeOther)
loc := resp.Header().Get("Location")
assert.Equal(t, setting.AppSubURL+"/", loc)
resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
assert.Equal(t, "Bad Request: invalid CSRF token",
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
)

// test web form csrf via header. TODO: should use an UI api to test
req = NewRequest(t, "POST", "/user/settings")
req.Header.Add("X-Csrf-Token", "fake_csrf")
session.MakeRequest(t, req, http.StatusSeeOther)

resp = session.MakeRequest(t, req, http.StatusSeeOther)
loc = resp.Header().Get("Location")
assert.Equal(t, setting.AppSubURL+"/", loc)
resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
htmlDoc = NewHTMLParser(t, resp.Body)
assert.Equal(t, "Bad Request: invalid CSRF token",
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
)
}
1 change: 1 addition & 0 deletions integrations/repo_topic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"

api "code.gitea.io/gitea/modules/structs"

"github.com/stretchr/testify/assert"
)

Expand Down
2 changes: 1 addition & 1 deletion modules/context/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func Toggle(options *ToggleOptions) func(ctx *Context) {
}

if !options.SignOutRequired && !options.DisableCSRF && ctx.Req.Method == "POST" {
Validate(ctx, ctx.csrf)
ctx.csrf.Validate(ctx)
if ctx.Written() {
return
}
Expand Down
10 changes: 6 additions & 4 deletions modules/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type Context struct {
Render Render
translation.Locale
Cache cache.Cache
csrf CSRF
csrf CSRFProtector
Flash *middleware.Flash
Session session.Store

Expand Down Expand Up @@ -666,7 +666,9 @@ func Auth(authMethod auth.Method) func(*Context) {
func Contexter() func(next http.Handler) http.Handler {
rnd := templates.HTMLRenderer()
csrfOpts := getCsrfOpts()

if !setting.IsProd {
CsrfTokenRegenerationInterval = 5 * time.Second // in dev, re-generate the tokens more aggressively for debug purpose
}
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
locale := middleware.Locale(resp, req)
Expand Down Expand Up @@ -697,7 +699,7 @@ func Contexter() func(next http.Handler) http.Handler {
ctx.Data["Context"] = &ctx

ctx.Req = WithContext(req, &ctx)
ctx.csrf = Csrfer(csrfOpts, &ctx)
ctx.csrf = PrepareCSRFProtector(csrfOpts, &ctx)

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

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

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

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

0 comments on commit 84ceaa9

Please sign in to comment.