MM-66424: Improve team filtering in common teams API (#34454) (#34854)

Cherry-pick 6404ab29acc04901c5cb1cf5ad97fc3c0693e2cd into release-10.11

server/channels/api4/channel.go

@@ -2461,7 +2461,7 @@ func getGroupMessageMembersCommonTeams(c *Context, w http.ResponseWriter, r *htt
 		return
 	}
 
-	teams, appErr := c.App.GetGroupMessageMembersCommonTeams(c.AppContext, c.Params.ChannelId)
+	teams, appErr := c.App.GetGroupMessageMembersCommonTeamsAsUser(c.AppContext, c.Params.ChannelId)
 	if appErr != nil {
 		c.Err = appErr
 		return

server/channels/api4/channel_common_teams_test.go

@@ -0,0 +1,169 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package api4
+
+import (
+	"context"
+	"testing"
+
+	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetGroupMessageMembersCommonTeams(t *testing.T) {
+	mainHelper.Parallel(t)
+
+	th := Setup(t).InitBasic()
+	client := th.Client
+
+	t.Run("requires authentication", func(t *testing.T) {
+		user1 := th.BasicUser
+		user2 := th.BasicUser2
+		user3 := th.CreateUser()
+
+		testClient := th.CreateClient()
+		_, _, err := testClient.Login(context.Background(), user1.Email, user1.Password)
+		require.NoError(t, err)
+
+		gmChannel, _, err := testClient.CreateGroupChannel(context.Background(), []string{user1.Id, user2.Id, user3.Id})
+		require.NoError(t, err)
+
+		_, err = testClient.Logout(context.Background())
+		require.NoError(t, err)
+
+		_, resp, err := testClient.GetGroupMessageMembersCommonTeams(context.Background(), gmChannel.Id)
+		require.Error(t, err)
+		CheckUnauthorizedStatus(t, resp)
+	})
+
+	t.Run("forbids guest users", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.GuestAccountsSettings.Enable = true
+		})
+		th.App.Srv().SetLicense(model.NewTestLicense())
+
+		guestUser, guestClient := th.CreateGuestAndClient(t)
+		team1 := th.BasicTeam
+		th.LinkUserToTeam(guestUser, team1)
+
+		user2 := th.BasicUser2
+		th.LinkUserToTeam(user2, team1)
+
+		user3 := th.CreateUser()
+		th.LinkUserToTeam(user3, team1)
+
+		gmChannel, _, err := th.SystemAdminClient.CreateGroupChannel(context.Background(), []string{guestUser.Id, user2.Id, user3.Id})
+		require.NoError(t, err)
+
+		_, resp, err := guestClient.GetGroupMessageMembersCommonTeams(context.Background(), gmChannel.Id)
+		require.Error(t, err)
+		CheckForbiddenStatus(t, resp)
+	})
+
+	t.Run("requires read permission on channel", func(t *testing.T) {
+		user1 := th.CreateUser()
+		user2 := th.CreateUser()
+		user3 := th.CreateUser()
+		team1 := th.CreateTeam()
+
+		th.LinkUserToTeam(user1, team1)
+		th.LinkUserToTeam(user2, team1)
+		th.LinkUserToTeam(user3, team1)
+
+		gmChannel, _, err := th.SystemAdminClient.CreateGroupChannel(context.Background(), []string{user1.Id, user2.Id, user3.Id})
+		require.NoError(t, err)
+
+		otherUser := th.CreateUser()
+		th.LinkUserToTeam(otherUser, team1)
+
+		otherClient := th.CreateClient()
+		_, _, err = otherClient.Login(context.Background(), otherUser.Email, otherUser.Password)
+		require.NoError(t, err)
+
+		_, resp, err := otherClient.GetGroupMessageMembersCommonTeams(context.Background(), gmChannel.Id)
+		require.Error(t, err)
+		CheckForbiddenStatus(t, resp)
+	})
+
+	t.Run("returns bad request for non-GM channel", func(t *testing.T) {
+		testClient := th.CreateClient()
+		_, _, err := testClient.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
+		require.NoError(t, err)
+
+		_, resp, err := testClient.GetGroupMessageMembersCommonTeams(context.Background(), th.BasicChannel.Id)
+		require.Error(t, err)
+		CheckBadRequestStatus(t, resp)
+	})
+
+	t.Run("returns common teams for GM channel members", func(t *testing.T) {
+		user1 := th.BasicUser
+		user2 := th.BasicUser2
+		user3 := th.CreateUser()
+		team1 := th.BasicTeam
+		team2 := th.CreateTeam()
+		team3 := th.CreateTeam()
+
+		th.LinkUserToTeam(user1, team1)
+		th.LinkUserToTeam(user1, team2)
+		th.LinkUserToTeam(user2, team1)
+		th.LinkUserToTeam(user2, team3)
+		th.LinkUserToTeam(user3, team1)
+
+		gmChannel, _, err := client.CreateGroupChannel(context.Background(), []string{user1.Id, user2.Id, user3.Id})
+		require.NoError(t, err)
+
+		teams, _, err := client.GetGroupMessageMembersCommonTeams(context.Background(), gmChannel.Id)
+		require.NoError(t, err)
+		require.Len(t, teams, 1, "should only return team1 since it's the only team all three users share")
+		assert.Equal(t, team1.Id, teams[0].Id)
+	})
+
+	t.Run("returns empty list when requesting user in channel but has no common teams with other members", func(t *testing.T) {
+		user1 := th.CreateUser()
+		user2 := th.CreateUser()
+		user3 := th.CreateUser()
+		team1 := th.CreateTeam()
+		team2 := th.CreateTeam()
+
+		th.LinkUserToTeam(user1, team1)
+		th.LinkUserToTeam(user2, team2)
+		th.LinkUserToTeam(user3, team1)
+
+		testClient := th.CreateClient()
+		_, _, err := testClient.Login(context.Background(), user1.Email, user1.Password)
+		require.NoError(t, err)
+
+		gmChannel, _, err := testClient.CreateGroupChannel(context.Background(), []string{user1.Id, user2.Id, user3.Id})
+		require.NoError(t, err)
+
+		teams, _, err := testClient.GetGroupMessageMembersCommonTeams(context.Background(), gmChannel.Id)
+		require.NoError(t, err)
+		require.Empty(t, teams)
+	})
+
+	t.Run("filters teams to only those common with requesting user", func(t *testing.T) {
+		user1 := th.CreateUser()
+		user2 := th.CreateUser()
+		user3 := th.BasicUser
+		team1 := th.CreateTeam()
+		team2 := th.CreateTeam()
+		team3 := th.CreateTeam()
+
+		th.LinkUserToTeam(user1, team1)
+		th.LinkUserToTeam(user1, team2)
+		th.LinkUserToTeam(user2, team1)
+		th.LinkUserToTeam(user2, team3)
+		th.LinkUserToTeam(user3, team1)
+		th.LinkUserToTeam(user3, team3)
+
+		gmChannel, _, err := client.CreateGroupChannel(context.Background(), []string{user1.Id, user2.Id, user3.Id})
+		require.NoError(t, err)
+
+		teams, _, err := client.GetGroupMessageMembersCommonTeams(context.Background(), gmChannel.Id)
+		require.NoError(t, err)
+		require.Len(t, teams, 1)
+		assert.Equal(t, team1.Id, teams[0].Id)
+	})
+}

