From 1457005f86837b2786ee14879508fbf472ac9c93 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Tue, 21 Apr 2020 07:17:50 +0000 Subject: [PATCH] fix: address review comments --- cmd/Desktop-Bridge/main.go | 3 +- internal/api/api.go | 4 +- internal/bridge/bridge.go | 18 ++++----- internal/bridge/bridge_test_exports.go | 6 +++ internal/bridge/credentials/credentials.go | 3 +- internal/bridge/user.go | 6 +-- internal/frontend/cli/frontend.go | 4 +- internal/frontend/frontend.go | 4 +- internal/frontend/qt/frontend.go | 4 +- internal/frontend/qt/frontend_nogui.go | 4 +- internal/imap/imap.go | 4 +- internal/smtp/smtp.go | 4 +- internal/store/event_loop.go | 2 +- internal/store/store_test_exports.go | 3 +- pkg/config/config.go | 4 +- pkg/keychain/keychain.go | 4 +- pkg/listener/listener.go | 4 +- pkg/logs/logs.go | 10 ----- pkg/pmapi/auth.go | 4 +- pkg/pmapi/clientmanager.go | 37 ++++++++----------- pkg/pmapi/config.go | 4 +- pkg/pmapi/config_dev.go | 2 +- pkg/pmapi/config_local.go | 2 +- pkg/pmapi/dialer_with_proxy.go | 2 +- ...h_test_export.go => pmapi_test_exports.go} | 5 +++ pkg/pmapi/proxy.go | 6 +-- pkg/pmapi/proxy_test.go | 12 +++--- pkg/updates/updates.go | 4 +- test/bridge_checks_test.go | 19 ++++------ test/fakeapi/auth.go | 2 +- test/fakeapi/events.go | 2 +- 31 files changed, 88 insertions(+), 104 deletions(-) create mode 100644 internal/bridge/bridge_test_exports.go delete mode 100644 pkg/logs/logs.go rename pkg/pmapi/{auth_test_export.go => pmapi_test_exports.go} (84%) diff --git a/cmd/Desktop-Bridge/main.go b/cmd/Desktop-Bridge/main.go index e38b40ac..23d29f34 100644 --- a/cmd/Desktop-Bridge/main.go +++ b/cmd/Desktop-Bridge/main.go @@ -56,7 +56,6 @@ import ( "github.com/ProtonMail/proton-bridge/pkg/args" "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" - "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/ProtonMail/proton-bridge/pkg/pmapi" "github.com/ProtonMail/proton-bridge/pkg/updates" "github.com/allan-simon/go-singleinstance" @@ -87,7 +86,7 @@ var ( longVersion = Version + " (" + Revision + ")" //nolint[gochecknoglobals] buildVersion = longVersion + " " + BuildTime //nolint[gochecknoglobals] - log = logs.GetLogEntry("main") //nolint[gochecknoglobals] + log = logrus.WithField("pkg", "main") //nolint[gochecknoglobals] // How many crashes in a row. numberOfCrashes = 0 //nolint[gochecknoglobals] diff --git a/internal/api/api.go b/internal/api/api.go index d23fc379..9478ca07 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -31,12 +31,12 @@ import ( "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" - "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/ProtonMail/proton-bridge/pkg/ports" + "github.com/sirupsen/logrus" ) var ( - log = logs.GetLogEntry("api") //nolint[gochecknoglobals] + log = logrus.WithField("pkg", "api") //nolint[gochecknoglobals] ) type apiServer struct { diff --git a/internal/bridge/bridge.go b/internal/bridge/bridge.go index de990fc2..6b667b7e 100644 --- a/internal/bridge/bridge.go +++ b/internal/bridge/bridge.go @@ -29,7 +29,6 @@ import ( "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/internal/store" "github.com/ProtonMail/proton-bridge/pkg/listener" - "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/ProtonMail/proton-bridge/pkg/pmapi" "github.com/hashicorp/go-multierror" "github.com/pkg/errors" @@ -37,8 +36,8 @@ import ( ) var ( - log = logs.GetLogEntry("bridge") //nolint[gochecknoglobals] - isApplicationOutdated = false //nolint[gochecknoglobals] + log = logrus.WithField("pkg", "bridge") //nolint[gochecknoglobals] + isApplicationOutdated = false //nolint[gochecknoglobals] ) // Bridge is a struct handling users. @@ -65,7 +64,8 @@ type Bridge struct { lock sync.RWMutex - cancel chan struct{} + // stopAll can be closed to stop all goroutines from looping (watchBridgeOutdated, watchAPIAuths, heartbeat etc). + stopAll chan struct{} userAgentClientName string userAgentClientVersion string @@ -94,7 +94,7 @@ func New( storeCache: store.NewCache(config.GetIMAPCachePath()), idleUpdates: make(chan interface{}), lock: sync.RWMutex{}, - cancel: make(chan struct{}), + stopAll: make(chan struct{}), } // Allow DoH before starting bridge if the user has previously set this setting. @@ -146,7 +146,7 @@ func (b *Bridge) heartbeat() { b.pref.Set(preferences.NextHeartbeatKey, strconv.FormatInt(nextTime.Unix(), 10)) } - case <-b.cancel: + case <-b.stopAll: return } } @@ -191,7 +191,7 @@ func (b *Bridge) watchBridgeOutdated() { isApplicationOutdated = true b.closeAllConnections() - case <-b.cancel: + case <-b.stopAll: return } } @@ -218,7 +218,7 @@ func (b *Bridge) watchAPIAuths() { Error("User logout failed while watching API auths") } - case <-b.cancel: + case <-b.stopAll: return } } @@ -568,7 +568,7 @@ func (b *Bridge) CheckConnection() error { // StopWatchers stops all bridge goroutines. func (b *Bridge) StopWatchers() { - close(b.cancel) + close(b.stopAll) } func (b *Bridge) updateCurrentUserAgent() { diff --git a/internal/bridge/bridge_test_exports.go b/internal/bridge/bridge_test_exports.go new file mode 100644 index 00000000..48c62008 --- /dev/null +++ b/internal/bridge/bridge_test_exports.go @@ -0,0 +1,6 @@ +package bridge + +// IsAuthorized returns whether the user has received an Auth from the API yet. +func (u *User) IsAuthorized() bool { + return u.isAuthorized +} diff --git a/internal/bridge/credentials/credentials.go b/internal/bridge/credentials/credentials.go index a1a6c0fd..b281afad 100644 --- a/internal/bridge/credentials/credentials.go +++ b/internal/bridge/credentials/credentials.go @@ -27,14 +27,13 @@ import ( "fmt" "strings" - "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/sirupsen/logrus" ) const sep = "\x00" var ( - log = logs.GetLogEntry("bridge") //nolint[gochecknoglobals] + log = logrus.WithField("pkg", "bridge") //nolint[gochecknoglobals] ErrWrongFormat = errors.New("backend/creds: malformed password") ) diff --git a/internal/bridge/user.go b/internal/bridge/user.go index 1fbcad62..f3871d39 100644 --- a/internal/bridge/user.go +++ b/internal/bridge/user.go @@ -164,7 +164,7 @@ func (u *User) SetIMAPIdleUpdateChannel() { // it tries to connect it. func (u *User) authorizeIfNecessary(emitEvent bool) (err error) { // If user is connected and has an auth channel, then perfect, nothing to do here. - if u.creds.IsConnected() && u.HasAPIAuth() { + if u.creds.IsConnected() && u.isAuthorized { // The keyring unlock is triggered here to resolve state where apiClient // is authenticated (we have auth token) but it was not possible to download // and unlock the keys (internet not reachable). @@ -585,7 +585,3 @@ func (u *User) CloseConnection(address string) { func (u *User) GetStore() *store.Store { return u.store } - -func (u *User) HasAPIAuth() bool { - return u.isAuthorized -} diff --git a/internal/frontend/cli/frontend.go b/internal/frontend/cli/frontend.go index c31c305a..52e9517b 100644 --- a/internal/frontend/cli/frontend.go +++ b/internal/frontend/cli/frontend.go @@ -24,13 +24,13 @@ import ( "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" - "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/abiosoft/ishell" + "github.com/sirupsen/logrus" ) var ( - log = logs.GetLogEntry("frontend/cli") //nolint[gochecknoglobals] + log = logrus.WithField("pkg", "frontend/cli") //nolint[gochecknoglobals] ) type frontendCLI struct { diff --git a/internal/frontend/frontend.go b/internal/frontend/frontend.go index 0a4d3a59..6010faef 100644 --- a/internal/frontend/frontend.go +++ b/internal/frontend/frontend.go @@ -26,11 +26,11 @@ import ( "github.com/ProtonMail/proton-bridge/internal/frontend/types" "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" - "github.com/ProtonMail/proton-bridge/pkg/logs" + "github.com/sirupsen/logrus" ) var ( - log = logs.GetLogEntry("frontend") // nolint[unused] + log = logrus.WithField("pkg", "frontend") // nolint[unused] ) // Frontend is an interface to be implemented by each frontend type (cli, gui, html). diff --git a/internal/frontend/qt/frontend.go b/internal/frontend/qt/frontend.go index 73d47b7a..1fa9cede 100644 --- a/internal/frontend/qt/frontend.go +++ b/internal/frontend/qt/frontend.go @@ -43,9 +43,9 @@ import ( "github.com/ProtonMail/proton-bridge/internal/frontend/types" "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/pkg/config" - "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/ProtonMail/proton-bridge/pkg/ports" "github.com/ProtonMail/proton-bridge/pkg/useragent" + "github.com/sirupsen/logrus" //"github.com/ProtonMail/proton-bridge/pkg/keychain" "github.com/ProtonMail/proton-bridge/pkg/listener" @@ -59,7 +59,7 @@ import ( "github.com/therecipe/qt/widgets" ) -var log = logs.GetLogEntry("frontend-qt") +var log = logrus.WithField("pkg", "frontend-qt") var accountMutex = &sync.Mutex{} // API between Bridge and Qt. diff --git a/internal/frontend/qt/frontend_nogui.go b/internal/frontend/qt/frontend_nogui.go index 34aef3e7..3a743067 100644 --- a/internal/frontend/qt/frontend_nogui.go +++ b/internal/frontend/qt/frontend_nogui.go @@ -26,10 +26,10 @@ import ( "github.com/ProtonMail/proton-bridge/internal/frontend/types" "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" - "github.com/ProtonMail/proton-bridge/pkg/logs" + "github.com/sirupsen/logrus" ) -var log = logs.GetLogEntry("frontend-nogui") //nolint[gochecknoglobals] +var log = logrus.WithField("pkg", "frontend-nogui") //nolint[gochecknoglobals] type FrontendHeadless struct{} diff --git a/internal/imap/imap.go b/internal/imap/imap.go index 294e042a..ba92213f 100644 --- a/internal/imap/imap.go +++ b/internal/imap/imap.go @@ -17,7 +17,7 @@ package imap -import "github.com/ProtonMail/proton-bridge/pkg/logs" +import "github.com/sirupsen/logrus" const ( fetchMessagesWorkers = 5 // In how many workers to fetch message (group list on IMAP). @@ -31,5 +31,5 @@ const ( ) var ( - log = logs.GetLogEntry("imap") //nolint[gochecknoglobals] + log = logrus.WithField("pkg", "imap") //nolint[gochecknoglobals] ) diff --git a/internal/smtp/smtp.go b/internal/smtp/smtp.go index 8ca02d11..19fd416c 100644 --- a/internal/smtp/smtp.go +++ b/internal/smtp/smtp.go @@ -18,8 +18,8 @@ // Package smtp provides SMTP server of the Bridge. package smtp -import "github.com/ProtonMail/proton-bridge/pkg/logs" +import "github.com/sirupsen/logrus" var ( - log = logs.GetLogEntry("smtp") //nolint[gochecknoglobals] + log = logrus.WithField("pkg", "smtp") //nolint[gochecknoglobals] ) diff --git a/internal/store/event_loop.go b/internal/store/event_loop.go index bdda76da..e781294d 100644 --- a/internal/store/event_loop.go +++ b/internal/store/event_loop.go @@ -247,7 +247,7 @@ func (loop *eventLoop) processNextEvent() (more bool, err error) { // nolint[fun } if event == nil { - return + return false, errors.New("received empty event") } l = l.WithField("newEventID", event.EventID) diff --git a/internal/store/store_test_exports.go b/internal/store/store_test_exports.go index e10005a9..42b65f35 100644 --- a/internal/store/store_test_exports.go +++ b/internal/store/store_test_exports.go @@ -19,7 +19,6 @@ package store import ( "encoding/json" - "errors" "fmt" "github.com/ProtonMail/proton-bridge/pkg/pmapi" @@ -56,7 +55,7 @@ func (store *Store) TestGetStoreFilePath() string { func (store *Store) TestDumpDB(tb assert.TestingT) { if store == nil || store.db == nil { fmt.Printf(">>>>>>>> NIL STORE / DB <<<<<\n\n") - assert.NoError(tb, errors.New("store or database is nil")) + assert.Fail(tb, "store or database is nil") return } diff --git a/pkg/config/config.go b/pkg/config/config.go index 0de4a848..7b3b8613 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -23,12 +23,12 @@ import ( "path/filepath" "github.com/ProtonMail/go-appdir" - "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/hashicorp/go-multierror" + "github.com/sirupsen/logrus" ) var ( - log = logs.GetLogEntry("config") //nolint[gochecknoglobals] + log = logrus.WithField("pkg", "config") //nolint[gochecknoglobals] ) type appDirProvider interface { diff --git a/pkg/keychain/keychain.go b/pkg/keychain/keychain.go index 16a7f036..c5be6631 100644 --- a/pkg/keychain/keychain.go +++ b/pkg/keychain/keychain.go @@ -23,8 +23,8 @@ import ( "strings" "sync" - "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/docker/docker-credential-helpers/credentials" + "github.com/sirupsen/logrus" ) const ( @@ -32,7 +32,7 @@ const ( ) var ( - log = logs.GetLogEntry("bridgeUtils/keychain") //nolint[gochecknoglobals] + log = logrus.WithField("pkg", "bridgeUtils/keychain") //nolint[gochecknoglobals] ErrWrongKeychainURL = errors.New("wrong keychain base URL") ErrMacKeychainRebuild = errors.New("keychain error -25293") diff --git a/pkg/listener/listener.go b/pkg/listener/listener.go index 0ea4524c..387197a3 100644 --- a/pkg/listener/listener.go +++ b/pkg/listener/listener.go @@ -21,10 +21,10 @@ import ( "sync" "time" - "github.com/ProtonMail/proton-bridge/pkg/logs" + "github.com/sirupsen/logrus" ) -var log = logs.GetLogEntry("bridgeUtils/listener") //nolint[gochecknoglobals] +var log = logrus.WithField("pkg", "bridgeUtils/listener") //nolint[gochecknoglobals] // Listener has a list of channels watching for updates. type Listener interface { diff --git a/pkg/logs/logs.go b/pkg/logs/logs.go deleted file mode 100644 index 7dc10abc..00000000 --- a/pkg/logs/logs.go +++ /dev/null @@ -1,10 +0,0 @@ -package logs - -import "github.com/sirupsen/logrus" - -// GetLogEntry returns logrus.Entry with PID and `packageName`. -func GetLogEntry(packageName string) *logrus.Entry { - return logrus.WithFields(logrus.Fields{ - "pkg": packageName, - }) -} diff --git a/pkg/pmapi/auth.go b/pkg/pmapi/auth.go index a0cdcbbf..8647edeb 100644 --- a/pkg/pmapi/auth.go +++ b/pkg/pmapi/auth.go @@ -218,7 +218,7 @@ func (c *client) sendAuth(auth *Auth) { } go func(auth ClientAuth) { - c.cm.GetClientAuthChannel() <- auth + c.cm.clientAuths <- auth }(ClientAuth{ UserID: c.userID, Auth: auth, @@ -425,7 +425,7 @@ func (c *client) Unlock(password string) (kr *pmcrypto.KeyRing, err error) { func (c *client) AuthRefresh(uidAndRefreshToken string) (auth *Auth, err error) { // If we don't yet have a saved access token, save this one in case the refresh fails! // That way we can try again later (see handleUnauthorizedStatus). - c.cm.SetTokenIfUnset(c.userID, uidAndRefreshToken) + c.cm.setTokenIfUnset(c.userID, uidAndRefreshToken) split := strings.Split(uidAndRefreshToken, ":") if len(split) != 2 { diff --git a/pkg/pmapi/clientmanager.go b/pkg/pmapi/clientmanager.go index 40c8164c..9057b847 100644 --- a/pkg/pmapi/clientmanager.go +++ b/pkg/pmapi/clientmanager.go @@ -32,8 +32,8 @@ type ClientManager struct { expiredTokens chan string expirationsLocker sync.Locker - bridgeAuths chan ClientAuth - clientAuths chan ClientAuth + clientAuths chan ClientAuth // auths received by clients from the API are received here and handled. + forwardedAuths chan ClientAuth // once auths are handled, they are forwarded on this channel. host, scheme string hostLocker sync.RWMutex @@ -82,12 +82,12 @@ func NewClientManager(config *ClientConfig) (cm *ClientManager) { expiredTokens: make(chan string), expirationsLocker: &sync.Mutex{}, - host: RootURL, + host: rootURL, scheme: rootScheme, hostLocker: sync.RWMutex{}, - bridgeAuths: make(chan ClientAuth), - clientAuths: make(chan ClientAuth), + forwardedAuths: make(chan ClientAuth), + clientAuths: make(chan ClientAuth), proxyProvider: newProxyProvider(dohProviders, proxyQuery), proxyUseDuration: proxyUseDuration, @@ -211,7 +211,7 @@ func (cm *ClientManager) DisallowProxy() { defer cm.hostLocker.Unlock() cm.allowProxy = false - cm.host = RootURL + cm.host = rootURL } // IsProxyEnabled returns whether we are currently proxying requests. @@ -219,7 +219,7 @@ func (cm *ClientManager) IsProxyEnabled() bool { cm.hostLocker.RLock() defer cm.hostLocker.RUnlock() - return cm.host != RootURL + return cm.host != rootURL } // switchToReachableServer switches to using a reachable server (either proxy or standard API). @@ -236,12 +236,12 @@ func (cm *ClientManager) switchToReachableServer() (proxy string, err error) { logrus.WithField("proxy", proxy).Info("Switching to a proxy") - // If the host is currently the RootURL, it's the first time we are enabling a proxy. + // If the host is currently the rootURL, it's the first time we are enabling a proxy. // This means we want to disable it again in 24 hours. - if cm.host == RootURL { + if cm.host == rootURL { go func() { <-time.After(cm.proxyUseDuration) - cm.host = RootURL + cm.host = rootURL }() } @@ -260,12 +260,7 @@ func (cm *ClientManager) GetToken(userID string) string { // GetAuthUpdateChannel returns a channel on which client auths can be received. func (cm *ClientManager) GetAuthUpdateChannel() chan ClientAuth { - return cm.bridgeAuths -} - -// GetClientAuthChannel returns a channel on which clients should send auths. -func (cm *ClientManager) GetClientAuthChannel() chan ClientAuth { - return cm.clientAuths + return cm.forwardedAuths } // Errors for possible connection issues @@ -330,19 +325,19 @@ func checkConnection(client *http.Client, url string, errorChannel chan error) { errorChannel <- nil } -// forwardClientAuths handles all incoming auths from clients before forwarding them on the bridge auth channel. +// forwardClientAuths handles all incoming auths from clients before forwarding them on the forwarded auths channel. func (cm *ClientManager) forwardClientAuths() { for auth := range cm.clientAuths { logrus.Debug("ClientManager received auth from client") cm.handleClientAuth(auth) - logrus.Debug("ClientManager is forwarding auth to bridge") - cm.bridgeAuths <- auth + logrus.Debug("ClientManager is forwarding auth") + cm.forwardedAuths <- auth } } -// SetTokenIfUnset sets the token for the given userID if it wasn't already set. +// setTokenIfUnset sets the token for the given userID if it wasn't already set. // The set token does not expire. -func (cm *ClientManager) SetTokenIfUnset(userID, token string) { +func (cm *ClientManager) setTokenIfUnset(userID, token string) { cm.tokensLocker.Lock() defer cm.tokensLocker.Unlock() diff --git a/pkg/pmapi/config.go b/pkg/pmapi/config.go index 0e46bf62..522d3697 100644 --- a/pkg/pmapi/config.go +++ b/pkg/pmapi/config.go @@ -22,13 +22,13 @@ import ( "runtime" ) -// RootURL is the API root URL. +// rootURL is the API root URL. // // This can be changed using build flags: pmapi_local for "localhost/api", pmapi_dev or pmapi_prod. // Default is pmapi_prod. // // It must not contain the protocol! The protocol should be in rootScheme. -var RootURL = "api.protonmail.ch" //nolint[gochecknoglobals] +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 diff --git a/pkg/pmapi/config_dev.go b/pkg/pmapi/config_dev.go index 2c6266e5..291f7000 100644 --- a/pkg/pmapi/config_dev.go +++ b/pkg/pmapi/config_dev.go @@ -20,6 +20,6 @@ package pmapi func init() { - RootURL = "dev.protonmail.com/api" + rootURL = "dev.protonmail.com/api" rootScheme = "https" } diff --git a/pkg/pmapi/config_local.go b/pkg/pmapi/config_local.go index 193bd357..981d8121 100644 --- a/pkg/pmapi/config_local.go +++ b/pkg/pmapi/config_local.go @@ -27,7 +27,7 @@ import ( func init() { // Use port above 1000 which doesn't need root access to start anything on it. // Now the port is rounded pi. :-) - RootURL = "127.0.0.1:3142/api" + rootURL = "127.0.0.1:3142/api" rootScheme = "http" // TLS certificate is self-signed diff --git a/pkg/pmapi/dialer_with_proxy.go b/pkg/pmapi/dialer_with_proxy.go index e03e628b..d669b73e 100644 --- a/pkg/pmapi/dialer_with_proxy.go +++ b/pkg/pmapi/dialer_with_proxy.go @@ -330,7 +330,7 @@ func (p *DialerWithPinning) dial(network, address string) (conn net.Conn, err er // If we are not dialing the standard API then we should skip cert verification checks. var tlsConfig *tls.Config = nil - if address != RootURL { + if address != rootURL { tlsConfig = &tls.Config{InsecureSkipVerify: true} // nolint[gosec] } diff --git a/pkg/pmapi/auth_test_export.go b/pkg/pmapi/pmapi_test_exports.go similarity index 84% rename from pkg/pmapi/auth_test_export.go rename to pkg/pmapi/pmapi_test_exports.go index de1efaad..5eb50032 100644 --- a/pkg/pmapi/auth_test_export.go +++ b/pkg/pmapi/pmapi_test_exports.go @@ -21,3 +21,8 @@ package pmapi func (s *Auth) DANGEROUSLYSetUID(uid string) { s.uid = uid } + +// GetClientAuthChannel returns a channel on which clients should send auths. +func (cm *ClientManager) GetClientAuthChannel() chan ClientAuth { + return cm.clientAuths +} diff --git a/pkg/pmapi/proxy.go b/pkg/pmapi/proxy.go index 3ec2d0c0..5bf3cadd 100644 --- a/pkg/pmapi/proxy.go +++ b/pkg/pmapi/proxy.go @@ -88,9 +88,9 @@ func (p *proxyProvider) findReachableServer() (proxy string, err error) { logrus.WithError(err).Warn("Failed to refresh proxy cache, cache may be out of date") } - // We want to switch back to the RootURL if possible. - if p.canReach(RootURL) { - proxyResult <- RootURL + // We want to switch back to the rootURL if possible. + if p.canReach(rootURL) { + proxyResult <- rootURL return } diff --git a/pkg/pmapi/proxy_test.go b/pkg/pmapi/proxy_test.go index 579e873e..e090be97 100644 --- a/pkg/pmapi/proxy_test.go +++ b/pkg/pmapi/proxy_test.go @@ -198,7 +198,7 @@ func TestProxyProvider_UseProxy_RevertAfterTime(t *testing.T) { require.Equal(t, proxy.URL, cm.getHost()) time.Sleep(2 * time.Second) - require.Equal(t, RootURL, cm.getHost()) + require.Equal(t, rootURL, cm.getHost()) } func TestProxyProvider_UseProxy_RevertIfProxyStopsWorkingAndOriginalAPIIsReachable(t *testing.T) { @@ -227,8 +227,8 @@ func TestProxyProvider_UseProxy_RevertIfProxyStopsWorkingAndOriginalAPIIsReachab // We should now find the original API URL if it is working again. url, err = cm.switchToReachableServer() require.NoError(t, err) - require.Equal(t, RootURL, url) - require.Equal(t, RootURL, cm.getHost()) + require.Equal(t, rootURL, url) + require.Equal(t, rootURL, cm.getHost()) } func TestProxyProvider_UseProxy_FindSecondAlternativeIfFirstFailsAndAPIIsStillBlocked(t *testing.T) { @@ -298,14 +298,14 @@ func TestProxyProvider_DoHLookup_FindProxyFirstProviderUnreachable(t *testing.T) } // testAPIURLBackup is used to hold the globalOriginalURL because we clear it for test purposes and need to restore it. -var testAPIURLBackup = RootURL +var testAPIURLBackup = rootURL // blockAPI prevents tests from reaching the standard API, forcing them to find a proxy. func blockAPI() { - RootURL = "" + rootURL = "" } // unblockAPI allow tests to reach the standard API again. func unblockAPI() { - RootURL = testAPIURLBackup + rootURL = testAPIURLBackup } diff --git a/pkg/updates/updates.go b/pkg/updates/updates.go index d7e74181..31426cc3 100644 --- a/pkg/updates/updates.go +++ b/pkg/updates/updates.go @@ -27,8 +27,8 @@ import ( "runtime" "strings" - "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/kardianos/osext" + "github.com/sirupsen/logrus" ) const ( @@ -44,7 +44,7 @@ var ( ) var ( - log = logs.GetLogEntry("bridgeUtils/updates") //nolint[gochecknoglobals] + log = logrus.WithField("pkg", "bridgeUtils/updates") //nolint[gochecknoglobals] installFileSuffix = map[string]string{ //nolint[gochecknoglobals] "darwin": ".dmg", diff --git a/test/bridge_checks_test.go b/test/bridge_checks_test.go index 12e19cc7..980fecba 100644 --- a/test/bridge_checks_test.go +++ b/test/bridge_checks_test.go @@ -35,8 +35,8 @@ func BridgeChecksFeatureContext(s *godog.Suite) { s.Step(`^"([^"]*)" does not have loaded store$`, userDoesNotHaveLoadedStore) s.Step(`^"([^"]*)" has running event loop$`, userHasRunningEventLoop) s.Step(`^"([^"]*)" does not have running event loop$`, userDoesNotHaveRunningEventLoop) - s.Step(`^"([^"]*)" does not have API auth$`, doesNotHaveAPIAuth) - s.Step(`^"([^"]*)" has API auth$`, hasAPIAuth) + s.Step(`^"([^"]*)" does not have API auth$`, isNotAuthorized) + s.Step(`^"([^"]*)" has API auth$`, isAuthorized) } func bridgeResponseIs(expectedResponse string) error { @@ -91,9 +91,7 @@ func userIsConnected(bddUserID string) error { if err != nil { return internalError(err, "getting user %s", account.Username()) } - a.Eventually(ctx.GetTestingT(), func() bool { - return bridgeUser.IsConnected() - }, 5*time.Second, 10*time.Millisecond) + a.Eventually(ctx.GetTestingT(), bridgeUser.IsConnected, 5*time.Second, 10*time.Millisecond) a.NotEmpty(t, bridgeUser.GetPrimaryAddress()) a.NotEmpty(t, bridgeUser.GetStoreAddresses()) return ctx.GetTestingError() @@ -175,7 +173,7 @@ func userDoesNotHaveRunningEventLoop(bddUserID string) error { return ctx.GetTestingError() } -func hasAPIAuth(accountName string) error { +func isAuthorized(accountName string) error { account := ctx.GetTestAccount(accountName) if account == nil { return godog.ErrPending @@ -184,14 +182,11 @@ func hasAPIAuth(accountName string) error { if err != nil { return internalError(err, "getting user %s", account.Username()) } - a.Eventually(ctx.GetTestingT(), - bridgeUser.HasAPIAuth, - 5*time.Second, 10*time.Millisecond, - ) + a.Eventually(ctx.GetTestingT(), bridgeUser.IsAuthorized, 5*time.Second, 10*time.Millisecond) return ctx.GetTestingError() } -func doesNotHaveAPIAuth(accountName string) error { +func isNotAuthorized(accountName string) error { account := ctx.GetTestAccount(accountName) if account == nil { return godog.ErrPending @@ -200,6 +195,6 @@ func doesNotHaveAPIAuth(accountName string) error { if err != nil { return internalError(err, "getting user %s", account.Username()) } - a.False(ctx.GetTestingT(), bridgeUser.HasAPIAuth()) + a.Eventually(ctx.GetTestingT(), func() bool { return !bridgeUser.IsAuthorized() }, 5*time.Second, 10*time.Millisecond) return ctx.GetTestingError() } diff --git a/test/fakeapi/auth.go b/test/fakeapi/auth.go index 904fd778..4cd74951 100644 --- a/test/fakeapi/auth.go +++ b/test/fakeapi/auth.go @@ -65,7 +65,7 @@ func (api *FakePMAPI) Auth(username, password string, authInfo *pmapi.AuthInfo) auth := &pmapi.Auth{ TwoFA: user.get2FAInfo(), RefreshToken: session.refreshToken, - ExpiresIn: 86400, + ExpiresIn: 86400, // seconds } auth.DANGEROUSLYSetUID(session.uid) diff --git a/test/fakeapi/events.go b/test/fakeapi/events.go index 6ec03863..38b5eb41 100644 --- a/test/fakeapi/events.go +++ b/test/fakeapi/events.go @@ -28,7 +28,7 @@ func (api *FakePMAPI) GetEvent(eventID string) (*pmapi.Event, error) { // Request for empty ID returns the latest event. if eventID == "" { if len(api.events) == 0 { - return &pmapi.Event{EventID: ""}, nil + return &pmapi.Event{EventID: "first-event-id"}, nil } return api.events[len(api.events)-1], nil }