From ba34b4607ce297c562cb14dca705a9914437478e Mon Sep 17 00:00:00 2001 From: happygaijin Date: Fri, 5 Apr 2019 07:35:51 -0700 Subject: [PATCH] MM-14575 - Automatically serve static files for plugins (#10476) * MM-14575 - Automatically serve static files for plugins * Added static handler for plugin public files * Added StaticFilesPath method to Environment for use by MainRouter * Added "static_files" property to Manifest Server * Added unit tests for these changes * MM-14575: Adding comment for cache control value * MM-14575: Moved Static Plugin Request handler to plugin_requests * Updated testing * MM-14575: Removing the StaticFiles from Manifest Server * MM-14575: Removing static files from test * MM-14575: Updating static files test * MM14575: Removing cache directive from plugin static files * MM14575: Moving plugin public directory to root * MM-14575: Updating tests for changed public directory * MM-14575: Moved compileGo to a common utils package for tests * MM-14575: Moving plugins initialization to InitPlugins find in tests * Update utils/test_files_compiler.go Adding Copyright header Co-Authored-By: happygaijin * MM-14575: Consistent usage of static vs public name * Removing spurious newline * Comment typo Co-Authored-By: happygaijin * Removing spurious new line Co-Authored-By: happygaijin * MM14575: Adding a test to make sure only public files can be requested * MM-14575 Adding a test for redirects on public files --- app/plugin_api_test.go | 41 ++++++++++++-- app/plugin_hooks_test.go | 16 +----- app/plugin_requests.go | 30 ++++++++++ app/server_app_adapters.go | 6 +- plugin/environment.go | 9 +++ plugin/supervisor_test.go | 16 +----- utils/test_files_compiler.go | 26 +++++++++ web/web_test.go | 107 ++++++++++++++++++++++++++++++++++- 8 files changed, 216 insertions(+), 35 deletions(-) create mode 100644 utils/test_files_compiler.go diff --git a/app/plugin_api_test.go b/app/plugin_api_test.go index dcc30ec0a9..f487691df6 100644 --- a/app/plugin_api_test.go +++ b/app/plugin_api_test.go @@ -8,7 +8,6 @@ import ( "encoding/json" "errors" "fmt" - "github.com/mattermost/mattermost-server/utils/fileutils" "image" "image/color" "image/png" @@ -21,13 +20,16 @@ import ( "text/template" "time" + "github.com/mattermost/mattermost-server/utils/fileutils" + "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" + "github.com/mattermost/mattermost-server/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func setupPluginApiTest(t *testing.T, pluginCode string, pluginManifest string, pluginId string, app *App) { +func setupPluginApiTest(t *testing.T, pluginCode string, pluginManifest string, pluginId string, app *App) string { pluginDir, err := ioutil.TempDir("", "") require.NoError(t, err) webappPluginDir, err := ioutil.TempDir("", "") @@ -39,7 +41,7 @@ func setupPluginApiTest(t *testing.T, pluginCode string, pluginManifest string, require.NoError(t, err) backend := filepath.Join(pluginDir, pluginId, "backend.exe") - compileGo(t, pluginCode, backend) + utils.CompileGo(t, pluginCode, backend) ioutil.WriteFile(filepath.Join(pluginDir, pluginId, "plugin.json"), []byte(pluginManifest), 0600) manifest, activated, reterr := env.Activate(pluginId) @@ -48,8 +50,39 @@ func setupPluginApiTest(t *testing.T, pluginCode string, pluginManifest string, require.True(t, activated) app.SetPluginsEnvironment(env) + + return pluginDir } +func TestPublicFilesPathConfiguration(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + pluginID := "com.mattermost.sample" + + pluginDir := setupPluginApiTest(t, + ` + package main + + import ( + "github.com/mattermost/mattermost-server/plugin" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `, + `{"id": "com.mattermost.sample", "server": {"executable": "backend.exe"}, "settings_schema": {"settings": []}}`, pluginID, th.App) + + publicFilesFolderInTest := filepath.Join(pluginDir, pluginID, "public") + publicFilesPath, err := th.App.GetPluginsEnvironment().PublicFilesPath(pluginID) + assert.NoError(t, err) + assert.Equal(t, publicFilesPath, publicFilesFolderInTest) +} func TestPluginAPIGetUsers(t *testing.T) { th := Setup(t) defer th.TearDown() @@ -460,7 +493,7 @@ func TestPluginAPIGetPlugins(t *testing.T) { var pluginManifests []*model.Manifest for _, pluginID := range pluginIDs { backend := filepath.Join(pluginDir, pluginID, "backend.exe") - compileGo(t, pluginCode, backend) + utils.CompileGo(t, pluginCode, backend) ioutil.WriteFile(filepath.Join(pluginDir, pluginID, "plugin.json"), []byte(fmt.Sprintf(`{"id": "%s", "server": {"executable": "backend.exe"}}`, pluginID)), 0600) manifest, activated, reterr := env.Activate(pluginID) diff --git a/app/plugin_hooks_test.go b/app/plugin_hooks_test.go index 838162611e..a6f8da8bd9 100644 --- a/app/plugin_hooks_test.go +++ b/app/plugin_hooks_test.go @@ -10,7 +10,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "path/filepath" "strings" "testing" @@ -22,22 +21,11 @@ import ( "github.com/mattermost/mattermost-server/plugin" "github.com/mattermost/mattermost-server/plugin/plugintest" "github.com/mattermost/mattermost-server/plugin/plugintest/mock" + "github.com/mattermost/mattermost-server/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func compileGo(t *testing.T, sourceCode, outputPath string) { - dir, err := ioutil.TempDir(".", "") - require.NoError(t, err) - defer os.RemoveAll(dir) - require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "main.go"), []byte(sourceCode), 0600)) - cmd := exec.Command("go", "build", "-o", outputPath, "main.go") - cmd.Dir = dir - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - require.NoError(t, cmd.Run(), "failed to compile go") -} - func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, apiFunc func(*model.Manifest) plugin.API) (func(), []string, []error) { pluginDir, err := ioutil.TempDir("", "") require.NoError(t, err) @@ -53,7 +41,7 @@ func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, a for _, code := range pluginCode { pluginId := model.NewId() backend := filepath.Join(pluginDir, pluginId, "backend.exe") - compileGo(t, code, backend) + utils.CompileGo(t, code, backend) ioutil.WriteFile(filepath.Join(pluginDir, pluginId, "plugin.json"), []byte(`{"id": "`+pluginId+`", "backend": {"executable": "backend.exe"}}`), 0600) _, _, activationErr := env.Activate(pluginId) diff --git a/app/plugin_requests.go b/app/plugin_requests.go index f7ba521e96..99425706cf 100644 --- a/app/plugin_requests.go +++ b/app/plugin_requests.go @@ -4,8 +4,10 @@ package app import ( + "fmt" "net/http" "path" + "path/filepath" "strings" "bytes" @@ -40,6 +42,34 @@ func (a *App) ServePluginRequest(w http.ResponseWriter, r *http.Request) { a.servePluginRequest(w, r, hooks.ServeHTTP) } +// ServePluginPublicRequest serves public plugin files +// at the URL http(s)://$SITE_URL/plugins/$PLUGIN_ID/public/{anything} +func (a *App) ServePluginPublicRequest(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/") { + http.NotFound(w, r) + return + } + + // Should be in the form of /$PLUGIN_ID/public/{anything} by the time we get here + vars := mux.Vars(r) + pluginID := vars["plugin_id"] + + publicFilesPath, err := a.GetPluginsEnvironment().PublicFilesPath(pluginID) + if err != nil { + http.NotFound(w, r) + return + } + + publicFilePath := path.Clean(r.URL.Path) + prefix := fmt.Sprintf("/plugins/%s/public/", pluginID) + if !strings.HasPrefix(publicFilePath, prefix) { + http.NotFound(w, r) + return + } + publicFile := filepath.Join(publicFilesPath, strings.TrimPrefix(publicFilePath, prefix)) + http.ServeFile(w, r, publicFile) +} + func (a *App) servePluginRequest(w http.ResponseWriter, r *http.Request, handler func(*plugin.Context, http.ResponseWriter, *http.Request)) { token := "" context := &plugin.Context{ diff --git a/app/server_app_adapters.go b/app/server_app_adapters.go index c721f3bc8d..89ddbd8516 100644 --- a/app/server_app_adapters.go +++ b/app/server_app_adapters.go @@ -100,8 +100,10 @@ func (s *Server) RunOldAppInitalization() error { return errors.Wrap(err, "failed to parse SiteURL subpath") } s.FakeApp().Srv.Router = s.FakeApp().Srv.RootRouter.PathPrefix(subpath).Subrouter() - s.FakeApp().Srv.Router.HandleFunc("/plugins/{plugin_id:[A-Za-z0-9\\_\\-\\.]+}", s.FakeApp().ServePluginRequest) - s.FakeApp().Srv.Router.HandleFunc("/plugins/{plugin_id:[A-Za-z0-9\\_\\-\\.]+}/{anything:.*}", s.FakeApp().ServePluginRequest) + pluginsRoute := s.FakeApp().Srv.Router.PathPrefix("/plugins/{plugin_id:[A-Za-z0-9\\_\\-\\.]+}").Subrouter() + pluginsRoute.HandleFunc("", s.FakeApp().ServePluginRequest) + pluginsRoute.HandleFunc("/public/{public_file:.*}", s.FakeApp().ServePluginPublicRequest) + pluginsRoute.HandleFunc("/{anything:.*}", s.FakeApp().ServePluginRequest) // If configured with a subpath, redirect 404s at the root back into the subpath. if subpath != "/" { diff --git a/plugin/environment.go b/plugin/environment.go index ddf042f816..fdcbb227a3 100644 --- a/plugin/environment.go +++ b/plugin/environment.go @@ -94,6 +94,15 @@ func (env *Environment) IsActive(id string) bool { return ok } +// PublicFilesPath returns a path and true if the plugin with the given id is active. +// It returns an empty string and false if the path is not set or invalid +func (env *Environment) PublicFilesPath(id string) (string, error) { + if _, ok := env.activePlugins.Load(id); !ok { + return "", fmt.Errorf("plugin not found: %v", id) + } + return filepath.Join(env.pluginDir, id, "public"), nil +} + // Statuses returns a list of plugin statuses representing the state of every plugin func (env *Environment) Statuses() (model.PluginStatuses, error) { plugins, err := env.Available() diff --git a/plugin/supervisor_test.go b/plugin/supervisor_test.go index 4c5275c433..8f9647d64f 100644 --- a/plugin/supervisor_test.go +++ b/plugin/supervisor_test.go @@ -6,12 +6,12 @@ package plugin import ( "io/ioutil" "os" - "os/exec" "path/filepath" "testing" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -26,18 +26,6 @@ func TestSupervisor(t *testing.T) { } } -func compileGo(t *testing.T, sourceCode, outputPath string) { - dir, err := ioutil.TempDir(".", "") - require.NoError(t, err) - defer os.RemoveAll(dir) - require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "main.go"), []byte(sourceCode), 0600)) - cmd := exec.Command("go", "build", "-o", outputPath, "main.go") - cmd.Dir = dir - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - require.NoError(t, cmd.Run()) -} - func testSupervisor_InvalidExecutablePath(t *testing.T) { dir, err := ioutil.TempDir("", "") require.NoError(t, err) @@ -83,7 +71,7 @@ func testSupervisor_StartTimeout(t *testing.T) { defer os.RemoveAll(dir) backend := filepath.Join(dir, "backend.exe") - compileGo(t, ` + utils.CompileGo(t, ` package main func main() { diff --git a/utils/test_files_compiler.go b/utils/test_files_compiler.go new file mode 100644 index 0000000000..7793966bcc --- /dev/null +++ b/utils/test_files_compiler.go @@ -0,0 +1,26 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package utils + +import ( + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func CompileGo(t *testing.T, sourceCode, outputPath string) { + dir, err := ioutil.TempDir(".", "") + require.NoError(t, err) + defer os.RemoveAll(dir) + require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "main.go"), []byte(sourceCode), 0600)) + cmd := exec.Command("go", "build", "-o", outputPath, "main.go") + cmd.Dir = dir + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + require.NoError(t, cmd.Run(), "failed to compile go") +} diff --git a/web/web_test.go b/web/web_test.go index 1bb6b9ec82..22f9826a30 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -5,11 +5,22 @@ package web import ( "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "os" + "path/filepath" "testing" + "github.com/mattermost/mattermost-server/testlib" + "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/config" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/plugin" + "github.com/mattermost/mattermost-server/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ApiClient *model.Client4 @@ -18,12 +29,15 @@ var URL string type TestHelper struct { App *app.App Server *app.Server + Web *Web BasicUser *model.User BasicChannel *model.Channel BasicTeam *model.Team SystemAdminUser *model.User + + tempWorkspace string } func Setup() *TestHelper { @@ -52,7 +66,7 @@ func Setup() *TestHelper { } a.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ListenAddress = prevListenAddress }) - New(s, s.AppOptions, s.Router) + web := New(s, s.AppOptions, s.Router) URL = fmt.Sprintf("http://localhost:%v", a.Srv.ListenAddr.Port) ApiClient = model.NewAPIv4Client(URL) @@ -68,11 +82,26 @@ func Setup() *TestHelper { th := &TestHelper{ App: a, Server: s, + Web: web, } return th } +func (th *TestHelper) InitPlugins() *TestHelper { + + if th.tempWorkspace == "" { + th.tempWorkspace, _ = testlib.SetupTestResources() + } + + pluginDir := filepath.Join(th.tempWorkspace, "plugins") + webappDir := filepath.Join(th.tempWorkspace, "webapp") + + th.App.InitPlugins(pluginDir, webappDir) + + return th +} + func (th *TestHelper) InitBasic() *TestHelper { th.SystemAdminUser, _ = th.App.CreateUser(&model.User{Email: model.NewId() + "success+test@simulator.amazonses.com", Nickname: "Corey Hulen", Password: "passwd1", EmailVerified: true, Roles: model.SYSTEM_ADMIN_ROLE_ID}) @@ -98,6 +127,82 @@ func (th *TestHelper) TearDown() { } } +func TestPublicFilesRequest(t *testing.T) { + th := Setup().InitPlugins() + defer th.TearDown() + + pluginDir, err := ioutil.TempDir("", "") + require.NoError(t, err) + webappPluginDir, err := ioutil.TempDir("", "") + require.NoError(t, err) + defer os.RemoveAll(pluginDir) + defer os.RemoveAll(webappPluginDir) + + env, err := plugin.NewEnvironment(th.App.NewPluginAPI, pluginDir, webappPluginDir, th.App.Log) + require.NoError(t, err) + + pluginID := "com.mattermost.sample" + pluginCode := + ` + package main + + import ( + "github.com/mattermost/mattermost-server/plugin" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + + ` + // Compile and write the plugin + backend := filepath.Join(pluginDir, pluginID, "backend.exe") + utils.CompileGo(t, pluginCode, backend) + + // Write the plugin.json manifest + pluginManifest := `{"id": "com.mattermost.sample", "server": {"executable": "backend.exe"}, "settings_schema": {"settings": []}}` + ioutil.WriteFile(filepath.Join(pluginDir, pluginID, "plugin.json"), []byte(pluginManifest), 0600) + + // Write the test public file + helloHTML := `Hello from the static files public folder for the com.mattermost.sample plugin!` + htmlFolderPath := filepath.Join(pluginDir, pluginID, "public") + os.MkdirAll(htmlFolderPath, os.ModePerm) + htmlFilePath := filepath.Join(htmlFolderPath, "hello.html") + + htmlFileErr := ioutil.WriteFile(htmlFilePath, []byte(helloHTML), 0600) + assert.NoError(t, htmlFileErr) + + nefariousHTML := `You shouldn't be able to get here!` + htmlFileErr = ioutil.WriteFile(filepath.Join(pluginDir, pluginID, "nefarious-file-access.html"), []byte(nefariousHTML), 0600) + assert.NoError(t, htmlFileErr) + + manifest, activated, reterr := env.Activate(pluginID) + require.Nil(t, reterr) + require.NotNil(t, manifest) + require.True(t, activated) + + th.App.SetPluginsEnvironment(env) + + req, _ := http.NewRequest("GET", "/plugins/com.mattermost.sample/public/hello.html", nil) + res := httptest.NewRecorder() + th.Web.MainRouter.ServeHTTP(res, req) + assert.Equal(t, helloHTML, res.Body.String()) + + req, _ = http.NewRequest("GET", "/plugins/com.mattermost.sample/nefarious-file-access.html", nil) + res = httptest.NewRecorder() + th.Web.MainRouter.ServeHTTP(res, req) + assert.Equal(t, 404, res.Code) + + req, _ = http.NewRequest("GET", "/plugins/com.mattermost.sample/public/../nefarious-file-access.html", nil) + res = httptest.NewRecorder() + th.Web.MainRouter.ServeHTTP(res, req) + assert.Equal(t, 301, res.Code) +} + /* Test disabled for now so we don't requrie the client to build. Maybe re-enable after client gets moved out. func TestStatic(t *testing.T) { Setup()