From 21d3d274523be3817caed94d2e85f6a77f1dc877 Mon Sep 17 00:00:00 2001 From: Leonard Gram Date: Tue, 12 Mar 2019 16:59:39 +0100 Subject: [PATCH] teams: editors can't remove the last admin from a team. --- pkg/api/api.go | 2 +- pkg/api/team_members.go | 9 ++++++-- pkg/models/team.go | 1 + pkg/models/team_member.go | 7 +++--- pkg/services/sqlstore/team.go | 35 ++++++++++++++++++++++++++++++ pkg/services/sqlstore/team_test.go | 17 +++++++++++++++ 6 files changed, 65 insertions(+), 6 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 9ffb0278935..9acd9485312 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -160,7 +160,7 @@ func (hs *HTTPServer) registerRoutes() { teamsRoute.Get("/:teamId/members", Wrap(GetTeamMembers)) teamsRoute.Post("/:teamId/members", bind(m.AddTeamMemberCommand{}), Wrap(AddTeamMember)) teamsRoute.Put("/:teamId/members/:userId", bind(m.UpdateTeamMemberCommand{}), Wrap(UpdateTeamMember)) - teamsRoute.Delete("/:teamId/members/:userId", Wrap(RemoveTeamMember)) + teamsRoute.Delete("/:teamId/members/:userId", Wrap(hs.RemoveTeamMember)) teamsRoute.Get("/:teamId/preferences", Wrap(GetTeamPreferences)) teamsRoute.Put("/:teamId/preferences", bind(dtos.UpdatePrefsCmd{}), Wrap(UpdateTeamPreferences)) }, reqAdminOrEditorCanAdmin) diff --git a/pkg/api/team_members.go b/pkg/api/team_members.go index 669326ded18..72aded688ec 100644 --- a/pkg/api/team_members.go +++ b/pkg/api/team_members.go @@ -81,7 +81,7 @@ func UpdateTeamMember(c *m.ReqContext, cmd m.UpdateTeamMemberCommand) Response { } // DELETE /api/teams/:teamId/members/:userId -func RemoveTeamMember(c *m.ReqContext) Response { +func (hs *HTTPServer) RemoveTeamMember(c *m.ReqContext) Response { orgId := c.OrgId teamId := c.ParamsInt64(":teamId") userId := c.ParamsInt64(":userId") @@ -90,7 +90,12 @@ func RemoveTeamMember(c *m.ReqContext) Response { return Error(403, "Not allowed to remove team member", err) } - if err := bus.Dispatch(&m.RemoveTeamMemberCommand{OrgId: orgId, TeamId: teamId, UserId: userId}); err != nil { + protectLastAdmin := false + if c.OrgRole == m.ROLE_EDITOR { + protectLastAdmin = true + } + + if err := bus.Dispatch(&m.RemoveTeamMemberCommand{OrgId: orgId, TeamId: teamId, UserId: userId, ProtectLastAdmin: protectLastAdmin}); err != nil { if err == m.ErrTeamNotFound { return Error(404, "Team not found", nil) } diff --git a/pkg/models/team.go b/pkg/models/team.go index bb9289ee5e5..5b659331601 100644 --- a/pkg/models/team.go +++ b/pkg/models/team.go @@ -10,6 +10,7 @@ var ( ErrTeamNotFound = errors.New("Team not found") ErrTeamNameTaken = errors.New("Team name is taken") ErrTeamMemberNotFound = errors.New("Team member not found") + ErrLastTeamAdmin = errors.New("Not allowed to remove last admin") ErrNotAllowedToUpdateTeam = errors.New("User not allowed to update team") ErrNotAllowedToUpdateTeamInDifferentOrg = errors.New("User not allowed to update team in another org") ) diff --git a/pkg/models/team_member.go b/pkg/models/team_member.go index 1140e39b095..0cc39b0f605 100644 --- a/pkg/models/team_member.go +++ b/pkg/models/team_member.go @@ -42,9 +42,10 @@ type UpdateTeamMemberCommand struct { } type RemoveTeamMemberCommand struct { - OrgId int64 `json:"-"` - UserId int64 - TeamId int64 + OrgId int64 `json:"-"` + UserId int64 + TeamId int64 + ProtectLastAdmin bool `json:"-"` } // ---------------------- diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index 7c5a5f88983..3848adcc7dc 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -285,6 +285,18 @@ func RemoveTeamMember(cmd *m.RemoveTeamMemberCommand) error { return err } + if cmd.ProtectLastAdmin { + lastAdmin, err := isLastAdmin(sess, cmd.OrgId, cmd.TeamId, cmd.UserId) + if err != nil { + return err + } + + if lastAdmin { + return m.ErrLastTeamAdmin + } + + } + var rawSql = "DELETE FROM team_member WHERE org_id=? and team_id=? and user_id=?" res, err := sess.Exec(rawSql, cmd.OrgId, cmd.TeamId, cmd.UserId) if err != nil { @@ -299,6 +311,29 @@ func RemoveTeamMember(cmd *m.RemoveTeamMemberCommand) error { }) } +func isLastAdmin(sess *DBSession, orgId int64, teamId int64, userId int64) (bool, error) { + rawSql := "SELECT user_id FROM team_member WHERE org_id=? and team_id=? and permission=?" + userIds := []*int64{} + err := sess.SQL(rawSql, orgId, teamId, m.PERMISSION_ADMIN).Find(&userIds) + if err != nil { + return false, err + } + + isAdmin := false + for _, adminId := range userIds { + if userId == *adminId { + isAdmin = true + break + } + } + + if isAdmin && len(userIds) == 1 { + return true, nil + } + + return false, err +} + // GetTeamMembers return a list of members for the specified team func GetTeamMembers(query *m.GetTeamMembersQuery) error { query.Result = make([]*m.TeamMemberDTO, 0) diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index 1c5f2024a79..ca5379bae65 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -152,6 +152,23 @@ func TestTeamCommandsAndQueries(t *testing.T) { So(len(q2.Result), ShouldEqual, 0) }) + Convey("When ProtectLastAdmin is set to true", func() { + err = AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[0], Permission: int64(m.PERMISSION_ADMIN)}) + So(err, ShouldBeNil) + + Convey("A user should not be able to remove the last admin", func() { + err = RemoveTeamMember(&m.RemoveTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[0], ProtectLastAdmin: true}) + So(err, ShouldEqual, m.ErrLastTeamAdmin) + }) + + Convey("A user should be able to remove an admin if there are other admins", func() { + err = AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[1], Permission: int64(m.PERMISSION_ADMIN)}) + err = RemoveTeamMember(&m.RemoveTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[0], ProtectLastAdmin: true}) + So(err, ShouldEqual, nil) + }) + + }) + Convey("Should be able to remove a group with users and permissions", func() { groupId := group2.Result.Id err := AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: groupId, UserId: userIds[1]})