From ba935a6cce2b92eb1a8b1a130eed964c5f45602f Mon Sep 17 00:00:00 2001 From: Leander Beernaert Date: Tue, 21 Nov 2023 11:58:21 +0100 Subject: [PATCH] fix(GODT-3129): Bad Event during after address order change When syncing an account, if the user creates a new address and then changes it to be the default address in combined address mode we need to update the connector maps so that the new primary address ID can be found in that map. Includes https://github.com/ProtonMail/go-proton-api/pull/130 --- go.mod | 2 +- go.sum | 4 +- internal/bridge/sync_test.go | 49 +++++++++++++++++++ internal/services/imapservice/service.go | 5 ++ .../imapservice/service_address_events.go | 28 +++++++++++ 5 files changed, 85 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index a247cae7..7182c146 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/Masterminds/semver/v3 v3.2.0 github.com/ProtonMail/gluon v0.17.1-0.20231114153341-2ecbdd2739f7 github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a - github.com/ProtonMail/go-proton-api v0.4.1-0.20231120150119-65479b90c447 + github.com/ProtonMail/go-proton-api v0.4.1-0.20231121110002-44e8224f0baf github.com/ProtonMail/gopenpgp/v2 v2.7.4-proton github.com/PuerkitoBio/goquery v1.8.1 github.com/abiosoft/ishell v2.0.0+incompatible diff --git a/go.sum b/go.sum index c1e72ec9..18c9332a 100644 --- a/go.sum +++ b/go.sum @@ -36,8 +36,8 @@ github.com/ProtonMail/go-message v0.13.1-0.20230526094639-b62c999c85b7 h1:+j+Kd/ github.com/ProtonMail/go-message v0.13.1-0.20230526094639-b62c999c85b7/go.mod h1:NBAn21zgCJ/52WLDyed18YvYFm5tEoeDauubFqLokM4= github.com/ProtonMail/go-mime v0.0.0-20230322103455-7d82a3887f2f h1:tCbYj7/299ekTTXpdwKYF8eBlsYsDVoggDAuAjoK66k= github.com/ProtonMail/go-mime v0.0.0-20230322103455-7d82a3887f2f/go.mod h1:gcr0kNtGBqin9zDW9GOHcVntrwnjrK+qdJ06mWYBybw= -github.com/ProtonMail/go-proton-api v0.4.1-0.20231120150119-65479b90c447 h1:Oov+xnbkN/yeR8NYBJUcyIUJdPEeDgzDbew9UEUUN4g= -github.com/ProtonMail/go-proton-api v0.4.1-0.20231120150119-65479b90c447/go.mod h1:WEXJqj5DSc2YI77SgXdpMY0nk33Qy92Vu2r4tOEazA8= +github.com/ProtonMail/go-proton-api v0.4.1-0.20231121110002-44e8224f0baf h1:BxajItLG4iJquxT+gnR0zK/d9cT3VDRIuVp0szwKmSg= +github.com/ProtonMail/go-proton-api v0.4.1-0.20231121110002-44e8224f0baf/go.mod h1:WEXJqj5DSc2YI77SgXdpMY0nk33Qy92Vu2r4tOEazA8= github.com/ProtonMail/go-smtp v0.0.0-20231109081432-2b3d50599865 h1:EP1gnxLL5Z7xBSymE9nSTM27nRYINuvssAtDmG0suD8= github.com/ProtonMail/go-smtp v0.0.0-20231109081432-2b3d50599865/go.mod h1:qm27SGYgoIPRot6ubfQ/GpiPy/g3PaZAVRxiO/sDUgQ= github.com/ProtonMail/go-srp v0.0.7 h1:Sos3Qk+th4tQR64vsxGIxYpN3rdnG9Wf9K4ZloC1JrI= diff --git a/internal/bridge/sync_test.go b/internal/bridge/sync_test.go index 07c0988a..6f0604bb 100644 --- a/internal/bridge/sync_test.go +++ b/internal/bridge/sync_test.go @@ -641,6 +641,55 @@ func TestBridge_CorruptedVaultClearsPreviousIMAPSyncState(t *testing.T) { }) } +func TestBridge_AddressOrderChangeDuringSyncInCombinedModeDoesNotTriggerBadEventOnNewMessage(t *testing.T) { + withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { + // Create a user. + userID, addrID, err := s.CreateUser("user", password) + require.NoError(t, err) + + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) { + userInfoChanged, done := chToType[events.Event, events.UserChanged](bridge.GetEvents(events.UserChanged{})) + defer done() + + withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) { + createNumMessages(ctx, t, c, addrID, proton.InboxLabel, 300) + }) + + _, err := bridge.LoginFull(ctx, "user", password, nil, nil) + require.NoError(t, err) + + info, err := bridge.GetUserInfo(userID) + require.NoError(t, err) + require.Equal(t, 1, len(info.Addresses)) + require.Equal(t, info.Addresses[0], "user@proton.local") + + addrID2, err := s.CreateAddress(userID, "foo@"+s.GetDomain(), password) + require.NoError(t, err) + + require.NoError(t, s.SetAddressOrder(userID, []string{addrID2, addrID})) + + withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) { + createNumMessages(ctx, t, c, addrID2, proton.InboxLabel, 1) + }) + + // Since we can't intercept events at this time, we sleep for a bit to make sure the + // new message does not get combined into the event below. This ensures the newly created + // goes through the full code flow which triggered the original bad event. + time.Sleep(time.Second) + require.NoError(t, s.SetAddressOrder(userID, []string{addrID, addrID2})) + + for i := 0; i < 2; i++ { + select { + case <-ctx.Done(): + return + case e := <-userInfoChanged: + require.Equal(t, userID, e.UserID) + } + } + }) + }) +} + func withClient(ctx context.Context, t *testing.T, s *server.Server, username string, password []byte, fn func(context.Context, *proton.Client)) { //nolint:unparam m := proton.New( proton.WithHostURL(s.GetHostURL()), diff --git a/internal/services/imapservice/service.go b/internal/services/imapservice/service.go index 20acd9c3..e3a5ab7d 100644 --- a/internal/services/imapservice/service.go +++ b/internal/services/imapservice/service.go @@ -274,6 +274,10 @@ func (s *Service) HandleRefreshEvent(ctx context.Context, _ proton.RefreshFlag) return err } + if err := s.rebuildConnectors(); err != nil { + return err + } + if err := s.syncStateProvider.ClearSyncStatus(ctx); err != nil { return fmt.Errorf("failed to clear sync status:%w", err) } @@ -292,6 +296,7 @@ func (s *Service) HandleUserEvent(_ context.Context, user *proton.User) error { return s.identityState.Write(func(identity *useridentity.State) error { identity.OnUserEvent(*user) + return nil }) } diff --git a/internal/services/imapservice/service_address_events.go b/internal/services/imapservice/service_address_events.go index 78abe4a3..44211920 100644 --- a/internal/services/imapservice/service_address_events.go +++ b/internal/services/imapservice/service_address_events.go @@ -24,12 +24,18 @@ import ( "github.com/ProtonMail/go-proton-api" "github.com/ProtonMail/proton-bridge/v3/internal/services/useridentity" "github.com/ProtonMail/proton-bridge/v3/internal/usertypes" + "github.com/sirupsen/logrus" ) func (s *Service) HandleAddressEvents(ctx context.Context, events []proton.AddressEvent) error { s.log.Debug("handling address event") if s.addressMode == usertypes.AddressModeCombined { + oldPrimaryAddr, err := s.identityState.GetPrimaryAddress() + if err != nil { + return fmt.Errorf("failed to get primary addr: %w", err) + } + if err := s.identityState.Write(func(identity *useridentity.State) error { identity.OnAddressEvents(events) return nil @@ -38,6 +44,28 @@ func (s *Service) HandleAddressEvents(ctx context.Context, events []proton.Addre return err } + newPrimaryAddr, err := s.identityState.GetPrimaryAddress() + if err != nil { + return fmt.Errorf("failed to get primary addr after update: %w", err) + } + + if oldPrimaryAddr.ID == newPrimaryAddr.ID { + return nil + } + + connector, ok := s.connectors[oldPrimaryAddr.ID] + if !ok { + return fmt.Errorf("could not find old primary addr conncetor after default address change") + } + + s.connectors[newPrimaryAddr.ID] = connector + delete(s.connectors, oldPrimaryAddr.ID) + + s.log.WithFields(logrus.Fields{ + "old": oldPrimaryAddr.Email, + "new": newPrimaryAddr.Email, + }).Debug("Primary address changed") + return nil }