Skip to content

Commit f788a8d

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 29b2800 commit f788a8d

File tree

10 files changed

+61
-28
lines changed

10 files changed

+61
-28
lines changed

‎models/git/protected_branch.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ func updateTeamWhitelist(ctx context.Context, repo *repo_model.Repository, curre
518518
return currentWhitelist, nil
519519
}
520520

521-
teams, err := organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead)
521+
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests)
522522
if err != nil {
523523
return nil, fmt.Errorf("GetTeamsWithAccessToRepo [org_id: %d, repo_id: %d]: %v", repo.OwnerID, repo.ID, err)
524524
}

‎models/organization/org.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -602,8 +602,3 @@ func getUserTeamIDsQueryBuilder(orgID, userID int64) *builder.Builder {
602602
"team_user.uid": userID,
603603
})
604604
}
605-
606-
// TeamsWithAccessToRepo returns all teams that have given access level to the repository.
607-
func (org *Organization) TeamsWithAccessToRepo(ctx context.Context, repoID int64, mode perm.AccessMode) ([]*Team, error) {
608-
return GetTeamsWithAccessToRepo(ctx, org.ID, repoID, mode)
609-
}

‎models/organization/team_repo.go

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"code.gitea.io/gitea/models/db"
1010
"code.gitea.io/gitea/models/perm"
1111
"code.gitea.io/gitea/models/unit"
12+
13+
"xorm.io/builder"
1214
)
1315

1416
// TeamRepo represents an team-repository relation.
@@ -48,26 +50,27 @@ func RemoveTeamRepo(ctx context.Context, teamID, repoID int64) error {
4850
return err
4951
}
5052

51-
// GetTeamsWithAccessToRepo returns all teams in an organization that have given access level to the repository.
52-
func GetTeamsWithAccessToRepo(ctx context.Context, orgID, repoID int64, mode perm.AccessMode) ([]*Team, error) {
53+
// GetTeamsWithAccessToAnyRepoUnit returns all teams in an organization that have given access level to the repository special unit.
54+
// 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.
55+
func GetTeamsWithAccessToAnyRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type, unitTypesMore ...unit.Type) ([]*Team, error) {
5356
teams := make([]*Team, 0, 5)
54-
return teams, db.GetEngine(ctx).Where("team.authorize >= ?", mode).
55-
Join("INNER", "team_repo", "team_repo.team_id = team.id").
56-
And("team_repo.org_id = ?", orgID).
57-
And("team_repo.repo_id = ?", repoID).
58-
OrderBy("name").
59-
Find(&teams)
60-
}
6157

62-
// GetTeamsWithAccessToRepoUnit returns all teams in an organization that have given access level to the repository special unit.
63-
func GetTeamsWithAccessToRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type) ([]*Team, error) {
64-
teams := make([]*Team, 0, 5)
65-
return teams, db.GetEngine(ctx).Where("team_unit.access_mode >= ?", mode).
58+
sub := builder.Select("team_id").From("team_unit").
59+
Where(builder.Expr("team_unit.team_id = team.id")).
60+
And(builder.In("team_unit.type", append([]unit.Type{unitType}, unitTypesMore...))).
61+
And(builder.Expr("team_unit.access_mode >= ?", mode))
62+
63+
// FIXME: maybe it should also check "team.includes_all_repositories = true" in the future
64+
err := db.GetEngine(ctx).
6665
Join("INNER", "team_repo", "team_repo.team_id = team.id").
67-
Join("INNER", "team_unit", "team_unit.team_id = team.id").
6866
And("team_repo.org_id = ?", orgID).
6967
And("team_repo.repo_id = ?", repoID).
70-
And("team_unit.type = ?", unitType).
68+
And(builder.Or(
69+
builder.Expr("team.authorize >= ?", mode),
70+
builder.In("team.id", sub),
71+
)).
7172
OrderBy("name").
7273
Find(&teams)
74+
75+
return teams, err
7376
}

‎models/organization/team_repo_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestGetTeamsWithAccessToRepoUnit(t *testing.T) {
2222
org41 := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 41})
2323
repo61 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 61})
2424

