Added checks for protected branches in pull requests (#3544)

* Added checks for protected branches in pull requests

Signed-off-by: Christian Wulff <[email protected]>

* Moved check for protected branch into new function CheckUserAllowedToMerge

Signed-off-by: Christian Wulff <[email protected]>

* Removed merge conflict lines from last commit

Signed-off-by: Christian Wulff <[email protected]>

* Explicit check for error type in ViewIssue

Signed-off-by: Christian Wulff <[email protected]>

Author: Chri-s

models/error.go

@@ -785,6 +785,21 @@ func (err ErrBranchNameConflict) Error() string {
 	return fmt.Sprintf("branch conflicts with existing branch [name: %s]", err.BranchName)
 }
 
+// ErrNotAllowedToMerge represents an error that a branch is protected and the current user is not allowed to modify it
+type ErrNotAllowedToMerge struct {
+	Reason string
+}
+
+// IsErrNotAllowedToMerge checks if an error is an ErrNotAllowedToMerge.
+func IsErrNotAllowedToMerge(err error) bool {
+	_, ok := err.(ErrNotAllowedToMerge)
+	return ok
+}
+
+func (err ErrNotAllowedToMerge) Error() string {
+	return fmt.Sprintf("not allowed to merge [reason: %s]", err.Reason)
+}
+
 // ErrTagAlreadyExists represents an error that tag with such name already exists
 type ErrTagAlreadyExists struct {
 	TagName string

models/pull.go

@@ -272,6 +272,31 @@ const (
 	MergeStyleSquash MergeStyle = "squash"
 )
 
+// CheckUserAllowedToMerge checks whether the user is allowed to merge
+func (pr *PullRequest) CheckUserAllowedToMerge(doer *User) (err error) {
+	if doer == nil {
+		return ErrNotAllowedToMerge{
+			"Not signed in",
+		}
+	}
+
+	if pr.BaseRepo == nil {
+		if err = pr.GetBaseRepo(); err != nil {
+			return fmt.Errorf("GetBaseRepo: %v", err)
+		}
+	}
+
+	if protected, err := pr.BaseRepo.IsProtectedBranch(pr.BaseBranch, doer); err != nil {
+		return fmt.Errorf("IsProtectedBranch: %v", err)
+	} else if protected {
+		return ErrNotAllowedToMerge{
+			"The branch is protected",
+		}
+	}
+
+	return nil
+}
+
 // Merge merges pull request to base repository.
 // FIXME: add repoWorkingPull make sure two merges does not happen at same time.
 func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle MergeStyle, message string) (err error) {
@@ -287,6 +312,10 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle
 	}
 	prConfig := prUnit.PullRequestsConfig()
 
+	if err := pr.CheckUserAllowedToMerge(doer); err != nil {
+		return fmt.Errorf("CheckUserAllowedToMerge: %v", err)
+	}
+
 	// Check if merge style is correct and allowed
 	if !prConfig.IsMergeStyleAllowed(mergeStyle) {
 		return ErrInvalidMergeStyle{pr.BaseRepo.ID, mergeStyle}

routers/repo/issue.go

@@ -734,6 +734,15 @@ func ViewIssue(ctx *context.Context) {
 		}
 		prConfig := prUnit.PullRequestsConfig()
 
+		ctx.Data["AllowMerge"] = ctx.Data["IsRepositoryWriter"]
+		if err := pull.CheckUserAllowedToMerge(ctx.User); err != nil {
+			if !models.IsErrNotAllowedToMerge(err) {
+				ctx.ServerError("CheckUserAllowedToMerge", err)
+				return
+			}
+			ctx.Data["AllowMerge"] = false
+		}
+
 		// Check correct values and select default
 		if ms, ok := ctx.Data["MergeStyle"].(models.MergeStyle); !ok ||
 			!prConfig.IsMergeStyleAllowed(ms) {

templates/repo/issue/view_content/pull.tmpl

@@ -37,7 +37,7 @@
 					<span class="octicon octicon-check"></span>
 					{{$.i18n.Tr "repo.pulls.can_auto_merge_desc"}}
 				</div>
-				{{if .IsRepositoryWriter}}
+				{{if .AllowMerge}}
 					{{$prUnit := .Repository.MustGetUnit $.UnitTypePullRequests}}
 					{{if or $prUnit.PullRequestsConfig.AllowMerge $prUnit.PullRequestsConfig.AllowRebase $prUnit.PullRequestsConfig.AllowSquash}}
 						<div class="ui divider"></div>