From 1bd6d9a90b1ad3076fe1d9fa580e30fbe1c66721 Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Mon, 8 Jul 2019 13:50:55 +0200 Subject: [PATCH] MM-10982 Support relative links in interactive message buttons (#11477) * Add support for relative callback URLs to plugins * Update relative URL check and make it more robust * Add leading slash to input relative path --- app/integration_action.go | 18 ++- app/integration_action_test.go | 229 +++++++++++++++++++++++++++++++++ 2 files changed, 244 insertions(+), 3 deletions(-) diff --git a/app/integration_action.go b/app/integration_action.go index ad5473ef8a..98309bfe34 100644 --- a/app/integration_action.go +++ b/app/integration_action.go @@ -207,6 +207,20 @@ func (a *App) DoPostActionWithCookie(postId, actionId, userId, selectedOption st // Perform an HTTP POST request to an integration's action endpoint. // Caller must consume and close returned http.Response as necessary. func (a *App) DoActionRequest(rawURL string, body []byte) (*http.Response, *model.AppError) { + inURL, err := url.Parse(rawURL) + if err != nil { + return nil, model.NewAppError("DoActionRequest", "api.post.do_action.action_integration.app_error", nil, err.Error(), http.StatusBadRequest) + } + + siteURL, _ := url.Parse(*a.Config().ServiceSettings.SiteURL) + rawURLPath := path.Clean(rawURL) + if siteURL != nil && (strings.HasPrefix(rawURLPath, "/plugins/") || strings.HasPrefix(rawURLPath, "plugins/")) { + inURL.Scheme = siteURL.Scheme + inURL.Host = siteURL.Host + inURL.Path = path.Join("/", siteURL.Path, rawURLPath) + rawURL = inURL.String() + } + req, err := http.NewRequest("POST", rawURL, bytes.NewReader(body)) if err != nil { return nil, model.NewAppError("DoActionRequest", "api.post.do_action.action_integration.app_error", nil, err.Error(), http.StatusBadRequest) @@ -216,10 +230,8 @@ func (a *App) DoActionRequest(rawURL string, body []byte) (*http.Response, *mode // Allow access to plugin routes for action buttons var httpClient *http.Client - url, _ := url.Parse(rawURL) - siteURL, _ := url.Parse(*a.Config().ServiceSettings.SiteURL) subpath, _ := utils.GetSubpathFromConfig(a.Config()) - if (url.Hostname() == "localhost" || url.Hostname() == "127.0.0.1" || url.Hostname() == siteURL.Hostname()) && strings.HasPrefix(url.Path, path.Join(subpath, "plugins")) { + if (inURL.Hostname() == "localhost" || inURL.Hostname() == "127.0.0.1" || inURL.Hostname() == siteURL.Hostname()) && strings.HasPrefix(inURL.Path, path.Join(subpath, "plugins")) { req.Header.Set(model.HEADER_AUTH, "Bearer "+a.Session.Token) httpClient = a.HTTPService.MakeClient(true) } else { diff --git a/app/integration_action_test.go b/app/integration_action_test.go index 06c248ed56..7ae926c4e4 100644 --- a/app/integration_action_test.go +++ b/app/integration_action_test.go @@ -64,6 +64,7 @@ func TestPostActionInvalidURL(t *testing.T) { _, err = th.App.DoPostAction(post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") require.NotNil(t, err) + require.True(t, strings.Contains(err.Error(), "missing protocol scheme")) } func TestPostAction(t *testing.T) { @@ -449,4 +450,232 @@ func TestSubmitInteractiveDialog(t *testing.T) { resp, err = th.App.SubmitInteractiveDialog(submit) assert.NotNil(t, err) assert.Nil(t, resp) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "" + *cfg.ServiceSettings.SiteURL = ts.URL + }) + + submit.URL = "/notvalid/myplugin/myaction" + resp, err = th.App.SubmitInteractiveDialog(submit) + assert.NotNil(t, err) + require.Nil(t, resp) + + submit.URL = "/plugins/myplugin/myaction" + resp, err = th.App.SubmitInteractiveDialog(submit) + assert.Nil(t, err) + require.NotNil(t, resp) + assert.Equal(t, "some error", resp.Errors["name1"]) +} + +func TestPostActionRelativeURL(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + request := model.PostActionIntegrationRequestFromJson(r.Body) + assert.NotNil(t, request) + fmt.Fprintf(w, `{"post": {"message": "updated"}, "ephemeral_text": "foo"}`) + })) + defer ts.Close() + + t.Run("invalid relative URL", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "" + *cfg.ServiceSettings.SiteURL = ts.URL + }) + + interactivePost := model.Post{ + Message: "Interactive post", + ChannelId: th.BasicChannel.Id, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + "attachments": []*model.SlackAttachment{ + { + Text: "hello", + Actions: []*model.PostAction{ + { + Integration: &model.PostActionIntegration{ + URL: "/notaplugin/some/path", + }, + Name: "action", + Type: "some_type", + }, + }, + }, + }, + }, + } + + post, err := th.App.CreatePostAsUser(&interactivePost, "") + require.Nil(t, err) + attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + require.True(t, ok) + require.NotEmpty(t, attachments[0].Actions) + require.NotEmpty(t, attachments[0].Actions[0].Id) + + _, err = th.App.DoPostAction(post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + require.NotNil(t, err) + }) + + t.Run("valid relative URL without SiteURL set", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "" + *cfg.ServiceSettings.SiteURL = "" + }) + + interactivePost := model.Post{ + Message: "Interactive post", + ChannelId: th.BasicChannel.Id, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + "attachments": []*model.SlackAttachment{ + { + Text: "hello", + Actions: []*model.PostAction{ + { + Integration: &model.PostActionIntegration{ + URL: "/plugins/myplugin/myaction", + }, + Name: "action", + Type: "some_type", + }, + }, + }, + }, + }, + } + + post, err := th.App.CreatePostAsUser(&interactivePost, "") + require.Nil(t, err) + attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + require.True(t, ok) + require.NotEmpty(t, attachments[0].Actions) + require.NotEmpty(t, attachments[0].Actions[0].Id) + + _, err = th.App.DoPostAction(post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + require.NotNil(t, err) + }) + + t.Run("valid relative URL with SiteURL set", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "" + *cfg.ServiceSettings.SiteURL = ts.URL + }) + + interactivePost := model.Post{ + Message: "Interactive post", + ChannelId: th.BasicChannel.Id, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + "attachments": []*model.SlackAttachment{ + { + Text: "hello", + Actions: []*model.PostAction{ + { + Integration: &model.PostActionIntegration{ + URL: "/plugins/myplugin/myaction", + }, + Name: "action", + Type: "some_type", + }, + }, + }, + }, + }, + } + + post, err := th.App.CreatePostAsUser(&interactivePost, "") + require.Nil(t, err) + attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + require.True(t, ok) + require.NotEmpty(t, attachments[0].Actions) + require.NotEmpty(t, attachments[0].Actions[0].Id) + + _, err = th.App.DoPostAction(post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + require.Nil(t, err) + }) + + t.Run("valid (but dirty) relative URL with SiteURL set", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "" + *cfg.ServiceSettings.SiteURL = ts.URL + }) + + interactivePost := model.Post{ + Message: "Interactive post", + ChannelId: th.BasicChannel.Id, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + "attachments": []*model.SlackAttachment{ + { + Text: "hello", + Actions: []*model.PostAction{ + { + Integration: &model.PostActionIntegration{ + URL: "//plugins/myplugin///myaction", + }, + Name: "action", + Type: "some_type", + }, + }, + }, + }, + }, + } + + post, err := th.App.CreatePostAsUser(&interactivePost, "") + require.Nil(t, err) + attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + require.True(t, ok) + require.NotEmpty(t, attachments[0].Actions) + require.NotEmpty(t, attachments[0].Actions[0].Id) + + _, err = th.App.DoPostAction(post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + require.Nil(t, err) + }) + + t.Run("valid relative URL with SiteURL set and no leading slash", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "" + *cfg.ServiceSettings.SiteURL = ts.URL + }) + + interactivePost := model.Post{ + Message: "Interactive post", + ChannelId: th.BasicChannel.Id, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + "attachments": []*model.SlackAttachment{ + { + Text: "hello", + Actions: []*model.PostAction{ + { + Integration: &model.PostActionIntegration{ + URL: "plugins/myplugin/myaction", + }, + Name: "action", + Type: "some_type", + }, + }, + }, + }, + }, + } + + post, err := th.App.CreatePostAsUser(&interactivePost, "") + require.Nil(t, err) + attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + require.True(t, ok) + require.NotEmpty(t, attachments[0].Actions) + require.NotEmpty(t, attachments[0].Actions[0].Id) + + _, err = th.App.DoPostAction(post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + require.Nil(t, err) + }) }