Add "Allow edits from maintainer" feature (#18002)

Adds a feature [like GitHub has](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork) (step 7).
If you create a new PR from a forked repo, you can select (and change later, but only if you are the PR creator/poster) the "Allow edits from maintainers" option.
Then users with write access to the base branch get more permissions on this branch:
* use the update pull request button
* push directly from the command line (`git push`)
* edit/delete/upload files via web UI
* use related API endpoints

You can't merge PRs to this branch with this enabled, you'll need "full" code write permissions.

This feature has a pretty big impact on the permission system. I might forgot changing some things or didn't find security vulnerabilities. In this case, please leave a review or comment on this PR.

Closes #17728

Co-authored-by: 6543 <6543@obermui.de>

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

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))
+}

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:

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 {

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) {

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,

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
 	}

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,

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"`
 }

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 {

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

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() {

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,

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,

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 {