From 9e054fbb592b8c01b12efd8aab9106d4a61f1be9 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Sat, 28 Dec 2024 13:10:40 +0100 Subject: [PATCH 1/4] add pull request test for blocked user --- tests/integration/pull_create_test.go | 77 +++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 4bc2a1da9ab5f..8e7e7d3f18917 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -12,7 +12,9 @@ import ( "strings" "testing" + auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/modules/git" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" @@ -83,6 +85,30 @@ func testPullCreateDirectly(t *testing.T, session *TestSession, baseRepoOwner, b return resp } +func testPullCreateFailure(t *testing.T, session *TestSession, baseRepoOwner, baseRepoName, baseBranch, headRepoOwner, headRepoName, headBranch, title string) *httptest.ResponseRecorder { + headCompare := headBranch + if headRepoOwner != "" { + if headRepoName != "" { + headCompare = fmt.Sprintf("%s/%s:%s", headRepoOwner, headRepoName, headBranch) + } else { + headCompare = fmt.Sprintf("%s:%s", headRepoOwner, headBranch) + } + } + req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/compare/%s...%s", baseRepoOwner, baseRepoName, baseBranch, headCompare)) + resp := session.MakeRequest(t, req, http.StatusOK) + + // Submit the form for creating the pull + htmlDoc := NewHTMLParser(t, resp.Body) + link, exists := htmlDoc.doc.Find("form.ui.form").Attr("action") + assert.True(t, exists, "The template has changed") + req = NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": htmlDoc.GetCSRF(), + "title": title, + }) + resp = session.MakeRequest(t, req, http.StatusInternalServerError) + return resp +} + func TestPullCreate(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user1") @@ -245,3 +271,54 @@ func TestCreateAgitPullWithReadPermission(t *testing.T) { }) }) } + +/* +Setup: user2 has repository, user1 forks it +--- + +1. User2 blocks User1 +2. User1 adds changes to fork +3. User1 attempts to create a pull request +4. User1 sees alert that the action is not allowed because of the block +*/ +func TestCreatePullWhenBlocked(t *testing.T) { + RepoOwner := "user2" + ForkOwner := "user1" + onGiteaRun(t, func(t *testing.T, u *url.URL) { + // Setup + // User1 forks repo1 from User2 + sessionFork := loginUser(t, ForkOwner) + testRepoFork(t, sessionFork, RepoOwner, "repo1", ForkOwner, "repo1", "") + + // 1. User2 blocks user1 + // sessionBase := loginUser(t, "user2") + token := getUserToken(t, RepoOwner, auth_model.AccessTokenScopeWriteUser) + + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/user/blocks/%s", ForkOwner)). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) + req = NewRequest(t, "PUT", fmt.Sprintf("/api/v1/user/blocks/%s", ForkOwner)). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNoContent) + req = NewRequest(t, "GET", "/api/v1/user/blocks"). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + + var users []api.User + DecodeJSON(t, resp, &users) + + assert.Len(t, users, 1) + + // 2. User1 adds changes to fork + testEditFile(t, sessionFork, ForkOwner, "repo1", "master", "README.md", "Hello, World (Edited)\n") + + // 3. User1 attempts to create a pull request + testPullCreateFailure(t, sessionFork, RepoOwner, "repo1", "master", ForkOwner, "repo1", "master", "This is a pull title") + + // Teardown + // Unblock user + req = NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/user/blocks/%s", RepoOwner)). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNoContent) + }) +} From a92bfb99a5aaf7b9ce953175b64c971bcc7034fc Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Sat, 28 Dec 2024 17:17:29 +0100 Subject: [PATCH 2/4] find user combination that works --- tests/integration/pull_create_test.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 8e7e7d3f18917..3da9639b1e4c6 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -14,7 +14,6 @@ import ( auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/modules/git" - api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" @@ -283,12 +282,12 @@ Setup: user2 has repository, user1 forks it */ func TestCreatePullWhenBlocked(t *testing.T) { RepoOwner := "user2" - ForkOwner := "user1" + ForkOwner := "user16" onGiteaRun(t, func(t *testing.T, u *url.URL) { // Setup // User1 forks repo1 from User2 sessionFork := loginUser(t, ForkOwner) - testRepoFork(t, sessionFork, RepoOwner, "repo1", ForkOwner, "repo1", "") + testRepoFork(t, sessionFork, RepoOwner, "repo1", ForkOwner, "forkrepo1", "") // 1. User2 blocks user1 // sessionBase := loginUser(t, "user2") @@ -300,24 +299,16 @@ func TestCreatePullWhenBlocked(t *testing.T) { req = NewRequest(t, "PUT", fmt.Sprintf("/api/v1/user/blocks/%s", ForkOwner)). AddTokenAuth(token) MakeRequest(t, req, http.StatusNoContent) - req = NewRequest(t, "GET", "/api/v1/user/blocks"). - AddTokenAuth(token) - resp := MakeRequest(t, req, http.StatusOK) - - var users []api.User - DecodeJSON(t, resp, &users) - - assert.Len(t, users, 1) // 2. User1 adds changes to fork - testEditFile(t, sessionFork, ForkOwner, "repo1", "master", "README.md", "Hello, World (Edited)\n") + testEditFile(t, sessionFork, ForkOwner, "forkrepo1", "master", "README.md", "Hello, World (Edited)\n") // 3. User1 attempts to create a pull request - testPullCreateFailure(t, sessionFork, RepoOwner, "repo1", "master", ForkOwner, "repo1", "master", "This is a pull title") + testPullCreateFailure(t, sessionFork, RepoOwner, "repo1", "master", ForkOwner, "forkrepo1", "master", "This is a pull title") // Teardown // Unblock user - req = NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/user/blocks/%s", RepoOwner)). + req = NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/user/blocks/%s", ForkOwner)). AddTokenAuth(token) MakeRequest(t, req, http.StatusNoContent) }) From b7d696d6078e064cdf381c60e65946cb08789089 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Sat, 28 Dec 2024 17:23:05 +0100 Subject: [PATCH 3/4] fix template arguments --- routers/web/repo/pull.go | 2 ++ tests/integration/pull_create_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 9f3d1c1b7c032..0e4840fae30bf 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1356,6 +1356,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.push_rejected"), "Summary": ctx.Tr("repo.pulls.new.blocked_user"), + "Details": "", }) if err != nil { ctx.ServerError("CompareAndPullRequest.HTMLString", err) @@ -1366,6 +1367,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.push_rejected"), "Summary": ctx.Tr("repo.pulls.new.must_collaborator"), + "Details": "", }) if err != nil { ctx.ServerError("CompareAndPullRequest.HTMLString", err) diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 3da9639b1e4c6..162ea532c8075 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -104,7 +104,7 @@ func testPullCreateFailure(t *testing.T, session *TestSession, baseRepoOwner, ba "_csrf": htmlDoc.GetCSRF(), "title": title, }) - resp = session.MakeRequest(t, req, http.StatusInternalServerError) + resp = session.MakeRequest(t, req, http.StatusBadRequest) return resp } From f6a7b3e948205b2b43bc55f9fe98458b6eca88b1 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Sat, 28 Dec 2024 18:43:59 +0100 Subject: [PATCH 4/4] make template conditional on details --- routers/web/repo/pull.go | 2 -- templates/base/alert_details.tmpl | 10 +++++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 0e4840fae30bf..9f3d1c1b7c032 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1356,7 +1356,6 @@ func CompareAndPullRequestPost(ctx *context.Context) { flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.push_rejected"), "Summary": ctx.Tr("repo.pulls.new.blocked_user"), - "Details": "", }) if err != nil { ctx.ServerError("CompareAndPullRequest.HTMLString", err) @@ -1367,7 +1366,6 @@ func CompareAndPullRequestPost(ctx *context.Context) { flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.push_rejected"), "Summary": ctx.Tr("repo.pulls.new.must_collaborator"), - "Details": "", }) if err != nil { ctx.ServerError("CompareAndPullRequest.HTMLString", err) diff --git a/templates/base/alert_details.tmpl b/templates/base/alert_details.tmpl index 6801c8240fd7d..6d4c1fb2db220 100644 --- a/templates/base/alert_details.tmpl +++ b/templates/base/alert_details.tmpl @@ -1,7 +1,11 @@ {{.Message}} +{{if .Details}}
{{.Summary}} - - {{.Details | SanitizeHTML}} - + {{.Details | SanitizeHTML}}
+{{else}} +
+ {{.Summary}} +
+{{end}}