From dd83cfcacc989d0e7cbd21ec5eba029fdfcb72dd Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 10 Oct 2024 11:48:21 +0800 Subject: [PATCH] Refactor CSRF token (#32216) --- routers/web/auth/auth.go | 8 ++- routers/web/auth/oauth.go | 4 +- services/auth/auth.go | 4 +- services/context/csrf.go | 4 +- tests/integration/admin_user_test.go | 4 +- tests/integration/api_httpsig_test.go | 2 +- .../api_packages_container_test.go | 4 +- tests/integration/attachment_test.go | 4 +- tests/integration/auth_ldap_test.go | 6 +- .../integration/change_default_branch_test.go | 4 +- tests/integration/delete_user_test.go | 4 +- tests/integration/editor_test.go | 4 +- tests/integration/empty_repo_test.go | 8 +-- tests/integration/git_test.go | 4 +- tests/integration/integration_test.go | 26 ++++----- tests/integration/issue_test.go | 20 +++---- tests/integration/mirror_push_test.go | 4 +- tests/integration/nonascii_branches_test.go | 2 +- tests/integration/org_project_test.go | 4 +- tests/integration/org_team_invite_test.go | 57 ++++++------------- tests/integration/privateactivity_test.go | 2 +- tests/integration/pull_merge_test.go | 6 +- tests/integration/pull_status_test.go | 6 +- tests/integration/rename_branch_test.go | 2 +- tests/integration/repo_branch_test.go | 9 +-- tests/integration/signin_test.go | 2 - tests/integration/user_avatar_test.go | 2 +- tests/integration/user_test.go | 8 +-- tests/integration/xss_test.go | 2 +- 29 files changed, 90 insertions(+), 126 deletions(-) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 5cbe2f5388..c9ef9193f1 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -98,7 +98,7 @@ func autoSignIn(ctx *context.Context) (bool, error) { return false, err } - ctx.Csrf.DeleteCookie(ctx) + ctx.Csrf.PrepareForSessionUser(ctx) return true, nil } @@ -359,8 +359,8 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe ctx.Locale = middleware.Locale(ctx.Resp, ctx.Req) } - // Clear whatever CSRF cookie has right now, force to generate a new one - ctx.Csrf.DeleteCookie(ctx) + // force to generate a new CSRF token + ctx.Csrf.PrepareForSessionUser(ctx) // Register last login 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) { return } + ctx.Csrf.PrepareForSessionUser(ctx) + if err := resetLocale(ctx, user); err != nil { ctx.ServerError("resetLocale", err) return diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index ccbb3bebf1..730d68051b 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -358,8 +358,8 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model return } - // Clear whatever CSRF cookie has right now, force to generate a new one - ctx.Csrf.DeleteCookie(ctx) + // force to generate a new CSRF token + ctx.Csrf.PrepareForSessionUser(ctx) if err := resetLocale(ctx, u); err != nil { ctx.ServerError("resetLocale", err) diff --git a/services/auth/auth.go b/services/auth/auth.go index a2523a2452..43ff95f053 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -103,8 +103,8 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore middleware.SetLocaleCookie(resp, user.Language, 0) - // Clear whatever CSRF has right now, force to generate a new one + // force to generate a new CSRF token if ctx := gitea_context.GetWebContext(req); ctx != nil { - ctx.Csrf.DeleteCookie(ctx) + ctx.Csrf.PrepareForSessionUser(ctx) } } diff --git a/services/context/csrf.go b/services/context/csrf.go index 9b66d613e3..7b475a8fd8 100644 --- a/services/context/csrf.go +++ b/services/context/csrf.go @@ -129,10 +129,8 @@ func (c *csrfProtector) PrepareForSessionUser(ctx *Context) { } if needsNew { - // FIXME: actionId. c.token = GenerateCsrfToken(c.opt.Secret, c.id, "POST", time.Now()) - cookie := newCsrfCookie(&c.opt, c.token) - ctx.Resp.Header().Add("Set-Cookie", cookie.String()) + ctx.Resp.Header().Add("Set-Cookie", newCsrfCookie(&c.opt, c.token).String()) } ctx.Data["CsrfToken"] = c.token diff --git a/tests/integration/admin_user_test.go b/tests/integration/admin_user_test.go index 669060c787..090e60da29 100644 --- a/tests/integration/admin_user_test.go +++ b/tests/integration/admin_user_test.go @@ -51,7 +51,7 @@ func testSuccessfullEdit(t *testing.T, formData user_model.User) { func makeRequest(t *testing.T, formData user_model.User, headerCode int) { session := loginUser(t, "user1") - csrf := GetCSRF(t, session, "/admin/users/"+strconv.Itoa(int(formData.ID))+"/edit") + csrf := GetUserCSRFToken(t, session) req := NewRequestWithValues(t, "POST", "/admin/users/"+strconv.Itoa(int(formData.ID))+"/edit", map[string]string{ "_csrf": csrf, "user_name": formData.Name, @@ -72,7 +72,7 @@ func TestAdminDeleteUser(t *testing.T) { session := loginUser(t, "user1") - csrf := GetCSRF(t, session, "/admin/users/8/edit") + csrf := GetUserCSRFToken(t, session) req := NewRequestWithValues(t, "POST", "/admin/users/8/delete", map[string]string{ "_csrf": csrf, }) diff --git a/tests/integration/api_httpsig_test.go b/tests/integration/api_httpsig_test.go index cca477f5e1..b9dc508ad0 100644 --- a/tests/integration/api_httpsig_test.go +++ b/tests/integration/api_httpsig_test.go @@ -95,7 +95,7 @@ func TestHTTPSigCert(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user1") - csrf := GetCSRF(t, session, "/user/settings/keys") + csrf := GetUserCSRFToken(t, session) req := NewRequestWithValues(t, "POST", "/user/settings/keys", map[string]string{ "_csrf": csrf, "content": "user1", diff --git a/tests/integration/api_packages_container_test.go b/tests/integration/api_packages_container_test.go index 409e7513a6..3905ad1b70 100644 --- a/tests/integration/api_packages_container_test.go +++ b/tests/integration/api_packages_container_test.go @@ -784,7 +784,7 @@ func TestPackageContainer(t *testing.T) { newOwnerName := "newUsername" req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "name": newOwnerName, "email": "user2@example.com", "language": "en-US", @@ -794,7 +794,7 @@ func TestPackageContainer(t *testing.T) { t.Run(fmt.Sprintf("Catalog[%s]", newOwnerName), checkCatalog(newOwnerName)) req = NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "name": user.Name, "email": "user2@example.com", "language": "en-US", diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go index 11aa03bb7e..30c394e9b0 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -57,14 +57,14 @@ func createAttachment(t *testing.T, session *TestSession, csrf, repoURL, filenam func TestCreateAnonymousAttachment(t *testing.T) { defer tests.PrepareTestEnv(t)() session := emptyTestSession(t) - createAttachment(t, session, GetCSRF(t, session, "/user/login"), "user2/repo1", "image.png", generateImg(), http.StatusSeeOther) + createAttachment(t, session, GetAnonymousCSRFToken(t, session), "user2/repo1", "image.png", generateImg(), http.StatusSeeOther) } func TestCreateIssueAttachment(t *testing.T) { defer tests.PrepareTestEnv(t)() const repoURL = "user2/repo1" session := loginUser(t, "user2") - uuid := createAttachment(t, session, GetCSRF(t, session, repoURL), repoURL, "image.png", generateImg(), http.StatusOK) + uuid := createAttachment(t, session, GetUserCSRFToken(t, session), repoURL, "image.png", generateImg(), http.StatusOK) req := NewRequest(t, "GET", repoURL+"/issues/new") resp := session.MakeRequest(t, req, http.StatusOK) diff --git a/tests/integration/auth_ldap_test.go b/tests/integration/auth_ldap_test.go index 317787f403..deb79187eb 100644 --- a/tests/integration/auth_ldap_test.go +++ b/tests/integration/auth_ldap_test.go @@ -156,7 +156,7 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute, groupFilter string, groupM groupTeamMap = groupMapParams[1] } session := loginUser(t, "user1") - csrf := GetCSRF(t, session, "/admin/auths/new") + csrf := GetUserCSRFToken(t, session) req := NewRequestWithValues(t, "POST", "/admin/auths/new", buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap, groupTeamMapRemoval)) session.MakeRequest(t, req, http.StatusSeeOther) } @@ -252,7 +252,7 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user1") - csrf := GetCSRF(t, session, "/admin/auths/new") + csrf := GetUserCSRFToken(t, session) payload := buildAuthSourceLDAPPayload(csrf, "", "", "", "") payload["attribute_username"] = "" req := NewRequestWithValues(t, "POST", "/admin/auths/new", payload) @@ -487,7 +487,7 @@ func TestLDAPPreventInvalidGroupTeamMap(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user1") - csrf := GetCSRF(t, session, "/admin/auths/new") + csrf := GetUserCSRFToken(t, session) req := NewRequestWithValues(t, "POST", "/admin/auths/new", buildAuthSourceLDAPPayload(csrf, "", "", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, "off")) session.MakeRequest(t, req, http.StatusOK) // StatusOK = failed, StatusSeeOther = ok } diff --git a/tests/integration/change_default_branch_test.go b/tests/integration/change_default_branch_test.go index 703834b712..729eb1e4ce 100644 --- a/tests/integration/change_default_branch_test.go +++ b/tests/integration/change_default_branch_test.go @@ -22,7 +22,7 @@ func TestChangeDefaultBranch(t *testing.T) { session := loginUser(t, owner.Name) branchesURL := fmt.Sprintf("/%s/%s/settings/branches", owner.Name, repo.Name) - csrf := GetCSRF(t, session, branchesURL) + csrf := GetUserCSRFToken(t, session) req := NewRequestWithValues(t, "POST", branchesURL, map[string]string{ "_csrf": csrf, "action": "default_branch", @@ -30,7 +30,7 @@ func TestChangeDefaultBranch(t *testing.T) { }) session.MakeRequest(t, req, http.StatusSeeOther) - csrf = GetCSRF(t, session, branchesURL) + csrf = GetUserCSRFToken(t, session) req = NewRequestWithValues(t, "POST", branchesURL, map[string]string{ "_csrf": csrf, "action": "default_branch", diff --git a/tests/integration/delete_user_test.go b/tests/integration/delete_user_test.go index 806b87dc4c..ad3c882882 100644 --- a/tests/integration/delete_user_test.go +++ b/tests/integration/delete_user_test.go @@ -33,7 +33,7 @@ func TestUserDeleteAccount(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user8") - csrf := GetCSRF(t, session, "/user/settings/account") + csrf := GetUserCSRFToken(t, session) urlStr := fmt.Sprintf("/user/settings/account/delete?password=%s", userPassword) req := NewRequestWithValues(t, "POST", urlStr, map[string]string{ "_csrf": csrf, @@ -48,7 +48,7 @@ func TestUserDeleteAccountStillOwnRepos(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") - csrf := GetCSRF(t, session, "/user/settings/account") + csrf := GetUserCSRFToken(t, session) urlStr := fmt.Sprintf("/user/settings/account/delete?password=%s", userPassword) req := NewRequestWithValues(t, "POST", urlStr, map[string]string{ "_csrf": csrf, diff --git a/tests/integration/editor_test.go b/tests/integration/editor_test.go index f510c79bc6..f0f71b80d1 100644 --- a/tests/integration/editor_test.go +++ b/tests/integration/editor_test.go @@ -49,7 +49,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user2") - csrf := GetCSRF(t, session, "/user2/repo1/settings/branches") + csrf := GetUserCSRFToken(t, session) // Change master branch to protected req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ "_csrf": csrf, @@ -84,7 +84,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { assert.Contains(t, resp.Body.String(), "Cannot commit to protected branch "master".") // remove the protected branch - csrf = GetCSRF(t, session, "/user2/repo1/settings/branches") + csrf = GetUserCSRFToken(t, session) // Change master branch to protected req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/1/delete", map[string]string{ diff --git a/tests/integration/empty_repo_test.go b/tests/integration/empty_repo_test.go index 002aa5600e..630a3c03af 100644 --- a/tests/integration/empty_repo_test.go +++ b/tests/integration/empty_repo_test.go @@ -29,7 +29,7 @@ import ( func testAPINewFile(t *testing.T, session *TestSession, user, repo, branch, treePath, content string) *httptest.ResponseRecorder { url := fmt.Sprintf("/%s/%s/_new/%s", user, repo, branch) req := NewRequestWithValues(t, "POST", url, map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "commit_choice": "direct", "tree_path": treePath, "content": content, @@ -63,7 +63,7 @@ func TestEmptyRepoAddFile(t *testing.T) { doc := NewHTMLParser(t, resp.Body).Find(`input[name="commit_choice"]`) assert.Empty(t, doc.AttrOr("checked", "_no_")) req = NewRequestWithValues(t, "POST", "/user30/empty/_new/"+setting.Repository.DefaultBranch, map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "commit_choice": "direct", "tree_path": "test-file.md", "content": "newly-added-test-file", @@ -89,7 +89,7 @@ func TestEmptyRepoUploadFile(t *testing.T) { body := &bytes.Buffer{} mpForm := multipart.NewWriter(body) - _ = mpForm.WriteField("_csrf", GetCSRF(t, session, "/user/settings")) + _ = mpForm.WriteField("_csrf", GetUserCSRFToken(t, session)) file, _ := mpForm.CreateFormFile("file", "uploaded-file.txt") _, _ = io.Copy(file, bytes.NewBufferString("newly-uploaded-test-file")) _ = mpForm.Close() @@ -101,7 +101,7 @@ func TestEmptyRepoUploadFile(t *testing.T) { assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), &respMap)) req = NewRequestWithValues(t, "POST", "/user30/empty/_upload/"+setting.Repository.DefaultBranch, map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "commit_choice": "direct", "files": respMap["uuid"], "tree_path": "", diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index ac56cffe5e..f024d22c4a 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -462,7 +462,7 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes func doProtectBranch(ctx APITestContext, branch, userToWhitelistPush, userToWhitelistForcePush, unprotectedFilePatterns string) func(t *testing.T) { // We are going to just use the owner to set the protection. return func(t *testing.T) { - csrf := GetCSRF(t, ctx.Session, fmt.Sprintf("/%s/%s/settings/branches", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame))) + csrf := GetUserCSRFToken(t, ctx.Session) formData := map[string]string{ "_csrf": csrf, @@ -644,7 +644,7 @@ func doPushCreate(ctx APITestContext, u *url.URL) func(t *testing.T) { func doBranchDelete(ctx APITestContext, owner, repo, branch string) func(*testing.T) { return func(t *testing.T) { - csrf := GetCSRF(t, ctx.Session, fmt.Sprintf("/%s/%s/branches", url.PathEscape(owner), url.PathEscape(repo))) + csrf := GetUserCSRFToken(t, ctx.Session) req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/branches/delete?name=%s", url.PathEscape(owner), url.PathEscape(repo), url.QueryEscape(branch)), map[string]string{ "_csrf": csrf, diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 1f12430fcf..f72ac5f51c 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -486,23 +486,19 @@ func VerifyJSONSchema(t testing.TB, resp *httptest.ResponseRecorder, schemaFile assert.True(t, result.Valid()) } -// GetCSRF returns CSRF token from body -// If it fails, it means the CSRF token is not found in the response body returned by the url with the given session. -// In this case, you should find a better url to get it. -func GetCSRF(t testing.TB, session *TestSession, urlStr string) string { +// GetUserCSRFToken returns CSRF token for current user +func GetUserCSRFToken(t testing.TB, session *TestSession) string { t.Helper() - req := NewRequest(t, "GET", urlStr) - resp := session.MakeRequest(t, req, http.StatusOK) - doc := NewHTMLParser(t, resp.Body) - csrf := doc.GetCSRF() - require.NotEmpty(t, csrf) - return csrf + cookie := session.GetCookie("_csrf") + require.NotEmpty(t, cookie) + return cookie.Value } -// GetCSRFFrom returns CSRF token from body -func GetCSRFFromCookie(t testing.TB, session *TestSession, urlStr string) string { +// GetUserCSRFToken returns CSRF token for anonymous user (not logged in) +func GetAnonymousCSRFToken(t testing.TB, session *TestSession) string { t.Helper() - req := NewRequest(t, "GET", urlStr) - session.MakeRequest(t, req, http.StatusOK) - return session.GetCookie("_csrf").Value + resp := session.MakeRequest(t, NewRequest(t, "GET", "/user/login"), http.StatusOK) + csrfToken := NewHTMLParser(t, resp.Body).GetCSRF() + require.NotEmpty(t, csrfToken) + return csrfToken } diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index 308b82d4b9..df45da84a5 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -197,21 +197,21 @@ func TestEditIssue(t *testing.T) { issueURL := testNewIssue(t, session, "user2", "repo1", "Title", "Description") req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/content", issueURL), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), + "_csrf": GetUserCSRFToken(t, session), "content": "modified content", "context": fmt.Sprintf("/%s/%s", "user2", "repo1"), }) session.MakeRequest(t, req, http.StatusOK) req = NewRequestWithValues(t, "POST", fmt.Sprintf("%s/content", issueURL), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), + "_csrf": GetUserCSRFToken(t, session), "content": "modified content", "context": fmt.Sprintf("/%s/%s", "user2", "repo1"), }) session.MakeRequest(t, req, http.StatusBadRequest) req = NewRequestWithValues(t, "POST", fmt.Sprintf("%s/content", issueURL), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), + "_csrf": GetUserCSRFToken(t, session), "content": "modified content", "content_version": "1", "context": fmt.Sprintf("/%s/%s", "user2", "repo1"), @@ -246,11 +246,11 @@ func TestIssueCommentDelete(t *testing.T) { // Using the ID of a comment that does not belong to the repository must fail req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d/delete", "user5", "repo4", commentID), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), + "_csrf": GetUserCSRFToken(t, session), }) session.MakeRequest(t, req, http.StatusNotFound) req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d/delete", "user2", "repo1", commentID), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), + "_csrf": GetUserCSRFToken(t, session), }) session.MakeRequest(t, req, http.StatusOK) unittest.AssertNotExistsBean(t, &issues_model.Comment{ID: commentID}) @@ -270,13 +270,13 @@ func TestIssueCommentUpdate(t *testing.T) { // Using the ID of a comment that does not belong to the repository must fail req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user5", "repo4", commentID), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), + "_csrf": GetUserCSRFToken(t, session), "content": modifiedContent, }) session.MakeRequest(t, req, http.StatusNotFound) req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), + "_csrf": GetUserCSRFToken(t, session), "content": modifiedContent, }) session.MakeRequest(t, req, http.StatusOK) @@ -298,7 +298,7 @@ func TestIssueCommentUpdateSimultaneously(t *testing.T) { modifiedContent := comment.Content + "MODIFIED" req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), + "_csrf": GetUserCSRFToken(t, session), "content": modifiedContent, }) session.MakeRequest(t, req, http.StatusOK) @@ -306,13 +306,13 @@ func TestIssueCommentUpdateSimultaneously(t *testing.T) { modifiedContent = comment.Content + "2" req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), + "_csrf": GetUserCSRFToken(t, session), "content": modifiedContent, }) session.MakeRequest(t, req, http.StatusBadRequest) req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ - "_csrf": GetCSRF(t, session, issueURL), + "_csrf": GetUserCSRFToken(t, session), "content": modifiedContent, "content_version": "1", }) diff --git a/tests/integration/mirror_push_test.go b/tests/integration/mirror_push_test.go index 1c262b3349..6b1c808cf4 100644 --- a/tests/integration/mirror_push_test.go +++ b/tests/integration/mirror_push_test.go @@ -81,7 +81,7 @@ func testMirrorPush(t *testing.T, u *url.URL) { func doCreatePushMirror(ctx APITestContext, address, username, password string) func(t *testing.T) { return func(t *testing.T) { - csrf := GetCSRF(t, ctx.Session, fmt.Sprintf("/%s/%s/settings", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame))) + csrf := GetUserCSRFToken(t, ctx.Session) req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)), map[string]string{ "_csrf": csrf, @@ -101,7 +101,7 @@ func doCreatePushMirror(ctx APITestContext, address, username, password string) func doRemovePushMirror(ctx APITestContext, address, username, password string, pushMirrorID int) func(t *testing.T) { return func(t *testing.T) { - csrf := GetCSRF(t, ctx.Session, fmt.Sprintf("/%s/%s/settings", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame))) + csrf := GetUserCSRFToken(t, ctx.Session) req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)), map[string]string{ "_csrf": csrf, diff --git a/tests/integration/nonascii_branches_test.go b/tests/integration/nonascii_branches_test.go index a189273eac..e5934a148d 100644 --- a/tests/integration/nonascii_branches_test.go +++ b/tests/integration/nonascii_branches_test.go @@ -17,7 +17,7 @@ import ( func setDefaultBranch(t *testing.T, session *TestSession, user, repo, branch string) { location := path.Join("/", user, repo, "settings/branches") - csrf := GetCSRF(t, session, location) + csrf := GetUserCSRFToken(t, session) req := NewRequestWithValues(t, "POST", location, map[string]string{ "_csrf": csrf, "action": "default_branch", diff --git a/tests/integration/org_project_test.go b/tests/integration/org_project_test.go index 31d10f16ff..c3894fd7af 100644 --- a/tests/integration/org_project_test.go +++ b/tests/integration/org_project_test.go @@ -34,7 +34,7 @@ func TestOrgProjectAccess(t *testing.T) { // change the org's visibility to private session := loginUser(t, "user2") req = NewRequestWithValues(t, "POST", "/org/org3/settings", map[string]string{ - "_csrf": GetCSRF(t, session, "/org3/-/projects"), + "_csrf": GetUserCSRFToken(t, session), "name": "org3", "visibility": "2", }) @@ -48,7 +48,7 @@ func TestOrgProjectAccess(t *testing.T) { // disable team1's project unit session = loginUser(t, "user2") req = NewRequestWithValues(t, "POST", "/org/org3/teams/team1/edit", map[string]string{ - "_csrf": GetCSRF(t, session, "/org3/-/projects"), + "_csrf": GetUserCSRFToken(t, session), "team_name": "team1", "repo_access": "specific", "permission": "read", diff --git a/tests/integration/org_team_invite_test.go b/tests/integration/org_team_invite_test.go index 919769a61a..274fde4085 100644 --- a/tests/integration/org_team_invite_test.go +++ b/tests/integration/org_team_invite_test.go @@ -40,7 +40,7 @@ func TestOrgTeamEmailInvite(t *testing.T) { session := loginUser(t, "user1") teamURL := fmt.Sprintf("/org/%s/teams/%s", org.Name, team.Name) - csrf := GetCSRF(t, session, teamURL) + csrf := GetUserCSRFToken(t, session) req := NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{ "_csrf": csrf, "uid": "1", @@ -59,7 +59,7 @@ func TestOrgTeamEmailInvite(t *testing.T) { // join the team inviteURL := fmt.Sprintf("/org/invite/%s", invites[0].Token) - csrf = GetCSRF(t, session, inviteURL) + csrf = GetUserCSRFToken(t, session) req = NewRequestWithValues(t, "POST", inviteURL, map[string]string{ "_csrf": csrf, }) @@ -94,7 +94,7 @@ func TestOrgTeamEmailInviteRedirectsExistingUser(t *testing.T) { teamURL := fmt.Sprintf("/org/%s/teams/%s", org.Name, team.Name) req := NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{ - "_csrf": GetCSRF(t, session, teamURL), + "_csrf": GetUserCSRFToken(t, session), "uid": "1", "uname": user.Email, }) @@ -137,7 +137,7 @@ func TestOrgTeamEmailInviteRedirectsExistingUser(t *testing.T) { // make the request req = NewRequestWithValues(t, "POST", test.RedirectURL(resp), map[string]string{ - "_csrf": GetCSRF(t, session, test.RedirectURL(resp)), + "_csrf": GetUserCSRFToken(t, session), }) resp = session.MakeRequest(t, req, http.StatusSeeOther) req = NewRequest(t, "GET", test.RedirectURL(resp)) @@ -165,7 +165,7 @@ func TestOrgTeamEmailInviteRedirectsNewUser(t *testing.T) { teamURL := fmt.Sprintf("/org/%s/teams/%s", org.Name, team.Name) req := NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{ - "_csrf": GetCSRF(t, session, teamURL), + "_csrf": GetUserCSRFToken(t, session), "uid": "1", "uname": "doesnotexist@example.com", }) @@ -210,7 +210,7 @@ func TestOrgTeamEmailInviteRedirectsNewUser(t *testing.T) { // make the redirected request req = NewRequestWithValues(t, "POST", test.RedirectURL(resp), map[string]string{ - "_csrf": GetCSRF(t, session, test.RedirectURL(resp)), + "_csrf": GetUserCSRFToken(t, session), }) resp = session.MakeRequest(t, req, http.StatusSeeOther) req = NewRequest(t, "GET", test.RedirectURL(resp)) @@ -233,22 +233,18 @@ func TestOrgTeamEmailInviteRedirectsNewUserWithActivation(t *testing.T) { } // enable email confirmation temporarily - defer func(prevVal bool) { - setting.Service.RegisterEmailConfirm = prevVal - }(setting.Service.RegisterEmailConfirm) - setting.Service.RegisterEmailConfirm = true - + defer test.MockVariableValue(&setting.Service.RegisterEmailConfirm, true)() defer tests.PrepareTestEnv(t)() org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}) team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) - // create the invite + // user1: create the invite session := loginUser(t, "user1") teamURL := fmt.Sprintf("/org/%s/teams/%s", org.Name, team.Name) req := NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{ - "_csrf": GetCSRF(t, session, teamURL), + "_csrf": GetUserCSRFToken(t, session), "uid": "1", "uname": "doesnotexist@example.com", }) @@ -261,53 +257,34 @@ func TestOrgTeamEmailInviteRedirectsNewUserWithActivation(t *testing.T) { assert.NoError(t, err) assert.Len(t, invites, 1) - // accept the invite + // new user: accept the invite + session = emptyTestSession(t) + inviteURL := fmt.Sprintf("/org/invite/%s", invites[0].Token) req = NewRequest(t, "GET", fmt.Sprintf("/user/sign_up?redirect_to=%s", url.QueryEscape(inviteURL))) - inviteResp := MakeRequest(t, req, http.StatusOK) - - doc := NewHTMLParser(t, resp.Body) + session.MakeRequest(t, req, http.StatusOK) req = NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{ - "_csrf": doc.GetCSRF(), "user_name": "doesnotexist", "email": "doesnotexist@example.com", "password": "examplePassword!1", "retype": "examplePassword!1", }) - for _, c := range inviteResp.Result().Cookies() { - req.AddCookie(c) - } - - resp = MakeRequest(t, req, http.StatusOK) + session.MakeRequest(t, req, http.StatusOK) user, err := user_model.GetUserByName(db.DefaultContext, "doesnotexist") assert.NoError(t, err) - ch := http.Header{} - ch.Add("Cookie", strings.Join(resp.Header()["Set-Cookie"], ";")) - cr := http.Request{Header: ch} - - session = emptyTestSession(t) - baseURL, err := url.Parse(setting.AppURL) - assert.NoError(t, err) - session.jar.SetCookies(baseURL, cr.Cookies()) - activateURL := fmt.Sprintf("/user/activate?code=%s", user.GenerateEmailActivateCode("doesnotexist@example.com")) req = NewRequestWithValues(t, "POST", activateURL, map[string]string{ "password": "examplePassword!1", }) - // use the cookies set by the signup request - for _, c := range inviteResp.Result().Cookies() { - req.AddCookie(c) - } - resp = session.MakeRequest(t, req, http.StatusSeeOther) // should be redirected to accept the invite assert.Equal(t, inviteURL, test.RedirectURL(resp)) req = NewRequestWithValues(t, "POST", test.RedirectURL(resp), map[string]string{ - "_csrf": GetCSRF(t, session, test.RedirectURL(resp)), + "_csrf": GetUserCSRFToken(t, session), }) resp = session.MakeRequest(t, req, http.StatusSeeOther) req = NewRequest(t, "GET", test.RedirectURL(resp)) @@ -342,7 +319,7 @@ func TestOrgTeamEmailInviteRedirectsExistingUserWithLogin(t *testing.T) { teamURL := fmt.Sprintf("/org/%s/teams/%s", org.Name, team.Name) req := NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{ - "_csrf": GetCSRF(t, session, teamURL), + "_csrf": GetUserCSRFToken(t, session), "uid": "1", "uname": user.Email, }) @@ -366,7 +343,7 @@ func TestOrgTeamEmailInviteRedirectsExistingUserWithLogin(t *testing.T) { // make the request req = NewRequestWithValues(t, "POST", test.RedirectURL(resp), map[string]string{ - "_csrf": GetCSRF(t, session, test.RedirectURL(resp)), + "_csrf": GetUserCSRFToken(t, session), }) resp = session.MakeRequest(t, req, http.StatusSeeOther) req = NewRequest(t, "GET", test.RedirectURL(resp)) diff --git a/tests/integration/privateactivity_test.go b/tests/integration/privateactivity_test.go index 5362462f7d..a1fbadec99 100644 --- a/tests/integration/privateactivity_test.go +++ b/tests/integration/privateactivity_test.go @@ -48,7 +48,7 @@ func testPrivateActivityDoSomethingForActionEntries(t *testing.T) { func testPrivateActivityHelperEnablePrivateActivity(t *testing.T) { session := loginUser(t, privateActivityTestUser) req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "name": privateActivityTestUser, "email": privateActivityTestUser + "@example.com", "language": "en-US", diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 9a412329a1..c1c8a8bf4e 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -694,7 +694,7 @@ func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) { }) // add protected branch for commit status - csrf := GetCSRF(t, session, "/user2/repo1/settings/branches") + csrf := GetUserCSRFToken(t, session) // Change master branch to protected req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ "_csrf": csrf, @@ -777,7 +777,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) { }) // add protected branch for commit status - csrf := GetCSRF(t, session, "/user2/repo1/settings/branches") + csrf := GetUserCSRFToken(t, session) // Change master branch to protected req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ "_csrf": csrf, @@ -905,7 +905,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing. session := loginUser(t, "user1") // add protected branch for commit status - csrf := GetCSRF(t, session, "/user2/repo1/settings/branches") + csrf := GetUserCSRFToken(t, session) // Change master branch to protected req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ "_csrf": csrf, diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index 26e1baeb11..ac9036ca96 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -29,7 +29,7 @@ func TestPullCreate_CommitStatus(t *testing.T) { url := path.Join("user1", "repo1", "compare", "master...status1") req := NewRequestWithValues(t, "POST", url, map[string]string{ - "_csrf": GetCSRF(t, session, url), + "_csrf": GetUserCSRFToken(t, session), "title": "pull request from status1", }, ) @@ -129,7 +129,7 @@ func TestPullCreate_EmptyChangesWithDifferentCommits(t *testing.T) { url := path.Join("user1", "repo1", "compare", "master...status1") req := NewRequestWithValues(t, "POST", url, map[string]string{ - "_csrf": GetCSRF(t, session, url), + "_csrf": GetUserCSRFToken(t, session), "title": "pull request from status1", }, ) @@ -152,7 +152,7 @@ func TestPullCreate_EmptyChangesWithSameCommits(t *testing.T) { url := path.Join("user1", "repo1", "compare", "master...status1") req := NewRequestWithValues(t, "POST", url, map[string]string{ - "_csrf": GetCSRF(t, session, url), + "_csrf": GetUserCSRFToken(t, session), "title": "pull request from status1", }, ) diff --git a/tests/integration/rename_branch_test.go b/tests/integration/rename_branch_test.go index 71bfb6b6cb..576264ba95 100644 --- a/tests/integration/rename_branch_test.go +++ b/tests/integration/rename_branch_test.go @@ -54,7 +54,7 @@ func testRenameBranch(t *testing.T, u *url.URL) { assert.Equal(t, "main", repo1.DefaultBranch) // create branch1 - csrf := GetCSRF(t, session, "/user2/repo1/src/branch/main") + csrf := GetUserCSRFToken(t, session) req = NewRequestWithValues(t, "POST", "/user2/repo1/branches/_new/branch/main", map[string]string{ "_csrf": csrf, diff --git a/tests/integration/repo_branch_test.go b/tests/integration/repo_branch_test.go index f5217374b0..6d1cc8afcf 100644 --- a/tests/integration/repo_branch_test.go +++ b/tests/integration/repo_branch_test.go @@ -27,14 +27,7 @@ import ( ) func testCreateBranch(t testing.TB, session *TestSession, user, repo, oldRefSubURL, newBranchName string, expectedStatus int) string { - var csrf string - if expectedStatus == http.StatusNotFound { - // src/branch/branch_name may not container "_csrf" input, - // so we need to get it from cookies not from body - csrf = GetCSRFFromCookie(t, session, path.Join(user, repo, "src/branch/master")) - } else { - csrf = GetCSRFFromCookie(t, session, path.Join(user, repo, "src", oldRefSubURL)) - } + csrf := GetUserCSRFToken(t, session) req := NewRequestWithValues(t, "POST", path.Join(user, repo, "branches/_new", oldRefSubURL), map[string]string{ "_csrf": csrf, "new_branch_name": newBranchName, diff --git a/tests/integration/signin_test.go b/tests/integration/signin_test.go index 77e19bba96..886d4a8259 100644 --- a/tests/integration/signin_test.go +++ b/tests/integration/signin_test.go @@ -21,7 +21,6 @@ import ( func testLoginFailed(t *testing.T, username, password, message string) { session := emptyTestSession(t) req := NewRequestWithValues(t, "POST", "/user/login", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/login"), "user_name": username, "password": password, }) @@ -68,7 +67,6 @@ func TestSigninWithRememberMe(t *testing.T) { session := emptyTestSession(t) req := NewRequestWithValues(t, "POST", "/user/login", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/login"), "user_name": user.Name, "password": userPassword, "remember": "on", diff --git a/tests/integration/user_avatar_test.go b/tests/integration/user_avatar_test.go index ec5813df0d..caca9a3e56 100644 --- a/tests/integration/user_avatar_test.go +++ b/tests/integration/user_avatar_test.go @@ -37,7 +37,7 @@ func TestUserAvatar(t *testing.T) { } session := loginUser(t, "user2") - csrf := GetCSRF(t, session, "/user/settings") + csrf := GetUserCSRFToken(t, session) imgData := &bytes.Buffer{} diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index c4544f37aa..53d88aeb37 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -33,7 +33,7 @@ func TestRenameUsername(t *testing.T) { session := loginUser(t, "user2") req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "name": "newUsername", "email": "user2@example.com", "language": "en-US", @@ -77,7 +77,7 @@ func TestRenameInvalidUsername(t *testing.T) { t.Logf("Testing username %s", invalidUsername) req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "name": invalidUsername, "email": "user2@example.com", }) @@ -135,7 +135,7 @@ func TestRenameReservedUsername(t *testing.T) { for _, reservedUsername := range reservedUsernames { t.Logf("Testing username %s", reservedUsername) req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "name": reservedUsername, "email": "user2@example.com", "language": "en-US", @@ -293,7 +293,7 @@ func TestUserLocationMapLink(t *testing.T) { session := loginUser(t, "user2") req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "name": "user2", "email": "user@example.com", "language": "en-US", diff --git a/tests/integration/xss_test.go b/tests/integration/xss_test.go index e575ed3990..a8eaa5fc62 100644 --- a/tests/integration/xss_test.go +++ b/tests/integration/xss_test.go @@ -21,7 +21,7 @@ func TestXSSUserFullName(t *testing.T) { session := loginUser(t, user.Name) req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ - "_csrf": GetCSRF(t, session, "/user/settings"), + "_csrf": GetUserCSRFToken(t, session), "name": user.Name, "full_name": fullName, "email": user.Email,