From 5bac1a692a9b7a69011dad23a89de9ed59566264 Mon Sep 17 00:00:00 2001
From: guillep2k <18600385+guillep2k@users.noreply.github.com>
Date: Sun, 6 Oct 2019 17:52:25 -0300
Subject: [PATCH] Allow users with explicit read access to give approvals
 (#8398)

---
 models/branches.go                       | 23 ++++++++++++++++++++++-
 models/repo.go                           | 13 +++++++++++++
 routers/repo/setting_protected_branch.go |  4 ++--
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/models/branches.go b/models/branches.go
index c5b8daf64c..5c259b8c65 100644
--- a/models/branches.go
+++ b/models/branches.go
@@ -192,7 +192,7 @@ func UpdateProtectBranch(repo *Repository, protectBranch *ProtectedBranch, opts
 	}
 	protectBranch.MergeWhitelistUserIDs = whitelist
 
-	whitelist, err = updateUserWhitelist(repo, protectBranch.ApprovalsWhitelistUserIDs, opts.ApprovalsUserIDs)
+	whitelist, err = updateApprovalWhitelist(repo, protectBranch.ApprovalsWhitelistUserIDs, opts.ApprovalsUserIDs)
 	if err != nil {
 		return err
 	}
@@ -298,6 +298,27 @@ func (repo *Repository) IsProtectedBranchForMerging(pr *PullRequest, branchName
 	return false, nil
 }
 
+// updateApprovalWhitelist checks whether the user whitelist changed and returns a whitelist with
+// the users from newWhitelist which have explicit read or write access to the repo.
+func updateApprovalWhitelist(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 {
+		if reader, err := repo.IsReader(userID); err != nil {
+			return nil, err
+		} else if !reader {
+			continue
+		}
+		whitelist = append(whitelist, userID)
+	}
+
+	return
+}
+
 // 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) {
diff --git a/models/repo.go b/models/repo.go
index 10105d3071..cde5fbff2d 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -707,11 +707,24 @@ func (repo *Repository) CanEnableEditor() bool {
 	return !repo.IsMirror
 }
 
+// GetReaders returns all users that have explicit read access or higher to the repository.
+func (repo *Repository) GetReaders() (_ []*User, err error) {
+	return repo.getUsersWithAccessMode(x, AccessModeRead)
+}
+
 // GetWriters returns all users that have write access to the repository.
 func (repo *Repository) GetWriters() (_ []*User, err error) {
 	return repo.getUsersWithAccessMode(x, AccessModeWrite)
 }
 
+// IsReader returns true if user has explicit read access or higher to the repository.
+func (repo *Repository) IsReader(userID int64) (bool, error) {
+	if repo.OwnerID == userID {
+		return true, nil
+	}
+	return x.Where("repo_id = ? AND user_id = ? AND mode >= ?", repo.ID, userID, AccessModeRead).Get(&Access{})
+}
+
 // getUsersWithAccessMode returns users that have at least given access mode to the repository.
 func (repo *Repository) getUsersWithAccessMode(e Engine, mode AccessMode) (_ []*User, err error) {
 	if err = repo.getOwner(e); err != nil {
diff --git a/routers/repo/setting_protected_branch.go b/routers/repo/setting_protected_branch.go
index b5a115b6a4..ad0a142592 100644
--- a/routers/repo/setting_protected_branch.go
+++ b/routers/repo/setting_protected_branch.go
@@ -116,9 +116,9 @@ func SettingsProtectedBranch(c *context.Context) {
 		}
 	}
 
-	users, err := c.Repo.Repository.GetWriters()
+	users, err := c.Repo.Repository.GetReaders()
 	if err != nil {
-		c.ServerError("Repo.Repository.GetWriters", err)
+		c.ServerError("Repo.Repository.GetReaders", err)
 		return
 	}
 	c.Data["Users"] = users
-- 
2.39.5