[MM-55323] Allow end users to fetch the group members list of groups which allow @-mentions (#26551)

* Allow end users to fetch the group members list of groups which allow @-mentions

* Update server/channels/api4/group_test.go

Co-authored-by: Ibrahim Serdar Acikgoz <serdaracikgoz86@gmail.com>

* Fix test name

* Move into subtest

---------

Co-authored-by: Ibrahim Serdar Acikgoz <serdaracikgoz86@gmail.com>

server/channels/api4/group.go

@@ -16,6 +16,7 @@ import (
 	"github.com/mattermost/mattermost/server/v8/channels/app"
 	"github.com/mattermost/mattermost/server/v8/channels/audit"
 	"github.com/mattermost/mattermost/server/v8/channels/store"
+	"github.com/mattermost/mattermost/server/v8/channels/web"
 )
 
 func (api *API) InitGroup() {
@@ -695,24 +696,13 @@ func getGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	group, appErr := c.App.GetGroup(c.Params.GroupId, nil, nil)
+	appErr := hasPermissionToReadGroupMembers(c, c.Params.GroupId)
-	if appErr != nil {
-		c.Err = appErr
-		return
-	}
-
-	appErr = licensedAndConfiguredForGroupBySource(c.App, group.Source)
 	if appErr != nil {
 		appErr.Where = "Api4.getGroupMembers"
 		c.Err = appErr
 		return
 	}
 
-	if group.Source == model.GroupSourceLdap && !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleReadUserManagementGroups) {
-		c.SetPermissionError(model.PermissionSysconsoleReadUserManagementGroups)
-		return
-	}
-
 	restrictions, appErr := c.App.GetViewUsersRestrictions(c.AppContext, c.AppContext.Session().UserId)
 	if appErr != nil {
 		c.Err = appErr
@@ -725,10 +715,7 @@ func getGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	b, err := json.Marshal(struct {
+	b, err := json.Marshal(model.GroupMemberList{
-		Members []*model.User `json:"members"`
-		Count   int           `json:"total_member_count"`
-	}{
 		Members: members,
 		Count:   count,
 	})
@@ -1376,6 +1363,26 @@ func deleteGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) {
 	w.Write(b)
 }
 
+// hasPermissionToReadGroupMembers check if a user has the permission to read the list of members of a given team.
+func hasPermissionToReadGroupMembers(c *web.Context, groupID string) *model.AppError {
+	group, err := c.App.GetGroup(groupID, nil, nil)
+	if err != nil {
+		return err
+	}
+
+	if lcErr := licensedAndConfiguredForGroupBySource(c.App, group.Source); lcErr != nil {
+		return lcErr
+	}
+
+	if group.Source == model.GroupSourceLdap && !group.AllowReference {
+		if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleReadUserManagementGroups) {
+			return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionSysconsoleReadUserManagementGroups})
+		}
+	}
+
+	return nil
+}
+
 // licensedAndConfiguredForGroupBySource returns an app error if not properly license or configured for the given group type. The returned app error
 // will have a blank 'Where' field, which should be subsequently set by the caller, for example:
 //

server/channels/api4/group_test.go

@@ -1704,6 +1704,72 @@ func TestGetGroupsByUserId(t *testing.T) {
 	assert.ElementsMatch(t, []*model.Group{group1, group2}, groups)
 }
 