server/channels/app/channel.go

@@ -3725,8 +3725,26 @@ func (a *App) getDirectChannel(c request.CTX, userID, otherUserID string) (*mode
 	return a.Srv().getDirectChannel(c, userID, otherUserID)
 }
 
-func (a *App) GetGroupMessageMembersCommonTeams(c request.CTX, channelID string) ([]*model.Team, *model.AppError) {
+// GetGroupMessageMembersCommonTeamsAsUser is a variant of GetGroupMessageMembersCommonTeams
-	channel, appErr := a.GetChannel(c, channelID)
+// that returns results relative to the requesting user from the session in the request context.
+func (a *App) GetGroupMessageMembersCommonTeamsAsUser(rctx request.CTX, channelID string) ([]*model.Team, *model.AppError) {
+	return a.getGroupMessageMembersCommonTeams(rctx, rctx.Session().UserId, channelID)
+}
+
+// GetGroupMessageMembersCommonTeams returns the set of teams in common for the members of the given GM channel.
+//
+// Prefer GetGroupMessageMembersCommonTeamsAsUser unless the request context is independent of any given user.
+func (a *App) GetGroupMessageMembersCommonTeams(rctx request.CTX, channelID string) ([]*model.Team, *model.AppError) {
+	return a.getGroupMessageMembersCommonTeams(rctx, "", channelID)
+}
+
+// getGroupMessageMembersCommonTeams returns the set of teams common to the members of the given GM channel.
+//
+// If a requesting user id is specified, but the user isn't an active member of the channel, we return an empty
+// set of channels. We don't just exclude all inactive users to offer more flexibility to the remaining users
+// on where to create the replacement channel.
+func (a *App) getGroupMessageMembersCommonTeams(rctx request.CTX, requestingUserID, channelID string) ([]*model.Team, *model.AppError) {
+	channel, appErr := a.GetChannel(rctx, channelID)
 	if appErr != nil {
 		return nil, appErr
 	}
@@ -3742,12 +3760,22 @@ func (a *App) GetGroupMessageMembersCommonTeams(c request.CTX, channelID string)
 		Inactive:    false,
 		Active:      true,
 	})
