From eff65aa05c74e93533c2504b8141b0474011e68c Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 7 Feb 2018 11:05:46 -0600 Subject: ABC-132: sign error page parameters (#8197) * sign error page parameters * add comments --- app/app.go | 22 +++++++++----- app/config.go | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ app/config_test.go | 9 ++++++ 3 files changed, 111 insertions(+), 8 deletions(-) (limited to 'app') diff --git a/app/app.go b/app/app.go index 1e46d29d0..0b5efa76b 100644 --- a/app/app.go +++ b/app/app.go @@ -4,6 +4,7 @@ package app import ( + "crypto/ecdsa" "html/template" "net" "net/http" @@ -60,13 +61,14 @@ type App struct { newStore func() store.Store - htmlTemplateWatcher *utils.HTMLTemplateWatcher - sessionCache *utils.Cache - roles map[string]*model.Role - configListenerId string - licenseListenerId string - disableConfigWatch bool - configWatcher *utils.ConfigWatcher + htmlTemplateWatcher *utils.HTMLTemplateWatcher + sessionCache *utils.Cache + roles map[string]*model.Role + configListenerId string + licenseListenerId string + disableConfigWatch bool + configWatcher *utils.ConfigWatcher + asymmetricSigningKey *ecdsa.PrivateKey pluginCommands []*PluginCommand pluginCommandsLock sync.RWMutex @@ -139,6 +141,10 @@ func New(options ...Option) (*App, error) { } app.Srv.Store = app.newStore() + if err := app.ensureAsymmetricSigningKey(); err != nil { + return nil, errors.Wrapf(err, "unable to ensure asymmetric signing key") + } + app.initJobs() app.initBuiltInPlugins() @@ -448,5 +454,5 @@ func (a *App) Handle404(w http.ResponseWriter, r *http.Request) { l4g.Debug("%v: code=404 ip=%v", r.URL.Path, utils.GetIpAddress(r)) - utils.RenderWebError(err, w, r) + utils.RenderWebAppError(w, r, err, a.AsymmetricSigningKey()) } diff --git a/app/config.go b/app/config.go index a2398f9e9..46426c442 100644 --- a/app/config.go +++ b/app/config.go @@ -4,7 +4,12 @@ package app import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/md5" + "crypto/rand" + "crypto/x509" + "encoding/base64" "encoding/json" "fmt" "runtime/debug" @@ -116,8 +121,91 @@ func (a *App) InvokeConfigListeners(old, current *model.Config) { } } +// EnsureAsymmetricSigningKey ensures that an asymmetric signing key exists and future calls to +// AsymmetricSigningKey will always return a valid signing key. +func (a *App) ensureAsymmetricSigningKey() error { + if a.asymmetricSigningKey != nil { + return nil + } + + var key *model.SystemAsymmetricSigningKey + + result := <-a.Srv.Store.System().GetByName(model.SYSTEM_ASYMMETRIC_SIGNING_KEY) + if result.Err == nil { + if err := json.Unmarshal([]byte(result.Data.(*model.System).Value), &key); err != nil { + return err + } + } + + // If we don't already have a key, try to generate one. + if key == nil { + newECDSAKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + return err + } + newKey := &model.SystemAsymmetricSigningKey{ + ECDSAKey: &model.SystemECDSAKey{ + Curve: "P-256", + X: newECDSAKey.X, + Y: newECDSAKey.Y, + D: newECDSAKey.D, + }, + } + system := &model.System{ + Name: model.SYSTEM_ASYMMETRIC_SIGNING_KEY, + } + v, err := json.Marshal(newKey) + if err != nil { + return err + } + system.Value = string(v) + if result = <-a.Srv.Store.System().Save(system); result.Err == nil { + // If we were able to save the key, use it, otherwise ignore the error. + key = newKey + } + } + + // If we weren't able to save a new key above, another server must have beat us to it. Get the + // key from the database, and if that fails, error out. + if key == nil { + result := <-a.Srv.Store.System().GetByName(model.SYSTEM_ASYMMETRIC_SIGNING_KEY) + if result.Err != nil { + return result.Err + } else if err := json.Unmarshal([]byte(result.Data.(*model.System).Value), &key); err != nil { + return err + } + } + + var curve elliptic.Curve + switch key.ECDSAKey.Curve { + case "P-256": + curve = elliptic.P256() + default: + return fmt.Errorf("unknown curve: " + key.ECDSAKey.Curve) + } + a.asymmetricSigningKey = &ecdsa.PrivateKey{ + PublicKey: ecdsa.PublicKey{ + Curve: curve, + X: key.ECDSAKey.X, + Y: key.ECDSAKey.Y, + }, + D: key.ECDSAKey.D, + } + a.regenerateClientConfig() + return nil +} + +// AsymmetricSigningKey will return a private key that can be used for asymmetric signing. +func (a *App) AsymmetricSigningKey() *ecdsa.PrivateKey { + return a.asymmetricSigningKey +} + func (a *App) regenerateClientConfig() { a.clientConfig = utils.GenerateClientConfig(a.Config(), a.DiagnosticId()) + if key := a.AsymmetricSigningKey(); key != nil { + der, _ := x509.MarshalPKIXPublicKey(&key.PublicKey) + a.clientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der) + } clientConfigJSON, _ := json.Marshal(a.clientConfig) a.clientConfigHash = fmt.Sprintf("%x", md5.Sum(clientConfigJSON)) } diff --git a/app/config_test.go b/app/config_test.go index e3d50b958..5ee999f0f 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -6,6 +6,8 @@ package app import ( "testing" + "github.com/stretchr/testify/assert" + "github.com/mattermost/mattermost-server/model" ) @@ -54,3 +56,10 @@ func TestConfigListener(t *testing.T) { t.Fatal("listener 2 should've been called") } } + +func TestAsymmetricSigningKey(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + assert.NotNil(t, th.App.AsymmetricSigningKey()) + assert.NotEmpty(t, th.App.ClientConfig()["AsymmetricSigningPublicKey"]) +} -- cgit v1.2.3-1-g7c22 From 0f703a3368a0b16fcd48b474377f0dbd2144f366 Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 7 Feb 2018 16:20:51 -0600 Subject: Eliminate utils.SetLicense calls (#8217) * eliminate utils.SetLicense calls * test fix * another test fix * more test fixes --- app/app.go | 3 +++ app/config.go | 8 -------- app/license.go | 16 +++++++++++++++- app/session_test.go | 13 +------------ 4 files changed, 19 insertions(+), 21 deletions(-) (limited to 'app') diff --git a/app/app.go b/app/app.go index 0b5efa76b..3c37ec252 100644 --- a/app/app.go +++ b/app/app.go @@ -88,6 +88,9 @@ func New(options ...Option) (*App, error) { panic("Only one App should exist at a time. Did you forget to call Shutdown()?") } + // TODO: remove this once utils global license state is eliminated + utils.SetLicense(nil) + app := &App{ goroutineExitSignal: make(chan struct{}, 1), Srv: &Server{ diff --git a/app/config.go b/app/config.go index 46426c442..55be24352 100644 --- a/app/config.go +++ b/app/config.go @@ -254,11 +254,3 @@ func (a *App) Desanitize(cfg *model.Config) { cfg.SqlSettings.DataSourceSearchReplicas[i] = actual.SqlSettings.DataSourceSearchReplicas[i] } } - -// License returns the currently active license or nil if the application is unlicensed. -func (a *App) License() *model.License { - if utils.IsLicensed() { - return utils.License() - } - return nil -} diff --git a/app/license.go b/app/license.go index c7fd07197..7402b5c22 100644 --- a/app/license.go +++ b/app/license.go @@ -59,7 +59,7 @@ func (a *App) SaveLicense(licenseBytes []byte) (*model.License, *model.AppError) } } - if ok := utils.SetLicense(license); !ok { + if ok := a.SetLicense(license); !ok { return nil, model.NewAppError("addLicense", model.EXPIRED_LICENSE_ERROR, nil, "", http.StatusBadRequest) } @@ -102,6 +102,20 @@ func (a *App) SaveLicense(licenseBytes []byte) (*model.License, *model.AppError) return license, nil } +// License returns the currently active license or nil if the application is unlicensed. +func (a *App) License() *model.License { + if utils.IsLicensed() { + return utils.License() + } + return nil +} + +func (a *App) SetLicense(license *model.License) bool { + ok := utils.SetLicense(license) + a.SetDefaultRolesBasedOnConfig() + return ok +} + func (a *App) RemoveLicense() *model.AppError { utils.RemoveLicense() diff --git a/app/session_test.go b/app/session_test.go index bca3b59b7..09d81f4ac 100644 --- a/app/session_test.go +++ b/app/session_test.go @@ -48,18 +48,7 @@ func TestGetSessionIdleTimeoutInMinutes(t *testing.T) { session, _ = th.App.CreateSession(session) - isLicensed := utils.IsLicensed() - license := utils.License() - timeout := *th.App.Config().ServiceSettings.SessionIdleTimeoutInMinutes - defer func() { - utils.SetIsLicensed(isLicensed) - utils.SetLicense(license) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SessionIdleTimeoutInMinutes = timeout }) - }() - utils.SetIsLicensed(true) - utils.SetLicense(&model.License{Features: &model.Features{}}) - utils.License().Features.SetDefaults() - *utils.License().Features.Compliance = true + th.App.SetLicense(model.NewTestLicense("compliance")) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SessionIdleTimeoutInMinutes = 5 }) rsession, err := th.App.GetSession(session.Token) -- cgit v1.2.3-1-g7c22 From a6309aaf48e216fe5f6779188071d4b621b643b6 Mon Sep 17 00:00:00 2001 From: Chris Date: Fri, 9 Feb 2018 10:04:48 -0600 Subject: Remove license globals entirely (#8229) * remove license globals entirely * fix infinite recursion * test fix --- app/admin.go | 3 +- app/app.go | 24 ++++++------ app/apptestlib.go | 5 ++- app/config.go | 2 +- app/email.go | 3 +- app/file.go | 3 +- app/license.go | 105 ++++++++++++++++++++++++++++++++++++++++++++++------ app/license_test.go | 75 ++++++++++++++++++++++++++++++++++++- app/role.go | 6 +-- app/session_test.go | 9 ++--- 10 files changed, 197 insertions(+), 38 deletions(-) (limited to 'app') diff --git a/app/admin.go b/app/admin.go index b838ed3bd..154fa8899 100644 --- a/app/admin.go +++ b/app/admin.go @@ -237,7 +237,8 @@ func (a *App) TestEmail(userId string, cfg *model.Config) *model.AppError { return err } else { T := utils.GetUserTranslations(user.Locale) - if err := utils.SendMailUsingConfig(user.Email, T("api.admin.test_email.subject"), T("api.admin.test_email.body"), cfg); err != nil { + license := a.License() + if err := utils.SendMailUsingConfig(user.Email, T("api.admin.test_email.subject"), T("api.admin.test_email.body"), cfg, license != nil && *license.Features.Compliance); err != nil { return err } } diff --git a/app/app.go b/app/app.go index 3c37ec252..dd5deb342 100644 --- a/app/app.go +++ b/app/app.go @@ -59,6 +59,10 @@ type App struct { configFile string configListeners map[string]func(*model.Config, *model.Config) + licenseValue atomic.Value + clientLicenseValue atomic.Value + licenseListeners map[string]func() + newStore func() store.Store htmlTemplateWatcher *utils.HTMLTemplateWatcher @@ -88,18 +92,16 @@ func New(options ...Option) (*App, error) { panic("Only one App should exist at a time. Did you forget to call Shutdown()?") } - // TODO: remove this once utils global license state is eliminated - utils.SetLicense(nil) - app := &App{ goroutineExitSignal: make(chan struct{}, 1), Srv: &Server{ Router: mux.NewRouter(), }, - sessionCache: utils.NewLru(model.SESSION_CACHE_SIZE), - configFile: "config.json", - configListeners: make(map[string]func(*model.Config, *model.Config)), - clientConfig: make(map[string]string), + sessionCache: utils.NewLru(model.SESSION_CACHE_SIZE), + configFile: "config.json", + configListeners: make(map[string]func(*model.Config, *model.Config)), + clientConfig: make(map[string]string), + licenseListeners: map[string]func(){}, } for _, option := range options { @@ -123,9 +125,9 @@ func New(options ...Option) (*App, error) { app.configListenerId = app.AddConfigListener(func(_, _ *model.Config) { app.configOrLicenseListener() }) - app.licenseListenerId = utils.AddLicenseListener(app.configOrLicenseListener) + app.licenseListenerId = app.AddLicenseListener(app.configOrLicenseListener) app.regenerateClientConfig() - app.SetDefaultRolesBasedOnConfig() + app.setDefaultRolesBasedOnConfig() l4g.Info(utils.T("api.server.new_server.init.info")) @@ -166,7 +168,7 @@ func New(options ...Option) (*App, error) { func (a *App) configOrLicenseListener() { a.regenerateClientConfig() - a.SetDefaultRolesBasedOnConfig() + a.setDefaultRolesBasedOnConfig() } func (a *App) Shutdown() { @@ -188,7 +190,7 @@ func (a *App) Shutdown() { } a.RemoveConfigListener(a.configListenerId) - utils.RemoveLicenseListener(a.licenseListenerId) + a.RemoveLicenseListener(a.licenseListenerId) l4g.Info(utils.T("api.server.stop_server.stopped.info")) a.DisableConfigWatch() diff --git a/app/apptestlib.go b/app/apptestlib.go index 016a68bec..c7846c9b5 100644 --- a/app/apptestlib.go +++ b/app/apptestlib.go @@ -106,9 +106,10 @@ func setupTestHelper(enterprise bool) *TestHelper { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableOpenServer = true }) - utils.SetIsLicensed(enterprise) if enterprise { - utils.License().Features.SetDefaults() + th.App.SetLicense(model.NewTestLicense()) + } else { + th.App.SetLicense(nil) } return th diff --git a/app/config.go b/app/config.go index 55be24352..b4925e8fb 100644 --- a/app/config.go +++ b/app/config.go @@ -201,7 +201,7 @@ func (a *App) AsymmetricSigningKey() *ecdsa.PrivateKey { } func (a *App) regenerateClientConfig() { - a.clientConfig = utils.GenerateClientConfig(a.Config(), a.DiagnosticId()) + a.clientConfig = utils.GenerateClientConfig(a.Config(), a.DiagnosticId(), a.License()) if key := a.AsymmetricSigningKey(); key != nil { der, _ := x509.MarshalPKIXPublicKey(&key.PublicKey) a.clientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der) diff --git a/app/email.go b/app/email.go index b809b972d..89de2ae65 100644 --- a/app/email.go +++ b/app/email.go @@ -316,5 +316,6 @@ func (a *App) NewEmailTemplate(name, locale string) *utils.HTMLTemplate { } func (a *App) SendMail(to, subject, htmlBody string) *model.AppError { - return utils.SendMailUsingConfig(to, subject, htmlBody, a.Config()) + license := a.License() + return utils.SendMailUsingConfig(to, subject, htmlBody, a.Config(), license != nil && *license.Features.Compliance) } diff --git a/app/file.go b/app/file.go index d66c64adb..bb20585bb 100644 --- a/app/file.go +++ b/app/file.go @@ -58,7 +58,8 @@ const ( ) func (a *App) FileBackend() (utils.FileBackend, *model.AppError) { - return utils.NewFileBackend(&a.Config().FileSettings) + license := a.License() + return utils.NewFileBackend(&a.Config().FileSettings, license != nil && *license.Features.Compliance) } func (a *App) ReadFile(path string) ([]byte, *model.AppError) { diff --git a/app/license.go b/app/license.go index 7402b5c22..efb725a20 100644 --- a/app/license.go +++ b/app/license.go @@ -4,16 +4,19 @@ package app import ( + "crypto/md5" + "fmt" "net/http" "strings" l4g "github.com/alecthomas/log4go" + "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" ) func (a *App) LoadLicense() { - utils.RemoveLicense() + a.SetLicense(nil) licenseId := "" if result := <-a.Srv.Store.System().Get(); result.Err == nil { @@ -36,7 +39,7 @@ func (a *App) LoadLicense() { if result := <-a.Srv.Store.License().Get(licenseId); result.Err == nil { record := result.Data.(*model.LicenseRecord) - utils.LoadLicense([]byte(record.Bytes)) + a.ValidateAndSetLicenseBytes([]byte(record.Bytes)) l4g.Info("License key valid unlocking enterprise features.") } else { l4g.Info(utils.T("mattermost.load_license.find.warn")) @@ -104,33 +107,113 @@ func (a *App) SaveLicense(licenseBytes []byte) (*model.License, *model.AppError) // License returns the currently active license or nil if the application is unlicensed. func (a *App) License() *model.License { - if utils.IsLicensed() { - return utils.License() - } - return nil + license, _ := a.licenseValue.Load().(*model.License) + return license } func (a *App) SetLicense(license *model.License) bool { - ok := utils.SetLicense(license) - a.SetDefaultRolesBasedOnConfig() - return ok + defer func() { + a.setDefaultRolesBasedOnConfig() + for _, listener := range a.licenseListeners { + listener() + } + }() + + if license != nil { + license.Features.SetDefaults() + + if !license.IsExpired() { + a.licenseValue.Store(license) + a.clientLicenseValue.Store(utils.GetClientLicense(license)) + return true + } + } + + a.licenseValue.Store((*model.License)(nil)) + a.SetClientLicense(map[string]string{"IsLicensed": "false"}) + return false +} + +func (a *App) ValidateAndSetLicenseBytes(b []byte) { + if success, licenseStr := utils.ValidateLicense(b); success { + license := model.LicenseFromJson(strings.NewReader(licenseStr)) + a.SetLicense(license) + return + } + + l4g.Warn(utils.T("utils.license.load_license.invalid.warn")) +} + +func (a *App) SetClientLicense(m map[string]string) { + a.clientLicenseValue.Store(m) +} + +func (a *App) ClientLicense() map[string]string { + clientLicense, _ := a.clientLicenseValue.Load().(map[string]string) + return clientLicense } func (a *App) RemoveLicense() *model.AppError { - utils.RemoveLicense() + if license, _ := a.licenseValue.Load().(*model.License); license == nil { + return nil + } sysVar := &model.System{} sysVar.Name = model.SYSTEM_ACTIVE_LICENSE_ID sysVar.Value = "" if result := <-a.Srv.Store.System().SaveOrUpdate(sysVar); result.Err != nil { - utils.RemoveLicense() return result.Err } + a.SetLicense(nil) a.ReloadConfig() a.InvalidateAllCaches() return nil } + +func (a *App) AddLicenseListener(listener func()) string { + id := model.NewId() + a.licenseListeners[id] = listener + return id +} + +func (a *App) RemoveLicenseListener(id string) { + delete(a.licenseListeners, id) +} + +func (a *App) GetClientLicenseEtag(useSanitized bool) string { + value := "" + + lic := a.ClientLicense() + + if useSanitized { + lic = a.GetSanitizedClientLicense() + } + + for k, v := range lic { + value += fmt.Sprintf("%s:%s;", k, v) + } + + return model.Etag(fmt.Sprintf("%x", md5.Sum([]byte(value)))) +} + +func (a *App) GetSanitizedClientLicense() map[string]string { + sanitizedLicense := make(map[string]string) + + for k, v := range a.ClientLicense() { + sanitizedLicense[k] = v + } + + delete(sanitizedLicense, "Id") + delete(sanitizedLicense, "Name") + delete(sanitizedLicense, "Email") + delete(sanitizedLicense, "PhoneNumber") + delete(sanitizedLicense, "IssuedAt") + delete(sanitizedLicense, "StartsAt") + delete(sanitizedLicense, "ExpiresAt") + + return sanitizedLicense +} diff --git a/app/license_test.go b/app/license_test.go index 5b73d9d18..f86d604d1 100644 --- a/app/license_test.go +++ b/app/license_test.go @@ -4,8 +4,9 @@ package app import ( - //"github.com/mattermost/mattermost-server/model" "testing" + + "github.com/mattermost/mattermost-server/model" ) func TestLoadLicense(t *testing.T) { @@ -37,3 +38,75 @@ func TestRemoveLicense(t *testing.T) { t.Fatal("should have removed license") } } + +func TestSetLicense(t *testing.T) { + th := Setup() + defer th.TearDown() + + l1 := &model.License{} + l1.Features = &model.Features{} + l1.Customer = &model.Customer{} + l1.StartsAt = model.GetMillis() - 1000 + l1.ExpiresAt = model.GetMillis() + 100000 + if ok := th.App.SetLicense(l1); !ok { + t.Fatal("license should have worked") + } + + l2 := &model.License{} + l2.Features = &model.Features{} + l2.Customer = &model.Customer{} + l2.StartsAt = model.GetMillis() - 1000 + l2.ExpiresAt = model.GetMillis() - 100 + if ok := th.App.SetLicense(l2); ok { + t.Fatal("license should have failed") + } + + l3 := &model.License{} + l3.Features = &model.Features{} + l3.Customer = &model.Customer{} + l3.StartsAt = model.GetMillis() + 10000 + l3.ExpiresAt = model.GetMillis() + 100000 + if ok := th.App.SetLicense(l3); !ok { + t.Fatal("license should have passed") + } +} + +func TestClientLicenseEtag(t *testing.T) { + th := Setup() + defer th.TearDown() + + etag1 := th.App.GetClientLicenseEtag(false) + + th.App.SetClientLicense(map[string]string{"SomeFeature": "true", "IsLicensed": "true"}) + + etag2 := th.App.GetClientLicenseEtag(false) + if etag1 == etag2 { + t.Fatal("etags should not match") + } + + th.App.SetClientLicense(map[string]string{"SomeFeature": "true", "IsLicensed": "false"}) + + etag3 := th.App.GetClientLicenseEtag(false) + if etag2 == etag3 { + t.Fatal("etags should not match") + } +} + +func TestGetSanitizedClientLicense(t *testing.T) { + th := Setup() + defer th.TearDown() + + l1 := &model.License{} + l1.Features = &model.Features{} + l1.Customer = &model.Customer{} + l1.Customer.Name = "TestName" + l1.StartsAt = model.GetMillis() - 1000 + l1.ExpiresAt = model.GetMillis() + 100000 + th.App.SetLicense(l1) + + m := th.App.GetSanitizedClientLicense() + + if _, ok := m["Name"]; ok { + t.Fatal("should have been sanatized") + } +} diff --git a/app/role.go b/app/role.go index 5f39dd623..9f271ea7a 100644 --- a/app/role.go +++ b/app/role.go @@ -12,8 +12,6 @@ func (a *App) Role(id string) *model.Role { return a.roles[id] } -// Updates the roles based on the app config and the global license check. You may need to invoke -// this when license changes are made. -func (a *App) SetDefaultRolesBasedOnConfig() { - a.roles = utils.DefaultRolesBasedOnConfig(a.Config()) +func (a *App) setDefaultRolesBasedOnConfig() { + a.roles = utils.DefaultRolesBasedOnConfig(a.Config(), a.License() != nil) } diff --git a/app/session_test.go b/app/session_test.go index 09d81f4ac..bf8198a4e 100644 --- a/app/session_test.go +++ b/app/session_test.go @@ -6,11 +6,10 @@ package app import ( "testing" - "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost-server/model" ) func TestCache(t *testing.T) { @@ -111,7 +110,7 @@ func TestGetSessionIdleTimeoutInMinutes(t *testing.T) { assert.Nil(t, err) // Test regular session with license off, should not timeout - *utils.License().Features.Compliance = false + th.App.SetLicense(nil) session = &model.Session{ UserId: model.NewId(), @@ -125,7 +124,7 @@ func TestGetSessionIdleTimeoutInMinutes(t *testing.T) { _, err = th.App.GetSession(session.Token) assert.Nil(t, err) - *utils.License().Features.Compliance = true + th.App.SetLicense(model.NewTestLicense("compliance")) // Test regular session with timeout set to 0, should not timeout th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SessionIdleTimeoutInMinutes = 0 }) -- cgit v1.2.3-1-g7c22 From 396e7513ecc7d86b04e56745586c802e56e5d763 Mon Sep 17 00:00:00 2001 From: Chris Date: Fri, 9 Feb 2018 15:41:06 -0600 Subject: Don't proxy same-site image urls (#8238) * don't proxy same-site urls * fix empty site url case --- app/post.go | 8 ++++++-- app/post_test.go | 10 ++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/post.go b/app/post.go index 005624605..5b0e59b23 100644 --- a/app/post.go +++ b/app/post.go @@ -876,6 +876,10 @@ func (a *App) imageProxyConfig() (proxyType, proxyURL, options, siteURL string) proxyURL += "/" } + if siteURL == "" || siteURL[len(siteURL)-1] != '/' { + siteURL += "/" + } + if cfg.ServiceSettings.ImageProxyOptions != nil { options = *cfg.ServiceSettings.ImageProxyOptions } @@ -890,12 +894,12 @@ func (a *App) ImageProxyAdder() func(string) string { } return func(url string) string { - if url == "" || strings.HasPrefix(url, proxyURL) { + if url == "" || strings.HasPrefix(url, siteURL) || strings.HasPrefix(url, proxyURL) { return url } if url[0] == '/' { - url = siteURL + url + url = siteURL + url[1:] } switch proxyType { diff --git a/app/post_test.go b/app/post_test.go index 3f3783265..e09d3a198 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -190,6 +190,10 @@ func TestImageProxy(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = "http://mymattermost.com" + }) + for name, tc := range map[string]struct { ProxyType string ProxyURL string @@ -211,6 +215,12 @@ func TestImageProxy(t *testing.T) { ImageURL: "http://mydomain.com/myimage", ProxiedImageURL: "https://127.0.0.1/x1000/http://mydomain.com/myimage", }, + "willnorris/imageproxy_SameSite": { + ProxyType: "willnorris/imageproxy", + ProxyURL: "https://127.0.0.1", + ImageURL: "http://mymattermost.com/myimage", + ProxiedImageURL: "http://mymattermost.com/myimage", + }, "willnorris/imageproxy_EmptyImageURL": { ProxyType: "willnorris/imageproxy", ProxyURL: "https://127.0.0.1", -- cgit v1.2.3-1-g7c22 From c1b6e8792c9f91c66c35737438c20757ef43066f Mon Sep 17 00:00:00 2001 From: Christopher Brown Date: Fri, 9 Feb 2018 20:08:39 -0600 Subject: minor addition to #8238 --- app/post.go | 6 +----- app/post_test.go | 6 ++++++ 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'app') diff --git a/app/post.go b/app/post.go index 5b0e59b23..be9374e10 100644 --- a/app/post.go +++ b/app/post.go @@ -894,14 +894,10 @@ func (a *App) ImageProxyAdder() func(string) string { } return func(url string) string { - if url == "" || strings.HasPrefix(url, siteURL) || strings.HasPrefix(url, proxyURL) { + if url == "" || url[0] == '/' || strings.HasPrefix(url, siteURL) || strings.HasPrefix(url, proxyURL) { return url } - if url[0] == '/' { - url = siteURL + url[1:] - } - switch proxyType { case "atmos/camo": mac := hmac.New(sha1.New, []byte(options)) diff --git a/app/post_test.go b/app/post_test.go index e09d3a198..409bc043d 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -221,6 +221,12 @@ func TestImageProxy(t *testing.T) { ImageURL: "http://mymattermost.com/myimage", ProxiedImageURL: "http://mymattermost.com/myimage", }, + "willnorris/imageproxy_PathOnly": { + ProxyType: "willnorris/imageproxy", + ProxyURL: "https://127.0.0.1", + ImageURL: "/myimage", + ProxiedImageURL: "/myimage", + }, "willnorris/imageproxy_EmptyImageURL": { ProxyType: "willnorris/imageproxy", ProxyURL: "https://127.0.0.1", -- cgit v1.2.3-1-g7c22 From 9707ac3aaf2cb4352c573aadf54b8535e237dd9e Mon Sep 17 00:00:00 2001 From: Jonathan Date: Mon, 12 Feb 2018 09:16:17 -0500 Subject: Added invite_id field to email invite url, along with validation of this field on the server (#8235) --- app/email.go | 1 + app/team.go | 5 +++++ app/team_test.go | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+) (limited to 'app') diff --git a/app/email.go b/app/email.go index b809b972d..764dc017a 100644 --- a/app/email.go +++ b/app/email.go @@ -276,6 +276,7 @@ func (a *App) SendInviteEmails(team *model.Team, senderName string, invites []st props["display_name"] = team.DisplayName props["name"] = team.Name props["time"] = fmt.Sprintf("%v", model.GetMillis()) + props["invite_id"] = team.InviteId data := model.MapToJson(props) hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, a.Config().EmailSettings.InviteSalt)) bodyPage.Props["Link"] = fmt.Sprintf("%s/signup_user_complete/?d=%s&h=%s", siteURL, url.QueryEscape(data), url.QueryEscape(hash)) diff --git a/app/team.go b/app/team.go index 21b8e5879..8e8c29e2a 100644 --- a/app/team.go +++ b/app/team.go @@ -234,6 +234,11 @@ func (a *App) AddUserToTeamByHash(userId string, hash string, data string) (*mod team = result.Data.(*model.Team) } + // verify that the team's invite id hasn't been changed since the invite was sent + if team.InviteId != props["invite_id"] { + return nil, model.NewAppError("JoinUserToTeamByHash", "api.user.create_user.signup_link_mismatched_invite_id.app_error", nil, "", http.StatusBadRequest) + } + var user *model.User if result := <-uchan; result.Err != nil { return nil, result.Err diff --git a/app/team_test.go b/app/team_test.go index 084558fb4..7cb20b6f6 100644 --- a/app/team_test.go +++ b/app/team_test.go @@ -7,7 +7,15 @@ import ( "strings" "testing" + "fmt" + + "sync/atomic" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" + "github.com/mattermost/mattermost-server/store/storetest" + "github.com/mattermost/mattermost-server/utils" + "github.com/stretchr/testify/assert" ) func TestCreateTeam(t *testing.T) { @@ -393,3 +401,62 @@ func TestSanitizeTeams(t *testing.T) { } }) } + +func TestAddUserToTeamByHashMismatchedInviteId(t *testing.T) { + mockStore := &storetest.Store{} + defer mockStore.AssertExpectations(t) + + teamId := model.NewId() + userId := model.NewId() + inviteSalt := model.NewId() + + inviteId := model.NewId() + teamInviteId := model.NewId() + + // generate a fake email invite - stolen from SendInviteEmails() in email.go + props := make(map[string]string) + props["email"] = model.NewId() + "@mattermost.com" + props["id"] = teamId + props["display_name"] = model.NewId() + props["name"] = model.NewId() + props["time"] = fmt.Sprintf("%v", model.GetMillis()) + props["invite_id"] = inviteId + data := model.MapToJson(props) + hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, inviteSalt)) + + // when the server tries to validate the invite, it will pull the user from our mock store + // this can return nil, because we'll fail before we get to trying to use it + mockStore.UserStore.On("Get", userId).Return( + storetest.NewStoreChannel(store.StoreResult{ + Data: nil, + Err: nil, + }), + ) + + // the server will also pull the team. the one we return has a different invite id than the one in the email invite we made above + mockStore.TeamStore.On("Get", teamId).Return( + storetest.NewStoreChannel(store.StoreResult{ + Data: &model.Team{ + InviteId: teamInviteId, + }, + Err: nil, + }), + ) + + app := App{ + Srv: &Server{ + Store: mockStore, + }, + config: atomic.Value{}, + } + app.config.Store(&model.Config{ + EmailSettings: model.EmailSettings{ + InviteSalt: inviteSalt, + }, + }) + + // this should fail because the invite ids are mismatched + team, err := app.AddUserToTeamByHash(userId, hash, data) + assert.Nil(t, team) + assert.Equal(t, "api.user.create_user.signup_link_mismatched_invite_id.app_error", err.Id) +} -- cgit v1.2.3-1-g7c22 From 07fd7aeeb8eb2b198b01b713a4ab57f6352faef2 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 12 Feb 2018 22:16:32 +0530 Subject: Add tests for the `platform server` command (#8231) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Cleanup app state on initialization error When returning an initialization error, the app state was not cleaned up. This is especially visible during tests, as `appCount` is not decremented, and makes the new app initialization fail. * Test the `platform server` command As the `platform server` command only exits when interrupted by a signal, it is not possible to test it as the other cobra commands. Instead we directly test the actual command function. The internal command handler is slighly refactored to take a channel in argument, and registers it as the signal handler. Nothing very different—except than controlling this channel from the outside allows the test to send the system signal itself, thus preventing the server to run forever. --- app/app.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/app.go b/app/app.go index dd5deb342..636f0a428 100644 --- a/app/app.go +++ b/app/app.go @@ -86,7 +86,7 @@ var appCount = 0 // New creates a new App. You must call Shutdown when you're done with it. // XXX: For now, only one at a time is allowed as some resources are still shared. -func New(options ...Option) (*App, error) { +func New(options ...Option) (outApp *App, outErr error) { appCount++ if appCount > 1 { panic("Only one App should exist at a time. Did you forget to call Shutdown()?") @@ -103,6 +103,11 @@ func New(options ...Option) (*App, error) { clientConfig: make(map[string]string), licenseListeners: map[string]func(){}, } + defer func() { + if outErr != nil { + app.Shutdown() + } + }() for _, option := range options { option(app) @@ -182,7 +187,9 @@ func (a *App) Shutdown() { a.ShutDownPlugins() a.WaitForGoroutines() - a.Srv.Store.Close() + if a.Srv.Store != nil { + a.Srv.Store.Close() + } a.Srv = nil if a.htmlTemplateWatcher != nil { -- cgit v1.2.3-1-g7c22