Skip to content

Commit

Permalink
Fix SQL Query for SearchTeam (go-gitea#20844)
Browse files Browse the repository at this point in the history
- Backport of go-gitea#20844
  - Currently the function takes in the UserID option, but isn't being used within the SQL query. This patch fixes that by checking that only teams are being returned that the user belongs to.
  - Resolves go-gitea#20829
  • Loading branch information
Gusted committed Aug 20, 2022
1 parent b88a4b4 commit e7a5f26
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 19 deletions.
2 changes: 1 addition & 1 deletion integrations/api_team_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func TestAPITeamSearch(t *testing.T) {
defer prepareTestEnv(t)()

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User)
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}).(*user_model.User)
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 17}).(*user_model.User)

var results TeamSearchResults

Expand Down
22 changes: 22 additions & 0 deletions integrations/api_user_orgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,19 @@ func TestUserOrgs(t *testing.T) {
orgs := getUserOrgs(t, adminUsername, normalUsername)

user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user3"}).(*user_model.User)
user17 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user17"})

assert.Equal(t, []*api.Organization{
{
ID: 17,
UserName: user17.Name,
FullName: user17.FullName,
AvatarURL: user17.AvatarLink(),
Description: "",
Website: "",
Location: "",
Visibility: "public",
},
{
ID: 3,
UserName: user3.Name,
Expand Down Expand Up @@ -82,8 +93,19 @@ func TestMyOrgs(t *testing.T) {
var orgs []*api.Organization
DecodeJSON(t, resp, &orgs)
user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user3"}).(*user_model.User)
user17 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user17"})

assert.Equal(t, []*api.Organization{
{
ID: 17,
UserName: user17.Name,
FullName: user17.FullName,
AvatarURL: user17.AvatarLink(),
Description: "",
Website: "",
Location: "",
Visibility: "public",
},
{
ID: 3,
UserName: user3.Name,
Expand Down
10 changes: 5 additions & 5 deletions integrations/org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ func TestOrgRestrictedUser(t *testing.T) {
func TestTeamSearch(t *testing.T) {
defer prepareTestEnv(t)()

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User)
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}).(*user_model.User)
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 15}).(*user_model.User)
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 17}).(*user_model.User)

var results TeamSearchResults

Expand All @@ -190,9 +190,9 @@ func TestTeamSearch(t *testing.T) {
req.Header.Add("X-Csrf-Token", csrf)
resp := session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &results)
assert.NotEmpty(t, results.Data)
assert.Len(t, results.Data, 1)
assert.Equal(t, "test_team", results.Data[0].Name)
assert.Len(t, results.Data, 2)
assert.Equal(t, "review_team", results.Data[0].Name)
assert.Equal(t, "test_team", results.Data[1].Name)

// no access if not organization member
user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}).(*user_model.User)
Expand Down
6 changes: 6 additions & 0 deletions models/fixtures/org_user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,9 @@
uid: 29
org_id: 17
is_public: true

-
id: 12
uid: 2
org_id: 17
is_public: true
2 changes: 1 addition & 1 deletion models/fixtures/user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@
avatar_email: user17@example.com
num_repos: 2
is_active: true
num_members: 3
num_members: 4
num_teams: 3

-
Expand Down
36 changes: 25 additions & 11 deletions models/organization/team.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,7 @@ type SearchTeamOptions struct {
IncludeDesc bool
}

// SearchTeam search for teams. Caller is responsible to check permissions.
func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
if opts.Page <= 0 {
opts.Page = 1
}
if opts.PageSize == 0 {
// Default limit
opts.PageSize = 10
}

func (opts *SearchTeamOptions) toCond() builder.Cond {
cond := builder.NewCond()

if len(opts.Keyword) > 0 {
Expand All @@ -117,17 +108,39 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
cond = cond.And(keywordCond)
}

cond = cond.And(builder.Eq{"org_id": opts.OrgID})
if opts.OrgID > 0 {
cond = cond.And(builder.Eq{"`team`.org_id": opts.OrgID})
}

if opts.UserID > 0 {
cond = cond.And(builder.Eq{"team_user.uid": opts.UserID})
}

return cond
}

// SearchTeam search for teams. Caller is responsible to check permissions.
func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
sess := db.GetEngine(db.DefaultContext)

opts.SetDefaultValues()
cond := opts.toCond()

if opts.UserID > 0 {
sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id")
}

count, err := sess.
Where(cond).
Count(new(Team))
if err != nil {
return nil, 0, err
}

if opts.UserID > 0 {
sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id")
}

sess = sess.Where(cond)
if opts.PageSize == -1 {
opts.PageSize = int(count)
Expand All @@ -137,6 +150,7 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {

teams := make([]*Team, 0, opts.PageSize)
if err = sess.
Where(cond).
OrderBy("lower_name").
Find(&teams); err != nil {
return nil, 0, err
Expand Down
2 changes: 1 addition & 1 deletion routers/web/org/teams.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func SearchTeam(ctx *context.Context) {
}

opts := &organization.SearchTeamOptions{
UserID: ctx.Doer.ID,
// UserID is not set because the router already requires the doer to be an org admin. Thus, we don't need to restrict to teams that the user belongs in
Keyword: ctx.FormTrim("q"),
OrgID: ctx.Org.Organization.ID,
IncludeDesc: ctx.FormString("include_desc") == "" || ctx.FormBool("include_desc"),
Expand Down

0 comments on commit e7a5f26

Please sign in to comment.