diff --git a/server/channels/api4/shared_channel_test.go b/server/channels/api4/shared_channel_test.go index cecb507c36..729af1f42b 100644 --- a/server/channels/api4/shared_channel_test.go +++ b/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 diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index 6e433bb35c..2571b3adc7 100644 --- a/server/channels/app/channel.go +++ b/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), diff --git a/server/channels/app/channel_test.go b/server/channels/app/channel_test.go index 0294c26405..317a4aabfb 100644 --- a/server/channels/app/channel_test.go +++ b/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) { diff --git a/server/channels/app/post.go b/server/channels/app/post.go index 4a254ad988..6ab4dec56e 100644 --- a/server/channels/app/post.go +++ b/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 diff --git a/server/channels/app/post_test.go b/server/channels/app/post_test.go index 5fdd6cd8f0..7e09b90ccf 100644 --- a/server/channels/app/post_test.go +++ b/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() diff --git a/server/i18n/en.json b/server/i18n/en.json index fa24a131bb..406c708ad0 100644 --- a/server/i18n/en.json +++ b/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." diff --git a/server/platform/services/sharedchannel/service_api.go b/server/platform/services/sharedchannel/service_api.go index b1d1804e06..3fbc0a8148 100644 --- a/server/platform/services/sharedchannel/service_api.go +++ b/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 {