User Agent do not contain bridge version, only client in format

Changelog.md

@@ -10,6 +10,7 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 * More logs about event loop activity
 
 ### Changed
+* GODT-162 User Agent does not contain bridge version, only client in format `client name/client version (os)`
 * GODT-225 Do not send an EXISTS reposnse after EXPUNGE or when nothing changed (fixes rebuild of mailboxes in Outlook for Mac)
 * GODT-165 Optimization of RebuildMailboxes
 * GODT-282 Completely delete old draft instead moving to trash when user updates draft

Makefile

@@ -127,7 +127,9 @@ check-has-go:
 	@which go || (echo "Install Go-lang!" && exit 1)
 
 check-license:
-	find . -not -path "./vendor/*" -not -name "*mock*.go" -regextype posix-egrep -regex ".*\.go|.*\.qml" -exec grep -L "Copyright (c) 2020 Proton Technologies AG" {} \;
+	RESULT=`find . -not -path "./vendor/*" -not -name "*mock*.go" -regextype posix-egrep -regex ".*\.go|.*\.qml" -exec grep -L 'Copyright (c) 2020 Proton Technologies AG' {} \;` ;\
+	echo $${RESULT} ;\
+	[[ -z "$${RESULT}" ]]
 
 test: gofiles
 	@# Listing packages manually to not run Qt folder (which needs to run qtsetup first) and integration tests.

cmd/Desktop-Bridge/main.go

@@ -84,7 +84,6 @@ func main() {
 		log.WithError(err).Errorln("Can not setup sentry DSN")
 	}
 	raven.SetRelease(constants.Revision)
-	bridge.UpdateCurrentUserAgent(constants.Version, runtime.GOOS, "", "")
 
 	args.FilterProcessSerialNumberFromArgs()
 	filterRestartNumberFromArgs()

internal/bridge/bridge.go

