Skip to content

Commit dd83cfc

Browse files
wxiaoguangwxiaoguang
authored
Refactor CSRF token (#32216)
1 parent 368b088 commit dd83cfc

29 files changed

+90
-126
lines changed

‎routers/web/auth/auth.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func autoSignIn(ctx *context.Context) (bool, error) {
9898
return false, err
9999
}
100100

101-
ctx.Csrf.DeleteCookie(ctx)
101+
ctx.Csrf.PrepareForSessionUser(ctx)
102102
return true, nil
103103
}
104104

@@ -359,8 +359,8 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
359359
ctx.Locale = middleware.Locale(ctx.Resp, ctx.Req)
360360
}
361361

362-
// Clear whatever CSRF cookie has right now, force to generate a new one
363-
ctx.Csrf.DeleteCookie(ctx)
362+
// force to generate a new CSRF token
363+
ctx.Csrf.PrepareForSessionUser(ctx)
364364

365365
// Register last login
366366
if err := user_service.UpdateUser(ctx, u, &user_service.UpdateOptions{SetLastLogin: true}); err != nil {
@@ -804,6 +804,8 @@ func handleAccountActivation(ctx *context.Context, user *user_model.User) {
804804
return
805805
}
806806

807+
ctx.Csrf.PrepareForSessionUser(ctx)
808+
807809
if err := resetLocale(ctx, user); err != nil {
808810
ctx.ServerError("resetLocale", err)
809811
return

‎routers/web/auth/oauth.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,8 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model
358358
return
359359
}
360360

361-
// Clear whatever CSRF cookie has right now, force to generate a new one
362-
ctx.Csrf.DeleteCookie(ctx)
361+
// force to generate a new CSRF token
362+
ctx.Csrf.PrepareForSessionUser(ctx)
363363

364364
if err := resetLocale(ctx, u); err != nil {
365365
ctx.ServerError("resetLocale", err)

‎services/auth/auth.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore
103103

104104
middleware.SetLocaleCookie(resp, user.Language, 0)
105105

106-
// Clear whatever CSRF has right now, force to generate a new one
106+
// force to generate a new CSRF token
107107
if ctx := gitea_context.GetWebContext(req); ctx != nil {
108-
ctx.Csrf.DeleteCookie(ctx)
108+
ctx.Csrf.PrepareForSessionUser(ctx)
109109
}
110110
}

‎services/context/csrf.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,8 @@ func (c *csrfProtector) PrepareForSessionUser(ctx *Context) {
129129
}
130130

131131
if needsNew {
132-
// FIXME: actionId.
133132
c.token = GenerateCsrfToken(c.opt.Secret, c.id, "POST", time.Now())
134-
cookie := newCsrfCookie(&c.opt, c.token)
135-
ctx.Resp.Header().Add("Set-Cookie", cookie.String())
133+
ctx.Resp.Header().Add("Set-Cookie", newCsrfCookie(&c.opt, c.token).String())
136134
}
137135

138136
ctx.Data["CsrfToken"] = c.token

‎tests/integration/admin_user_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func testSuccessfullEdit(t *testing.T, formData user_model.User) {
5151

5252
func makeRequest(t *testing.T, formData user_model.User, headerCode int) {
5353
session := loginUser(t, "user1")
54-
csrf := GetCSRF(t, session, "/admin/users/"+strconv.Itoa(int(formData.ID))+"/edit")
54+
csrf := GetUserCSRFToken(t, session)
5555
req := NewRequestWithValues(t, "POST", "/admin/users/"+strconv.Itoa(int(formData.ID))+"/edit", map[string]string{
5656
"_csrf": csrf,
5757
"user_name": formData.Name,
@@ -72,7 +72,7 @@ func TestAdminDeleteUser(t *testing.T) {
7272

7373
session := loginUser(t, "user1")
7474

75-
csrf := GetCSRF(t, session, "/admin/users/8/edit")
75+
csrf := GetUserCSRFToken(t, session)
7676
req := NewRequestWithValues(t, "POST", "/admin/users/8/delete", map[string]string{
7777
"_csrf": csrf,
7878
})

‎tests/integration/api_httpsig_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func TestHTTPSigCert(t *testing.T) {
9595
defer tests.PrepareTestEnv(t)()
9696
session := loginUser(t, "user1")
9797

98-
csrf := GetCSRF(t, session, "/user/settings/keys")
98+
csrf := GetUserCSRFToken(t, session)
9999
req := NewRequestWithValues(t, "POST", "/user/settings/keys", map[string]string{
100100
"_csrf": csrf,
101101
"content": "user1",

‎tests/integration/api_packages_container_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,7 @@ func TestPackageContainer(t *testing.T) {
784784
newOwnerName := "newUsername"
785785

786786
req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
787-
"_csrf": GetCSRF(t, session, "/user/settings"),
787+
"_csrf": GetUserCSRFToken(t, session),
788788
"name": newOwnerName,
789789
"email": "[email protected]",
790790
"language": "en-US",
@@ -794,7 +794,7 @@ func TestPackageContainer(t *testing.T) {
794794
t.Run(fmt.Sprintf("Catalog[%s]", newOwnerName), checkCatalog(newOwnerName))
795795

796796
req = NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
797-
"_csrf": GetCSRF(t, session, "/user/settings"),
797+
"_csrf": GetUserCSRFToken(t, session),
798798
"name": user.Name,
799799
"email": "[email protected]",
800800
"language": "en-US",

‎tests/integration/attachment_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,14 @@ func createAttachment(t *testing.T, session *TestSession, csrf, repoURL, filenam
5757
func TestCreateAnonymousAttachment(t *testing.T) {
5858
defer tests.PrepareTestEnv(t)()
5959
session := emptyTestSession(t)
60-
createAttachment(t, session, GetCSRF(t, session, "/user/login"), "user2/repo1", "image.png", generateImg(), http.StatusSeeOther)
60+
createAttachment(t, session, GetAnonymousCSRFToken(t, session), "user2/repo1", "image.png", generateImg(), http.StatusSeeOther)
6161
}
6262

6363
func TestCreateIssueAttachment(t *testing.T) {
6464
defer tests.PrepareTestEnv(t)()
6565
const repoURL = "user2/repo1"
6666
session := loginUser(t, "user2")
67-
uuid := createAttachment(t, session, GetCSRF(t, session, repoURL), repoURL, "image.png", generateImg(), http.StatusOK)
67+
uuid := createAttachment(t, session, GetUserCSRFToken(t, session), repoURL, "image.png", generateImg(), http.StatusOK)
6868

6969
req := NewRequest(t, "GET", repoURL+"/issues/new")
7070
resp := session.MakeRequest(t, req, http.StatusOK)

‎tests/integration/auth_ldap_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute, groupFilter string, groupM
156156
groupTeamMap = groupMapParams[1]
157157
}
158158
session := loginUser(t, "user1")
159-
csrf := GetCSRF(t, session, "/admin/auths/new")
159+
csrf := GetUserCSRFToken(t, session)
160160
req := NewRequestWithValues(t, "POST", "/admin/auths/new", buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap, groupTeamMapRemoval))
161161
session.MakeRequest(t, req, http.StatusSeeOther)
162162
}
@@ -252,7 +252,7 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) {
252252
defer tests.PrepareTestEnv(t)()
253253

254254
session := loginUser(t, "user1")
255-
csrf := GetCSRF(t, session, "/admin/auths/new")
255+
csrf := GetUserCSRFToken(t, session)
256256
payload := buildAuthSourceLDAPPayload(csrf, "", "", "", "")
257257
payload["attribute_username"] = ""
258258
req := NewRequestWithValues(t, "POST", "/admin/auths/new", payload)
@@ -487,7 +487,7 @@ func TestLDAPPreventInvalidGroupTeamMap(t *testing.T) {
487487
defer tests.PrepareTestEnv(t)()
488488

489489
session := loginUser(t, "user1")
490-
csrf := GetCSRF(t, session, "/admin/auths/new")
490+
csrf := GetUserCSRFToken(t, session)
491491
req := NewRequestWithValues(t, "POST", "/admin/auths/new", buildAuthSourceLDAPPayload(csrf, "", "", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, "off"))
492492
session.MakeRequest(t, req, http.StatusOK) // StatusOK = failed, StatusSeeOther = ok
493493
}

‎tests/integration/change_default_branch_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,15 @@ func TestChangeDefaultBranch(t *testing.T) {
2222
session := loginUser(t, owner.Name)
2323
branchesURL := fmt.Sprintf("/%s/%s/settings/branches", owner.Name, repo.Name)
2424

25-
csrf := GetCSRF(t, session, branchesURL)
25+
csrf := GetUserCSRFToken(t, session)
2626
req := NewRequestWithValues(t, "POST", branchesURL, map[string]string{
2727
"_csrf": csrf,
2828
"action": "default_branch",
2929
"branch": "DefaultBranch",
3030
})
3131
session.MakeRequest(t, req, http.StatusSeeOther)
3232

33-
csrf = GetCSRF(t, session, branchesURL)
33+
csrf = GetUserCSRFToken(t, session)
3434
req = NewRequestWithValues(t, "POST", branchesURL, map[string]string{
3535
"_csrf": csrf,
3636
"action": "default_branch",

0 commit comments

Comments
 (0)