diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index ba8ca85bc..9e46791ec 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -61,6 +61,7 @@ type Version struct { // update minDBVersion accordingly var migrations = []Migration{ // Gitea 1.5.0 ends at v69 + // v70 -> v71 NewMigration("add issue_dependencies", addIssueDependencies), // v71 -> v72 @@ -380,6 +381,8 @@ var migrations = []Migration{ NewMigration("Create ForeignReference table", createForeignReferenceTable), // v212 -> v213 NewMigration("Add package tables", addPackageTables), + // v213 -> v214 + NewMigration("Add allow edits from maintainers to PullRequest table", addAllowMaintainerEdit), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v213.go b/models/migrations/v213.go new file mode 100644 index 000000000..b1dbf98d1 --- /dev/null +++ b/models/migrations/v213.go @@ -0,0 +1,18 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "xorm.io/xorm" +) + +func addAllowMaintainerEdit(x *xorm.Engine) error { + // PullRequest represents relation between pull request and repositories. + type PullRequest struct { + AllowMaintainerEdit bool `xorm:"NOT NULL DEFAULT false"` + } + + return x.Sync2(new(PullRequest)) +} diff --git a/models/pull.go b/models/pull.go index e04263af9..437b202d3 100644 --- a/models/pull.go +++ b/models/pull.go @@ -69,15 +69,16 @@ type PullRequest struct { Issue *Issue `xorm:"-"` Index int64 - HeadRepoID int64 `xorm:"INDEX"` - HeadRepo *repo_model.Repository `xorm:"-"` - BaseRepoID int64 `xorm:"INDEX"` - BaseRepo *repo_model.Repository `xorm:"-"` - HeadBranch string - HeadCommitID string `xorm:"-"` - BaseBranch string - ProtectedBranch *ProtectedBranch `xorm:"-"` - MergeBase string `xorm:"VARCHAR(40)"` + HeadRepoID int64 `xorm:"INDEX"` + HeadRepo *repo_model.Repository `xorm:"-"` + BaseRepoID int64 `xorm:"INDEX"` + BaseRepo *repo_model.Repository `xorm:"-"` + HeadBranch string + HeadCommitID string `xorm:"-"` + BaseBranch string + ProtectedBranch *ProtectedBranch `xorm:"-"` + MergeBase string `xorm:"VARCHAR(40)"` + AllowMaintainerEdit bool `xorm:"NOT NULL DEFAULT false"` HasMerged bool `xorm:"INDEX"` MergedCommitID string `xorm:"VARCHAR(40)"` @@ -711,6 +712,14 @@ func (pr *PullRequest) GetHeadBranchHTMLURL() string { return pr.HeadRepo.HTMLURL() + "/src/branch/" + util.PathEscapeSegments(pr.HeadBranch) } +// UpdateAllowEdits update if PR can be edited from maintainers +func UpdateAllowEdits(ctx context.Context, pr *PullRequest) error { + if _, err := db.GetEngine(ctx).ID(pr.ID).Cols("allow_maintainer_edit").Update(pr); err != nil { + return err + } + return nil +} + // Mergeable returns if the pullrequest is mergeable. func (pr *PullRequest) Mergeable() bool { // If a pull request isn't mergable if it's: diff --git a/models/repo_permission.go b/models/repo_permission.go index b4291f236..bc31b873f 100644 --- a/models/repo_permission.go +++ b/models/repo_permission.go @@ -103,6 +103,39 @@ func (p *Permission) CanWriteIssuesOrPulls(isPull bool) bool { return p.CanWrite(unit.TypeIssues) } +// CanWriteToBranch checks if the branch is writable by the user +func (p *Permission) CanWriteToBranch(user *user_model.User, branch string) bool { + if p.CanWrite(unit.TypeCode) { + return true + } + + if len(p.Units) < 1 { + return false + } + + prs, err := GetUnmergedPullRequestsByHeadInfo(p.Units[0].RepoID, branch) + if err != nil { + return false + } + + for _, pr := range prs { + if pr.AllowMaintainerEdit { + err = pr.LoadBaseRepo() + if err != nil { + continue + } + prPerm, err := GetUserRepoPermission(db.DefaultContext, pr.BaseRepo, user) + if err != nil { + continue + } + if prPerm.CanWrite(unit.TypeCode) { + return true + } + } + } + return false +} + // ColorFormat writes a colored string for these Permissions func (p *Permission) ColorFormat(s fmt.State) { noColor := log.ColorBytes(log.Reset) @@ -160,6 +193,7 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use perm) }() } + // anonymous user visit private repo. // TODO: anonymous user visit public unit of private repo??? if user == nil && repo.IsPrivate { diff --git a/modules/context/permission.go b/modules/context/permission.go index 142b86fae..8dc3b3cd4 100644 --- a/modules/context/permission.go +++ b/modules/context/permission.go @@ -29,6 +29,16 @@ func RequireRepoWriter(unitType unit.Type) func(ctx *Context) { } } +// CanEnableEditor checks if the user is allowed to write to the branch of the repo +func CanEnableEditor() func(ctx *Context) { + return func(ctx *Context) { + if !ctx.Repo.Permission.CanWriteToBranch(ctx.Doer, ctx.Repo.BranchName) { + ctx.NotFound("CanWriteToBranch denies permission", nil) + return + } + } +} + // RequireRepoWriterOr returns a middleware for requiring repository write to one of the unit permission func RequireRepoWriterOr(unitTypes ...unit.Type) func(ctx *Context) { return func(ctx *Context) { diff --git a/modules/context/repo.go b/modules/context/repo.go index a02bb7e86..b2c9a21f8 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -78,8 +78,8 @@ type Repository struct { } // CanEnableEditor returns true if repository is editable and user has proper access level. -func (r *Repository) CanEnableEditor() bool { - return r.Permission.CanWrite(unit_model.TypeCode) && r.Repository.CanEnableEditor() && r.IsViewBranch && !r.Repository.IsArchived +func (r *Repository) CanEnableEditor(user *user_model.User) bool { + return r.IsViewBranch && r.Permission.CanWriteToBranch(user, r.BranchName) && r.Repository.CanEnableEditor() && !r.Repository.IsArchived } // CanCreateBranch returns true if repository is editable and user has proper access level. @@ -123,7 +123,7 @@ func (r *Repository) CanCommitToBranch(ctx context.Context, doer *user_model.Use sign, keyID, _, err := asymkey_service.SignCRUDAction(ctx, r.Repository.RepoPath(), doer, r.Repository.RepoPath(), git.BranchPrefix+r.BranchName) - canCommit := r.CanEnableEditor() && userCanPush + canCommit := r.CanEnableEditor(doer) && userCanPush if requireSigned { canCommit = canCommit && sign } @@ -139,7 +139,7 @@ func (r *Repository) CanCommitToBranch(ctx context.Context, doer *user_model.Use return CanCommitToBranchResults{ CanCommitToBranch: canCommit, - EditorEnabled: r.CanEnableEditor(), + EditorEnabled: r.CanEnableEditor(doer), UserCanPush: userCanPush, RequireSigned: requireSigned, WillSign: sign, diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 3f565f76e..bd06f4dbf 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -41,12 +41,19 @@ func ToEmail(email *user_model.EmailAddress) *api.Email { func ToBranch(repo *repo_model.Repository, b *git.Branch, c *git.Commit, bp *models.ProtectedBranch, user *user_model.User, isRepoAdmin bool) (*api.Branch, error) { if bp == nil { var hasPerm bool + var canPush bool var err error if user != nil { hasPerm, err = models.HasAccessUnit(user, repo, unit.TypeCode, perm.AccessModeWrite) if err != nil { return nil, err } + + perms, err := models.GetUserRepoPermission(db.DefaultContext, repo, user) + if err != nil { + return nil, err + } + canPush = perms.CanWriteToBranch(user, b.Name) } return &api.Branch{ @@ -56,7 +63,7 @@ func ToBranch(repo *repo_model.Repository, b *git.Branch, c *git.Commit, bp *mod RequiredApprovals: 0, EnableStatusCheck: false, StatusCheckContexts: []string{}, - UserCanPush: hasPerm, + UserCanPush: canPush, UserCanMerge: hasPerm, }, nil } diff --git a/modules/convert/pull.go b/modules/convert/pull.go index 6034327a9..a2f54270e 100644 --- a/modules/convert/pull.go +++ b/modules/convert/pull.go @@ -73,6 +73,8 @@ func ToAPIPullRequest(ctx context.Context, pr *models.PullRequest, doer *user_mo Created: pr.Issue.CreatedUnix.AsTimePtr(), Updated: pr.Issue.UpdatedUnix.AsTimePtr(), + AllowMaintainerEdit: pr.AllowMaintainerEdit, + Base: &api.PRBranchInfo{ Name: pr.BaseBranch, Ref: pr.BaseBranch, diff --git a/modules/structs/pull.go b/modules/structs/pull.go index 653091b2f..b63b3edfd 100644 --- a/modules/structs/pull.go +++ b/modules/structs/pull.go @@ -31,9 +31,10 @@ type PullRequest struct { Mergeable bool `json:"mergeable"` HasMerged bool `json:"merged"` // swagger:strfmt date-time - Merged *time.Time `json:"merged_at"` - MergedCommitID *string `json:"merge_commit_sha"` - MergedBy *User `json:"merged_by"` + Merged *time.Time `json:"merged_at"` + MergedCommitID *string `json:"merge_commit_sha"` + MergedBy *User `json:"merged_by"` + AllowMaintainerEdit bool `json:"allow_maintainer_edit"` Base *PRBranchInfo `json:"base"` Head *PRBranchInfo `json:"head"` @@ -90,6 +91,7 @@ type EditPullRequestOption struct { Labels []int64 `json:"labels"` State *string `json:"state"` // swagger:strfmt date-time - Deadline *time.Time `json:"due_date"` - RemoveDeadline *bool `json:"unset_due_date"` + Deadline *time.Time `json:"due_date"` + RemoveDeadline *bool `json:"unset_due_date"` + AllowMaintainerEdit *bool `json:"allow_maintainer_edit"` } diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index e2947bf7a..135e6484c 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -30,6 +30,11 @@ type CreateFileOptions struct { Content string `json:"content"` } +// Branch returns branch name +func (o *CreateFileOptions) Branch() string { + return o.FileOptions.BranchName +} + // DeleteFileOptions options for deleting files (used for other File structs below) // Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) type DeleteFileOptions struct { @@ -39,6 +44,11 @@ type DeleteFileOptions struct { SHA string `json:"sha" binding:"Required"` } +// Branch returns branch name +func (o *DeleteFileOptions) Branch() string { + return o.FileOptions.BranchName +} + // UpdateFileOptions options for updating files // Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) type UpdateFileOptions struct { @@ -50,6 +60,16 @@ type UpdateFileOptions struct { FromPath string `json:"from_path" binding:"MaxSize(500)"` } +// Branch returns branch name +func (o *UpdateFileOptions) Branch() string { + return o.FileOptions.BranchName +} + +// FileOptionInterface provides a unified interface for the different file options +type FileOptionInterface interface { + Branch() string +} + // ApplyDiffPatchFileOptions options for applying a diff patch // Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) type ApplyDiffPatchFileOptions struct { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 5ec001b6d..0175c8bfc 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1488,6 +1488,9 @@ pulls.desc = Enable pull requests and code reviews. pulls.new = New Pull Request pulls.view = View Pull Request pulls.compare_changes = New Pull Request +pulls.allow_edits_from_maintainers = Allow edits from maintainers +pulls.allow_edits_from_maintainers_desc = Users with write access to the base branch can also push to this branch +pulls.allow_edits_from_maintainers_err = Updating failed pulls.compare_changes_desc = Select the branch to merge into and the branch to pull from. pulls.compare_base = merge into pulls.compare_compare = pull from diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index cca0f37ba..782500e6c 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -283,6 +283,15 @@ func reqRepoWriter(unitTypes ...unit.Type) func(ctx *context.APIContext) { } } +// reqRepoBranchWriter user should have a permission to write to a branch, or be a site admin +func reqRepoBranchWriter(ctx *context.APIContext) { + options, ok := web.GetForm(ctx).(api.FileOptionInterface) + if !ok || (!ctx.Repo.CanWriteToBranch(ctx.Doer, options.Branch()) && !ctx.IsUserSiteAdmin()) { + ctx.Error(http.StatusForbidden, "reqRepoBranchWriter", "user should have a permission to write to this branch") + return + } +} + // reqRepoReader user should have specific read permission or be a repo admin or a site admin func reqRepoReader(unitType unit.Type) func(ctx *context.APIContext) { return func(ctx *context.APIContext) { @@ -1021,10 +1030,10 @@ func Routes() *web.Route { m.Get("", repo.GetContentsList) m.Get("/*", repo.GetContents) m.Group("/*", func() { - m.Post("", bind(api.CreateFileOptions{}), repo.CreateFile) - m.Put("", bind(api.UpdateFileOptions{}), repo.UpdateFile) - m.Delete("", bind(api.DeleteFileOptions{}), repo.DeleteFile) - }, reqRepoWriter(unit.TypeCode), reqToken()) + m.Post("", bind(api.CreateFileOptions{}), reqRepoBranchWriter, repo.CreateFile) + m.Put("", bind(api.UpdateFileOptions{}), reqRepoBranchWriter, repo.UpdateFile) + m.Delete("", bind(api.DeleteFileOptions{}), reqRepoBranchWriter, repo.DeleteFile) + }, reqToken()) }, reqRepoReader(unit.TypeCode)) m.Get("/signing-key.gpg", misc.SigningKey) m.Group("/topics", func() { diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index d907e770a..b8b72f95e 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -173,8 +173,10 @@ func GetEditorconfig(ctx *context.APIContext) { } // canWriteFiles returns true if repository is editable and user has proper access level. -func canWriteFiles(r *context.Repository) bool { - return r.Permission.CanWrite(unit.TypeCode) && !r.Repository.IsMirror && !r.Repository.IsArchived +func canWriteFiles(ctx *context.APIContext, branch string) bool { + return ctx.Repo.Permission.CanWriteToBranch(ctx.Doer, branch) && + !ctx.Repo.Repository.IsMirror && + !ctx.Repo.Repository.IsArchived } // canReadFiles returns true if repository is readable and user has proper access level. @@ -376,7 +378,7 @@ func handleCreateOrUpdateFileError(ctx *context.APIContext, err error) { // Called from both CreateFile or UpdateFile to handle both func createOrUpdateFile(ctx *context.APIContext, opts *files_service.UpdateRepoFileOptions) (*api.FileResponse, error) { - if !canWriteFiles(ctx.Repo) { + if !canWriteFiles(ctx, opts.OldBranch) { return nil, models.ErrUserDoesNotHaveAccessToRepo{ UserID: ctx.Doer.ID, RepoName: ctx.Repo.Repository.LowerName, @@ -433,7 +435,7 @@ func DeleteFile(ctx *context.APIContext) { // "$ref": "#/responses/error" apiOpts := web.GetForm(ctx).(*api.DeleteFileOptions) - if !canWriteFiles(ctx.Repo) { + if !canWriteFiles(ctx, apiOpts.BranchName) { ctx.Error(http.StatusForbidden, "DeleteFile", models.ErrUserDoesNotHaveAccessToRepo{ UserID: ctx.Doer.ID, RepoName: ctx.Repo.Repository.LowerName, diff --git a/routers/api/v1/repo/patch.go b/routers/api/v1/repo/patch.go index ae64c6efe..6dbf97970 100644 --- a/routers/api/v1/repo/patch.go +++ b/routers/api/v1/repo/patch.go @@ -77,7 +77,7 @@ func ApplyDiffPatch(ctx *context.APIContext) { opts.Message = "apply-patch" } - if !canWriteFiles(ctx.Repo) { + if !canWriteFiles(ctx, apiOpts.BranchName) { ctx.Error(http.StatusInternalServerError, "ApplyPatch", models.ErrUserDoesNotHaveAccessToRepo{ UserID: ctx.Doer.ID, RepoName: ctx.Repo.Repository.LowerName, diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 6b076eff8..951780218 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -616,6 +616,18 @@ func EditPullRequest(ctx *context.APIContext) { notification.NotifyPullRequestChangeTargetBranch(ctx.Doer, pr, form.Base) } + // update allow edits + if form.AllowMaintainerEdit != nil { + if err := pull_service.SetAllowEdits(ctx, ctx.Doer, pr, *form.AllowMaintainerEdit); err != nil { + if errors.Is(pull_service.ErrUserHasNoPermissionForAction, err) { + ctx.Error(http.StatusForbidden, "SetAllowEdits", fmt.Sprintf("SetAllowEdits: %s", err)) + return + } + ctx.ServerError("SetAllowEdits", err) + return + } + } + // Refetch from database pr, err = models.GetPullRequestByIndex(ctx.Repo.Repository.ID, pr.Index) if err != nil { diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index ccb693378..8824d9cc3 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -45,6 +45,8 @@ type preReceiveContext struct { env []string opts *private.HookOptions + + branchName string } // CanWriteCode returns true if pusher can write code @@ -53,7 +55,7 @@ func (ctx *preReceiveContext) CanWriteCode() bool { if !ctx.loadPusherAndPermission() { return false } - ctx.canWriteCode = ctx.userPerm.CanWrite(unit.TypeCode) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite + ctx.canWriteCode = ctx.userPerm.CanWriteToBranch(ctx.user, ctx.branchName) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite ctx.checkedCanWriteCode = true } return ctx.canWriteCode @@ -134,13 +136,15 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { } func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullName string) { + branchName := strings.TrimPrefix(refFullName, git.BranchPrefix) + ctx.branchName = branchName + if !ctx.AssertCanWriteCode() { return } repo := ctx.Repo.Repository gitRepo := ctx.Repo.GitRepo - branchName := strings.TrimPrefix(refFullName, git.BranchPrefix) if branchName == repo.DefaultBranch && newCommitID == git.EmptySHA { log.Warn("Forbidden: Branch: %s is the default branch in %-v and cannot be deleted", branchName, repo) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 9f7bef43e..7721507ba 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -398,6 +398,7 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo { } ctx.Data["HeadRepo"] = ci.HeadRepo + ctx.Data["BaseCompareRepo"] = ctx.Repo.Repository // Now we need to assert that the ctx.Doer has permission to read // the baseRepo's code and pulls @@ -436,6 +437,7 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo { ctx.NotFound("ParseCompareInfo", nil) return nil } + ctx.Data["CanWriteToHeadRepo"] = permHead.CanWrite(unit.TypeCode) } // If we have a rootRepo and it's different from: diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 8e865e448..a54ad3306 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1529,7 +1529,7 @@ func ViewIssue(ctx *context.Context) { if ctx.IsSigned { if err := pull.LoadHeadRepoCtx(ctx); err != nil { log.Error("LoadHeadRepo: %v", err) - } else if pull.HeadRepo != nil && pull.HeadBranch != pull.HeadRepo.DefaultBranch { + } else if pull.HeadRepo != nil { perm, err := models.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer) if err != nil { ctx.ServerError("GetUserRepoPermission", err) @@ -1537,12 +1537,15 @@ func ViewIssue(ctx *context.Context) { } if perm.CanWrite(unit.TypeCode) { // Check if branch is not protected - if protected, err := models.IsProtectedBranch(pull.HeadRepo.ID, pull.HeadBranch); err != nil { - log.Error("IsProtectedBranch: %v", err) - } else if !protected { - canDelete = true - ctx.Data["DeleteBranchLink"] = issue.Link() + "/cleanup" + if pull.HeadBranch != pull.HeadRepo.DefaultBranch { + if protected, err := models.IsProtectedBranch(pull.HeadRepo.ID, pull.HeadBranch); err != nil { + log.Error("IsProtectedBranch: %v", err) + } else if !protected { + canDelete = true + ctx.Data["DeleteBranchLink"] = issue.Link() + "/cleanup" + } } + ctx.Data["CanWriteToHeadRepo"] = true } } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index a03e16f39..99faa5413 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1132,14 +1132,15 @@ func CompareAndPullRequestPost(ctx *context.Context) { Content: form.Content, } pullRequest := &models.PullRequest{ - HeadRepoID: ci.HeadRepo.ID, - BaseRepoID: repo.ID, - HeadBranch: ci.HeadBranch, - BaseBranch: ci.BaseBranch, - HeadRepo: ci.HeadRepo, - BaseRepo: repo, - MergeBase: ci.CompareInfo.MergeBase, - Type: models.PullRequestGitea, + HeadRepoID: ci.HeadRepo.ID, + BaseRepoID: repo.ID, + HeadBranch: ci.HeadBranch, + BaseBranch: ci.BaseBranch, + HeadRepo: ci.HeadRepo, + BaseRepo: repo, + MergeBase: ci.CompareInfo.MergeBase, + Type: models.PullRequestGitea, + AllowMaintainerEdit: form.AllowMaintainerEdit, } // FIXME: check error in the case two people send pull request at almost same time, give nice error prompt // instead of 500. @@ -1411,3 +1412,31 @@ func UpdatePullRequestTarget(ctx *context.Context) { "base_branch": pr.BaseBranch, }) } + +// SetAllowEdits allow edits from maintainers to PRs +func SetAllowEdits(ctx *context.Context) { + form := web.GetForm(ctx).(*forms.UpdateAllowEditsForm) + + pr, err := models.GetPullRequestByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + if err != nil { + if models.IsErrPullRequestNotExist(err) { + ctx.NotFound("GetPullRequestByIndex", err) + } else { + ctx.ServerError("GetPullRequestByIndex", err) + } + return + } + + if err := pull_service.SetAllowEdits(ctx, ctx.Doer, pr, form.AllowMaintainerEdit); err != nil { + if errors.Is(pull_service.ErrUserHasNoPermissionForAction, err) { + ctx.Error(http.StatusForbidden) + return + } + ctx.ServerError("SetAllowEdits", err) + return + } + + ctx.JSON(http.StatusOK, map[string]interface{}{ + "allow_maintainer_edit": pr.AllowMaintainerEdit, + }) +} diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 168927d10..86fc36fad 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -579,7 +579,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st ctx.Data["LineEscapeStatus"] = statuses } if !isLFSFile { - if ctx.Repo.CanEnableEditor() { + if ctx.Repo.CanEnableEditor(ctx.Doer) { if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID { ctx.Data["CanEditFile"] = false ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.this_file_locked") @@ -589,7 +589,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st } } else if !ctx.Repo.IsViewBranch { ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch") - } else if !ctx.Repo.CanWrite(unit_model.TypeCode) { + } else if !ctx.Repo.CanWriteToBranch(ctx.Doer, ctx.Repo.BranchName) { ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.fork_before_edit") } } @@ -629,7 +629,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st } } - if ctx.Repo.CanEnableEditor() { + if ctx.Repo.CanEnableEditor(ctx.Doer) { if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID { ctx.Data["CanDeleteFile"] = false ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.this_file_locked") @@ -639,7 +639,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st } } else if !ctx.Repo.IsViewBranch { ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch") - } else if !ctx.Repo.CanWrite(unit_model.TypeCode) { + } else if !ctx.Repo.CanWriteToBranch(ctx.Doer, ctx.Repo.BranchName) { ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.must_have_write_access") } } diff --git a/routers/web/web.go b/routers/web/web.go index 0de6f1372..22b8e7cdf 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -574,6 +574,7 @@ func RegisterRoutes(m *web.Route) { reqRepoAdmin := context.RequireRepoAdmin() reqRepoCodeWriter := context.RequireRepoWriter(unit.TypeCode) + canEnableEditor := context.CanEnableEditor() reqRepoCodeReader := context.RequireRepoReader(unit.TypeCode) reqRepoReleaseWriter := context.RequireRepoWriter(unit.TypeReleases) reqRepoReleaseReader := context.RequireRepoReader(unit.TypeReleases) @@ -925,12 +926,12 @@ func RegisterRoutes(m *web.Route) { Post(bindIgnErr(forms.EditRepoFileForm{}), repo.NewDiffPatchPost) m.Combo("/_cherrypick/{sha:([a-f0-9]{7,40})}/*").Get(repo.CherryPick). Post(bindIgnErr(forms.CherryPickForm{}), repo.CherryPickPost) - }, context.RepoRefByType(context.RepoRefBranch), repo.MustBeEditable) + }, repo.MustBeEditable) m.Group("", func() { m.Post("/upload-file", repo.UploadFileToServer) m.Post("/upload-remove", bindIgnErr(forms.RemoveUploadFileForm{}), repo.RemoveUploadFileFromServer) - }, context.RepoRef(), repo.MustBeEditable, repo.MustBeAbleToUpload) - }, context.RepoMustNotBeArchived(), reqRepoCodeWriter, repo.MustBeNotEmpty) + }, repo.MustBeEditable, repo.MustBeAbleToUpload) + }, context.RepoRef(), canEnableEditor, context.RepoMustNotBeArchived(), repo.MustBeNotEmpty) m.Group("/branches", func() { m.Group("/_new", func() { @@ -1105,6 +1106,7 @@ func RegisterRoutes(m *web.Route) { m.Get("/commits", context.RepoRef(), repo.ViewPullCommits) m.Post("/merge", context.RepoMustNotBeArchived(), bindIgnErr(forms.MergePullRequestForm{}), repo.MergePullRequest) m.Post("/update", repo.UpdatePullRequest) + m.Post("/set_allow_maintainer_edit", bindIgnErr(forms.UpdateAllowEditsForm{}), repo.SetAllowEdits) m.Post("/cleanup", context.RepoMustNotBeArchived(), context.RepoRef(), repo.CleanUpPullRequest) m.Group("/files", func() { m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.ViewPullFiles) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 80123e9af..f40e99a04 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -422,15 +422,16 @@ func (f *NewPackagistHookForm) Validate(req *http.Request, errs binding.Errors) // CreateIssueForm form for creating issue type CreateIssueForm struct { - Title string `binding:"Required;MaxSize(255)"` - LabelIDs string `form:"label_ids"` - AssigneeIDs string `form:"assignee_ids"` - Ref string `form:"ref"` - MilestoneID int64 - ProjectID int64 - AssigneeID int64 - Content string - Files []string + Title string `binding:"Required;MaxSize(255)"` + LabelIDs string `form:"label_ids"` + AssigneeIDs string `form:"assignee_ids"` + Ref string `form:"ref"` + MilestoneID int64 + ProjectID int64 + AssigneeID int64 + Content string + Files []string + AllowMaintainerEdit bool } // Validate validates the fields @@ -684,6 +685,11 @@ type DismissReviewForm struct { Message string } +// UpdateAllowEditsForm form for changing if PR allows edits from maintainers +type UpdateAllowEditsForm struct { + AllowMaintainerEdit bool +} + // __________ .__ // \______ \ ____ | | ____ _____ ______ ____ // | _// __ \| | _/ __ \\__ \ / ___// __ \ diff --git a/services/pull/edits.go b/services/pull/edits.go new file mode 100644 index 000000000..68515ec14 --- /dev/null +++ b/services/pull/edits.go @@ -0,0 +1,40 @@ +// Copyright 2022 The Gitea Authors. +// All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package pull + +import ( + "context" + "errors" + + "code.gitea.io/gitea/models" + unit_model "code.gitea.io/gitea/models/unit" + user_model "code.gitea.io/gitea/models/user" +) + +var ErrUserHasNoPermissionForAction = errors.New("user not allowed to do this action") + +// SetAllowEdits allow edits from maintainers to PRs +func SetAllowEdits(ctx context.Context, doer *user_model.User, pr *models.PullRequest, allow bool) error { + if doer == nil || !pr.Issue.IsPoster(doer.ID) { + return ErrUserHasNoPermissionForAction + } + + if err := pr.LoadHeadRepo(); err != nil { + return err + } + + permission, err := models.GetUserRepoPermission(ctx, pr.HeadRepo, doer) + if err != nil { + return err + } + + if !permission.CanWrite(unit_model.TypeCode) { + return ErrUserHasNoPermissionForAction + } + + pr.AllowMaintainerEdit = allow + return models.UpdateAllowEdits(ctx, pr) +} diff --git a/services/pull/update.go b/services/pull/update.go index 899bee1f1..2d845e850 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -111,11 +111,25 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *models.PullRequest, user * return false, false, nil } + baseRepoPerm, err := models.GetUserRepoPermission(ctx, pull.BaseRepo, user) + if err != nil { + return false, false, err + } + mergeAllowed, err = IsUserAllowedToMerge(pr, headRepoPerm, user) if err != nil { return false, false, err } + if pull.AllowMaintainerEdit { + mergeAllowedMaintainer, err := IsUserAllowedToMerge(pr, baseRepoPerm, user) + if err != nil { + return false, false, err + } + + mergeAllowed = mergeAllowed || mergeAllowedMaintainer + } + return mergeAllowed, rebaseAllowed, nil } diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 1089c8241..9a4548643 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -235,6 +235,15 @@ {{end}} + {{if and .PageIsComparePull (not (eq .HeadRepo.FullName .BaseCompareRepo.FullName)) .CanWriteToHeadRepo}} +
+
+
+ + +
+
+ {{end}} diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index e673add81..9e9bc670a 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -667,5 +667,21 @@ {{end}} + + {{if and .Issue.IsPull .IsIssuePoster (not .Issue.IsClosed)}} + {{if and (not (eq .Issue.PullRequest.HeadRepo.FullName .Issue.PullRequest.BaseRepo.FullName)) .CanWriteToHeadRepo}} +
+
+
+ + +
+
+ {{end}} + {{end}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index ea03380d4..d57a3a580 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -14938,6 +14938,10 @@ "description": "EditPullRequestOption options when modify pull request", "type": "object", "properties": { + "allow_maintainer_edit": { + "type": "boolean", + "x-go-name": "AllowMaintainerEdit" + }, "assignee": { "type": "string", "x-go-name": "Assignee" @@ -17045,6 +17049,10 @@ "description": "PullRequest represents a pull request", "type": "object", "properties": { + "allow_maintainer_edit": { + "type": "boolean", + "x-go-name": "AllowMaintainerEdit" + }, "assignee": { "$ref": "#/definitions/User" }, diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 14b1976bb..a39a704f4 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -288,6 +288,39 @@ export function initRepoPullRequestMergeInstruction() { }); } +export function initRepoPullRequestAllowMaintainerEdit() { + const $checkbox = $('#allow-edits-from-maintainers'); + if (!$checkbox.length) return; + + const promptTip = $checkbox.attr('data-prompt-tip'); + const promptError = $checkbox.attr('data-prompt-error'); + $checkbox.popup({content: promptTip}); + $checkbox.checkbox({ + 'onChange': () => { + const checked = $checkbox.checkbox('is checked'); + let url = $checkbox.attr('data-url'); + url += '/set_allow_maintainer_edit'; + $checkbox.checkbox('set disabled'); + $.ajax({url, type: 'POST', + data: {_csrf: csrfToken, allow_maintainer_edit: checked}, + error: () => { + $checkbox.popup({ + content: promptError, + onHidden: () => { + // the error popup should be shown only once, then we restore the popup to the default message + $checkbox.popup({content: promptTip}); + }, + }); + $checkbox.popup('show'); + }, + complete: () => { + $checkbox.checkbox('set enabled'); + }, + }); + }, + }); +} + export function initRepoIssueReferenceRepositorySearch() { $('.issue_reference_repository_search') .dropdown({ diff --git a/web_src/js/index.js b/web_src/js/index.js index f5d72aff9..5b95a0d8e 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -36,6 +36,7 @@ import { initRepoIssueTimeTracking, initRepoIssueWipTitle, initRepoPullRequestMergeInstruction, + initRepoPullRequestAllowMaintainerEdit, initRepoPullRequestReview, } from './features/repo-issue.js'; import { @@ -158,6 +159,7 @@ $(document).ready(() => { initRepoMigrationStatusChecker(); initRepoProject(); initRepoPullRequestMergeInstruction(); + initRepoPullRequestAllowMaintainerEdit(); initRepoPullRequestReview(); initRepoRelease(); initRepoReleaseEditor();