Skip to content

Commit d06f9ce

Browse files
authored
Redirect on bad CSRF instead of presenting bad page (#14937)
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 <[email protected]>
1 parent fc1607b commit d06f9ce

File tree

3 files changed

+31
-5
lines changed

3 files changed

+31
-5
lines changed

integrations/repo_branch_test.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"strings"
1212
"testing"
1313

14+
"code.gitea.io/gitea/modules/setting"
1415
"code.gitea.io/gitea/modules/test"
1516

1617
"github.com/stretchr/testify/assert"
@@ -134,5 +135,13 @@ func TestCreateBranchInvalidCSRF(t *testing.T) {
134135
"_csrf": "fake_csrf",
135136
"new_branch_name": "test",
136137
})
137-
session.MakeRequest(t, req, http.StatusBadRequest)
138+
resp := session.MakeRequest(t, req, http.StatusFound)
139+
loc := resp.Header().Get("Location")
140+
assert.Equal(t, setting.AppSubURL+"/", loc)
141+
resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
142+
htmlDoc := NewHTMLParser(t, resp.Body)
143+
assert.Equal(t,
144+
"Bad Request: Invalid CSRF token",
145+
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
146+
)
138147
}

modules/context/csrf.go

+19-4
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"net/http"
2323
"time"
2424

25+
"code.gitea.io/gitea/modules/setting"
2526
"code.gitea.io/gitea/modules/web/middleware"
2627

2728
"github.com/unknwon/com"
@@ -266,7 +267,12 @@ func Validate(ctx *Context, x CSRF) {
266267
-1,
267268
x.GetCookiePath(),
268269
x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
269-
x.Error(ctx.Resp)
270+
if middleware.IsAPIPath(ctx.Req) {
271+
x.Error(ctx.Resp)
272+
return
273+
}
274+
ctx.Flash.Error(ctx.Tr("error.invalid_csrf"))
275+
ctx.Redirect(setting.AppSubURL + "/")
270276
}
271277
return
272278
}
@@ -277,10 +283,19 @@ func Validate(ctx *Context, x CSRF) {
277283
-1,
278284
x.GetCookiePath(),
279285
x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
280-
x.Error(ctx.Resp)
286+
if middleware.IsAPIPath(ctx.Req) {
287+
x.Error(ctx.Resp)
288+
return
289+
}
290+
ctx.Flash.Error(ctx.Tr("error.invalid_csrf"))
291+
ctx.Redirect(setting.AppSubURL + "/")
281292
}
282293
return
283294
}
284-
285-
http.Error(ctx.Resp, "Bad Request: no CSRF token present", http.StatusBadRequest)
295+
if middleware.IsAPIPath(ctx.Req) {
296+
http.Error(ctx.Resp, "Bad Request: no CSRF token present", http.StatusBadRequest)
297+
return
298+
}
299+
ctx.Flash.Error(ctx.Tr("error.missing_csrf"))
300+
ctx.Redirect(setting.AppSubURL + "/")
286301
}

options/locale/locale_en-US.ini

+2
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ never = Never
100100
[error]
101101
occurred = An error has occurred
102102
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.
103+
missing_csrf = Bad Request: no CSRF token present
104+
invalid_csrf = Bad Request: Invalid CSRF token
103105

104106
[startpage]
105107
app_desc = A painless, self-hosted Git service

0 commit comments

Comments
 (0)