Fixing update roles API (#4060)

api/team.go

@@ -35,6 +35,7 @@ func InitTeam() {
 
 	BaseRoutes.NeedTeam.Handle("/me", ApiUserRequired(getMyTeam)).Methods("GET")
 	BaseRoutes.NeedTeam.Handle("/update", ApiUserRequired(updateTeam)).Methods("POST")
+	BaseRoutes.NeedTeam.Handle("/update_member_roles", ApiUserRequired(updateMemberRoles)).Methods("POST")
 
 	BaseRoutes.NeedTeam.Handle("/invite_members", ApiUserRequired(inviteMembers)).Methods("POST")
 
@@ -784,6 +785,62 @@ func updateTeam(c *Context, w http.ResponseWriter, r *http.Request) {
 	w.Write([]byte(oldTeam.ToJson()))
 }
 
+func updateMemberRoles(c *Context, w http.ResponseWriter, r *http.Request) {
+	props := model.MapFromJson(r.Body)
+
+	userId := props["user_id"]
+	if len(userId) != 26 {
+		c.SetInvalidParam("updateMemberRoles", "user_id")
+		return
+	}
+
+	mchan := Srv.Store.Team().GetTeamsForUser(userId)
+
+	teamId := c.TeamId
+
+	newRoles := props["new_roles"]
+	if !(model.IsValidUserRoles(newRoles)) {
+		c.SetInvalidParam("updateMemberRoles", "new_roles")
+		return
+	}
+
+	if !HasPermissionToTeamContext(c, teamId, model.PERMISSION_MANAGE_ROLES) {
+		return
+	}
+
+	var member *model.TeamMember
+	if result := <-mchan; result.Err != nil {
+		c.Err = result.Err
+		return
+	} else {
+		members := result.Data.([]*model.TeamMember)
+		for _, m := range members {
+			if m.TeamId == teamId {
+				member = m
+			}
+		}
+	}
+
+	if member == nil {
+		c.Err = model.NewLocAppError("updateMemberRoles", "api.team.update_member_roles.not_a_member", nil, "userId="+userId+" teamId="+teamId)
+		c.Err.StatusCode = http.StatusBadRequest
+		return
+	}
+
+	member.Roles = newRoles
+
+	if result := <-Srv.Store.Team().UpdateMember(member); result.Err != nil {
+		c.Err = result.Err
+		return
+	}
+
+	RemoveAllSessionsForUserId(userId)
+
+	rdata := map[string]string{}
+	rdata["status"] = "ok"
+	w.Write([]byte(model.MapToJson(rdata)))
+}
+
 func PermanentDeleteTeam(c *Context, team *model.Team) *model.AppError {
 	l4g.Warn(utils.T("api.team.permanent_delete_team.attempting.warn"), team.Name, team.Id)
 	c.Path = "/teams/permanent_delete"

api/team_test.go

@@ -567,3 +567,66 @@ func TestGetTeamMembers(t *testing.T) {
 		t.Log(members)
 	}
 }
+
+func TestUpdateTeamMemberRoles(t *testing.T) {
+	th := Setup().InitSystemAdmin().InitBasic()
+	th.SystemAdminClient.SetTeamId(th.BasicTeam.Id)
+	LinkUserToTeam(th.SystemAdminUser, th.BasicTeam)
+
+	const BASIC_MEMBER = "team_user"
+	const TEAM_ADMIN = "team_user team_admin"
+
+	// user 1 trying to promote user 2
+	if _, err := th.BasicClient.UpdateTeamRoles(th.BasicUser2.Id, TEAM_ADMIN); err == nil {
+		t.Fatal("Should have errored, not team admin")
+	}
+
+	// user 1 trying to promote themselves
+	if _, err := th.BasicClient.UpdateTeamRoles(th.BasicUser.Id, TEAM_ADMIN); err == nil {
+		t.Fatal("Should have errored, not team admin")
+	}
+
+	// user 1 trying to demote someone
+	if _, err := th.BasicClient.UpdateTeamRoles(th.SystemAdminUser.Id, BASIC_MEMBER); err == nil {
+		t.Fatal("Should have errored, not team admin")
+	}
+
+	// system admin promoting user1
+	if _, err := th.SystemAdminClient.UpdateTeamRoles(th.BasicUser.Id, TEAM_ADMIN); err != nil {
+		t.Fatal("Should have worked: " + err.Error())
+	}
+
+	// user 1 trying to promote user 2
+	if _, err := th.BasicClient.UpdateTeamRoles(th.BasicUser2.Id, TEAM_ADMIN); err != nil {
+		t.Fatal("Should have worked, user is team admin: " + th.BasicUser.Id)
+	}
+
+	// user 1 trying to demote user 2
+	if _, err := th.BasicClient.UpdateTeamRoles(th.BasicUser2.Id, BASIC_MEMBER); err != nil {
+		t.Fatal("Should have worked, user is team admin")
+	}
+
+	// user 1 trying to demote a system admin
+	if _, err := th.BasicClient.UpdateTeamRoles(th.SystemAdminUser.Id, BASIC_MEMBER); err != nil {
+		t.Fatal("Should have worked, user is team admin and has the ability to manage permissions on this team.")
+		// Note to anyone who thinks this test is wrong:
+		// This operation will not effect the system admin's permissions because they have global access to all teams.
+		// Their team level permissions are irrelavent. A team admin should be able to manage team level permissions.
+	}
+
+	// System admins should be able to manipulate permission no matter what their team level permissions are.
+	// systemAdmin trying to promote user 2
+	if _, err := th.SystemAdminClient.UpdateTeamRoles(th.BasicUser2.Id, TEAM_ADMIN); err != nil {
+		t.Fatal("Should have worked, user is system admin")
+	}
+
+	// system admin trying to demote user 2
+	if _, err := th.SystemAdminClient.UpdateTeamRoles(th.BasicUser2.Id, BASIC_MEMBER); err != nil {
+		t.Fatal("Should have worked, user is system admin")
+	}
+
+	// user 1 trying to demote himself
+	if _, err := th.BasicClient.UpdateTeamRoles(th.BasicUser.Id, BASIC_MEMBER); err != nil {
+		t.Fatal("Should have worked, user is team admin")
+	}
+}

api/user.go

@@ -39,7 +39,6 @@ func InitUser() {
 
 	BaseRoutes.Users.Handle("/create", ApiAppHandler(createUser)).Methods("POST")
 	BaseRoutes.Users.Handle("/update", ApiUserRequired(updateUser)).Methods("POST")
-	BaseRoutes.Users.Handle("/update_roles", ApiUserRequired(updateRoles)).Methods("POST")
 	BaseRoutes.Users.Handle("/update_active", ApiUserRequired(updateActive)).Methods("POST")
 	BaseRoutes.Users.Handle("/update_notify", ApiUserRequired(updateUserNotify)).Methods("POST")
 	BaseRoutes.Users.Handle("/newpassword", ApiUserRequired(updatePassword)).Methods("POST")
@@ -71,6 +70,7 @@ func InitUser() {
 	BaseRoutes.NeedUser.Handle("/sessions", ApiUserRequired(getSessions)).Methods("GET")
 	BaseRoutes.NeedUser.Handle("/audits", ApiUserRequired(getAudits)).Methods("GET")
 	BaseRoutes.NeedUser.Handle("/image", ApiUserRequiredTrustRequester(getProfileImage)).Methods("GET")
+	BaseRoutes.NeedUser.Handle("/update_roles", ApiUserRequired(updateRoles)).Methods("POST")
 
 	BaseRoutes.Root.Handle("/login/sso/saml", AppHandlerIndependent(loginWithSaml)).Methods("GET")
 	BaseRoutes.Root.Handle("/login/sso/saml", AppHandlerIndependent(completeSaml)).Methods("POST")
@@ -1428,142 +1428,64 @@ func updatePassword(c *Context, w http.ResponseWriter, r *http.Request) {
 
 func updateRoles(c *Context, w http.ResponseWriter, r *http.Request) {
 	props := model.MapFromJson(r.Body)
+	params := mux.Vars(r)
 
-	user_id := props["user_id"]
+	userId := params["user_id"]
-	if len(user_id) != 26 {
+	if len(userId) != 26 {
-		c.SetInvalidParam("updateRoles", "user_id")
+		c.SetInvalidParam("updateMemberRoles", "user_id")
 		return
 	}
 
-	team_id := props["team_id"]
+	newRoles := props["new_roles"]
-
+	if !(model.IsValidUserRoles(newRoles)) {
-	// Set context TeamId as the team_id in the request cause at this point c.TeamId is empty
+		c.SetInvalidParam("updateMemberRoles", "new_roles")
-	if len(c.TeamId) == 0 {
-		c.TeamId = team_id
-	}
-
-	if !(len(user_id) == 26 || len(user_id) == 0) {
-		c.SetInvalidParam("updateRoles", "team_id")
 		return
 	}
 
-	new_roles := props["new_roles"]
+	if !HasPermissionToContext(c, model.PERMISSION_MANAGE_ROLES) {
-	if !(model.IsValidUserRoles(new_roles)) {
-		c.SetInvalidParam("updateRoles", "new_roles")
-		return
-	}
-
-	// If you are not the team admin then you can only demote yourself
-	if user_id != c.Session.UserId && !HasPermissionToTeamContext(c, team_id, model.PERMISSION_MANAGE_ROLES) {
-		c.Err = model.NewLocAppError("updateRoles", "api.user.update_roles.team_admin_needed.app_error", nil, "")
-		c.Err.StatusCode = http.StatusForbidden
-		return
-	}
-
-	// If your trying to assign the system admin role, you must have that permission
-	if model.IsInRole(new_roles, model.ROLE_SYSTEM_ADMIN.Id) && !HasPermissionToContext(c, model.PERMISSION_ASSIGN_SYSTEM_ADMIN_ROLE) {
-		c.Err = model.NewLocAppError("updateRoles", "api.user.update_roles.system_admin_set.app_error", nil, "")
 		return
 	}
 
 	var user *model.User
-	if result := <-Srv.Store.User().Get(user_id); result.Err != nil {
+	if result := <-Srv.Store.User().Get(userId); result.Err != nil {
 		c.Err = result.Err
 		return
 	} else {
 		user = result.Data.(*model.User)
 	}
 
-	// only another system admin can modify another system admin
+	UpdateUserRoles(c, user, newRoles)
-	if model.IsInRole(user.GetRawRoles(), model.ROLE_SYSTEM_ADMIN.Id) && !HasPermissionToContext(c, model.PERMISSION_ASSIGN_SYSTEM_ADMIN_ROLE) {
+	if c.Err != nil {
-		c.Err = model.NewLocAppError("updateRoles", "api.user.update_roles.system_admin_needed.app_error", nil, "")
-		c.Err.StatusCode = http.StatusForbidden
 		return
 	}
 
-	// if the team role has changed then lets update team members
+	rdata := map[string]string{}
-	if len(team_id) > 0 {
+	rdata["status"] = "ok"
-
+	w.Write([]byte(model.MapToJson(rdata)))
-		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
-			}
-		}
-
-		if member == nil {
-			c.SetInvalidParam("updateRoles", "team_id")
-			return
-		}
-
-		if !HasPermissionToContext(c, model.PERMISSION_MANAGE_SYSTEM) {
-			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 model.IsInRole(new_roles, model.ROLE_TEAM_ADMIN.Id) && !HasPermissionToCurrentTeamContext(c, model.PERMISSION_MANAGE_ROLES) {
-				c.Err = model.NewLocAppError("updateRoles", "api.user.update_roles.team_admin_needed.app_error", nil, "")
-				c.Err.StatusCode = http.StatusForbidden
-				return
-			}
-		}
-		c.Err = nil
-
-		member.Roles = new_roles
-
-		if result := <-Srv.Store.Team().UpdateMember(member); result.Err != nil {
-			c.Err = result.Err
-			return
-		}
-	} else {
-		// If the users role has changed then lets update the user
-		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)
-		}
-	}
-
-	RemoveAllSessionsForUserId(user_id)
-
-	data := make(map[string]string)
-	data["user_id"] = user_id
-	w.Write([]byte(model.MapToJson(data)))
 }
 
-func UpdateUserRoles(c *Context, user *model.User, roles string) *model.User {
+func UpdateUserRoles(c *Context, user *model.User, newRoles string) *model.User {
 
-	user.Roles = roles
+	user.Roles = newRoles
+	uchan := Srv.Store.User().Update(user, true)
+	schan := Srv.Store.Session().UpdateRoles(user.Id, newRoles)
 
 	var ruser *model.User
-	if result := <-Srv.Store.User().Update(user, true); result.Err != nil {
+	if result := <-uchan; result.Err != nil {
 		c.Err = result.Err
 		return nil
 	} else {
-		c.LogAuditWithUserId(user.Id, "roles="+roles)
+		c.LogAuditWithUserId(user.Id, "roles="+newRoles)
 		ruser = result.Data.([2]*model.User)[0]
 	}
 
+	if result := <-schan; result.Err != nil {
+		// soft error since the user roles were still updated
+		l4g.Error(result.Err)
+	}
+
+	RemoveAllSessionsForUserId(user.Id)
+
 	return ruser
 }
 

api/user_test.go

@@ -924,18 +924,14 @@ func TestUserUpdateRoles(t *testing.T) {
 	LinkUserToTeam(user2, team)
 	store.Must(Srv.Store.User().VerifyEmail(user2.Id))
 
-	data := make(map[string]string)
+	if _, err := Client.UpdateUserRoles(user.Id, ""); err == nil {
-	data["user_id"] = user.Id
-	data["new_roles"] = ""
-
-	if _, err := Client.UpdateUserRoles(data); err == nil {
 		t.Fatal("Should have errored, not logged in")
 	}
 
 	Client.Login(user2.Email, "passwd1")
 	Client.SetTeamId(team.Id)
 
-	if _, err := Client.UpdateUserRoles(data); err == nil {
+	if _, err := Client.UpdateUserRoles(user.Id, ""); err == nil {
 		t.Fatal("Should have errored, not admin")
 	}
 
@@ -950,158 +946,78 @@ func TestUserUpdateRoles(t *testing.T) {
 	Client.Login(user3.Email, "passwd1")
 	Client.SetTeamId(team2.Id)
 
-	data["user_id"] = user2.Id
+	if _, err := Client.UpdateUserRoles(user2.Id, ""); err == nil {
-
-	if _, err := Client.UpdateUserRoles(data); err == nil {
 		t.Fatal("Should have errored, wrong team")
 	}
 
 	Client.Login(user.Email, "passwd1")
 
-	data["user_id"] = "junk"
+	if _, err := Client.UpdateUserRoles("junk", ""); err == nil {
-
-	if _, err := Client.UpdateUserRoles(data); err == nil {
 		t.Fatal("Should have errored, bad id")
 	}
 
-	data["user_id"] = "12345678901234567890123456"
+	if _, err := Client.UpdateUserRoles("12345678901234567890123456", ""); err == nil {
-
-	if _, err := Client.UpdateUserRoles(data); err == nil {
 		t.Fatal("Should have errored, bad id")
 	}
 
-	data["user_id"] = user2.Id
+	if _, err := Client.UpdateUserRoles(user2.Id, "junk"); err == nil {
-	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()
+	th.SystemAdminClient.SetTeamId(th.BasicTeam.Id)
+	LinkUserToTeam(th.SystemAdminUser, th.BasicTeam)
 
-	data := make(map[string]string)
+	const BASIC_USER = "system_user"
-
+	const SYSTEM_ADMIN = "system_user system_admin"
-	// 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
+	if _, err := th.BasicClient.UpdateUserRoles(th.BasicUser2.Id, SYSTEM_ADMIN); err == nil {
-	data["new_roles"] = model.ROLE_TEAM_ADMIN.Id
+		t.Fatal("Should have errored, basic user is not a 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 user 2
-	data["user_id"] = th.BasicUser2.Id
-	data["new_roles"] = model.ROLE_SYSTEM_ADMIN.Id
-	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.Id
-	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.Id
-	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.Id
-	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.Id
-	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.Id
-	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.Id
-	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
+	if _, err := th.BasicClient.UpdateUserRoles(th.SystemAdminUser.Id, BASIC_USER); err == nil {
-	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
+	// user 1 is trying to promote himself
-	data["user_id"] = th.BasicUser.Id
+	if _, err := th.BasicClient.UpdateUserRoles(th.BasicUser.Id, SYSTEM_ADMIN); err == nil {
-	data["new_roles"] = ""
+		t.Fatal("Should have errored, can only be system admin")
-	data["team_id"] = th.BasicTeam.Id
+	}
-	if _, err := th.BasicClient.UpdateUserRoles(data); err != nil {
+
-		t.Fatal("Should have succeeded")
+	// System admin promoting user 2
+	if _, err := th.SystemAdminClient.UpdateUserRoles(th.BasicUser2.Id, SYSTEM_ADMIN); err != nil {
+		t.Fatal("Should have succeeded since they are system admin")
+	}
+
+	// System admin demoting user 2
+	if _, err := th.SystemAdminClient.UpdateUserRoles(th.BasicUser2.Id, BASIC_USER); err != nil {
+		t.Fatal("Should have succeeded since they are system admin")
+	}
+
+	// Setting user to team admin should have no effect on results
+	th.BasicClient.Must(th.SystemAdminClient.UpdateTeamRoles(th.BasicUser.Id, "team_user team_admin"))
+
+	// user 1 is trying to promote user 2
+	if _, err := th.BasicClient.UpdateUserRoles(th.BasicUser2.Id, SYSTEM_ADMIN); err == nil {
+		t.Fatal("Should have errored, basic user is not a system admin")
+	}
+
+	// user 1 is trying to demote system admin
+	if _, err := th.BasicClient.UpdateUserRoles(th.SystemAdminUser.Id, BASIC_USER); err == nil {
+		t.Fatal("Should have errored, can only be system admin")
+	}
+
+	// user 1 is trying to promote himself
+	if _, err := th.BasicClient.UpdateUserRoles(th.BasicUser.Id, SYSTEM_ADMIN); err == nil {
+		t.Fatal("Should have errored, can only be system admin")
 	}
 
 	// system admin demoting himself
-	data["user_id"] = th.SystemAdminUser.Id
+	if _, err := th.SystemAdminClient.UpdateUserRoles(th.SystemAdminUser.Id, BASIC_USER); err != nil {
-	data["new_roles"] = ""
-	data["team_id"] = ""
-	if _, err := th.SystemAdminClient.UpdateUserRoles(data); err != nil {
 		t.Fatal("Should have succeeded since they are system admin")
 	}
 }

i18n/en.json

@@ -1559,6 +1559,10 @@
     "id": "api.status.save_status.error",
     "translation": "Failed to save status for user_id=%v, err=%v"
   },
+  {
+    "id": "api.team.update_member_roles.not_a_member",
+    "translation": "Specified user is not a member of specified team."
+  },
   {
     "id": "api.team.create_team.email_disabled.app_error",
     "translation": "Team sign-up with email is disabled."

model/client.go

@@ -108,6 +108,10 @@ func (c *Client) GetChannelRoute(channelId string) string {
 	return fmt.Sprintf("/teams/%v/channels/%v", c.GetTeamId(), channelId)
 }
 
+func (c *Client) GetUserRequiredRoute(userId string) string {
+	return fmt.Sprintf("/users/%v", userId)
+}
+
 func (c *Client) GetChannelNameRoute(channelName string) string {
 	return fmt.Sprintf("/teams/%v/channels/name/%v", c.GetTeamId(), channelName)
 }
@@ -1370,8 +1374,25 @@ func (c *Client) UpdateUser(user *User) (*Result, *AppError) {
 	}
 }
 
-func (c *Client) UpdateUserRoles(data map[string]string) (*Result, *AppError) {
+func (c *Client) UpdateUserRoles(userId string, roles string) (*Result, *AppError) {
-	if r, err := c.DoApiPost("/users/update_roles", MapToJson(data)); err != nil {
+	data := make(map[string]string)
+	data["new_roles"] = roles
+
+	if r, err := c.DoApiPost(c.GetUserRequiredRoute(userId)+"/update_roles", MapToJson(data)); err != nil {
+		return nil, err
+	} else {
+		defer closeBody(r)
+		return &Result{r.Header.Get(HEADER_REQUEST_ID),
+			r.Header.Get(HEADER_ETAG_SERVER), MapFromJson(r.Body)}, nil
+	}
+}
+
+func (c *Client) UpdateTeamRoles(userId string, roles string) (*Result, *AppError) {
+	data := make(map[string]string)
+	data["new_roles"] = roles
+	data["user_id"] = userId
+
+	if r, err := c.DoApiPost(c.GetTeamRoute()+"/update_member_roles", MapToJson(data)); err != nil {
 		return nil, err
 	} else {
 		defer closeBody(r)

webapp/client/client.jsx

@@ -77,6 +77,10 @@ export default class Client {
         return `${this.url}${this.urlVersion}/teams/${this.getTeamId()}`;
     }
 
+    getTeamNeededManualRoute(teamId) {
+        return `${this.url}${this.urlVersion}/teams/${teamId}`;
+    }
+
     getChannelsRoute() {
         return `${this.url}${this.urlVersion}/teams/${this.getTeamId()}/channels`;
     }
@@ -709,22 +713,37 @@ export default class Client {
         this.track('api', 'api_users_update_notification_settings');
     }
 
-    updateRoles(teamId, userId, newRoles, success, error) {
+    updateUserRoles(userId, newRoles, success, error) {
+        var data = {
+            new_roles: newRoles
+        };
+
+        request.
+            post(`${this.getUserNeededRoute(userId)}/update_roles`).
+            set(this.defaultHeaders).
+            type('application/json').
+            accept('application/json').
+            send(data).
+            end(this.handleResponse.bind(this, 'updateUserRoles', success, error));
+
+        this.track('api', 'api_users_update_user_roles');
+    }
+
+    updateTeamMemberRoles(teamId, userId, newRoles, success, error) {
         var data = {
-            team_id: teamId,
             user_id: userId,
             new_roles: newRoles
         };
 
         request.
-            post(`${this.getUsersRoute()}/update_roles`).
+            post(`${this.getTeamNeededManualRoute(teamId)}/update_member_roles`).
             set(this.defaultHeaders).
             type('application/json').
             accept('application/json').
             send(data).
-            end(this.handleResponse.bind(this, 'updateRoles', success, error));
+            end(this.handleResponse.bind(this, 'updateTeamMemberRoles', success, error));
 
-        this.track('api', 'api_users_update_roles');
+        this.track('api', 'api_teams_update_member_roles');
     }
 
     updateActive(userId, active, success, error) {
@@ -740,7 +759,7 @@ export default class Client {
             send(data).
             end(this.handleResponse.bind(this, 'updateActive', success, error));
 
-        this.track('api', 'api_users_update_roles');
+        this.track('api', 'api_users_update_active');
     }
 
     sendPasswordReset(email, success, error) {

webapp/components/admin_console/user_item.jsx

@@ -20,13 +20,15 @@ export default class UserItem extends React.Component {
         this.handleRemoveFromTeam = this.handleRemoveFromTeam.bind(this);
         this.handleMakeActive = this.handleMakeActive.bind(this);
         this.handleMakeNotActive = this.handleMakeNotActive.bind(this);
-        this.handleMakeAdmin = this.handleMakeAdmin.bind(this);
+        this.handleMakeTeamAdmin = this.handleMakeTeamAdmin.bind(this);
         this.handleMakeSystemAdmin = this.handleMakeSystemAdmin.bind(this);
         this.handleResetPassword = this.handleResetPassword.bind(this);
         this.handleResetMfa = this.handleResetMfa.bind(this);
-        this.handleDemote = this.handleDemote.bind(this);
+        this.handleDemoteSystemAdmin = this.handleDemoteSystemAdmin.bind(this);
         this.handleDemoteSubmit = this.handleDemoteSubmit.bind(this);
         this.handleDemoteCancel = this.handleDemoteCancel.bind(this);
+        this.doMakeMember = this.doMakeMember.bind(this);
+        this.doMakeTeamAdmin = this.doMakeTeamAdmin.bind(this);
 
         this.state = {
             serverError: null,
@@ -36,23 +38,37 @@ export default class UserItem extends React.Component {
         };
     }
 
+    doMakeMember() {
+        Client.updateUserRoles(
+            this.props.user.id,
+            'system_user',
+            () => {
+                this.props.refreshProfiles();
+            },
+            (err) => {
+                this.setState({serverError: err.message});
+            }
+        );
+        Client.updateTeamMemberRoles(
+            this.props.team.id,
+            this.props.user.id,
+            'team_user',
+            () => {
+                this.props.refreshProfiles();
+            },
+            (err) => {
+                this.setState({serverError: err.message});
+            }
+        );
+    }
+
     handleMakeMember(e) {
         e.preventDefault();
         const me = UserStore.getCurrentUser();
         if (this.props.user.id === me.id) {
-            this.handleDemote(this.props.user, '');
+            this.handleDemoteSystemAdmin(this.props.user, 'member');
         } else {
-            Client.updateRoles(
+            this.doMakeMember();
-                this.props.team.id,
-                this.props.user.id,
-                '',
-                () => {
-                    this.props.refreshProfiles();
-                },
-                (err) => {
-                    this.setState({serverError: err.message});
-                }
-            );
         }
     }
 
@@ -93,31 +109,34 @@ export default class UserItem extends React.Component {
         );
     }
 
-    handleMakeAdmin(e) {
+    doMakeTeamAdmin() {
+        Client.updateTeamMemberRoles(
+            this.props.team.id,
+            this.props.user.id,
+            'team_user team_admin',
+            () => {
+                this.props.refreshProfiles();
+            },
+            (err) => {
+                this.setState({serverError: err.message});
+            }
+        );
+    }
+
+    handleMakeTeamAdmin(e) {
         e.preventDefault();
         const me = UserStore.getCurrentUser();
         if (this.props.user.id === me.id) {
-            this.handleDemote(this.props.user, 'team_user team_admin');
+            this.handleDemoteSystemAdmin(this.props.user, 'teamadmin');
         } else {
-            Client.updateRoles(
+            this.doMakeTeamAdmin();
-                this.props.team.id,
-                this.props.user.id,
-                'team_user team_admin',
-                () => {
-                    this.props.refreshProfiles();
-                },
-                (err) => {
-                    this.setState({serverError: err.message});
-                }
-            );
         }
     }
 
     handleMakeSystemAdmin(e) {
         e.preventDefault();
 
-        Client.updateRoles(
+        Client.updateUserRoles(
-            this.props.team.id,
             this.props.user.id,
             'system_user system_admin',
             () => {
@@ -147,7 +166,7 @@ export default class UserItem extends React.Component {
         );
     }
 
-    handleDemote(user, role) {
+    handleDemoteSystemAdmin(user, role) {
         this.setState({
             serverError: this.state.serverError,
             showDemoteModal: true,
@@ -166,34 +185,19 @@ export default class UserItem extends React.Component {
     }
 
     handleDemoteSubmit() {
-        Client.updateRoles(
+        if (this.state.role === 'member') {
-            this.props.team.id,
+            this.doMakeMember();
-            this.props.user.id,
+        } else {
-            this.state.role,
+            this.doMakeTeamAdmin();
-            () => {
+        }
-                this.props.refreshProfiles();
 
-                this.setState({
+        const teamUrl = TeamStore.getCurrentTeamUrl();
-                    serverError: null,
+        if (teamUrl) {
-                    showDemoteModal: false,
+            // the channel is added to the URL cause endless loading not being fully fixed
-                    user: null,
+            window.location.href = teamUrl + '/channels/town-square';
-                    role: null
+        } else {
-                });
+            window.location.href = '/';
-
+        }
-                const teamUrl = TeamStore.getCurrentTeamUrl();
-                if (teamUrl) {
-                    // the channel is added to the URL cause endless loading not being fully fixed
-                    window.location.href = teamUrl + '/channels/town-square';
-                } else {
-                    window.location.href = '/';
-                }
-            },
-            (err) => {
-                this.setState({
-                    serverError: err.message
-                });
-            }
-        );
     }
 
     render() {
@@ -290,7 +294,7 @@ export default class UserItem extends React.Component {
                     <a
                         role='menuitem'
                         href='#'
-                        onClick={this.handleMakeAdmin}
+                        onClick={this.handleMakeTeamAdmin}
                     >
                         <FormattedMessage
                             id='admin.user_item.makeTeamAdmin'

webapp/components/team_members_dropdown.jsx

@@ -39,7 +39,7 @@ export default class TeamMembersDropdown extends React.Component {
         if (this.props.user.id === me.id) {
             this.handleDemote(this.props.user, 'team_user');
         } else {
-            Client.updateRoles(
+            Client.updateTeamMemberRoles(
                 this.props.teamMember.team_id,
                 this.props.user.id,
                 'team_user',
@@ -95,7 +95,7 @@ export default class TeamMembersDropdown extends React.Component {
         if (this.props.user.id === me.id) {
             this.handleDemote(this.props.user, 'team_user team_admin');
         } else {
-            Client.updateRoles(
+            Client.updateTeamMemberRoles(
                 this.props.teamMember.team_id,
                 this.props.user.id,
                 'team_user team_admin',
@@ -128,7 +128,7 @@ export default class TeamMembersDropdown extends React.Component {
         });
     }
     handleDemoteSubmit() {
-        Client.updateRoles(
+        Client.updateTeamMemberRoles(
             this.props.teamMember.team_id,
             this.props.user.id,
             this.state.newRole,

webapp/tests/client_team.test.jsx

@@ -240,5 +240,24 @@ describe('Client.Team', function() {
             );
         });
     });
+
+    it('updateTeamMemberRoles', function(done) {
+        TestHelper.initBasic(() => {
+            var user = TestHelper.basicUser();
+            var team = TestHelper.basicTeam();
+
+            TestHelper.basicClient().updateTeamMemberRoles(
+                team.id,
+                user.id,
+                '',
+                function() {
+                    done();
+                },
+                function(err) {
+                    done(new Error(err.message));
+                }
+            );
+        });
+    });
 });
 

webapp/tests/client_user.test.jsx

@@ -211,21 +211,18 @@ describe('Client.User', function() {
         });
     });
 
-    it('updateRoles', function(done) {
+    it('updateUserRoles', function(done) {
         TestHelper.initBasic(() => {
             var user = TestHelper.basicUser();
-            var team = TestHelper.basicTeam();
 
-            TestHelper.basicClient().updateRoles(
+            TestHelper.basicClient().updateUserRoles(
-                team.id,
                 user.id,
                 '',
-                function(data) {
+                function() {
-                    assert.equal(data.user_id, user.id);
+                    done(new Error('Not supposed to work'));
-                    done();
                 },
-                function(err) {
+                function() {
-                    done(new Error(err.message));
+                    done();
                 }
             );
         });