GODT-2223: Treat label/message deletion errors as non-critical

This commit is contained in:
James Houlahan 2023-01-20 14:14:35 +01:00 committed by Jakub
parent 40cc6b54c9
commit 94703bcf37
6 changed files with 189 additions and 69 deletions

2
go.mod
View File

@ -5,7 +5,7 @@ go 1.18
require (
github.com/0xAX/notificator v0.0.0-20220220101646-ee9b8921e557
github.com/Masterminds/semver/v3 v3.1.1
github.com/ProtonMail/gluon v0.14.2-0.20230118120413-542c2bf244a0
github.com/ProtonMail/gluon v0.14.2-0.20230123154940-b7793a0c0bd4
github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a
github.com/ProtonMail/go-proton-api v0.3.1-0.20230118091111-93ad9245e8ee
github.com/ProtonMail/go-rfc5322 v0.11.0

2
go.sum
View File

@ -30,6 +30,8 @@ github.com/ProtonMail/docker-credential-helpers v1.1.0 h1:+kvUIpwWcbtP3WFv5sSvkF
github.com/ProtonMail/docker-credential-helpers v1.1.0/go.mod h1:mK0aBveCxhnQ756AmaTfXMZDeULvheYVhF/MWMErN5g=
github.com/ProtonMail/gluon v0.14.2-0.20230118120413-542c2bf244a0 h1:T9wtn35Mqy4Mkr707iECwqo+FhbTVOI9PlRozR5wCO8=
github.com/ProtonMail/gluon v0.14.2-0.20230118120413-542c2bf244a0/go.mod h1:z2AxLIiBCT1K+0OBHyaDI7AEaO5qI6/BEC2TE42vs4Q=
github.com/ProtonMail/gluon v0.14.2-0.20230123154940-b7793a0c0bd4 h1:AkRcjX1iArf8fVL4vZd6eaBjE3+SFnwZQKsH3OMyExU=
github.com/ProtonMail/gluon v0.14.2-0.20230123154940-b7793a0c0bd4/go.mod h1:z2AxLIiBCT1K+0OBHyaDI7AEaO5qI6/BEC2TE42vs4Q=
github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a h1:D+aZah+k14Gn6kmL7eKxoo/4Dr/lK3ChBcwce2+SQP4=
github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a/go.mod h1:oTGdE7/DlWIr23G0IKW3OXK9wZ5Hw1GGiaJFccTvZi4=
github.com/ProtonMail/go-crypto v0.0.0-20210428141323-04723f9f07d7/go.mod h1:z4/9nQmJSSwwds7ejkxaJwO37dru3geImFUdJlaLzQo=

View File

@ -393,6 +393,18 @@ func clientStore(client *client.Client, from, to int, isUID bool, item imap.Stor
)
}
func clientList(client *client.Client) []*imap.MailboxInfo {
resCh := make(chan *imap.MailboxInfo)
go func() {
if err := client.List("", "*", resCh); err != nil {
panic(err)
}
}()
return iterator.Collect(iterator.Chan(resCh))
}
func createNumMessages(ctx context.Context, t *testing.T, c *proton.Client, addrID, labelID string, count int) []string {
literal, err := os.ReadFile(filepath.Join("testdata", "text-plain.eml"))
require.NoError(t, err)

View File

@ -0,0 +1,169 @@
// Copyright (c) 2023 Proton AG
//
// This file is part of Proton Mail Bridge.
//
// Proton Mail 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.
//
// Proton Mail 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 Proton Mail Bridge. If not, see <https://www.gnu.org/licenses/>.
package bridge_test
import (
"context"
"fmt"
"net/http"
"testing"
"time"
"github.com/ProtonMail/go-proton-api"
"github.com/ProtonMail/go-proton-api/server"
"github.com/ProtonMail/proton-bridge/v3/internal/bridge"
"github.com/ProtonMail/proton-bridge/v3/internal/constants"
"github.com/ProtonMail/proton-bridge/v3/internal/events"
"github.com/bradenaw/juniper/xslices"
"github.com/emersion/go-imap"
"github.com/emersion/go-imap/client"
"github.com/golang/mock/gomock"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
)
func TestBridge_User_BadEvents(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)
labelID, err := s.CreateLabel(userID, "folder", "", proton.LabelTypeFolder)
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, labelID, 10)
})
// The initial user should be fully synced.
withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) {
syncCh, done := chToType[events.Event, events.SyncFinished](bridge.GetEvents(events.SyncFinished{}))
defer done()
userID, err := bridge.LoginFull(ctx, "user", password, nil, nil)
require.NoError(t, err)
require.Equal(t, userID, (<-syncCh).UserID)
})
var messageIDs []string
// Create 10 more messages for the user, generating events.
withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) {
messageIDs = createNumMessages(ctx, t, c, addrID, labelID, 10)
})
// If bridge attempts to sync the new messages, it should get a BadRequest error.
s.AddStatusHook(func(req *http.Request) (int, bool) {
if xslices.Index(xslices.Map(messageIDs, func(messageID string) string {
return "/mail/v4/messages/" + messageID
}), req.URL.Path) < 0 {
return 0, false
}
return http.StatusBadRequest, true
})
// The user will continue to process events and will receive bad request errors.
withMocks(t, func(mocks *bridge.Mocks) {
mocks.Reporter.EXPECT().ReportMessageWithContext(gomock.Any(), gomock.Any()).MinTimes(1)
// The user will eventually be logged out due to the bad request errors.
withBridgeNoMocks(ctx, t, mocks, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge) {
require.Eventually(t, func() bool {
return len(bridge.GetUserIDs()) == 1 && len(getConnectedUserIDs(t, bridge)) == 0
}, 10*time.Second, 100*time.Millisecond)
})
})
})
}
func TestBridge_User_BadEvents_MessageLabelDeleted(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)
labelID, err := s.CreateLabel(userID, "folder", "", proton.LabelTypeFolder)
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, labelID, 10)
})
// The initial user should be fully synced.
withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) {
syncCh, done := chToType[events.Event, events.SyncFinished](bridge.GetEvents(events.SyncFinished{}))
defer done()
userID, err := bridge.LoginFull(ctx, "user", password, nil, nil)
require.NoError(t, err)
require.Equal(t, userID, (<-syncCh).UserID)
})
// Create and delete 10 more messages for the user, generating delete events.
withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) {
messageIDs := createNumMessages(ctx, t, c, addrID, labelID, 10)
require.NoError(t, c.DeleteMessage(ctx, messageIDs...))
})
// Create and delete 10 labels for the user, generating delete events.
withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) {
for i := 0; i < 10; i++ {
label, err := c.CreateLabel(ctx, proton.CreateLabelReq{
Name: uuid.NewString(),
Color: "#f66",
Type: proton.LabelTypeLabel,
})
require.NoError(t, err)
require.NoError(t, c.DeleteLabel(ctx, label.ID))
}
})
// The user will continue to process events and will not receive any bad request errors.
withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) {
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() }()
// Create a new label.
withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) {
require.NoError(t, getErr(c.CreateLabel(ctx, proton.CreateLabelReq{
Name: "blabla",
Color: "#f66",
Type: proton.LabelTypeLabel,
})))
})
// Wait for the label to be created.
require.Eventually(t, func() bool {
return xslices.IndexFunc(clientList(client), func(mailbox *imap.MailboxInfo) bool {
return mailbox.Name == "Labels/blabla"
}) >= 0
}, 10*time.Second, 100*time.Millisecond)
})
})
}

