Check for protected branch also when committing to new branch

Author: brechtvl

routers/api/v1/api.go

@@ -434,7 +434,7 @@ 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, ctx.Doer, options.Branch()) && !ctx.IsUserSiteAdmin()) {
+	if !ok || (!context.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.Repository, options.Branch()) && !ctx.IsUserSiteAdmin()) {
 		ctx.APIError(http.StatusForbidden, "user should have a permission to write to this branch")
 		return
 	}

routers/api/v1/repo/file.go

@@ -405,7 +405,7 @@ func GetEditorconfig(ctx *context.APIContext) {
 
 // canWriteFiles returns true if repository is editable and user has proper access level.
 func canWriteFiles(ctx *context.APIContext, branch string) bool {
-	return ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, branch) &&
+	return context.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.Repository, branch) &&
 		!ctx.Repo.Repository.IsMirror &&
 		!ctx.Repo.Repository.IsArchived
 }

routers/web/repo/editor.go

@@ -53,7 +53,7 @@ func canCreateBasePullRequest(ctx *context.Context, editRepo *repo_model.Reposit
 func renderCommitRights(ctx *context.Context, editRepo *repo_model.Repository) bool {
 	if editRepo == ctx.Repo.Repository {
 		// Editing the same repository that we are viewing
-		canCommitToBranch, err := ctx.Repo.CanCommitToBranch(ctx, ctx.Doer)
+		canCommitToBranch, err := context.CanCommitToBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName)
 		if err != nil {
 			log.Error("CanCommitToBranch: %v", err)
 		}
@@ -64,7 +64,7 @@ func renderCommitRights(ctx *context.Context, editRepo *repo_model.Repository) b
 		return canCommitToBranch.CanCommitToBranch
 	}
 
-	// Editing a user fork of the repository we are viewing
+	// Editing a user fork of the repository we are viewing, always choose a new branch
 	ctx.Data["CanCommitToBranch"] = context.CanCommitToBranchResults{}
 	ctx.Data["CanCreatePullRequest"] = canCreateBasePullRequest(ctx, editRepo)
 
@@ -121,7 +121,7 @@ func getParentTreeFields(treePath string) (treeNames, treePaths []string) {
 // This may be a fork of the repository owned by the user. If no repository can be found
 // for editing, nil is returned along with a message explaining why editing is not possible.
 func getEditRepository(ctx *context.Context) (*repo_model.Repository, any) {
-	if ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.BranchName) {
+	if context.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName) {
 		return ctx.Repo.Repository, nil
 	}
 
@@ -197,18 +197,23 @@ func GetEditRepositoryOrError(ctx *context.Context, tpl templates.TplName, form
 
 // CheckPushEditBranch chesk if pushing to the branch in the edit repository is possible,
 // and if not renders an error and returns false.
-func CheckCanPushEditBranch(ctx *context.Context, editRepo *repo_model.Repository, branchName string, canCommit bool, tpl templates.TplName, form any) bool {
+func CheckCanPushEditBranch(ctx *context.Context, editRepo *repo_model.Repository, branchName string, tpl templates.TplName, form any) bool {
+	// When pushing to a fork or another branch on the same repository, it should not exist yet
 	if ctx.Repo.Repository != editRepo || ctx.Repo.BranchName != branchName {
-		// When pushing to a fork or another branch on the same repository, it should not exist yet
 		if exist, err := git_model.IsBranchExist(ctx, editRepo.ID, branchName); err == nil && exist {
 			ctx.Data["Err_NewBranchName"] = true
 			ctx.RenderWithErr(ctx.Tr("repo.editor.branch_already_exists", branchName), tpl, form)
 			return false
 		}
-	} else if ctx.Repo.Repository == editRepo && !canCommit {
-		// When we can't commit to the same branch on the same repository, it's protected
+	}
+
+	// Check for protected branch
+	canCommitToBranch, err := context.CanCommitToBranch(ctx, ctx.Doer, editRepo, branchName)
+	if err != nil {
+		log.Error("CanCommitToBranch: %v", err)
+	}
+	if !canCommitToBranch.CanCommitToBranch {
 		ctx.Data["Err_NewBranchName"] = true
-		ctx.Data["commit_choice"] = frmCommitChoiceNewBranch
 		ctx.RenderWithErr(ctx.Tr("repo.editor.cannot_commit_to_protected_branch", branchName), tpl, form)
 		return false
 	}
@@ -464,10 +469,10 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
 		return
 	}
 
-	canCommit := renderCommitRights(ctx, editRepo)
+	renderCommitRights(ctx, editRepo)
 
 	// Cannot commit to an existing branch if user doesn't have rights
-	if !CheckCanPushEditBranch(ctx, editRepo, branchName, canCommit, tplEditFile, &form) {
+	if !CheckCanPushEditBranch(ctx, editRepo, branchName, tplEditFile, &form) {
 		return
 	}
 
@@ -704,10 +709,10 @@ func DeleteFilePost(ctx *context.Context) {
 		return
 	}
 
-	canCommit := renderCommitRights(ctx, editRepo)
+	renderCommitRights(ctx, editRepo)
 
 	// Cannot commit to an existing branch if user doesn't have rights
-	if !CheckCanPushEditBranch(ctx, editRepo, branchName, canCommit, tplDeleteFile, &form) {
+	if !CheckCanPushEditBranch(ctx, editRepo, branchName, tplDeleteFile, &form) {
 		return
 	}
 
@@ -903,9 +908,9 @@ func UploadFilePost(ctx *context.Context) {
 		return
 	}
 
-	canCommit := renderCommitRights(ctx, editRepo)
+	renderCommitRights(ctx, editRepo)
 
-	if !CheckCanPushEditBranch(ctx, editRepo, branchName, canCommit, tplUploadFile, &form) {
+	if !CheckCanPushEditBranch(ctx, editRepo, branchName, tplUploadFile, &form) {
 		return
 	}
 

routers/web/repo/patch.go

@@ -76,10 +76,10 @@ func NewDiffPatchPost(ctx *context.Context) {
 		return
 	}
 
-	canCommit := renderCommitRights(ctx, editRepo)
+	renderCommitRights(ctx, editRepo)
 
 	// Cannot commit to an existing branch if user doesn't have rights
-	if !CheckCanPushEditBranch(ctx, editRepo, branchName, canCommit, tplPatchFile, &form) {
+	if !CheckCanPushEditBranch(ctx, editRepo, branchName, tplPatchFile, &form) {
 		return
 	}
 

services/context/permission.go

@@ -24,7 +24,7 @@ func RequireRepoAdmin() func(ctx *Context) {
 // MustBeAbleToCherryPick checks if the user is allowed to cherry-pick to a branch of the repo
 func MustBeAbleToCherryPick() func(ctx *Context) {
 	return func(ctx *Context) {
-		if !ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.BranchName) || !ctx.Repo.Repository.CanEnableEditor() {
+		if !CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.BranchName) || !ctx.Repo.Repository.CanEnableEditor() {
 			ctx.NotFound(nil)
 			return
 		}

services/context/repo.go

@@ -67,8 +67,13 @@ 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)
+func CanWriteToBranch(ctx context.Context, user *user_model.User, repo *repo_model.Repository, branch string) bool {
+	permission, err := access_model.GetUserRepoPermission(ctx, repo, user)
+	if err != nil {
+		return false
+	}
+
+	return issues_model.CanMaintainerWriteToBranch(ctx, permission, branch, user)
 }
 
 // CanEnableEditor returns true if the web editor can be enabled for this repository,
@@ -124,24 +129,23 @@ type CanCommitToBranchResults struct {
 }
 
 // CanCommitToBranch returns true if repository is editable and user has proper access level
-//
 // and branch is not protected for push
-func (r *Repository) CanCommitToBranch(ctx context.Context, doer *user_model.User) (CanCommitToBranchResults, error) {
-	protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, r.Repository.ID, r.BranchName)
+func CanCommitToBranch(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, branchName string) (CanCommitToBranchResults, error) {
+	protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName)
 	if err != nil {
 		return CanCommitToBranchResults{}, err
 	}
 	userCanPush := true
 	requireSigned := false
 	if protectedBranch != nil {
-		protectedBranch.Repo = r.Repository
+		protectedBranch.Repo = repo
 		userCanPush = protectedBranch.CanUserPush(ctx, doer)
 		requireSigned = protectedBranch.RequireSignedCommits
 	}
 
-	sign, keyID, _, err := asymkey_service.SignCRUDAction(ctx, r.Repository.RepoPath(), doer, r.Repository.RepoPath(), git.BranchPrefix+r.BranchName)
+	sign, keyID, _, err := asymkey_service.SignCRUDAction(ctx, repo.RepoPath(), doer, repo.RepoPath(), git.BranchPrefix+branchName)
 
-	canCommit := r.CanEnableEditor() && r.CanWriteToBranch(ctx, doer, r.BranchName) && userCanPush
+	canCommit := repo.CanEnableEditor() && CanWriteToBranch(ctx, doer, repo, branchName) && userCanPush
 	if requireSigned {
 		canCommit = canCommit && sign
 	}