Allow users with explicit read access to give approvals (#8382)

1 files changed, 22 insertions, 1 deletions

diff --git a/models/branches.go b/models/branches.go
index 9daaa487e7..fa8beb866c 100644
--- a/models/branches.go
+++ b/models/branches.go
@@ -195,7 +195,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
 	}
@@ -301,6 +301,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) {