MM-22560: Fix crash during slack import (#14054)

* MM-22560: Fix crash during slack import

We were not checking for existence of the users map value
before accessing the Id field. We fix it by checking for
nil pointer first.

Also, we check the length of the Members slice to prevent range
panics.

And finally, while we are here, we move to a switch-case
to make the code a little more idiomatic.

* Adding warn logs when user not found

Co-authored-by: mattermod <mattermod@users.noreply.github.com>

app/slackimport.go

@@ -845,30 +845,47 @@ func (a *App) oldImportUser(team *model.Team, user *model.User) *model.User {
 }
 
 func (a *App) oldImportChannel(channel *model.Channel, sChannel SlackChannel, users map[string]*model.User) *model.Channel {
-	if channel.Type == model.CHANNEL_DIRECT {
-		sc, err := a.createDirectChannel(users[sChannel.Members[0]].Id, users[sChannel.Members[1]].Id)
+	switch {
+	case channel.Type == model.CHANNEL_DIRECT:
+		if len(sChannel.Members) < 2 {
+			return nil
+		}
+		u1 := users[sChannel.Members[0]]
+		u2 := users[sChannel.Members[1]]
+		if u1 == nil || u2 == nil {
+			mlog.Warn("Either or both of user ids not found in users.json. Ignoring.", mlog.String("id1", sChannel.Members[0]), mlog.String("id2", sChannel.Members[1]))
+			return nil
+		}
+		sc, err := a.createDirectChannel(u1.Id, u2.Id)
 		if err != nil {
 			return nil
 		}
 
 		return sc
-	}
-
 	// check if direct channel has less than 8 members and if not import as private channel instead
-	if channel.Type == model.CHANNEL_GROUP && len(sChannel.Members) < 8 {
+	case channel.Type == model.CHANNEL_GROUP && len(sChannel.Members) < 8:
 		members := make([]string, len(sChannel.Members))
 
 		for i := range sChannel.Members {
-			members[i] = users[sChannel.Members[i]].Id
+			u := users[sChannel.Members[i]]
+			if u == nil {
+				mlog.Warn("User not found in users.json. Ignoring.", mlog.String("id", sChannel.Members[i]))
+				continue
+			}
+			members[i] = u.Id
 		}
 
-		sc, err := a.createGroupChannel(members, users[sChannel.Creator].Id)
+		creator := users[sChannel.Creator]
+		if creator == nil {
+			return nil
+		}
+		sc, err := a.createGroupChannel(members, creator.Id)
 		if err != nil {
 			return nil
 		}
 
 		return sc
-	} else if channel.Type == model.CHANNEL_GROUP {
+	case channel.Type == model.CHANNEL_GROUP:
 		channel.Type = model.CHANNEL_PRIVATE
 		sc, err := a.CreateChannel(channel, false)
 		if err != nil {

app/slackimport_test.go

@@ -311,3 +311,51 @@ in this~.`,
 
 	assert.Equal(t, expectedOutput, SlackConvertPostsMarkup(input))
 }
+
+func TestOldImportChannel(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	u1 := th.CreateUser()
+	u2 := th.CreateUser()
+	t.Run("No panic on direct channel", func(t *testing.T) {
+		ch := th.CreateDmChannel(u1)
+		users := map[string]*model.User{
+			th.BasicUser.Id: th.BasicUser,
+		}
+		sCh := SlackChannel{
+			Id:      "someid",
+			Members: []string{u1.Id, "randomID"},
+			Creator: "randomID2",
+		}
+
+		_ = th.App.oldImportChannel(ch, sCh, users)
+	})
+
+	t.Run("No panic on direct channel with 1 member", func(t *testing.T) {
+		ch := th.CreateDmChannel(u1)
+		users := map[string]*model.User{
+			th.BasicUser.Id: th.BasicUser,
+		}
+		sCh := SlackChannel{
+			Id:      "someid",
+			Members: []string{th.BasicUser.Id},
+			Creator: "randomID2",
+		}
+
+		_ = th.App.oldImportChannel(ch, sCh, users)
+	})
+
+	t.Run("No panic on group channel", func(t *testing.T) {
+		ch := th.CreateGroupChannel(u1, u2)
+		users := map[string]*model.User{
+			th.BasicUser.Id: th.BasicUser,
+		}
+		sCh := SlackChannel{
+			Id:      "someid",
+			Members: []string{th.BasicUser.Id},
+			Creator: "randomID2",
+		}
+		_ = th.App.oldImportChannel(ch, sCh, users)
+	})
+}