diff --git a/app/channel.go b/app/channel.go index 0b5d96ff3e..c7bcf727ed 100644 --- a/app/channel.go +++ b/app/channel.go @@ -315,12 +315,26 @@ func (a *App) GetOrCreateDirectChannel(c *request.Context, userID, otherUserID s if *a.Config().TeamSettings.RestrictDirectMessage == model.DirectMessageTeam && !a.SessionHasPermissionTo(*c.Session(), model.PermissionManageSystem) { - commonTeamIDs, err := a.GetCommonTeamIDsForTwoUsers(userID, otherUserID) + users, err := a.GetUsersByIds([]string{userID, otherUserID}, &store.UserGetByIdsOpts{}) if err != nil { return nil, err } - if len(commonTeamIDs) == 0 { - return nil, model.NewAppError("createDirectChannel", "api.channel.create_channel.direct_channel.team_restricted_error", nil, "", http.StatusForbidden) + var isBot bool + for _, user := range users { + if user.IsBot { + isBot = true + break + } + } + // if one of the users is a bot, don't restrict to team members + if !isBot { + commonTeamIDs, err := a.GetCommonTeamIDsForTwoUsers(userID, otherUserID) + if err != nil { + return nil, err + } + if len(commonTeamIDs) == 0 { + return nil, model.NewAppError("createDirectChannel", "api.channel.create_channel.direct_channel.team_restricted_error", nil, "", http.StatusForbidden) + } } } @@ -398,10 +412,10 @@ func (a *App) createDirectChannel(userID string, otherUserID string, channelOpti return nil, model.NewAppError("CreateDirectChannel", "api.channel.create_direct_channel.invalid_user.app_error", nil, fmt.Sprintf("No users found for ids: %s. %s", userID, otherUserID), http.StatusBadRequest) } - // The potential swap dance bellow is necessary in order to guarantee determinism when creating a direct channel. + // The potential swap dance below is necessary in order to guarantee determinism when creating a direct channel. // When we query the database for some given user ids, the database result is not deterministic, meaning we can get // the same results but in different order. In order to conform the contract of Channel.CreateDirectChannel method - // bellow we need to identify which user is who. + // below we need to identify which user is who. user := users[0] otherUser := users[1] if user.Id != userID { diff --git a/app/channel_test.go b/app/channel_test.go index bb1e9d2b15..45fbbee47c 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -400,6 +400,50 @@ func TestUpdateChannelPrivacy(t *testing.T) { assert.Equal(t, publicChannel.Type, model.ChannelTypeOpen) } +func TestGetOrCreateDirectChannel(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + team1 := th.CreateTeam() + team2 := th.CreateTeam() + + user1 := th.CreateUser() + th.LinkUserToTeam(user1, team1) + + user2 := th.CreateUser() + th.LinkUserToTeam(user2, team2) + + bot1 := th.CreateBot() + + t.Run("Bot can create with restriction", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + setting := model.DirectMessageTeam + cfg.TeamSettings.RestrictDirectMessage = &setting + }) + + // check with bot in first userid param + channel, appErr := th.App.GetOrCreateDirectChannel(th.Context, bot1.UserId, user1.Id) + require.NotNil(t, channel, "channel should be non-nil") + require.Nil(t, appErr) + + // check with bot in second userid param + channel, appErr = th.App.GetOrCreateDirectChannel(th.Context, user1.Id, bot1.UserId) + require.NotNil(t, channel, "channel should be non-nil") + require.Nil(t, appErr) + }) + + t.Run("User from other team cannot create with restriction", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + setting := model.DirectMessageTeam + cfg.TeamSettings.RestrictDirectMessage = &setting + }) + + channel, appErr := th.App.GetOrCreateDirectChannel(th.Context, user1.Id, user2.Id) + require.Nil(t, channel, "channel should be nil") + require.NotNil(t, appErr) + }) +} + func TestCreateGroupChannelCreatesChannelMemberHistoryRecord(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown()