From 68478e908a084baa622e829ae18fd1ae6572a495 Mon Sep 17 00:00:00 2001 From: Kat Yang <69819079+yangkb09@users.noreply.github.com> Date: Wed, 20 Apr 2022 15:11:37 -0400 Subject: [PATCH] Chore: Remove x from team (#47905) * Chore: Remove x from team * Update pkg/services/sqlstore/team.go Co-authored-by: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> * Update pkg/services/sqlstore/team.go Co-authored-by: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> * Refactor dialects and add ISAdminOfTeams to Store * Add IsAdminOfTeams to mockstore Co-authored-by: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> --- pkg/middleware/auth.go | 2 +- pkg/services/sqlstore/mockstore/mockstore.go | 4 + pkg/services/sqlstore/store.go | 1 + pkg/services/sqlstore/team.go | 298 ++++++++++--------- pkg/services/sqlstore/team_test.go | 4 +- 5 files changed, 161 insertions(+), 148 deletions(-) diff --git a/pkg/middleware/auth.go b/pkg/middleware/auth.go index a3b2a2f2e89..a3b95627640 100644 --- a/pkg/middleware/auth.go +++ b/pkg/middleware/auth.go @@ -207,7 +207,7 @@ func OrgAdminFolderAdminOrTeamAdmin(ss sqlstore.Store) func(c *models.ReqContext } isAdminOfTeamsQuery := models.IsAdminOfTeamsQuery{SignedInUser: c.SignedInUser} - if err := sqlstore.IsAdminOfTeams(c.Req.Context(), &isAdminOfTeamsQuery); err != nil { + if err := ss.IsAdminOfTeams(c.Req.Context(), &isAdminOfTeamsQuery); err != nil { c.JsonApiErr(500, "Failed to check if user is a team admin", err) } diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index f66b16b723e..c8e323d1123 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -672,3 +672,7 @@ func (m *SQLStoreMock) GetDashboardPermissionsForUser(ctx context.Context, query func (m *SQLStoreMock) GetDashboardSlugById(ctx context.Context, query *models.GetDashboardSlugByIdQuery) error { return m.ExpectedError } + +func (m *SQLStoreMock) IsAdminOfTeams(ctx context.Context, query *models.IsAdminOfTeamsQuery) error { + return m.ExpectedError +} diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index 35e66d58350..26451462708 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -149,4 +149,5 @@ type Store interface { HasAdminPermissionInFolders(ctx context.Context, query *models.HasAdminPermissionInFoldersQuery) error GetDashboardPermissionsForUser(ctx context.Context, query *models.GetDashboardPermissionsForUserQuery) error GetDashboardSlugById(ctx context.Context, query *models.GetDashboardSlugByIdQuery) error + IsAdminOfTeams(ctx context.Context, query *models.IsAdminOfTeamsQuery) error } diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index 46c88f8f0bc..d78c1aa873e 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -176,129 +176,133 @@ func isTeamNameTaken(orgId int64, name string, existingId int64, sess *DBSession } func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQuery) error { - query.Result = models.SearchTeamQueryResult{ - Teams: make([]*models.TeamDTO, 0), - } - queryWithWildcards := "%" + query.Query + "%" + return ss.WithDbSession(ctx, func(sess *DBSession) error { + query.Result = models.SearchTeamQueryResult{ + Teams: make([]*models.TeamDTO, 0), + } + queryWithWildcards := "%" + query.Query + "%" - var sql bytes.Buffer - params := make([]interface{}, 0) + var sql bytes.Buffer + params := make([]interface{}, 0) - filteredUsers := getFilteredUsers(query.SignedInUser, query.HiddenUsers) - for _, user := range filteredUsers { - params = append(params, user) - } + filteredUsers := getFilteredUsers(query.SignedInUser, query.HiddenUsers) + for _, user := range filteredUsers { + params = append(params, user) + } - if query.UserIdFilter == models.FilterIgnoreUser { - sql.WriteString(getTeamSelectSQLBase(filteredUsers)) - } else { - sql.WriteString(getTeamSelectWithPermissionsSQLBase(filteredUsers)) - params = append(params, query.UserIdFilter) - } + if query.UserIdFilter == models.FilterIgnoreUser { + sql.WriteString(getTeamSelectSQLBase(filteredUsers)) + } else { + sql.WriteString(getTeamSelectWithPermissionsSQLBase(filteredUsers)) + params = append(params, query.UserIdFilter) + } - sql.WriteString(` WHERE team.org_id = ?`) - params = append(params, query.OrgId) + sql.WriteString(` WHERE team.org_id = ?`) + params = append(params, query.OrgId) - if query.Query != "" { - sql.WriteString(` and team.name ` + dialect.LikeStr() + ` ?`) - params = append(params, queryWithWildcards) - } + if query.Query != "" { + sql.WriteString(` and team.name ` + ss.Dialect.LikeStr() + ` ?`) + params = append(params, queryWithWildcards) + } - if query.Name != "" { - sql.WriteString(` and team.name = ?`) - params = append(params, query.Name) - } + if query.Name != "" { + sql.WriteString(` and team.name = ?`) + params = append(params, query.Name) + } - var ( - acFilter ac.SQLFilter - err error - ) - if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { - acFilter, err = ac.Filter(query.SignedInUser, "team.id", "teams:id:", ac.ActionTeamsRead) - if err != nil { + var ( + acFilter ac.SQLFilter + err error + ) + if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + acFilter, err = ac.Filter(query.SignedInUser, "team.id", "teams:id:", ac.ActionTeamsRead) + if err != nil { + return err + } + sql.WriteString(` and` + acFilter.Where) + params = append(params, acFilter.Args...) + } + + sql.WriteString(` order by team.name asc`) + + if query.Limit != 0 { + offset := query.Limit * (query.Page - 1) + sql.WriteString(ss.Dialect.LimitOffset(int64(query.Limit), int64(offset))) + } + + if err := sess.SQL(sql.String(), params...).Find(&query.Result.Teams); err != nil { return err } - sql.WriteString(` and` + acFilter.Where) - params = append(params, acFilter.Args...) - } - sql.WriteString(` order by team.name asc`) + team := models.Team{} + countSess := sess.Table("team") + countSess.Where("team.org_id=?", query.OrgId) - if query.Limit != 0 { - offset := query.Limit * (query.Page - 1) - sql.WriteString(dialect.LimitOffset(int64(query.Limit), int64(offset))) - } + if query.Query != "" { + countSess.Where(`name `+dialect.LikeStr()+` ?`, queryWithWildcards) + } - if err := x.SQL(sql.String(), params...).Find(&query.Result.Teams); err != nil { - return err - } + if query.Name != "" { + countSess.Where("name=?", query.Name) + } - team := models.Team{} - countSess := x.Table("team") - countSess.Where("team.org_id=?", query.OrgId) - - if query.Query != "" { - countSess.Where(`name `+dialect.LikeStr()+` ?`, queryWithWildcards) - } - - if query.Name != "" { - countSess.Where("name=?", query.Name) - } - - // If we're not retrieving all results, then only search for teams that this user has access to - if query.UserIdFilter != models.FilterIgnoreUser { - countSess. - Where(` + // If we're not retrieving all results, then only search for teams that this user has access to + if query.UserIdFilter != models.FilterIgnoreUser { + countSess. + Where(` team.id IN ( SELECT team_id FROM team_member WHERE team_member.user_id = ? )`, query.UserIdFilter) - } + } - // Only count teams user can see - if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { - countSess.Where(acFilter.Where, acFilter.Args...) - } + // Only count teams user can see + if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + countSess.Where(acFilter.Where, acFilter.Args...) + } - count, err := countSess.Count(&team) - query.Result.TotalCount = count + count, err := countSess.Count(&team) + query.Result.TotalCount = count - return err + return err + }) } func (ss *SQLStore) GetTeamById(ctx context.Context, query *models.GetTeamByIdQuery) error { - var sql bytes.Buffer - params := make([]interface{}, 0) + return ss.WithDbSession(ctx, func(sess *DBSession) error { + var sql bytes.Buffer + params := make([]interface{}, 0) - filteredUsers := getFilteredUsers(query.SignedInUser, query.HiddenUsers) - sql.WriteString(getTeamSelectSQLBase(filteredUsers)) - for _, user := range filteredUsers { - params = append(params, user) - } + filteredUsers := getFilteredUsers(query.SignedInUser, query.HiddenUsers) + sql.WriteString(getTeamSelectSQLBase(filteredUsers)) + for _, user := range filteredUsers { + params = append(params, user) + } - if query.UserIdFilter != models.FilterIgnoreUser { - sql.WriteString(` INNER JOIN team_member ON team.id = team_member.team_id AND team_member.user_id = ?`) - params = append(params, query.UserIdFilter) - } + if query.UserIdFilter != models.FilterIgnoreUser { + sql.WriteString(` INNER JOIN team_member ON team.id = team_member.team_id AND team_member.user_id = ?`) + params = append(params, query.UserIdFilter) + } - sql.WriteString(` WHERE team.org_id = ? and team.id = ?`) - params = append(params, query.OrgId, query.Id) + sql.WriteString(` WHERE team.org_id = ? and team.id = ?`) + params = append(params, query.OrgId, query.Id) - var team models.TeamDTO - exists, err := x.SQL(sql.String(), params...).Get(&team) + var team models.TeamDTO + exists, err := sess.SQL(sql.String(), params...).Get(&team) - if err != nil { - return err - } + if err != nil { + return err + } - if !exists { - return models.ErrTeamNotFound - } + if !exists { + return models.ErrTeamNotFound + } - query.Result = &team - return nil + query.Result = &team + return nil + }) } // GetTeamsByUser is used by the Guardian when checking a users' permissions @@ -513,7 +517,7 @@ func (ss *SQLStore) GetTeamMembers(ctx context.Context, query *models.GetTeamMem // Note we assume that checking SignedInUser is allowed to see team members for this team has already been performed // If the signed in user is not set no member will be returned if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { - sqlID := fmt.Sprintf("%s.%s", x.Dialect().Quote("user"), x.Dialect().Quote("id")) + sqlID := fmt.Sprintf("%s.%s", ss.engine.Dialect().Quote("user"), ss.engine.Dialect().Quote("id")) *acFilter, err = ac.Filter(query.SignedInUser, sqlID, "users:id:", ac.ActionOrgUsersRead) if err != nil { return err @@ -525,67 +529,71 @@ func (ss *SQLStore) GetTeamMembers(ctx context.Context, query *models.GetTeamMem // getTeamMembers return a list of members for the specified team func (ss *SQLStore) getTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery, acUserFilter *ac.SQLFilter) error { - query.Result = make([]*models.TeamMemberDTO, 0) - sess := x.Table("team_member") - sess.Join("INNER", x.Dialect().Quote("user"), - fmt.Sprintf("team_member.user_id=%s.%s", x.Dialect().Quote("user"), x.Dialect().Quote("id")), - ) + return ss.WithDbSession(ctx, func(dbSess *DBSession) error { + query.Result = make([]*models.TeamMemberDTO, 0) + sess := dbSess.Table("team_member") + sess.Join("INNER", ss.Dialect.Quote("user"), + fmt.Sprintf("team_member.user_id=%s.%s", ss.Dialect.Quote("user"), ss.Dialect.Quote("id")), + ) - if acUserFilter != nil { - sess.Where(acUserFilter.Where, acUserFilter.Args...) - } + if acUserFilter != nil { + sess.Where(acUserFilter.Where, acUserFilter.Args...) + } - // Join with only most recent auth module - authJoinCondition := `( + // Join with only most recent auth module + authJoinCondition := `( SELECT id from user_auth WHERE user_auth.user_id = team_member.user_id ORDER BY user_auth.created DESC ` - authJoinCondition = "user_auth.id=" + authJoinCondition + dialect.Limit(1) + ")" - sess.Join("LEFT", "user_auth", authJoinCondition) + authJoinCondition = "user_auth.id=" + authJoinCondition + ss.Dialect.Limit(1) + ")" + sess.Join("LEFT", "user_auth", authJoinCondition) - if query.OrgId != 0 { - sess.Where("team_member.org_id=?", query.OrgId) - } - if query.TeamId != 0 { - sess.Where("team_member.team_id=?", query.TeamId) - } - if query.UserId != 0 { - sess.Where("team_member.user_id=?", query.UserId) - } - if query.External { - sess.Where("team_member.external=?", dialect.BooleanStr(true)) - } - sess.Cols( - "team_member.org_id", - "team_member.team_id", - "team_member.user_id", - "user.email", - "user.name", - "user.login", - "team_member.external", - "team_member.permission", - "user_auth.auth_module", - ) - sess.Asc("user.login", "user.email") + if query.OrgId != 0 { + sess.Where("team_member.org_id=?", query.OrgId) + } + if query.TeamId != 0 { + sess.Where("team_member.team_id=?", query.TeamId) + } + if query.UserId != 0 { + sess.Where("team_member.user_id=?", query.UserId) + } + if query.External { + sess.Where("team_member.external=?", ss.Dialect.BooleanStr(true)) + } + sess.Cols( + "team_member.org_id", + "team_member.team_id", + "team_member.user_id", + "user.email", + "user.name", + "user.login", + "team_member.external", + "team_member.permission", + "user_auth.auth_module", + ) + sess.Asc("user.login", "user.email") - err := sess.Find(&query.Result) - return err -} - -func IsAdminOfTeams(ctx context.Context, query *models.IsAdminOfTeamsQuery) error { - builder := &SQLBuilder{} - builder.Write("SELECT COUNT(team.id) AS count FROM team INNER JOIN team_member ON team_member.team_id = team.id WHERE team.org_id = ? AND team_member.user_id = ? AND team_member.permission = ?", query.SignedInUser.OrgId, query.SignedInUser.UserId, models.PERMISSION_ADMIN) - - type teamCount struct { - Count int64 - } - - resp := make([]*teamCount, 0) - if err := x.SQL(builder.GetSQLString(), builder.params...).Find(&resp); err != nil { + err := sess.Find(&query.Result) return err - } + }) +} - query.Result = len(resp) > 0 && resp[0].Count > 0 +func (ss *SQLStore) IsAdminOfTeams(ctx context.Context, query *models.IsAdminOfTeamsQuery) error { + return ss.WithDbSession(ctx, func(sess *DBSession) error { + builder := &SQLBuilder{} + builder.Write("SELECT COUNT(team.id) AS count FROM team INNER JOIN team_member ON team_member.team_id = team.id WHERE team.org_id = ? AND team_member.user_id = ? AND team_member.permission = ?", query.SignedInUser.OrgId, query.SignedInUser.UserId, models.PERMISSION_ADMIN) - return nil + type teamCount struct { + Count int64 + } + + resp := make([]*teamCount, 0) + if err := sess.SQL(builder.GetSQLString(), builder.params...).Find(&resp); err != nil { + return err + } + + query.Result = len(resp) > 0 && resp[0].Count > 0 + + return nil + }) } diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index 23445ccbfe4..63bdd2b76f7 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -302,12 +302,12 @@ func TestTeamCommandsAndQueries(t *testing.T) { require.NoError(t, err) query := &models.IsAdminOfTeamsQuery{SignedInUser: &models.SignedInUser{OrgId: testOrgID, UserId: userIds[0]}} - err = IsAdminOfTeams(context.Background(), query) + err = sqlStore.IsAdminOfTeams(context.Background(), query) require.NoError(t, err) require.False(t, query.Result) query = &models.IsAdminOfTeamsQuery{SignedInUser: &models.SignedInUser{OrgId: testOrgID, UserId: userIds[1]}} - err = IsAdminOfTeams(context.Background(), query) + err = sqlStore.IsAdminOfTeams(context.Background(), query) require.NoError(t, err) require.True(t, query.Result) })