Fix branch_protection api shows users/teams who has no readAccess

models/organization/org.go

@@ -658,6 +658,11 @@ func (org *Organization) TeamsWithAccessToRepo(ctx context.Context, repoID int64
 	return GetTeamsWithAccessToRepo(ctx, org.ID, repoID, mode)
 }
 
+// TeamsWithAccessToRepoFromIDs returns teams that have given access level to the repository based on the given teamIDs.
+func (org *Organization) TeamsWithAccessToRepoFromIDs(ctx context.Context, repoID int64, mode perm.AccessMode, teamIDs []int64) ([]*Team, error) {
+	return GetTeamsWithAccessToRepoFromIDs(ctx, org.ID, repoID, mode, teamIDs)
+}
+
 // GetUserTeamIDs returns of all team IDs of the organization that user is member of.
 func (org *Organization) GetUserTeamIDs(ctx context.Context, userID int64) ([]int64, error) {
 	return org.getUserTeamIDs(ctx, userID)

models/organization/team_repo.go

@@ -83,3 +83,15 @@ func GetTeamsWithAccessToRepo(ctx context.Context, orgID, repoID int64, mode per
 		OrderBy("name").
 		Find(&teams)
 }
+
+// GetTeamsWithAccessToRepoFromIDs returns teams in an organization that have given access level to the repository based on given teamIDs.
+func GetTeamsWithAccessToRepoFromIDs(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, teamIDs []int64) ([]*Team, error) {
+	teams := make([]*Team, 0)
+	return teams, db.GetEngine(ctx).Where("team.authorize >= ?", mode).
+		In("team.id", teamIDs).
+		Join("INNER", "team_repo", "team_repo.team_id = team.id").
+		And("team_repo.org_id = ?", orgID).
+		And("team_repo.repo_id = ?", repoID).
+		OrderBy("name").
+		Find(&teams)
+}

models/perm/access/repo_permission.go

@@ -398,6 +398,32 @@ func GetRepoReaders(ctx context.Context, repo *repo_model.Repository) (_ []*user
 	return getUsersWithAccessMode(ctx, repo, perm_model.AccessModeRead)
 }
 
+// getUsersWithAccessModeFromIDs returns users that have at least given access mode to the repository based on given userIDs.
+func getUsersWithAccessModeFromIDs(ctx context.Context, repo *repo_model.Repository, mode perm_model.AccessMode, userIDs []int64) (_ []*user_model.User, err error) {
+	e := db.GetEngine(ctx)
+	accesses := make([]*Access, 0, 10)
+	if err = e.Where("repo_id = ? AND mode >= ?", repo.ID, mode).In("user_id", userIDs).Find(&accesses); err != nil {
+		return nil, err
+	}
+	users := make([]*user_model.User, 0)
+	if len(accesses) > 0 {
+		userIDs := make([]int64, len(accesses))
+		for i := 0; i < len(accesses); i++ {
+			userIDs[i] = accesses[i].UserID
+		}
+		if err = e.In("id", userIDs).Find(&users); err != nil {
+			return nil, err
+		}
+	}
+
+	return users, nil
+}
+
+// GetRepoReadersFromIDs returns the users who has explicit read access or higher to the repository based on given userIDs.
+func GetRepoReadersFromIDs(ctx context.Context, repo *repo_model.Repository, userIDs []int64) (_ []*user_model.User, err error) {
+	return getUsersWithAccessModeFromIDs(ctx, repo, perm_model.AccessModeRead, userIDs)
+}
+
 // GetRepoWriters returns all users that have write access to the repository.
 func GetRepoWriters(ctx context.Context, repo *repo_model.Repository) (_ []*user_model.User, err error) {
 	return getUsersWithAccessMode(ctx, repo, perm_model.AccessModeWrite)

routers/api/v1/repo/branch.go

@@ -437,7 +437,7 @@ func GetBranchProtection(ctx *context.APIContext) {
 		return
 	}
 
-	ctx.JSON(http.StatusOK, convert.ToBranchProtection(ctx, bp))
+	ctx.JSON(http.StatusOK, convert.ToBranchProtection(ctx, bp, repo))
 }
 
 // ListBranchProtections list branch protections for a repo
@@ -470,7 +470,7 @@ func ListBranchProtections(ctx *context.APIContext) {
 	}
 	apiBps := make([]*api.BranchProtection, len(bps))
 	for i := range bps {
-		apiBps[i] = convert.ToBranchProtection(ctx, bps[i])
+		apiBps[i] = convert.ToBranchProtection(ctx, bps[i], repo)
 	}
 
 	ctx.JSON(http.StatusOK, apiBps)
@@ -681,7 +681,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
 		return
 	}
 
-	ctx.JSON(http.StatusCreated, convert.ToBranchProtection(ctx, bp))
+	ctx.JSON(http.StatusCreated, convert.ToBranchProtection(ctx, bp, repo))
 }
 
 // EditBranchProtection edits a branch protection for a repo
