diff options
| author | 6543 | 2021-07-08 20:12:20 +0200 |
|---|---|---|
| committer | GitHub | 2021-07-08 20:12:20 +0200 |
| commit | ac0f452b30e12f0cf41f04fee6677030ad125b13 (patch) | |
| tree | 27f8a11edfe493e446140f26717be673157aa6ec | |
| parent | 6e5fd5c584fe5f5f4324667de4e8e274cfb3b4aa (diff) | |
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 <art27@cantab.net>
Co-authored-by: zeripath <art27@cantab.net>
| -rw-r--r-- | integrations/repo_branch_test.go | 11 | ||||
| -rw-r--r-- | modules/context/csrf.go | 23 | ||||
| -rw-r--r-- | 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 de4e66898..af5c475ea 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 ba0e9f6cd..8d179ca90 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 8a17b4008..ba689d1a6 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 <strong>does not exist</stron [error] occurred = An error has occurred report_message = If you are sure this is a Gitea bug, please search for issue on <a href="https://github.com/go-gitea/gitea/issues">GitHub</a> 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 |