From cdff2ef79214b60f6e7208e6c82ef27f025ef936 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 8 Feb 2023 16:09:24 +0100 Subject: [PATCH] fix(GODT-2351): Bump GPA to properly handle net.OpError and add tests --- go.mod | 2 +- go.sum | 4 +-- internal/bridge/user.go | 1 + internal/bridge/user_event_test.go | 58 ++++++++++++++++++++++++++++++ internal/bridge/user_test.go | 46 ++++++++++++++++++++++++ 5 files changed, 108 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index fa2a2b88..f8a5cf6d 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/Masterminds/semver/v3 v3.1.1 github.com/ProtonMail/gluon v0.14.2-0.20230206162331-cf36d870802b github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a - github.com/ProtonMail/go-proton-api v0.3.1-0.20230207140717-7eee7f487b13 + github.com/ProtonMail/go-proton-api v0.3.1-0.20230208134215-f345f51a344b github.com/ProtonMail/go-rfc5322 v0.11.0 github.com/ProtonMail/gopenpgp/v2 v2.4.10 github.com/PuerkitoBio/goquery v1.8.0 diff --git a/go.sum b/go.sum index 553ce168..ef6a6f6b 100644 --- a/go.sum +++ b/go.sum @@ -41,8 +41,8 @@ github.com/ProtonMail/go-message v0.0.0-20210611055058-fabeff2ec753/go.mod h1:NB github.com/ProtonMail/go-mime v0.0.0-20220302105931-303f85f7fe0f/go.mod h1:NYt+V3/4rEeDuaev/zw1zCq8uqVEuPHzDPo3OZrlGJ4= github.com/ProtonMail/go-mime v0.0.0-20220429130430-2192574d760f h1:4IWzKjHzZxdrW9k4zl/qCwenOVHDbVDADPPHFLjs0Oc= github.com/ProtonMail/go-mime v0.0.0-20220429130430-2192574d760f/go.mod h1:qRZgbeASl2a9OwmsV85aWwRqic0NHPh+9ewGAzb4cgM= -github.com/ProtonMail/go-proton-api v0.3.1-0.20230207140717-7eee7f487b13 h1:VmLI4lVyYGJPQXAWaZXxvu0coEooVzqdTU185cq8ULo= -github.com/ProtonMail/go-proton-api v0.3.1-0.20230207140717-7eee7f487b13/go.mod h1:JUo5IQG0hNuPRuDpOUsCOvtee6UjTEHHF1QN2i8RSos= +github.com/ProtonMail/go-proton-api v0.3.1-0.20230208134215-f345f51a344b h1:fHDSfYQ0/zmfRBGDyhNOXtoIDVaL8elarYaiJAGgtj8= +github.com/ProtonMail/go-proton-api v0.3.1-0.20230208134215-f345f51a344b/go.mod h1:JUo5IQG0hNuPRuDpOUsCOvtee6UjTEHHF1QN2i8RSos= github.com/ProtonMail/go-rfc5322 v0.11.0 h1:o5Obrm4DpmQEffvgsVqG6S4BKwC1Wat+hYwjIp2YcCY= github.com/ProtonMail/go-rfc5322 v0.11.0/go.mod h1:6oOKr0jXvpoE6pwTx/HukigQpX2J9WUf6h0auplrFTw= github.com/ProtonMail/go-srp v0.0.5 h1:xhUioxZgDbCnpo9JehyFhwwsn9JLWkUGfB0oiKXgiGg= diff --git a/internal/bridge/user.go b/internal/bridge/user.go index 758fa1f2..6aed8503 100644 --- a/internal/bridge/user.go +++ b/internal/bridge/user.go @@ -380,6 +380,7 @@ func (bridge *Bridge) loadUser(ctx context.Context, user *vault.User) error { logrus.WithError(err).Warn("Failed to clear user secrets") } } + return fmt.Errorf("failed to create API client: %w", err) } diff --git a/internal/bridge/user_event_test.go b/internal/bridge/user_event_test.go index e8d06d31..e7a02a28 100644 --- a/internal/bridge/user_event_test.go +++ b/internal/bridge/user_event_test.go @@ -20,6 +20,7 @@ package bridge_test import ( "context" "fmt" + "net" "net/http" "strings" "sync/atomic" @@ -291,6 +292,63 @@ func TestBridge_User_Network_NoBadEvents(t *testing.T) { }) } +func TestBridge_User_DropConn_NoBadEvent(t *testing.T) { + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + + dropListener := proton.NewListener(l, proton.NewDropConn) + defer func() { _ = dropListener.Close() }() + + withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { + // Create a user. + _, addrID, err := s.CreateUser("user", password) + require.NoError(t, err) + + // Create 10 messages for the user. + withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) { + createNumMessages(ctx, t, c, addrID, proton.InboxLabel, 10) + }) + + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { + userLoginAndSync(ctx, t, bridge, "user", password) + + mocks.Reporter.EXPECT().ReportMessageWithContext(gomock.Any(), gomock.Any()).AnyTimes() + + // Create 10 more messages for the user, generating events. + withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) { + createNumMessages(ctx, t, c, addrID, proton.InboxLabel, 10) + }) + + var count int + + // The first 10 times bridge attempts to sync any of the messages, drop the connection. + s.AddStatusHook(func(req *http.Request) (int, bool) { + if strings.Contains(req.URL.Path, "/mail/v4/messages") { + if count++; count < 10 { + dropListener.DropAll() + } + } + + return 0, false + }) + + info, err := bridge.QueryUserInfo("user") + require.NoError(t, err) + + client, err := client.Dial(fmt.Sprintf("%v:%v", constants.Host, bridge.GetIMAPPort())) + require.NoError(t, err) + require.NoError(t, client.Login(info.Addresses[0], string(info.BridgePass))) + defer func() { _ = client.Logout() }() + + // The IMAP client will eventually see 20 messages. + require.Eventually(t, func() bool { + status, err := client.Status("INBOX", []imap.StatusItem{imap.StatusMessages}) + return err == nil && status.Messages == 20 + }, 10*time.Second, 100*time.Millisecond) + }) + }, server.WithListener(dropListener)) +} + // userLoginAndSync logs in user and waits until user is fully synced. func userLoginAndSync( ctx context.Context, diff --git a/internal/bridge/user_test.go b/internal/bridge/user_test.go index 4c70f3a9..417e7bd7 100644 --- a/internal/bridge/user_test.go +++ b/internal/bridge/user_test.go @@ -20,6 +20,8 @@ package bridge_test import ( "context" "fmt" + "net" + "net/http" "testing" "time" @@ -61,6 +63,50 @@ func TestBridge_Login(t *testing.T) { }) } +func TestBridge_Login_DropConn(t *testing.T) { + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + + dropListener := proton.NewListener(l, proton.NewDropConn) + defer func() { _ = dropListener.Close() }() + + withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { + // Login the user. + userID, err := bridge.LoginFull(ctx, username, password, nil, nil) + require.NoError(t, err) + + // The user is now connected. + require.Equal(t, []string{userID}, bridge.GetUserIDs()) + require.Equal(t, []string{userID}, getConnectedUserIDs(t, bridge)) + }) + + // Whether to allow the user to be created. + var allowUser bool + + s.AddStatusHook(func(req *http.Request) (int, bool) { + // Drop any request to the users endpoint. + if !allowUser && req.URL.Path == "/core/v4/users" { + dropListener.DropAll() + } + + // After the ping request, allow the user to be created. + if req.URL.Path == "/tests/ping" { + allowUser = true + } + + return 0, false + }) + + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { + // The user is eventually connected. + require.Eventually(t, func() bool { + return len(bridge.GetUserIDs()) == 1 && len(getConnectedUserIDs(t, bridge)) == 1 + }, 5*time.Second, 100*time.Millisecond) + }) + }, server.WithListener(dropListener)) +} + func TestBridge_LoginTwice(t *testing.T) { withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {