From 971149d2b298b3408af7218c868ed0b3edd83c2e Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Sat, 4 Jun 2016 10:52:25 -0400 Subject: Don't allow users to be added to a channel they are not in the team of (#3246) --- api/channel.go | 11 +++++++++-- api/channel_test.go | 4 ++++ api/post.go | 5 ++++- i18n/en.json | 4 ++++ store/sql_channel_store.go | 8 ++++---- store/sql_team_store.go | 8 +++++++- 6 files changed, 32 insertions(+), 8 deletions(-) diff --git a/api/channel.go b/api/channel.go index ba6de1a48..6d1604900 100644 --- a/api/channel.go +++ b/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 { diff --git a/api/channel_test.go b/api/channel_test.go index b2bb56952..175b0a14a 100644 --- a/api/channel_test.go +++ b/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) { diff --git a/api/post.go b/api/post.go index 831591784..cf83c4d0d 100644 --- a/api/post.go +++ b/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) diff --git a/i18n/en.json b/i18n/en.json index 2a51826fa..01d514cf4 100644 --- a/i18n/en.json +++ b/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" diff --git a/store/sql_channel_store.go b/store/sql_channel_store.go index 0a8873b57..3636fb62f 100644 --- a/store/sql_channel_store.go +++ b/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()) } diff --git a/store/sql_team_store.go b/store/sql_team_store.go index a9f265273..6e1deeb20 100644 --- a/store/sql_team_store.go +++ b/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 } -- cgit v1.2.3-1-g7c22