Don't allow users to be added to a channel they are not in the team of (#3246)

api/channel.go

@@ -512,8 +512,15 @@ func AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelM
 		return nil, model.NewLocAppError("AddUserToChannel", "api.channel.add_user_to_channel.type.app_error", nil, "")
 	}
 
-	if result := <-Srv.Store.Channel().GetMember(channel.Id, user.Id); result.Err != nil {
-		if result.Err.Id != store.MISSING_MEMBER_ERROR {
+	tmchan := Srv.Store.Team().GetMember(channel.TeamId, user.Id)
+	cmchan := Srv.Store.Channel().GetMember(channel.Id, user.Id)
+
+	if result := <-tmchan; result.Err != nil {
+		return nil, result.Err
+	}
+
+	if result := <-cmchan; result.Err != nil {
+		if result.Err.Id != store.MISSING_CHANNEL_MEMBER_ERROR {
 			return nil, result.Err
 		}
 	} else {

api/channel_test.go

@@ -749,6 +749,7 @@ func TestAddChannelMember(t *testing.T) {
 	Client := th.BasicClient
 	team := th.BasicTeam
 	user2 := th.BasicUser2
+	user3 := th.CreateUser(Client)
 
 	channel1 := &model.Channel{DisplayName: "A Test API Name", Name: "a" + model.NewId() + "a", Type: model.CHANNEL_OPEN, TeamId: team.Id}
 	channel1 = Client.Must(Client.CreateChannel(channel1)).Data.(*model.Channel)
@@ -790,6 +791,9 @@ func TestAddChannelMember(t *testing.T) {
 		t.Fatal("Should have errored, channel deleted")
 	}
 
+	if _, err := Client.AddChannelMember(channel1.Id, user3.Id); err == nil {
+		t.Fatal("Should have errored, user not on team")
+	}
 }
 
 func TestRemoveChannelMember(t *testing.T) {

api/post.go

@@ -603,7 +603,10 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *
 
 	mentionedUsersList := make([]string, 0, len(mentionedUserIds))
 
-	senderName := profileMap[post.UserId].Username
+	senderName := ""
+	if profile, ok := profileMap[post.UserId]; ok {
+		senderName = profile.Username
+	}
 
 	for id := range mentionedUserIds {
 		mentionedUsersList = append(mentionedUsersList, id)

i18n/en.json

@@ -3491,6 +3491,10 @@
     "id": "store.sql_team.get_by_name.app_error",
     "translation": "We couldn't find the existing team"
   },
+  {
+    "id": "store.sql_team.get_member.missing.app_error",
+    "translation": "No team member found for that user id and team id"
+  },
   {
     "id": "store.sql_team.get_member.app_error",
     "translation": "We couldn't get the team member"

store/sql_channel_store.go

@@ -11,9 +11,9 @@ import (
 )
 
 const (
-	MISSING_CHANNEL_ERROR = "store.sql_channel.get_by_name.missing.app_error"
-	MISSING_MEMBER_ERROR  = "store.sql_channel.get_member.missing.app_error"
-	CHANNEL_EXISTS_ERROR  = "store.sql_channel.save_channel.exists.app_error"
+	MISSING_CHANNEL_ERROR        = "store.sql_channel.get_by_name.missing.app_error"
+	MISSING_CHANNEL_MEMBER_ERROR = "store.sql_channel.get_member.missing.app_error"
+	CHANNEL_EXISTS_ERROR         = "store.sql_channel.save_channel.exists.app_error"
 )
 
 type SqlChannelStore struct {
@@ -579,7 +579,7 @@ func (s SqlChannelStore) GetMember(channelId string, userId string) StoreChannel
 
 		if err := s.GetReplica().SelectOne(&member, "SELECT * FROM ChannelMembers WHERE ChannelId = :ChannelId AND UserId = :UserId", map[string]interface{}{"ChannelId": channelId, "UserId": userId}); err != nil {
 			if err == sql.ErrNoRows {
-				result.Err = model.NewLocAppError("SqlChannelStore.GetMember", MISSING_MEMBER_ERROR, nil, "channel_id="+channelId+"user_id="+userId+","+err.Error())
+				result.Err = model.NewLocAppError("SqlChannelStore.GetMember", MISSING_CHANNEL_MEMBER_ERROR, nil, "channel_id="+channelId+"user_id="+userId+","+err.Error())
 			} else {
 				result.Err = model.NewLocAppError("SqlChannelStore.GetMember", "store.sql_channel.get_member.app_error", nil, "channel_id="+channelId+"user_id="+userId+","+err.Error())
 			}

store/sql_team_store.go

@@ -4,6 +4,8 @@
 package store
 
 import (
+	"database/sql"
+
 	"github.com/mattermost/platform/model"
 	"github.com/mattermost/platform/utils"
 )
@@ -420,7 +422,11 @@ func (s SqlTeamStore) GetMember(teamId string, userId string) StoreChannel {
 		var member model.TeamMember
 		err := s.GetReplica().SelectOne(&member, "SELECT * FROM TeamMembers WHERE TeamId = :TeamId AND UserId = :UserId", map[string]interface{}{"TeamId": teamId, "UserId": userId})
 		if err != nil {
-			result.Err = model.NewLocAppError("SqlTeamStore.GetMember", "store.sql_team.get_member.app_error", nil, "teamId="+teamId+" userId="+userId+" "+err.Error())
+			if err == sql.ErrNoRows {
+				result.Err = model.NewLocAppError("SqlTeamStore.GetMember", "store.sql_team.get_member.missing.app_error", nil, "teamId="+teamId+" userId="+userId+" "+err.Error())
+			} else {
+				result.Err = model.NewLocAppError("SqlTeamStore.GetMember", "store.sql_team.get_member.app_error", nil, "teamId="+teamId+" userId="+userId+" "+err.Error())
+			}
 		} else {
 			result.Data = member
 		}