Set a proper HTTP user-agent header (#9482)

Previously, mattermost-server would always request with the default
user-agent of Go's net/http package that is `Go-http-client/1.1` or
something similar.
This has several disadvantages, one is that the default user-agent
made it pretty hard to distinguish mattermost requests from other
service requests in a network log for example.

Now a user-agent of the form `mattermost-<current-version>` is set in
the client.

- [x] Added or updated unit tests (required for all new features)

app/post.go

@@ -16,12 +16,14 @@ import (
 	"strings"
 
 	"github.com/dyatlov/go-opengraph/opengraph"
+	"golang.org/x/net/html/charset"
+
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/plugin"
+	"github.com/mattermost/mattermost-server/services/httpservice"
 	"github.com/mattermost/mattermost-server/store"
 	"github.com/mattermost/mattermost-server/utils"
-	"golang.org/x/net/html/charset"
 )
 
 func (a *App) CreatePostAsUser(post *model.Post, clearPushNotifications bool) (*model.Post, *model.AppError) {
@@ -919,7 +921,7 @@ func (a *App) DoPostAction(postId, actionId, userId, selectedOption string) *mod
 	req.Header.Set("Accept", "application/json")
 
 	// Allow access to plugin routes for action buttons
-	var httpClient *http.Client
+	var httpClient *httpservice.Client
 	url, _ := url.Parse(action.Integration.URL)
 	siteURL, _ := url.Parse(*a.Config().ServiceSettings.SiteURL)
 	subpath, _ := utils.GetSubpathFromConfig(a.Config())

services/httpservice/client.go

@@ -10,6 +10,8 @@ import (
 	"net"
 	"net/http"
 	"time"
+
+	"github.com/mattermost/mattermost-server/model"
 )
 
 const (
@@ -28,6 +30,8 @@ func IsReservedIP(ip net.IP) bool {
 	return false
 }
 
+var defaultUserAgent string
+
 func init() {
 	for _, cidr := range []string{
 		// See https://tools.ietf.org/html/rfc6890
@@ -48,6 +52,7 @@ func init() {
 		}
 		reservedIPRanges = append(reservedIPRanges, parsed)
 	}
+	defaultUserAgent = "mattermost-" + model.CurrentVersion
 }
 
 type DialContextFunction func(ctx context.Context, network, addr string) (net.Conn, error)
@@ -97,6 +102,15 @@ func dialContextFilter(dial DialContextFunction, allowHost func(host string) boo
 	}
 }
 
+type Client struct {
+	*http.Client
+}
+
+func (c *Client) Do(req *http.Request) (*http.Response, error) {
+	req.Header.Set("User-Agent", defaultUserAgent)
+	return c.Client.Do(req)
+}
+
 // NewHTTPClient returns a variation the default implementation of Client.
 // It uses a Transport with the same settings as the default Transport
 // but with the following modifications:
@@ -104,7 +118,7 @@ func dialContextFilter(dial DialContextFunction, allowHost func(host string) boo
 //   "connectTimeout")
 // - timeout for the end-to-end request (defined as constant
 //   "requestTimeout")
-func NewHTTPClient(enableInsecureConnections bool, allowHost func(host string) bool, allowIP func(ip net.IP) bool) *http.Client {
+func NewHTTPClient(enableInsecureConnections bool, allowHost func(host string) bool, allowIP func(ip net.IP) bool) *Client {
 	dialContext := (&net.Dialer{
 		Timeout:   connectTimeout,
 		KeepAlive: 30 * time.Second,
@@ -129,5 +143,5 @@ func NewHTTPClient(enableInsecureConnections bool, allowHost func(host string) b
 		Timeout: requestTimeout,
 	}
 
-	return client
+	return &Client{Client: client}
 }

services/httpservice/client_test.go

@@ -118,3 +118,24 @@ func TestDialContextFilter(t *testing.T) {
 		}
 	}
 }
+
+func TestUserAgentIsSet(t *testing.T) {
+	testUserAgent := "test-user-agent"
+	defaultUserAgent = testUserAgent
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		ua := req.UserAgent()
+		if ua == "" {
+			t.Error("expected user-agent to be non-empty")
+		}
+		if ua != testUserAgent {
+			t.Errorf("expected user-agent to be %q but was %q", testUserAgent, ua)
+		}
+	}))
+	defer ts.Close()
+	client := NewHTTPClient(true, nil, nil)
+	req, err := http.NewRequest("GET", ts.URL, nil)
+	if err != nil {
+		t.Fatal("NewRequest failed", err)
+	}
+	client.Do(req)
+}

services/httpservice/httpservice.go

@@ -5,7 +5,6 @@ package httpservice
 
 import (
 	"net"
-	"net/http"
 	"strings"
 
 	"github.com/mattermost/mattermost-server/services/configservice"
@@ -13,7 +12,7 @@ import (
 
 // Wraps the functionality for creating a new http.Client to encapsulate that and allow it to be mocked when testing
 type HTTPService interface {
-	MakeClient(trustURLs bool) *http.Client
+	MakeClient(trustURLs bool) *Client
 	Close()
 }
 
@@ -25,7 +24,7 @@ func MakeHTTPService(configService configservice.ConfigService) HTTPService {
 	return &HTTPServiceImpl{configService}
 }
 
-func (h *HTTPServiceImpl) MakeClient(trustURLs bool) *http.Client {
+func (h *HTTPServiceImpl) MakeClient(trustURLs bool) *Client {
 	insecure := h.configService.Config().ServiceSettings.EnableInsecureOutgoingConnections != nil && *h.configService.Config().ServiceSettings.EnableInsecureOutgoingConnections
 
 	if trustURLs {

utils/testutils/mocked_http_service.go

@@ -4,6 +4,7 @@
 package testutils
 
 import (
+	"github.com/mattermost/mattermost-server/services/httpservice"
 	"net/http"
 	"net/http/httptest"
 )
@@ -18,8 +19,8 @@ func MakeMockedHTTPService(handler http.Handler) *MockedHTTPService {
 	}
 }
 
-func (h *MockedHTTPService) MakeClient(trustURLs bool) *http.Client {
-	return h.Server.Client()
+func (h *MockedHTTPService) MakeClient(trustURLs bool) *httpservice.Client {
+	return &httpservice.Client{Client: h.Server.Client()}
 }
 
 func (h *MockedHTTPService) Close() {