Ignore blank role names in getRolesByName call. (#8507)

2 files changed, 19 insertions, 3 deletions

diff --git a/api4/role.go b/api4/role.go
index e7654011d..c4203137b 100644
--- a/api4/role.go
+++ b/api4/role.go
@@ -5,6 +5,7 @@ package api4
 
 import (
 	"net/http"
+	"strings"
 
 	"github.com/mattermost/mattermost-server/model"
 )
@@ -52,14 +53,21 @@ func getRolesByNames(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	var cleanedRoleNames []string
 	for _, rolename := range rolenames {
+		if strings.TrimSpace(rolename) == "" {
+			continue
+		}
+
 		if !model.IsValidRoleName(rolename) {
 			c.SetInvalidParam("rolename")
 			return
 		}
+
+		cleanedRoleNames = append(cleanedRoleNames, rolename)
 	}
 
-	if roles, err := c.App.GetRolesByNames(rolenames); err != nil {
+	if roles, err := c.App.GetRolesByNames(cleanedRoleNames); err != nil {
 		c.Err = err
 		return
 	} else {
diff --git a/api4/role_test.go b/api4/role_test.go
index 3fbf6808d..c5d8e303e 100644
--- a/api4/role_test.go
+++ b/api4/role_test.go
@@ -129,13 +129,21 @@ func TestGetRolesByNames(t *testing.T) {
 	assert.Contains(t, received, role2)
 	assert.Contains(t, received, role3)
 
-	// Check a list of invalid roles.
-	// TODO: Confirm whether no error for invalid role names is intended.
+	// Check a list of non-existant roles.
 	received, resp = th.Client.GetRolesByNames([]string{model.NewId(), model.NewId()})
 	CheckNoError(t, resp)
 
+	// Empty list should error.
 	_, resp = th.SystemAdminClient.GetRolesByNames([]string{})
 	CheckBadRequestStatus(t, resp)
+
+	// Invalid role name should error.
+	received, resp = th.Client.GetRolesByNames([]string{model.NewId(), model.NewId(), "!!!!!!"})
+	CheckBadRequestStatus(t, resp)
+
+	// Empty/whitespace rolenames should be ignored.
+	received, resp = th.Client.GetRolesByNames([]string{model.NewId(), model.NewId(), "", "    "})
+	CheckNoError(t, resp)
 }
 
 func TestPatchRole(t *testing.T) {