Add protected branch whitelists for merging (#3689)

* Add database migrations for merge whitelist

* Add merge whitelist settings for protected branches

* Add checks for merge whitelists

4 files changed, 149 insertions, 36 deletions

diff --git a/models/branches.go b/models/branches.go
index 0a3d19858b..faa3ba6af8 100644
--- a/models/branches.go
+++ b/models/branches.go
@@ -23,15 +23,18 @@ const (
 
 // ProtectedBranch struct
 type ProtectedBranch struct {
-	ID               int64  `xorm:"pk autoincr"`
-	RepoID           int64  `xorm:"UNIQUE(s)"`
-	BranchName       string `xorm:"UNIQUE(s)"`
-	CanPush          bool   `xorm:"NOT NULL DEFAULT false"`
-	EnableWhitelist  bool
-	WhitelistUserIDs []int64        `xorm:"JSON TEXT"`
-	WhitelistTeamIDs []int64        `xorm:"JSON TEXT"`
-	CreatedUnix      util.TimeStamp `xorm:"created"`
-	UpdatedUnix      util.TimeStamp `xorm:"updated"`
+	ID                    int64  `xorm:"pk autoincr"`
+	RepoID                int64  `xorm:"UNIQUE(s)"`
+	BranchName            string `xorm:"UNIQUE(s)"`
+	CanPush               bool   `xorm:"NOT NULL DEFAULT false"`
+	EnableWhitelist       bool
+	WhitelistUserIDs      []int64        `xorm:"JSON TEXT"`
+	WhitelistTeamIDs      []int64        `xorm:"JSON TEXT"`
+	EnableMergeWhitelist  bool           `xorm:"NOT NULL DEFAULT false"`
+	MergeWhitelistUserIDs []int64        `xorm:"JSON TEXT"`
+	MergeWhitelistTeamIDs []int64        `xorm:"JSON TEXT"`
+	CreatedUnix           util.TimeStamp `xorm:"created"`
+	UpdatedUnix           util.TimeStamp `xorm:"updated"`
 }
 
 // IsProtected returns if the branch is protected
@@ -61,6 +64,28 @@ func (protectBranch *ProtectedBranch) CanUserPush(userID int64) bool {
 	return in
 }
 
+// CanUserMerge returns if some user could merge a pull request to this protected branch
+func (protectBranch *ProtectedBranch) CanUserMerge(userID int64) bool {
+	if !protectBranch.EnableMergeWhitelist {
+		return true
+	}
+
+	if base.Int64sContains(protectBranch.MergeWhitelistUserIDs, userID) {
+		return true
+	}
+
+	if len(protectBranch.WhitelistTeamIDs) == 0 {
+		return false
+	}
+
+	in, err := IsUserInTeams(userID, protectBranch.MergeWhitelistTeamIDs)
+	if err != nil {
+		log.Error(1, "IsUserInTeams:", err)
+		return false
+	}
+	return in
+}
+
 // GetProtectedBranchByRepoID getting protected branch by repo ID
 func GetProtectedBranchByRepoID(RepoID int64) ([]*ProtectedBranch, error) {
 	protectedBranches := make([]*ProtectedBranch, 0)
@@ -97,40 +122,35 @@ func GetProtectedBranchByID(id int64) (*ProtectedBranch, error) {
 // If ID is 0, it creates a new record. Otherwise, updates existing record.
 // This function also performs check if whitelist user and team's IDs have been changed
 // to avoid unnecessary whitelist delete and regenerate.
-func UpdateProtectBranch(repo *Repository, protectBranch *ProtectedBranch, whitelistUserIDs, whitelistTeamIDs []int64) (err error) {
+func UpdateProtectBranch(repo *Repository, protectBranch *ProtectedBranch, whitelistUserIDs, whitelistTeamIDs, mergeWhitelistUserIDs, mergeWhitelistTeamIDs []int64) (err error) {
 	if err = repo.GetOwner(); err != nil {
 		return fmt.Errorf("GetOwner: %v", err)
 	}
 
-	hasUsersChanged := !util.IsSliceInt64Eq(protectBranch.WhitelistUserIDs, whitelistUserIDs)
-	if hasUsersChanged {
-		protectBranch.WhitelistUserIDs = make([]int64, 0, len(whitelistUserIDs))
-		for _, userID := range whitelistUserIDs {
-			has, err := hasAccess(x, userID, repo, AccessModeWrite)
-			if err != nil {
-				return fmt.Errorf("HasAccess [user_id: %d, repo_id: %d]: %v", userID, protectBranch.RepoID, err)
-			} else if !has {
-				continue // Drop invalid user ID
-			}
+	whitelist, err := updateUserWhitelist(repo, protectBranch.WhitelistUserIDs, whitelistUserIDs)
+	if err != nil {
+		return err
+	}
+	protectBranch.WhitelistUserIDs = whitelist
 
-			protectBranch.WhitelistUserIDs = append(protectBranch.WhitelistUserIDs, userID)
-		}
+	whitelist, err = updateUserWhitelist(repo, protectBranch.MergeWhitelistUserIDs, mergeWhitelistUserIDs)
+	if err != nil {
+		return err
 	}
+	protectBranch.MergeWhitelistUserIDs = whitelist
 
-	// if the repo is in an orgniziation
-	hasTeamsChanged := !util.IsSliceInt64Eq(protectBranch.WhitelistTeamIDs, whitelistTeamIDs)
-	if hasTeamsChanged {
-		teams, err := GetTeamsWithAccessToRepo(repo.OwnerID, repo.ID, AccessModeWrite)
-		if err != nil {
-			return fmt.Errorf("GetTeamsWithAccessToRepo [org_id: %d, repo_id: %d]: %v", repo.OwnerID, repo.ID, err)
-		}
-		protectBranch.WhitelistTeamIDs = make([]int64, 0, len(teams))
-		for i := range teams {
-			if teams[i].HasWriteAccess() && com.IsSliceContainsInt64(whitelistTeamIDs, teams[i].ID) {
-				protectBranch.WhitelistTeamIDs = append(protectBranch.WhitelistTeamIDs, teams[i].ID)
-			}
-		}
+	// if the repo is in an organization
+	whitelist, err = updateTeamWhitelist(repo, protectBranch.WhitelistTeamIDs, whitelistTeamIDs)
+	if err != nil {
+		return err
+	}
+	protectBranch.WhitelistTeamIDs = whitelist
+
+	whitelist, err = updateTeamWhitelist(repo, protectBranch.MergeWhitelistTeamIDs, mergeWhitelistTeamIDs)
+	if err != nil {
+		return err
 	}
+	protectBranch.MergeWhitelistTeamIDs = whitelist
 
 	// Make sure protectBranch.ID is not 0 for whitelists
 	if protectBranch.ID == 0 {
@@ -174,6 +194,73 @@ func (repo *Repository) IsProtectedBranch(branchName string, doer *User) (bool,
 	return false, nil
 }
 
+// IsProtectedBranchForMerging checks if branch is protected for merging
+func (repo *Repository) IsProtectedBranchForMerging(branchName string, doer *User) (bool, error) {
+	if doer == nil {
+		return true, nil
+	}
+
+	protectedBranch := &ProtectedBranch{
+		RepoID:     repo.ID,
+		BranchName: branchName,
+	}
+
+	has, err := x.Get(protectedBranch)
+	if err != nil {
+		return true, err
+	} else if has {
+		return !protectedBranch.CanUserMerge(doer.ID), nil
+	}
+
+	return false, nil
+}
+
+// updateUserWhitelist checks whether the user whitelist changed and returns a whitelist with
+// the users from newWhitelist which have write access to the repo.
+func updateUserWhitelist(repo *Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
+	hasUsersChanged := !util.IsSliceInt64Eq(currentWhitelist, newWhitelist)
+	if !hasUsersChanged {
+		return currentWhitelist, nil
+	}
+
+	whitelist = make([]int64, 0, len(newWhitelist))
+	for _, userID := range newWhitelist {
+		has, err := hasAccess(x, userID, repo, AccessModeWrite)
+		if err != nil {
+			return nil, fmt.Errorf("HasAccess [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
+		} else if !has {
+			continue // Drop invalid user ID
+		}
+
+		whitelist = append(whitelist, userID)
+	}
+
+	return
+}
+
+// updateTeamWhitelist checks whether the team whitelist changed and returns a whitelist with
+// the teams from newWhitelist which have write access to the repo.
+func updateTeamWhitelist(repo *Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
+	hasTeamsChanged := !util.IsSliceInt64Eq(currentWhitelist, newWhitelist)
+	if !hasTeamsChanged {
+		return currentWhitelist, nil
+	}
+
+	teams, err := GetTeamsWithAccessToRepo(repo.OwnerID, repo.ID, AccessModeWrite)
+	if err != nil {
+		return nil, fmt.Errorf("GetTeamsWithAccessToRepo [org_id: %d, repo_id: %d]: %v", repo.OwnerID, repo.ID, err)
+	}
+
+	whitelist = make([]int64, 0, len(teams))
+	for i := range teams {
+		if teams[i].HasWriteAccess() && com.IsSliceContainsInt64(newWhitelist, teams[i].ID) {
+			whitelist = append(whitelist, teams[i].ID)
+		}
+	}
+
+	return
+}
+
 // DeleteProtectedBranch removes ProtectedBranch relation between the user and repository.
 func (repo *Repository) DeleteProtectedBranch(id int64) (err error) {
 	protectedBranch := &ProtectedBranch{
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index cc5d0bae81..6c3404bcd7 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -170,6 +170,8 @@ var migrations = []Migration{
 	NewMigration("add closed_unix column for issues", addIssueClosedTime),
 	// v58 -> v59
 	NewMigration("add label descriptions", addLabelsDescriptions),
+	// v59 -> v60
+	NewMigration("add merge whitelist for protected branches", addProtectedBranchMergeWhitelist),
 }
 
 // Migrate database to current version
diff --git a/models/migrations/v59.go b/models/migrations/v59.go
new file mode 100644
index 0000000000..0a05495e76
--- /dev/null
+++ b/models/migrations/v59.go
@@ -0,0 +1,24 @@
+// Copyright 2018 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"fmt"
+
+	"github.com/go-xorm/xorm"
+)
+
+func addProtectedBranchMergeWhitelist(x *xorm.Engine) error {
+	type ProtectedBranch struct {
+		EnableMergeWhitelist  bool    `xorm:"NOT NULL DEFAULT false"`
+		MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
+		MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
+	}
+
+	if err := x.Sync2(new(ProtectedBranch)); err != nil {
+		return fmt.Errorf("Sync2: %v", err)
+	}
+	return nil
+}
diff --git a/models/pull.go b/models/pull.go
index 137900beed..6862d11a1d 100644
--- a/models/pull.go
+++ b/models/pull.go
@@ -286,7 +286,7 @@ func (pr *PullRequest) CheckUserAllowedToMerge(doer *User) (err error) {
 		}
 	}
 
-	if protected, err := pr.BaseRepo.IsProtectedBranch(pr.BaseBranch, doer); err != nil {
+	if protected, err := pr.BaseRepo.IsProtectedBranchForMerging(pr.BaseBranch, doer); err != nil {
 		return fmt.Errorf("IsProtectedBranch: %v", err)
 	} else if protected {
 		return ErrNotAllowedToMerge{