From ac0f452b30e12f0cf41f04fee6677030ad125b13 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 8 Jul 2021 20:12:20 +0200 Subject: [PATCH] Redirect on bad CSRF instead of presenting bad page (#14937) (#16378) The current CSRF handler is a bit harsh with bad CSRF tokens on webpages I think we can be a little kinder and redirect to base page with a flash error Signed-off-by: Andrew Thornton Co-authored-by: zeripath --- integrations/repo_branch_test.go | 11 ++++++++++- modules/context/csrf.go | 23 +++++++++++++++++++---- options/locale/locale_en-US.ini | 2 ++ 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/integrations/repo_branch_test.go b/integrations/repo_branch_test.go index de4e668987..af5c475ea7 100644 --- a/integrations/repo_branch_test.go +++ b/integrations/repo_branch_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" "github.com/stretchr/testify/assert" @@ -134,5 +135,13 @@ func TestCreateBranchInvalidCSRF(t *testing.T) { "_csrf": "fake_csrf", "new_branch_name": "test", }) - session.MakeRequest(t, req, http.StatusBadRequest) + resp := session.MakeRequest(t, req, http.StatusFound) + 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()), + ) } diff --git a/modules/context/csrf.go b/modules/context/csrf.go index ba0e9f6cde..8d179ca904 100644 --- a/modules/context/csrf.go +++ b/modules/context/csrf.go @@ -22,6 +22,7 @@ import ( "net/http" "time" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web/middleware" "github.com/unknwon/com" @@ -266,7 +267,12 @@ func Validate(ctx *Context, x CSRF) { -1, x.GetCookiePath(), x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too? - x.Error(ctx.Resp) + if middleware.IsAPIPath(ctx.Req) { + x.Error(ctx.Resp) + return + } + ctx.Flash.Error(ctx.Tr("error.invalid_csrf")) + ctx.Redirect(setting.AppSubURL + "/") } return } @@ -277,10 +283,19 @@ func Validate(ctx *Context, x CSRF) { -1, x.GetCookiePath(), x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too? - x.Error(ctx.Resp) + if middleware.IsAPIPath(ctx.Req) { + x.Error(ctx.Resp) + return + } + ctx.Flash.Error(ctx.Tr("error.invalid_csrf")) + ctx.Redirect(setting.AppSubURL + "/") } return } - - http.Error(ctx.Resp, "Bad Request: no CSRF token present", http.StatusBadRequest) + if middleware.IsAPIPath(ctx.Req) { + http.Error(ctx.Resp, "Bad Request: no CSRF token present", http.StatusBadRequest) + return + } + ctx.Flash.Error(ctx.Tr("error.missing_csrf")) + ctx.Redirect(setting.AppSubURL + "/") } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 8a17b40083..ba689d1a68 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -96,6 +96,8 @@ error404 = The page you are trying to reach either does not existGitHub and open new issue if necessary. +missing_csrf = Bad Request: no CSRF token present +invalid_csrf = Bad Request: Invalid CSRF token [startpage] app_desc = A painless, self-hosted Git service