View File

@ -20,7 +20,6 @@ package bridge_test
import (
"context"
"fmt"
"net/http"
"testing"
"time"
@ -30,7 +29,6 @@ import (
mocksPkg "github.com/ProtonMail/proton-bridge/v3/internal/bridge/mocks"
"github.com/ProtonMail/proton-bridge/v3/internal/events"
"github.com/ProtonMail/proton-bridge/v3/internal/vault"
"github.com/bradenaw/juniper/xslices"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
)
@ -664,63 +662,6 @@ func TestBridge_User_Refresh(t *testing.T) {
})
}
func TestBridge_User_BadEvents(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)
labelID, err := s.CreateLabel(userID, "folder", "", proton.LabelTypeFolder)
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, labelID, 10)
})
// The initial user should be fully synced.
withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) {
syncCh, done := chToType[events.Event, events.SyncFinished](bridge.GetEvents(events.SyncFinished{}))
defer done()
userID, err := bridge.LoginFull(ctx, "user", password, nil, nil)
require.NoError(t, err)
require.Equal(t, userID, (<-syncCh).UserID)
})
var messageIDs []string
// Create 10 more messages for the user, generating events.
withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) {
messageIDs = createNumMessages(ctx, t, c, addrID, labelID, 10)
})
// If bridge attempts to sync the new messages, it should get a BadRequest error.
s.AddStatusHook(func(req *http.Request) (int, bool) {
if xslices.Index(xslices.Map(messageIDs, func(messageID string) string {
return "/mail/v4/messages/" + messageID
}), req.URL.Path) < 0 {
return 0, false
}
return http.StatusBadRequest, true
})
// The user will continue to process events and will receive bad request errors.
withMocks(t, func(mocks *bridge.Mocks) {
mocks.Reporter.EXPECT().ReportMessageWithContext(gomock.Any(), gomock.Any()).MinTimes(1)
// The user will eventually be logged out due to the bad request errors.
withBridgeNoMocks(ctx, t, mocks, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge) {
require.Eventually(t, func() bool {
return len(bridge.GetUserIDs()) == 1 && len(getConnectedUserIDs(t, bridge)) == 0
}, 10*time.Second, 100*time.Millisecond)
})
})
})
}
// getErr returns the error that was passed to it.
func getErr[T any](val T, err error) error {
return err

View File

@ -44,9 +44,7 @@ func (user *User) handleAPIEvent(ctx context.Context, event proton.Event) error
}
if event.User != nil {
if err := user.handleUserEvent(ctx, *event.User); err != nil {
return err
}
user.handleUserEvent(ctx, *event.User)
}
if len(event.Addresses) > 0 {
@ -133,8 +131,8 @@ func (user *User) handleRefreshEvent(ctx context.Context, refresh proton.Refresh
}
// handleUserEvent handles the given user event.
func (user *User) handleUserEvent(_ context.Context, userEvent proton.User) error {
return safe.LockRet(func() error {
func (user *User) handleUserEvent(_ context.Context, userEvent proton.User) {
safe.Lock(func() {
user.log.WithFields(logrus.Fields{
"userID": userEvent.ID,
"username": logging.Sensitive(userEvent.Name),
@ -145,8 +143,6 @@ func (user *User) handleUserEvent(_ context.Context, userEvent proton.User) erro
user.eventCh.Enqueue(events.UserChanged{
UserID: user.apiUser.ID,
})
return nil
}, user.apiUserLock)
}
@ -322,7 +318,7 @@ func (user *User) handleLabelEvents(ctx context.Context, labelEvents []proton.La
}
if err := waitOnIMAPUpdates(ctx, updates); err != nil {
return err
return fmt.Errorf("failed to handle delete label event in gluon: %w", err)
}
}
}
@ -497,7 +493,7 @@ func (user *User) handleMessageEvents(ctx context.Context, messageEvents []proto
}
if err := waitOnIMAPUpdates(ctx, updates); err != nil {
return err
return fmt.Errorf("failed to handle delete message event in gluon: %w", err)
}
}
}