diff options
| -rw-r--r-- | api4/apitestlib.go | 6 | ||||
| -rw-r--r-- | api4/command.go | 18 | ||||
| -rw-r--r-- | api4/command_test.go | 131 | ||||
| -rw-r--r-- | model/client4.go | 18 |
4 files changed, 165 insertions, 8 deletions
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) +} diff --git a/model/client4.go b/model/client4.go index 941145043..5703c4143 100644 --- a/model/client4.go +++ b/model/client4.go @@ -2816,7 +2816,7 @@ func (c *Client4) ListCommands(teamId string, customOnly bool) ([]*Command, *Res } } -// ExecuteCommand executes a given command. +// ExecuteCommand executes a given slash command. func (c *Client4) ExecuteCommand(channelId, command string) (*CommandResponse, *Response) { commandArgs := &CommandArgs{ ChannelId: channelId, @@ -2830,6 +2830,22 @@ func (c *Client4) ExecuteCommand(channelId, command string) (*CommandResponse, * } } +// ExecuteCommand executes a given slash command against the specified team +// Use this when executing slash commands in a DM/GM, since the team id cannot be inferred in that case +func (c *Client4) ExecuteCommandWithTeam(channelId, teamId, command string) (*CommandResponse, *Response) { + commandArgs := &CommandArgs{ + ChannelId: channelId, + TeamId: teamId, + Command: command, + } + if r, err := c.DoApiPost(c.GetCommandsRoute()+"/execute", commandArgs.ToJson()); err != nil { + return nil, BuildErrorResponse(r, err) + } else { + defer closeBody(r) + return CommandResponseFromJson(r.Body), BuildResponse(r) + } +} + // ListCommands will retrieve a list of commands available in the team. func (c *Client4) ListAutocompleteCommands(teamId string) ([]*Command, *Response) { if r, err := c.DoApiGet(c.GetTeamAutoCompleteCommandsRoute(teamId), ""); err != nil { |