MM-11931 Add support for AllowedUntrustedInternalConnections to be comma-separated (#11614)

* Add support for AllowedUntrustedInternalConnections to be comma-separated

* Add comprehensive test cases for fields splitting function

api4/command_test.go

@@ -627,7 +627,7 @@ func TestExecuteCommandAgainstChannelOnAnotherTeam(t *testing.T) {
 	}()
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true })
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	expectedCommandResponse := &model.CommandResponse{
@@ -677,7 +677,7 @@ func TestExecuteCommandAgainstChannelUserIsNotIn(t *testing.T) {
 	}()
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true })
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	expectedCommandResponse := &model.CommandResponse{
@@ -732,7 +732,7 @@ func TestExecuteCommandInDirectMessageChannel(t *testing.T) {
 	}()
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true })
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	// create a team that the user isn't a part of
@@ -792,7 +792,7 @@ func TestExecuteCommandInTeamUserIsNotOn(t *testing.T) {
 	}()
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true })
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	// create a team that the user isn't a part of

api4/integration_action_test.go

@@ -43,7 +43,7 @@ func TestPostActionCookies(t *testing.T) {
 	Client := th.Client
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	handler := &testHandler{t}
@@ -99,7 +99,7 @@ func TestOpenDialog(t *testing.T) {
 	Client := th.Client
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	_, triggerId, err := model.GenerateTriggerId(th.BasicUser.Id, th.App.AsymmetricSigningKey())
@@ -160,7 +160,7 @@ func TestSubmitDialog(t *testing.T) {
 	Client := th.Client
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	submit := model.SubmitDialogRequest{

api4/openGraph_test.go

@@ -30,7 +30,7 @@ func TestGetOpenGraphMetadata(t *testing.T) {
 	}()
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableLinkPreviews = true })
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	ogDataCacheMissCount := 0

api4/post_test.go

@@ -186,7 +186,7 @@ func testCreatePostWithOutgoingHook(
 
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOutgoingWebhooks = true })
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	var hook *model.OutgoingWebhook

app/integration_action_test.go

@@ -23,7 +23,7 @@ func TestPostActionInvalidURL(t *testing.T) {
 	defer th.TearDown()
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -72,7 +72,7 @@ func TestPostAction(t *testing.T) {
 	defer th.TearDown()
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -316,7 +316,7 @@ func TestPostActionProps(t *testing.T) {
 	defer th.TearDown()
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -399,7 +399,7 @@ func TestSubmitInteractiveDialog(t *testing.T) {
 	defer th.TearDown()
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 
 	submit := model.SubmitDialogRequest{

app/webhook_test.go

@@ -601,7 +601,7 @@ func TestTriggerOutGoingWebhookWithUsernameAndIconURL(t *testing.T) {
 	defer th.TearDown()
 
 	th.App.UpdateConfig(func(cfg *model.Config) {
-		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
 	})
 	createdPost := make(chan *model.Post)
 

services/httpservice/client_test.go

@@ -11,7 +11,10 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestHTTPClient(t *testing.T) {
@@ -186,3 +189,44 @@ func TestIsOwnIP(t *testing.T) {
 		})
 	}
 }
+
+func TestSplitHostnames(t *testing.T) {
+	var config string
+	var hostnames []string
+
+	config = ""
+	hostnames = strings.FieldsFunc(config, splitFields)
+	require.Equal(t, []string{}, hostnames)
+
+	config = "127.0.0.1 localhost"
+	hostnames = strings.FieldsFunc(config, splitFields)
+	require.Equal(t, []string{"127.0.0.1", "localhost"}, hostnames)
+
+	config = "127.0.0.1,localhost"
+	hostnames = strings.FieldsFunc(config, splitFields)
+	require.Equal(t, []string{"127.0.0.1", "localhost"}, hostnames)
+
+	config = "127.0.0.1,,localhost"
+	hostnames = strings.FieldsFunc(config, splitFields)
+	require.Equal(t, []string{"127.0.0.1", "localhost"}, hostnames)
+
+	config = "127.0.0.1  localhost"
+	hostnames = strings.FieldsFunc(config, splitFields)
+	require.Equal(t, []string{"127.0.0.1", "localhost"}, hostnames)
+
+	config = "127.0.0.1 , localhost"
+	hostnames = strings.FieldsFunc(config, splitFields)
+	require.Equal(t, []string{"127.0.0.1", "localhost"}, hostnames)
+
+	config = "127.0.0.1  localhost  "
+	hostnames = strings.FieldsFunc(config, splitFields)
+	require.Equal(t, []string{"127.0.0.1", "localhost"}, hostnames)
+
+	config = " 127.0.0.1  ,,localhost  , , ,,"
+	hostnames = strings.FieldsFunc(config, splitFields)
+	require.Equal(t, []string{"127.0.0.1", "localhost"}, hostnames)
+
+	config = "127.0.0.1 localhost, 192.168.1.0"
+	hostnames = strings.FieldsFunc(config, splitFields)
+	require.Equal(t, []string{"127.0.0.1", "localhost", "192.168.1.0"}, hostnames)
+}

services/httpservice/httpservice.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"strings"
 	"time"
+	"unicode"
 
 	"github.com/mattermost/mattermost-server/services/configservice"
 )
@@ -33,6 +34,10 @@ type HTTPServiceImpl struct {
 	RequestTimeout time.Duration
 }
 
+func splitFields(c rune) bool {
+	return unicode.IsSpace(c) || c == ','
+}
+
 func MakeHTTPService(configService configservice.ConfigService) HTTPService {
 	return &HTTPServiceImpl{
 		configService,
@@ -58,7 +63,7 @@ func (h *HTTPServiceImpl) MakeTransport(trustURLs bool) http.RoundTripper {
 		if h.configService.Config().ServiceSettings.AllowedUntrustedInternalConnections == nil {
 			return false
 		}
-		for _, allowed := range strings.Fields(*h.configService.Config().ServiceSettings.AllowedUntrustedInternalConnections) {
+		for _, allowed := range strings.FieldsFunc(*h.configService.Config().ServiceSettings.AllowedUntrustedInternalConnections, splitFields) {
 			if host == allowed {
 				return true
 			}
@@ -85,7 +90,7 @@ func (h *HTTPServiceImpl) MakeTransport(trustURLs bool) http.RoundTripper {
 		}
 
 		// In the case it's the self-assigned IP, enforce that it needs to be explicitly added to the AllowedUntrustedInternalConnections
-		for _, allowed := range strings.Fields(*h.configService.Config().ServiceSettings.AllowedUntrustedInternalConnections) {
+		for _, allowed := range strings.FieldsFunc(*h.configService.Config().ServiceSettings.AllowedUntrustedInternalConnections, splitFields) {
 			if _, ipRange, err := net.ParseCIDR(allowed); err == nil && ipRange.Contains(ip) {
 				return true
 			}