+func TestGetGroupMembers(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	id := model.NewId()
+	group, appErr := th.App.CreateGroup(&model.Group{
+		DisplayName: "dn-foo_" + id,
+		Name:        model.NewString("name" + id),
+		Source:      model.GroupSourceLdap,
+		Description: "description_" + id,
+		RemoteId:    model.NewString(model.NewId()),
+	})
+	assert.Nil(t, appErr)
+
+	user1, appErr := th.App.CreateUser(th.Context, &model.User{Email: th.GenerateTestEmail(), Nickname: "test user1", Password: "test-password-1", Username: "test-user-1", Roles: model.SystemUserRoleId})
+	assert.Nil(t, appErr)
+
+	user2, appErr := th.App.CreateUser(th.Context, &model.User{Email: th.GenerateTestEmail(), Nickname: "test user2", Password: "test-password-2", Username: "test-user-2", Roles: model.SystemUserRoleId})
+	assert.Nil(t, appErr)
+
+	_, appErr = th.App.UpsertGroupMembers(group.Id, []string{user1.Id, user2.Id})
+	require.Nil(t, appErr)
+
+	t.Run("Requires ldap license", func(t *testing.T) {
+		members, response, err := th.SystemAdminClient.GetGroupMembers(context.Background(), group.Id)
+		assert.Error(t, err)
+		CheckNotImplementedStatus(t, response)
+		assert.Nil(t, members)
+	})
+
+	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
+
+	t.Run("Non admins are not allowed to get members for LDAP groups", func(t *testing.T) {
+		members, response, err := th.Client.GetGroupMembers(context.Background(), group.Id)
+		assert.Error(t, err)
+		CheckForbiddenStatus(t, response)
+		assert.Nil(t, members)
+	})
+
+	t.Run("Admins are allowed to get members for LDAP groups", func(t *testing.T) {
+		members, response, err := th.SystemAdminClient.GetGroupMembers(context.Background(), group.Id)
+		assert.NoError(t, err)
+		CheckOKStatus(t, response)
+		require.NotNil(t, members)
+		assert.Equal(t, 2, members.Count)
+	})
+
+	t.Run("If AllowReference is enabled, non admins are allowed to get members for LDAP groups", func(t *testing.T) {
+		group.AllowReference = true
+		group, appErr = th.App.UpdateGroup(group)
+		assert.Nil(t, appErr)
+
+		t.Cleanup(func() {
+			group.AllowReference = false
+			group, appErr = th.App.UpdateGroup(group)
+			assert.Nil(t, appErr)
+		})
+
+		members, response, err := th.Client.GetGroupMembers(context.Background(), group.Id)
+		assert.NoError(t, err)
+		CheckOKStatus(t, response)
+		require.NotNil(t, members)
+		assert.Equal(t, 2, members.Count)
+	})
+}
+
 func TestGetGroupStats(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()

server/channels/api4/user.go

@@ -20,7 +20,6 @@ import (
 	"github.com/mattermost/mattermost/server/v8/channels/audit"
 	"github.com/mattermost/mattermost/server/v8/channels/store"
 	"github.com/mattermost/mattermost/server/v8/channels/utils"
-	"github.com/mattermost/mattermost/server/v8/channels/web"
 )
 
 func (api *API) InitUser() {
@@ -875,7 +874,7 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) {
 			profiles, appErr = c.App.GetUsersInChannelPage(userGetOptions, c.IsSystemAdmin())
 		}
 	} else if inGroupId != "" {
-		if gErr := requireGroupAccess(c, inGroupId); gErr != nil {
+		if gErr := hasPermissionToReadGroupMembers(c, inGroupId); gErr != nil {
 			gErr.Where = "Api.getUsers"
 			c.Err = gErr
 			return
@@ -895,7 +894,7 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) {
 			profiles, _, appErr = c.App.GetGroupMemberUsersPage(inGroupId, c.Params.Page, c.Params.PerPage, userGetOptions.ViewRestrictions)
 		}
 	} else if notInGroupId != "" {
-		appErr = requireGroupAccess(c, notInGroupId)
+		appErr = hasPermissionToReadGroupMembers(c, notInGroupId)
 		if appErr != nil {
 			appErr.Where = "Api.getUsers"
 			c.Err = appErr
@@ -935,25 +934,6 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) {
 	w.Write(js)
 }
 
-func requireGroupAccess(c *web.Context, groupID string) *model.AppError {
-	group, err := c.App.GetGroup(groupID, nil, nil)
-	if err != nil {
-		return err
-	}
-
-	if lcErr := licensedAndConfiguredForGroupBySource(c.App, group.Source); lcErr != nil {
-		return lcErr
-	}
-
-	if group.Source == model.GroupSourceLdap {
-		if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleReadUserManagementGroups) {
-			return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionSysconsoleReadUserManagementGroups})
-		}
-	}
-
-	return nil
-}
-
 func getUsersByIds(c *Context, w http.ResponseWriter, r *http.Request) {
 	userIDs, err := model.SortedArrayFromJSON(r.Body)
 	if err != nil {
@@ -1067,7 +1047,7 @@ func searchUsers(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 
 	if props.InGroupId != "" {
-		if appErr := requireGroupAccess(c, props.InGroupId); appErr != nil {
+		if appErr := hasPermissionToReadGroupMembers(c, props.InGroupId); appErr != nil {
 			appErr.Where = "Api.searchUsers"
 			c.Err = appErr
 			return
@@ -1075,7 +1055,7 @@ func searchUsers(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 
 	if props.NotInGroupId != "" {
-		if appErr := requireGroupAccess(c, props.NotInGroupId); appErr != nil {
+		if appErr := hasPermissionToReadGroupMembers(c, props.NotInGroupId); appErr != nil {
 			appErr.Where = "Api.searchUsers"
 			c.Err = appErr
 			return

server/public/model/client4.go

@@ -7690,6 +7690,19 @@ func (c *Client4) PatchGroup(ctx context.Context, groupID string, patch *GroupPa
 	return &g, BuildResponse(r), nil
 }
 
+func (c *Client4) GetGroupMembers(ctx context.Context, groupID string) (*GroupMemberList, *Response, error) {
+	r, err := c.DoAPIGet(ctx, c.groupRoute(groupID)+"/members", "")
+	if err != nil {
+		return nil, BuildResponse(r), err
+	}
+	defer closeBody(r)
+	var ml GroupMemberList
+	if err := json.NewDecoder(r.Body).Decode(&ml); err != nil {
+		return nil, nil, NewAppError("UpsertGroupMembers", "api.unmarshal_error", nil, "", http.StatusInternalServerError).Wrap(err)
+	}
+	return &ml, BuildResponse(r), nil
+}
+
 func (c *Client4) UpsertGroupMembers(ctx context.Context, groupID string, userIds *GroupModifyMembers) ([]*GroupMember, *Response, error) {
 	payload, err := json.Marshal(userIds)
 	if err != nil {

server/public/model/group_member.go

@@ -21,3 +21,8 @@ func (gm *GroupMember) IsValid() *AppError {
 	}
 	return nil
 }
+
+type GroupMemberList struct {
+	Members []*User `json:"members"`
+	Count   int     `json:"total_member_count"`
+}