Skip to content

Commit 3d909f8

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 0ce7d66 commit 3d909f8

File tree

7 files changed

+83
-56
lines changed

7 files changed

+83
-56
lines changed

‎models/git/protected_branch.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -466,11 +466,13 @@ func updateApprovalWhitelist(ctx context.Context, repo *repo_model.Repository, c
466466
return currentWhitelist, nil
467467
}
468468

469+
prUserIDs, err := access_model.GetUserIDsWithUnitAccess(ctx, repo, perm.AccessModeRead, unit.TypePullRequests)
470+
if err != nil {
471+
return nil, err
472+
}
469473
whitelist = make([]int64, 0, len(newWhitelist))
470474
for _, userID := range newWhitelist {
471-
if reader, err := access_model.IsRepoReader(ctx, repo, userID); err != nil {
472-
return nil, err
473-
} else if !reader {
475+
if !prUserIDs.Contains(userID) {
474476
continue
475477
}
476478
whitelist = append(whitelist, userID)

‎models/organization/team_repo.go‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,24 +53,45 @@ func RemoveTeamRepo(ctx context.Context, teamID, repoID int64) error {
5353
// GetTeamsWithAccessToAnyRepoUnit returns all teams in an organization that have given access level to the repository special unit.
5454
// This function is only used for finding some teams that can be used as branch protection allowlist or reviewers, it isn't really used for access control.
5555
// FIXME: TEAM-UNIT-PERMISSION this logic is not complete, search the fixme keyword to see more details
56-
func GetTeamsWithAccessToAnyRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type, unitTypesMore ...unit.Type) ([]*Team, error) {
57-
teams := make([]*Team, 0, 5)
56+
func GetTeamsWithAccessToAnyRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type, unitTypesMore ...unit.Type) (teams []*Team, err error) {
57+
teamIDs, err := getTeamIDsWithAccessToAnyRepoUnit(ctx, orgID, repoID, mode, unitType, unitTypesMore...)
58+
if err != nil {
59+
return nil, err
60+
}
61+
if len(teamIDs) == 0 {
62+
return teams, nil
63+
}
64+
err = db.GetEngine(ctx).Where(builder.In("id", teamIDs)).OrderBy("team.name").Find(&teams)
65+
return teams, err
66+
}
5867

68+
func getTeamIDsWithAccessToAnyRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type, unitTypesMore ...unit.Type) (teamIDs []int64, err error) {
5969
sub := builder.Select("team_id").From("team_unit").
6070
Where(builder.Expr("team_unit.team_id = team.id")).
6171
And(builder.In("team_unit.type", append([]unit.Type{unitType}, unitTypesMore...))).
6272
And(builder.Expr("team_unit.access_mode >= ?", mode))
6373

64-
err := db.GetEngine(ctx).
74+
err = db.GetEngine(ctx).
75+
Select("team.id").
76+
Table("team").
6577
Join("INNER", "team_repo", "team_repo.team_id = team.id").
66-
And("team_repo.org_id = ?", orgID).
67-
And("team_repo.repo_id = ?", repoID).
78+
And("team_repo.org_id = ? AND team_repo.repo_id = ?", orgID, repoID).
6879
And(builder.Or(
6980
builder.Expr("team.authorize >= ?", mode),
7081
builder.In("team.id", sub),
7182
)).
72-
OrderBy("name").
73-
Find(&teams)
83+
Find(&teamIDs)
84+
return teamIDs, err
85+
}
7486

75-
return teams, err
87+
func GetTeamUserIDsWithAccessToAnyRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type, unitTypesMore ...unit.Type) (userIDs []int64, err error) {
88+
teamIDs, err := getTeamIDsWithAccessToAnyRepoUnit(ctx, orgID, repoID, mode, unitType, unitTypesMore...)
89+
if err != nil {
90+
return nil, err
91+
}
92+
if len(teamIDs) == 0 {
93+
return userIDs, nil
94+
}
95+
err = db.GetEngine(ctx).Table("team_user").Select("uid").Where(builder.In("team_id", teamIDs)).Find(&userIDs)
96+
return userIDs, err
7697
}

‎models/perm/access/repo_permission.go‎

Lines changed: 28 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
repo_model "code.gitea.io/gitea/models/repo"
1717
"code.gitea.io/gitea/models/unit"
1818
user_model "code.gitea.io/gitea/models/user"
19+
"code.gitea.io/gitea/modules/container"
1920
"code.gitea.io/gitea/modules/log"
2021
"code.gitea.io/gitea/modules/setting"
2122
"code.gitea.io/gitea/modules/util"
@@ -498,54 +499,44 @@ func HasAnyUnitAccess(ctx context.Context, userID int64, repo *repo_model.Reposi
498499
return perm.HasAnyUnitAccess(), nil
499500
}
500501

