Fix delete branch perm checking

models/perm/access/repo_permission.go

@@ -18,6 +18,21 @@ import (
 	"code.gitea.io/gitea/modules/util"
 )
 
+type ErrPermissionDenied struct {
+	RepoID int64
+	Unit   unit.Type
+	Perm   perm_model.AccessMode
+}
+
+func (e ErrPermissionDenied) Error() string {
+	return fmt.Sprintf("permission denied to access repo %d unit %s with mode %s", e.RepoID, e.Unit.LogString(), e.Perm.LogString())
+}
+
+func IsErrPermissionDenied(err error) bool {
+	_, ok := err.(ErrPermissionDenied)
+	return ok
+}
+
 // Permission contains all the permissions related variables to a repository for a user
 type Permission struct {
 	AccessMode perm_model.AccessMode

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,53 @@ 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)
+	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

@@ -1404,7 +1404,7 @@ func CleanUpPullRequest(ctx *context.Context) {
 	pr := issue.PullRequest
 
 	// Don't cleanup unmerged and unclosed PRs
-	if !pr.HasMerged && !issue.IsClosed {
+	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 access_model.IsErrPermissionDenied(err) {
+			ctx.NotFound("CanDeleteBranch", nil)
+		} else {
+			ctx.ServerError("CanDeleteBranch", err)
+		}
 		return
 	}
 

services/repository/branch.go

@@ -14,7 +14,10 @@ import (
 	"code.gitea.io/gitea/models/db"
 	git_model "code.gitea.io/gitea/models/git"
 	issues_model "code.gitea.io/gitea/models/issues"
+	perm_model "code.gitea.io/gitea/models/perm"
+	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 +466,21 @@ 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 access_model.ErrPermissionDenied{
+			RepoID: repo.ID,
+			Unit:   unit.TypeCode,
+			Perm:   perm_model.AccessModeWrite,
+		}
 	}
 
 	isProtected, err := git_model.IsBranchProtected(ctx, repo.ID, branchName)
@@ -481,6 +490,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) {