Fix branch_protection api shows users/teams who has no readAccess (#30291) (#30544)

Backport #30291 by @edwardzhanged

Add some logic in `convert.ToBranchProtection` to return only the names
associated with readAccess instead of returning all names. This will
ensure consistency in behavior between the frontend and backend.
Fixes: #27694
Co-authored-by: Edward Zhang <45360012+edwardzhanged@users.noreply.github.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.com>
Co-authored-by: wenzhuo.zhang <wenzhuo.zhang@geely.com>

diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go
index 5e6b6a86586bcedd7b990a52d31a5c7451d1299b..baab486e522fc70c00134007498b28efb2d07051 100644 (file)
--- a/routers/api/v1/repo/branch.go
+++ b/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

diff --git a/services/convert/convert.go b/services/convert/convert.go
index ca3ec32a4042f193622066e441ca6d0b5a8b3fe9..5df0303646b1cfdc1ff8f92804cde7de36f00dd3 100644 (file)
--- a/services/convert/convert.go
+++ b/services/convert/convert.go
@@ -21,6 +21,7 @@ import (
        repo_model "code.gitea.io/gitea/models/repo"
        "code.gitea.io/gitea/models/unit"
        user_model "code.gitea.io/gitea/models/user"
+       "code.gitea.io/gitea/modules/container"
        "code.gitea.io/gitea/modules/git"
        "code.gitea.io/gitea/modules/log"
        api "code.gitea.io/gitea/modules/structs"
@@ -105,33 +106,46 @@ func ToBranch(ctx context.Context, repo *repo_model.Repository, branchName strin
        return branch, nil
 }
 
-// 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)
-       if err != nil {
-               log.Error("GetUserNamesByIDs (WhitelistUserIDs): %v", err)
-       }
-       mergeWhitelistUsernames, err := user_model.GetUserNamesByIDs(ctx, bp.MergeWhitelistUserIDs)
-       if err != nil {
-               log.Error("GetUserNamesByIDs (MergeWhitelistUserIDs): %v", err)
-       }
-       approvalsWhitelistUsernames, err := user_model.GetUserNamesByIDs(ctx, bp.ApprovalsWhitelistUserIDs)
-       if err != nil {
-               log.Error("GetUserNamesByIDs (ApprovalsWhitelistUserIDs): %v", err)
-       }
-       pushWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.WhitelistTeamIDs)
-       if err != nil {
-               log.Error("GetTeamNamesByID (WhitelistTeamIDs): %v", err)
+// getWhitelistEntities returns the names of the entities that are in the whitelist
+func getWhitelistEntities[T *user_model.User | *organization.Team](entities []T, whitelistIDs []int64) []string {
+       whitelistUserIDsSet := container.SetOf(whitelistIDs...)
+       whitelistNames := make([]string, 0)
+       for _, entity := range entities {
+               switch v := any(entity).(type) {
+               case *user_model.User:
+                       if whitelistUserIDsSet.Contains(v.ID) {
+                               whitelistNames = append(whitelistNames, v.Name)
+                       }
+               case *organization.Team:
+                       if whitelistUserIDsSet.Contains(v.ID) {
+                               whitelistNames = append(whitelistNames, v.Name)
+                       }
+               }
        }
-       mergeWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.MergeWhitelistTeamIDs)
+
+       return whitelistNames
+}
+
+// ToBranchProtection convert a ProtectedBranch to api.BranchProtection
+func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo *repo_model.Repository) *api.BranchProtection {
+       readers, err := access_model.GetRepoReaders(ctx, repo)
        if err != nil {
-               log.Error("GetTeamNamesByID (MergeWhitelistTeamIDs): %v", err)
+               log.Error("GetRepoReaders: %v", err)
        }
-       approvalsWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.ApprovalsWhitelistTeamIDs)
+
+       pushWhitelistUsernames := getWhitelistEntities(readers, bp.WhitelistUserIDs)
+       mergeWhitelistUsernames := getWhitelistEntities(readers, bp.MergeWhitelistUserIDs)
+       approvalsWhitelistUsernames := getWhitelistEntities(readers, bp.ApprovalsWhitelistUserIDs)
+
+       teamReaders, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepo(ctx, repo.ID, perm.AccessModeRead)
        if err != nil {
-               log.Error("GetTeamNamesByID (ApprovalsWhitelistTeamIDs): %v", err)
+               log.Error("Repo.Owner.TeamsWithAccessToRepo: %v", err)
        }
 
+       pushWhitelistTeams := getWhitelistEntities(teamReaders, bp.WhitelistTeamIDs)
+       mergeWhitelistTeams := getWhitelistEntities(teamReaders, bp.MergeWhitelistTeamIDs)
+       approvalsWhitelistTeams := getWhitelistEntities(teamReaders, bp.ApprovalsWhitelistTeamIDs)
+
        branchName := ""
        if !git_model.IsRuleNameSpecial(bp.RuleName) {
                branchName = bp.RuleName