Forbid users from creating DMs and GMs with remote users (#28855)

* Forbid users from creating DMs and GMs with remote users

This change prevents the application from creating new DMs and GMs
when remote users are involved, and prevents as well new posts from
being created into preexisting DMs and GMs with remote users created
in previous versions.

* Adds i18n

* Skip preexisting DM tests and create a new one for the new behavior

server/channels/api4/shared_channel_test.go

@@ -161,7 +161,29 @@ func TestGetRemoteClusterById(t *testing.T) {
 }
 
 func TestCreateDirectChannelWithRemoteUser(t *testing.T) {
+	t.Run("should not create a local DM channel that is shared", func(t *testing.T) {
+		th := setupForSharedChannels(t).InitBasic()
+		defer th.TearDown()
+		client := th.Client
+		defer func() {
+			_, err := client.Logout(context.Background())
+			require.NoError(t, err)
+		}()
+
+		localUser := th.BasicUser
+		remoteUser := th.CreateUser()
+		remoteUser.RemoteId = model.NewPointer(model.NewId())
+		remoteUser, appErr := th.App.UpdateUser(th.Context, remoteUser, false)
+		require.Nil(t, appErr)
+
+		dm, _, err := client.CreateDirectChannel(context.Background(), localUser.Id, remoteUser.Id)
+		require.Error(t, err)
+		require.Nil(t, dm)
+	})
+
 	t.Run("creates a local DM channel that is shared", func(t *testing.T) {
+		t.Skip("Remote DMs are currently disabled")
+
 		th := setupForSharedChannels(t).InitBasic()
 		defer th.TearDown()
 		client := th.Client
@@ -185,6 +207,8 @@ func TestCreateDirectChannelWithRemoteUser(t *testing.T) {
 	})
 
 	t.Run("sends a shared channel invitation to the remote", func(t *testing.T) {
+		t.Skip("Remote DMs are currently disabled")
+
 		th := setupForSharedChannels(t).InitBasic()
 		defer th.TearDown()
 		client := th.Client
@@ -217,6 +241,8 @@ func TestCreateDirectChannelWithRemoteUser(t *testing.T) {
 	})
 
 	t.Run("does not send a shared channel invitation to the remote when creator is remote", func(t *testing.T) {
+		t.Skip("Remote DMs are currently disabled")
+
 		th := setupForSharedChannels(t).InitBasic()
 		defer th.TearDown()
 		client := th.Client

server/channels/app/channel.go

@@ -424,6 +424,10 @@ func (a *App) createDirectChannel(c request.CTX, userID string, otherUserID stri
 }
 
 func (a *App) createDirectChannelWithUser(c request.CTX, user, otherUser *model.User, channelOptions ...model.ChannelOption) (*model.Channel, *model.AppError) {
+	if user.IsRemote() || otherUser.IsRemote() {
+		return nil, model.NewAppError("createDirectChannelWithUser", "api.channel.create_channel.direct_channel.remote_restricted.app_error", nil, "", http.StatusForbidden)
+	}
+
 	channel, nErr := a.Srv().Store().Channel().CreateDirectChannel(c, user, otherUser, channelOptions...)
 	if nErr != nil {
 		var invErr *store.ErrInvalidInput
@@ -524,6 +528,12 @@ func (a *App) createGroupChannel(c request.CTX, userIDs []string) (*model.Channe
 		return nil, model.NewAppError("CreateGroupChannel", "api.channel.create_group.bad_user.app_error", nil, "user_ids="+model.ArrayToJSON(userIDs), http.StatusBadRequest)
 	}
 
+	for _, user := range users {
+		if user.IsRemote() {
+			return nil, model.NewAppError("createGroupChannel", "api.channel.create_group.remote_restricted.app_error", nil, "", http.StatusForbidden)
+		}
+	}
+
 	group := &model.Channel{
 		Name:        model.GetGroupNameFromUserIds(userIDs),
 		DisplayName: model.GetGroupDisplayNameFromUsers(users, true),

server/channels/app/channel_test.go

@@ -471,6 +471,39 @@ func TestGetOrCreateDirectChannel(t *testing.T) {
 		require.Nil(t, channel, "channel should be nil")
 		require.NotNil(t, appErr)
 	})
+
+	t.Run("Cannot create with a remote user", func(t *testing.T) {
+		user2.RemoteId = model.NewPointer(model.NewId())
+		_, appErr := th.App.UpdateUser(th.Context, user2, false)
+		require.Nil(t, appErr)
+
+		dm, appErr := th.App.GetOrCreateDirectChannel(th.Context, user1.Id, user2.Id)
+		require.Nil(t, dm)
+		require.NotNil(t, appErr)
+	})
+}
+
+func TestCreateGroupChannel(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	user1 := th.CreateUser()
+	user2 := th.CreateUser()
+
+	groupUserIds := make([]string, 0)
+	groupUserIds = append(groupUserIds, user1.Id)
+	groupUserIds = append(groupUserIds, user2.Id)
+	groupUserIds = append(groupUserIds, th.BasicUser.Id)
+
+	t.Run("Should not allow to create a group with a remote user", func(t *testing.T) {
+		user2.RemoteId = model.NewPointer(model.NewId())
+		_, appErr := th.App.UpdateUser(th.Context, user2, false)
+		require.Nil(t, appErr)
+
+		dm, appErr := th.App.CreateGroupChannel(th.Context, groupUserIds, th.BasicUser.Id)
+		require.NotNil(t, appErr)
+		require.Nil(t, dm)
+	})
 }
 
 func TestCreateGroupChannelCreatesChannelMemberHistoryRecord(t *testing.T) {

server/channels/app/post.go

@@ -141,6 +141,10 @@ func (a *App) deduplicateCreatePost(rctx request.CTX, post *model.Post) (foundPo
 }
 
 func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel, triggerWebhooks, setOnline bool) (savedPost *model.Post, err *model.AppError) {
+	if channel.IsShared() && (channel.Type == model.ChannelTypeDirect || channel.Type == model.ChannelTypeGroup) {
+		return nil, model.NewAppError("CreatePost", "app.post.create_post.shared_dm_or_gm.app_error", nil, "", http.StatusBadRequest)
+	}
+
 	foundPost, err := a.deduplicateCreatePost(c, post)
 	if err != nil {
 		return nil, err

server/channels/app/post_test.go

@@ -1050,6 +1050,85 @@ func TestCreatePost(t *testing.T) {
 		}
 	})
 
+	t.Run("Should not allow to create posts on shared DMs", func(t *testing.T) {
+		th := setupSharedChannels(t).InitBasic()
+		defer th.TearDown()
+
+		user1 := th.CreateUser()
+		user2 := th.CreateUser()
+		dm, err := th.App.createDirectChannel(th.Context, user1.Id, user2.Id)
+		require.Nil(t, err)
+		require.NotNil(t, dm)
+
+		// we can't create direct channels with remote users, so we
+		// have to force the channel to be shared through the store to
+		// simulate preexisting shared DMs
+		sc := &model.SharedChannel{
+			ChannelId: dm.Id,
+			Type:      dm.Type,
+			Home:      true,
+			ShareName: "shareddm",
+			CreatorId: user1.Id,
+			RemoteId:  model.NewId(),
+		}
+		_, scErr := th.Server.Store().SharedChannel().Save(sc)
+		require.NoError(t, scErr)
+
+		// and we update the channel to mark it as shared
+		dm.Shared = model.NewPointer(true)
+		_, cErr := th.Server.Store().Channel().Update(th.Context, dm)
+		require.NoError(t, cErr)
+
+		newPost := &model.Post{
+			ChannelId: dm.Id,
+			Message:   "hello world",
+			UserId:    user1.Id,
+		}
+		createdPost, err := th.App.CreatePost(th.Context, newPost, dm, false, false)
+		require.NotNil(t, err)
+		require.Nil(t, createdPost)
+	})
+
+	t.Run("Should not allow to create posts on shared GMs", func(t *testing.T) {
+		th := setupSharedChannels(t).InitBasic()
+		defer th.TearDown()
+
+		user1 := th.CreateUser()
+		user2 := th.CreateUser()
+		user3 := th.CreateUser()
+		gm, gErr := th.App.createGroupChannel(th.Context, []string{user1.Id, user2.Id, user3.Id})
+		require.Nil(t, gErr)
+		require.NotNil(t, gm)
+
+		// we can't create group channels with remote users, so we
+		// have to force the channel to be shared through the store to
+		// simulate preexisting shared GMs
+		sc := &model.SharedChannel{
+			ChannelId: gm.Id,
+			Type:      gm.Type,
+			Home:      true,
+			ShareName: "sharedgm",
+			CreatorId: user1.Id,
+			RemoteId:  model.NewId(),
+		}
+		_, scErr := th.Server.Store().SharedChannel().Save(sc)
+		require.NoError(t, scErr)
+
+		// and we update the channel to mark it as shared
+		gm.Shared = model.NewPointer(true)
+		_, cErr := th.Server.Store().Channel().Update(th.Context, gm)
+		require.NoError(t, cErr)
+
+		newPost := &model.Post{
+			ChannelId: gm.Id,
+			Message:   "hello world",
+			UserId:    user1.Id,
+		}
+		createdPost, err := th.App.CreatePost(th.Context, newPost, gm, false, false)
+		require.NotNil(t, err)
+		require.Nil(t, createdPost)
+	})
+
 	t.Run("MM-40016 should not panic with `concurrent map read and map write`", func(t *testing.T) {
 		th := Setup(t).InitBasic()
 		defer th.TearDown()

server/i18n/en.json

@@ -299,6 +299,10 @@
     "id": "api.channel.create_channel.direct_channel.app_error",
     "translation": "Must use createDirectChannel API service for direct message channel creation."
   },
+  {
+    "id": "api.channel.create_channel.direct_channel.remote_restricted.app_error",
+    "translation": "Cannot create a direct channel with a remote user"
+  },
   {
     "id": "api.channel.create_channel.direct_channel.team_restricted_error",
     "translation": "A direct channel cannot be created between these users because they do not share a team in common."
@@ -335,6 +339,10 @@
     "id": "api.channel.create_group.bad_user.app_error",
     "translation": "One of the provided users does not exist."
   },
+  {
+    "id": "api.channel.create_group.remote_restricted.app_error",
+    "translation": "Cannot create a group channel with remote users"
+  },
   {
     "id": "api.channel.delete_channel.archived",
     "translation": "%v archived the channel."
@@ -6154,6 +6162,10 @@
     "id": "app.post.cloud.get.app_error",
     "translation": "Can not fetch the post as it is past the cloud's plan limit."
   },
+  {
+    "id": "app.post.create_post.shared_dm_or_gm.app_error",
+    "translation": "Cannot create the post in a DM or GM with remote users"
+  },
   {
     "id": "app.post.delete.app_error",
     "translation": "Unable to delete the post."

server/platform/services/sharedchannel/service_api.go

@@ -22,6 +22,10 @@ func (scs *Service) ShareChannel(sc *model.SharedChannel) (*model.SharedChannel,
 		return nil, fmt.Errorf("cannot fetch channel while sharing channel %s: %w", sc.ChannelId, err)
 	}
 
+	if channel.Type == model.ChannelTypeDirect || channel.Type == model.ChannelTypeGroup {
+		return nil, errors.New("cannot share a direct or group channel")
+	}
+
 	// check if channel is already shared
 	scExisting, err := scs.server.GetStore().SharedChannel().Get(sc.ChannelId)
 	if err == nil {