501-
// getUsersWithAccessMode returns users that have at least given access mode to the repository.
502-
func getUsersWithAccessMode(ctx context.Context, repo *repo_model.Repository, mode perm_model.AccessMode) (_ []*user_model.User, err error) {
503-
if err = repo.LoadOwner(ctx); err != nil {
502+
func GetUsersWithUnitAccess(ctx context.Context, repo *repo_model.Repository, mode perm_model.AccessMode, unitType unit.Type) (users []*user_model.User, err error) {
503+
userIDs, err := GetUserIDsWithUnitAccess(ctx, repo, mode, unitType)
504+
if err != nil {
504505
return nil, err
505506
}
507+
if len(userIDs) == 0 {
508+
return users, nil
509+
}
510+
if err = db.GetEngine(ctx).In("id", userIDs.Values()).OrderBy("`name`").Find(&users); err != nil {
511+
return nil, err
512+
}
513+
return users, nil
514+
}
506515

516+
func GetUserIDsWithUnitAccess(ctx context.Context, repo *repo_model.Repository, mode perm_model.AccessMode, unitType unit.Type) (container.Set[int64], error) {
517+
userIDs := container.Set[int64]{}
507518
e := db.GetEngine(ctx)
508519
accesses := make([]*Access, 0, 10)
509-
if err = e.Where("repo_id = ? AND mode >= ?", repo.ID, mode).Find(&accesses); err != nil {
520+
if err := e.Where("repo_id = ? AND mode >= ?", repo.ID, mode).Find(&accesses); err != nil {
510521
return nil, err
511522
}
523+
for _, a := range accesses {
524+
userIDs.Add(a.UserID)
525+
}
512526

513-
// Leave a seat for owner itself to append later, but if owner is an organization
514-
// and just waste 1 unit is cheaper than re-allocate memory once.
515-
users := make([]*user_model.User, 0, len(accesses)+1)
516-
if len(accesses) > 0 {
517-
userIDs := make([]int64, len(accesses))
518-
for i := 0; i < len(accesses); i++ {
519-
userIDs[i] = accesses[i].UserID
520-
}
521-
522-
if err = e.In("id", userIDs).Find(&users); err != nil {
523-
return nil, err
524-
}
527+
if err := repo.LoadOwner(ctx); err != nil {
528+
return nil, err
525529
}
526530
if !repo.Owner.IsOrganization() {
527-
users = append(users, repo.Owner)
528-
}
529-
530-
return users, nil
531-
}
532-
533-
// GetRepoReaders returns all users that have explicit read access or higher to the repository.
534-
func GetRepoReaders(ctx context.Context, repo *repo_model.Repository) (_ []*user_model.User, err error) {
535-
return getUsersWithAccessMode(ctx, repo, perm_model.AccessModeRead)
536-
}
537-
538-
// GetRepoWriters returns all users that have write access to the repository.
539-
func GetRepoWriters(ctx context.Context, repo *repo_model.Repository) (_ []*user_model.User, err error) {
540-
return getUsersWithAccessMode(ctx, repo, perm_model.AccessModeWrite)
541-
}
542-
543-
// IsRepoReader returns true if user has explicit read access or higher to the repository.
544-
func IsRepoReader(ctx context.Context, repo *repo_model.Repository, userID int64) (bool, error) {
545-
if repo.OwnerID == userID {
546-
return true, nil
531+
userIDs.Add(repo.Owner.ID)
532+
} else {
533+
teamUserIDs, err := organization.GetTeamUserIDsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, mode, unitType)
534+
if err != nil {
535+
return nil, err
536+
}
537+
userIDs.AddMultiple(teamUserIDs...)
547538
}
548-
return db.GetEngine(ctx).Where("repo_id = ? AND user_id = ? AND mode >= ?", repo.ID, userID, perm_model.AccessModeRead).Get(&Access{})
539+
return userIDs, nil
549540
}
550541

551542
// CheckRepoUnitUser check whether user could visit the unit of this repository

‎models/perm/access/repo_permission_test.go‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,9 @@ func TestGetUserRepoPermission(t *testing.T) {
169169
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
170170
team := &organization.Team{OrgID: org.ID, LowerName: "test_team"}
171171
require.NoError(t, db.Insert(ctx, team))
172+
require.NoError(t, db.Insert(ctx, &organization.TeamUser{OrgID: org.ID, TeamID: team.ID, UID: user.ID}))
172173

173174
t.Run("DoerInTeamWithNoRepo", func(t *testing.T) {
174-
require.NoError(t, db.Insert(ctx, &organization.TeamUser{OrgID: org.ID, TeamID: team.ID, UID: user.ID}))
175175
perm, err := GetUserRepoPermission(ctx, repo32, user)
176176
require.NoError(t, err)
177177
assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode)
@@ -219,6 +219,15 @@ func TestGetUserRepoPermission(t *testing.T) {
219219
assert.Equal(t, perm_model.AccessModeNone, perm.AccessMode)
220220
assert.Equal(t, perm_model.AccessModeNone, perm.unitsMode[unit.TypeCode])
221221
assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues])
222+
223+
users, err := GetUsersWithUnitAccess(ctx, repo3, perm_model.AccessModeRead, unit.TypeIssues)
224+
require.NoError(t, err)
225+
require.Len(t, users, 1)
226+
assert.Equal(t, user.ID, users[0].ID)
227+
228+
users, err = GetUsersWithUnitAccess(ctx, repo3, perm_model.AccessModeWrite, unit.TypeIssues)
229+
require.NoError(t, err)
230+
require.Empty(t, users)
222231
})
223232

224233
require.NoError(t, db.Insert(ctx, repo_model.Collaboration{RepoID: repo3.ID, UserID: user.ID, Mode: perm_model.AccessModeWrite}))
@@ -229,5 +238,10 @@ func TestGetUserRepoPermission(t *testing.T) {
229238
assert.Equal(t, perm_model.AccessModeWrite, perm.AccessMode)
230239
assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeCode])
231240
assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeIssues])
241+
242+
users, err := GetUsersWithUnitAccess(ctx, repo3, perm_model.AccessModeWrite, unit.TypeIssues)
243+
require.NoError(t, err)
244+
require.Len(t, users, 1)
245+
assert.Equal(t, user.ID, users[0].ID)
232246
})
233247
}

