From a2a49c93c78a81d1eaa6b0eaf84a0c3f4bcd2487 Mon Sep 17 00:00:00 2001 From: Chri-s Date: Tue, 13 Mar 2018 04:46:14 +0100 Subject: [PATCH] Added checks for protected branches in pull requests (#3544) * Added checks for protected branches in pull requests Signed-off-by: Christian Wulff * Moved check for protected branch into new function CheckUserAllowedToMerge Signed-off-by: Christian Wulff * Removed merge conflict lines from last commit Signed-off-by: Christian Wulff * Explicit check for error type in ViewIssue Signed-off-by: Christian Wulff --- models/error.go | 15 +++++++++++ models/pull.go | 29 +++++++++++++++++++++ routers/repo/issue.go | 9 +++++++ templates/repo/issue/view_content/pull.tmpl | 2 +- 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/models/error.go b/models/error.go index a1c24f4ec..d24da2642 100644 --- a/models/error.go +++ b/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 diff --git a/models/pull.go b/models/pull.go index 15b3a4fe1..137900bee 100644 --- a/models/pull.go +++ b/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} diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 230b0e939..a63572d5b 100644 --- a/routers/repo/issue.go +++ b/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) { diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 9bd51b7c3..d4bf5a327 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -37,7 +37,7 @@ {{$.i18n.Tr "repo.pulls.can_auto_merge_desc"}} - {{if .IsRepositoryWriter}} + {{if .AllowMerge}} {{$prUnit := .Repository.MustGetUnit $.UnitTypePullRequests}} {{if or $prUnit.PullRequestsConfig.AllowMerge $prUnit.PullRequestsConfig.AllowRebase $prUnit.PullRequestsConfig.AllowSquash}}