Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow users with explicit read access to give approvals #8382

Merged
merged 2 commits into from
Oct 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion models/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 13 additions & 0 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -735,11 +735,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 {
Expand Down
4 changes: 2 additions & 2 deletions routers/repo/setting_protected_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,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
Expand Down