MM-19250 - new endpoint to retrieve command by id (#13598)

* MM-19250 add endpoint to retrieve command by id

* endpoint
* client
* unit tests

* MM-19250 update comment; remove redundant unit test

* MM-19250 rename GetCommand to GetCommandById

* MM-19250 don't filter on autocomplete flag

* MM-19250: require team_id when using GetCommandById

* team_id added to endpoint query string for GET
* unit test to check for mismatch teamid param and command teamid

* Revert "MM-19250: require team_id when using GetCommandById"

This reverts commit ed78e2796426f75bc23d0f16064be95fa37305fd.

* MM-19250 don't leak existence of id when user doesn't have perms

* return 404 not_found when id not found
* return 404 not_found when id exists but user missing perms to view team
* return 404 not_found when id exists but user missing perms to manage commands

* MM-19250 fix typos in comments

Co-authored-by: mattermod <mattermod@users.noreply.github.com>

api4/command.go

@@ -16,6 +16,7 @@ func (api *API) InitCommand() {
 	api.BaseRoutes.Commands.Handle("", api.ApiSessionRequired(listCommands)).Methods("GET")
 	api.BaseRoutes.Commands.Handle("/execute", api.ApiSessionRequired(executeCommand)).Methods("POST")
 
+	api.BaseRoutes.Command.Handle("", api.ApiSessionRequired(getCommand)).Methods("GET")
 	api.BaseRoutes.Command.Handle("", api.ApiSessionRequired(updateCommand)).Methods("PUT")
 	api.BaseRoutes.Command.Handle("", api.ApiSessionRequired(deleteCommand)).Methods("DELETE")
 
@@ -142,7 +143,6 @@ func listCommands(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 
 	teamId := r.URL.Query().Get("team_id")
-
 	if len(teamId) == 0 {
 		c.SetInvalidParam("team_id")
 		return
@@ -185,6 +185,35 @@ func listCommands(c *Context, w http.ResponseWriter, r *http.Request) {
 	w.Write([]byte(model.CommandListToJson(commands)))
 }
 
+func getCommand(c *Context, w http.ResponseWriter, r *http.Request) {
+	c.RequireCommandId()
+	if c.Err != nil {
+		return
+	}
+
+	cmd, err := c.App.GetCommand(c.Params.CommandId)
+	if err != nil {
+		c.SetCommandNotFoundError()
+		return
+	}
+
+	// check for permissions to view this command; must have perms to view team and
+	// PERMISSION_MANAGE_SLASH_COMMANDS for the team the command belongs to.
+
+	if !c.App.SessionHasPermissionToTeam(c.App.Session, cmd.TeamId, model.PERMISSION_VIEW_TEAM) {
+		// here we return Not_found instead of a permissions error so we don't leak the existence of
+		// a command to someone without permissions for the team it belongs to.
+		c.SetCommandNotFoundError()
+		return
+	}
+	if !c.App.SessionHasPermissionToTeam(c.App.Session, cmd.TeamId, model.PERMISSION_MANAGE_SLASH_COMMANDS) {
+		// again, return not_found to ensure id existence does not leak.
+		c.SetCommandNotFoundError()
+		return
+	}
+	w.Write([]byte(cmd.ToJson()))
+}
+
 func executeCommand(c *Context, w http.ResponseWriter, r *http.Request) {
 	commandArgs := model.CommandArgsFromJson(r.Body)
 	if commandArgs == nil {

api4/command_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -348,6 +349,65 @@ func TestListAutocompleteCommands(t *testing.T) {
 	})
 }
 
+func TestGetCommand(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+	Client := th.Client
+
+	enableCommands := *th.App.Config().ServiceSettings.EnableCommands
+	defer func() {
+		th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands })
+	}()
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true })
+
+	newCmd := &model.Command{
+		CreatorId: th.BasicUser.Id,
+		TeamId:    th.BasicTeam.Id,
+		URL:       "http://nowhere.com",
+		Method:    model.COMMAND_METHOD_POST,
+		Trigger:   "roger"}
+
+	newCmd, resp := th.SystemAdminClient.CreateCommand(newCmd)
+	CheckNoError(t, resp)
+
+	t.Run("ValidId", func(t *testing.T) {
+		cmd, resp := th.SystemAdminClient.GetCommandById(newCmd.Id)
+		CheckNoError(t, resp)
+
+		require.Equal(t, newCmd.Id, cmd.Id)
+		require.Equal(t, newCmd.CreatorId, cmd.CreatorId)
+		require.Equal(t, newCmd.TeamId, cmd.TeamId)
+		require.Equal(t, newCmd.URL, cmd.URL)
+		require.Equal(t, newCmd.Method, cmd.Method)
+		require.Equal(t, newCmd.Trigger, cmd.Trigger)
+	})
+
+	t.Run("InvalidId", func(t *testing.T) {
+		_, resp := th.SystemAdminClient.GetCommandById(strings.Repeat("z", len(newCmd.Id)))
+		require.Error(t, resp.Error)
+	})
+
+	t.Run("UserWithNoPermissionForCustomCommands", func(t *testing.T) {
+		_, resp := Client.GetCommandById(newCmd.Id)
+		CheckNotFoundStatus(t, resp)
+	})
+
+	t.Run("NoMember", func(t *testing.T) {
+		Client.Logout()
+		user := th.CreateUser()
+		th.SystemAdminClient.RemoveTeamMember(th.BasicTeam.Id, user.Id)
+		Client.Login(user.Email, user.Password)
+		_, resp := Client.GetCommandById(newCmd.Id)
+		CheckNotFoundStatus(t, resp)
+	})
+
+	t.Run("NotLoggedIn", func(t *testing.T) {
+		Client.Logout()
+		_, resp := Client.GetCommandById(newCmd.Id)
+		CheckUnauthorizedStatus(t, resp)
+	})
+}
+
 func TestRegenToken(t *testing.T) {
 	th := Setup().InitBasic()
 	defer th.TearDown()

model/client4.go

@@ -4015,6 +4015,17 @@ func (c *Client4) ListCommands(teamId string, customOnly bool) ([]*Command, *Res
 	return CommandListFromJson(r.Body), BuildResponse(r)
 }
 
+// GetCommandById will retrieve a command by id.
+func (c *Client4) GetCommandById(cmdId string) (*Command, *Response) {
+	url := fmt.Sprintf("%s/%s", c.GetCommandsRoute(), cmdId)
+	r, err := c.DoApiGet(url, "")
+	if err != nil {
+		return nil, BuildErrorResponse(r, err)
+	}
+	defer closeBody(r)
+	return CommandFromJson(r.Body), BuildResponse(r)
+}
+
 // ExecuteCommand executes a given slash command.
 func (c *Client4) ExecuteCommand(channelId, command string) (*CommandResponse, *Response) {
 	commandArgs := &CommandArgs{

web/context.go

@@ -169,6 +169,10 @@ func (c *Context) SetServerBusyError() {
 	c.Err = NewServerBusyError()
 }
 
+func (c *Context) SetCommandNotFoundError() {
+	c.Err = model.NewAppError("GetCommand", "store.sql_command.save.get.app_error", nil, "", http.StatusNotFound)
+}
+
 func (c *Context) HandleEtag(etag string, routeName string, w http.ResponseWriter, r *http.Request) bool {
 	metrics := c.App.Metrics
 	if et := r.Header.Get(model.HEADER_ETAG_CLIENT); len(etag) > 0 {