[MM-53057] Disallow malformed channel names for DMs (#24404)

This commit is contained in:
Ben Schumacher 2023-10-24 15:08:14 +02:00 committed by GitHub
parent 24f5672da8
commit 5d3ba7483b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 49 additions and 39 deletions

View File

@ -196,7 +196,7 @@ func TestSearchFileInfoStore(t *testing.T, s store.Store, testEngine *SearchTest
}
func testFileInfoSearchFileInfosIncludingDMs(t *testing.T, th *SearchTestHelper) {
direct, err := th.createDirectChannel(th.Team.Id, "direct-"+th.Team.Id, "direct-"+th.Team.Id, []*model.User{th.User, th.User2})
direct, err := th.createDirectChannel(th.Team.Id, "direct-"+th.Team.Id, []*model.User{th.User, th.User2})
require.NoError(t, err)
defer th.deleteChannel(direct)
@ -237,7 +237,7 @@ func testFileInfoSearchFileInfosIncludingDMs(t *testing.T, th *SearchTestHelper)
}
func testFileInfoSearchFileInfosWithPagination(t *testing.T, th *SearchTestHelper) {
direct, err := th.createDirectChannel(th.Team.Id, "direct", "direct", []*model.User{th.User, th.User2})
direct, err := th.createDirectChannel(th.Team.Id, "direct", []*model.User{th.User, th.User2})
require.NoError(t, err)
defer th.deleteChannel(direct)
@ -651,7 +651,7 @@ func testFileInfoSearchOrExcludeFileInfosInChannel(t *testing.T, th *SearchTestH
}
func testFileInfoSearchOrExcludeFileInfosInDMGM(t *testing.T, th *SearchTestHelper) {
direct, err := th.createDirectChannel(th.Team.Id, "direct", "direct", []*model.User{th.User, th.User2})
direct, err := th.createDirectChannel(th.Team.Id, "direct", []*model.User{th.User, th.User2})
require.NoError(t, err)
defer th.deleteChannel(direct)

View File

@ -275,10 +275,10 @@ func (th *SearchTestHelper) createChannel(teamID, name, displayName, purpose str
return channel, nil
}
func (th *SearchTestHelper) createDirectChannel(teamID, name, displayName string, users []*model.User) (*model.Channel, error) {
func (th *SearchTestHelper) createDirectChannel(teamID, displayName string, users []*model.User) (*model.Channel, error) {
channel := &model.Channel{
TeamId: teamID,
Name: name,
Name: model.GetDMNameFromIds(users[0].Id, users[1].Id),
DisplayName: displayName,
Type: model.ChannelTypeDirect,
}
@ -290,7 +290,7 @@ func (th *SearchTestHelper) createDirectChannel(teamID, name, displayName string
m2 := &model.ChannelMember{}
m2.ChannelId = channel.Id
m2.UserId = users[0].Id
m2.UserId = users[1].Id
m2.NotifyProps = model.GetDefaultChannelNotifyProps()
channel, err := th.Store.Channel().SaveDirectChannel(channel, m1, m2)

View File

@ -288,7 +288,7 @@ func TestSearchPostStore(t *testing.T, s store.Store, testEngine *SearchTestEngi
}
func testSearchPostsIncludingDMs(t *testing.T, th *SearchTestHelper) {
direct, err := th.createDirectChannel(th.Team.Id, "direct", "direct", []*model.User{th.User, th.User2})
direct, err := th.createDirectChannel(th.Team.Id, "direct", []*model.User{th.User, th.User2})
require.NoError(t, err)
defer th.deleteChannel(direct)
@ -310,7 +310,7 @@ func testSearchPostsIncludingDMs(t *testing.T, th *SearchTestHelper) {
}
func testSearchPostsWithPagination(t *testing.T, th *SearchTestHelper) {
direct, err := th.createDirectChannel(th.Team.Id, "direct", "direct", []*model.User{th.User, th.User2})
direct, err := th.createDirectChannel(th.Team.Id, "direct", []*model.User{th.User, th.User2})
require.NoError(t, err)
defer th.deleteChannel(direct)
@ -675,7 +675,7 @@ func testSearchOrExcludePostsInChannel(t *testing.T, th *SearchTestHelper) {
}
func testSearchOrExcludePostsInDMGM(t *testing.T, th *SearchTestHelper) {
direct, err := th.createDirectChannel(th.Team.Id, "direct", "direct", []*model.User{th.User, th.User2})
direct, err := th.createDirectChannel(th.Team.Id, "direct", []*model.User{th.User, th.User2})
require.NoError(t, err)
defer th.deleteChannel(direct)

View File

@ -172,7 +172,7 @@ func testChannelStoreSave(t *testing.T, ss store.Store) {
require.Error(t, nErr, "should be unique name")
o1.Id = ""
o1.Name = NewTestId()
o1.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o1.Type = model.ChannelTypeDirect
_, nErr = ss.Channel().Save(&o1, -1)
require.Error(t, nErr, "should not be able to save direct channel")
@ -209,7 +209,7 @@ func testChannelStoreSaveDirectChannel(t *testing.T, ss store.Store, s SqlStore)
o1 := model.Channel{}
o1.TeamId = teamId
o1.DisplayName = "Name"
o1.Name = NewTestId()
o1.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o1.Type = model.ChannelTypeDirect
u1 := &model.User{}
@ -272,7 +272,7 @@ func testChannelStoreSaveDirectChannel(t *testing.T, ss store.Store, s SqlStore)
// Save yourself Direct Message
o1.Id = ""
o1.DisplayName = "Myself"
o1.Name = NewTestId()
o1.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o1.Type = model.ChannelTypeDirect
_, nErr = ss.Channel().SaveDirectChannel(&o1, &m1, &m1)
require.NoError(t, nErr, "couldn't save direct channel", nErr)
@ -446,7 +446,7 @@ func testChannelStoreGet(t *testing.T, ss store.Store, s SqlStore) {
o2 := model.Channel{}
o2.TeamId = model.NewId()
o2.DisplayName = "Direct Name"
o2.Name = NewTestId()
o2.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o2.Type = model.ChannelTypeDirect
m1 := model.ChannelMember{}
@ -548,7 +548,7 @@ func testChannelStoreGetChannelsByIds(t *testing.T, ss store.Store) {
o2 := model.Channel{}
o2.TeamId = model.NewId()
o2.DisplayName = "Direct Name"
o2.Name = "bb" + model.NewId()
o2.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o2.Type = model.ChannelTypeDirect
o3 := model.Channel{}
@ -580,8 +580,7 @@ func testChannelStoreGetChannelsByIds(t *testing.T, ss store.Store) {
r1, err := ss.Channel().GetChannelsByIds([]string{o1.Id, o2.Id}, false)
require.NoError(t, err, err)
require.Len(t, r1, 2, "invalid returned channels, expected 2 and got "+strconv.Itoa(len(r1)))
require.Equal(t, channelToJSON(t, &o1), channelToJSON(t, r1[0]))
require.Equal(t, channelToJSON(t, &o2), channelToJSON(t, r1[1]))
require.ElementsMatch(t, []*model.Channel{&o1, &o2}, r1)
})
t.Run("Get 1 existing and 1 not existing channel", func(t *testing.T) {
@ -589,16 +588,14 @@ func testChannelStoreGetChannelsByIds(t *testing.T, ss store.Store) {
r2, err := ss.Channel().GetChannelsByIds([]string{o1.Id, nonexistentId}, false)
require.NoError(t, err, err)
require.Len(t, r2, 1, "invalid returned channels, expected 1 and got "+strconv.Itoa(len(r2)))
require.Equal(t, channelToJSON(t, &o1), channelToJSON(t, r2[0]), "invalid returned channel")
require.ElementsMatch(t, []*model.Channel{&o1}, r2, "invalid returned channel")
})
t.Run("Get 2 existing and 1 deleted channel", func(t *testing.T) {
r1, err := ss.Channel().GetChannelsByIds([]string{o1.Id, o2.Id, o3.Id}, true)
require.NoError(t, err, err)
require.Len(t, r1, 3, "invalid returned channels, expected 3 and got "+strconv.Itoa(len(r1)))
require.Equal(t, channelToJSON(t, &o1), channelToJSON(t, r1[0]))
require.Equal(t, channelToJSON(t, &o2), channelToJSON(t, r1[1]))
require.Equal(t, channelToJSON(t, &o3), channelToJSON(t, r1[2]))
require.ElementsMatch(t, []*model.Channel{&o1, &o2, &o3}, r1)
})
}
@ -640,7 +637,7 @@ func testGetChannelsWithTeamDataByIds(t *testing.T, ss store.Store) {
c2 := model.Channel{}
c2.TeamId = t1.Id
c2.DisplayName = "Direct Name"
c2.Name = "bb" + model.NewId()
c2.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
c2.Type = model.ChannelTypeDirect
c3 := model.Channel{}
@ -671,10 +668,20 @@ func testGetChannelsWithTeamDataByIds(t *testing.T, ss store.Store) {
res, err := ss.Channel().GetChannelsWithTeamDataByIds([]string{c1.Id, c2.Id}, false)
require.NoError(t, err)
require.Len(t, res, 2)
assert.Equal(t, res[0].Id, c1.Id)
assert.Equal(t, res[0].TeamName, t1.Name)
assert.Equal(t, res[1].Id, c2.Id)
assert.Equal(t, res[1].TeamName, "")
if res[0].Id == c1.Id {
assert.Equal(t, res[0].TeamName, t1.Name)
assert.Equal(t, res[1].Id, c2.Id)
assert.Equal(t, res[1].TeamName, "")
} else if res[0].Id == c2.Id {
assert.Equal(t, res[0].TeamName, "")
assert.Equal(t, res[1].Id, c1.Id)
assert.Equal(t, res[1].TeamName, t1.Name)
} else {
assert.Fail(t, "unknown channel id")
}
}
func testChannelStoreGetForPost(t *testing.T, ss store.Store) {
@ -7469,7 +7476,7 @@ func testChannelStoreExportAllDirectChannels(t *testing.T, ss store.Store, s Sql
o1 := model.Channel{}
o1.TeamId = teamId
o1.DisplayName = "Name" + model.NewId()
o1.Name = NewTestId()
o1.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o1.Type = model.ChannelTypeDirect
userIds := []string{model.NewId(), model.NewId(), model.NewId()}
@ -7526,7 +7533,7 @@ func testChannelStoreExportAllDirectChannelsExcludePrivateAndPublic(t *testing.T
o1 := model.Channel{}
o1.TeamId = teamId
o1.DisplayName = "The Direct Channel" + model.NewId()
o1.Name = NewTestId()
o1.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o1.Type = model.ChannelTypeDirect
o2 := model.Channel{}
@ -7588,7 +7595,7 @@ func testChannelStoreExportAllDirectChannelsDeletedChannel(t *testing.T, ss stor
o1 := model.Channel{}
o1.TeamId = teamId
o1.DisplayName = "Different Name" + model.NewId()
o1.Name = NewTestId()
o1.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o1.Type = model.ChannelTypeDirect
u1 := &model.User{}

View File

@ -3055,7 +3055,7 @@ func testPostStoreGetFlaggedPostsForTeam(t *testing.T, ss store.Store, s SqlStor
c2 := &model.Channel{}
c2.DisplayName = "DMChannel1"
c2.Name = NewTestId()
c2.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
c2.Type = model.ChannelTypeDirect
m1 := &model.ChannelMember{}
@ -4410,7 +4410,7 @@ func testPostStoreGetDirectPostParentsForExportAfter(t *testing.T, ss store.Stor
o1 := model.Channel{}
o1.TeamId = teamId
o1.DisplayName = "Name"
o1.Name = NewTestId()
o1.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o1.Type = model.ChannelTypeDirect
u1 := &model.User{}
@ -4464,7 +4464,7 @@ func testPostStoreGetDirectPostParentsForExportAfterDeleted(t *testing.T, ss sto
o1 := model.Channel{}
o1.TeamId = teamId
o1.DisplayName = "Name"
o1.Name = NewTestId()
o1.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o1.Type = model.ChannelTypeDirect
u1 := &model.User{}
@ -4530,7 +4530,7 @@ func testPostStoreGetDirectPostParentsForExportAfterBatched(t *testing.T, ss sto
o1 := model.Channel{}
o1.TeamId = teamId
o1.DisplayName = "Name"
o1.Name = NewTestId()
o1.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
o1.Type = model.ChannelTypeDirect
var postIds []string

View File

@ -2398,7 +2398,7 @@ func testUserUnreadCount(t *testing.T, ss store.Store) {
c2 := model.Channel{}
c2.TeamId = teamId
c2.DisplayName = "Unread Direct"
c2.Name = "unread-direct-" + model.NewId()
c2.Name = model.GetDMNameFromIds(NewTestId(), NewTestId())
c2.Type = model.ChannelTypeDirect
u1 := &model.User{}

View File

@ -344,7 +344,7 @@ func TestOldImportChannel(t *testing.T) {
// ch := th.CreateDmChannel(u1)
ch := &model.Channel{
Type: model.ChannelTypeDirect,
Name: "test-channel",
Name: model.GetDMNameFromIds(u1.Id, u2.Id),
}
users := map[string]*model.User{
u2.Id: u2,
@ -364,7 +364,7 @@ func TestOldImportChannel(t *testing.T) {
t.Run("No panic on direct channel with 1 member", func(t *testing.T) {
ch := &model.Channel{
Type: model.ChannelTypeDirect,
Name: "test-channel",
Name: model.GetDMNameFromIds(u1.Id, u1.Id),
}
users := map[string]*model.User{
u1.Id: u1,

View File

@ -676,7 +676,7 @@ func TestIsBotChannel(t *testing.T) {
{
Name: "a direct channel with another user",
Channel: &Channel{
Name: "user1__user2",
Name: GetDMNameFromIds("user1", "user2"),
Type: ChannelTypeDirect,
},
Expected: false,
@ -684,7 +684,7 @@ func TestIsBotChannel(t *testing.T) {
{
Name: "a direct channel with the name containing the bot's ID first",
Channel: &Channel{
Name: "botUserID__user2",
Name: GetDMNameFromIds("botUserID", "user2"),
Type: ChannelTypeDirect,
},
Expected: true,
@ -692,7 +692,7 @@ func TestIsBotChannel(t *testing.T) {
{
Name: "a direct channel with the name containing the bot's ID second",
Channel: &Channel{
Name: "user1__botUserID",
Name: GetDMNameFromIds("user1", "botUserID"),
Type: ChannelTypeDirect,
},
Expected: true,

View File

@ -339,6 +339,9 @@ func (o *Channel) GetOtherUserIdForDM(userId string) string {
}
userIds := strings.Split(o.Name, "__")
if len(userIds) != 2 {
return ""
}
var otherUserId string

View File

@ -634,7 +634,7 @@ func TestShouldProcessMessage(t *testing.T) {
channelID := "1"
api := setupAPI()
channel := model.Channel{
Name: "user1__" + expectedBotID,
Name: model.GetDMNameFromIds("user1", expectedBotID),
Type: model.ChannelTypeDirect,
}
api.On("GetChannel", channelID).Return(&channel, nil)