+	if appErr != nil {
+		return nil, appErr
+	}
 
 	userIDs := make([]string, len(users))
 	for i := 0; i < len(users); i++ {
 		userIDs[i] = users[i].Id
 	}
 
+	// If a requesting user is specified, but we don't find them above as an active member
+	// of the channel, just short-circuit and return an empty set. We don't return an error
+	// as this is a valid result for some callers.
+	if requestingUserID != "" && !slices.Contains(userIDs, requestingUserID) {
+		return nil, nil
+	}
+
 	commonTeamIDs, err := a.Srv().Store().Team().GetCommonTeamIDsForMultipleUsers(userIDs)
 	if err != nil {
 		return nil, model.NewAppError("GetGroupMessageMembersCommonTeams", "app.channel.get_common_teams.store_get_common_teams_error", nil, "", http.StatusInternalServerError).Wrap(err)

server/channels/app/channel_test.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"slices"
 	"sort"
 	"strings"
 	"sync"
@@ -2864,59 +2865,156 @@ func TestIsCRTEnabledForUser(t *testing.T) {
 func TestGetGroupMessageMembersCommonTeams(t *testing.T) {
 	mainHelper.Parallel(t)
 	th := Setup(t).InitBasic()
-	defer th.TearDown()
 
-	teamsToCreate := 2
+	team1 := th.CreateTeam()
-	usersToCreate := 4 // at least 3 users to create a GM channel, last user is not in any team
+	team2 := th.CreateTeam()
-	teams := make([]string, 0, teamsToCreate)
+
-	for i := 0; i < cap(teams); i++ {
+	user1 := th.CreateUser()
-		team := th.CreateTeam()
+	user2 := th.CreateUser()
-		defer func(team *model.Team) {
+	user3 := th.CreateUser()
-			appErr := th.App.PermanentDeleteTeam(th.Context, team)
+	user4NotInAnyTeams := th.CreateUser()
-			require.Nil(t, appErr)
+	unrelatedUser := th.CreateUser()
-		}(team)
+
-		teams = append(teams, team.Id)
+	// All of user1, user2 and user3, and unrelatedUser on team1
+	th.LinkUserToTeam(user1, team1)
+	th.LinkUserToTeam(user2, team1)
+	th.LinkUserToTeam(user3, team1)
+	th.LinkUserToTeam(unrelatedUser, team1)
+
+	// Only user2, user3, and unrelatedUser on team2
+	th.LinkUserToTeam(user2, team2)
+	th.LinkUserToTeam(user3, team2)
+	th.LinkUserToTeam(unrelatedUser, team2)
+
+	assertNoTeamsInCommon := func(t *testing.T, commonTeams []*model.Team) {
+		t.Helper()
+		assert.Empty(t, commonTeams, "expected no teams in common")
 	}
 
-	users := make([]string, 0, usersToCreate)
+	assertTeam1InCommon := func(t *testing.T, commonTeams []*model.Team) {
-	for i := 0; i < cap(users); i++ {
+		t.Helper()
-		user := th.CreateUser()
+		if assert.Len(t, commonTeams, 1, "expected 1 team in common") {
-		defer func(user *model.User) {
+			assert.Equal(t, team1.Id, commonTeams[0].Id, "expected team1 in common")
-			appErr := th.App.PermanentDeleteUser(th.Context, user)
-			require.Nil(t, appErr)
-		}(user)
-		users = append(users, user.Id)
-	}
-
-	for _, teamId := range teams {
-		// add first 3 users to each team, last user is not in any team
-		for i := range 3 {
-			_, _, appErr := th.App.AddUserToTeam(th.Context, teamId, users[i], "")
-			require.Nil(t, appErr)
 		}
 	}
 
-	// create GM channel with first 3 users who share common teams
+	assertTeam1And2InCommon := func(t *testing.T, commonTeams []*model.Team) {
-	gmChannel, appErr := th.App.createGroupChannel(th.Context, users[:3], users[0])
+		t.Helper()
-	require.Nil(t, appErr)
+		if assert.Len(t, commonTeams, 2, "expected 2 teams in common") {
-	require.NotNil(t, gmChannel)
+			assert.True(t, slices.ContainsFunc(commonTeams, func(team *model.Team) bool {
+				return team.Id == team1.Id
+			}), "expected team1 in common")
+			assert.True(t, slices.ContainsFunc(commonTeams, func(team *model.Team) bool {
+				return team.Id == team2.Id
+			}), "expected team2 in common")
+		}
+	}
 
-	// normally you can't create a GM channel with users that don't share any teams, but we do it here to test the edge case
+	t.Run("teams for gm with user1, user2 and user3", func(t *testing.T) {
-	// create GM channel with last 3 users, where last member is not in any team
+		gmChannel, appErr := th.App.createGroupChannel(th.Context, []string{user1.Id, user2.Id, user3.Id}, user1.Id)
-	otherGMChannel, appErr := th.App.createGroupChannel(th.Context, users[1:], users[0])
+		require.Nil(t, appErr)
-	require.Nil(t, appErr)
+		require.NotNil(t, gmChannel)
-	require.NotNil(t, otherGMChannel)
 
-	t.Run("Get teams for GM channel", func(t *testing.T) {
 		commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeams(th.Context, gmChannel.Id)
 		require.Nil(t, appErr)
-		require.Equal(t, 2, len(commonTeams))
+		assertTeam1InCommon(t, commonTeams)
+
+		t.Run("as user1", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: user1.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+			assertTeam1InCommon(t, commonTeams)
+		})
+
+		t.Run("as user2", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: user2.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+			assertTeam1InCommon(t, commonTeams)
+		})
+
+		t.Run("as user3", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: user3.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+			assertTeam1InCommon(t, commonTeams)
+		})
+
+		t.Run("as unrelatedUser", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: unrelatedUser.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+			assertNoTeamsInCommon(t, commonTeams)
+		})
 	})
 
-	t.Run("No common teams", func(t *testing.T) {
+	t.Run("teams for gm with user2, user3, and user4NotInAnyTeams", func(t *testing.T) {
-		commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeams(th.Context, otherGMChannel.Id)
+		gmChannel, appErr := th.App.createGroupChannel(th.Context, []string{user2.Id, user3.Id, user4NotInAnyTeams.Id}, user1.Id)
 		require.Nil(t, appErr)
-		require.Equal(t, 0, len(commonTeams))
+		require.NotNil(t, gmChannel)
+
+		commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeams(th.Context, gmChannel.Id)
+		require.Nil(t, appErr)
+		assertNoTeamsInCommon(t, commonTeams)
+
+		t.Run("as user2", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: user2.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+			assertNoTeamsInCommon(t, commonTeams)
+		})
+
+		t.Run("as user3", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: user3.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+			assertNoTeamsInCommon(t, commonTeams)
+		})
+
+		t.Run("as unrelatedUser", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: unrelatedUser.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+			assertNoTeamsInCommon(t, commonTeams)
+		})
+	})
+
+	t.Run("teams for gm with user2, user3, and deactivatedUser", func(t *testing.T) {
+		deactivatedUser := th.CreateUser()
+
+		// deactiverUser on team2 only
+		th.LinkUserToTeam(deactivatedUser, team2)
+
+		gmChannel, appErr := th.App.createGroupChannel(th.Context, []string{user2.Id, user3.Id, deactivatedUser.Id}, user1.Id)
+		require.Nil(t, appErr)
+		require.NotNil(t, gmChannel)
+
+		_, appErr = th.App.UpdateActive(th.Context, deactivatedUser, false)
+		require.Nil(t, appErr)
+
+		commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeams(th.Context, gmChannel.Id)
+		require.Nil(t, appErr)
+		// By default, we return the teams common only to active users.
+		assertTeam1And2InCommon(t, commonTeams)
+
+		t.Run("as user2", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: user2.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+			assertTeam1And2InCommon(t, commonTeams)
+		})
+
+		t.Run("as user3", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: user3.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+			assertTeam1And2InCommon(t, commonTeams)
+		})
+
+		t.Run("as deactivatedUser", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: deactivatedUser.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+
+			// When requesting as deactivated user in the gm, no teams are considered in common.
+			assertNoTeamsInCommon(t, commonTeams)
+		})
+
+		t.Run("as unrelatedUser", func(t *testing.T) {
+			commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeamsAsUser(th.Context.WithSession(&model.Session{UserId: unrelatedUser.Id}), gmChannel.Id)
+			require.Nil(t, appErr)
+			assertNoTeamsInCommon(t, commonTeams)
+		})
 	})
 }
 

