From 63379001e348243fe0d3b0b1bbc6a0b74cc6db29 Mon Sep 17 00:00:00 2001 From: Jakub Date: Tue, 22 Feb 2022 16:41:32 +0100 Subject: [PATCH] GODT-1515: Do not crash when bridge users got disconnected. --- internal/users/user.go | 8 +++++++- test/context/context.go | 4 ++++ test/context/credentials.go | 24 ++++++++++++++++++++++++ test/features/start.feature | 9 +++++++++ test/users_setup_test.go | 6 ++++++ 5 files changed, 50 insertions(+), 1 deletion(-) diff --git a/internal/users/user.go b/internal/users/user.go index c5b09e3c..a5c946bf 100644 --- a/internal/users/user.go +++ b/internal/users/user.go @@ -345,6 +345,10 @@ func (u *User) GetAddressID(address string) (id string, err error) { return u.store.GetAddressID(address) } + if u.client == nil { + return "", errors.New("bridge account is not fully connected to server") + } + addresses := u.client.Addresses() pmapiAddress := addresses.ByEmail(address) if pmapiAddress != nil { @@ -473,7 +477,9 @@ func (u *User) Logout() error { return nil } - if err := u.client.AuthDelete(context.Background()); err != nil { + if u.client == nil { + u.log.Warn("Failed to delete auth: no client") + } else if err := u.client.AuthDelete(context.Background()); err != nil { u.log.WithError(err).Warn("Failed to delete auth") } diff --git a/test/context/context.go b/test/context/context.go index 426e3f84..409ae56d 100644 --- a/test/context/context.go +++ b/test/context/context.go @@ -172,3 +172,7 @@ func (ctx *TestContext) MessagePreparationStarted(username string) { func (ctx *TestContext) MessagePreparationFinished(username string) { ctx.pmapiController.UnlockEvents(username) } + +func (ctx *TestContext) CredentialsFailsOnWrite(shouldFail bool) { + ctx.credStore.(*fakeCredStore).failOnWrite = shouldFail +} diff --git a/test/context/credentials.go b/test/context/credentials.go index 1f899e90..c0f78fff 100644 --- a/test/context/credentials.go +++ b/test/context/credentials.go @@ -21,6 +21,7 @@ import ( "strings" "github.com/ProtonMail/proton-bridge/internal/users/credentials" + "github.com/pkg/errors" ) // bridgePassword is password to be used for IMAP or SMTP under tests. @@ -28,6 +29,8 @@ const bridgePassword = "bridgepassword" type fakeCredStore struct { credentials map[string]*credentials.Credentials + + failOnWrite bool } // newFakeCredStore returns a fake credentials store (optionally configured with the given credentials). @@ -52,6 +55,9 @@ func (c *fakeCredStore) List() (userIDs []string, err error) { } func (c *fakeCredStore) Add(userID, userName, uid, ref string, mailboxPassword []byte, emails []string) (*credentials.Credentials, error) { + if c.failOnWrite { + return nil, errors.New("An invalid attempt to change the owner of an item. (-25244)") + } bridgePassword := bridgePassword if c, ok := c.credentials[userID]; ok { bridgePassword = c.BridgePassword @@ -73,14 +79,23 @@ func (c *fakeCredStore) Get(userID string) (*credentials.Credentials, error) { } func (c *fakeCredStore) SwitchAddressMode(userID string) (*credentials.Credentials, error) { + if c.failOnWrite { + return nil, errors.New("An invalid attempt to change the owner of an item. (-25244)") + } return c.credentials[userID], nil } func (c *fakeCredStore) UpdateEmails(userID string, emails []string) (*credentials.Credentials, error) { + if c.failOnWrite { + return nil, errors.New("An invalid attempt to change the owner of an item. (-25244)") + } return c.credentials[userID], nil } func (c *fakeCredStore) UpdatePassword(userID string, password []byte) (*credentials.Credentials, error) { + if c.failOnWrite { + return nil, errors.New("An invalid attempt to change the owner of an item. (-25244)") + } creds, err := c.Get(userID) if err != nil { return nil, err @@ -90,6 +105,9 @@ func (c *fakeCredStore) UpdatePassword(userID string, password []byte) (*credent } func (c *fakeCredStore) UpdateToken(userID, uid, ref string) (*credentials.Credentials, error) { + if c.failOnWrite { + return nil, errors.New("An invalid attempt to change the owner of an item. (-25244)") + } creds, err := c.Get(userID) if err != nil { return nil, err @@ -99,12 +117,18 @@ func (c *fakeCredStore) UpdateToken(userID, uid, ref string) (*credentials.Crede } func (c *fakeCredStore) Logout(userID string) (*credentials.Credentials, error) { + if c.failOnWrite { + return nil, errors.New("An invalid attempt to change the owner of an item. (-25244)") + } c.credentials[userID].APIToken = "" c.credentials[userID].MailboxPassword = []byte{} return c.credentials[userID], nil } func (c *fakeCredStore) Delete(userID string) error { + if c.failOnWrite { + return errors.New("An invalid attempt to change the owner of an item. (-25244)") + } delete(c.credentials, userID) return nil } diff --git a/test/features/start.feature b/test/features/start.feature index 75a9da54..4a1d8061 100644 --- a/test/features/start.feature +++ b/test/features/start.feature @@ -79,3 +79,12 @@ Feature: Start bridge And "user" does not have loaded store And "user" does not have running event loop And "user" has zero space + + Scenario: Start with connected user, database file and internet connection, but no write access to credentials + Given there is user "user" which just logged in + And credentials are locked + And there is database file for "user" + When bridge starts + Then "user" is connected + When IMAP client authenticates "user" + Then IMAP response is "NO" diff --git a/test/users_setup_test.go b/test/users_setup_test.go index 4702f816..30eb2acd 100644 --- a/test/users_setup_test.go +++ b/test/users_setup_test.go @@ -33,6 +33,7 @@ func UsersSetupFeatureContext(s *godog.ScenarioContext) { s.Step(`^there is database file for "([^"]*)"$`, thereIsDatabaseFileForUser) s.Step(`^there is no database file for "([^"]*)"$`, thereIsNoDatabaseFileForUser) s.Step(`^there is "([^"]*)" in "([^"]*)" address mode$`, thereIsUserWithAddressMode) + s.Step(`^credentials? (?:are|is) locked$`, credentialsAreLocked) } func thereIsUser(bddUserID string) error { @@ -150,3 +151,8 @@ func thereIsUserWithAddressMode(bddUserID, wantAddressMode string) error { ctx.EventuallySyncIsFinishedForUsername(user.Username()) return nil } + +func credentialsAreLocked() error { + ctx.CredentialsFailsOnWrite(true) + return nil +}