Skip to content

Commit

Permalink
Allow maintainers to view and edit files of private repos when "Allow…
Browse files Browse the repository at this point in the history
… maintainers to edit" is enabled (#32215)

Fix #31539
  • Loading branch information
Zettat123 authored Oct 11, 2024
1 parent aebb741 commit 0fe5e2b
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 3 deletions.
2 changes: 0 additions & 2 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions services/context/permission.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"+
Expand Down
10 changes: 9 additions & 1 deletion services/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
78 changes: 78 additions & 0 deletions tests/integration/pull_compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}
}
})
}

0 comments on commit 0fe5e2b

Please sign in to comment.