diff --git a/Changelog.md b/Changelog.md index 2c13449d..d52222e0 100644 --- a/Changelog.md +++ b/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 diff --git a/Makefile b/Makefile index b70e59f7..0b3bddb2 100644 --- a/Makefile +++ b/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. diff --git a/cmd/Desktop-Bridge/main.go b/cmd/Desktop-Bridge/main.go index 66b1c942..d2f8ab11 100644 --- a/cmd/Desktop-Bridge/main.go +++ b/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() diff --git a/internal/bridge/bridge.go b/internal/bridge/bridge.go index ba6e8cdc..1977ba4e 100644 --- a/internal/bridge/bridge.go +++ b/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 { diff --git a/internal/bridge/bridge_test_exports.go b/internal/bridge/bridge_test_exports.go index 48c62008..9a81b887 100644 --- a/internal/bridge/bridge_test_exports.go +++ b/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 . + package bridge // IsAuthorized returns whether the user has received an Auth from the API yet. diff --git a/internal/bridge/mocks/mocks.go b/internal/bridge/mocks/mocks.go index dad77b09..7d7540f1 100644 --- a/internal/bridge/mocks/mocks.go +++ b/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 diff --git a/internal/bridge/types.go b/internal/bridge/types.go index 3cbf3669..d7533673 100644 --- a/internal/bridge/types.go +++ b/internal/bridge/types.go @@ -60,4 +60,5 @@ type ClientManager interface { DisallowProxy() GetAuthUpdateChannel() chan pmapi.ClientAuth CheckConnection() error + SetUserAgent(clientName, clientVersion, os string) } diff --git a/internal/imap/server.go b/internal/imap/server.go index 3bca832d..b2056d69 100644 --- a/internal/imap/server.go +++ b/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() { - imapBackend.setLastMailClient(id.ID()) - return + // 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()) + break + } + if time.Since(start) > 10*time.Second { + break + } + time.Sleep(100 * time.Millisecond) + } + }() } } }) diff --git a/internal/store/mocks/mocks.go b/internal/store/mocks/mocks.go index 8fd5c2d6..467863be 100644 --- a/internal/store/mocks/mocks.go +++ b/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 diff --git a/internal/store/mocks/utils_mocks.go b/internal/store/mocks/utils_mocks.go index 940bf172..3f43bb43 100644 --- a/internal/store/mocks/utils_mocks.go +++ b/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 diff --git a/pkg/config/logs.go b/pkg/config/logs.go index ddc4e4dc..9f0d299b 100644 --- a/pkg/config/logs.go +++ b/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) } } diff --git a/pkg/config/pmapi_prod.go b/pkg/config/pmapi_prod.go index 5107d8e1..38cd22a6 100644 --- a/pkg/config/pmapi_prod.go +++ b/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) diff --git a/pkg/pmapi/client.go b/pkg/pmapi/client.go index 4d69f764..119b1d3d 100644 --- a/pkg/pmapi/client.go +++ b/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)) diff --git a/pkg/pmapi/clientmanager.go b/pkg/pmapi/clientmanager.go index 853f3702..e74c02d1 100644 --- a/pkg/pmapi/clientmanager.go +++ b/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 . + 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 { diff --git a/pkg/pmapi/config.go b/pkg/pmapi/config.go index 522d3697..2901f9f1 100644 --- a/pkg/pmapi/config.go +++ b/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, diff --git a/pkg/pmapi/dialer_pinning.go b/pkg/pmapi/dialer_pinning.go index d31be145..25132f3b 100644 --- a/pkg/pmapi/dialer_pinning.go +++ b/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, ) } diff --git a/pkg/pmapi/mocks/mocks.go b/pkg/pmapi/mocks/mocks.go index be46bd1f..7fc2e8b4 100644 --- a/pkg/pmapi/mocks/mocks.go +++ b/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 diff --git a/pkg/pmapi/pin_checker.go b/pkg/pmapi/pin_checker.go index be8cc699..8798ffe6 100644 --- a/pkg/pmapi/pin_checker.go +++ b/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) { diff --git a/pkg/pmapi/req.go b/pkg/pmapi/req.go index b4ff9fde..82fa3c2e 100644 --- a/pkg/pmapi/req.go +++ b/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. diff --git a/pkg/pmapi/tlsreport.go b/pkg/pmapi/tlsreport.go index 058ecac9..2e57e0d4 100644 --- a/pkg/pmapi/tlsreport.go +++ b/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) diff --git a/internal/bridge/useragent.go b/pkg/pmapi/useragent.go similarity index 65% rename from internal/bridge/useragent.go rename to pkg/pmapi/useragent.go index 3ebd3a31..6c8f452f 100644 --- a/internal/bridge/useragent.go +++ b/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 . -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) } diff --git a/internal/bridge/useragent_test.go b/pkg/pmapi/useragent_test.go similarity index 62% rename from internal/bridge/useragent_test.go rename to pkg/pmapi/useragent_test.go index 560a93af..0ccf596b 100644 --- a/internal/bridge/useragent_test.go +++ b/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 . -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) } diff --git a/test/context/bridge.go b/test/context/bridge.go index fb04169e..8c9214dd 100644 --- a/test/context/bridge.go +++ b/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) }