From b884c8c4104fc83aa382575df4ea95302506e8f1 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Tue, 17 Oct 2017 13:21:12 -0400 Subject: PLT-7193: Regression - Custom slash commands don't work in direct or group message channels (#7635) * No longer overriding specified team id for DMs/GMs, as these types of channels don't belong to a team, and doing so breaks slash commands for them * Ensured user is on specified team in case of GM/DM, extended test suite --- api4/apitestlib.go | 6 ++- api4/command.go | 18 +++++-- api4/command_test.go | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 148 insertions(+), 7 deletions(-) (limited to 'api4') diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 640f38fe6..d54420f57 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -296,13 +296,17 @@ func (me *TestHelper) CreatePrivateChannel() *model.Channel { } func (me *TestHelper) CreateChannelWithClient(client *model.Client4, channelType string) *model.Channel { + return me.CreateChannelWithClientAndTeam(client, channelType, me.BasicTeam.Id) +} + +func (me *TestHelper) CreateChannelWithClientAndTeam(client *model.Client4, channelType string, teamId string) *model.Channel { id := model.NewId() channel := &model.Channel{ DisplayName: "dn_" + id, Name: GenerateTestChannelName(), Type: channelType, - TeamId: me.BasicTeam.Id, + TeamId: teamId, } utils.DisableDebugLogForTest() diff --git a/api4/command.go b/api4/command.go index 4314a184d..33e6a6c0c 100644 --- a/api4/command.go +++ b/api4/command.go @@ -201,6 +201,7 @@ func executeCommand(c *Context, w http.ResponseWriter, r *http.Request) { return } + // checks that user is a member of the specified channel, and that they have permission to use slash commands in it if !c.App.SessionHasPermissionToChannel(c.Session, commandArgs.ChannelId, model.PERMISSION_USE_SLASH_COMMANDS) { c.SetPermissionError(model.PERMISSION_USE_SLASH_COMMANDS) return @@ -210,12 +211,21 @@ func executeCommand(c *Context, w http.ResponseWriter, r *http.Request) { if err != nil { c.Err = err return + } else if channel.Type != model.CHANNEL_DIRECT && channel.Type != model.CHANNEL_GROUP { + // if this isn't a DM or GM, the team id is implicitly taken from the channel so that slash commands created on + // some other team can't be run against this one + commandArgs.TeamId = channel.TeamId + } else { + // if the slash command was used in a DM or GM, ensure that the user is a member of the specified team, so that + // they can't just execute slash commands against arbitrary teams + if c.Session.GetTeamByTeamId(commandArgs.TeamId) == nil { + if !app.SessionHasPermissionTo(c.Session, model.PERMISSION_USE_SLASH_COMMANDS) { + c.SetPermissionError(model.PERMISSION_USE_SLASH_COMMANDS) + return + } + } } - // team id is implicitly taken from channel so that slash commands - // created on some other team can't be run against this one - commandArgs.TeamId = channel.TeamId - commandArgs.UserId = c.Session.UserId commandArgs.T = c.T commandArgs.Session = c.Session diff --git a/api4/command_test.go b/api4/command_test.go index 52ef0f841..716b3e414 100644 --- a/api4/command_test.go +++ b/api4/command_test.go @@ -493,7 +493,7 @@ func TestExecuteCommand(t *testing.T) { } func TestExecuteCommandAgainstChannelOnAnotherTeam(t *testing.T) { - th := Setup().InitBasic().InitSystemAdmin() + th := Setup().InitBasic() defer th.TearDown() Client := th.Client channel := th.BasicChannel @@ -516,7 +516,6 @@ func TestExecuteCommandAgainstChannelOnAnotherTeam(t *testing.T) { Method: model.COMMAND_METHOD_POST, Trigger: "postcommand", } - if _, err := th.App.CreateCommand(postCmd); err != nil { t.Fatal("failed to create post command") } @@ -526,3 +525,131 @@ func TestExecuteCommandAgainstChannelOnAnotherTeam(t *testing.T) { _, resp := Client.ExecuteCommand(channel.Id, "/postcommand") CheckNotFoundStatus(t, resp) } + +func TestExecuteCommandAgainstChannelUserIsNotIn(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + client := th.Client + + enableCommands := *utils.Cfg.ServiceSettings.EnableCommands + allowedInternalConnections := *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections + defer func() { + utils.Cfg.ServiceSettings.EnableCommands = &enableCommands + utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections + }() + *utils.Cfg.ServiceSettings.EnableCommands = true + *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost" + + // create a slash command on some other team where we have permission to do so + team2 := th.CreateTeam() + postCmd := &model.Command{ + CreatorId: th.BasicUser.Id, + TeamId: team2.Id, + URL: fmt.Sprintf("http://localhost:%v", th.App.Srv.ListenAddr.Port) + model.API_URL_SUFFIX_V4 + "/teams/command_test", + Method: model.COMMAND_METHOD_POST, + Trigger: "postcommand", + } + if _, err := th.App.CreateCommand(postCmd); err != nil { + t.Fatal("failed to create post command") + } + + // make a channel on that team, ensuring that our test user isn't in it + channel2 := th.CreateChannelWithClientAndTeam(client, model.CHANNEL_OPEN, team2.Id) + if success, _ := client.RemoveUserFromChannel(channel2.Id, th.BasicUser.Id); !success { + t.Fatal("Failed to remove user from channel") + } + + // we should not be able to run the slash command in channel2, because we aren't in it + _, resp := client.ExecuteCommandWithTeam(channel2.Id, team2.Id, "/postcommand") + CheckForbiddenStatus(t, resp) +} + +func TestExecuteCommandInDirectMessageChannel(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + client := th.Client + + enableCommands := *utils.Cfg.ServiceSettings.EnableCommands + allowedInternalConnections := *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections + defer func() { + utils.Cfg.ServiceSettings.EnableCommands = &enableCommands + utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections + }() + *utils.Cfg.ServiceSettings.EnableCommands = true + *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost" + + // create a slash command on some other team where we have permission to do so + team2 := th.CreateTeam() + postCmd := &model.Command{ + CreatorId: th.BasicUser.Id, + TeamId: team2.Id, + URL: fmt.Sprintf("http://localhost:%v", th.App.Srv.ListenAddr.Port) + model.API_URL_SUFFIX_V4 + "/teams/command_test", + Method: model.COMMAND_METHOD_POST, + Trigger: "postcommand", + } + if _, err := th.App.CreateCommand(postCmd); err != nil { + t.Fatal("failed to create post command") + } + + // make a direct message channel + dmChannel, response := client.CreateDirectChannel(th.BasicUser.Id, th.BasicUser2.Id) + CheckCreatedStatus(t, response) + + // we should be able to run the slash command in the DM channel + _, resp := client.ExecuteCommandWithTeam(dmChannel.Id, team2.Id, "/postcommand") + CheckOKStatus(t, resp) + + // but we can't run the slash command in the DM channel if we sub in some other team's id + _, resp = client.ExecuteCommandWithTeam(dmChannel.Id, th.BasicTeam.Id, "/postcommand") + CheckNotFoundStatus(t, resp) +} + +func TestExecuteCommandInTeamUserIsNotOn(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + client := th.Client + + enableCommands := *utils.Cfg.ServiceSettings.EnableCommands + allowedInternalConnections := *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections + defer func() { + utils.Cfg.ServiceSettings.EnableCommands = &enableCommands + utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections + }() + *utils.Cfg.ServiceSettings.EnableCommands = true + *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost" + + // create a team that the user isn't a part of + team2 := th.CreateTeam() + + // create a slash command on that team + postCmd := &model.Command{ + CreatorId: th.BasicUser.Id, + TeamId: team2.Id, + URL: fmt.Sprintf("http://localhost:%v", th.App.Srv.ListenAddr.Port) + model.API_URL_SUFFIX_V4 + "/teams/command_test", + Method: model.COMMAND_METHOD_POST, + Trigger: "postcommand", + } + if _, err := th.App.CreateCommand(postCmd); err != nil { + t.Fatal("failed to create post command") + } + + // make a direct message channel + dmChannel, response := client.CreateDirectChannel(th.BasicUser.Id, th.BasicUser2.Id) + CheckCreatedStatus(t, response) + + // we should be able to run the slash command in the DM channel + _, resp := client.ExecuteCommandWithTeam(dmChannel.Id, team2.Id, "/postcommand") + CheckOKStatus(t, resp) + + // if the user is removed from the team, they should NOT be able to run the slash command in the DM channel + if success, _ := client.RemoveTeamMember(team2.Id, th.BasicUser.Id); !success { + t.Fatal("Failed to remove user from team") + } + _, resp = client.ExecuteCommandWithTeam(dmChannel.Id, team2.Id, "/postcommand") + CheckForbiddenStatus(t, resp) + + // if we omit the team id from the request, the slash command will fail because this is a DM channel, and the + // team id can't be inherited from the channel + _, resp = client.ExecuteCommand(dmChannel.Id, "/postcommand") + CheckForbiddenStatus(t, resp) +} -- cgit v1.2.3-1-g7c22