Allow maintainers to view and edit files of private repos when "Allow maintainers...

Fix #31539

diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go
index 02d9b429b557b98288dd7f9eb695c58a62795a06..0efe1be76a310fdcc0a3ca478009d4b887a6fdcb 100644 (file)
--- 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.HeadRepo.Link() + "/src/commit/" + endCommitID
-
                        if !pull.HasMerged && ctx.Doer != nil {
                                perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer)
                                if err != nil {

diff --git a/services/context/permission.go b/services/context/permission.go
index 14a9801dccba09fe1f4d948fa4e37aa1757a6fd1..9338587257cdce725bf5f51cd879bb1632fd260e 100644 (file)
--- 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 0072b63b7c99215fda54fccc2df1274184563282..2df2b7ea403875bd9a5b7285433fb6be36cf7a47 100644 (file)
--- 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 !ctx.Repo.Permission.HasAnyUnitAccessOrEveryoneAccess() && !canWriteAsMaintainer(ctx) {
                if ctx.FormString("go-get") == "1" {
                        EarlyResponseForGoGetMeta(ctx)
                        return
@@ -1058,3 +1058,11 @@ func GitHookService() func(ctx *Context) {
                }
        }
 }
+
+// canWriteAsMaintainer check if the doer can write to a branch as a maintainer
+func canWriteAsMaintainer(ctx *Context) bool {
+       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
+}

diff --git a/tests/integration/pull_compare_test.go b/tests/integration/pull_compare_test.go
index aed699fd20018fd27f516bb5b42ede4cbabec1f2..def6506253f904948c08b90f7a9c22de65c3086f 100644 (file)
--- 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)
+                       }
+               }
+       })
+}