‎routers/web/repo/setting/protected_branch.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,9 @@ func SettingsProtectedBranch(c *context.Context) {
7373

7474
c.Data["PageIsSettingsBranches"] = true
7575
c.Data["Title"] = c.Locale.TrString("repo.settings.protected_branch") + " - " + rule.RuleName
76-
77-
users, err := access_model.GetRepoReaders(c, c.Repo.Repository)
76+
users, err := access_model.GetUsersWithUnitAccess(c, c.Repo.Repository, perm.AccessModeRead, unit.TypePullRequests)
7877
if err != nil {
79-
c.ServerError("Repo.Repository.GetReaders", err)
78+
c.ServerError("GetUsersWithUnitAccess", err)
8079
return
8180
}
8281
c.Data["Users"] = users

‎routers/web/repo/setting/protected_tag.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,9 @@ func setTagsContext(ctx *context.Context) error {
149149
}
150150
ctx.Data["ProtectedTags"] = protectedTags
151151

152-
users, err := access_model.GetRepoReaders(ctx, ctx.Repo.Repository)
152+
users, err := access_model.GetUsersWithUnitAccess(ctx, ctx.Repo.Repository, perm.AccessModeRead, unit.TypePullRequests)
153153
if err != nil {
154-
ctx.ServerError("Repo.Repository.GetReaders", err)
154+
ctx.ServerError("GetUsersWithUnitAccess", err)
155155
return err
156156
}
157157
ctx.Data["Users"] = users

‎services/convert/convert.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func getWhitelistEntities[T *user_model.User | *organization.Team](entities []T,
139139

140140
// ToBranchProtection convert a ProtectedBranch to api.BranchProtection
141141
func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo *repo_model.Repository) *api.BranchProtection {
142-
readers, err := access_model.GetRepoReaders(ctx, repo)
142+
readers, err := access_model.GetUsersWithUnitAccess(ctx, repo, perm.AccessModeRead, unit.TypePullRequests)
143143
if err != nil {
144144
log.Error("GetRepoReaders: %v", err)
145145
}
@@ -720,7 +720,7 @@ func ToAnnotatedTagObject(repo *repo_model.Repository, commit *git.Commit) *api.
720720

721721
// ToTagProtection convert a git.ProtectedTag to an api.TagProtection
722722
func ToTagProtection(ctx context.Context, pt *git_model.ProtectedTag, repo *repo_model.Repository) *api.TagProtection {
723-
readers, err := access_model.GetRepoReaders(ctx, repo)
723+
readers, err := access_model.GetUsersWithUnitAccess(ctx, repo, perm.AccessModeRead, unit.TypePullRequests)
724724
if err != nil {
725725
log.Error("GetRepoReaders: %v", err)
726726
}

0 commit comments

Comments
 (0)