@@ -959,7 +959,7 @@ func EditBranchProtection(ctx *context.APIContext) {
 		return
 	}
 
-	ctx.JSON(http.StatusOK, convert.ToBranchProtection(ctx, bp))
+	ctx.JSON(http.StatusOK, convert.ToBranchProtection(ctx, bp, repo))
 }
 
 // DeleteBranchProtection deletes a branch protection for a repo

services/convert/convert.go

@@ -106,30 +106,59 @@ func ToBranch(ctx context.Context, repo *repo_model.Repository, branchName strin
 }
 
 // ToBranchProtection convert a ProtectedBranch to api.BranchProtection
-func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch) *api.BranchProtection {
-	pushWhitelistUsernames, err := user_model.GetUserNamesByIDs(ctx, bp.WhitelistUserIDs)
+func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo *repo_model.Repository) *api.BranchProtection {
+	pushWhitelistUsernamesRaw, err := access_model.GetRepoReadersFromIDs(ctx, repo, bp.WhitelistUserIDs)
 	if err != nil {
-		log.Error("GetUserNamesByIDs (WhitelistUserIDs): %v", err)
+		log.Error("GetRepoReadersFromIDs (WhitelistUserIDs): %v", err)
 	}
-	mergeWhitelistUsernames, err := user_model.GetUserNamesByIDs(ctx, bp.MergeWhitelistUserIDs)
+	var pushWhitelistUsernames []string
+	for _, username := range pushWhitelistUsernamesRaw {
+		pushWhitelistUsernames = append(pushWhitelistUsernames, username.Name)
+	}
+
+	mergeWhitelistUsernamesRaw, err := access_model.GetRepoReadersFromIDs(ctx, repo, bp.MergeWhitelistUserIDs)
 	if err != nil {
-		log.Error("GetUserNamesByIDs (MergeWhitelistUserIDs): %v", err)
+		log.Error("GetRepoReadersFromIDs (MergeWhitelistUserIDs): %v", err)
+	}
+	var mergeWhitelistUsernames []string
+	for _, username := range mergeWhitelistUsernamesRaw {
+		mergeWhitelistUsernames = append(mergeWhitelistUsernames, username.Name)
 	}
-	approvalsWhitelistUsernames, err := user_model.GetUserNamesByIDs(ctx, bp.ApprovalsWhitelistUserIDs)
+
+	approvalsWhitelistUsernamesRaw, err := access_model.GetRepoReadersFromIDs(ctx, repo, bp.ApprovalsWhitelistUserIDs)
 	if err != nil {
-		log.Error("GetUserNamesByIDs (ApprovalsWhitelistUserIDs): %v", err)
+		log.Error("GetRepoReadersFromIDs (ApprovalsWhitelistUserIDs): %v", err)
+	}
+	var approvalsWhitelistUsernames []string
+	for _, username := range approvalsWhitelistUsernamesRaw {
+		approvalsWhitelistUsernames = append(approvalsWhitelistUsernames, username.Name)
 	}
-	pushWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.WhitelistTeamIDs)
+
+	pushWhitelistTeamsRaw, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepoFromIDs(ctx, repo.ID, perm.AccessModeRead, bp.WhitelistTeamIDs)
 	if err != nil {
-		log.Error("GetTeamNamesByID (WhitelistTeamIDs): %v", err)
+		log.Error("TeamsWithAccessToRepoFromIDs (WhitelistTeamIDs): %v", err)
 	}
-	mergeWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.MergeWhitelistTeamIDs)
+	var pushWhitelistTeams []string
+	for _, team := range pushWhitelistTeamsRaw {
+		pushWhitelistTeams = append(pushWhitelistTeams, team.Name)
+	}
+
+	mergeWhitelistTeamsRaw, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepoFromIDs(ctx, repo.ID, perm.AccessModeRead, bp.MergeWhitelistTeamIDs)
 	if err != nil {
-		log.Error("GetTeamNamesByID (MergeWhitelistTeamIDs): %v", err)
+		log.Error("TeamsWithAccessToRepoFromIDs (MergeWhitelistTeamIDs): %v", err)
+	}
+	var mergeWhitelistTeams []string
+	for _, team := range mergeWhitelistTeamsRaw {
+		mergeWhitelistTeams = append(mergeWhitelistTeams, team.Name)
 	}
-	approvalsWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.ApprovalsWhitelistTeamIDs)
+
+	approvalsWhitelistTeamsRaw, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepoFromIDs(ctx, repo.ID, perm.AccessModeRead, bp.ApprovalsWhitelistTeamIDs)
 	if err != nil {
-		log.Error("GetTeamNamesByID (ApprovalsWhitelistTeamIDs): %v", err)
+		log.Error("TeamsWithAccessToRepoFromIDs (ApprovalsWhitelistTeamIDs): %v", err)
+	}
+	var approvalsWhitelistTeams []string
+	for _, team := range approvalsWhitelistTeamsRaw {
+		approvalsWhitelistTeams = append(approvalsWhitelistTeams, team.Name)
 	}
 
 	branchName := ""