diff --git a/api4/plugin.go b/api4/plugin.go index 4ce44f9746..1f045afc71 100644 --- a/api4/plugin.go +++ b/api4/plugin.go @@ -10,6 +10,7 @@ import ( "io/ioutil" "net/http" "net/url" + "time" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" @@ -17,6 +18,9 @@ import ( const ( MAXIMUM_PLUGIN_FILE_SIZE = 50 * 1024 * 1024 + // INSTALL_PLUGIN_FROM_URL_HTTP_REQUEST_TIMEOUT defines a high timeout for installing plugins + // from an external URL to avoid slow connections or large plugins from failing to install. + INSTALL_PLUGIN_FROM_URL_HTTP_REQUEST_TIMEOUT = 60 * time.Minute ) func (api *API) InitPlugin() { @@ -115,6 +119,8 @@ func installPluginFromUrl(c *Context, w http.ResponseWriter, r *http.Request) { } client := c.App.HTTPService.MakeClient(true) + client.Timeout = INSTALL_PLUGIN_FROM_URL_HTTP_REQUEST_TIMEOUT + resp, err := client.Get(downloadUrl) if err != nil { c.Err = model.NewAppError("installPluginFromUrl", "api.plugin.install.download_failed.app_error", nil, err.Error(), http.StatusBadRequest) diff --git a/api4/plugin_test.go b/api4/plugin_test.go index 482ad32f6f..fef1ac3918 100644 --- a/api4/plugin_test.go +++ b/api4/plugin_test.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/testlib" @@ -59,6 +60,24 @@ func TestPlugin(t *testing.T) { CheckNoError(t, resp) assert.Equal(t, "testplugin", manifest.Id) + t.Run("install plugin from URL with slow response time", func(t *testing.T) { + if testing.Short() { + t.Skip("skipping test to install plugin from a slow response server") + } + + // Install from URL - slow server to simulate longer bundle download times + slowTestServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + time.Sleep(60 * time.Second) // Wait longer than the previous default 30 seconds timeout + res.WriteHeader(http.StatusOK) + res.Write(tarData) + })) + defer func() { slowTestServer.Close() }() + + manifest, resp = th.SystemAdminClient.InstallPluginFromUrl(slowTestServer.URL, true) + CheckNoError(t, resp) + assert.Equal(t, "testplugin", manifest.Id) + }) + // Stored in File Store: Install Plugin from URL case pluginStored, err := th.App.FileExists("./plugins/" + manifest.Id + ".tar.gz") assert.Nil(t, err) @@ -255,6 +274,8 @@ func TestNotifyClusterPluginEvent(t *testing.T) { t.Fatal(err) } + testCluster.ClearMessages() + // Successful upload manifest, resp := th.SystemAdminClient.UploadPlugin(bytes.NewReader(tarData)) CheckNoError(t, resp) @@ -266,6 +287,7 @@ func TestNotifyClusterPluginEvent(t *testing.T) { require.Nil(t, err) require.True(t, pluginStored) + messages := testCluster.GetMessages() expectedPluginData := model.PluginEventData{ Id: manifest.Id, } @@ -275,30 +297,141 @@ func TestNotifyClusterPluginEvent(t *testing.T) { WaitForAllToSend: true, Data: expectedPluginData.ToJson(), } - expectedMessages := findClusterMessages(model.CLUSTER_EVENT_INSTALL_PLUGIN, testCluster.GetMessages()) - require.Equal(t, []*model.ClusterMessage{expectedInstallMessage}, expectedMessages) + actualMessages := findClusterMessages(model.CLUSTER_EVENT_INSTALL_PLUGIN, messages) + require.Equal(t, []*model.ClusterMessage{expectedInstallMessage}, actualMessages) + + // Upgrade + testCluster.ClearMessages() + manifest, resp = th.SystemAdminClient.UploadPluginForced(bytes.NewReader(tarData)) + CheckNoError(t, resp) + require.Equal(t, "testplugin", manifest.Id) // Successful remove testCluster.ClearMessages() - ok, resp := th.SystemAdminClient.RemovePlugin(manifest.Id) CheckNoError(t, resp) require.True(t, ok) + messages = testCluster.GetMessages() + expectedRemoveMessage := &model.ClusterMessage{ Event: model.CLUSTER_EVENT_REMOVE_PLUGIN, SendType: model.CLUSTER_SEND_RELIABLE, WaitForAllToSend: true, Data: expectedPluginData.ToJson(), } - expectedMessages = findClusterMessages(model.CLUSTER_EVENT_REMOVE_PLUGIN, testCluster.GetMessages()) - require.Equal(t, []*model.ClusterMessage{expectedRemoveMessage}, expectedMessages) + actualMessages = findClusterMessages(model.CLUSTER_EVENT_REMOVE_PLUGIN, messages) + require.Equal(t, []*model.ClusterMessage{expectedRemoveMessage}, actualMessages) pluginStored, err = th.App.FileExists(expectedPath) require.Nil(t, err) require.False(t, pluginStored) } +func TestDisableOnRemove(t *testing.T) { + path, _ := fileutils.FindDir("tests") + tarData, err := ioutil.ReadFile(filepath.Join(path, "testplugin.tar.gz")) + if err != nil { + t.Fatal(err) + } + + testCases := []struct { + Description string + Upgrade bool + }{ + { + "Remove without upgrading", + false, + }, + { + "Remove after upgrading", + true, + }, + } + + for _, tc := range testCases { + t.Run(tc.Description, func(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.Enable = true + *cfg.PluginSettings.EnableUploads = true + }) + + // Upload + manifest, resp := th.SystemAdminClient.UploadPlugin(bytes.NewReader(tarData)) + CheckNoError(t, resp) + require.Equal(t, "testplugin", manifest.Id) + + // Check initial status + pluginsResp, resp := th.SystemAdminClient.GetPlugins() + CheckNoError(t, resp) + require.Len(t, pluginsResp.Active, 0) + require.Equal(t, pluginsResp.Inactive, []*model.PluginInfo{&model.PluginInfo{ + Manifest: *manifest, + }}) + + // Enable plugin + ok, resp := th.SystemAdminClient.EnablePlugin(manifest.Id) + CheckNoError(t, resp) + require.True(t, ok) + + // Confirm enabled status + pluginsResp, resp = th.SystemAdminClient.GetPlugins() + CheckNoError(t, resp) + require.Len(t, pluginsResp.Inactive, 0) + require.Equal(t, pluginsResp.Active, []*model.PluginInfo{&model.PluginInfo{ + Manifest: *manifest, + }}) + + if tc.Upgrade { + // Upgrade + manifest, resp = th.SystemAdminClient.UploadPluginForced(bytes.NewReader(tarData)) + CheckNoError(t, resp) + require.Equal(t, "testplugin", manifest.Id) + + // Plugin should remain active + pluginsResp, resp = th.SystemAdminClient.GetPlugins() + CheckNoError(t, resp) + require.Len(t, pluginsResp.Inactive, 0) + require.Equal(t, pluginsResp.Active, []*model.PluginInfo{&model.PluginInfo{ + Manifest: *manifest, + }}) + } + + // Remove plugin + ok, resp = th.SystemAdminClient.RemovePlugin(manifest.Id) + CheckNoError(t, resp) + require.True(t, ok) + + // Plugin should have no status + pluginsResp, resp = th.SystemAdminClient.GetPlugins() + CheckNoError(t, resp) + require.Len(t, pluginsResp.Inactive, 0) + require.Len(t, pluginsResp.Active, 0) + + // Upload same plugin + manifest, resp = th.SystemAdminClient.UploadPlugin(bytes.NewReader(tarData)) + CheckNoError(t, resp) + require.Equal(t, "testplugin", manifest.Id) + + // Plugin should be inactive + pluginsResp, resp = th.SystemAdminClient.GetPlugins() + CheckNoError(t, resp) + require.Len(t, pluginsResp.Active, 0) + require.Equal(t, pluginsResp.Inactive, []*model.PluginInfo{&model.PluginInfo{ + Manifest: *manifest, + }}) + + // Clean up + ok, resp = th.SystemAdminClient.RemovePlugin(manifest.Id) + CheckNoError(t, resp) + require.True(t, ok) + }) + } +} + func findClusterMessages(event string, msgs []*model.ClusterMessage) []*model.ClusterMessage { var result []*model.ClusterMessage for _, msg := range msgs { diff --git a/api4/system.go b/api4/system.go index 00f98e7445..fba02a2e6e 100644 --- a/api4/system.go +++ b/api4/system.go @@ -8,6 +8,7 @@ import ( "fmt" "net/http" "runtime" + "time" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" @@ -64,11 +65,32 @@ func getSystemPing(c *Context, w http.ResponseWriter, r *http.Request) { if r.FormValue("get_server_status") != "" { dbStatusKey := "database_status" s[dbStatusKey] = model.STATUS_OK - _, appErr := c.App.Srv.Store.System().Get() - if appErr != nil { - mlog.Debug(fmt.Sprintf("Unable to get database status: %s", appErr.Error())) + + // Database Write/Read Check + currentTime := fmt.Sprintf("%d", time.Now().Unix()) + healthCheckKey := "health_check" + + writeErr := c.App.Srv.Store.System().SaveOrUpdate(&model.System{ + Name: healthCheckKey, + Value: currentTime, + }) + if writeErr != nil { + mlog.Debug(fmt.Sprintf("Unable to write to database: %s", writeErr.Error())) s[dbStatusKey] = model.STATUS_UNHEALTHY s[model.STATUS] = model.STATUS_UNHEALTHY + } else { + healthCheck, readErr := c.App.Srv.Store.System().GetByName(healthCheckKey) + if readErr != nil { + mlog.Debug(fmt.Sprintf("Unable to read from database: %s", readErr.Error())) + s[dbStatusKey] = model.STATUS_UNHEALTHY + s[model.STATUS] = model.STATUS_UNHEALTHY + } else if healthCheck.Value != currentTime { + mlog.Debug(fmt.Sprintf("Incorrect healthcheck value, expected %s, got %s", currentTime, healthCheck.Value)) + s[dbStatusKey] = model.STATUS_UNHEALTHY + s[model.STATUS] = model.STATUS_UNHEALTHY + } else { + mlog.Debug("Able to write/read files to database") + } } filestoreStatusKey := "filestore_status" diff --git a/app/notification.go b/app/notification.go index 8efa292156..1d4a2ccb49 100644 --- a/app/notification.go +++ b/app/notification.go @@ -640,7 +640,7 @@ type postNotification struct { func (n *postNotification) GetChannelName(userNameFormat string, excludeId string) string { switch n.channel.Type { case model.CHANNEL_DIRECT: - return n.sender.GetDisplayName(userNameFormat) + return n.sender.GetDisplayNameWithPrefix(userNameFormat, "@") case model.CHANNEL_GROUP: names := []string{} for _, user := range n.profileMap { @@ -670,7 +670,7 @@ func (n *postNotification) GetSenderName(userNameFormat string, overridesAllowed } } - return n.sender.GetDisplayName(userNameFormat) + return n.sender.GetDisplayNameWithPrefix(userNameFormat, "@") } // addMentionedUsers will add the mentioned user id in the struct's list for mentioned users diff --git a/app/notification_test.go b/app/notification_test.go index c07778896e..744b9738d4 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -1207,12 +1207,12 @@ func TestPostNotificationGetChannelName(t *testing.T) { }, "direct channel, unspecified": { channel: &model.Channel{Type: model.CHANNEL_DIRECT}, - expected: "sender", + expected: "@sender", }, "direct channel, username": { channel: &model.Channel{Type: model.CHANNEL_DIRECT}, nameFormat: model.SHOW_USERNAME, - expected: "sender", + expected: "@sender", }, "direct channel, full name": { channel: &model.Channel{Type: model.CHANNEL_DIRECT}, @@ -1290,11 +1290,11 @@ func TestPostNotificationGetSenderName(t *testing.T) { expected string }{ "name format unspecified": { - expected: sender.Username, + expected: "@" + sender.Username, }, "name format username": { nameFormat: model.SHOW_USERNAME, - expected: sender.Username, + expected: "@" + sender.Username, }, "name format full name": { nameFormat: model.SHOW_FULLNAME, @@ -1317,12 +1317,12 @@ func TestPostNotificationGetSenderName(t *testing.T) { channel: &model.Channel{Type: model.CHANNEL_DIRECT}, post: overriddenPost, allowOverrides: true, - expected: sender.Username, + expected: "@" + sender.Username, }, "overridden username, overrides disabled": { post: overriddenPost, allowOverrides: false, - expected: sender.Username, + expected: "@" + sender.Username, }, } { t.Run(name, func(t *testing.T) { diff --git a/app/plugin.go b/app/plugin.go index 5c60e682c9..2a49a2ecb9 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -14,6 +14,7 @@ import ( "github.com/mattermost/mattermost-server/plugin" "github.com/mattermost/mattermost-server/services/filesstore" "github.com/mattermost/mattermost-server/utils/fileutils" + "github.com/pkg/errors" ) // GetPluginsEnvironment returns the plugin environment for use if plugins are enabled and @@ -99,10 +100,11 @@ func (a *App) SyncPluginsActiveState() { continue } - if activated && updatedManifest.HasClient() { - message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_ENABLED, "", "", "", nil) - message.Add("manifest", updatedManifest.ClientManifest()) - a.Publish(message) + if activated { + // Notify all cluster clients if ready + if err := a.notifyPluginEnabled(updatedManifest); err != nil { + a.Log.Error("Failed to notify cluster on plugin enable", mlog.Err(err)) + } } } } @@ -287,6 +289,7 @@ func (a *App) GetActivePluginManifests() ([]*model.Manifest, *model.AppError) { // EnablePlugin will set the config for an installed plugin to enabled, triggering asynchronous // activation if inactive anywhere in the cluster. +// Notifies cluster peers through config change. func (a *App) EnablePlugin(id string) *model.AppError { pluginsEnvironment := a.GetPluginsEnvironment() if pluginsEnvironment == nil { @@ -316,7 +319,7 @@ func (a *App) EnablePlugin(id string) *model.AppError { cfg.PluginSettings.PluginStates[id] = &model.PluginState{Enable: true} }) - // This call will cause SyncPluginsActiveState to be called and the plugin to be activated + // This call will implicitly invoke SyncPluginsActiveState which will activate enabled plugins. if err := a.SaveConfig(a.Config(), true); err != nil { if err.Id == "ent.cluster.save_config.error" { return model.NewAppError("EnablePlugin", "app.plugin.cluster.save_config.app_error", nil, "", http.StatusInternalServerError) @@ -328,6 +331,7 @@ func (a *App) EnablePlugin(id string) *model.AppError { } // DisablePlugin will set the config for an installed plugin to disabled, triggering deactivation if active. +// Notifies cluster peers through config change. func (a *App) DisablePlugin(id string) *model.AppError { pluginsEnvironment := a.GetPluginsEnvironment() if pluginsEnvironment == nil { @@ -358,6 +362,7 @@ func (a *App) DisablePlugin(id string) *model.AppError { }) a.UnregisterPluginCommands(id) + // This call will implicitly invoke SyncPluginsActiveState which will deactivate disabled plugins. if err := a.SaveConfig(a.Config(), true); err != nil { return model.NewAppError("DisablePlugin", "app.plugin.config.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -394,3 +399,54 @@ func (a *App) GetPlugins() (*model.PluginsResponse, *model.AppError) { return resp, nil } + +// notifyPluginEnabled notifies connected websocket clients across all peers if the version of the given +// plugin is same across them. +// +// When a peer finds itself in agreement with all other peers as to the version of the given plugin, +// it will notify all connected websocket clients (across all peers) to trigger the (re-)installation. +// There is a small chance that this never occurs, because the last server to finish installing dies before it can announce. +// There is also a chance that multiple servers notify, but the webapp handles this idempotently. +func (a *App) notifyPluginEnabled(manifest *model.Manifest) error { + pluginsEnvironment := a.GetPluginsEnvironment() + if pluginsEnvironment == nil { + return errors.New("pluginsEnvironment is nil") + } + if !manifest.HasClient() || !pluginsEnvironment.IsActive(manifest.Id) { + return nil + } + + var statuses model.PluginStatuses + + if a.Cluster != nil { + var err *model.AppError + statuses, err = a.Cluster.GetPluginStatuses() + if err != nil { + return err + } + } + + localStatus, err := a.GetPluginStatus(manifest.Id) + if err != nil { + return err + } + statuses = append(statuses, localStatus) + + // This will not guard against the race condition of enabling a plugin immediately after installation. + // As GetPluginStatuses() will not return the new plugin (since other peers are racing to install), + // this peer will end up checking status against itself and will notify all webclients (including peer webclients), + // which may result in a 404. + for _, status := range statuses { + if status.PluginId == manifest.Id && status.Version != manifest.Version { + mlog.Debug("Not ready to notify webclients", mlog.String("cluster_id", status.ClusterId), mlog.String("plugin_id", manifest.Id)) + return nil + } + } + + // Notify all cluster peer clients. + message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_ENABLED, "", "", "", nil) + message.Add("manifest", manifest.ClientManifest()) + a.Publish(message) + + return nil +} diff --git a/app/plugin_install.go b/app/plugin_install.go index d6a8dc9242..10fe455db1 100644 --- a/app/plugin_install.go +++ b/app/plugin_install.go @@ -1,6 +1,39 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +// Installing a managed plugin consists of copying the uploaded plugin (*.tar.gz) to the filestore, +// unpacking to the configured local directory (PluginSettings.Directory), and copying any webapp bundle therein +// to the configured local client directory (PluginSettings.ClientDirectory). The unpacking and copy occurs +// each time the server starts, ensuring it remains synchronized with the set of installed plugins. +// +// When a plugin is enabled, all connected websocket clients are notified so as to fetch any webapp bundle and +// load the client-side portion of the plugin. This works well in a single-server system, but requires careful +// coordination in a high-availability cluster with multiple servers. In particular, websocket clients must not be +// notified of the newly enabled plugin until all servers in the cluster have finished unpacking the plugin, otherwise +// the webapp bundle might not yet be available. Ideally, each server would just notify its own set of connected peers +// after it finishes this process, but nothing prevents those clients from re-connecting to a different server behind +// the load balancer that hasn't finished unpacking. +// +// To achieve this coordination, each server instead checks the status of its peers after unpacking. If it finds peers with +// differing versions of the plugin, it skips the notification. If it finds all peers with the same version of the plugin, +// it notifies all websocket clients connected to all peers. There's a small chance that this never occurs if the the last +// server to finish unpacking dies before it can announce. There is also a chance that multiple servers decide to notify, +// but the webapp handles this idempotently. +// +// Complicating this flow further are the various means of notifying. In addition to websocket events, there are cluster +// messages between peers. There is a cluster message when the config changes and a plugin is enabled or disabled. +// There is a cluster message when installing or uninstalling a plugin. There is a cluster message when peer's plugin change +// its status. And finally the act of notifying websocket clients is propagated itself via a cluster message. +// +// The key methods involved in handling these notifications are notifyPluginEnabled and notifyPluginStatusesChanged. +// Note that none of this complexity applies to single-server systems or to plugins without a webapp bundle. +// +// Finally, in addition to managed plugins, note that there are unmanaged and prepackaged plugins. +// Unmanaged plugins are plugins installed manually to the configured local directory (PluginSettings.Directory). +// Prepackaged plugins are included with the server. They otherwise follow the above flow, except do not get uploaded +// to the filestore. Prepackaged plugins override all other plugins with the same plugin id. Managed plugins +// override unmanaged plugins with the same plugin id. +// package app import ( @@ -34,9 +67,18 @@ func (a *App) InstallPluginFromData(data model.PluginEventData) { } defer reader.Close() - if _, appErr = a.installPluginLocally(reader, true); appErr != nil { + manifest, appErr := a.installPluginLocally(reader, true) + if appErr != nil { mlog.Error("Failed to unpack plugin from filestore", mlog.Err(appErr), mlog.String("path", fileStorePath)) } + + if err := a.notifyPluginEnabled(manifest); err != nil { + mlog.Error("Failed notify plugin enabled", mlog.Err(err)) + } + + if err := a.notifyPluginStatusesChanged(); err != nil { + mlog.Error("Failed to notify plugin status changed", mlog.Err(err)) + } } func (a *App) RemovePluginFromData(data model.PluginEventData) { @@ -45,6 +87,10 @@ func (a *App) RemovePluginFromData(data model.PluginEventData) { if err := a.removePluginLocally(data.Id); err != nil { mlog.Error("Failed to remove plugin locally", mlog.Err(err), mlog.String("id", data.Id)) } + + if err := a.notifyPluginStatusesChanged(); err != nil { + mlog.Error("failed to notify plugin status changed", mlog.Err(err)) + } } // InstallPlugin unpacks and installs a plugin but does not enable or activate it. @@ -61,8 +107,8 @@ func (a *App) installPlugin(pluginFile io.ReadSeeker, replace bool) (*model.Mani // Store bundle in the file store to allow access from other servers. pluginFile.Seek(0, 0) - if _, err := a.WriteFile(pluginFile, a.getBundleStorePath(manifest.Id)); err != nil { - return nil, model.NewAppError("uploadPlugin", "app.plugin.store_bundle.app_error", nil, err.Error(), http.StatusInternalServerError) + if _, appErr := a.WriteFile(pluginFile, a.getBundleStorePath(manifest.Id)); appErr != nil { + return nil, model.NewAppError("uploadPlugin", "app.plugin.store_bundle.app_error", nil, appErr.Error(), http.StatusInternalServerError) } a.notifyClusterPluginEvent( @@ -72,29 +118,37 @@ func (a *App) installPlugin(pluginFile io.ReadSeeker, replace bool) (*model.Mani }, ) + if err := a.notifyPluginEnabled(manifest); err != nil { + mlog.Error("Failed notify plugin enabled", mlog.Err(err)) + } + + if err := a.notifyPluginStatusesChanged(); err != nil { + mlog.Error("Failed to notify plugin status changed", mlog.Err(err)) + } + return manifest, nil } func (a *App) installPluginLocally(pluginFile io.ReadSeeker, replace bool) (*model.Manifest, *model.AppError) { pluginsEnvironment := a.GetPluginsEnvironment() if pluginsEnvironment == nil { - return nil, model.NewAppError("installPlugin", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) + return nil, model.NewAppError("installPluginLocally", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) } tmpDir, err := ioutil.TempDir("", "plugintmp") if err != nil { - return nil, model.NewAppError("installPlugin", "app.plugin.filesystem.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, model.NewAppError("installPluginLocally", "app.plugin.filesystem.app_error", nil, err.Error(), http.StatusInternalServerError) } defer os.RemoveAll(tmpDir) if err = utils.ExtractTarGz(pluginFile, tmpDir); err != nil { - return nil, model.NewAppError("installPlugin", "app.plugin.extract.app_error", nil, err.Error(), http.StatusBadRequest) + return nil, model.NewAppError("installPluginLocally", "app.plugin.extract.app_error", nil, err.Error(), http.StatusBadRequest) } tmpPluginDir := tmpDir dir, err := ioutil.ReadDir(tmpDir) if err != nil { - return nil, model.NewAppError("installPlugin", "app.plugin.filesystem.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, model.NewAppError("installPluginLocally", "app.plugin.filesystem.app_error", nil, err.Error(), http.StatusInternalServerError) } if len(dir) == 1 && dir[0].IsDir() { @@ -103,30 +157,27 @@ func (a *App) installPluginLocally(pluginFile io.ReadSeeker, replace bool) (*mod manifest, _, err := model.FindManifest(tmpPluginDir) if err != nil { - return nil, model.NewAppError("installPlugin", "app.plugin.manifest.app_error", nil, err.Error(), http.StatusBadRequest) + return nil, model.NewAppError("installPluginLocally", "app.plugin.manifest.app_error", nil, err.Error(), http.StatusBadRequest) } if !plugin.IsValidId(manifest.Id) { - return nil, model.NewAppError("installPlugin", "app.plugin.invalid_id.app_error", map[string]interface{}{"Min": plugin.MinIdLength, "Max": plugin.MaxIdLength, "Regex": plugin.ValidIdRegex}, "", http.StatusBadRequest) + return nil, model.NewAppError("installPluginLocally", "app.plugin.invalid_id.app_error", map[string]interface{}{"Min": plugin.MinIdLength, "Max": plugin.MaxIdLength, "Regex": plugin.ValidIdRegex}, "", http.StatusBadRequest) } - // Stash the previous state of the plugin, if available - stashed := a.Config().PluginSettings.PluginStates[manifest.Id] - bundles, err := pluginsEnvironment.Available() if err != nil { - return nil, model.NewAppError("installPlugin", "app.plugin.install.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, model.NewAppError("installPluginLocally", "app.plugin.install.app_error", nil, err.Error(), http.StatusInternalServerError) } // Check that there is no plugin with the same ID for _, bundle := range bundles { if bundle.Manifest != nil && bundle.Manifest.Id == manifest.Id { if !replace { - return nil, model.NewAppError("installPlugin", "app.plugin.install_id.app_error", nil, "", http.StatusBadRequest) + return nil, model.NewAppError("installPluginLocally", "app.plugin.install_id.app_error", nil, "", http.StatusBadRequest) } if err := a.removePluginLocally(manifest.Id); err != nil { - return nil, model.NewAppError("installPlugin", "app.plugin.install_id_failed_remove.app_error", nil, "", http.StatusBadRequest) + return nil, model.NewAppError("installPluginLocally", "app.plugin.install_id_failed_remove.app_error", nil, "", http.StatusBadRequest) } } } @@ -134,22 +185,32 @@ func (a *App) installPluginLocally(pluginFile io.ReadSeeker, replace bool) (*mod pluginPath := filepath.Join(*a.Config().PluginSettings.Directory, manifest.Id) err = utils.CopyDir(tmpPluginDir, pluginPath) if err != nil { - return nil, model.NewAppError("installPlugin", "app.plugin.mvdir.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, model.NewAppError("installPluginLocally", "app.plugin.mvdir.app_error", nil, err.Error(), http.StatusInternalServerError) } // Flag plugin locally as managed by the filestore. f, err := os.Create(filepath.Join(pluginPath, managedPluginFileName)) if err != nil { - return nil, model.NewAppError("uploadPlugin", "app.plugin.flag_managed.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, model.NewAppError("installPluginLocally", "app.plugin.flag_managed.app_error", nil, err.Error(), http.StatusInternalServerError) } f.Close() - if stashed != nil && stashed.Enable { - a.EnablePlugin(manifest.Id) + if manifest.HasWebapp() { + updatedManifest, err := pluginsEnvironment.UnpackWebappBundle(manifest.Id) + if err != nil { + return nil, model.NewAppError("installPluginLocally", "app.plugin.webapp_bundle.app_error", nil, err.Error(), http.StatusInternalServerError) + } + manifest = updatedManifest } - if err := a.notifyPluginStatusesChanged(); err != nil { - mlog.Error("failed to notify plugin status changed", mlog.Err(err)) + // Activate plugin if it was previously activated. + pluginState := a.Config().PluginSettings.PluginStates[manifest.Id] + if pluginState != nil && pluginState.Enable { + updatedManifest, _, err := pluginsEnvironment.Activate(manifest.Id) + if err != nil { + return nil, model.NewAppError("installPluginLocally", "app.plugin.restart.app_error", nil, err.Error(), http.StatusInternalServerError) + } + manifest = updatedManifest } return manifest, nil @@ -160,6 +221,12 @@ func (a *App) RemovePlugin(id string) *model.AppError { } func (a *App) removePlugin(id string) *model.AppError { + // Disable plugin before removal to make sure this + // plugin remains disabled on re-install. + if err := a.DisablePlugin(id); err != nil { + return err + } + if err := a.removePluginLocally(id); err != nil { return err } @@ -212,18 +279,11 @@ func (a *App) removePluginLocally(id string) *model.AppError { return model.NewAppError("removePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusBadRequest) } - if pluginsEnvironment.IsActive(id) && manifest.HasClient() { - message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_DISABLED, "", "", "", nil) - message.Add("manifest", manifest.ClientManifest()) - a.Publish(message) - } - pluginsEnvironment.Deactivate(id) pluginsEnvironment.RemovePlugin(id) a.UnregisterPluginCommands(id) - err = os.RemoveAll(pluginPath) - if err != nil { + if err := os.RemoveAll(pluginPath); err != nil { return model.NewAppError("removePlugin", "app.plugin.remove.app_error", nil, err.Error(), http.StatusInternalServerError) } diff --git a/app/post_metadata.go b/app/post_metadata.go index 9eb9e987f4..f5d751bbf5 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -176,7 +176,10 @@ func (a *App) getEmbedForPost(post *model.Post, firstLink string, isNewPost bool }, nil } - return nil, nil + return &model.PostEmbed{ + Type: model.POST_EMBED_LINK, + URL: firstLink, + }, nil } func (a *App) getImagesForPost(post *model.Post, imageURLs []string, isNewPost bool) map[string]*model.PostImage { diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index e03ac54c46..d9dbe85f3a 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -536,6 +536,13 @@ func TestGetEmbedForPost(t *testing.T) { w.Header().Set("Content-Type", "image/png") w.Write(file) + } else if r.URL.Path == "/other" { + w.Header().Set("Content-Type", "text/html") + w.Write([]byte(` + + + + `)) } else { t.Fatal("Invalid path", r.URL.Path) } @@ -544,6 +551,7 @@ func TestGetEmbedForPost(t *testing.T) { ogURL := server.URL + "/index.html" imageURL := server.URL + "/image.png" + otherURL := server.URL + "/other" t.Run("with link previews enabled", func(t *testing.T) { th := Setup(t) @@ -593,6 +601,16 @@ func TestGetEmbedForPost(t *testing.T) { }, embed) assert.Nil(t, err) }) + + t.Run("should return a link embed", func(t *testing.T) { + embed, err := th.App.getEmbedForPost(&model.Post{}, otherURL, false) + + assert.Equal(t, &model.PostEmbed{ + Type: model.POST_EMBED_LINK, + URL: otherURL, + }, embed) + assert.Nil(t, err) + }) }) t.Run("with link previews disabled", func(t *testing.T) { @@ -634,6 +652,13 @@ func TestGetEmbedForPost(t *testing.T) { assert.Nil(t, embed) assert.Nil(t, err) }) + + t.Run("should not return a link embed", func(t *testing.T) { + embed, err := th.App.getEmbedForPost(&model.Post{}, otherURL, false) + + assert.Nil(t, embed) + assert.Nil(t, err) + }) }) } diff --git a/i18n/en.json b/i18n/en.json index e695d7bd07..6fc931802a 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -3506,6 +3506,10 @@ "id": "app.plugin.remove_bundle.app_error", "translation": "Unable to remove plugin bundle from file store." }, + { + "id": "app.plugin.restart.app_error", + "translation": "Unable to restart plugin on upgrade." + }, { "id": "app.plugin.store_bundle.app_error", "translation": "Unable to store the plugin to the configured file store." @@ -3522,6 +3526,10 @@ "id": "app.plugin.upload_disabled.app_error", "translation": "Plugins and/or plugin uploads have been disabled." }, + { + "id": "app.plugin.webapp_bundle.app_error", + "translation": "Unable to generate plugin webapp bundle." + }, { "id": "app.role.check_roles_exist.role_not_found", "translation": "The provided role does not exist" diff --git a/model/client4.go b/model/client4.go index cdc70f26b8..0195bef380 100644 --- a/model/client4.go +++ b/model/client4.go @@ -2898,7 +2898,7 @@ func (c *Client4) GetPing() (string, *Response) { } // GetPingWithServerStatus will return ok if several basic server health checks -// all psss successfully. +// all pass successfully. func (c *Client4) GetPingWithServerStatus() (string, *Response) { r, err := c.DoApiGet(c.GetSystemRoute()+"/ping?get_server_status=true", "") if r != nil && r.StatusCode == 500 { diff --git a/model/post_embed.go b/model/post_embed.go index 6e8e33cadb..15d061fc79 100644 --- a/model/post_embed.go +++ b/model/post_embed.go @@ -7,6 +7,7 @@ const ( POST_EMBED_IMAGE PostEmbedType = "image" POST_EMBED_MESSAGE_ATTACHMENT PostEmbedType = "message_attachment" POST_EMBED_OPENGRAPH PostEmbedType = "opengraph" + POST_EMBED_LINK PostEmbedType = "link" ) type PostEmbedType string diff --git a/model/user.go b/model/user.go index 4d71c13587..db0621e04e 100644 --- a/model/user.go +++ b/model/user.go @@ -539,8 +539,8 @@ func (u *User) GetFullName() string { } } -func (u *User) GetDisplayName(nameFormat string) string { - displayName := u.Username +func (u *User) getDisplayName(baseName, nameFormat string) string { + displayName := baseName if nameFormat == SHOW_NICKNAME_FULLNAME { if len(u.Nickname) > 0 { @@ -557,6 +557,18 @@ func (u *User) GetDisplayName(nameFormat string) string { return displayName } +func (u *User) GetDisplayName(nameFormat string) string { + displayName := u.Username + + return u.getDisplayName(displayName, nameFormat) +} + +func (u *User) GetDisplayNameWithPrefix(nameFormat, prefix string) string { + displayName := prefix + u.Username + + return u.getDisplayName(displayName, nameFormat) +} + func (u *User) GetRoles() []string { return strings.Fields(u.Roles) } diff --git a/model/user_test.go b/model/user_test.go index 0211283dcb..95cb03a493 100644 --- a/model/user_test.go +++ b/model/user_test.go @@ -252,6 +252,42 @@ func TestUserGetDisplayName(t *testing.T) { } } +func TestUserGetDisplayNameWithPrefix(t *testing.T) { + user := User{Username: "username"} + + if displayName := user.GetDisplayNameWithPrefix(SHOW_FULLNAME, "@"); displayName != "@username" { + t.Fatal("Display name should be username") + } + + if displayName := user.GetDisplayNameWithPrefix(SHOW_NICKNAME_FULLNAME, "@"); displayName != "@username" { + t.Fatal("Display name should be username") + } + + if displayName := user.GetDisplayNameWithPrefix(SHOW_USERNAME, "@"); displayName != "@username" { + t.Fatal("Display name should be username") + } + + user.FirstName = "first" + user.LastName = "last" + + if displayName := user.GetDisplayNameWithPrefix(SHOW_FULLNAME, "@"); displayName != "first last" { + t.Fatal("Display name should be full name") + } + + if displayName := user.GetDisplayNameWithPrefix(SHOW_NICKNAME_FULLNAME, "@"); displayName != "first last" { + t.Fatal("Display name should be full name since there is no nickname") + } + + if displayName := user.GetDisplayNameWithPrefix(SHOW_USERNAME, "@"); displayName != "@username" { + t.Fatal("Display name should be username") + } + + user.Nickname = "nickname" + if displayName := user.GetDisplayNameWithPrefix(SHOW_NICKNAME_FULLNAME, "@"); displayName != "nickname" { + t.Fatal("Display name should be nickname") + } +} + var usernames = []struct { value string expected bool diff --git a/plugin/environment.go b/plugin/environment.go index 9bdf62b425..d301a8942e 100644 --- a/plugin/environment.go +++ b/plugin/environment.go @@ -217,38 +217,11 @@ func (env *Environment) Activate(id string) (manifest *model.Manifest, activated componentActivated := false if pluginInfo.Manifest.HasWebapp() { - bundlePath := filepath.Clean(pluginInfo.Manifest.Webapp.BundlePath) - if bundlePath == "" || bundlePath[0] == '.' { - return nil, false, fmt.Errorf("invalid webapp bundle path") - } - bundlePath = filepath.Join(env.pluginDir, id, bundlePath) - destinationPath := filepath.Join(env.webappPluginDir, id) - - if err := os.RemoveAll(destinationPath); err != nil { - return nil, false, errors.Wrapf(err, "unable to remove old webapp bundle directory: %v", destinationPath) - } - - if err := utils.CopyDir(filepath.Dir(bundlePath), destinationPath); err != nil { - return nil, false, errors.Wrapf(err, "unable to copy webapp bundle directory: %v", id) - } - - sourceBundleFilepath := filepath.Join(destinationPath, filepath.Base(bundlePath)) - - sourceBundleFileContents, err := ioutil.ReadFile(sourceBundleFilepath) + updatedManifest, err := env.UnpackWebappBundle(id) if err != nil { - return nil, false, errors.Wrapf(err, "unable to read webapp bundle: %v", id) - } - - hash := fnv.New64a() - hash.Write(sourceBundleFileContents) - pluginInfo.Manifest.Webapp.BundleHash = hash.Sum([]byte{}) - - if err := os.Rename( - sourceBundleFilepath, - filepath.Join(destinationPath, fmt.Sprintf("%s_%x_bundle.js", id, pluginInfo.Manifest.Webapp.BundleHash)), - ); err != nil { - return nil, false, errors.Wrapf(err, "unable to rename webapp bundle: %v", id) + return nil, false, errors.Wrapf(err, "unable to generate webapp bundle: %v", id) } + pluginInfo.Manifest.Webapp.BundleHash = updatedManifest.Webapp.BundleHash componentActivated = true } @@ -327,6 +300,63 @@ func (env *Environment) Shutdown() { }) } +// UnpackWebappBundle unpacks webapp bundle for a given plugin id on disk. +func (env *Environment) UnpackWebappBundle(id string) (*model.Manifest, error) { + plugins, err := env.Available() + if err != nil { + return nil, errors.New("Unable to get available plugins") + } + var manifest *model.Manifest + for _, p := range plugins { + if p.Manifest != nil && p.Manifest.Id == id { + if manifest != nil { + return nil, fmt.Errorf("multiple plugins found: %v", id) + } + manifest = p.Manifest + } + } + if manifest == nil { + return nil, fmt.Errorf("plugin not found: %v", id) + } + + bundlePath := filepath.Clean(manifest.Webapp.BundlePath) + if bundlePath == "" || bundlePath[0] == '.' { + return nil, fmt.Errorf("invalid webapp bundle path") + } + bundlePath = filepath.Join(env.pluginDir, id, bundlePath) + destinationPath := filepath.Join(env.webappPluginDir, id) + + if err = os.RemoveAll(destinationPath); err != nil { + return nil, errors.Wrapf(err, "unable to remove old webapp bundle directory: %v", destinationPath) + } + + if err = utils.CopyDir(filepath.Dir(bundlePath), destinationPath); err != nil { + return nil, errors.Wrapf(err, "unable to copy webapp bundle directory: %v", id) + } + + sourceBundleFilepath := filepath.Join(destinationPath, filepath.Base(bundlePath)) + + sourceBundleFileContents, err := ioutil.ReadFile(sourceBundleFilepath) + if err != nil { + return nil, errors.Wrapf(err, "unable to read webapp bundle: %v", id) + } + + hash := fnv.New64a() + if _, err = hash.Write(sourceBundleFileContents); err != nil { + return nil, errors.Wrapf(err, "unable to generate hash for webapp bundle: %v", id) + } + manifest.Webapp.BundleHash = hash.Sum([]byte{}) + + if err = os.Rename( + sourceBundleFilepath, + filepath.Join(destinationPath, fmt.Sprintf("%s_%x_bundle.js", id, manifest.Webapp.BundleHash)), + ); err != nil { + return nil, errors.Wrapf(err, "unable to rename webapp bundle: %v", id) + } + + return manifest, nil +} + // HooksForPlugin returns the hooks API for the plugin with the given id. // // Consider using RunMultiPluginHook instead.