mirror of
https://github.com/go-gitea/gitea
synced 2026-02-03 11:10:40 +00:00
Replace CSRF cookie with CrossOriginProtection (#36183)
Removes the CSRF cookie in favor of [`CrossOriginProtection`](https://pkg.go.dev/net/http#CrossOriginProtection) which relies purely on HTTP headers. Fixes: https://github.com/go-gitea/gitea/issues/11188 Fixes: https://github.com/go-gitea/gitea/issues/30333 Helps: https://github.com/go-gitea/gitea/issues/35107 TODOs: - [x] Fix tests - [ ] Ideally add tests to validates the protection --------- Signed-off-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
silverwind
@@ -51,38 +51,27 @@ type MergeOptions struct {
|
||||
DeleteBranch bool
|
||||
}
|
||||
|
||||
func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum string, mergeOptions MergeOptions) *httptest.ResponseRecorder {
|
||||
req := NewRequest(t, "GET", path.Join(user, repo, "pulls", pullnum))
|
||||
resp := session.MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
htmlDoc := NewHTMLParser(t, resp.Body)
|
||||
link := path.Join(user, repo, "pulls", pullnum, "merge")
|
||||
|
||||
func testPullMerge(t *testing.T, session *TestSession, user, repo, pullNum string, mergeOptions MergeOptions) *httptest.ResponseRecorder {
|
||||
options := map[string]string{
|
||||
"_csrf": htmlDoc.GetCSRF(),
|
||||
"do": string(mergeOptions.Style),
|
||||
"head_commit_id": mergeOptions.HeadCommitID,
|
||||
"do": string(mergeOptions.Style),
|
||||
"head_commit_id": mergeOptions.HeadCommitID,
|
||||
"delete_branch_after_merge": util.Iif(mergeOptions.DeleteBranch, "on", ""),
|
||||
}
|
||||
var resp *httptest.ResponseRecorder
|
||||
require.Eventually(t, func() bool {
|
||||
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/pulls/%s/merge", user, repo, pullNum), options)
|
||||
resp = session.MakeRequest(t, req, NoExpectedStatus)
|
||||
return resp.Code == http.StatusOK
|
||||
}, 5*time.Second, 50*time.Millisecond, "Timed out waiting for pull merge to succeed")
|
||||
|
||||
if mergeOptions.DeleteBranch {
|
||||
options["delete_branch_after_merge"] = "on"
|
||||
}
|
||||
redirect := test.RedirectURL(resp)
|
||||
assert.Equal(t, fmt.Sprintf("/%s/%s/pulls/%s", user, repo, pullNum), redirect)
|
||||
|
||||
req = NewRequestWithValues(t, "POST", link, options)
|
||||
resp = session.MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
respJSON := struct {
|
||||
Redirect string
|
||||
}{}
|
||||
DecodeJSON(t, resp, &respJSON)
|
||||
|
||||
assert.Equal(t, fmt.Sprintf("/%s/%s/pulls/%s", user, repo, pullnum), respJSON.Redirect)
|
||||
|
||||
pullnumInt, err := strconv.ParseInt(pullnum, 10, 64)
|
||||
pullNumInt, err := strconv.ParseInt(pullNum, 10, 64)
|
||||
assert.NoError(t, err)
|
||||
repository, err := repo_model.GetRepositoryByOwnerAndName(t.Context(), user, repo)
|
||||
assert.NoError(t, err)
|
||||
pull, err := issues_model.GetPullRequestByIndex(t.Context(), repository.ID, pullnumInt)
|
||||
pull, err := issues_model.GetPullRequestByIndex(t.Context(), repository.ID, pullNumInt)
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, pull.HasMerged)
|
||||
|
||||
@@ -97,9 +86,7 @@ func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum str
|
||||
htmlDoc := NewHTMLParser(t, resp.Body)
|
||||
link, exists := htmlDoc.doc.Find(".timeline-item .delete-branch-after-merge").Attr("data-url")
|
||||
assert.True(t, exists, "The template has changed, can not find delete button url")
|
||||
req = NewRequestWithValues(t, "POST", link, map[string]string{
|
||||
"_csrf": htmlDoc.GetCSRF(),
|
||||
})
|
||||
req = NewRequest(t, "POST", link)
|
||||
resp = session.MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
return resp
|
||||
@@ -844,11 +831,8 @@ func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) {
|
||||
HeadBranch: "master",
|
||||
})
|
||||
|
||||
// add protected branch for commit status
|
||||
csrf := GetUserCSRFToken(t, session)
|
||||
// Change the "master" branch to "protected"
|
||||
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{
|
||||
"_csrf": csrf,
|
||||
"rule_name": "master",
|
||||
"enable_push": "true",
|
||||
"enable_status_check": "true",
|
||||
@@ -937,11 +921,8 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) {
|
||||
HeadBranch: "master",
|
||||
})
|
||||
|
||||
// add protected branch for commit status
|
||||
csrf := GetUserCSRFToken(t, session)
|
||||
// Change master branch to protected
|
||||
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{
|
||||
"_csrf": csrf,
|
||||
"rule_name": "master",
|
||||
"enable_push": "true",
|
||||
"enable_status_check": "true",
|
||||
@@ -993,10 +974,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) {
|
||||
|
||||
// approve the PR from non-author
|
||||
approveSession := loginUser(t, "user2")
|
||||
req = NewRequest(t, "GET", fmt.Sprintf("/user2/repo1/pulls/%d", pr.Index))
|
||||
resp := approveSession.MakeRequest(t, req, http.StatusOK)
|
||||
htmlDoc := NewHTMLParser(t, resp.Body)
|
||||
testSubmitReview(t, approveSession, htmlDoc.GetCSRF(), "user2", "repo1", strconv.Itoa(int(pr.Index)), sha, "approve", http.StatusOK)
|
||||
testSubmitReview(t, approveSession, "user2", "repo1", strconv.Itoa(int(pr.Index)), sha, "approve", http.StatusOK)
|
||||
|
||||
time.Sleep(2 * time.Second)
|
||||
|
||||
@@ -1067,11 +1045,8 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing.
|
||||
})
|
||||
|
||||
session := loginUser(t, "user1")
|
||||
// add protected branch for commit status
|
||||
csrf := GetUserCSRFToken(t, session)
|
||||
// Change master branch to protected
|
||||
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{
|
||||
"_csrf": csrf,
|
||||
"rule_name": "master",
|
||||
"enable_push": "true",
|
||||
"enable_status_check": "true",
|
||||
@@ -1122,10 +1097,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing.
|
||||
|
||||
// approve the PR from non-author
|
||||
approveSession := loginUser(t, "user1")
|
||||
req = NewRequest(t, "GET", fmt.Sprintf("/user2/repo1/pulls/%d", pr.Index))
|
||||
resp := approveSession.MakeRequest(t, req, http.StatusOK)
|
||||
htmlDoc := NewHTMLParser(t, resp.Body)
|
||||
testSubmitReview(t, approveSession, htmlDoc.GetCSRF(), "user2", "repo1", strconv.Itoa(int(pr.Index)), sha, "approve", http.StatusOK)
|
||||
testSubmitReview(t, approveSession, "user2", "repo1", strconv.Itoa(int(pr.Index)), sha, "approve", http.StatusOK)
|
||||
|
||||
// reload pr again
|
||||
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
|
||||
@@ -1156,11 +1128,8 @@ func TestPullNonMergeForAdminWithBranchProtection(t *testing.T) {
|
||||
HeadBranch: "master",
|
||||
})
|
||||
|
||||
// add protected branch for commit status
|
||||
csrf := GetUserCSRFToken(t, session)
|
||||
// Change master branch to protected
|
||||
pbCreateReq := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{
|
||||
"_csrf": csrf,
|
||||
"rule_name": "master",
|
||||
"enable_push": "true",
|
||||
"enable_status_check": "true",
|
||||
@@ -1172,7 +1141,6 @@ func TestPullNonMergeForAdminWithBranchProtection(t *testing.T) {
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
|
||||
|
||||
mergeReq := NewRequestWithValues(t, "POST", "/api/v1/repos/user2/repo1/pulls/6/merge", map[string]string{
|
||||
"_csrf": csrf,
|
||||
"head_commit_id": "",
|
||||
"merge_when_checks_succeed": "false",
|
||||
"force_merge": "true",
|
||||
|
||||
Reference in New Issue
Block a user