diff --git a/server/configuration.go b/server/configuration.go new file mode 100644 index 000000000..1ab2f5127 --- /dev/null +++ b/server/configuration.go @@ -0,0 +1,89 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License for license information. + +package main + +import ( + "reflect" + + "github.com/pkg/errors" +) + +// configuration captures the plugin's external configuration as exposed in the Mattermost server +// configuration, as well as values computed from the configuration. Any public fields will be +// deserialized from the Mattermost server configuration in OnConfigurationChange. +// +// As plugins are inherently concurrent (hooks being called asynchronously), and the plugin +// configuration can change at any time, access to the configuration must be synchronized. The +// strategy used in this plugin is to guard a pointer to the configuration, and clone the entire +// struct whenever it changes. You may replace this with whatever strategy you choose. +// +// If you add non-reference types to your configuration struct, be sure to rewrite Clone as a deep +// copy appropriate for your types. +type configuration struct { + Enabled bool + Secret string + UserName string +} + +// Clone shallow copies the configuration. Your implementation may require a deep copy if +// your configuration has reference types. +func (c *configuration) Clone() *configuration { + var clone = *c + return &clone +} + +// getConfiguration retrieves the active configuration under lock, making it safe to use +// concurrently. The active configuration may change underneath the client of this method, but +// the struct returned by this API call is considered immutable. +func (p *Plugin) getConfiguration() *configuration { + p.configurationLock.RLock() + defer p.configurationLock.RUnlock() + + if p.configuration == nil { + return &configuration{} + } + + return p.configuration +} + +// setConfiguration replaces the active configuration under lock. +// +// Do not call setConfiguration while holding the configurationLock, as sync.Mutex is not +// reentrant. In particular, avoid using the plugin API entirely, as this may in turn trigger a +// hook back into the plugin. If that hook attempts to acquire this lock, a deadlock may occur. +// +// This method panics if setConfiguration is called with the existing configuration. This almost +// certainly means that the configuration was modified without being cloned and may result in +// an unsafe access. +func (p *Plugin) setConfiguration(configuration *configuration) { + p.configurationLock.Lock() + defer p.configurationLock.Unlock() + + if configuration != nil && p.configuration == configuration { + // Ignore assignment if the configuration struct is empty. Go will optimize the + // allocation for same to point at the same memory address, breaking the check + // above. + if reflect.ValueOf(*configuration).NumField() == 0 { + return + } + + panic("setConfiguration called with the existing configuration") + } + + p.configuration = configuration +} + +// OnConfigurationChange is invoked when configuration changes may have been made. +func (p *Plugin) OnConfigurationChange() error { + var configuration = new(configuration) + + // Load the public configuration fields from the Mattermost server configuration. + if err := p.API.LoadPluginConfiguration(configuration); err != nil { + return errors.Wrap(err, "failed to load plugin configuration") + } + + p.setConfiguration(configuration) + + return nil +} diff --git a/server/plugin.go b/server/plugin.go index 18ecc7739..094999a31 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -7,6 +7,7 @@ import ( "crypto/subtle" "encoding/json" "net/http" + "sync" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" @@ -15,13 +16,17 @@ import ( type Plugin struct { plugin.MattermostPlugin - Enabled bool - Secret string - UserName string + // configurationLock synchronizes access to the configuration. + configurationLock sync.RWMutex + + // configuration is the active plugin configuration. Consult getConfiguration and + // setConfiguration for usage. + configuration *configuration } func (p *Plugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Request) { - if !p.Enabled || p.Secret == "" || p.UserName == "" { + config := p.getConfiguration() + if !config.Enabled || config.Secret == "" || config.UserName == "" { http.Error(w, "This plugin is not configured.", http.StatusForbidden) return } else if r.URL.Path != "/webhook" { @@ -30,7 +35,7 @@ func (p *Plugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Req } else if r.Method != http.MethodPost { http.Error(w, "method not allowed", http.StatusMethodNotAllowed) return - } else if subtle.ConstantTimeCompare([]byte(r.URL.Query().Get("secret")), []byte(p.Secret)) != 1 { + } else if subtle.ConstantTimeCompare([]byte(r.URL.Query().Get("secret")), []byte(config.Secret)) != 1 { http.Error(w, "You must provide the configured secret.", http.StatusForbidden) return } @@ -45,7 +50,7 @@ func (p *Plugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Req return } else if r.URL.Query().Get("channel") == "" { http.Error(w, "You must provide a channel.", http.StatusBadRequest) - } else if user, err := p.API.GetUserByUsername(p.UserName); err != nil { + } else if user, err := p.API.GetUserByUsername(config.UserName); err != nil { http.Error(w, err.Message, err.StatusCode) } else if channel, err := p.API.GetChannelByNameForTeamName(r.URL.Query().Get("team"), r.URL.Query().Get("channel"), false); err != nil { http.Error(w, err.Message, err.StatusCode) diff --git a/server/plugin_test.go b/server/plugin_test.go index fb075a9e6..86801e699 100644 --- a/server/plugin_test.go +++ b/server/plugin_test.go @@ -147,9 +147,11 @@ func TestPlugin(t *testing.T) { }, (*model.AppError)(nil)) p := Plugin{} - p.Enabled = tc.Configuration.Enabled - p.Secret = tc.Configuration.Secret - p.UserName = tc.Configuration.UserName + p.setConfiguration(&configuration{ + Enabled: tc.Configuration.Enabled, + Secret: tc.Configuration.Secret, + UserName: tc.Configuration.UserName, + }) p.SetAPI(api) w := httptest.NewRecorder()