Skip to content

Commit 0fe5e2b

Browse files
authored
Allow maintainers to view and edit files of private repos when "Allow maintainers to edit" is enabled (#32215)
Fix #31539
1 parent aebb741 commit 0fe5e2b

File tree

4 files changed

+90
-3
lines changed

4 files changed

+90
-3
lines changed

routers/web/repo/pull.go

-2
Original file line numberDiff line numberDiff line change
@@ -887,8 +887,6 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
887887
}
888888

889889
if pull.HeadRepo != nil {
890-
ctx.Data["SourcePath"] = pull.HeadRepo.Link() + "/src/commit/" + endCommitID
891-
892890
if !pull.HasMerged && ctx.Doer != nil {
893891
perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer)
894892
if err != nil {

services/context/permission.go

+3
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ func RequireRepoWriterOr(unitTypes ...unit.Type) func(ctx *Context) {
5858
func RequireRepoReader(unitType unit.Type) func(ctx *Context) {
5959
return func(ctx *Context) {
6060
if !ctx.Repo.CanRead(unitType) {
61+
if unitType == unit.TypeCode && canWriteAsMaintainer(ctx) {
62+
return
63+
}
6164
if log.IsTrace() {
6265
if ctx.IsSigned {
6366
log.Trace("Permission Denied: User %-v cannot read %-v in Repo %-v\n"+

services/context/repo.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) {
374374
return
375375
}
376376

377-
if !ctx.Repo.Permission.HasAnyUnitAccessOrEveryoneAccess() {
377+
if !ctx.Repo.Permission.HasAnyUnitAccessOrEveryoneAccess() && !canWriteAsMaintainer(ctx) {
378378
if ctx.FormString("go-get") == "1" {
379379
EarlyResponseForGoGetMeta(ctx)
380380
return
@@ -1058,3 +1058,11 @@ func GitHookService() func(ctx *Context) {
10581058
}
10591059
}
10601060
}
1061+
1062+
// canWriteAsMaintainer check if the doer can write to a branch as a maintainer
1063+
func canWriteAsMaintainer(ctx *Context) bool {
1064+
branchName := getRefNameFromPath(ctx.Repo, ctx.PathParam("*"), func(branchName string) bool {
1065+
return issues_model.CanMaintainerWriteToBranch(ctx, ctx.Repo.Permission, branchName, ctx.Doer)
1066+
})
1067+
return len(branchName) > 0
1068+
}

tests/integration/pull_compare_test.go

+78
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
repo_model "code.gitea.io/gitea/models/repo"
1515
"code.gitea.io/gitea/models/unittest"
1616
user_model "code.gitea.io/gitea/models/user"
17+
"code.gitea.io/gitea/modules/test"
1718
repo_service "code.gitea.io/gitea/services/repository"
1819
"code.gitea.io/gitea/tests"
1920

@@ -73,3 +74,80 @@ func TestPullCompare(t *testing.T) {
7374
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")
7475
})
7576
}
77+
78+
func TestPullCompare_EnableAllowEditsFromMaintainer(t *testing.T) {
79+
onGiteaRun(t, func(t *testing.T, u *url.URL) {
80+
// repo3 is private
81+
repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
82+
assert.True(t, repo3.IsPrivate)
83+
84+
// user4 forks repo3
85+
user4Session := loginUser(t, "user4")
86+
forkedRepoName := "user4-forked-repo3"
87+
testRepoFork(t, user4Session, repo3.OwnerName, repo3.Name, "user4", forkedRepoName, "")
88+
forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user4", Name: forkedRepoName})
89+
assert.True(t, forkedRepo.IsPrivate)
90+
91+
// user4 creates a new branch and a PR
92+
testEditFileToNewBranch(t, user4Session, "user4", forkedRepoName, "master", "user4/update-readme", "README.md", "Hello, World\n(Edited by user4)\n")
93+
resp := testPullCreateDirectly(t, user4Session, repo3.OwnerName, repo3.Name, "master", "user4", forkedRepoName, "user4/update-readme", "PR for user4 forked repo3")
94+
prURL := test.RedirectURL(resp)
95+
96+
// user2 (admin of repo3) goes to the PR files page
97+
user2Session := loginUser(t, "user2")
98+
resp = user2Session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("%s/files", prURL)), http.StatusOK)
99+
htmlDoc := NewHTMLParser(t, resp.Body)
100+
nodes := htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .dropdown .menu a")
101+
if assert.Equal(t, 1, nodes.Length()) {
102+
// there is only "View File" button, no "Edit File" button
103+
assert.Equal(t, "View File", nodes.First().Text())
104+
viewFileLink, exists := nodes.First().Attr("href")
105+
if assert.True(t, exists) {
106+
user2Session.MakeRequest(t, NewRequest(t, "GET", viewFileLink), http.StatusOK)
107+
}
108+
}
109+
110+
// user4 goes to the PR page and enable "Allow maintainers to edit"
111+
resp = user4Session.MakeRequest(t, NewRequest(t, "GET", prURL), http.StatusOK)
112+
htmlDoc = NewHTMLParser(t, resp.Body)
113+
dataURL, exists := htmlDoc.doc.Find("#allow-edits-from-maintainers").Attr("data-url")
114+
assert.True(t, exists)
115+
req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/set_allow_maintainer_edit", dataURL), map[string]string{
116+
"_csrf": htmlDoc.GetCSRF(),
117+
"allow_maintainer_edit": "true",
118+
})
119+
user4Session.MakeRequest(t, req, http.StatusOK)
120+
121+
// user2 (admin of repo3) goes to the PR files page again
122+
resp = user2Session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("%s/files", prURL)), http.StatusOK)
123+
htmlDoc = NewHTMLParser(t, resp.Body)
124+
nodes = htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .dropdown .menu a")
125+
if assert.Equal(t, 2, nodes.Length()) {
126+
// there are "View File" button and "Edit File" button
127+
assert.Equal(t, "View File", nodes.First().Text())
128+
viewFileLink, exists := nodes.First().Attr("href")
129+
if assert.True(t, exists) {
130+
user2Session.MakeRequest(t, NewRequest(t, "GET", viewFileLink), http.StatusOK)
131+
}
132+
133+
assert.Equal(t, "Edit File", nodes.Last().Text())
134+
editFileLink, exists := nodes.Last().Attr("href")
135+
if assert.True(t, exists) {
136+
// edit the file
137+
resp := user2Session.MakeRequest(t, NewRequest(t, "GET", editFileLink), http.StatusOK)
138+
htmlDoc := NewHTMLParser(t, resp.Body)
139+
lastCommit := htmlDoc.GetInputValueByName("last_commit")
140+
assert.NotEmpty(t, lastCommit)
141+
req := NewRequestWithValues(t, "POST", editFileLink, map[string]string{
142+
"_csrf": htmlDoc.GetCSRF(),
143+
"last_commit": lastCommit,
144+
"tree_path": "README.md",
145+
"content": "File is edited by the maintainer user2",
146+
"commit_summary": "user2 updated the file",
147+
"commit_choice": "direct",
148+
})
149+
user2Session.MakeRequest(t, req, http.StatusSeeOther)
150+
}
151+
}
152+
})
153+
}

0 commit comments

Comments
 (0)