From 0432f995ec27de9ee6cc2f5847d4a17fcc095a26 Mon Sep 17 00:00:00 2001 From: Christopher Speller Date: Fri, 18 May 2018 08:37:43 -0700 Subject: MM-9983 Requiring SiteURL to be set. (#8769) * Requiring SiteURL to be set. * Modifying to make tests pass. * Fixing test. --- cmd/mattermost/commands/server.go | 16 +++++++++++++++- cmd/mattermost/commands/server_test.go | 10 +++++++++- config/default.json | 2 +- i18n/en.json | 2 +- model/config.go | 14 -------------- model/config_test.go | 7 +++++++ 6 files changed, 33 insertions(+), 18 deletions(-) diff --git a/cmd/mattermost/commands/server.go b/cmd/mattermost/commands/server.go index 9d0e5a917..299005b6a 100644 --- a/cmd/mattermost/commands/server.go +++ b/cmd/mattermost/commands/server.go @@ -6,6 +6,8 @@ package commands import ( "fmt" "net" + "net/http" + "net/url" "os" "os/signal" "syscall" @@ -129,7 +131,19 @@ func runServer(configFileLocation string, disableConfigWatch bool, usedPlatform // Enable developer settings if this is a "dev" build if model.BuildNumber == "dev" { - a.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableDeveloper = true }) + a.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableDeveloper = true + if *cfg.ServiceSettings.SiteURL == "" { + *cfg.ServiceSettings.SiteURL = "http://localhost:8065" + } + }) + } + + // SiteURL should be set at this point. Either by a user or by the dev mode above + // This is here instead of in config.IsValid because there are many tests that make the assumption + // that the default config is valid. Which it is not. + if _, err := url.ParseRequestURI(*a.Config().ServiceSettings.SiteURL); err != nil { + return model.NewAppError("Config.IsValid", "model.config.is_valid.site_url.app_error", nil, "", http.StatusBadRequest) } resetStatuses(a) diff --git a/cmd/mattermost/commands/server_test.go b/cmd/mattermost/commands/server_test.go index 0f825e316..a0c7c6948 100644 --- a/cmd/mattermost/commands/server_test.go +++ b/cmd/mattermost/commands/server_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/mattermost/mattermost-server/jobs" + "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" "github.com/stretchr/testify/require" ) @@ -20,6 +21,7 @@ type ServerTestHelper struct { disableConfigWatch bool interruptChan chan os.Signal originalInterval int + oldBuildNumber string } func SetupServerTest() *ServerTestHelper { @@ -41,14 +43,20 @@ func SetupServerTest() *ServerTestHelper { interruptChan: interruptChan, originalInterval: originalInterval, } + + // Run in dev mode so SiteURL gets set + th.oldBuildNumber = model.BuildNumber + model.BuildNumber = "dev" + return th } func (th *ServerTestHelper) TearDownServerTest() { jobs.DEFAULT_WATCHER_POLLING_INTERVAL = th.originalInterval + model.BuildNumber = th.oldBuildNumber } -func TestRunServerSuccess(t *testing.T) { +func TestRunServerSiteURL(t *testing.T) { th := SetupServerTest() defer th.TearDownServerTest() diff --git a/config/default.json b/config/default.json index 23bae64f9..97b2696aa 100644 --- a/config/default.json +++ b/config/default.json @@ -1,6 +1,6 @@ { "ServiceSettings": { - "SiteURL": "http://localhost:8065", + "SiteURL": "", "WebsocketURL": "", "LicenseFileLocation": "", "ListenAddress": ":8065", diff --git a/i18n/en.json b/i18n/en.json index f7c21a3eb..47bb743bb 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -5228,7 +5228,7 @@ }, { "id": "model.config.is_valid.site_url.app_error", - "translation": "Site URL must be a valid URL and start with http:// or https://" + "translation": "Site URL must be set, a valid URL, and start with http:// or https://" }, { "id": "model.config.is_valid.site_url_email_batching.app_error", diff --git a/model/config.go b/model/config.go index a54d44110..4710658ec 100644 --- a/model/config.go +++ b/model/config.go @@ -1870,18 +1870,10 @@ func (o *Config) SetDefaults() { } func (o *Config) IsValid() *AppError { - if len(*o.ServiceSettings.SiteURL) == 0 && *o.EmailSettings.EnableEmailBatching { - return NewAppError("Config.IsValid", "model.config.is_valid.site_url_email_batching.app_error", nil, "", http.StatusBadRequest) - } - if *o.ClusterSettings.Enable && *o.EmailSettings.EnableEmailBatching { return NewAppError("Config.IsValid", "model.config.is_valid.cluster_email_batching.app_error", nil, "", http.StatusBadRequest) } - if len(*o.ServiceSettings.SiteURL) == 0 && *o.ServiceSettings.AllowCookiesForSubdomains { - return NewAppError("Config.IsValid", "Allowing cookies for subdomains requires SiteURL to be set.", nil, "", http.StatusBadRequest) - } - if err := o.TeamSettings.isValid(); err != nil { return err } @@ -2187,12 +2179,6 @@ func (ss *ServiceSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.login_attempts.app_error", nil, "", http.StatusBadRequest) } - if len(*ss.SiteURL) != 0 { - if _, err := url.ParseRequestURI(*ss.SiteURL); err != nil { - return NewAppError("Config.IsValid", "model.config.is_valid.site_url.app_error", nil, "", http.StatusBadRequest) - } - } - if len(*ss.WebsocketURL) != 0 { if _, err := url.ParseRequestURI(*ss.WebsocketURL); err != nil { return NewAppError("Config.IsValid", "model.config.is_valid.websocket_url.app_error", nil, "", http.StatusBadRequest) diff --git a/model/config_test.go b/model/config_test.go index b7533145b..e39ecef3b 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -82,6 +82,13 @@ func TestConfigDefaultFileSettingsS3SSE(t *testing.T) { } } +func TestConfigDefaultSiteURL(t *testing.T) { + c1 := Config{} + c1.SetDefaults() + + assert.Equal(t, "", *c1.ServiceSettings.SiteURL, "SiteURL should be empty by default.") +} + func TestConfigDefaultServiceSettingsExperimentalGroupUnreadChannels(t *testing.T) { c1 := Config{} c1.SetDefaults() -- cgit v1.2.3-1-g7c22