Adds a team check to get groups by team (#25462)

* Adds a team check to get groups by team

* Update to take into account groups_by_channel and new permissions

* Update API docs

api/v4/source/groups.yaml

@@ -1006,6 +1006,9 @@
       description: |
         Retrieve the list of groups associated with a given team.
 
+        ##### Permissions
+        Must have the `list_team_channels` permission.
+
         __Minimum server version__: 5.11
       operationId: GetGroupsByTeam
       parameters:
@@ -1061,7 +1064,7 @@
           Retrieve the set of groups associated with the channels in the given team grouped by channel.
 
           ##### Permissions
-          Must have `manage_system` permission or can access only for current user
+          Must have the `list_team_channels` permission.
 
           __Minimum server version__: 5.11
         operationId: GetGroupsAssociatedToChannelsByTeam

server/channels/api4/group.go

@@ -831,6 +831,10 @@ func getGroupsByTeamCommon(c *Context, r *http.Request) ([]byte, *model.AppError
 		return nil, model.NewAppError("Api4.getGroupsByTeam", "api.ldap_groups.license_error", nil, "", http.StatusForbidden)
 	}
 
+	if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionListTeamChannels) {
+		return nil, c.App.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionListTeamChannels})
+	}
+
 	opts := model.GroupSearchOpts{
 		Q:                    c.Params.Q,
 		IncludeMemberCount:   c.Params.IncludeMemberCount,
@@ -859,6 +863,7 @@ func getGroupsByTeamCommon(c *Context, r *http.Request) ([]byte, *model.AppError
 
 	return b, nil
 }
+
 func getGroupsByChannelCommon(c *Context, r *http.Request) ([]byte, *model.AppError) {
 	if c.App.Channels().License() == nil || !*c.App.Channels().License().Features.LDAPGroups {
 		return nil, model.NewAppError("Api4.getGroupsByChannel", "api.ldap_groups.license_error", nil, "", http.StatusForbidden)
@@ -922,6 +927,11 @@ func getGroupsAssociatedToChannelsByTeam(c *Context, w http.ResponseWriter, r *h
 		return
 	}
 
+	if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionListTeamChannels) {
+		c.Err = c.App.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionListTeamChannels})
+		return
+	}
+
 	opts := model.GroupSearchOpts{
 		Q:                    c.Params.Q,
 		IncludeMemberCount:   c.Params.IncludeMemberCount,

server/channels/api4/group_test.go

@@ -1111,6 +1111,23 @@ func TestGetGroupsAssociatedToChannelsByTeam(t *testing.T) {
 	groups, _, err = th.SystemAdminClient.GetGroupsAssociatedToChannelsByTeam(context.Background(), model.NewId(), opts)
 	assert.NoError(t, err)
 	assert.Empty(t, groups)
+
+	t.Run("should get the groups ok when belonging to the team", func(t *testing.T) {
+		groups, resp, err := th.Client.GetGroupsAssociatedToChannelsByTeam(context.Background(), th.BasicTeam.Id, opts)
+		require.NoError(t, err)
+		CheckOKStatus(t, resp)
+		require.NotEmpty(t, groups)
+	})
+
+	t.Run("should return forbidden when the user doesn't have the right permissions", func(t *testing.T) {
+		require.Nil(t, th.App.RemoveUserFromTeam(th.Context, th.BasicTeam.Id, th.BasicUser.Id, th.SystemAdminUser.Id))
+		defer th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, th.BasicUser.Id, th.SystemAdminUser.Id)
+
+		groups, resp, err := th.Client.GetGroupsAssociatedToChannelsByTeam(context.Background(), th.BasicTeam.Id, opts)
+		require.Error(t, err)
+		CheckForbiddenStatus(t, resp)
+		require.Empty(t, groups)
+	})
 }
 
 func TestGetGroupsByTeam(t *testing.T) {
@@ -1189,6 +1206,36 @@ func TestGetGroupsByTeam(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Empty(t, groups)
 	})
+
+	t.Run("groups should be fetched only by users with the right permissions", func(t *testing.T) {
+		th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
+			groups, _, _, err := client.GetGroupsByTeam(context.Background(), th.BasicTeam.Id, opts)
+			require.NoError(t, err)
+			require.Len(t, groups, 1)
+			require.ElementsMatch(t, []*model.GroupWithSchemeAdmin{{Group: *group, SchemeAdmin: model.NewBool(true)}}, groups)
+			require.NotNil(t, groups[0].SchemeAdmin)
+			require.True(t, *groups[0].SchemeAdmin)
+		}, "groups can be fetched by system admins even if they're not part of a team")
+
+		t.Run("user can fetch groups if it's part of the team", func(t *testing.T) {
+			groups, _, _, err := th.Client.GetGroupsByTeam(context.Background(), th.BasicTeam.Id, opts)
+			require.NoError(t, err)
+			require.Len(t, groups, 1)
+			require.ElementsMatch(t, []*model.GroupWithSchemeAdmin{{Group: *group, SchemeAdmin: model.NewBool(true)}}, groups)
+			require.NotNil(t, groups[0].SchemeAdmin)
+			require.True(t, *groups[0].SchemeAdmin)
+		})
+
+		t.Run("user can't fetch groups if it's not part of the team", func(t *testing.T) {
+			require.Nil(t, th.App.RemoveUserFromTeam(th.Context, th.BasicTeam.Id, th.BasicUser.Id, th.SystemAdminUser.Id))
+			defer th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, th.BasicUser.Id, th.SystemAdminUser.Id)
+
+			groups, _, response, err := th.Client.GetGroupsByTeam(context.Background(), th.BasicTeam.Id, opts)
+			require.Error(t, err)
+			CheckForbiddenStatus(t, response)
+			require.Empty(t, groups)
+		})
+	})
 }
 
 func TestGetGroups(t *testing.T) {