From 18c47f9f822aac7619a7f99aefc3553bbbdf4232 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 29 Apr 2024 17:31:37 +0800 Subject: [PATCH] Fix permission check of maintainer --- models/issues/pull_list.go | 27 ++++++++++++++------------- routers/private/hook_pre_receive.go | 2 +- routers/web/repo/pull.go | 2 +- services/context/repo.go | 2 +- services/convert/convert.go | 2 +- services/pull/review.go | 6 +++++- 6 files changed, 23 insertions(+), 18 deletions(-) diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go index a1d46f8cd4..b9423453ee 100644 --- a/models/issues/pull_list.go +++ b/models/issues/pull_list.go @@ -50,7 +50,7 @@ func listPullRequestStatement(ctx context.Context, baseRepoID int64, opts *PullR } // GetUnmergedPullRequestsByHeadInfo returns all pull requests that are open and has not been merged -func GetUnmergedPullRequestsByHeadInfo(ctx context.Context, repoID int64, branch string) ([]*PullRequest, error) { +func GetUnmergedPullRequestsByHeadInfo(ctx context.Context, repoID int64, branch string) (PullRequestList, error) { prs := make([]*PullRequest, 0, 2) sess := db.GetEngine(ctx). Join("INNER", "issue", "issue.id = pull_request.issue_id"). @@ -58,29 +58,30 @@ func GetUnmergedPullRequestsByHeadInfo(ctx context.Context, repoID int64, branch return prs, sess.Find(&prs) } -// CanMaintainerWriteToBranch check whether user is a maintainer and could write to the branch -func CanMaintainerWriteToBranch(ctx context.Context, p access_model.Permission, branch string, user *user_model.User) bool { +func CanUserWriteToBranch(ctx context.Context, p access_model.Permission, headRepoID int64, branch string, user *user_model.User) bool { if p.CanWrite(unit.TypeCode) { return true } - // the code below depends on units to get the repository ID, not ideal but just keep it for now - firstUnitRepoID := p.GetFirstUnitRepoID() - if firstUnitRepoID == 0 { - return false - } + return canMaintainerWriteToHeadBranch(ctx, p, headRepoID, branch, user) +} - prs, err := GetUnmergedPullRequestsByHeadInfo(ctx, firstUnitRepoID, branch) +// canMaintainerWriteToHeadBranch check whether user is a maintainer and could write to the branch +func canMaintainerWriteToHeadBranch(ctx context.Context, p access_model.Permission, headRepoID int64, branch string, user *user_model.User) bool { + prs, err := GetUnmergedPullRequestsByHeadInfo(ctx, headRepoID, branch) if err != nil { + log.Error("GetUnmergedPullRequestsByHeadInfo: %v", err) return false } + if err := prs.LoadRepositories(ctx); err != nil { + log.Error("LoadBaseRepos: %v", err) + return false + } + + // user can write to the branch once one pull request allowed the user edit the branch for _, pr := range prs { if pr.AllowMaintainerEdit { - err = pr.LoadBaseRepo(ctx) - if err != nil { - continue - } prPerm, err := access_model.GetUserRepoPermission(ctx, pr.BaseRepo, user) if err != nil { continue diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 0a3c8e2559..04ad85265f 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -55,7 +55,7 @@ func (ctx *preReceiveContext) CanWriteCode() bool { if !ctx.loadPusherAndPermission() { return false } - ctx.canWriteCode = issues_model.CanMaintainerWriteToBranch(ctx, ctx.userPerm, ctx.branchName, ctx.user) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite + ctx.canWriteCode = issues_model.CanUserWriteToBranch(ctx, ctx.userPerm, ctx.Repo.Repository.ID, ctx.branchName, ctx.user) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite ctx.checkedCanWriteCode = true } return ctx.canWriteCode diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 92e0a1674e..21f99c74d5 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -871,7 +871,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi return } - if perm.CanWrite(unit.TypeCode) || issues_model.CanMaintainerWriteToBranch(ctx, perm, pull.HeadBranch, ctx.Doer) { + if issues_model.CanUserWriteToBranch(ctx, perm, pull.HeadRepoID, pull.HeadBranch, ctx.Doer) { ctx.Data["CanEditFile"] = true ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.edit_this_file") ctx.Data["HeadRepoLink"] = pull.HeadRepo.Link() diff --git a/services/context/repo.go b/services/context/repo.go index d9a3feaf27..29a602fd6f 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -70,7 +70,7 @@ type Repository struct { // CanWriteToBranch checks if the branch is writable by the user func (r *Repository) CanWriteToBranch(ctx context.Context, user *user_model.User, branch string) bool { - return issues_model.CanMaintainerWriteToBranch(ctx, r.Permission, branch, user) + return issues_model.CanUserWriteToBranch(ctx, r.Permission, r.Repository.ID, branch, user) } // CanEnableEditor returns true if repository is editable and user has proper access level. diff --git a/services/convert/convert.go b/services/convert/convert.go index c44179632e..b7b1d9b2d7 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -67,7 +67,7 @@ func ToBranch(ctx context.Context, repo *repo_model.Repository, branchName strin if err != nil { return nil, err } - canPush = issues_model.CanMaintainerWriteToBranch(ctx, perms, branchName, user) + canPush = issues_model.CanUserWriteToBranch(ctx, perms, repo.ID, branchName, user) } return &api.Branch{ diff --git a/services/pull/review.go b/services/pull/review.go index 3d5eca779f..8ba80cc009 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -16,6 +16,7 @@ import ( issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" @@ -71,7 +72,10 @@ func InvalidateCodeComments(ctx context.Context, prs issues_model.PullRequestLis if len(prs) == 0 { return nil } - issueIDs := prs.GetIssueIDs() + + issueIDs := container.FilterSlice(prs, func(pr *issues_model.PullRequest) (int64, bool) { + return pr.IssueID, true + }) codeComments, err := db.Find[issues_model.Comment](ctx, issues_model.FindCommentsOptions{ ListOptions: db.ListOptionsAll,