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
2019-07-08 13:50:55 +02:00
--- 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 {
--- 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)
+	})
 }