From 8ead10effba729f9170af971635276595c7e8c3d Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Mon, 14 Jan 2019 16:54:03 -0500 Subject: [PATCH] MM-13276: expose Websocket(URL|(Secure)Port) in limited client config (#10110) This fixes a race condition client-side that fails to connect to websockets during MFA enforcement since the necessary config data isn't fetched. There are no security concerns in exposing this data to non-authenticated users, though we'd like to revisit this to tighten it down later: https://mattermost.atlassian.net/browse/MM-13785. --- utils/config.go | 7 ++--- utils/config_test.go | 75 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/utils/config.go b/utils/config.go index 966dba347a..065eef52d2 100644 --- a/utils/config.go +++ b/utils/config.go @@ -428,7 +428,6 @@ func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.L props := GenerateLimitedClientConfig(c, diagnosticId, license) props["SiteURL"] = strings.TrimRight(*c.ServiceSettings.SiteURL, "/") - props["WebsocketURL"] = strings.TrimRight(*c.ServiceSettings.WebsocketURL, "/") props["EnableUserDeactivation"] = strconv.FormatBool(*c.TeamSettings.EnableUserDeactivation) props["RestrictDirectMessage"] = *c.TeamSettings.RestrictDirectMessage props["EnableXToLeaveChannelsFromLHS"] = strconv.FormatBool(*c.TeamSettings.EnableXToLeaveChannelsFromLHS) @@ -476,9 +475,6 @@ func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.L props["EnableFileAttachments"] = strconv.FormatBool(*c.FileSettings.EnableFileAttachments) props["EnablePublicLink"] = strconv.FormatBool(c.FileSettings.EnablePublicLink) - props["WebsocketPort"] = fmt.Sprintf("%v", *c.ServiceSettings.WebsocketPort) - props["WebsocketSecurePort"] = fmt.Sprintf("%v", *c.ServiceSettings.WebsocketSecurePort) - props["AvailableLocales"] = *c.LocalizationSettings.AvailableLocales props["SQLDriverName"] = *c.SqlSettings.DriverName @@ -610,6 +606,9 @@ func GenerateLimitedClientConfig(c *model.Config, diagnosticId string, license * props["BuildEnterpriseReady"] = model.BuildEnterpriseReady props["SiteName"] = c.TeamSettings.SiteName + props["WebsocketURL"] = strings.TrimRight(*c.ServiceSettings.WebsocketURL, "/") + props["WebsocketPort"] = fmt.Sprintf("%v", *c.ServiceSettings.WebsocketPort) + props["WebsocketSecurePort"] = fmt.Sprintf("%v", *c.ServiceSettings.WebsocketSecurePort) props["EnableUserCreation"] = strconv.FormatBool(*c.TeamSettings.EnableUserCreation) props["EnableOpenServer"] = strconv.FormatBool(*c.TeamSettings.EnableOpenServer) diff --git a/utils/config_test.go b/utils/config_test.go index 5dec73b549..bc0a63b22e 100644 --- a/utils/config_test.go +++ b/utils/config_test.go @@ -483,6 +483,11 @@ func TestGetClientConfig(t *testing.T) { // Ignored, since not licensed. AllowCustomThemes: bToP(false), }, + ServiceSettings: model.ServiceSettings{ + WebsocketURL: sToP("ws://mattermost.example.com:8065"), + WebsocketPort: iToP(80), + WebsocketSecurePort: iToP(443), + }, }, "", nil, @@ -491,6 +496,9 @@ func TestGetClientConfig(t *testing.T) { "EmailNotificationContentsType": "full", "AllowCustomThemes": "true", "EnforceMultifactorAuthentication": "false", + "WebsocketURL": "ws://mattermost.example.com:8065", + "WebsocketPort": "80", + "WebsocketSecurePort": "443", }, }, { @@ -570,8 +578,67 @@ func TestGetClientConfig(t *testing.T) { configMap := GenerateClientConfig(testCase.config, testCase.diagnosticId, testCase.license) for expectedField, expectedValue := range testCase.expectedFields { actualValue, ok := configMap[expectedField] - assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) - assert.Equal(t, expectedValue, actualValue) + if assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) { + assert.Equal(t, expectedValue, actualValue) + } + } + }) + } +} + +func TestGetLimitedClientConfig(t *testing.T) { + t.Parallel() + testCases := []struct { + description string + config *model.Config + diagnosticId string + license *model.License + expectedFields map[string]string + }{ + { + "unlicensed", + &model.Config{ + EmailSettings: model.EmailSettings{ + EmailNotificationContentsType: sToP(model.EMAIL_NOTIFICATION_CONTENTS_FULL), + }, + ThemeSettings: model.ThemeSettings{ + // Ignored, since not licensed. + AllowCustomThemes: bToP(false), + }, + ServiceSettings: model.ServiceSettings{ + WebsocketURL: sToP("ws://mattermost.example.com:8065"), + WebsocketPort: iToP(80), + WebsocketSecurePort: iToP(443), + }, + }, + "", + nil, + map[string]string{ + "DiagnosticId": "", + "EnforceMultifactorAuthentication": "false", + "WebsocketURL": "ws://mattermost.example.com:8065", + "WebsocketPort": "80", + "WebsocketSecurePort": "443", + }, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.description, func(t *testing.T) { + t.Parallel() + + testCase.config.SetDefaults() + if testCase.license != nil { + testCase.license.Features.SetDefaults() + } + + configMap := GenerateLimitedClientConfig(testCase.config, testCase.diagnosticId, testCase.license) + for expectedField, expectedValue := range testCase.expectedFields { + actualValue, ok := configMap[expectedField] + if assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) { + assert.Equal(t, expectedValue, actualValue) + } } }) } @@ -585,6 +652,10 @@ func bToP(b bool) *bool { return &b } +func iToP(i int) *int { + return &i +} + func TestGetDefaultsFromStruct(t *testing.T) { s := struct { TestSettings struct {