From e1e0b5f95121ea456fb31bce3d1aff69597ec096 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 9 Feb 2018 17:26:15 +0100 Subject: [PATCH] teams: use orgId in all team and team member operations (#10862) Also fixes issue in org users tests for postgres --- pkg/api/api.go | 14 ++++++------ pkg/api/team.go | 7 +++--- pkg/api/team_members.go | 4 ++-- pkg/models/team.go | 6 ++++- pkg/models/team_member.go | 2 ++ pkg/services/guardian/guardian.go | 2 +- pkg/services/sqlstore/org_users.go | 2 +- pkg/services/sqlstore/team.go | 33 ++++++++++++++-------------- pkg/services/sqlstore/team_test.go | 35 ++++++++++++++++-------------- 9 files changed, 58 insertions(+), 47 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 752af7602f5..ad77e410dbf 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -150,13 +150,13 @@ func (hs *HttpServer) registerRoutes() { apiRoute.Group("/teams", func(teamsRoute RouteRegister) { teamsRoute.Get("/:teamId", wrap(GetTeamById)) teamsRoute.Get("/search", wrap(SearchTeams)) - teamsRoute.Post("/", quota("teams"), reqOrgAdmin, bind(m.CreateTeamCommand{}), wrap(CreateTeam)) - teamsRoute.Put("/:teamId", reqOrgAdmin, bind(m.UpdateTeamCommand{}), wrap(UpdateTeam)) - teamsRoute.Delete("/:teamId", reqOrgAdmin, wrap(DeleteTeamById)) - teamsRoute.Get("/:teamId/members", reqOrgAdmin, wrap(GetTeamMembers)) - teamsRoute.Post("/:teamId/members", reqOrgAdmin, quota("teams"), bind(m.AddTeamMemberCommand{}), wrap(AddTeamMember)) - teamsRoute.Delete("/:teamId/members/:userId", reqOrgAdmin, wrap(RemoveTeamMember)) - }) + teamsRoute.Post("/", quota("teams"), bind(m.CreateTeamCommand{}), wrap(CreateTeam)) + teamsRoute.Put("/:teamId", bind(m.UpdateTeamCommand{}), wrap(UpdateTeam)) + teamsRoute.Delete("/:teamId", wrap(DeleteTeamById)) + teamsRoute.Get("/:teamId/members", wrap(GetTeamMembers)) + teamsRoute.Post("/:teamId/members", quota("teams"), bind(m.AddTeamMemberCommand{}), wrap(AddTeamMember)) + teamsRoute.Delete("/:teamId/members/:userId", wrap(RemoveTeamMember)) + }, reqOrgAdmin) // org information available to all users. apiRoute.Group("/org", func(orgRoute RouteRegister) { diff --git a/pkg/api/team.go b/pkg/api/team.go index af537224d41..f11eca68b91 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -26,6 +26,7 @@ func CreateTeam(c *middleware.Context, cmd m.CreateTeamCommand) Response { // PUT /api/teams/:teamId func UpdateTeam(c *middleware.Context, cmd m.UpdateTeamCommand) Response { + cmd.OrgId = c.OrgId cmd.Id = c.ParamsInt64(":teamId") if err := bus.Dispatch(&cmd); err != nil { if err == m.ErrTeamNameTaken { @@ -39,7 +40,7 @@ func UpdateTeam(c *middleware.Context, cmd m.UpdateTeamCommand) Response { // DELETE /api/teams/:teamId func DeleteTeamById(c *middleware.Context) Response { - if err := bus.Dispatch(&m.DeleteTeamCommand{Id: c.ParamsInt64(":teamId")}); err != nil { + if err := bus.Dispatch(&m.DeleteTeamCommand{OrgId: c.OrgId, Id: c.ParamsInt64(":teamId")}); err != nil { if err == m.ErrTeamNotFound { return ApiError(404, "Failed to delete Team. ID not found", nil) } @@ -60,11 +61,11 @@ func SearchTeams(c *middleware.Context) Response { } query := m.SearchTeamsQuery{ + OrgId: c.OrgId, Query: c.Query("query"), Name: c.Query("name"), Page: page, Limit: perPage, - OrgId: c.OrgId, } if err := bus.Dispatch(&query); err != nil { @@ -83,7 +84,7 @@ func SearchTeams(c *middleware.Context) Response { // GET /api/teams/:teamId func GetTeamById(c *middleware.Context) Response { - query := m.GetTeamByIdQuery{Id: c.ParamsInt64(":teamId")} + query := m.GetTeamByIdQuery{OrgId: c.OrgId, Id: c.ParamsInt64(":teamId")} if err := bus.Dispatch(&query); err != nil { if err == m.ErrTeamNotFound { diff --git a/pkg/api/team_members.go b/pkg/api/team_members.go index 412e142edb7..59dfc20b791 100644 --- a/pkg/api/team_members.go +++ b/pkg/api/team_members.go @@ -10,7 +10,7 @@ import ( // GET /api/teams/:teamId/members func GetTeamMembers(c *middleware.Context) Response { - query := m.GetTeamMembersQuery{TeamId: c.ParamsInt64(":teamId")} + query := m.GetTeamMembersQuery{OrgId: c.OrgId, TeamId: c.ParamsInt64(":teamId")} if err := bus.Dispatch(&query); err != nil { return ApiError(500, "Failed to get Team Members", err) @@ -42,7 +42,7 @@ func AddTeamMember(c *middleware.Context, cmd m.AddTeamMemberCommand) Response { // DELETE /api/teams/:teamId/members/:userId func RemoveTeamMember(c *middleware.Context) Response { - if err := bus.Dispatch(&m.RemoveTeamMemberCommand{TeamId: c.ParamsInt64(":teamId"), UserId: c.ParamsInt64(":userId")}); err != nil { + if err := bus.Dispatch(&m.RemoveTeamMemberCommand{OrgId: c.OrgId, TeamId: c.ParamsInt64(":teamId"), UserId: c.ParamsInt64(":userId")}); err != nil { return ApiError(500, "Failed to remove Member from Team", err) } return ApiSuccess("Team Member removed") diff --git a/pkg/models/team.go b/pkg/models/team.go index d2912f431b8..f789f125aa1 100644 --- a/pkg/models/team.go +++ b/pkg/models/team.go @@ -37,18 +37,22 @@ type UpdateTeamCommand struct { Id int64 Name string Email string + OrgId int64 `json:"-"` } type DeleteTeamCommand struct { - Id int64 + OrgId int64 + Id int64 } type GetTeamByIdQuery struct { + OrgId int64 Id int64 Result *Team } type GetTeamsByUserQuery struct { + OrgId int64 UserId int64 `json:"userId"` Result []*Team `json:"teams"` } diff --git a/pkg/models/team_member.go b/pkg/models/team_member.go index 9970678a1ae..19cf657292d 100644 --- a/pkg/models/team_member.go +++ b/pkg/models/team_member.go @@ -31,6 +31,7 @@ type AddTeamMemberCommand struct { } type RemoveTeamMemberCommand struct { + OrgId int64 `json:"-"` UserId int64 TeamId int64 } @@ -39,6 +40,7 @@ type RemoveTeamMemberCommand struct { // QUERIES type GetTeamMembersQuery struct { + OrgId int64 TeamId int64 Result []*TeamMemberDTO } diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index f4056841c33..b448561494d 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -160,7 +160,7 @@ func (g *DashboardGuardian) getTeams() ([]*m.Team, error) { return g.groups, nil } - query := m.GetTeamsByUserQuery{UserId: g.user.UserId} + query := m.GetTeamsByUserQuery{OrgId: g.orgId, UserId: g.user.UserId} err := bus.Dispatch(&query) g.groups = query.Result diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index 79745c30386..0b991c73c55 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -82,7 +82,7 @@ func GetOrgUsers(query *m.GetOrgUsersQuery) error { if query.Query != "" { queryWithWildcards := "%" + query.Query + "%" - whereConditions = append(whereConditions, "(user.email "+dialect.LikeStr()+" ? OR user.name "+dialect.LikeStr()+" ? OR user.login "+dialect.LikeStr()+" ?)") + whereConditions = append(whereConditions, "(email "+dialect.LikeStr()+" ? OR name "+dialect.LikeStr()+" ? OR login "+dialect.LikeStr()+" ?)") whereParams = append(whereParams, queryWithWildcards, queryWithWildcards, queryWithWildcards) } diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index 98bb1a36eb9..ecb34ad927b 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -25,7 +25,7 @@ func init() { func CreateTeam(cmd *m.CreateTeamCommand) error { return inTransaction(func(sess *DBSession) error { - if isNameTaken, err := isTeamNameTaken(cmd.Name, 0, sess); err != nil { + if isNameTaken, err := isTeamNameTaken(cmd.OrgId, cmd.Name, 0, sess); err != nil { return err } else if isNameTaken { return m.ErrTeamNameTaken @@ -50,7 +50,7 @@ func CreateTeam(cmd *m.CreateTeamCommand) error { func UpdateTeam(cmd *m.UpdateTeamCommand) error { return inTransaction(func(sess *DBSession) error { - if isNameTaken, err := isTeamNameTaken(cmd.Name, cmd.Id, sess); err != nil { + if isNameTaken, err := isTeamNameTaken(cmd.OrgId, cmd.Name, cmd.Id, sess); err != nil { return err } else if isNameTaken { return m.ErrTeamNameTaken @@ -80,20 +80,20 @@ func UpdateTeam(cmd *m.UpdateTeamCommand) error { func DeleteTeam(cmd *m.DeleteTeamCommand) error { return inTransaction(func(sess *DBSession) error { - if res, err := sess.Query("SELECT 1 from team WHERE id=?", cmd.Id); err != nil { + if res, err := sess.Query("SELECT 1 from team WHERE org_id=? and id=?", cmd.OrgId, cmd.Id); err != nil { return err } else if len(res) != 1 { return m.ErrTeamNotFound } deletes := []string{ - "DELETE FROM team_member WHERE team_id = ?", - "DELETE FROM team WHERE id = ?", - "DELETE FROM dashboard_acl WHERE team_id = ?", + "DELETE FROM team_member WHERE org_id=? and team_id = ?", + "DELETE FROM team WHERE org_id=? and id = ?", + "DELETE FROM dashboard_acl WHERE org_id=? and team_id = ?", } for _, sql := range deletes { - _, err := sess.Exec(sql, cmd.Id) + _, err := sess.Exec(sql, cmd.OrgId, cmd.Id) if err != nil { return err } @@ -102,9 +102,9 @@ func DeleteTeam(cmd *m.DeleteTeamCommand) error { }) } -func isTeamNameTaken(name string, existingId int64, sess *DBSession) (bool, error) { +func isTeamNameTaken(orgId int64, name string, existingId int64, sess *DBSession) (bool, error) { var team m.Team - exists, err := sess.Where("name=?", name).Get(&team) + exists, err := sess.Where("org_id=? and name=?", orgId, name).Get(&team) if err != nil { return false, nil @@ -128,6 +128,7 @@ func SearchTeams(query *m.SearchTeamsQuery) error { sql.WriteString(`select team.id as id, + team.org_id, team.name as name, team.email as email, (select count(*) from team_member where team_member.team_id = team.id) as member_count @@ -176,7 +177,7 @@ func SearchTeams(query *m.SearchTeamsQuery) error { func GetTeamById(query *m.GetTeamByIdQuery) error { var team m.Team - exists, err := x.Id(query.Id).Get(&team) + exists, err := x.Where("org_id=? and id=?", query.OrgId, query.Id).Get(&team) if err != nil { return err } @@ -194,7 +195,7 @@ func GetTeamsByUser(query *m.GetTeamsByUserQuery) error { sess := x.Table("team") sess.Join("INNER", "team_member", "team.id=team_member.team_id") - sess.Where("team_member.user_id=?", query.UserId) + sess.Where("team.org_id=? and team_member.user_id=?", query.OrgId, query.UserId) err := sess.Find(&query.Result) if err != nil { @@ -206,13 +207,13 @@ func GetTeamsByUser(query *m.GetTeamsByUserQuery) error { func AddTeamMember(cmd *m.AddTeamMemberCommand) error { return inTransaction(func(sess *DBSession) error { - if res, err := sess.Query("SELECT 1 from team_member WHERE team_id=? and user_id=?", cmd.TeamId, cmd.UserId); err != nil { + if res, err := sess.Query("SELECT 1 from team_member WHERE org_id=? and team_id=? and user_id=?", cmd.OrgId, cmd.TeamId, cmd.UserId); err != nil { return err } else if len(res) == 1 { return m.ErrTeamMemberAlreadyAdded } - if res, err := sess.Query("SELECT 1 from team WHERE id=?", cmd.TeamId); err != nil { + if res, err := sess.Query("SELECT 1 from team WHERE org_id=? and id=?", cmd.OrgId, cmd.TeamId); err != nil { return err } else if len(res) != 1 { return m.ErrTeamNotFound @@ -233,8 +234,8 @@ func AddTeamMember(cmd *m.AddTeamMemberCommand) error { func RemoveTeamMember(cmd *m.RemoveTeamMemberCommand) error { return inTransaction(func(sess *DBSession) error { - var rawSql = "DELETE FROM team_member WHERE team_id=? and user_id=?" - _, err := sess.Exec(rawSql, cmd.TeamId, cmd.UserId) + var rawSql = "DELETE FROM team_member WHERE org_id=? and team_id=? and user_id=?" + _, err := sess.Exec(rawSql, cmd.OrgId, cmd.TeamId, cmd.UserId) if err != nil { return err } @@ -247,7 +248,7 @@ func GetTeamMembers(query *m.GetTeamMembersQuery) error { query.Result = make([]*m.TeamMemberDTO, 0) sess := x.Table("team_member") sess.Join("INNER", "user", fmt.Sprintf("team_member.user_id=%s.id", x.Dialect().Quote("user"))) - sess.Where("team_member.team_id=?", query.TeamId) + sess.Where("team_member.org_id=? and team_member.team_id=?", query.OrgId, query.TeamId) sess.Cols("user.org_id", "team_member.team_id", "team_member.user_id", "user.email", "user.login") sess.Asc("user.login", "user.email") diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index dbae4545266..bebe59f4238 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -27,8 +27,9 @@ func TestTeamCommandsAndQueries(t *testing.T) { userIds = append(userIds, userCmd.Result.Id) } - group1 := m.CreateTeamCommand{Name: "group1 name", Email: "test1@test.com"} - group2 := m.CreateTeamCommand{Name: "group2 name", Email: "test2@test.com"} + var testOrgId int64 = 1 + group1 := m.CreateTeamCommand{OrgId: testOrgId, Name: "group1 name", Email: "test1@test.com"} + group2 := m.CreateTeamCommand{OrgId: testOrgId, Name: "group2 name", Email: "test2@test.com"} err := CreateTeam(&group1) So(err, ShouldBeNil) @@ -36,7 +37,7 @@ func TestTeamCommandsAndQueries(t *testing.T) { So(err, ShouldBeNil) Convey("Should be able to create teams and add users", func() { - query := &m.SearchTeamsQuery{Name: "group1 name", Page: 1, Limit: 10} + query := &m.SearchTeamsQuery{OrgId: testOrgId, Name: "group1 name", Page: 1, Limit: 10} err = SearchTeams(query) So(err, ShouldBeNil) So(query.Page, ShouldEqual, 1) @@ -44,25 +45,27 @@ func TestTeamCommandsAndQueries(t *testing.T) { team1 := query.Result.Teams[0] So(team1.Name, ShouldEqual, "group1 name") So(team1.Email, ShouldEqual, "test1@test.com") + So(team1.OrgId, ShouldEqual, testOrgId) - err = AddTeamMember(&m.AddTeamMemberCommand{OrgId: 1, TeamId: team1.Id, UserId: userIds[0]}) + err = AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: team1.Id, UserId: userIds[0]}) So(err, ShouldBeNil) - q1 := &m.GetTeamMembersQuery{TeamId: team1.Id} + q1 := &m.GetTeamMembersQuery{OrgId: testOrgId, TeamId: team1.Id} err = GetTeamMembers(q1) So(err, ShouldBeNil) So(q1.Result[0].TeamId, ShouldEqual, team1.Id) So(q1.Result[0].Login, ShouldEqual, "loginuser0") + So(q1.Result[0].OrgId, ShouldEqual, testOrgId) }) Convey("Should be able to search for teams", func() { - query := &m.SearchTeamsQuery{Query: "group", Page: 1} + query := &m.SearchTeamsQuery{OrgId: testOrgId, Query: "group", Page: 1} err = SearchTeams(query) So(err, ShouldBeNil) So(len(query.Result.Teams), ShouldEqual, 2) So(query.Result.TotalCount, ShouldEqual, 2) - query2 := &m.SearchTeamsQuery{Query: ""} + query2 := &m.SearchTeamsQuery{OrgId: testOrgId, Query: ""} err = SearchTeams(query2) So(err, ShouldBeNil) So(len(query2.Result.Teams), ShouldEqual, 2) @@ -70,9 +73,9 @@ func TestTeamCommandsAndQueries(t *testing.T) { Convey("Should be able to return all teams a user is member of", func() { groupId := group2.Result.Id - err := AddTeamMember(&m.AddTeamMemberCommand{OrgId: 1, TeamId: groupId, UserId: userIds[0]}) + err := AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: groupId, UserId: userIds[0]}) - query := &m.GetTeamsByUserQuery{UserId: userIds[0]} + query := &m.GetTeamsByUserQuery{OrgId: testOrgId, UserId: userIds[0]} err = GetTeamsByUser(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 1) @@ -81,7 +84,7 @@ func TestTeamCommandsAndQueries(t *testing.T) { }) Convey("Should be able to remove users from a group", func() { - err = RemoveTeamMember(&m.RemoveTeamMemberCommand{TeamId: group1.Result.Id, UserId: userIds[0]}) + err = RemoveTeamMember(&m.RemoveTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[0]}) So(err, ShouldBeNil) q1 := &m.GetTeamMembersQuery{TeamId: group1.Result.Id} @@ -92,20 +95,20 @@ func TestTeamCommandsAndQueries(t *testing.T) { Convey("Should be able to remove a group with users and permissions", func() { groupId := group2.Result.Id - err := AddTeamMember(&m.AddTeamMemberCommand{OrgId: 1, TeamId: groupId, UserId: userIds[1]}) + err := AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: groupId, UserId: userIds[1]}) So(err, ShouldBeNil) - err = AddTeamMember(&m.AddTeamMemberCommand{OrgId: 1, TeamId: groupId, UserId: userIds[2]}) + err = AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: groupId, UserId: userIds[2]}) So(err, ShouldBeNil) - err = SetDashboardAcl(&m.SetDashboardAclCommand{DashboardId: 1, OrgId: 1, Permission: m.PERMISSION_EDIT, TeamId: groupId}) + err = SetDashboardAcl(&m.SetDashboardAclCommand{DashboardId: 1, OrgId: testOrgId, Permission: m.PERMISSION_EDIT, TeamId: groupId}) - err = DeleteTeam(&m.DeleteTeamCommand{Id: groupId}) + err = DeleteTeam(&m.DeleteTeamCommand{OrgId: testOrgId, Id: groupId}) So(err, ShouldBeNil) - query := &m.GetTeamByIdQuery{Id: groupId} + query := &m.GetTeamByIdQuery{OrgId: testOrgId, Id: groupId} err = GetTeamById(query) So(err, ShouldEqual, m.ErrTeamNotFound) - permQuery := &m.GetDashboardAclInfoListQuery{DashboardId: 1, OrgId: 1} + permQuery := &m.GetDashboardAclInfoListQuery{DashboardId: 1, OrgId: testOrgId} err = GetDashboardAclInfoList(permQuery) So(err, ShouldBeNil)