Fix delete branch perm checking

routers/api/v1/repo/branch.go

@@ -150,11 +150,6 @@ func DeleteBranch(ctx *context.APIContext) {
 }
 }
 
-if ctx.Repo.Repository.IsMirror {
-ctx.Error(http.StatusForbidden, "IsMirrored", fmt.Errorf("can not delete branch of an mirror repository"))
-return
-}
-
 if err := repo_service.DeleteBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, branchName); err != nil {
 switch {
 case git.IsErrBranchNotExist(err):

routers/api/v1/repo/pull.go

@@ -1057,49 +1057,54 @@ func MergePullRequest(ctx *context.APIContext) {
 }
 log.Trace("Pull request merged: %d", pr.ID)
 
-if form.DeleteBranchAfterMerge {
-// Don't cleanup when there are other PR's that use this branch as head branch.
-exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
-if err != nil {
-ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
-return
-}
-if exist {
-ctx.Status(http.StatusOK)
-return
-}
-
-var headRepo *git.Repository
-if ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.HeadRepoID && ctx.Repo.GitRepo != nil {
-headRepo = ctx.Repo.GitRepo
-} else {
-headRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo)
+// for agit flow, we should not delete the agit reference after merge
+if form.DeleteBranchAfterMerge && pr.Flow == issues_model.PullRequestFlowGithub {
+// check permission even it has been checked in repo_service.DeleteBranch so that we don't need to
+// do RetargetChildrenOnMerge
+if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err == nil {
+// Don't cleanup when there are other PR's that use this branch as head branch.
+exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
 if err != nil {
-ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.FullName()), err)
+ctx.ServerError("HasUnmergedPullRequestsByHeadInfo", err)
 return
 }
-defer headRepo.Close()
-}
-if err := pull_service.RetargetChildrenOnMerge(ctx, ctx.Doer, pr); err != nil {
-ctx.Error(http.StatusInternalServerError, "RetargetChildrenOnMerge", err)
-return
-}
-if err := repo_service.DeleteBranch(ctx, ctx.Doer, pr.HeadRepo, headRepo, pr.HeadBranch); err != nil {
-switch {
-case git.IsErrBranchNotExist(err):
-ctx.NotFound(err)
-case errors.Is(err, repo_service.ErrBranchIsDefault):
-ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
-case errors.Is(err, git_model.ErrBranchIsProtected):
-ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected"))
-default:
-ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
+if exist {
+ctx.Status(http.StatusOK)
+return
+}
+
+var headRepo *git.Repository
+if ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.HeadRepoID && ctx.Repo.GitRepo != nil {
+headRepo = ctx.Repo.GitRepo
+} else {
+headRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo)
+if err != nil {
+ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.FullName()), err)
+return
+}
+defer headRepo.Close()
+}
+if err := pull_service.RetargetChildrenOnMerge(ctx, ctx.Doer, pr); err != nil {
+ctx.Error(http.StatusInternalServerError, "RetargetChildrenOnMerge", err)
+return
+}
+if err := repo_service.DeleteBranch(ctx, ctx.Doer, pr.HeadRepo, headRepo, pr.HeadBranch); err != nil {
+switch {
+case git.IsErrBranchNotExist(err):
+ctx.NotFound(err)
+case errors.Is(err, repo_service.ErrBranchIsDefault):
+ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
+case errors.Is(err, git_model.ErrBranchIsProtected):
+ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected"))
+default:
+ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
+}
+return
+}
+if err := issues_model.AddDeletePRBranchComment(ctx, ctx.Doer, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil {
+// Do not fail here as branch has already been deleted
+log.Error("DeleteBranch: %v", err)
 }
-return
-}
-if err := issues_model.AddDeletePRBranchComment(ctx, ctx.Doer, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil {
-// Do not fail here as branch has already been deleted
-log.Error("DeleteBranch: %v", err)
 }
 }
 

routers/web/repo/pull.go

@@ -1403,8 +1403,8 @@ func CleanUpPullRequest(ctx *context.Context) {
 
 pr := issue.PullRequest
 
-// Don't cleanup unmerged and unclosed PRs
-if !pr.HasMerged && !issue.IsClosed {
+// Don't cleanup unmerged and unclosed PRs and agit PRs
+if !pr.HasMerged && !issue.IsClosed && pr.Flow != issues_model.PullRequestFlowGithub {
 ctx.NotFound("CleanUpPullRequest", nil)
 return
 }
@@ -1435,13 +1435,12 @@ func CleanUpPullRequest(ctx *context.Context) {
 return
 }
 
-perm, err := access_model.GetUserRepoPermission(ctx, pr.HeadRepo, ctx.Doer)
-if err != nil {
-ctx.ServerError("GetUserRepoPermission", err)
-return
-}
-if !perm.CanWrite(unit.TypeCode) {
-ctx.NotFound("CleanUpPullRequest", nil)
+if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err != nil {
+if errors.Is(err, util.ErrPermissionDenied) {
+ctx.NotFound("CanDeleteBranch", nil)
+} else {
+ctx.ServerError("CanDeleteBranch", err)
+}
 return
 }
 

services/repository/branch.go

@@ -14,7 +14,9 @@ import (
 "code.gitea.io/gitea/models/db"
 git_model "code.gitea.io/gitea/models/git"
 issues_model "code.gitea.io/gitea/models/issues"
+access_model "code.gitea.io/gitea/models/perm/access"
 repo_model "code.gitea.io/gitea/models/repo"
+"code.gitea.io/gitea/models/unit"
 user_model "code.gitea.io/gitea/models/user"
 "code.gitea.io/gitea/modules/cache"
 "code.gitea.io/gitea/modules/git"
@@ -463,15 +465,17 @@ var (
 ErrBranchIsDefault = errors.New("branch is default")
 )
 
-// DeleteBranch delete branch
-func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, branchName string) error {
-err := repo.MustNotBeArchived()
+func CanDeleteBranch(ctx context.Context, repo *repo_model.Repository, branchName string, doer *user_model.User) error {
+if branchName == repo.DefaultBranch {
+return ErrBranchIsDefault
+}
+
+perm, err := access_model.GetUserRepoPermission(ctx, repo, doer)
 if err != nil {
 return err
 }
-
-if branchName == repo.DefaultBranch {
-return ErrBranchIsDefault
+if !perm.CanWrite(unit.TypeCode) {
+return util.NewPermissionDeniedErrorf("permission denied to access repo %d unit %s", repo.ID, unit.TypeCode.LogString())
 }
 
 isProtected, err := git_model.IsBranchProtected(ctx, repo.ID, branchName)
@@ -481,6 +485,19 @@ func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.R
 if isProtected {
 return git_model.ErrBranchIsProtected
 }
+return nil
+}
+
+// DeleteBranch delete branch
+func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, branchName string) error {
+err := repo.MustNotBeArchived()
+if err != nil {
+return err
+}
+
+if err := CanDeleteBranch(ctx, repo, branchName, doer); err != nil {
+return err
+}
 
 rawBranch, err := git_model.GetBranch(ctx, repo.ID, branchName)
 if err != nil && !git_model.IsErrBranchNotExist(err) {