server/public/model/client4.go

@@ -3637,6 +3637,23 @@ func (c *Client4) MoveChannel(ctx context.Context, channelId, teamId string, for
 	return ch, BuildResponse(r), nil
 }
 
+// GetGroupMessageMembersCommonTeams returns the set of teams in common for members of a GM channel.
+func (c *Client4) GetGroupMessageMembersCommonTeams(ctx context.Context, channelId string) ([]*Team, *Response, error) {
+	r, err := c.DoAPIGet(ctx, c.channelRoute(channelId)+"/common_teams", "")
+	if err != nil {
+		return nil, BuildResponse(r), err
+	}
+	defer closeBody(r)
+
+	var teams []*Team
+	err = json.NewDecoder(r.Body).Decode(&teams)
+	if err != nil {
+		return nil, BuildResponse(r), NewAppError("GetGroupMessageMembersCommonTeams", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err)
+	}
+
+	return teams, BuildResponse(r), nil
+}
+
 // GetChannelByName returns a channel based on the provided channel name and team id strings.
 func (c *Client4) GetChannelByName(ctx context.Context, channelName, teamId string, etag string) (*Channel, *Response, error) {
 	r, err := c.DoAPIGet(ctx, c.channelByNameRoute(channelName, teamId), etag)

server/public/model/example_channel_test.go

@@ -491,3 +491,19 @@ func ExampleClient4_UpdateChannelScheme() {
 		log.Fatal(err)
 	}
 }
+
+func ExampleClient4_GetGroupMessageMembersCommonTeams() {
+	client := model.NewAPIv4Client(os.Getenv("MM_SERVICESETTINGS_SITEURL"))
+	client.SetToken(os.Getenv("MM_AUTHTOKEN"))
+
+	channelID := "gm_channel_id"
+	teams, _, err := client.GetGroupMessageMembersCommonTeams(context.Background(), channelID)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	fmt.Printf("Found %d common teams for members of channel %s\n", len(teams), channelID)
+	for _, team := range teams {
+		fmt.Printf("  - %s (%s)\n", team.DisplayName, team.Name)
+	}
+}