Fix SQL Query for `SearchTeam` (#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

Co-authored-by: delvh <dev.lh@web.de>
This commit is contained in:
Gusted 2022-08-21 18:24:05 +02:00 committed by GitHub
parent 6d3181406d
commit 0b4c166e8a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 61 additions and 19 deletions

View File

@ -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 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 17})
var results TeamSearchResults var results TeamSearchResults

View File

@ -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"}) user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user3"})
user17 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user17"})
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"}) user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user3"})
user17 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user17"})
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,

View File

@ -197,8 +197,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 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 15})
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 17})
var results TeamSearchResults var results TeamSearchResults
@ -209,8 +209,9 @@ func TestTeamSearch(t *testing.T) {
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.NotEmpty(t, results.Data)
assert.Len(t, results.Data, 1) assert.Len(t, results.Data, 2)
assert.Equal(t, "test_team", results.Data[0].Name) assert.Equal(t, "review_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}) user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})

View File

@ -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

View File

@ -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
- -

View File

@ -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,7 +137,10 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
return nil, 0, err return nil, 0, err
} }
sess = sess.Where(cond) if opts.UserID > 0 {
sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id")
}
if opts.PageSize == -1 { if opts.PageSize == -1 {
opts.PageSize = int(count) opts.PageSize = int(count)
} else { } else {
@ -137,6 +149,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

View File

@ -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"),