Skip to content

Commit 7e5fda4

Browse files
committedJan 14, 2019
MM-13276: expose Websocket(URL|(Secure)Port) in limited client config (mattermost#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.
1 parent 3d01458 commit 7e5fda4

File tree

2 files changed

+76
-6
lines changed

2 files changed

+76
-6
lines changed
 

‎utils/config.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,6 @@ func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.L
515515
props := GenerateLimitedClientConfig(c, diagnosticId, license)
516516

517517
props["SiteURL"] = strings.TrimRight(*c.ServiceSettings.SiteURL, "/")
518-
props["WebsocketURL"] = strings.TrimRight(*c.ServiceSettings.WebsocketURL, "/")
519518
props["EnableUserDeactivation"] = strconv.FormatBool(*c.TeamSettings.EnableUserDeactivation)
520519
props["RestrictDirectMessage"] = *c.TeamSettings.RestrictDirectMessage
521520
props["EnableXToLeaveChannelsFromLHS"] = strconv.FormatBool(*c.TeamSettings.EnableXToLeaveChannelsFromLHS)
@@ -563,9 +562,6 @@ func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.L
563562
props["EnableFileAttachments"] = strconv.FormatBool(*c.FileSettings.EnableFileAttachments)
564563
props["EnablePublicLink"] = strconv.FormatBool(c.FileSettings.EnablePublicLink)
565564

566-
props["WebsocketPort"] = fmt.Sprintf("%v", *c.ServiceSettings.WebsocketPort)
567-
props["WebsocketSecurePort"] = fmt.Sprintf("%v", *c.ServiceSettings.WebsocketSecurePort)
568-
569565
props["AvailableLocales"] = *c.LocalizationSettings.AvailableLocales
570566
props["SQLDriverName"] = *c.SqlSettings.DriverName
571567

@@ -697,6 +693,9 @@ func GenerateLimitedClientConfig(c *model.Config, diagnosticId string, license *
697693
props["BuildEnterpriseReady"] = model.BuildEnterpriseReady
698694

699695
props["SiteName"] = c.TeamSettings.SiteName
696+
props["WebsocketURL"] = strings.TrimRight(*c.ServiceSettings.WebsocketURL, "/")
697+
props["WebsocketPort"] = fmt.Sprintf("%v", *c.ServiceSettings.WebsocketPort)
698+
props["WebsocketSecurePort"] = fmt.Sprintf("%v", *c.ServiceSettings.WebsocketSecurePort)
700699
props["EnableUserCreation"] = strconv.FormatBool(*c.TeamSettings.EnableUserCreation)
701700
props["EnableOpenServer"] = strconv.FormatBool(*c.TeamSettings.EnableOpenServer)
702701

‎utils/config_test.go

+73-2
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,11 @@ func TestGetClientConfig(t *testing.T) {
772772
// Ignored, since not licensed.
773773
AllowCustomThemes: bToP(false),
774774
},
775+
ServiceSettings: model.ServiceSettings{
776+
WebsocketURL: sToP("ws://mattermost.example.com:8065"),
777+
WebsocketPort: iToP(80),
778+
WebsocketSecurePort: iToP(443),
779+
},
775780
},
776781
"",
777782
nil,
@@ -780,6 +785,9 @@ func TestGetClientConfig(t *testing.T) {
780785
"EmailNotificationContentsType": "full",
781786
"AllowCustomThemes": "true",
782787
"EnforceMultifactorAuthentication": "false",
788+
"WebsocketURL": "ws://mattermost.example.com:8065",
789+
"WebsocketPort": "80",
790+
"WebsocketSecurePort": "443",
783791
},
784792
},
785793
{
@@ -859,8 +867,67 @@ func TestGetClientConfig(t *testing.T) {
859867
configMap := GenerateClientConfig(testCase.config, testCase.diagnosticId, testCase.license)
860868
for expectedField, expectedValue := range testCase.expectedFields {
861869
actualValue, ok := configMap[expectedField]
862-
assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField))
863-
assert.Equal(t, expectedValue, actualValue)
870+
if assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) {
871+
assert.Equal(t, expectedValue, actualValue)
872+
}
873+
}
874+
})
875+
}
876+
}
877+
878+
func TestGetLimitedClientConfig(t *testing.T) {
879+
t.Parallel()
880+
testCases := []struct {
881+
description string
882+
config *model.Config
883+
diagnosticId string
884+
license *model.License
885+
expectedFields map[string]string
886+
}{
887+
{
888+
"unlicensed",
889+
&model.Config{
890+
EmailSettings: model.EmailSettings{
891+
EmailNotificationContentsType: sToP(model.EMAIL_NOTIFICATION_CONTENTS_FULL),
892+
},
893+
ThemeSettings: model.ThemeSettings{
894+
// Ignored, since not licensed.
895+
AllowCustomThemes: bToP(false),
896+
},
897+
ServiceSettings: model.ServiceSettings{
898+
WebsocketURL: sToP("ws://mattermost.example.com:8065"),
899+
WebsocketPort: iToP(80),
900+
WebsocketSecurePort: iToP(443),
901+
},
902+
},
903+
"",
904+
nil,
905+
map[string]string{
906+
"DiagnosticId": "",
907+
"EnforceMultifactorAuthentication": "false",
908+
"WebsocketURL": "ws://mattermost.example.com:8065",
909+
"WebsocketPort": "80",
910+
"WebsocketSecurePort": "443",
911+
},
912+
},
913+
}
914+
915+
for _, testCase := range testCases {
916+
testCase := testCase
917+
t.Run(testCase.description, func(t *testing.T) {
918+
t.Parallel()
919+
920+
testCase.config.SetDefaults()
921+
if testCase.license != nil {
922+
testCase.license.Features.SetDefaults()
923+
}
924+
925+
configMap := GenerateLimitedClientConfig(testCase.config, testCase.diagnosticId, testCase.license)
926+
for expectedField, expectedValue := range testCase.expectedFields {
927+
actualValue, ok := configMap[expectedField]
928+
if assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) {
929+
assert.Equal(t, expectedValue, actualValue)
930+
}
864931
}
865932
})
866933
}
@@ -874,6 +941,10 @@ func bToP(b bool) *bool {
874941
return &b
875942
}
876943

944+
func iToP(i int) *int {
945+
return &i
946+
}
947+
877948
func TestGetDefaultsFromStruct(t *testing.T) {
878949
s := struct {
879950
TestSettings struct {

0 commit comments

Comments
 (0)
Failed to load comments.