From b3a6596b54bdfa75b9f82c4d73185e676c32727f Mon Sep 17 00:00:00 2001 From: Edward Zhang <45360012+edwardzhanged@users.noreply.github.com> Date: Wed, 17 Apr 2024 21:24:07 +0800 Subject: [PATCH] Fix branch_protection api shows users/teams who has no readAccess (#30291) 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: techknowlogick Co-authored-by: wenzhuo.zhang Co-authored-by: Giteabot (cherry picked from commit 02e183bf3fa502b7cef76e8dcdbf01b85ce641f0) --- routers/api/v1/repo/branch.go | 8 ++--- services/convert/convert.go | 56 ++++++++++++++++++++++------------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index c33beee0ae..852b7a2ee0 100644 --- 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) @@ -682,7 +682,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 @@ -964,7 +964,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 5b06b9b1a0..55996d8fe3 100644 --- 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 } +// 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) + } + } + } + + return whitelistNames +} + // 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 { + readers, err := access_model.GetRepoReaders(ctx, repo) if err != nil { - log.Error("GetUserNamesByIDs (WhitelistUserIDs): %v", err) + log.Error("GetRepoReaders: %v", err) } - mergeWhitelistUsernames, err := user_model.GetUserNamesByIDs(ctx, bp.MergeWhitelistUserIDs) + + 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("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) - } - mergeWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.MergeWhitelistTeamIDs) - if err != nil { - log.Error("GetTeamNamesByID (MergeWhitelistTeamIDs): %v", err) - } - approvalsWhitelistTeams, err := organization.GetTeamNamesByID(ctx, bp.ApprovalsWhitelistTeamIDs) - 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