From 2cbc7d6a676770a38dcb3730dec3b4a00ac08bcd Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 25 Sep 2024 14:42:02 +0800 Subject: [PATCH 1/8] test: fix GetCSRF --- tests/integration/integration_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index ae8ff51d43613..1f12430fcfbf6 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -37,6 +37,7 @@ import ( "github.com/PuerkitoBio/goquery" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/xeipuuv/gojsonschema" ) @@ -486,12 +487,16 @@ func VerifyJSONSchema(t testing.TB, resp *httptest.ResponseRecorder, schemaFile } // 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 { t.Helper() req := NewRequest(t, "GET", urlStr) resp := session.MakeRequest(t, req, http.StatusOK) doc := NewHTMLParser(t, resp.Body) - return doc.GetCSRF() + csrf := doc.GetCSRF() + require.NotEmpty(t, csrf) + return csrf } // GetCSRFFrom returns CSRF token from body From 74cf5a8f90a5a40a2e6be2861a00d10162eeb2e5 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 25 Sep 2024 15:18:26 +0800 Subject: [PATCH 2/8] test: fix TestCreateAnonymousAttachment --- tests/integration/attachment_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go index 40969d26f2a22..891f2ea6e2db2 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -41,7 +41,7 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri err = writer.Close() assert.NoError(t, err) - csrf := GetCSRF(t, session, repoURL) + csrf := GetCSRF(t, session, "/user/login") req := NewRequestWithBody(t, "POST", repoURL+"/issues/attachments", body) req.Header.Add("X-Csrf-Token", csrf) @@ -59,8 +59,7 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri func TestCreateAnonymousAttachment(t *testing.T) { defer tests.PrepareTestEnv(t)() session := emptyTestSession(t) - // this test is not right because it just doesn't pass the CSRF validation - createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusBadRequest) + createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusSeeOther) } func TestCreateIssueAttachment(t *testing.T) { From 3b07e1f0af195eb6dea68ac1166341c34cf53e27 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 25 Sep 2024 16:10:56 +0800 Subject: [PATCH 3/8] test: drop csrf token for GET requests --- tests/integration/org_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/integration/org_test.go b/tests/integration/org_test.go index 94c4e197271c5..ef4ef2bb9b429 100644 --- a/tests/integration/org_test.go +++ b/tests/integration/org_test.go @@ -204,9 +204,7 @@ func TestTeamSearch(t *testing.T) { var results TeamSearchResults session := loginUser(t, user.Name) - csrf := GetCSRF(t, session, "/"+org.Name) req := NewRequestf(t, "GET", "/org/%s/teams/-/search?q=%s", org.Name, "_team") - req.Header.Add("X-Csrf-Token", csrf) resp := session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &results) assert.NotEmpty(t, results.Data) @@ -217,8 +215,6 @@ func TestTeamSearch(t *testing.T) { // no access if not organization member user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) session = loginUser(t, user5.Name) - csrf = GetCSRF(t, session, "/"+org.Name) req = NewRequestf(t, "GET", "/org/%s/teams/-/search?q=%s", org.Name, "team") - req.Header.Add("X-Csrf-Token", csrf) session.MakeRequest(t, req, http.StatusNotFound) } From 10b2d633c5b67fbbb81661fdc7362d7e7bb13e3d Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 25 Sep 2024 16:41:49 +0800 Subject: [PATCH 4/8] chore: use /explore/repos --- tests/integration/attachment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go index 891f2ea6e2db2..54aa9ccc58e57 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -41,7 +41,7 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri err = writer.Close() assert.NoError(t, err) - csrf := GetCSRF(t, session, "/user/login") + csrf := GetCSRF(t, session, "/explore/repos") req := NewRequestWithBody(t, "POST", repoURL+"/issues/attachments", body) req.Header.Add("X-Csrf-Token", csrf) From 9338b41b46e78214e4c6b123ace2b6676582a606 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 25 Sep 2024 17:07:38 +0800 Subject: [PATCH 5/8] Revert "chore: use /explore/repos" This reverts commit 10b2d633c5b67fbbb81661fdc7362d7e7bb13e3d. --- tests/integration/attachment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go index 54aa9ccc58e57..891f2ea6e2db2 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -41,7 +41,7 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri err = writer.Close() assert.NoError(t, err) - csrf := GetCSRF(t, session, "/explore/repos") + csrf := GetCSRF(t, session, "/user/login") req := NewRequestWithBody(t, "POST", repoURL+"/issues/attachments", body) req.Header.Add("X-Csrf-Token", csrf) From 60b42594b8e226ef5f75ab07c98b0710a10c4417 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 25 Sep 2024 17:14:46 +0800 Subject: [PATCH 6/8] chore: use different urls --- tests/integration/attachment_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go index 891f2ea6e2db2..276323261e066 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -41,7 +41,15 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri err = writer.Close() assert.NoError(t, err) - csrf := GetCSRF(t, session, "/user/login") + var csrf string + // FIXME: It's quite hacky to determine if it's a logged in user or not and use the right URL to get the CSRF token + if expectedStatus == http.StatusSeeOther { + // the session is not logged in + csrf = GetCSRF(t, session, "/user/login") + } else { + // the session is logged in + csrf = GetCSRF(t, session, repoURL) + } req := NewRequestWithBody(t, "POST", repoURL+"/issues/attachments", body) req.Header.Add("X-Csrf-Token", csrf) From 58bc78645f62edb8ed7f7ba14d707b98d56ddf5b Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 25 Sep 2024 17:56:29 +0800 Subject: [PATCH 7/8] chore: improve createAttachment --- tests/integration/attachment_test.go | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go index 276323261e066..d856a5c6a243b 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -29,7 +29,7 @@ func generateImg() bytes.Buffer { return buff } -func createAttachment(t *testing.T, session *TestSession, repoURL, filename string, buff bytes.Buffer, expectedStatus int) string { +func createAttachment(t *testing.T, session *TestSession, csrf string, repoURL, filename string, buff bytes.Buffer, expectedStatus int) string { body := &bytes.Buffer{} // Setup multi-part @@ -41,16 +41,6 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri err = writer.Close() assert.NoError(t, err) - var csrf string - // FIXME: It's quite hacky to determine if it's a logged in user or not and use the right URL to get the CSRF token - if expectedStatus == http.StatusSeeOther { - // the session is not logged in - csrf = GetCSRF(t, session, "/user/login") - } else { - // the session is logged in - csrf = GetCSRF(t, session, repoURL) - } - req := NewRequestWithBody(t, "POST", repoURL+"/issues/attachments", body) req.Header.Add("X-Csrf-Token", csrf) req.Header.Add("Content-Type", writer.FormDataContentType()) @@ -67,14 +57,14 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri func TestCreateAnonymousAttachment(t *testing.T) { defer tests.PrepareTestEnv(t)() session := emptyTestSession(t) - createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusSeeOther) + createAttachment(t, session, GetCSRF(t, session, "/user/login"), "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, repoURL, "image.png", generateImg(), http.StatusOK) + uuid := createAttachment(t, session, GetCSRF(t, session, repoURL), repoURL, "image.png", generateImg(), http.StatusOK) req := NewRequest(t, "GET", repoURL+"/issues/new") resp := session.MakeRequest(t, req, http.StatusOK) From 4c4233d11a2342c0ce714995e8905ad9f13d306f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 25 Sep 2024 18:01:53 +0800 Subject: [PATCH 8/8] fix lint --- tests/integration/attachment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go index d856a5c6a243b..11aa03bb7e715 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -29,7 +29,7 @@ func generateImg() bytes.Buffer { return buff } -func createAttachment(t *testing.T, session *TestSession, csrf string, repoURL, filename string, buff bytes.Buffer, expectedStatus int) string { +func createAttachment(t *testing.T, session *TestSession, csrf, repoURL, filename string, buff bytes.Buffer, expectedStatus int) string { body := &bytes.Buffer{} // Setup multi-part