From 40d8c458d2fca9b2182e2bd3afbabb0b87e9ba11 Mon Sep 17 00:00:00 2001 From: Michal Horejsek Date: Mon, 25 May 2020 15:34:18 +0200 Subject: [PATCH] Store factory to make store optional --- Changelog.md | 2 + Makefile | 2 +- internal/bridge/bridge.go | 48 +++++- internal/bridge/store_factory.go | 69 ++++++++ internal/bridge/types.go | 37 ++++ internal/users/credentials/credentials.go | 41 +++-- .../users/credentials/credentials_test.go | 67 +++++++ internal/users/mocks/mocks.go | 163 ++++++------------ internal/users/types.go | 15 +- internal/users/user.go | 23 +-- internal/users/user_credentials_test.go | 6 +- internal/users/user_new_test.go | 4 +- internal/users/user_test.go | 4 +- internal/users/users.go | 52 +----- internal/users/users_new_test.go | 10 +- internal/users/users_test.go | 30 ++-- 16 files changed, 338 insertions(+), 235 deletions(-) create mode 100644 internal/bridge/store_factory.go create mode 100644 internal/bridge/types.go create mode 100644 internal/users/credentials/credentials_test.go diff --git a/Changelog.md b/Changelog.md index 825be04b..df1dd619 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,6 +5,8 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) ## Unreleased ### Changed +* GODT-388 support for both bridge and import/export credentials by package users +* GODT-387 store factory to make store optional * GODT-386 renamed bridge to general users and keep bridge only for bridge stuff * GODT-308 better user error message when request is canceled * GODT-312 validate recipient emails in send before asking for their public keys diff --git a/Makefile b/Makefile index 5b2bc982..fcef9159 100644 --- a/Makefile +++ b/Makefile @@ -171,7 +171,7 @@ coverage: test go tool cover -html=/tmp/coverage.out -o=coverage.html mocks: - mockgen --package mocks github.com/ProtonMail/proton-bridge/internal/users Configer,PreferenceProvider,PanicHandler,ClientManager,CredentialsStorer > internal/users/mocks/mocks.go + mockgen --package mocks github.com/ProtonMail/proton-bridge/internal/users Configer,PanicHandler,ClientManager,CredentialsStorer,StoreMaker > internal/users/mocks/mocks.go mockgen --package mocks github.com/ProtonMail/proton-bridge/internal/store PanicHandler,ClientManager,BridgeUser > internal/store/mocks/mocks.go mockgen --package mocks github.com/ProtonMail/proton-bridge/pkg/listener Listener > internal/store/mocks/utils_mocks.go mockgen --package mocks github.com/ProtonMail/proton-bridge/pkg/pmapi Client > pkg/pmapi/mocks/mocks.go diff --git a/internal/bridge/bridge.go b/internal/bridge/bridge.go index 2063747c..95199585 100644 --- a/internal/bridge/bridge.go +++ b/internal/bridge/bridge.go @@ -19,6 +19,11 @@ package bridge import ( + "strconv" + "time" + + "github.com/ProtonMail/proton-bridge/internal/metrics" + "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/internal/users" "github.com/ProtonMail/proton-bridge/pkg/listener" @@ -32,6 +37,7 @@ var ( type Bridge struct { *users.Users + pref PreferenceProvider clientManager users.ClientManager userAgentClientName string @@ -40,19 +46,53 @@ type Bridge struct { } func New( - config users.Configer, - pref users.PreferenceProvider, + config Configer, + pref PreferenceProvider, panicHandler users.PanicHandler, eventListener listener.Listener, clientManager users.ClientManager, credStorer users.CredentialsStorer, ) *Bridge { - u := users.New(config, pref, panicHandler, eventListener, clientManager, credStorer) - return &Bridge{ + storeFactory := newStoreFactory(config, panicHandler, clientManager, eventListener) + u := users.New(config, panicHandler, eventListener, clientManager, credStorer, storeFactory) + b := &Bridge{ Users: u, + pref: pref, clientManager: clientManager, } + + // Allow DoH before starting the app if the user has previously set this setting. + // This allows us to start even if protonmail is blocked. + if pref.GetBool(preferences.AllowProxyKey) { + b.AllowProxy() + } + + if pref.GetBool(preferences.FirstStartKey) { + b.SendMetric(metrics.New(metrics.Setup, metrics.FirstStart, metrics.Label(config.GetVersion()))) + } + + go b.heartbeat() + + return b +} + +// heartbeat sends a heartbeat signal once a day. +func (b *Bridge) heartbeat() { + ticker := time.NewTicker(1 * time.Minute) + + for range ticker.C { + next, err := strconv.ParseInt(b.pref.Get(preferences.NextHeartbeatKey), 10, 64) + if err != nil { + continue + } + nextTime := time.Unix(next, 0) + if time.Now().After(nextTime) { + b.SendMetric(metrics.New(metrics.Heartbeat, metrics.Daily, metrics.NoLabel)) + nextTime = nextTime.Add(24 * time.Hour) + b.pref.Set(preferences.NextHeartbeatKey, strconv.FormatInt(nextTime.Unix(), 10)) + } + } } // GetCurrentClient returns currently connected client (e.g. Thunderbird). diff --git a/internal/bridge/store_factory.go b/internal/bridge/store_factory.go new file mode 100644 index 00000000..563b0029 --- /dev/null +++ b/internal/bridge/store_factory.go @@ -0,0 +1,69 @@ +// 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 + +import ( + "fmt" + "path/filepath" + + "github.com/ProtonMail/proton-bridge/internal/store" + "github.com/ProtonMail/proton-bridge/internal/users" + + "github.com/ProtonMail/proton-bridge/pkg/listener" +) + +type storeFactory struct { + config StoreFactoryConfiger + panicHandler users.PanicHandler + clientManager users.ClientManager + eventListener listener.Listener + storeCache *store.Cache +} + +func newStoreFactory( + config StoreFactoryConfiger, + panicHandler users.PanicHandler, + clientManager users.ClientManager, + eventListener listener.Listener, +) *storeFactory { + return &storeFactory{ + config: config, + panicHandler: panicHandler, + clientManager: clientManager, + eventListener: eventListener, + storeCache: store.NewCache(config.GetIMAPCachePath()), + } +} + +// New creates new store for given user. +func (f *storeFactory) New(user store.BridgeUser) (*store.Store, error) { + storePath := getUserStorePath(f.config.GetDBDir(), user.ID()) + return store.New(f.panicHandler, user, f.clientManager, f.eventListener, storePath, f.storeCache) +} + +// Remove removes all store files for given user. +func (f *storeFactory) Remove(userID string) error { + storePath := getUserStorePath(f.config.GetDBDir(), userID) + return store.RemoveStore(f.storeCache, storePath, userID) +} + +// getUserStorePath returns the file path of the store database for the given userID. +func getUserStorePath(storeDir string, userID string) (path string) { + fileName := fmt.Sprintf("mailbox-%v.db", userID) + return filepath.Join(storeDir, fileName) +} diff --git a/internal/bridge/types.go b/internal/bridge/types.go new file mode 100644 index 00000000..837a5622 --- /dev/null +++ b/internal/bridge/types.go @@ -0,0 +1,37 @@ +// 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 + +import "github.com/ProtonMail/proton-bridge/internal/users" + +type Configer interface { + users.Configer + StoreFactoryConfiger +} + +type StoreFactoryConfiger interface { + GetDBDir() string + GetIMAPCachePath() string +} + +type PreferenceProvider interface { + Get(key string) string + GetBool(key string) bool + GetInt(key string) int + Set(key string, value string) +} diff --git a/internal/users/credentials/credentials.go b/internal/users/credentials/credentials.go index b269238b..f6736cc7 100644 --- a/internal/users/credentials/credentials.go +++ b/internal/users/credentials/credentials.go @@ -30,12 +30,17 @@ import ( "github.com/sirupsen/logrus" ) -const sep = "\x00" +const ( + sep = "\x00" + + itemLengthBridge = 9 + itemLengthImportExport = 6 // Old format for Import/Export. +) var ( log = logrus.WithField("pkg", "credentials") //nolint[gochecknoglobals] - ErrWrongFormat = errors.New("backend/creds: malformed password") + ErrWrongFormat = errors.New("malformed credentials") ) type Credentials struct { @@ -85,7 +90,7 @@ func (s *Credentials) Unmarshal(secret string) error { } items := strings.Split(string(b), sep) - if len(items) != 9 { + if len(items) != itemLengthBridge && len(items) != itemLengthImportExport { return ErrWrongFormat } @@ -93,16 +98,26 @@ func (s *Credentials) Unmarshal(secret string) error { s.Emails = items[1] s.APIToken = items[2] s.MailboxPassword = items[3] - s.BridgePassword = items[4] - s.Version = items[5] - if _, err = fmt.Sscan(items[6], &s.Timestamp); err != nil { - s.Timestamp = 0 - } - if s.IsHidden = false; items[7] == "1" { - s.IsHidden = true - } - if s.IsCombinedAddressMode = false; items[8] == "1" { - s.IsCombinedAddressMode = true + + switch len(items) { + case itemLengthBridge: + s.BridgePassword = items[4] + s.Version = items[5] + if _, err = fmt.Sscan(items[6], &s.Timestamp); err != nil { + s.Timestamp = 0 + } + if s.IsHidden = false; items[7] == "1" { + s.IsHidden = true + } + if s.IsCombinedAddressMode = false; items[8] == "1" { + s.IsCombinedAddressMode = true + } + + case itemLengthImportExport: + s.Version = items[4] + if _, err = fmt.Sscan(items[5], &s.Timestamp); err != nil { + s.Timestamp = 0 + } } return nil } diff --git a/internal/users/credentials/credentials_test.go b/internal/users/credentials/credentials_test.go new file mode 100644 index 00000000..348987fb --- /dev/null +++ b/internal/users/credentials/credentials_test.go @@ -0,0 +1,67 @@ +// 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 credentials + +import ( + "encoding/base64" + "fmt" + "strings" + "testing" + "time" + + r "github.com/stretchr/testify/require" +) + +var wantCredentials = Credentials{ + UserID: "1", + Name: "name", + Emails: "email1;email2", + APIToken: "token", + MailboxPassword: "mailbox pass", + BridgePassword: "bridge pass", + Version: "k11", + Timestamp: time.Now().Unix(), + IsHidden: false, + IsCombinedAddressMode: false, +} + +func TestUnmarshallBridge(t *testing.T) { + encoded := wantCredentials.Marshal() + haveCredentials := Credentials{UserID: "1"} + r.NoError(t, haveCredentials.Unmarshal(encoded)) + r.Equal(t, wantCredentials, haveCredentials) +} + +func TestUnmarshallImportExport(t *testing.T) { + items := []string{ + wantCredentials.Name, + wantCredentials.Emails, + wantCredentials.APIToken, + wantCredentials.MailboxPassword, + "k11", + fmt.Sprint(wantCredentials.Timestamp), + } + + str := strings.Join(items, sep) + encoded := base64.StdEncoding.EncodeToString([]byte(str)) + + haveCredentials := Credentials{UserID: "1"} + haveCredentials.BridgePassword = wantCredentials.BridgePassword // This one is not used. + r.NoError(t, haveCredentials.Unmarshal(encoded)) + r.Equal(t, wantCredentials, haveCredentials) +} diff --git a/internal/users/mocks/mocks.go b/internal/users/mocks/mocks.go index 624883f6..524d3273 100644 --- a/internal/users/mocks/mocks.go +++ b/internal/users/mocks/mocks.go @@ -1,15 +1,15 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/ProtonMail/proton-bridge/internal/users (interfaces: Configer,PreferenceProvider,PanicHandler,ClientManager,CredentialsStorer) +// Source: github.com/ProtonMail/proton-bridge/internal/users (interfaces: Configer,PanicHandler,ClientManager,CredentialsStorer,StoreMaker) // Package mocks is a generated GoMock package. package mocks import ( - reflect "reflect" - + store "github.com/ProtonMail/proton-bridge/internal/store" credentials "github.com/ProtonMail/proton-bridge/internal/users/credentials" pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockConfiger is a mock of Configer interface @@ -63,34 +63,6 @@ func (mr *MockConfigerMockRecorder) GetAPIConfig() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAPIConfig", reflect.TypeOf((*MockConfiger)(nil).GetAPIConfig)) } -// GetDBDir mocks base method -func (m *MockConfiger) GetDBDir() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetDBDir") - ret0, _ := ret[0].(string) - return ret0 -} - -// GetDBDir indicates an expected call of GetDBDir -func (mr *MockConfigerMockRecorder) GetDBDir() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDBDir", reflect.TypeOf((*MockConfiger)(nil).GetDBDir)) -} - -// GetIMAPCachePath mocks base method -func (m *MockConfiger) GetIMAPCachePath() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetIMAPCachePath") - ret0, _ := ret[0].(string) - return ret0 -} - -// GetIMAPCachePath indicates an expected call of GetIMAPCachePath -func (mr *MockConfigerMockRecorder) GetIMAPCachePath() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetIMAPCachePath", reflect.TypeOf((*MockConfiger)(nil).GetIMAPCachePath)) -} - // GetVersion mocks base method func (m *MockConfiger) GetVersion() string { m.ctrl.T.Helper() @@ -105,83 +77,6 @@ func (mr *MockConfigerMockRecorder) GetVersion() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVersion", reflect.TypeOf((*MockConfiger)(nil).GetVersion)) } -// MockPreferenceProvider is a mock of PreferenceProvider interface -type MockPreferenceProvider struct { - ctrl *gomock.Controller - recorder *MockPreferenceProviderMockRecorder -} - -// MockPreferenceProviderMockRecorder is the mock recorder for MockPreferenceProvider -type MockPreferenceProviderMockRecorder struct { - mock *MockPreferenceProvider -} - -// NewMockPreferenceProvider creates a new mock instance -func NewMockPreferenceProvider(ctrl *gomock.Controller) *MockPreferenceProvider { - mock := &MockPreferenceProvider{ctrl: ctrl} - mock.recorder = &MockPreferenceProviderMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockPreferenceProvider) EXPECT() *MockPreferenceProviderMockRecorder { - return m.recorder -} - -// Get mocks base method -func (m *MockPreferenceProvider) Get(arg0 string) string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", arg0) - ret0, _ := ret[0].(string) - return ret0 -} - -// Get indicates an expected call of Get -func (mr *MockPreferenceProviderMockRecorder) Get(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockPreferenceProvider)(nil).Get), arg0) -} - -// GetBool mocks base method -func (m *MockPreferenceProvider) GetBool(arg0 string) bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetBool", arg0) - ret0, _ := ret[0].(bool) - return ret0 -} - -// GetBool indicates an expected call of GetBool -func (mr *MockPreferenceProviderMockRecorder) GetBool(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBool", reflect.TypeOf((*MockPreferenceProvider)(nil).GetBool), arg0) -} - -// GetInt mocks base method -func (m *MockPreferenceProvider) GetInt(arg0 string) int { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetInt", arg0) - ret0, _ := ret[0].(int) - return ret0 -} - -// GetInt indicates an expected call of GetInt -func (mr *MockPreferenceProviderMockRecorder) GetInt(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetInt", reflect.TypeOf((*MockPreferenceProvider)(nil).GetInt), arg0) -} - -// Set mocks base method -func (m *MockPreferenceProvider) Set(arg0, arg1 string) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "Set", arg0, arg1) -} - -// Set indicates an expected call of Set -func (mr *MockPreferenceProviderMockRecorder) Set(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Set", reflect.TypeOf((*MockPreferenceProvider)(nil).Set), arg0, arg1) -} - // MockPanicHandler is a mock of PanicHandler interface type MockPanicHandler struct { ctrl *gomock.Controller @@ -483,3 +378,55 @@ func (mr *MockCredentialsStorerMockRecorder) UpdateToken(arg0, arg1 interface{}) mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateToken", reflect.TypeOf((*MockCredentialsStorer)(nil).UpdateToken), arg0, arg1) } + +// MockStoreMaker is a mock of StoreMaker interface +type MockStoreMaker struct { + ctrl *gomock.Controller + recorder *MockStoreMakerMockRecorder +} + +// MockStoreMakerMockRecorder is the mock recorder for MockStoreMaker +type MockStoreMakerMockRecorder struct { + mock *MockStoreMaker +} + +// NewMockStoreMaker creates a new mock instance +func NewMockStoreMaker(ctrl *gomock.Controller) *MockStoreMaker { + mock := &MockStoreMaker{ctrl: ctrl} + mock.recorder = &MockStoreMakerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockStoreMaker) EXPECT() *MockStoreMakerMockRecorder { + return m.recorder +} + +// New mocks base method +func (m *MockStoreMaker) New(arg0 store.BridgeUser) (*store.Store, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "New", arg0) + ret0, _ := ret[0].(*store.Store) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// New indicates an expected call of New +func (mr *MockStoreMakerMockRecorder) New(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "New", reflect.TypeOf((*MockStoreMaker)(nil).New), arg0) +} + +// Remove mocks base method +func (m *MockStoreMaker) Remove(arg0 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Remove", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// Remove indicates an expected call of Remove +func (mr *MockStoreMakerMockRecorder) Remove(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Remove", reflect.TypeOf((*MockStoreMaker)(nil).Remove), arg0) +} diff --git a/internal/users/types.go b/internal/users/types.go index 0e9a4e7c..73ce1119 100644 --- a/internal/users/types.go +++ b/internal/users/types.go @@ -18,25 +18,17 @@ package users import ( + "github.com/ProtonMail/proton-bridge/internal/store" "github.com/ProtonMail/proton-bridge/internal/users/credentials" "github.com/ProtonMail/proton-bridge/pkg/pmapi" ) type Configer interface { ClearData() error - GetDBDir() string GetVersion() string - GetIMAPCachePath() string GetAPIConfig() *pmapi.ClientConfig } -type PreferenceProvider interface { - Get(key string) string - GetBool(key string) bool - GetInt(key string) int - Set(key string, value string) -} - type PanicHandler interface { HandlePanic() } @@ -62,3 +54,8 @@ type ClientManager interface { CheckConnection() error SetUserAgent(clientName, clientVersion, os string) } + +type StoreMaker interface { + New(user store.BridgeUser) (*store.Store, error) + Remove(userID string) error +} diff --git a/internal/users/user.go b/internal/users/user.go index f82ceb8c..9e217ed7 100644 --- a/internal/users/user.go +++ b/internal/users/user.go @@ -18,8 +18,6 @@ package users import ( - "fmt" - "path/filepath" "runtime" "strings" "sync" @@ -47,9 +45,8 @@ type User struct { imapUpdatesChannel chan imapBackend.Update - store *store.Store - storeCache *store.Cache - storePath string + storeFactory StoreMaker + store *store.Store userID string creds *credentials.Credentials @@ -68,8 +65,7 @@ func newUser( eventListener listener.Listener, credStorer CredentialsStorer, clientManager ClientManager, - storeCache *store.Cache, - storeDir string, + storeFactory StoreMaker, ) (u *User, err error) { log := log.WithField("user", userID) log.Debug("Creating or loading user") @@ -85,8 +81,7 @@ func newUser( listener: eventListener, credStorer: credStorer, clientManager: clientManager, - storeCache: storeCache, - storePath: getUserStorePath(storeDir, userID), + storeFactory: storeFactory, userID: userID, creds: creds, } @@ -140,7 +135,7 @@ func (u *User) init(idleUpdates chan imapBackend.Update) (err error) { } u.store = nil } - store, err := store.New(u.panicHandler, u, u.clientManager, u.listener, u.storePath, u.storeCache) + store, err := u.storeFactory.New(u) if err != nil { return errors.Wrap(err, "failed to create store") } @@ -267,7 +262,7 @@ func (u *User) clearStore() error { } } else { u.log.Warn("Store is not initialized: cleaning up store files manually") - if err := store.RemoveStore(u.storeCache, u.storePath, u.userID); err != nil { + if err := u.storeFactory.Remove(u.userID); err != nil { return errors.Wrap(err, "failed to remove store manually") } } @@ -287,12 +282,6 @@ func (u *User) closeStore() error { return nil } -// getUserStorePath returns the file path of the store database for the given userID. -func getUserStorePath(storeDir string, userID string) (path string) { - fileName := fmt.Sprintf("mailbox-%v.db", userID) - return filepath.Join(storeDir, fileName) -} - // GetTemporaryPMAPIClient returns an authorised PMAPI client. // Do not use! It's only for backward compatibility of old SMTP and IMAP implementations. // After proper refactor of SMTP and IMAP remove this method. diff --git a/internal/users/user_credentials_test.go b/internal/users/user_credentials_test.go index 89415d3a..9521078b 100644 --- a/internal/users/user_credentials_test.go +++ b/internal/users/user_credentials_test.go @@ -190,11 +190,7 @@ func TestCheckBridgeLoginLoggedOut(t *testing.T) { m.credentialsStore.EXPECT().Get("user").Return(testCredentialsDisconnected, nil) - user, err := newUser( - m.PanicHandler, "user", - m.eventListener, m.credentialsStore, - m.clientManager, m.storeCache, "/tmp", - ) + user, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeMaker) assert.NoError(t, err) m.clientManager.EXPECT().GetClient(gomock.Any()).Return(m.pmapiClient).MinTimes(1) diff --git a/internal/users/user_new_test.go b/internal/users/user_new_test.go index 6a7331b7..c70e0f8f 100644 --- a/internal/users/user_new_test.go +++ b/internal/users/user_new_test.go @@ -34,7 +34,7 @@ func TestNewUserNoCredentialsStore(t *testing.T) { m.credentialsStore.EXPECT().Get("user").Return(nil, errors.New("fail")) - _, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeCache, "/tmp") + _, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeMaker) a.Error(t, err) } @@ -162,7 +162,7 @@ func TestNewUser(t *testing.T) { } func checkNewUserHasCredentials(creds *credentials.Credentials, m mocks) { - user, _ := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeCache, "/tmp") + user, _ := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeMaker) defer cleanUpUserData(user) _ = user.init(nil) diff --git a/internal/users/user_test.go b/internal/users/user_test.go index 0367b02e..73bcde1b 100644 --- a/internal/users/user_test.go +++ b/internal/users/user_test.go @@ -38,7 +38,7 @@ func testNewUser(m mocks) *User { m.pmapiClient.EXPECT().ListMessages(gomock.Any()).Return([]*pmapi.Message{}, 0, nil).MaxTimes(1), ) - user, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeCache, "/tmp") + user, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeMaker) assert.NoError(m.t, err) err = user.init(nil) @@ -60,7 +60,7 @@ func testNewUserForLogout(m mocks) *User { m.pmapiClient.EXPECT().ListMessages(gomock.Any()).Return([]*pmapi.Message{}, 0, nil).MaxTimes(1), ) - user, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeCache, "/tmp") + user, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeMaker) assert.NoError(m.t, err) err = user.init(nil) diff --git a/internal/users/users.go b/internal/users/users.go index 9bab233f..d2679494 100644 --- a/internal/users/users.go +++ b/internal/users/users.go @@ -19,15 +19,11 @@ package users import ( - "strconv" "strings" "sync" - "time" "github.com/ProtonMail/proton-bridge/internal/events" "github.com/ProtonMail/proton-bridge/internal/metrics" - "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/pmapi" imapBackend "github.com/emersion/go-imap/backend" @@ -44,12 +40,11 @@ var ( // Users is a struct handling users. type Users struct { config Configer - pref PreferenceProvider panicHandler PanicHandler events listener.Listener clientManager ClientManager credStorer CredentialsStorer - storeCache *store.Cache + storeFactory StoreMaker // users is a list of accounts that have been added to the app. // They are stored sorted in the credentials store in the order @@ -70,33 +65,26 @@ type Users struct { func New( config Configer, - pref PreferenceProvider, panicHandler PanicHandler, eventListener listener.Listener, clientManager ClientManager, credStorer CredentialsStorer, + storeFactory StoreMaker, ) *Users { log.Trace("Creating new users") u := &Users{ config: config, - pref: pref, panicHandler: panicHandler, events: eventListener, clientManager: clientManager, credStorer: credStorer, - storeCache: store.NewCache(config.GetIMAPCachePath()), + storeFactory: storeFactory, idleUpdates: make(chan imapBackend.Update), lock: sync.RWMutex{}, stopAll: make(chan struct{}), } - // Allow DoH before starting the app if the user has previously set this setting. - // This allows us to start even if protonmail is blocked. - if pref.GetBool(preferences.AllowProxyKey) { - u.AllowProxy() - } - go func() { defer panicHandler.HandlePanic() u.watchAppOutdated() @@ -107,45 +95,15 @@ func New( u.watchAPIAuths() }() - go u.heartbeat() - if u.credStorer == nil { log.Error("No credentials store is available") } else if err := u.loadUsersFromCredentialsStore(); err != nil { log.WithError(err).Error("Could not load all users from credentials store") } - if pref.GetBool(preferences.FirstStartKey) { - u.SendMetric(metrics.New(metrics.Setup, metrics.FirstStart, metrics.Label(config.GetVersion()))) - } - return u } -// heartbeat sends a heartbeat signal once a day. -func (u *Users) heartbeat() { - ticker := time.NewTicker(1 * time.Minute) - - for { - select { - case <-ticker.C: - next, err := strconv.ParseInt(u.pref.Get(preferences.NextHeartbeatKey), 10, 64) - if err != nil { - continue - } - nextTime := time.Unix(next, 0) - if time.Now().After(nextTime) { - u.SendMetric(metrics.New(metrics.Heartbeat, metrics.Daily, metrics.NoLabel)) - nextTime = nextTime.Add(24 * time.Hour) - u.pref.Set(preferences.NextHeartbeatKey, strconv.FormatInt(nextTime.Unix(), 10)) - } - - case <-u.stopAll: - return - } - } -} - func (u *Users) loadUsersFromCredentialsStore() (err error) { u.lock.Lock() defer u.lock.Unlock() @@ -158,7 +116,7 @@ func (u *Users) loadUsersFromCredentialsStore() (err error) { for _, userID := range userIDs { l := log.WithField("user", userID) - user, newUserErr := newUser(u.panicHandler, userID, u.events, u.credStorer, u.clientManager, u.storeCache, u.config.GetDBDir()) + user, newUserErr := newUser(u.panicHandler, userID, u.events, u.credStorer, u.clientManager, u.storeFactory) if newUserErr != nil { l.WithField("user", userID).WithError(newUserErr).Warn("Could not load user, skipping") continue @@ -345,7 +303,7 @@ func (u *Users) addNewUser(apiUser *pmapi.User, auth *pmapi.Auth, hashedPassword return errors.Wrap(err, "failed to add user to credentials store") } - user, err := newUser(u.panicHandler, apiUser.ID, u.events, u.credStorer, u.clientManager, u.storeCache, u.config.GetDBDir()) + user, err := newUser(u.panicHandler, apiUser.ID, u.events, u.credStorer, u.clientManager, u.storeFactory) if err != nil { return errors.Wrap(err, "failed to create user") } diff --git a/internal/users/users_new_test.go b/internal/users/users_new_test.go index 2dceb5ba..f151800f 100644 --- a/internal/users/users_new_test.go +++ b/internal/users/users_new_test.go @@ -22,8 +22,6 @@ import ( "testing" "github.com/ProtonMail/proton-bridge/internal/events" - "github.com/ProtonMail/proton-bridge/internal/metrics" - "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/internal/users/credentials" "github.com/ProtonMail/proton-bridge/pkg/pmapi" gomock "github.com/golang/mock/gomock" @@ -162,13 +160,7 @@ func TestNewUsersFirstStart(t *testing.T) { m := initMocks(t) defer m.ctrl.Finish() - gomock.InOrder( - m.credentialsStore.EXPECT().List().Return([]string{}, nil), - m.prefProvider.EXPECT().GetBool(preferences.FirstStartKey).Return(true), - m.clientManager.EXPECT().GetAnonymousClient().Return(m.pmapiClient), - m.pmapiClient.EXPECT().SendSimpleMetric(string(metrics.Setup), string(metrics.FirstStart), gomock.Any()), - m.pmapiClient.EXPECT().Logout(), - ) + m.credentialsStore.EXPECT().List().Return([]string{}, nil) testNewUsers(t, m) } diff --git a/internal/users/users_test.go b/internal/users/users_test.go index 6709d79e..10110a42 100644 --- a/internal/users/users_test.go +++ b/internal/users/users_test.go @@ -26,8 +26,6 @@ import ( "time" "github.com/ProtonMail/proton-bridge/internal/events" - "github.com/ProtonMail/proton-bridge/internal/metrics" - "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/internal/store" "github.com/ProtonMail/proton-bridge/internal/users/credentials" usersmocks "github.com/ProtonMail/proton-bridge/internal/users/mocks" @@ -133,9 +131,9 @@ type mocks struct { ctrl *gomock.Controller config *usersmocks.MockConfiger PanicHandler *usersmocks.MockPanicHandler - prefProvider *usersmocks.MockPreferenceProvider clientManager *usersmocks.MockClientManager credentialsStore *usersmocks.MockCredentialsStorer + storeMaker *usersmocks.MockStoreMaker eventListener *MockListener pmapiClient *pmapimocks.MockClient @@ -174,9 +172,9 @@ func initMocks(t *testing.T) mocks { ctrl: mockCtrl, config: usersmocks.NewMockConfiger(mockCtrl), PanicHandler: usersmocks.NewMockPanicHandler(mockCtrl), - prefProvider: usersmocks.NewMockPreferenceProvider(mockCtrl), clientManager: usersmocks.NewMockClientManager(mockCtrl), credentialsStore: usersmocks.NewMockCredentialsStorer(mockCtrl), + storeMaker: usersmocks.NewMockStoreMaker(mockCtrl), eventListener: NewMockListener(mockCtrl), pmapiClient: pmapimocks.NewMockClient(mockCtrl), @@ -184,14 +182,17 @@ func initMocks(t *testing.T) mocks { storeCache: store.NewCache(cacheFile.Name()), } - // Ignore heartbeat calls because they always happen. - m.pmapiClient.EXPECT().SendSimpleMetric(string(metrics.Heartbeat), gomock.Any(), gomock.Any()).AnyTimes() - m.prefProvider.EXPECT().Get(preferences.NextHeartbeatKey).AnyTimes() - m.prefProvider.EXPECT().Set(preferences.NextHeartbeatKey, gomock.Any()).AnyTimes() - // Called during clean-up. m.PanicHandler.EXPECT().HandlePanic().AnyTimes() + // Set up store factory. + m.storeMaker.EXPECT().New(gomock.Any()).DoAndReturn(func(user store.BridgeUser) (*store.Store, error) { + dbFile, err := ioutil.TempFile("", "bridge-store-db-*.db") + require.NoError(t, err, "could not get temporary file for store db") + return store.New(m.PanicHandler, user, m.clientManager, m.eventListener, dbFile.Name(), m.storeCache) + }).AnyTimes() + m.storeMaker.EXPECT().Remove(gomock.Any()).AnyTimes() + return m } @@ -236,19 +237,12 @@ func testNewUsersWithUsers(t *testing.T, m mocks) *Users { return users } -func testNewUsers(t *testing.T, m mocks) *Users { - cacheFile, err := ioutil.TempFile("", "bridge-store-cache-*.db") - require.NoError(t, err, "could not get temporary file for store cache") - - m.prefProvider.EXPECT().GetBool(preferences.FirstStartKey).Return(false).AnyTimes() - m.prefProvider.EXPECT().GetBool(preferences.AllowProxyKey).Return(false).AnyTimes() - m.config.EXPECT().GetDBDir().Return("/tmp").AnyTimes() - m.config.EXPECT().GetIMAPCachePath().Return(cacheFile.Name()).AnyTimes() +func testNewUsers(t *testing.T, m mocks) *Users { //nolint[unparam] m.config.EXPECT().GetVersion().Return("ver").AnyTimes() m.eventListener.EXPECT().Add(events.UpgradeApplicationEvent, gomock.Any()) m.clientManager.EXPECT().GetAuthUpdateChannel().Return(make(chan pmapi.ClientAuth)) - users := New(m.config, m.prefProvider, m.PanicHandler, m.eventListener, m.clientManager, m.credentialsStore) + users := New(m.config, m.PanicHandler, m.eventListener, m.clientManager, m.credentialsStore, m.storeMaker) waitForEvents()