From 4809d97cb11b2080a4f7d2dbae5db194b6d30c34 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Thu, 16 Apr 2020 16:05:05 +0200 Subject: [PATCH] feat: clientmanager has checkconnection --- go.sum | 2 + internal/bridge/bridge.go | 6 ++ internal/bridge/mocks/mocks.go | 14 +++ internal/bridge/types.go | 1 + internal/frontend/cli/system.go | 3 +- internal/frontend/qt/frontend.go | 6 +- internal/frontend/qt/helpers.go | 5 -- internal/frontend/types/types.go | 1 + pkg/connection/check_connection.go | 90 ------------------- .../check_connection_test.go | 2 +- pkg/pmapi/clientmanager.go | 65 +++++++++++++- 11 files changed, 93 insertions(+), 102 deletions(-) delete mode 100644 pkg/connection/check_connection.go rename pkg/{connection => pmapi}/check_connection_test.go (99%) diff --git a/go.sum b/go.sum index 0e74c458..c9b7534c 100644 --- a/go.sum +++ b/go.sum @@ -183,7 +183,9 @@ github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/therecipe/qt v0.0.0-20200126204426-5074eb6d8c41 h1:yBVcrpbaQYJBdKT2pxTdlL4hBE/eM4UPcyj9YpyvSok= github.com/therecipe/qt v0.0.0-20200126204426-5074eb6d8c41/go.mod h1:SUUR2j3aE1z6/g76SdD6NwACEpvCxb3fvG82eKbD6us= +github.com/therecipe/qt/internal/binding/files/docs/5.12.0 v0.0.0-20200126204426-5074eb6d8c41 h1:My9HYsfDI/fJPZGyilw6066buBiZ7pgKRRgAyvKK5lA= github.com/therecipe/qt/internal/binding/files/docs/5.12.0 v0.0.0-20200126204426-5074eb6d8c41/go.mod h1:7m8PDYDEtEVqfjoUQc2UrFqhG0CDmoVJjRlQxexndFc= +github.com/therecipe/qt/internal/binding/files/docs/5.13.0 v0.0.0-20200126204426-5074eb6d8c41 h1:jTzKrQ6EIPvKw1B9/wwoKJLrXF+ManMsXoUzufxAdsg= github.com/therecipe/qt/internal/binding/files/docs/5.13.0 v0.0.0-20200126204426-5074eb6d8c41/go.mod h1:mH55Ek7AZcdns5KPp99O0bg+78el64YCYWHiQKrOdt4= github.com/twinj/uuid v1.0.0 h1:fzz7COZnDrXGTAOHGuUGYd6sG+JMq+AoE7+Jlu0przk= github.com/twinj/uuid v1.0.0/go.mod h1:mMgcE1RHFUFqe5AfiwlINXisXfDGro23fWdPUfOMjRY= diff --git a/internal/bridge/bridge.go b/internal/bridge/bridge.go index 1787a92b..f6917ed9 100644 --- a/internal/bridge/bridge.go +++ b/internal/bridge/bridge.go @@ -535,6 +535,12 @@ func (b *Bridge) DisallowProxy() { b.clientManager.DisallowProxy() } +// CheckConnection returns whether there is an internet connection. +// This should use the connection manager when it is eventually implemented. +func (b *Bridge) CheckConnection() error { + return b.clientManager.CheckConnection() +} + func (b *Bridge) updateCurrentUserAgent() { UpdateCurrentUserAgent(b.version, b.userAgentOS, b.userAgentClientName, b.userAgentClientVersion) } diff --git a/internal/bridge/mocks/mocks.go b/internal/bridge/mocks/mocks.go index 54de9505..3fbf72f0 100644 --- a/internal/bridge/mocks/mocks.go +++ b/internal/bridge/mocks/mocks.go @@ -238,6 +238,20 @@ func (mr *MockClientManagerMockRecorder) AllowProxy() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AllowProxy", reflect.TypeOf((*MockClientManager)(nil).AllowProxy)) } +// CheckConnection mocks base method +func (m *MockClientManager) CheckConnection() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CheckConnection") + ret0, _ := ret[0].(error) + return ret0 +} + +// CheckConnection indicates an expected call of CheckConnection +func (mr *MockClientManagerMockRecorder) CheckConnection() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckConnection", reflect.TypeOf((*MockClientManager)(nil).CheckConnection)) +} + // DisallowProxy mocks base method func (m *MockClientManager) DisallowProxy() { m.ctrl.T.Helper() diff --git a/internal/bridge/types.go b/internal/bridge/types.go index c3b1012b..d7da9a2b 100644 --- a/internal/bridge/types.go +++ b/internal/bridge/types.go @@ -58,4 +58,5 @@ type ClientManager interface { AllowProxy() DisallowProxy() GetAuthUpdateChannel() chan pmapi.ClientAuth + CheckConnection() error } diff --git a/internal/frontend/cli/system.go b/internal/frontend/cli/system.go index 7ac619c4..aaa3cef8 100644 --- a/internal/frontend/cli/system.go +++ b/internal/frontend/cli/system.go @@ -23,7 +23,6 @@ import ( "strings" "github.com/ProtonMail/proton-bridge/internal/preferences" - "github.com/ProtonMail/proton-bridge/pkg/connection" "github.com/ProtonMail/proton-bridge/pkg/ports" "github.com/abiosoft/ishell" ) @@ -41,7 +40,7 @@ func (f *frontendCLI) restart(c *ishell.Context) { } func (f *frontendCLI) checkInternetConnection(c *ishell.Context) { - if connection.CheckInternetConnection() == nil { + if f.bridge.CheckConnection() == nil { f.Println("Internet connection is available.") } else { f.Println("Can not contact server please check you internet connection.") diff --git a/internal/frontend/qt/frontend.go b/internal/frontend/qt/frontend.go index f588eac0..73d47b7a 100644 --- a/internal/frontend/qt/frontend.go +++ b/internal/frontend/qt/frontend.go @@ -49,7 +49,7 @@ import ( //"github.com/ProtonMail/proton-bridge/pkg/keychain" "github.com/ProtonMail/proton-bridge/pkg/listener" - pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi" + "github.com/ProtonMail/proton-bridge/pkg/pmapi" "github.com/ProtonMail/proton-bridge/pkg/updates" "github.com/kardianos/osext" "github.com/skratchdot/open-golang/open" @@ -86,7 +86,7 @@ type FrontendQt struct { programName string // Program name (shown in taskbar). programVer string // Program version (shown in help). - authClient bridge.PMAPIProvider + authClient pmapi.Client auth *pmapi.Auth @@ -569,7 +569,7 @@ func (s *FrontendQt) isSMTPSTARTTLS() bool { } func (s *FrontendQt) checkInternet() { - s.Qml.SetConnectionStatus(IsInternetAvailable()) + s.Qml.SetConnectionStatus(s.bridge.CheckConnection() == nil) } func (s *FrontendQt) switchAddressModeUser(iAccount int) { diff --git a/internal/frontend/qt/helpers.go b/internal/frontend/qt/helpers.go index 89be86e6..32cc470e 100644 --- a/internal/frontend/qt/helpers.go +++ b/internal/frontend/qt/helpers.go @@ -25,7 +25,6 @@ import ( "os/exec" "time" - "github.com/ProtonMail/proton-bridge/pkg/connection" "github.com/therecipe/qt/core" ) @@ -64,10 +63,6 @@ func PauseLong() { time.Sleep(3 * time.Second) } -func IsInternetAvailable() bool { - return connection.CheckInternetConnection() == nil -} - // FIXME: Not working in test... func WaitForEnter() { log.Print("Press 'Enter' to continue...") diff --git a/internal/frontend/types/types.go b/internal/frontend/types/types.go index b05fd0b0..46558bc9 100644 --- a/internal/frontend/types/types.go +++ b/internal/frontend/types/types.go @@ -54,6 +54,7 @@ type Bridger interface { ClearData() error AllowProxy() DisallowProxy() + CheckConnection() error } // BridgeUser is an interface of user needed by frontend. diff --git a/pkg/connection/check_connection.go b/pkg/connection/check_connection.go deleted file mode 100644 index ec542dcd..00000000 --- a/pkg/connection/check_connection.go +++ /dev/null @@ -1,90 +0,0 @@ -// 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 connection - -import ( - "errors" - "fmt" - "net/http" - - "github.com/ProtonMail/proton-bridge/pkg/logs" - "github.com/ProtonMail/proton-bridge/pkg/pmapi" -) - -// Errors for possible connection issues -var ( - ErrNoInternetConnection = errors.New("no internet connection") - ErrCanNotReachAPI = errors.New("can not reach PM API") - log = logs.GetLogEntry("connection") //nolint[gochecknoglobals] -) - -// CheckInternetConnection does a check of API connection. It checks two of our endpoints in parallel. -// One endpoint is part of the protonmail API, while the other is not. -// This allows us to determine whether there is a problem with the connection itself or only a problem with our API. -// Two errors can be returned, ErrNoInternetConnection or ErrCanNotReachAPI. -func CheckInternetConnection() error { - client := &http.Client{ - // TODO: Set transport properly! (Need access to ClientManager somehow) - // Transport: pmapi.NewDialerWithPinning(pmapi.CurrentUserAgent).TransportWithPinning(), - } - - // Do not cumulate timeouts, use goroutines. - retStatus := make(chan error) - retAPI := make(chan error) - - // Check protonstatus.com without SSL for performance reasons. vpn_status endpoint is fast and - // returns only OK; this endpoint is not known by the public. We check the connection only. - go checkConnection(client, "http://protonstatus.com/vpn_status", retStatus) - - // Check of API reachability also uses a fast endpoint. - // TODO: This should check active proxy, not the RootURL - go checkConnection(client, pmapi.RootURL+"/tests/ping", retAPI) - - errStatus := <-retStatus - errAPI := <-retAPI - - if errStatus != nil { - if errAPI != nil { - log.Error("Checking internet connection failed with ", errStatus, " and ", errAPI) - return ErrNoInternetConnection - } - log.Warning("API OK, but status: ", errStatus) - return nil - } - - if errAPI != nil { - log.Error("Status OK, but API: ", errAPI) - return ErrCanNotReachAPI - } - - return nil -} - -func checkConnection(client *http.Client, url string, errorChannel chan error) { - resp, err := client.Get(url) - if err != nil { - errorChannel <- err - return - } - _ = resp.Body.Close() - if resp.StatusCode != 200 { - errorChannel <- fmt.Errorf("HTTP status code %d", resp.StatusCode) - return - } - errorChannel <- nil -} diff --git a/pkg/connection/check_connection_test.go b/pkg/pmapi/check_connection_test.go similarity index 99% rename from pkg/connection/check_connection_test.go rename to pkg/pmapi/check_connection_test.go index 167d793a..75c9754c 100644 --- a/pkg/connection/check_connection_test.go +++ b/pkg/pmapi/check_connection_test.go @@ -15,7 +15,7 @@ // You should have received a copy of the GNU General Public License // along with ProtonMail Bridge. If not, see . -package connection +package pmapi import ( "net/http" diff --git a/pkg/pmapi/clientmanager.go b/pkg/pmapi/clientmanager.go index f2e59192..a681e2a1 100644 --- a/pkg/pmapi/clientmanager.go +++ b/pkg/pmapi/clientmanager.go @@ -255,6 +255,68 @@ func (cm *ClientManager) GetClientAuthChannel() chan ClientAuth { return cm.clientAuths } +// Errors for possible connection issues +var ( + ErrNoInternetConnection = errors.New("no internet connection") + ErrCanNotReachAPI = errors.New("can not reach PM API") +) + +// CheckConnection returns an error if there is no internet connection. +// This should be moved to the ConnectionManager when it is implemented. +func (cm *ClientManager) CheckConnection() error { + client := getHTTPClient(cm.config, cm.roundTripper) + + // Do not cumulate timeouts, use goroutines. + retStatus := make(chan error) + retAPI := make(chan error) + + // Check protonstatus.com without SSL for performance reasons. vpn_status endpoint is fast and + // returns only OK; this endpoint is not known by the public. We check the connection only. + go checkConnection(client, "http://protonstatus.com/vpn_status", retStatus) + + // Check of API reachability also uses a fast endpoint. + go checkConnection(client, cm.GetRootURL()+"/tests/ping", retAPI) + + errStatus := <-retStatus + errAPI := <-retAPI + + switch { + case errStatus == nil && errAPI == nil: + return nil + + case errStatus == nil && errAPI != nil: + cm.log.Error("ProtonStatus is reachable but API is not") + return ErrCanNotReachAPI + + case errStatus != nil && errAPI == nil: + cm.log.Warn("API is reachable but protonstatus is not") + return nil + + case errStatus != nil && errAPI != nil: + cm.log.Error("Both ProtonStatus and API are unreachable") + return ErrNoInternetConnection + } + + return nil +} + +func checkConnection(client *http.Client, url string, errorChannel chan error) { + resp, err := client.Get(url) + if err != nil { + errorChannel <- err + return + } + + _ = resp.Body.Close() + + if resp.StatusCode != 200 { + errorChannel <- fmt.Errorf("HTTP status code %d", resp.StatusCode) + return + } + + errorChannel <- nil +} + // forwardClientAuths handles all incoming auths from clients before forwarding them on the bridge auth channel. func (cm *ClientManager) forwardClientAuths() { for auth := range cm.clientAuths { @@ -266,7 +328,7 @@ func (cm *ClientManager) forwardClientAuths() { } // SetTokenIfUnset sets the token for the given userID if it wasn't already set. -// The token does not expire. +// The set token does not expire. func (cm *ClientManager) SetTokenIfUnset(userID, token string) { cm.tokensLocker.Lock() defer cm.tokensLocker.Unlock() @@ -352,6 +414,7 @@ func (cm *ClientManager) handleClientAuth(ca ClientAuth) { cm.setToken(ca.UserID, ca.Auth.GenToken(), time.Duration(ca.Auth.ExpiresIn)*time.Second) } +// watchTokenExpirations refreshes any tokens which are about to expire. func (cm *ClientManager) watchTokenExpirations() { for userID := range cm.expiredTokens { log := cm.log.WithField("userID", userID)