From 0a5f792d2d6ceaa6c9bdb3050acbc4050c0c02f5 Mon Sep 17 00:00:00 2001 From: George Goldberg Date: Wed, 12 Sep 2018 15:32:05 +0100 Subject: MM-11230: Make permissions checks in commands failsafe. (#9392) Also add additional unit tests to make sure the permissions tests are completely solid. --- app/command_channel_header.go | 39 ++++++++++--- app/command_channel_header_test.go | 6 +- app/command_channel_purpose.go | 48 +++++++++++----- app/command_channel_purpose_test.go | 93 ++++++++++++++++++++++++++++++ app/command_channel_rename.go | 57 ++++++++++++++----- app/command_channel_rename_test.go | 65 ++++++++++++++++++++- app/command_invite.go | 103 +++++++++++++++++++++++++--------- app/command_invite_test.go | 7 +-- app/command_join.go | 53 ++++++++++-------- app/command_join_test.go | 106 +++++++++++++++++++++++++++-------- app/command_remove.go | 19 ++++--- app/command_remove_test.go | 109 ++++++++++++++++++++++++++++++++++++ 12 files changed, 577 insertions(+), 128 deletions(-) create mode 100644 app/command_channel_purpose_test.go create mode 100644 app/command_remove_test.go (limited to 'app') diff --git a/app/command_channel_header.go b/app/command_channel_header.go index 100135f48..db92f68b2 100644 --- a/app/command_channel_header.go +++ b/app/command_channel_header.go @@ -4,9 +4,9 @@ package app import ( - "github.com/mattermost/mattermost-server/model" - goi18n "github.com/nicksnyder/go-i18n/i18n" + + "github.com/mattermost/mattermost-server/model" ) type HeaderProvider struct { @@ -37,33 +37,51 @@ func (me *HeaderProvider) GetCommand(a *App, T goi18n.TranslateFunc) *model.Comm func (me *HeaderProvider) DoCommand(a *App, args *model.CommandArgs, message string) *model.CommandResponse { channel, err := a.GetChannel(args.ChannelId) if err != nil { - return &model.CommandResponse{Text: args.T("api.command_channel_header.channel.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_header.channel.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } switch channel.Type { case model.CHANNEL_OPEN: if !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) { - return &model.CommandResponse{Text: args.T("api.command_channel_header.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_header.permission.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } case model.CHANNEL_PRIVATE: if !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) { - return &model.CommandResponse{Text: args.T("api.command_channel_header.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_header.permission.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } case model.CHANNEL_GROUP, model.CHANNEL_DIRECT: // Modifying the header is not linked to any specific permission for group/dm channels, so just check for membership. channelMember, err := a.GetChannelMember(args.ChannelId, args.Session.UserId) if err != nil || channelMember == nil { - return &model.CommandResponse{Text: args.T("api.command_channel_header.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_header.permission.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } default: - return &model.CommandResponse{Text: args.T("api.command_channel_header.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_header.permission.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } if len(message) == 0 { - return &model.CommandResponse{Text: args.T("api.command_channel_header.message.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_header.message.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } patch := &model.ChannelPatch{ @@ -73,7 +91,10 @@ func (me *HeaderProvider) DoCommand(a *App, args *model.CommandArgs, message str _, err = a.PatchChannel(channel, patch, args.UserId) if err != nil { - return &model.CommandResponse{Text: args.T("api.command_channel_header.update_channel.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_header.update_channel.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } return &model.CommandResponse{} diff --git a/app/command_channel_header_test.go b/app/command_channel_header_test.go index 21735e044..99f3b0cc3 100644 --- a/app/command_channel_header_test.go +++ b/app/command_channel_header_test.go @@ -1,10 +1,14 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + package app import ( "testing" - "github.com/mattermost/mattermost-server/model" "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost-server/model" ) func TestHeaderProviderDoCommand(t *testing.T) { diff --git a/app/command_channel_purpose.go b/app/command_channel_purpose.go index 547406692..0ddbf1d64 100644 --- a/app/command_channel_purpose.go +++ b/app/command_channel_purpose.go @@ -4,8 +4,9 @@ package app import ( - "github.com/mattermost/mattermost-server/model" goi18n "github.com/nicksnyder/go-i18n/i18n" + + "github.com/mattermost/mattermost-server/model" ) type PurposeProvider struct { @@ -36,23 +37,39 @@ func (me *PurposeProvider) GetCommand(a *App, T goi18n.TranslateFunc) *model.Com func (me *PurposeProvider) DoCommand(a *App, args *model.CommandArgs, message string) *model.CommandResponse { channel, err := a.GetChannel(args.ChannelId) if err != nil { - return &model.CommandResponse{Text: args.T("api.command_channel_purpose.channel.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } - - if channel.Type == model.CHANNEL_OPEN && !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) { - return &model.CommandResponse{Text: args.T("api.command_channel_purpose.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } - - if channel.Type == model.CHANNEL_PRIVATE && !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) { - return &model.CommandResponse{Text: args.T("api.command_channel_purpose.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_purpose.channel.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } - if channel.Type == model.CHANNEL_GROUP || channel.Type == model.CHANNEL_DIRECT { - return &model.CommandResponse{Text: args.T("api.command_channel_purpose.direct_group.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + switch channel.Type { + case model.CHANNEL_OPEN: + if !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) { + return &model.CommandResponse{ + Text: args.T("api.command_channel_purpose.permission.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } + } + case model.CHANNEL_PRIVATE: + if !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) { + return &model.CommandResponse{ + Text: args.T("api.command_channel_purpose.permission.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } + } + default: + return &model.CommandResponse{ + Text: args.T("api.command_channel_purpose.direct_group.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } if len(message) == 0 { - return &model.CommandResponse{Text: args.T("api.command_channel_purpose.message.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_purpose.message.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } patch := &model.ChannelPatch{ @@ -62,7 +79,10 @@ func (me *PurposeProvider) DoCommand(a *App, args *model.CommandArgs, message st _, err = a.PatchChannel(channel, patch, args.UserId) if err != nil { - return &model.CommandResponse{Text: args.T("api.command_channel_purpose.update_channel.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_purpose.update_channel.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } return &model.CommandResponse{} diff --git a/app/command_channel_purpose_test.go b/app/command_channel_purpose_test.go new file mode 100644 index 000000000..3bdaa4e4f --- /dev/null +++ b/app/command_channel_purpose_test.go @@ -0,0 +1,93 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost-server/model" +) + +func TestPurposeProviderDoCommand(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + pp := PurposeProvider{} + + // Try a public channel *with* permission. + args := &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: th.BasicChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: model.TEAM_USER_ROLE_ID}}}, + } + + for msg, expected := range map[string]string{ + "": "api.command_channel_purpose.message.app_error", + "hello": "", + } { + actual := pp.DoCommand(th.App, args, msg).Text + assert.Equal(t, expected, actual) + } + + // Try a public channel *without* permission. + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: th.BasicChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual := pp.DoCommand(th.App, args, "hello").Text + assert.Equal(t, "api.command_channel_purpose.permission.app_error", actual) + + // Try a private channel *with* permission. + privateChannel := th.CreatePrivateChannel(th.BasicTeam) + + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: privateChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: model.TEAM_USER_ROLE_ID}}}, + } + + actual = pp.DoCommand(th.App, args, "hello").Text + assert.Equal(t, "", actual) + + // Try a private channel *without* permission. + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: privateChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = pp.DoCommand(th.App, args, "hello").Text + assert.Equal(t, "api.command_channel_purpose.permission.app_error", actual) + + // Try a group channel *with* being a member. + user1 := th.CreateUser() + user2 := th.CreateUser() + + groupChannel := th.CreateGroupChannel(user1, user2) + + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: groupChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = pp.DoCommand(th.App, args, "hello").Text + assert.Equal(t, "api.command_channel_purpose.direct_group.app_error", actual) + + // Try a direct channel *with* being a member. + directChannel := th.CreateDmChannel(user1) + + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: directChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = pp.DoCommand(th.App, args, "hello").Text + assert.Equal(t, "api.command_channel_purpose.direct_group.app_error", actual) +} diff --git a/app/command_channel_rename.go b/app/command_channel_rename.go index ddcfea67a..a2e45ed46 100644 --- a/app/command_channel_rename.go +++ b/app/command_channel_rename.go @@ -4,8 +4,9 @@ package app import ( - "github.com/mattermost/mattermost-server/model" goi18n "github.com/nicksnyder/go-i18n/i18n" + + "github.com/mattermost/mattermost-server/model" ) type RenameProvider struct { @@ -36,27 +37,50 @@ func (me *RenameProvider) GetCommand(a *App, T goi18n.TranslateFunc) *model.Comm func (me *RenameProvider) DoCommand(a *App, args *model.CommandArgs, message string) *model.CommandResponse { channel, err := a.GetChannel(args.ChannelId) if err != nil { - return &model.CommandResponse{Text: args.T("api.command_channel_rename.channel.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } - - if channel.Type == model.CHANNEL_OPEN && !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) { - return &model.CommandResponse{Text: args.T("api.command_channel_rename.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } - - if channel.Type == model.CHANNEL_PRIVATE && !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) { - return &model.CommandResponse{Text: args.T("api.command_channel_rename.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_rename.channel.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } - if channel.Type == model.CHANNEL_GROUP || channel.Type == model.CHANNEL_DIRECT { + switch channel.Type { + case model.CHANNEL_OPEN: + if !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) { + return &model.CommandResponse{ + Text: args.T("api.command_channel_rename.permission.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } + } + case model.CHANNEL_PRIVATE: + if !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) { + return &model.CommandResponse{ + Text: args.T("api.command_channel_rename.permission.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } + } + default: return &model.CommandResponse{Text: args.T("api.command_channel_rename.direct_group.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} } if len(message) == 0 { - return &model.CommandResponse{Text: args.T("api.command_channel_rename.message.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_rename.message.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } else if len(message) > model.CHANNEL_NAME_UI_MAX_LENGTH { - return &model.CommandResponse{Text: args.T("api.command_channel_rename.too_long.app_error", map[string]interface{}{"Length": model.CHANNEL_NAME_UI_MAX_LENGTH}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_rename.too_long.app_error", map[string]interface{}{ + "Length": model.CHANNEL_NAME_UI_MAX_LENGTH, + }), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } else if len(message) < model.CHANNEL_NAME_MIN_LENGTH { - return &model.CommandResponse{Text: args.T("api.command_channel_rename.too_short.app_error", map[string]interface{}{"Length": model.CHANNEL_NAME_MIN_LENGTH}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_rename.too_short.app_error", map[string]interface{}{ + "Length": model.CHANNEL_NAME_MIN_LENGTH, + }), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } patch := &model.ChannelPatch{ @@ -66,7 +90,10 @@ func (me *RenameProvider) DoCommand(a *App, args *model.CommandArgs, message str _, err = a.PatchChannel(channel, patch, args.UserId) if err != nil { - return &model.CommandResponse{Text: args.T("api.command_channel_rename.update_channel.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_channel_rename.update_channel.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } return &model.CommandResponse{} diff --git a/app/command_channel_rename_test.go b/app/command_channel_rename_test.go index 9c86b18e0..d4cdeda51 100644 --- a/app/command_channel_rename_test.go +++ b/app/command_channel_rename_test.go @@ -1,10 +1,14 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + package app import ( "testing" - "github.com/mattermost/mattermost-server/model" "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost-server/model" ) func TestRenameProviderDoCommand(t *testing.T) { @@ -29,4 +33,63 @@ func TestRenameProviderDoCommand(t *testing.T) { actual := rp.DoCommand(th.App, args, msg).Text assert.Equal(t, expected, actual) } + + // Try a public channel *without* permission. + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: th.BasicChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual := rp.DoCommand(th.App, args, "hello").Text + assert.Equal(t, "api.command_channel_rename.permission.app_error", actual) + + // Try a private channel *with* permission. + privateChannel := th.CreatePrivateChannel(th.BasicTeam) + + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: privateChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: model.TEAM_USER_ROLE_ID}}}, + } + + actual = rp.DoCommand(th.App, args, "hello").Text + assert.Equal(t, "", actual) + + // Try a private channel *without* permission. + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: privateChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = rp.DoCommand(th.App, args, "hello").Text + assert.Equal(t, "api.command_channel_rename.permission.app_error", actual) + + // Try a group channel *with* being a member. + user1 := th.CreateUser() + user2 := th.CreateUser() + + groupChannel := th.CreateGroupChannel(user1, user2) + + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: groupChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = rp.DoCommand(th.App, args, "hello").Text + assert.Equal(t, "api.command_channel_rename.direct_group.app_error", actual) + + // Try a direct channel *with* being a member. + directChannel := th.CreateDmChannel(user1) + + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: directChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = rp.DoCommand(th.App, args, "hello").Text + assert.Equal(t, "api.command_channel_rename.direct_group.app_error", actual) } diff --git a/app/command_invite.go b/app/command_invite.go index 86cc5fdbb..e6167dad6 100644 --- a/app/command_invite.go +++ b/app/command_invite.go @@ -6,9 +6,10 @@ package app import ( "strings" + goi18n "github.com/nicksnyder/go-i18n/i18n" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" - goi18n "github.com/nicksnyder/go-i18n/i18n" ) type InviteProvider struct { @@ -38,7 +39,10 @@ func (me *InviteProvider) GetCommand(a *App, T goi18n.TranslateFunc) *model.Comm func (me *InviteProvider) DoCommand(a *App, args *model.CommandArgs, message string) *model.CommandResponse { if message == "" { - return &model.CommandResponse{Text: args.T("api.command_invite.missing_message.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_invite.missing_message.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } splitMessage := strings.SplitN(message, " ", 2) @@ -48,7 +52,10 @@ func (me *InviteProvider) DoCommand(a *App, args *model.CommandArgs, message str var userProfile *model.User if result := <-a.Srv.Store.User().GetByUsername(targetUsername); result.Err != nil { mlog.Error(result.Err.Error()) - return &model.CommandResponse{Text: args.T("api.command_invite.missing_user.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_invite.missing_user.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } else { userProfile = result.Data.(*model.User) } @@ -60,49 +67,89 @@ func (me *InviteProvider) DoCommand(a *App, args *model.CommandArgs, message str targetChannelName := strings.TrimPrefix(strings.TrimSpace(splitMessage[1]), "~") if channelToJoin, err = a.GetChannelByName(targetChannelName, args.TeamId, false); err != nil { - return &model.CommandResponse{Text: args.T("api.command_invite.channel.error", map[string]interface{}{"Channel": targetChannelName}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_invite.channel.error", map[string]interface{}{ + "Channel": targetChannelName, + }), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } } else { channelToJoin, err = a.GetChannel(args.ChannelId) if err != nil { - return &model.CommandResponse{Text: args.T("api.command_invite.channel.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_invite.channel.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } } - // Check if is a Direct Channel - if channelToJoin.Type == model.CHANNEL_DIRECT || channelToJoin.Type == model.CHANNEL_GROUP { - return &model.CommandResponse{Text: args.T("api.command_invite.directchannel.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } - - // Check Permissions - if channelToJoin.Type == model.CHANNEL_OPEN && !a.SessionHasPermissionToChannel(args.Session, channelToJoin.Id, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) { - return &model.CommandResponse{Text: args.T("api.command_invite.permission.app_error", map[string]interface{}{"User": userProfile.Username, "Channel": channelToJoin.Name}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } - - // Check if the user who wants to add another is trying to add in a pvt channel, but does not have permission - // but is in the channel - _, err = a.GetChannelMember(channelToJoin.Id, args.UserId) - if channelToJoin.Type == model.CHANNEL_PRIVATE && !a.SessionHasPermissionToChannel(args.Session, channelToJoin.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) && err == nil { - return &model.CommandResponse{Text: args.T("api.command_invite.permission.app_error", map[string]interface{}{"User": userProfile.Username, "Channel": channelToJoin.Name}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } - - // In this case just check if is a pvt channel and user has permission - if channelToJoin.Type == model.CHANNEL_PRIVATE && !a.SessionHasPermissionToChannel(args.Session, channelToJoin.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) { - return &model.CommandResponse{Text: args.T("api.command_invite.private_channel.app_error", map[string]interface{}{"Channel": channelToJoin.Name}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + // Permissions Check + switch channelToJoin.Type { + case model.CHANNEL_OPEN: + if !a.SessionHasPermissionToChannel(args.Session, channelToJoin.Id, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) { + return &model.CommandResponse{ + Text: args.T("api.command_invite.permission.app_error", map[string]interface{}{ + "User": userProfile.Username, + "Channel": channelToJoin.Name, + }), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } + } + case model.CHANNEL_PRIVATE: + if !a.SessionHasPermissionToChannel(args.Session, channelToJoin.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) { + if _, err = a.GetChannelMember(channelToJoin.Id, args.UserId); err == nil { + // User doing the inviting is a member of the channel. + return &model.CommandResponse{ + Text: args.T("api.command_invite.permission.app_error", map[string]interface{}{ + "User": userProfile.Username, + "Channel": channelToJoin.Name, + }), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } + } else { + // User doing the inviting is *not* a member of the channel. + return &model.CommandResponse{ + Text: args.T("api.command_invite.private_channel.app_error", map[string]interface{}{ + "Channel": channelToJoin.Name, + }), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } + } + } + default: + return &model.CommandResponse{ + Text: args.T("api.command_invite.directchannel.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } // Check if user is already in the channel _, err = a.GetChannelMember(channelToJoin.Id, userProfile.Id) if err == nil { - return &model.CommandResponse{Text: args.T("api.command_invite.user_already_in_channel.app_error", map[string]interface{}{"User": userProfile.Username}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_invite.user_already_in_channel.app_error", map[string]interface{}{ + "User": userProfile.Username, + }), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } if _, err := a.AddChannelMember(userProfile.Id, channelToJoin, args.Session.UserId, ""); err != nil { - return &model.CommandResponse{Text: args.T("api.command_invite.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_invite.fail.app_error"), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } if args.ChannelId != channelToJoin.Id { - return &model.CommandResponse{Text: args.T("api.command_invite.success", map[string]interface{}{"User": userProfile.Username, "Channel": channelToJoin.Name}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{ + Text: args.T("api.command_invite.success", map[string]interface{}{ + "User": userProfile.Username, + "Channel": channelToJoin.Name, + }), + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + } } return &model.CommandResponse{} diff --git a/app/command_invite_test.go b/app/command_invite_test.go index 0d1db4a07..f141cb239 100644 --- a/app/command_invite_test.go +++ b/app/command_invite_test.go @@ -95,12 +95,7 @@ func TestInviteProvider(t *testing.T) { msg: basicUser4.Username, }, { - desc: "try to add a user to a direct channel", - expected: "api.command_invite.directchannel.app_error", - msg: userAndDMChannel, - }, - { - desc: "try to add a user to a privante channel with no permission", + desc: "try to add a user to a private channel with no permission", expected: "api.command_invite.private_channel.app_error", msg: userAndInvalidPrivate, }, diff --git a/app/command_join.go b/app/command_join.go index 61ed65ba6..b913014b8 100644 --- a/app/command_join.go +++ b/app/command_join.go @@ -4,9 +4,11 @@ package app import ( - "github.com/mattermost/mattermost-server/model" - goi18n "github.com/nicksnyder/go-i18n/i18n" "strings" + + goi18n "github.com/nicksnyder/go-i18n/i18n" + + "github.com/mattermost/mattermost-server/model" ) type JoinProvider struct { @@ -41,33 +43,38 @@ func (me *JoinProvider) DoCommand(a *App, args *model.CommandArgs, message strin channelName = message[1:] } - if result := <-a.Srv.Store.Channel().GetByName(args.TeamId, channelName, true); result.Err != nil { + result := <-a.Srv.Store.Channel().GetByName(args.TeamId, channelName, true) + if result.Err != nil { return &model.CommandResponse{Text: args.T("api.command_join.list.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } else { - channel := result.Data.(*model.Channel) + } - if channel.Name == channelName { - allowed := false - if (channel.Type == model.CHANNEL_PRIVATE && a.SessionHasPermissionToChannel(args.Session, channel.Id, model.PERMISSION_READ_CHANNEL)) || channel.Type == model.CHANNEL_OPEN { - allowed = true - } + channel := result.Data.(*model.Channel) - if !allowed { - return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } + if channel.Name != channelName { + return &model.CommandResponse{ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, Text: args.T("api.command_join.missing.app_error")} + } - if err := a.JoinChannel(channel, args.UserId); err != nil { - return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } + switch channel.Type { + case model.CHANNEL_OPEN: + if !a.SessionHasPermissionToChannel(args.Session, channel.Id, model.PERMISSION_JOIN_PUBLIC_CHANNELS) { + return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + } + case model.CHANNEL_PRIVATE: + if !a.SessionHasPermissionToChannel(args.Session, channel.Id, model.PERMISSION_READ_CHANNEL) { + return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + } + default: + return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + } - team, err := a.GetTeam(channel.TeamId) - if err != nil { - return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } + if err := a.JoinChannel(channel, args.UserId); err != nil { + return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + } - return &model.CommandResponse{GotoLocation: args.SiteURL + "/" + team.Name + "/channels/" + channel.Name} - } + team, err := a.GetTeam(channel.TeamId) + if err != nil { + return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} } - return &model.CommandResponse{ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, Text: args.T("api.command_join.missing.app_error")} + return &model.CommandResponse{GotoLocation: args.SiteURL + "/" + team.Name + "/channels/" + channel.Name} } diff --git a/app/command_join_test.go b/app/command_join_test.go index 77574217b..e5f42f31e 100644 --- a/app/command_join_test.go +++ b/app/command_join_test.go @@ -5,9 +5,11 @@ package app import ( "testing" - "github.com/mattermost/mattermost-server/model" + "github.com/nicksnyder/go-i18n/i18n" "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost-server/model" ) func TestJoinCommandNoChannel(t *testing.T) { @@ -20,10 +22,11 @@ func TestJoinCommandNoChannel(t *testing.T) { cmd := &JoinProvider{} resp := cmd.DoCommand(th.App, &model.CommandArgs{ - T: i18n.IdentityTfunc(), - UserId: th.BasicUser2.Id, + T: i18n.IdentityTfunc(), + UserId: th.BasicUser2.Id, SiteURL: "http://test.url", - TeamId: th.BasicTeam.Id, + TeamId: th.BasicTeam.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: model.TEAM_USER_ROLE_ID}}}, }, "asdsad") assert.Equal(t, "api.command_join.list.app_error", resp.Text) @@ -38,20 +41,20 @@ func TestJoinCommandForExistingChannel(t *testing.T) { } channel2, _ := th.App.CreateChannel(&model.Channel{ - DisplayName: "AA", - Name: "aa" + model.NewId() + "a", - Type: model.CHANNEL_OPEN, - TeamId: th.BasicTeam.Id, - CreatorId: th.BasicUser.Id, + DisplayName: "AA", + Name: "aa" + model.NewId() + "a", + Type: model.CHANNEL_OPEN, + TeamId: th.BasicTeam.Id, + CreatorId: th.BasicUser.Id, }, false) - cmd := &JoinProvider{} resp := cmd.DoCommand(th.App, &model.CommandArgs{ - T: i18n.IdentityTfunc(), - UserId: th.BasicUser2.Id, + T: i18n.IdentityTfunc(), + UserId: th.BasicUser2.Id, SiteURL: "http://test.url", - TeamId: th.BasicTeam.Id, + TeamId: th.BasicTeam.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: model.TEAM_USER_ROLE_ID}}}, }, channel2.Name) assert.Equal(t, "", resp.Text) @@ -67,22 +70,81 @@ func TestJoinCommandWithTilde(t *testing.T) { } channel2, _ := th.App.CreateChannel(&model.Channel{ - DisplayName: "AA", - Name: "aa" + model.NewId() + "a", - Type: model.CHANNEL_OPEN, - TeamId: th.BasicTeam.Id, - CreatorId: th.BasicUser.Id, + DisplayName: "AA", + Name: "aa" + model.NewId() + "a", + Type: model.CHANNEL_OPEN, + TeamId: th.BasicTeam.Id, + CreatorId: th.BasicUser.Id, }, false) - cmd := &JoinProvider{} resp := cmd.DoCommand(th.App, &model.CommandArgs{ - T: i18n.IdentityTfunc(), - UserId: th.BasicUser2.Id, + T: i18n.IdentityTfunc(), + UserId: th.BasicUser2.Id, SiteURL: "http://test.url", - TeamId: th.BasicTeam.Id, + TeamId: th.BasicTeam.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: model.TEAM_USER_ROLE_ID}}}, }, "~"+channel2.Name) assert.Equal(t, "", resp.Text) assert.Equal(t, "http://test.url/"+th.BasicTeam.Name+"/channels/"+channel2.Name, resp.GotoLocation) } + +func TestJoinCommandPermissions(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + channel2, _ := th.App.CreateChannel(&model.Channel{ + DisplayName: "AA", + Name: "aa" + model.NewId() + "a", + Type: model.CHANNEL_OPEN, + TeamId: th.BasicTeam.Id, + CreatorId: th.BasicUser.Id, + }, false) + + cmd := &JoinProvider{} + + // Try a public channel *without* permission. + args := &model.CommandArgs{ + T: i18n.IdentityTfunc(), + UserId: th.BasicUser2.Id, + SiteURL: "http://test.url", + TeamId: th.BasicTeam.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual := cmd.DoCommand(th.App, args, "~"+channel2.Name).Text + assert.Equal(t, "api.command_join.fail.app_error", actual) + + // Try a public channel with permission. + args = &model.CommandArgs{ + T: i18n.IdentityTfunc(), + UserId: th.BasicUser2.Id, + SiteURL: "http://test.url", + TeamId: th.BasicTeam.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: model.TEAM_USER_ROLE_ID}}}, + } + + actual = cmd.DoCommand(th.App, args, "~"+channel2.Name).Text + assert.Equal(t, "", actual) + + // Try a private channel *without* permission. + channel3, _ := th.App.CreateChannel(&model.Channel{ + DisplayName: "BB", + Name: "aa" + model.NewId() + "a", + Type: model.CHANNEL_PRIVATE, + TeamId: th.BasicTeam.Id, + CreatorId: th.BasicUser.Id, + }, false) + + args = &model.CommandArgs{ + T: i18n.IdentityTfunc(), + UserId: th.BasicUser2.Id, + SiteURL: "http://test.url", + TeamId: th.BasicTeam.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: model.TEAM_USER_ROLE_ID}}}, + } + + actual = cmd.DoCommand(th.App, args, "~"+channel3.Name).Text + assert.Equal(t, "api.command_join.fail.app_error", actual) +} diff --git a/app/command_remove.go b/app/command_remove.go index 3671a2063..6a67996e9 100644 --- a/app/command_remove.go +++ b/app/command_remove.go @@ -70,15 +70,16 @@ func doCommand(a *App, args *model.CommandArgs, message string) *model.CommandRe return &model.CommandResponse{Text: args.T("api.command_channel_rename.channel.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} } - if channel.Type == model.CHANNEL_OPEN && !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) { - return &model.CommandResponse{Text: args.T("api.command_remove.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } - - if channel.Type == model.CHANNEL_PRIVATE && !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) { - return &model.CommandResponse{Text: args.T("api.command_remove.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} - } - - if channel.Type == model.CHANNEL_GROUP || channel.Type == model.CHANNEL_DIRECT { + switch channel.Type { + case model.CHANNEL_OPEN: + if !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) { + return &model.CommandResponse{Text: args.T("api.command_remove.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + } + case model.CHANNEL_PRIVATE: + if !a.SessionHasPermissionToChannel(args.Session, args.ChannelId, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) { + return &model.CommandResponse{Text: args.T("api.command_remove.permission.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + } + default: return &model.CommandResponse{Text: args.T("api.command_remove.direct_group.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} } diff --git a/app/command_remove_test.go b/app/command_remove_test.go new file mode 100644 index 000000000..f17a70bad --- /dev/null +++ b/app/command_remove_test.go @@ -0,0 +1,109 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost-server/model" +) + +func TestRemoveProviderDoCommand(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + rp := RemoveProvider{} + + publicChannel, _ := th.App.CreateChannel(&model.Channel{ + DisplayName: "AA", + Name: "aa" + model.NewId() + "a", + Type: model.CHANNEL_OPEN, + TeamId: th.BasicTeam.Id, + CreatorId: th.BasicUser.Id, + }, false) + + privateChannel, _ := th.App.CreateChannel(&model.Channel{ + DisplayName: "BB", + Name: "aa" + model.NewId() + "a", + Type: model.CHANNEL_OPEN, + TeamId: th.BasicTeam.Id, + CreatorId: th.BasicUser.Id, + }, false) + + targetUser := th.CreateUser() + th.App.AddUserToTeam(th.BasicTeam.Id, targetUser.Id, targetUser.Id) + th.App.AddUserToChannel(targetUser, publicChannel) + th.App.AddUserToChannel(targetUser, privateChannel) + + // Try a public channel *without* permission. + args := &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: publicChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual := rp.DoCommand(th.App, args, targetUser.Username).Text + assert.Equal(t, "api.command_remove.permission.app_error", actual) + + // Try a public channel *with* permission. + th.App.AddUserToChannel(th.BasicUser, publicChannel) + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: publicChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = rp.DoCommand(th.App, args, targetUser.Username).Text + assert.Equal(t, "", actual) + + // Try a private channel *without* permission. + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: privateChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = rp.DoCommand(th.App, args, targetUser.Username).Text + assert.Equal(t, "api.command_remove.permission.app_error", actual) + + // Try a private channel *with* permission. + th.App.AddUserToChannel(th.BasicUser, privateChannel) + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: privateChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = rp.DoCommand(th.App, args, targetUser.Username).Text + assert.Equal(t, "", actual) + + // Try a group channel + user1 := th.CreateUser() + user2 := th.CreateUser() + + groupChannel := th.CreateGroupChannel(user1, user2) + + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: groupChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = rp.DoCommand(th.App, args, user1.Username).Text + assert.Equal(t, "api.command_remove.direct_group.app_error", actual) + + // Try a direct channel *with* being a member. + directChannel := th.CreateDmChannel(user1) + + args = &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: directChannel.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: ""}}}, + } + + actual = rp.DoCommand(th.App, args, user1.Username).Text + assert.Equal(t, "api.command_remove.direct_group.app_error", actual) +} -- cgit v1.2.3-1-g7c22