Fix SQL Query for `SearchTeam` (#20844) (#20872)

Backport #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.
 
Fix  #20829
tokarchuk/v1.17
Gusted 2 years ago committed by GitHub
parent 17d3a474e0
commit e6ec411491
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      integrations/api_team_test.go
  2. 22
      integrations/api_user_orgs_test.go
  3. 10
      integrations/org_test.go
  4. 6
      models/fixtures/org_user.yml
  5. 2
      models/fixtures/user.yml
  6. 36
      models/organization/team.go
  7. 2
      routers/web/org/teams.go

@ -223,7 +223,7 @@ func TestAPITeamSearch(t *testing.T) {
defer prepareTestEnv(t)() defer prepareTestEnv(t)()
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User) 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 var results TeamSearchResults

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

@ -179,8 +179,8 @@ func TestOrgRestrictedUser(t *testing.T) {
func TestTeamSearch(t *testing.T) { func TestTeamSearch(t *testing.T) {
defer prepareTestEnv(t)() defer prepareTestEnv(t)()
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 15}).(*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 var results TeamSearchResults
@ -190,9 +190,9 @@ func TestTeamSearch(t *testing.T) {
req.Header.Add("X-Csrf-Token", csrf) req.Header.Add("X-Csrf-Token", csrf)
resp := session.MakeRequest(t, req, http.StatusOK) resp := session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &results) DecodeJSON(t, resp, &results)
assert.NotEmpty(t, results.Data) assert.Len(t, results.Data, 2)
assert.Len(t, results.Data, 1) assert.Equal(t, "review_team", results.Data[0].Name)
assert.Equal(t, "test_team", results.Data[0].Name) assert.Equal(t, "test_team", results.Data[1].Name)
// no access if not organization member // no access if not organization member
user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}).(*user_model.User) user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}).(*user_model.User)

@ -63,3 +63,9 @@
uid: 29 uid: 29
org_id: 17 org_id: 17
is_public: true is_public: true
-
id: 12
uid: 2
org_id: 17
is_public: true

@ -309,7 +309,7 @@
avatar_email: user17@example.com avatar_email: user17@example.com
num_repos: 2 num_repos: 2
is_active: true is_active: true
num_members: 3 num_members: 4
num_teams: 3 num_teams: 3
- -

@ -96,16 +96,7 @@ type SearchTeamOptions struct {
IncludeDesc bool IncludeDesc bool
} }
// SearchTeam search for teams. Caller is responsible to check permissions. func (opts *SearchTeamOptions) toCond() builder.Cond {
func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
if opts.Page <= 0 {
opts.Page = 1
}
if opts.PageSize == 0 {
// Default limit
opts.PageSize = 10
}
cond := builder.NewCond() cond := builder.NewCond()
if len(opts.Keyword) > 0 { if len(opts.Keyword) > 0 {
@ -117,10 +108,28 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
cond = cond.And(keywordCond) 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) 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. count, err := sess.
Where(cond). Where(cond).
Count(new(Team)) Count(new(Team))
@ -128,6 +137,10 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
return nil, 0, err return nil, 0, err
} }
if opts.UserID > 0 {
sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id")
}
sess = sess.Where(cond) sess = sess.Where(cond)
if opts.PageSize == -1 { if opts.PageSize == -1 {
opts.PageSize = int(count) opts.PageSize = int(count)
@ -137,6 +150,7 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
teams := make([]*Team, 0, opts.PageSize) teams := make([]*Team, 0, opts.PageSize)
if err = sess. if err = sess.
Where(cond).
OrderBy("lower_name"). OrderBy("lower_name").
Find(&teams); err != nil { Find(&teams); err != nil {
return nil, 0, err return nil, 0, err

@ -339,7 +339,7 @@ func SearchTeam(ctx *context.Context) {
} }
opts := &organization.SearchTeamOptions{ 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"), Keyword: ctx.FormTrim("q"),
OrgID: ctx.Org.Organization.ID, OrgID: ctx.Org.Organization.ID,
IncludeDesc: ctx.FormString("include_desc") == "" || ctx.FormBool("include_desc"), IncludeDesc: ctx.FormString("include_desc") == "" || ctx.FormBool("include_desc"),

Loading…
Cancel
Save