From 6a9a833196498140588d978eb74dcce3d91128ed Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Tue, 8 Oct 2024 18:04:12 +0800 Subject: [PATCH 1/5] check if the doer is a maintainer --- routers/web/repo/pull.go | 2 +- services/context/permission.go | 3 +++ services/context/repo.go | 10 +++++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index ced0bbc15a00e..17a80c9135953 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -887,7 +887,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } if pull.HeadRepo != nil { - ctx.Data["SourcePath"] = pull.HeadRepo.Link() + "/src/commit/" + endCommitID + ctx.Data["SourcePath"] = pull.BaseRepo.Link() + "/src/commit/" + endCommitID if !pull.HasMerged && ctx.Doer != nil { perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer) diff --git a/services/context/permission.go b/services/context/permission.go index 14a9801dccba0..9338587257cdc 100644 --- a/services/context/permission.go +++ b/services/context/permission.go @@ -58,6 +58,9 @@ func RequireRepoWriterOr(unitTypes ...unit.Type) func(ctx *Context) { func RequireRepoReader(unitType unit.Type) func(ctx *Context) { return func(ctx *Context) { if !ctx.Repo.CanRead(unitType) { + if unitType == unit.TypeCode && canWriteAsMaintainer(ctx) { + return + } if log.IsTrace() { if ctx.IsSigned { log.Trace("Permission Denied: User %-v cannot read %-v in Repo %-v\n"+ diff --git a/services/context/repo.go b/services/context/repo.go index e0d3a0bfd3e43..7b7f14f1e1874 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -374,7 +374,7 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) { return } - if !ctx.Repo.Permission.HasAnyUnitAccessOrEveryoneAccess() { + if !canWriteAsMaintainer(ctx) && !ctx.Repo.Permission.HasAnyUnitAccessOrEveryoneAccess() { if ctx.FormString("go-get") == "1" { EarlyResponseForGoGetMeta(ctx) return @@ -1048,3 +1048,11 @@ func GitHookService() func(ctx *Context) { } } } + +// canWriteAsMaintainer check if the doer can write to a branch as a maintainer +func canWriteAsMaintainer(ctx *Context) bool { + // There is no need to check if the branch exists. + // If the branch does not exist, CanMaintainerWriteToBranch will return false. + branchName := getRefNameFromPath(ctx.Repo, ctx.PathParam("*"), func(_ string) bool { return true }) + return issues_model.CanMaintainerWriteToBranch(ctx, ctx.Repo.Permission, branchName, ctx.Doer) +} From bcb78846202ab38fbc897eab3d160e2a2450039c Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Tue, 8 Oct 2024 18:11:09 +0800 Subject: [PATCH 2/5] fix SourcePath --- routers/web/repo/pull.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 17a80c9135953..1bd31726ed35c 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -887,8 +887,6 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } if pull.HeadRepo != nil { - ctx.Data["SourcePath"] = pull.BaseRepo.Link() + "/src/commit/" + endCommitID - if !pull.HasMerged && ctx.Doer != nil { perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer) if err != nil { From 0e83e0454baba604c9640ec7db4ac9f3124dfade Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Wed, 9 Oct 2024 09:30:33 +0800 Subject: [PATCH 3/5] fix canWriteAsMaintainer --- services/context/repo.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/context/repo.go b/services/context/repo.go index 7b7f14f1e1874..b18e2688b10f8 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -1051,8 +1051,8 @@ func GitHookService() func(ctx *Context) { // canWriteAsMaintainer check if the doer can write to a branch as a maintainer func canWriteAsMaintainer(ctx *Context) bool { - // There is no need to check if the branch exists. - // If the branch does not exist, CanMaintainerWriteToBranch will return false. - branchName := getRefNameFromPath(ctx.Repo, ctx.PathParam("*"), func(_ string) bool { return true }) - return issues_model.CanMaintainerWriteToBranch(ctx, ctx.Repo.Permission, branchName, ctx.Doer) + branchName := getRefNameFromPath(ctx.Repo, ctx.PathParam("*"), func(branchName string) bool { + return issues_model.CanMaintainerWriteToBranch(ctx, ctx.Repo.Permission, branchName, ctx.Doer) + }) + return len(branchName) > 0 } From e58189e8202ae5edd33c8d40aa75bb2a1c18d678 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Wed, 9 Oct 2024 12:33:50 +0800 Subject: [PATCH 4/5] fix --- services/context/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/context/repo.go b/services/context/repo.go index b18e2688b10f8..a578fc46b5813 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -374,7 +374,7 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) { return } - if !canWriteAsMaintainer(ctx) && !ctx.Repo.Permission.HasAnyUnitAccessOrEveryoneAccess() { + if !ctx.Repo.Permission.HasAnyUnitAccessOrEveryoneAccess() && !canWriteAsMaintainer(ctx) { if ctx.FormString("go-get") == "1" { EarlyResponseForGoGetMeta(ctx) return From b02fffed0a3f1c502401c6f1153735cb9a72e596 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Thu, 10 Oct 2024 15:09:13 +0800 Subject: [PATCH 5/5] add TestPullCompare_EnableAllowEditsFromMaintainer --- tests/integration/pull_compare_test.go | 78 ++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/tests/integration/pull_compare_test.go b/tests/integration/pull_compare_test.go index aed699fd20018..def6506253f90 100644 --- a/tests/integration/pull_compare_test.go +++ b/tests/integration/pull_compare_test.go @@ -14,6 +14,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/test" repo_service "code.gitea.io/gitea/services/repository" "code.gitea.io/gitea/tests" @@ -73,3 +74,80 @@ func TestPullCompare(t *testing.T) { assert.EqualValues(t, editButtonCount, 0, "Expected not to find a button to edit a file in the PR diff view because head repository has been deleted") }) } + +func TestPullCompare_EnableAllowEditsFromMaintainer(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + // repo3 is private + repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + assert.True(t, repo3.IsPrivate) + + // user4 forks repo3 + user4Session := loginUser(t, "user4") + forkedRepoName := "user4-forked-repo3" + testRepoFork(t, user4Session, repo3.OwnerName, repo3.Name, "user4", forkedRepoName, "") + forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user4", Name: forkedRepoName}) + assert.True(t, forkedRepo.IsPrivate) + + // user4 creates a new branch and a PR + testEditFileToNewBranch(t, user4Session, "user4", forkedRepoName, "master", "user4/update-readme", "README.md", "Hello, World\n(Edited by user4)\n") + resp := testPullCreateDirectly(t, user4Session, repo3.OwnerName, repo3.Name, "master", "user4", forkedRepoName, "user4/update-readme", "PR for user4 forked repo3") + prURL := test.RedirectURL(resp) + + // user2 (admin of repo3) goes to the PR files page + user2Session := loginUser(t, "user2") + resp = user2Session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("%s/files", prURL)), http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + nodes := htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .dropdown .menu a") + if assert.Equal(t, 1, nodes.Length()) { + // there is only "View File" button, no "Edit File" button + assert.Equal(t, "View File", nodes.First().Text()) + viewFileLink, exists := nodes.First().Attr("href") + if assert.True(t, exists) { + user2Session.MakeRequest(t, NewRequest(t, "GET", viewFileLink), http.StatusOK) + } + } + + // user4 goes to the PR page and enable "Allow maintainers to edit" + resp = user4Session.MakeRequest(t, NewRequest(t, "GET", prURL), http.StatusOK) + htmlDoc = NewHTMLParser(t, resp.Body) + dataURL, exists := htmlDoc.doc.Find("#allow-edits-from-maintainers").Attr("data-url") + assert.True(t, exists) + req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/set_allow_maintainer_edit", dataURL), map[string]string{ + "_csrf": htmlDoc.GetCSRF(), + "allow_maintainer_edit": "true", + }) + user4Session.MakeRequest(t, req, http.StatusOK) + + // user2 (admin of repo3) goes to the PR files page again + resp = user2Session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("%s/files", prURL)), http.StatusOK) + htmlDoc = NewHTMLParser(t, resp.Body) + nodes = htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .dropdown .menu a") + if assert.Equal(t, 2, nodes.Length()) { + // there are "View File" button and "Edit File" button + assert.Equal(t, "View File", nodes.First().Text()) + viewFileLink, exists := nodes.First().Attr("href") + if assert.True(t, exists) { + user2Session.MakeRequest(t, NewRequest(t, "GET", viewFileLink), http.StatusOK) + } + + assert.Equal(t, "Edit File", nodes.Last().Text()) + editFileLink, exists := nodes.Last().Attr("href") + if assert.True(t, exists) { + // edit the file + resp := user2Session.MakeRequest(t, NewRequest(t, "GET", editFileLink), http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + lastCommit := htmlDoc.GetInputValueByName("last_commit") + assert.NotEmpty(t, lastCommit) + req := NewRequestWithValues(t, "POST", editFileLink, map[string]string{ + "_csrf": htmlDoc.GetCSRF(), + "last_commit": lastCommit, + "tree_path": "README.md", + "content": "File is edited by the maintainer user2", + "commit_summary": "user2 updated the file", + "commit_choice": "direct", + }) + user2Session.MakeRequest(t, req, http.StatusSeeOther) + } + } + }) +}