From 6c75662b824491a20a757a5eec59556a866374b5 Mon Sep 17 00:00:00 2001 From: Corey Hulen Date: Fri, 6 May 2016 11:28:22 -0700 Subject: PLT-2697 Fixing team admins (#2900) * PLT-2697 Fixing team admins * Fixing eslint error * Fixing loc issues * Fixing func * Fixing func --- api/apitestlib.go | 2 +- api/context.go | 6 +-- api/team_test.go | 4 +- api/user.go | 95 +++++++++++++++++++++++++++++---------- api/user_test.go | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 208 insertions(+), 31 deletions(-) (limited to 'api') diff --git a/api/apitestlib.go b/api/apitestlib.go index a7c2a9406..6372ea6b1 100644 --- a/api/apitestlib.go +++ b/api/apitestlib.go @@ -89,7 +89,7 @@ func (me *TestHelper) InitSystemAdmin() *TestHelper { c := &Context{} c.RequestId = model.NewId() c.IpAddress = "cmd_line" - UpdateRoles(c, me.SystemAdminUser, model.ROLE_SYSTEM_ADMIN) + UpdateUserRoles(c, me.SystemAdminUser, model.ROLE_SYSTEM_ADMIN) me.SystemAdminUser.Password = "Password1" me.LoginSystemAdmin() me.SystemAdminChannel = me.CreateChannel(me.SystemAdminClient, me.SystemAdminTeam) diff --git a/api/context.go b/api/context.go index 03d0046be..67b04c391 100644 --- a/api/context.go +++ b/api/context.go @@ -372,12 +372,12 @@ func (c *Context) IsTeamAdmin() bool { return true } - team := c.Session.GetTeamByTeamId(c.TeamId) - if team == nil { + teamMember := c.Session.GetTeamByTeamId(c.TeamId) + if teamMember == nil { return false } - return model.IsInRole(team.Roles, model.ROLE_TEAM_ADMIN) + return teamMember.IsTeamAdmin() } func (c *Context) RemoveSessionCookie(w http.ResponseWriter, r *http.Request) { diff --git a/api/team_test.go b/api/team_test.go index a58260fd2..30952b4d8 100644 --- a/api/team_test.go +++ b/api/team_test.go @@ -252,7 +252,7 @@ func TestGetAllTeams(t *testing.T) { c := &Context{} c.RequestId = model.NewId() c.IpAddress = "cmd_line" - UpdateRoles(c, user, model.ROLE_SYSTEM_ADMIN) + UpdateUserRoles(c, user, model.ROLE_SYSTEM_ADMIN) Client.Login(user.Email, "pwd") Client.SetTeamId(team.Id) @@ -301,7 +301,7 @@ func TestGetAllTeamListings(t *testing.T) { c := &Context{} c.RequestId = model.NewId() c.IpAddress = "cmd_line" - UpdateRoles(c, user, model.ROLE_SYSTEM_ADMIN) + UpdateUserRoles(c, user, model.ROLE_SYSTEM_ADMIN) Client.Login(user.Email, "pwd") Client.SetTeamId(team.Id) diff --git a/api/user.go b/api/user.go index de4242f24..60162d8f1 100644 --- a/api/user.go +++ b/api/user.go @@ -1372,16 +1372,23 @@ func updateRoles(c *Context, w http.ResponseWriter, r *http.Request) { return } + team_id := props["team_id"] + if !(len(user_id) == 26 || len(user_id) == 0) { + c.SetInvalidParam("updateRoles", "team_id") + return + } + new_roles := props["new_roles"] - if !model.IsValidUserRoles(new_roles) { + if !(model.IsValidUserRoles(new_roles) || model.IsValidTeamRoles(new_roles)) { c.SetInvalidParam("updateRoles", "new_roles") return } // If you are not the system admin then you can only demote yourself if !c.IsSystemAdmin() && user_id != c.Session.UserId { - c.Err = model.NewLocAppError("updateRoles", "api.user.update_roles.system_admin_set.app_error", nil, "") + c.Err = model.NewLocAppError("updateRoles", "api.user.update_roles.system_admin_needed.app_error", nil, "") c.Err.StatusCode = http.StatusForbidden + return } // Only another system admin can add the system admin role @@ -1399,36 +1406,78 @@ func updateRoles(c *Context, w http.ResponseWriter, r *http.Request) { user = result.Data.(*model.User) } - ruser := UpdateRoles(c, user, new_roles) - if c.Err != nil { - return - } + // if the team role has changed then lets update team members + if model.IsValidTeamRoles(new_roles) && len(team_id) > 0 { + + var members []*model.TeamMember + if result := <-Srv.Store.Team().GetTeamsForUser(user_id); result.Err != nil { + c.Err = result.Err + return + } else { + members = result.Data.([]*model.TeamMember) + } + + var member *model.TeamMember + for _, m := range members { + if m.TeamId == team_id { + member = m + } + } - uchan := Srv.Store.Session().UpdateRoles(user.Id, new_roles) - gchan := Srv.Store.Session().GetSessions(user.Id) + if member == nil { + c.SetInvalidParam("updateRoles", "team_id") + return + } - if result := <-uchan; result.Err != nil { - // soft error since the user roles were still updated - l4g.Error(result.Err) + if !c.IsSystemAdmin() { + currentUserTeamMember := c.Session.GetTeamByTeamId(team_id) + + // Only the system admin can modify other team + if currentUserTeamMember == nil { + c.Err = model.NewLocAppError("updateRoles", "api.user.update_roles.system_admin_needed.app_error", nil, "") + c.Err.StatusCode = http.StatusForbidden + return + } + + // Only another team admin can make a team admin + if !currentUserTeamMember.IsTeamAdmin() && model.IsInRole(new_roles, model.ROLE_TEAM_ADMIN) { + c.Err = model.NewLocAppError("updateRoles", "api.user.update_roles.team_admin_needed.app_error", nil, "") + c.Err.StatusCode = http.StatusForbidden + return + } + } + + member.Roles = new_roles + + if result := <-Srv.Store.Team().UpdateMember(member); result.Err != nil { + c.Err = result.Err + return + } } - if result := <-gchan; result.Err != nil { - // soft error since the user roles were still updated - l4g.Error(result.Err) - } else { - sessions := result.Data.([]*model.Session) - for _, s := range sessions { - sessionCache.Remove(s.Token) + // If the users role has changed then lets update the user + if model.IsValidUserRoles(new_roles) { + UpdateUserRoles(c, user, new_roles) + if c.Err != nil { + return + } + + uchan := Srv.Store.Session().UpdateRoles(user.Id, new_roles) + + if result := <-uchan; result.Err != nil { + // soft error since the user roles were still updated + l4g.Error(result.Err) } } - options := utils.Cfg.GetSanitizeOptions() - options["passwordupdate"] = false - ruser.Sanitize(options) - w.Write([]byte(ruser.ToJson())) + RemoveAllSessionsForUserId(user_id) + + data := make(map[string]string) + data["user_id"] = user_id + w.Write([]byte(model.MapToJson(data))) } -func UpdateRoles(c *Context, user *model.User, roles string) *model.User { +func UpdateUserRoles(c *Context, user *model.User, roles string) *model.User { user.Roles = roles diff --git a/api/user_test.go b/api/user_test.go index 629f7d257..1a3b36d4b 100644 --- a/api/user_test.go +++ b/api/user_test.go @@ -366,7 +366,7 @@ func TestGetUser(t *testing.T) { c := &Context{} c.RequestId = model.NewId() c.IpAddress = "cmd_line" - UpdateRoles(c, ruser.Data.(*model.User), model.ROLE_SYSTEM_ADMIN) + UpdateUserRoles(c, ruser.Data.(*model.User), model.ROLE_SYSTEM_ADMIN) Client.Login(user.Email, "pwd") @@ -791,7 +791,6 @@ func TestUserUpdateRoles(t *testing.T) { Client.Login(user.Email, "pwd") data["user_id"] = "junk" - data["new_roles"] = "admin" if _, err := Client.UpdateUserRoles(data); err == nil { t.Fatal("Should have errored, bad id") @@ -804,12 +803,141 @@ func TestUserUpdateRoles(t *testing.T) { } data["user_id"] = user2.Id + data["new_roles"] = "junk" if _, err := Client.UpdateUserRoles(data); err == nil { t.Fatal("Should have errored, bad role") } } +func TestUserUpdateRolesMoreCases(t *testing.T) { + th := Setup().InitSystemAdmin().InitBasic() + + data := make(map[string]string) + + // invalid team Id + data["user_id"] = th.BasicUser2.Id + data["new_roles"] = "" + data["team_id"] = model.NewId() + if _, err := th.BasicClient.UpdateUserRoles(data); err == nil { + t.Fatal("Should have errored") + } + + // user 1 is trying to change user 2 + data["user_id"] = th.BasicUser2.Id + data["new_roles"] = "" + data["team_id"] = th.BasicTeam.Id + if _, err := th.BasicClient.UpdateUserRoles(data); err == nil { + t.Fatal("Should have errored, you can only demote yourself") + } + + // user 1 is trying to promote user 2 + data["user_id"] = th.BasicUser2.Id + data["new_roles"] = model.ROLE_TEAM_ADMIN + data["team_id"] = th.BasicTeam.Id + if _, err := th.BasicClient.UpdateUserRoles(data); err == nil { + t.Fatal("Should have errored, you can only demote yourself") + } + + // user 1 is trying to promote user 2 + data["user_id"] = th.BasicUser2.Id + data["new_roles"] = model.ROLE_SYSTEM_ADMIN + data["team_id"] = th.BasicTeam.Id + if _, err := th.BasicClient.UpdateUserRoles(data); err == nil { + t.Fatal("Should have errored, you can only demote yourself") + } + + // user 1 is trying to promote himself + data["user_id"] = th.BasicUser.Id + data["new_roles"] = model.ROLE_TEAM_ADMIN + data["team_id"] = th.BasicTeam.Id + if _, err := th.BasicClient.UpdateUserRoles(data); err == nil { + t.Fatal("Should have errored, you cannot elevate your permissions") + } + + // user 1 is trying to promote himself + data["user_id"] = th.BasicUser.Id + data["new_roles"] = model.ROLE_SYSTEM_ADMIN + data["team_id"] = th.BasicTeam.Id + if _, err := th.BasicClient.UpdateUserRoles(data); err == nil { + t.Fatal("Should have errored, you cannot elevate your permissions") + } + + th.LoginSystemAdmin() + + // promote user to team admin + data["user_id"] = th.BasicUser.Id + data["new_roles"] = model.ROLE_TEAM_ADMIN + data["team_id"] = th.BasicTeam.Id + if _, err := th.SystemAdminClient.UpdateUserRoles(data); err != nil { + t.Fatal("Should have succeeded since they are system admin") + } + + // demote team admin to basic member + data["user_id"] = th.BasicUser.Id + data["new_roles"] = "" + data["team_id"] = th.BasicTeam.Id + if _, err := th.SystemAdminClient.UpdateUserRoles(data); err != nil { + t.Fatal("Should have succeeded since they are system admin") + } + + // re-promote user to team admin + data["user_id"] = th.BasicUser.Id + data["new_roles"] = model.ROLE_TEAM_ADMIN + data["team_id"] = th.BasicTeam.Id + if _, err := th.SystemAdminClient.UpdateUserRoles(data); err != nil { + t.Fatal("Should have succeeded since they are system admin") + } + + // user 1 is promoting user 2 to team admin + data["user_id"] = th.BasicUser2.Id + data["new_roles"] = model.ROLE_TEAM_ADMIN + data["team_id"] = th.BasicTeam.Id + if _, err := th.BasicClient.UpdateUserRoles(data); err == nil { + t.Fatal("Should have succeeded since they are team admin") + } + + // user 1 is trying to promote user 2 from team admin to system admin + data["user_id"] = th.BasicUser2.Id + data["new_roles"] = model.ROLE_SYSTEM_ADMIN + data["team_id"] = th.BasicTeam.Id + if _, err := th.BasicClient.UpdateUserRoles(data); err == nil { + t.Fatal("Should have errored, can only be system admin") + } + + // user 1 is demoting user 2 to a regular member + data["user_id"] = th.BasicUser2.Id + data["new_roles"] = "" + data["team_id"] = th.BasicTeam.Id + if _, err := th.BasicClient.UpdateUserRoles(data); err == nil { + t.Fatal("Should have succeeded since they are team admin") + } + + // user 1 is trying to demote system admin + data["user_id"] = th.SystemAdminUser.Id + data["new_roles"] = "" + data["team_id"] = th.BasicTeam.Id + if _, err := th.BasicClient.UpdateUserRoles(data); err == nil { + t.Fatal("Should have errored, can only be system admin") + } + + // user 1 as team admin is demoting himself + data["user_id"] = th.BasicUser.Id + data["new_roles"] = "" + data["team_id"] = th.BasicTeam.Id + if _, err := th.BasicClient.UpdateUserRoles(data); err != nil { + t.Fatal("Should have succeeded") + } + + // system admin demoting himself + data["user_id"] = th.SystemAdminUser.Id + data["new_roles"] = "" + data["team_id"] = "" + if _, err := th.SystemAdminClient.UpdateUserRoles(data); err != nil { + t.Fatal("Should have succeeded since they are system admin") + } +} + func TestUserUpdateDeviceId(t *testing.T) { th := Setup() Client := th.CreateClient() -- cgit v1.2.3-1-g7c22