MM-10234: Make CLI roles command advanced-permissions aware. (#8771)

* MM-10234: Make CLI roles command advanced-permissions aware.

* Fix for loop scope.

* Fix style.

2 files changed, 58 insertions, 5 deletions

diff --git a/cmd/commands/roles.go b/cmd/commands/roles.go
index 6d832d82a..72192c925 100644
--- a/cmd/commands/roles.go
+++ b/cmd/commands/roles.go
@@ -5,9 +5,12 @@ package commands
 
 import (
 	"errors"
+	"strings"
 
-	"github.com/mattermost/mattermost-server/cmd"
 	"github.com/spf13/cobra"
+
+	"github.com/mattermost/mattermost-server/cmd"
+	"github.com/mattermost/mattermost-server/model"
 )
 
 var RolesCmd = &cobra.Command{
@@ -56,7 +59,27 @@ func makeSystemAdminCmdF(command *cobra.Command, args []string) error {
 			return errors.New("Unable to find user '" + args[i] + "'")
 		}
 
-		if _, err := a.UpdateUserRoles(user.Id, "system_admin system_user", true); err != nil {
+		systemAdmin := false
+		systemUser := false
+
+		roles := strings.Fields(user.Roles)
+		for _, role := range roles {
+			switch role {
+			case model.SYSTEM_ADMIN_ROLE_ID:
+				systemAdmin = true
+			case model.SYSTEM_USER_ROLE_ID:
+				systemUser = true
+			}
+		}
+
+		if !systemUser {
+			roles = append(roles, model.SYSTEM_USER_ROLE_ID)
+		}
+		if !systemAdmin {
+			roles = append(roles, model.SYSTEM_ADMIN_ROLE_ID)
+		}
+
+		if _, err := a.UpdateUserRoles(user.Id, strings.Join(roles, " "), true); err != nil {
 			return err
 		}
 	}
@@ -81,7 +104,26 @@ func makeMemberCmdF(command *cobra.Command, args []string) error {
 			return errors.New("Unable to find user '" + args[i] + "'")
 		}
 
-		if _, err := a.UpdateUserRoles(user.Id, "system_user", true); err != nil {
+		systemUser := false
+		var newRoles []string
+
+		roles := strings.Fields(user.Roles)
+		for _, role := range roles {
+			switch role {
+			case model.SYSTEM_ADMIN_ROLE_ID:
+			default:
+				if role == model.SYSTEM_USER_ROLE_ID {
+					systemUser = true
+				}
+				newRoles = append(newRoles, role)
+			}
+		}
+
+		if !systemUser {
+			newRoles = append(roles, model.SYSTEM_USER_ROLE_ID)
+		}
+
+		if _, err := a.UpdateUserRoles(user.Id, strings.Join(newRoles, " "), true); err != nil {
 			return err
 		}
 	}
diff --git a/cmd/commands/roles_test.go b/cmd/commands/roles_test.go
index 1e0a46a4e..7179a9157 100644
--- a/cmd/commands/roles_test.go
+++ b/cmd/commands/roles_test.go
@@ -21,8 +21,19 @@ func TestAssignRole(t *testing.T) {
 		t.Fatal()
 	} else {
 		user := result.Data.(*model.User)
-		if user.Roles != "system_admin system_user" {
-			t.Fatal()
+		if user.Roles != "system_user system_admin" {
+			t.Fatal("Got wrong roles:", user.Roles)
+		}
+	}
+
+	cmd.CheckCommand(t, "roles", "member", th.BasicUser.Email)
+
+	if result := <-th.App.Srv.Store.User().GetByEmail(th.BasicUser.Email); result.Err != nil {
+		t.Fatal()
+	} else {
+		user := result.Data.(*model.User)
+		if user.Roles != "system_user" {
+			t.Fatal("Got wrong roles:", user.Roles, user.Id)
 		}
 	}
 }