PLT-3502 Fix Team admins can't give "team admin" privilege to members (#3499)

2 files changed, 18 insertions, 5 deletions

diff --git a/api/user.go b/api/user.go
index 47f20f6bf..38ee05a22 100644
--- a/api/user.go
+++ b/api/user.go
@@ -1402,6 +1402,12 @@ func updateRoles(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 
 	team_id := props["team_id"]
+
+	// Set context TeamId as the team_id in the request cause at this point c.TeamId is empty
+	if len(c.TeamId) == 0 {
+		c.TeamId = team_id
+	}
+
 	if !(len(user_id) == 26 || len(user_id) == 0) {
 		c.SetInvalidParam("updateRoles", "team_id")
 		return
@@ -1413,9 +1419,9 @@ func updateRoles(c *Context, w http.ResponseWriter, r *http.Request) {
 		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_needed.app_error", nil, "")
+	// If you are not the team admin then you can only demote yourself
+	if !c.IsTeamAdmin() && user_id != c.Session.UserId {
+		c.Err = model.NewLocAppError("updateRoles", "api.user.update_roles.team_admin_needed.app_error", nil, "")
 		c.Err.StatusCode = http.StatusForbidden
 		return
 	}
@@ -1435,6 +1441,13 @@ func updateRoles(c *Context, w http.ResponseWriter, r *http.Request) {
 		user = result.Data.(*model.User)
 	}
 
+	// only another system admin can remove another system admin
+	if model.IsInRole(user.Roles, model.ROLE_SYSTEM_ADMIN) && !c.IsSystemAdmin() {
+		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
 	if model.IsValidTeamRoles(new_roles) && len(team_id) > 0 {
 
diff --git a/api/user_test.go b/api/user_test.go
index c34d32c11..311a5ea21 100644
--- a/api/user_test.go
+++ b/api/user_test.go
@@ -910,7 +910,7 @@ func TestUserUpdateRolesMoreCases(t *testing.T) {
 	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 {
+	if _, err := th.BasicClient.UpdateUserRoles(data); err != nil {
 		t.Fatal("Should have succeeded since they are team admin")
 	}
 
@@ -926,7 +926,7 @@ func TestUserUpdateRolesMoreCases(t *testing.T) {
 	data["user_id"] = th.BasicUser2.Id
 	data["new_roles"] = ""
 	data["team_id"] = th.BasicTeam.Id
-	if _, err := th.BasicClient.UpdateUserRoles(data); err == nil {
+	if _, err := th.BasicClient.UpdateUserRoles(data); err != nil {
 		t.Fatal("Should have succeeded since they are team admin")
 	}