25-
teams, err := organization.GetTeamsWithAccessToRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests)
25+
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests)
2626
assert.NoError(t, err)
2727
if assert.Len(t, teams, 2) {
2828
assert.EqualValues(t, 21, teams[0].ID)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"code.gitea.io/gitea/models/perm"
1818
access_model "code.gitea.io/gitea/models/perm/access"
1919
repo_model "code.gitea.io/gitea/models/repo"
20+
"code.gitea.io/gitea/models/unit"
2021
"code.gitea.io/gitea/modules/base"
2122
"code.gitea.io/gitea/modules/templates"
2223
"code.gitea.io/gitea/modules/web"
@@ -89,7 +90,7 @@ func SettingsProtectedBranch(c *context.Context) {
8990
c.Data["recent_status_checks"] = contexts
9091

9192
if c.Repo.Owner.IsOrganization() {
92-
teams, err := organization.OrgFromUser(c.Repo.Owner).TeamsWithAccessToRepo(c, c.Repo.Repository.ID, perm.AccessModeRead)
93+
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(c, c.Repo.Owner.ID, c.Repo.Repository.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests)
9394
if err != nil {
9495
c.ServerError("Repo.Owner.TeamsWithAccessToRepo", err)
9596
return

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"code.gitea.io/gitea/models/organization"
1313
"code.gitea.io/gitea/models/perm"
1414
access_model "code.gitea.io/gitea/models/perm/access"
15+
"code.gitea.io/gitea/models/unit"
1516
"code.gitea.io/gitea/modules/base"
1617
"code.gitea.io/gitea/modules/setting"
1718
"code.gitea.io/gitea/modules/templates"
@@ -156,7 +157,7 @@ func setTagsContext(ctx *context.Context) error {
156157
ctx.Data["Users"] = users
157158

158159
if ctx.Repo.Owner.IsOrganization() {
159-
teams, err := organization.OrgFromUser(ctx.Repo.Owner).TeamsWithAccessToRepo(ctx, ctx.Repo.Repository.ID, perm.AccessModeRead)
160+
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, ctx.Repo.Owner.ID, ctx.Repo.Repository.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests)
160161
if err != nil {
161162
ctx.ServerError("Repo.Owner.TeamsWithAccessToRepo", err)
162163
return err

‎services/convert/convert.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo
149149
mergeWhitelistUsernames := getWhitelistEntities(readers, bp.MergeWhitelistUserIDs)
150150
approvalsWhitelistUsernames := getWhitelistEntities(readers, bp.ApprovalsWhitelistUserIDs)
151151

152-
teamReaders, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepo(ctx, repo.ID, perm.AccessModeRead)
152+
teamReaders, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.Owner.ID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests)
153153
if err != nil {
154154
log.Error("Repo.Owner.TeamsWithAccessToRepo: %v", err)
155155
}
@@ -727,7 +727,7 @@ func ToTagProtection(ctx context.Context, pt *git_model.ProtectedTag, repo *repo
727727

728728
whitelistUsernames := getWhitelistEntities(readers, pt.AllowlistUserIDs)
729729

730-
teamReaders, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepo(ctx, repo.ID, perm.AccessModeRead)
730+
teamReaders, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.Owner.ID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests)
731731
if err != nil {
732732
log.Error("Repo.Owner.TeamsWithAccessToRepo: %v", err)
733733
}

‎services/issue/assignee.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, rep
304304

305305
// If the repo's owner is an organization, members of teams with read permission on pull requests can change reviewers
306306
if repo.Owner.IsOrganization() {
307-
teams, err := organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead)
307+
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
308308
if err != nil {
309309
log.Error("GetTeamsWithAccessToRepo: %v", err)
310310
return false

‎services/pull/reviewer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,5 @@ func GetReviewerTeams(ctx context.Context, repo *repo_model.Repository) ([]*orga
8585
return nil, nil
8686
}
8787

88-
return organization.GetTeamsWithAccessToRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
88+
return organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
8989
}

‎tests/integration/org_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,17 @@ import (
1010
"testing"
1111

1212
auth_model "code.gitea.io/gitea/models/auth"
13+
"code.gitea.io/gitea/models/db"
14+
"code.gitea.io/gitea/models/organization"
15+
"code.gitea.io/gitea/models/perm"
16+
"code.gitea.io/gitea/models/unit"
1317
"code.gitea.io/gitea/models/unittest"
1418
user_model "code.gitea.io/gitea/models/user"
1519
api "code.gitea.io/gitea/modules/structs"
1620
"code.gitea.io/gitea/tests"
1721

1822
"github.com/stretchr/testify/assert"
23+
"github.com/stretchr/testify/require"
1924
)
2025

2126
func TestOrgRepos(t *testing.T) {
@@ -217,4 +222,32 @@ func TestTeamSearch(t *testing.T) {
217222
session = loginUser(t, user5.Name)
218223
req = NewRequestf(t, "GET", "/org/%s/teams/-/search?q=%s", org.Name, "team")
219224
session.MakeRequest(t, req, http.StatusNotFound)
225+
226+
t.Run("SearchWithPermission", func(t *testing.T) {
227+
ctx := t.Context()
228+
const testOrgID int64 = 500
229+
const testRepoID int64 = 2000
230+
testTeam := &organization.Team{OrgID: testOrgID, LowerName: "test_team", AccessMode: perm.AccessModeNone}
231+
require.NoError(t, db.Insert(ctx, testTeam))
232+
require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: testOrgID, TeamID: testTeam.ID, RepoID: testRepoID}))
233+
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: testOrgID, TeamID: testTeam.ID, Type: unit.TypeCode, AccessMode: perm.AccessModeRead}))
234+
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: testOrgID, TeamID: testTeam.ID, Type: unit.TypeIssues, AccessMode: perm.AccessModeWrite}))
235+
236+
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeRead, unit.TypeCode, unit.TypeIssues)
237+
require.NoError(t, err)
238+
assert.Len(t, teams, 1) // can read "code" or "issues"
239+
240+
teams, err = organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeWrite, unit.TypeCode)
241+
require.NoError(t, err)
242+
assert.Empty(t, teams) // cannot write "code"
243+
244+
teams, err = organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeWrite, unit.TypeIssues)
245+
require.NoError(t, err)
246+
assert.Len(t, teams, 1) // can write "issues"
247+
248+
_, _ = db.GetEngine(ctx).ID(testTeam.ID).Update(&organization.Team{AccessMode: perm.AccessModeWrite})
249+
teams, err = organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeWrite, unit.TypeCode)
250+
require.NoError(t, err)
251+
assert.Len(t, teams, 1) // team permission is "write", so can write "code"
252+
})
220253
}

0 commit comments

Comments
 (0)