@@ -526,14 +526,18 @@ func (b *Bridge) GetCurrentClient() string {
 func (b *Bridge) SetCurrentClient(clientName, clientVersion string) {
 	b.userAgentClientName = clientName
 	b.userAgentClientVersion = clientVersion
-	b.updateCurrentUserAgent()
+	b.updateUserAgent()
 }
 
 // SetCurrentOS updates OS and sets the user agent on pmapi. By default we use
 // `runtime.GOOS`, but this can be overridden in case of better detection.
 func (b *Bridge) SetCurrentOS(os string) {
 	b.userAgentOS = os
-	b.updateCurrentUserAgent()
+	b.updateUserAgent()
+}
+
+func (b *Bridge) updateUserAgent() {
+	b.clientManager.SetUserAgent(b.userAgentClientName, b.userAgentClientVersion, b.userAgentOS)
 }
 
 // GetIMAPUpdatesChannel sets the channel on which idle events should be sent.
@@ -568,10 +572,6 @@ func (b *Bridge) StopWatchers() {
 	close(b.stopAll)
 }
 
-func (b *Bridge) updateCurrentUserAgent() {
-	UpdateCurrentUserAgent(b.config.GetVersion(), b.userAgentOS, b.userAgentClientName, b.userAgentClientVersion)
-}
-
 // hasUser returns whether the bridge currently has a user with ID `id`.
 func (b *Bridge) hasUser(id string) (user *User, ok bool) {
 	for _, u := range b.users {

internal/bridge/bridge_test_exports.go

@@ -1,3 +1,20 @@
+// Copyright (c) 2020 Proton Technologies AG
+//
+// This file is part of ProtonMail Bridge.
+//
+// ProtonMail Bridge is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// ProtonMail Bridge is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
+
 package bridge
 
 // IsAuthorized returns whether the user has received an Auth from the API yet.

internal/bridge/mocks/mocks.go

@@ -5,11 +5,10 @@
 package mocks
 
 import (
-	reflect "reflect"
-
 	credentials "github.com/ProtonMail/proton-bridge/internal/bridge/credentials"
 	pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	gomock "github.com/golang/mock/gomock"
+	reflect "reflect"
 )
 
 // MockConfiger is a mock of Configer interface
@@ -320,6 +319,18 @@ func (mr *MockClientManagerMockRecorder) GetClient(arg0 interface{}) *gomock.Cal
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetClient", reflect.TypeOf((*MockClientManager)(nil).GetClient), arg0)
 }
 
+// SetUserAgent mocks base method
+func (m *MockClientManager) SetUserAgent(arg0, arg1, arg2 string) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "SetUserAgent", arg0, arg1, arg2)
+}
+
+// SetUserAgent indicates an expected call of SetUserAgent
+func (mr *MockClientManagerMockRecorder) SetUserAgent(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetUserAgent", reflect.TypeOf((*MockClientManager)(nil).SetUserAgent), arg0, arg1, arg2)
+}
+
 // MockCredentialsStorer is a mock of CredentialsStorer interface
 type MockCredentialsStorer struct {
 	ctrl     *gomock.Controller

internal/bridge/types.go

@@ -60,4 +60,5 @@ type ClientManager interface {
 	DisallowProxy()
 	GetAuthUpdateChannel() chan pmapi.ClientAuth
 	CheckConnection() error
+	SetUserAgent(clientName, clientVersion, os string)
 }

internal/imap/server.go

@@ -76,8 +76,21 @@ func NewIMAPServer(debugClient, debugServer bool, port int, tls *tls.Config, ima
 		conn.Server().ForEachConn(func(candidate imapserver.Conn) {
 			if id, ok := candidate.(imapid.Conn); ok {
 				if conn.Context() == candidate.Context() {
+					// ID is not available right at the beginning of the connection.
+					// Clients send ID quickly after AUTH. We need to wait for it.
+					go func() {
+						start := time.Now()
+						for {
+							if id.ID() != nil {
 								imapBackend.setLastMailClient(id.ID())
-					return
+								break
+							}
+							if time.Since(start) > 10*time.Second {
+								break
+							}
+							time.Sleep(100 * time.Millisecond)
+						}
+					}()
 				}
 			}
 		})

internal/store/mocks/mocks.go

@@ -5,10 +5,9 @@
 package mocks
 
 import (
-	reflect "reflect"
-
 	pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	gomock "github.com/golang/mock/gomock"
+	reflect "reflect"
 )
 
 // MockPanicHandler is a mock of PanicHandler interface

internal/store/mocks/utils_mocks.go

@@ -5,10 +5,9 @@
 package mocks
 
 import (
+	gomock "github.com/golang/mock/gomock"
 	reflect "reflect"
 	time "time"
-
-	gomock "github.com/golang/mock/gomock"
 )
 
 // MockListener is a mock of Listener interface

pkg/config/logs.go

@@ -31,7 +31,6 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	"github.com/ProtonMail/proton-bridge/pkg/sentry"
 	"github.com/sirupsen/logrus"
 )
@@ -60,9 +59,7 @@ var logCrashRgx = regexp.MustCompile("^v.*_crash_.*\\.log$") //nolint[gochecknog
 func HandlePanic(cfg *Config, output string) {
 	if !cfg.IsDevMode() {
 		apiCfg := cfg.GetAPIConfig()
-		clientID := apiCfg.ClientID
-		appVersion := apiCfg.AppVersion
-		if err := sentry.ReportSentryCrash(clientID, appVersion, pmapi.CurrentUserAgent, errors.New(output)); err != nil {
+		if err := sentry.ReportSentryCrash(apiCfg.ClientID, apiCfg.AppVersion, apiCfg.UserAgent, errors.New(output)); err != nil {
 			log.Error("Sentry crash report failed: ", err)
 		}
 	}

pkg/config/pmapi_prod.go

@@ -48,7 +48,7 @@ func (c *Config) GetRoundTripper(cm *pmapi.ClientManager, listener listener.List
 
 	// We want any pin mismatches to be communicated back to bridge GUI and reported.
 	pinningDialer.SetTLSIssueNotifier(func() { listener.Emit(events.TLSCertIssue, "") })
-	pinningDialer.EnableRemoteTLSIssueReporting(c.GetAPIConfig().AppVersion)
+	pinningDialer.EnableRemoteTLSIssueReporting(c.GetAPIConfig().AppVersion, c.GetAPIConfig().UserAgent)
 
 	// We wrap the pinning dialer in a layer which adds "alternative routing" feature.
 	proxyDialer := pmapi.NewProxyTLSDialer(pinningDialer, cm)

pkg/pmapi/client.go

@@ -79,6 +79,13 @@ type ClientConfig struct {
 	// The client application name and version.
 	AppVersion string
 
+	// The client application user agent in format `client name/client version (os)`, e.g.:
+	// (Intel Mac OS X 10_15_3)
+	// Mac OS X Mail/13.0 (3608.60.0.2.5) (Intel Mac OS X 10_15_3)
+	// Thunderbird/1.5.0 (Ubuntu 18.04.4 LTS)
+	// MSOffice 12 (Windows 10 (10.0))
+	UserAgent string
+
 	// The client ID.
 	ClientID string
 
@@ -213,6 +220,7 @@ func (c *client) Do(req *http.Request, retryUnauthorized bool) (res *http.Respon
 func (c *client) doBuffered(req *http.Request, bodyBuffer []byte, retryUnauthorized bool) (res *http.Response, err error) { // nolint[funlen]
 	isAuthReq := strings.Contains(req.URL.Path, "/auth")
 
+	req.Header.Set("User-Agent", c.cm.config.UserAgent)
 	req.Header.Set("x-pm-appversion", c.cm.config.AppVersion)
 	req.Header.Set("x-pm-apiversion", strconv.Itoa(Version))
 

pkg/pmapi/clientmanager.go

@@ -1,3 +1,20 @@
+// Copyright (c) 2020 Proton Technologies AG
+//
+// This file is part of ProtonMail Bridge.
+//
+// ProtonMail Bridge is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// ProtonMail Bridge is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
+
 package pmapi
 
 import (
@@ -96,6 +113,7 @@ func NewClientManager(config *ClientConfig) (cm *ClientManager) {
 	cm.newClient = func(userID string) Client {
 		return newClient(cm, userID)
 	}
+	cm.SetUserAgent("", "", "") // Set default user agent.
 
 	go cm.watchTokenExpirations()
 
@@ -113,6 +131,10 @@ func (cm *ClientManager) SetRoundTripper(rt http.RoundTripper) {
 	cm.roundTripper = rt
 }
 
+func (cm *ClientManager) SetUserAgent(clientName, clientVersion, os string) {
+	cm.config.UserAgent = formatUserAgent(clientName, clientVersion, os)
+}
+
 // GetClient returns a client for the given userID.
 // If the client does not exist already, it is created.
 func (cm *ClientManager) GetClient(userID string) Client {

pkg/pmapi/config.go

@@ -19,7 +19,6 @@ package pmapi
 
 import (
 	"net/http"
-	"runtime"
 )
 
 // rootURL is the API root URL.
@@ -31,11 +30,6 @@ import (
 var rootURL = "api.protonmail.ch" //nolint[gochecknoglobals]
 var rootScheme = "https"          //nolint[gochecknoglobals]
 
-// CurrentUserAgent is the default User-Agent for go-pmapi lib. This can be changed to program
-// version and email client.
-// e.g. Bridge/1.0.4 (Windows) MicrosoftOutlook/16.0.9330.2087
-var CurrentUserAgent = "GoPMAPI/1.0.14 (" + runtime.GOOS + "; no client)" //nolint[gochecknoglobals]
-
 // The HTTP transport to use by default.
 var defaultTransport = &http.Transport{ //nolint[gochecknoglobals]
 	Proxy: http.ProxyFromEnvironment,

pkg/pmapi/dialer_pinning.go

@@ -39,6 +39,9 @@ type PinningTLSDialer struct {
 	// appVersion is needed to report TLS mismatches.
 	appVersion string
 
+	// userAgent is needed to report TLS mismatches.
+	userAgent string
+
 	// enableRemoteReporting instructs the dialer to report TLS mismatches.
 	enableRemoteReporting bool
 
@@ -61,9 +64,10 @@ func (p *PinningTLSDialer) SetTLSIssueNotifier(notifier func()) {
 	p.tlsIssueNotifier = notifier
 }
 
-func (p *PinningTLSDialer) EnableRemoteTLSIssueReporting(appVersion string) {
+func (p *PinningTLSDialer) EnableRemoteTLSIssueReporting(appVersion, userAgent string) {
 	p.enableRemoteReporting = true
 	p.appVersion = appVersion
+	p.userAgent = userAgent
 }
 
 // DialTLS dials the given network/address, returning an error if the certificates don't match the trusted pins.
@@ -89,6 +93,7 @@ func (p *PinningTLSDialer) DialTLS(network, address string) (conn net.Conn, err
 				time.Now().Format(time.RFC3339),
 				tlsConn.ConnectionState(),
 				p.appVersion,
+				p.userAgent,
 			)
 		}
 

pkg/pmapi/mocks/mocks.go

@@ -5,12 +5,11 @@
 package mocks
 
 import (
-	io "io"
-	reflect "reflect"
-
 	crypto "github.com/ProtonMail/gopenpgp/crypto"
 	pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	gomock "github.com/golang/mock/gomock"
+	io "io"
+	reflect "reflect"
 )
 
 // MockClient is a mock of Client interface

pkg/pmapi/pin_checker.go

@@ -46,7 +46,7 @@ func certFingerprint(cert *x509.Certificate) string {
 }
 
 // ReportCertIssue reports a TLS key mismatch.
-func (p *PinChecker) ReportCertIssue(host, port, datetime string, connState tls.ConnectionState, appVersion string) {
+func (p *PinChecker) ReportCertIssue(host, port, datetime string, connState tls.ConnectionState, appVersion, userAgent string) {
 	var certChain []string
 
 	if len(connState.VerifiedChains) > 0 {
@@ -57,7 +57,7 @@ func (p *PinChecker) ReportCertIssue(host, port, datetime string, connState tls.
 
 	report := NewTLSReport(host, port, connState.ServerName, certChain, p.trustedPins, appVersion)
 
-	go postCertIssueReport(report)
+	go postCertIssueReport(report, userAgent)
 }
 
 func marshalCert7468(certs []*x509.Certificate) (pemCerts []string) {

pkg/pmapi/req.go

@@ -26,13 +26,8 @@ import (
 )
 
 // NewRequest creates a new request.
-func (c *client) NewRequest(method, path string, body io.Reader) (req *http.Request, err error) {
-	req, err = http.NewRequest(method, c.cm.GetRootURL()+path, body)
-
-	if req != nil {
-		req.Header.Set("User-Agent", CurrentUserAgent)
-	}
-	return
+func (c *client) NewRequest(method, path string, body io.Reader) (*http.Request, error) {
+	return http.NewRequest(method, c.cm.GetRootURL()+path, body)
 }
 
 // NewJSONRequest create a new JSON request.

pkg/pmapi/tlsreport.go

@@ -109,7 +109,7 @@ func NewTLSReport(host, port, server string, certChain, knownPins []string, appV
 }
 
 // postCertIssueReport posts the given TLS report to the standard TLS Report URI.
-func postCertIssueReport(report TLSReport) {
+func postCertIssueReport(report TLSReport, userAgent string) {
 	b, err := json.Marshal(report)
 	if err != nil {
 		logrus.WithError(err).Error("Failed to marshal TLS report")
@@ -123,7 +123,7 @@ func postCertIssueReport(report TLSReport) {
 	}
 
 	req.Header.Add("Content-Type", "application/json")
-	req.Header.Set("User-Agent", CurrentUserAgent)
+	req.Header.Set("User-Agent", userAgent)
 	req.Header.Set("x-pm-apiversion", strconv.Itoa(Version))
 	req.Header.Set("x-pm-appversion", report.AppVersion)
 

pkg/pmapi/useragent.go

@@ -15,27 +15,23 @@
 // You should have received a copy of the GNU General Public License
 // along with ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
 
-package bridge
+package pmapi
 
 import (
 	"fmt"
 	"runtime"
-
-	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
 )
 
-// UpdateCurrentUserAgent updates user agent on pmapi so each request has this
-// information in headers for statistic purposes.
-func UpdateCurrentUserAgent(bridgeVersion, os, clientName, clientVersion string) {
+func formatUserAgent(clientName, clientVersion, os string) string {
+	client := ""
+	if clientName != "" {
+		client = clientName
+		if clientVersion != "" {
+			client += "/" + clientVersion
+		}
+	}
 	if os == "" {
 		os = runtime.GOOS
 	}
-	mailClient := "unknown client"
-	if clientName != "" {
-		mailClient = clientName
-		if clientVersion != "" {
-			mailClient += "/" + clientVersion
-		}
-	}
-	pmapi.CurrentUserAgent = fmt.Sprintf("Bridge/%s (%s; %s)", bridgeVersion, os, mailClient)
+	return fmt.Sprintf("%s (%s)", client, os)
 }

pkg/pmapi/useragent_test.go

@@ -15,37 +15,36 @@
 // You should have received a copy of the GNU General Public License
 // along with ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
 
-package bridge
+package pmapi
 
 import (
 	"runtime"
 	"testing"
 
-	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestUpdateCurrentUserAgentGOOS(t *testing.T) {
-	UpdateCurrentUserAgent("ver", "", "", "")
-	assert.Equal(t, "Bridge/ver ("+runtime.GOOS+"; unknown client)", pmapi.CurrentUserAgent)
+	userAgent := formatUserAgent("", "", "")
+	assert.Equal(t, " ("+runtime.GOOS+")", userAgent)
 }
 
 func TestUpdateCurrentUserAgentOS(t *testing.T) {
-	UpdateCurrentUserAgent("ver", "os", "", "")
-	assert.Equal(t, "Bridge/ver (os; unknown client)", pmapi.CurrentUserAgent)
+	userAgent := formatUserAgent("", "", "os")
+	assert.Equal(t, " (os)", userAgent)
 }
 
 func TestUpdateCurrentUserAgentClientVer(t *testing.T) {
-	UpdateCurrentUserAgent("ver", "os", "", "cver")
-	assert.Equal(t, "Bridge/ver (os; unknown client)", pmapi.CurrentUserAgent)
+	userAgent := formatUserAgent("", "ver", "os")
+	assert.Equal(t, " (os)", userAgent)
 }
 
 func TestUpdateCurrentUserAgentClientName(t *testing.T) {
-	UpdateCurrentUserAgent("ver", "os", "mail", "")
-	assert.Equal(t, "Bridge/ver (os; mail)", pmapi.CurrentUserAgent)
+	userAgent := formatUserAgent("mail", "", "os")
+	assert.Equal(t, "mail (os)", userAgent)
 }
 
 func TestUpdateCurrentUserAgentClientNameAndVersion(t *testing.T) {
-	UpdateCurrentUserAgent("ver", "os", "mail", "cver")
-	assert.Equal(t, "Bridge/ver (os; mail/cver)", pmapi.CurrentUserAgent)
+	userAgent := formatUserAgent("mail", "ver", "os")
+	assert.Equal(t, "mail/ver (os)", userAgent)
 }

test/context/bridge.go

@@ -18,11 +18,8 @@
 package context
 
 import (
-	"runtime"
-
 	"github.com/ProtonMail/proton-bridge/internal/bridge"
 	"github.com/ProtonMail/proton-bridge/internal/preferences"
-	"github.com/ProtonMail/proton-bridge/pkg/constants"
 	"github.com/ProtonMail/proton-bridge/pkg/listener"
 )
 
@@ -64,11 +61,8 @@ func newBridgeInstance(
 	eventListener listener.Listener,
 	clientManager bridge.ClientManager,
 ) *bridge.Bridge {
-	bridge.UpdateCurrentUserAgent(constants.Version, runtime.GOOS, "", "")
-
 	panicHandler := &panicHandler{t: t}
 	pref := preferences.New(cfg)
-
 	return bridge.New(cfg, pref, panicHandler, eventListener, clientManager, credStore)
 }