Skip to content

Commit 564e3db

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 29b2800 commit 564e3db

File tree

10 files changed

+63
-28
lines changed

10 files changed

+63
-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: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package organization
55

66
import (
77
"context"
8+
"xorm.io/builder"
89

910
"code.gitea.io/gitea/models/db"
1011
"code.gitea.io/gitea/models/perm"
@@ -48,26 +49,26 @@ func RemoveTeamRepo(ctx context.Context, teamID, repoID int64) error {
4849
return err
4950
}
5051

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) {
52+
// GetTeamsWithAccessToAnyRepoUnit returns all teams in an organization that have given access level to the repository special unit.
53+
func GetTeamsWithAccessToAnyRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type, unitTypesMore ...unit.Type) ([]*Team, error) {
5354
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-
}
6155

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).
56+
sub := builder.Select("team_id").From("team_unit").
57+
Where(builder.Expr("team_unit.team_id = team.id")).
58+
And(builder.In("team_unit.type", append([]unit.Type{unitType}, unitTypesMore...))).
59+
And(builder.Expr("team_unit.access_mode >= ?", mode))
60+
61+
// FIXME: maybe it should also check "team.includes_all_repositories = true" in the future
62+
err := db.GetEngine(ctx).
6663
Join("INNER", "team_repo", "team_repo.team_id = team.id").
67-
Join("INNER", "team_unit", "team_unit.team_id = team.id").
6864
And("team_repo.org_id = ?", orgID).
6965
And("team_repo.repo_id = ?", repoID).
70-
And("team_unit.type = ?", unitType).
66+
And(builder.Or(
67+
builder.Expr("team.authorize >= ?", mode),
68+
builder.In("team.id", sub),
69+
)).
7170
OrderBy("name").
7271
Find(&teams)
72+
73+
return teams, err
7374
}

‎models/organization/team_repo_test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
package organization_test
55

66
import (
7+
"code.gitea.io/gitea/services/org"
8+
"github.com/stretchr/testify/require"
9+
testing2 "gitlab.com/gitlab-org/api/client-go/testing"
710
"testing"
811

912
"code.gitea.io/gitea/models/db"
@@ -22,10 +25,11 @@ func TestGetTeamsWithAccessToRepoUnit(t *testing.T) {
2225
org41 := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 41})
2326
repo61 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 61})
2427

25-
teams, err := organization.GetTeamsWithAccessToRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests)
28+
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests)
2629
assert.NoError(t, err)
2730
if assert.Len(t, teams, 2) {
2831
assert.EqualValues(t, 21, teams[0].ID)
2932
assert.EqualValues(t, 22, teams[1].ID)
3033
}
34+
3135
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package setting
55

66
import (
7+
"code.gitea.io/gitea/models/unit"
78
"errors"
89
"fmt"
910
"net/http"
@@ -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
@@ -4,6 +4,7 @@
44
package setting
55

66
import (
7+
"code.gitea.io/gitea/models/unit"
78
"fmt"
89
"net/http"
910
"strings"
@@ -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
@@ -4,7 +4,12 @@
44
package integration
55

66
import (
7+
"code.gitea.io/gitea/models/db"
8+
"code.gitea.io/gitea/models/organization"
9+
"code.gitea.io/gitea/models/perm"
10+
"code.gitea.io/gitea/models/unit"
711
"fmt"
12+
"github.com/stretchr/testify/require"
813
"net/http"
914
"strings"
1015
"testing"
@@ -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+
var testOrgID int64 = 500
229+
var testTeamID int64 = 1000
230+
var testRepoID int64 = 2000
231+
require.NoError(t, db.Insert(ctx, &organization.Team{ID: testTeamID, OrgID: testOrgID, LowerName: "test_team", AccessMode: perm.AccessModeNone}))
232+
require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: testOrgID, TeamID: testTeamID, RepoID: testRepoID}))
233+
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: testOrgID, TeamID: testTeamID, Type: unit.TypeCode, AccessMode: perm.AccessModeRead}))
234+
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: testOrgID, TeamID: testTeamID, 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"
239+
240+
teams, err = organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeWrite, unit.TypeCode)
241+
require.NoError(t, err)
242+
assert.Len(t, teams, 0) // 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(testTeamID).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)