diff --git a/Changelog.md b/Changelog.md index 78d3397c..becde52a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,35 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) +## [Bridge 3.0.6] Perth Narrows + +### Fixed +* GODT-2187: Skip messages during sync that fail to build/parse. + +## [Bridge 3.0.5] Perth Narrows + +### Fixed +* GODT-2178: Bump go-proton-api to fix drafts. +* GODT-2180: Allow login with FIDO2. + +## [Bridge 3.0.4] Perth Narrows + +### Changed +* Other: Do not list \Deleted flag for All Mail. +* Other: Disable perma-delete for expunge on Spam folder. + +### Fixed +* Other: Ensure expunge feature test pushes to error stack. +* GODT-2170: Use client-side draft update in integration tests. +* GODT-2170: Improving test server behaviour. +* GODT-2170: Update draft event means delete old and create new message. +* GODT-2170: User create draft route: first steps. + +## [Bridge 3.0.3] Perth Narrows + +### Fixed +* GPA v0.1.4: fix token expiration mechanism. + ## [Bridge 3.0.2] Perth Narrows ### Changed diff --git a/Makefile b/Makefile index 2c7fac84..079487d9 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ ROOT_DIR:=$(shell dirname $(realpath $(firstword $(MAKEFILE_LIST)))) .PHONY: build build-gui build-nogui build-launcher versioner hasher # Keep version hardcoded so app build works also without Git repository. -BRIDGE_APP_VERSION?=3.0.1+git +BRIDGE_APP_VERSION?=3.0.6+git APP_VERSION:=${BRIDGE_APP_VERSION} APP_FULL_NAME:=Proton Mail Bridge APP_VENDOR:=Proton AG diff --git a/go.mod b/go.mod index 09f4774b..b1f5321c 100644 --- a/go.mod +++ b/go.mod @@ -5,9 +5,9 @@ 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.20221129155908-e3c82359cafa + github.com/ProtonMail/gluon v0.14.2-0.20221206104410-725ddb9db68a github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a - github.com/ProtonMail/go-proton-api v0.1.2 + github.com/ProtonMail/go-proton-api v0.2.1 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 dc07416d..8b03525c 100644 --- a/go.sum +++ b/go.sum @@ -28,8 +28,8 @@ github.com/ProtonMail/bcrypt v0.0.0-20211005172633-e235017c1baf h1:yc9daCCYUefEs github.com/ProtonMail/bcrypt v0.0.0-20211005172633-e235017c1baf/go.mod h1:o0ESU9p83twszAU8LBeJKFAAMX14tISa0yk4Oo5TOqo= github.com/ProtonMail/docker-credential-helpers v1.1.0 h1:+kvUIpwWcbtP3WFv5sSvkFn/XLzSqPOB5AAthuk9xPk= github.com/ProtonMail/docker-credential-helpers v1.1.0/go.mod h1:mK0aBveCxhnQ756AmaTfXMZDeULvheYVhF/MWMErN5g= -github.com/ProtonMail/gluon v0.14.2-0.20221129155908-e3c82359cafa h1:SsU+Ueo4D6eK4A4x9qVr4qlpQPBKkt5QomTKZtwFa4A= -github.com/ProtonMail/gluon v0.14.2-0.20221129155908-e3c82359cafa/go.mod h1:z2AxLIiBCT1K+0OBHyaDI7AEaO5qI6/BEC2TE42vs4Q= +github.com/ProtonMail/gluon v0.14.2-0.20221206104410-725ddb9db68a h1:BwWVZcvvf9Pw353+wZGD3X433kPFT4SjQVnYKD0YBRY= +github.com/ProtonMail/gluon v0.14.2-0.20221206104410-725ddb9db68a/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= @@ -43,8 +43,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.1.2 h1:MD0lbo8ohU1O+1mbMU6EkDmVj4BAq5e5cCPkIZgDF9Q= -github.com/ProtonMail/go-proton-api v0.1.2/go.mod h1:jqvJ2HqLHqiPJoEb+BTIB1IF7wvr6p+8ZfA6PO2NRNk= +github.com/ProtonMail/go-proton-api v0.2.1 h1:M15/zzfx6EPiskv2+gogUkmvx7Y1SmRRtLT6GiBh5T0= +github.com/ProtonMail/go-proton-api v0.2.1/go.mod h1:jqvJ2HqLHqiPJoEb+BTIB1IF7wvr6p+8ZfA6PO2NRNk= 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 90d93786..0df30f9e 100644 --- a/internal/bridge/user.go +++ b/internal/bridge/user.go @@ -187,7 +187,7 @@ func (bridge *Bridge) LoginFull( return "", fmt.Errorf("failed to begin login process: %w", err) } - if auth.TwoFA.Enabled == proton.TOTPEnabled { + if auth.TwoFA.Enabled&proton.HasTOTP != 0 { logrus.WithField("userID", auth.UserID).Info("Requesting TOTP") totp, err := getTOTP() @@ -446,9 +446,9 @@ func (bridge *Bridge) addUserWithVault( ctx, vault, client, + bridge.reporter, apiUser, bridge.crashHandler, - bridge.reporter, bridge.vault.SyncWorkers(), bridge.vault.GetShowAllMail(), ) diff --git a/internal/frontend/cli/accounts.go b/internal/frontend/cli/accounts.go index 00fa69ee..93f6b2b9 100644 --- a/internal/frontend/cli/accounts.go +++ b/internal/frontend/cli/accounts.go @@ -149,7 +149,7 @@ func (f *frontendCLI) loginAccount(c *ishell.Context) { //nolint:funlen return } - if auth.TwoFA.Enabled == proton.TOTPEnabled { + if auth.TwoFA.Enabled&proton.HasTOTP != 0 { code := f.readStringInAttempts("Two factor code", c.ReadLine, isNotEmpty) if code == "" { f.printAndLogError("Cannot login: need two factor code") diff --git a/internal/frontend/grpc/service_methods.go b/internal/frontend/grpc/service_methods.go index 0bcf7df1..2633dbf4 100644 --- a/internal/frontend/grpc/service_methods.go +++ b/internal/frontend/grpc/service_methods.go @@ -428,7 +428,7 @@ func (s *Service) Login(ctx context.Context, login *LoginRequest) (*emptypb.Empt s.auth = auth switch { - case auth.TwoFA.Enabled == proton.TOTPEnabled: + case auth.TwoFA.Enabled&proton.HasTOTP != 0: _ = s.SendEvent(NewLoginTfaRequestedEvent(login.Username)) case auth.PasswordMode == proton.TwoPasswordMode: diff --git a/internal/user/events.go b/internal/user/events.go index 3389b705..22856c95 100644 --- a/internal/user/events.go +++ b/internal/user/events.go @@ -23,6 +23,7 @@ import ( "github.com/ProtonMail/gluon/imap" "github.com/ProtonMail/gluon/queue" + "github.com/ProtonMail/gluon/reporter" "github.com/ProtonMail/go-proton-api" "github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/ProtonMail/proton-bridge/v3/internal/events" @@ -391,6 +392,18 @@ func (user *User) handleMessageEvents(ctx context.Context, messageEvents []proto } case proton.EventUpdate, proton.EventUpdateFlags: + // Draft update means to completely remove old message and upload the new data again. + if event.Message.IsDraft() { + if err := user.handleUpdateDraftEvent( + logging.WithLogrusField(ctx, "action", "update draft"), + event, + ); err != nil { + return fmt.Errorf("failed to handle update draft event: %w", err) + } + + return nil + } + // GODT-2028 - Use better events here. It should be possible to have 3 separate events that refrain to // whether the flags, labels or read only data (header+body) has been changed. This requires fixing proton // first so that it correctly reports those cases. @@ -402,16 +415,6 @@ func (user *User) handleMessageEvents(ctx context.Context, messageEvents []proto return fmt.Errorf("failed to handle update message event: %w", err) } - // Only issue body updates if the message is a draft. - if event.Message.IsDraft() { - if err := user.handleUpdateDraftEvent( - logging.WithLogrusField(ctx, "action", "update draft"), - event, - ); err != nil { - return fmt.Errorf("failed to handle update draft event: %w", err) - } - } - case proton.EventDelete: if err := user.handleDeleteMessageEvent( logging.WithLogrusField(ctx, "action", "delete message"), @@ -438,12 +441,30 @@ func (user *User) handleCreateMessageEvent(ctx context.Context, event proton.Mes }).Info("Handling message created event") return withAddrKR(user.apiUser, user.apiAddrs[event.Message.AddressID], user.vault.KeyPass(), func(_, addrKR *crypto.KeyRing) error { - buildRes, err := buildRFC822(user.apiLabels, full, addrKR) - if err != nil { - return fmt.Errorf("failed to build RFC822 message: %w", err) + res := buildRFC822(user.apiLabels, full, addrKR) + + if res.err != nil { + user.log.WithError(err).Error("Failed to build RFC822 message") + + if err := user.vault.AddFailedMessageID(event.ID); err != nil { + user.log.WithError(err).Error("Failed to add failed message ID to vault") + } + + if err := user.reporter.ReportMessageWithContext("Failed to build message (event create)", reporter.Context{ + "messageID": res.messageID, + "error": res.err, + }); err != nil { + user.log.WithError(err).Error("Failed to report message build error") + } + + return nil } - user.updateCh[full.AddressID].Enqueue(imap.NewMessagesCreated(buildRes.update)) + if err := user.vault.RemFailedMessageID(event.ID); err != nil { + user.log.WithError(err).Error("Failed to remove failed message ID from vault") + } + + user.updateCh[full.AddressID].Enqueue(imap.NewMessagesCreated(res.update)) return nil }) @@ -493,16 +514,34 @@ func (user *User) handleUpdateDraftEvent(ctx context.Context, event proton.Messa } return withAddrKR(user.apiUser, user.apiAddrs[event.Message.AddressID], user.vault.KeyPass(), func(_, addrKR *crypto.KeyRing) error { - buildRes, err := buildRFC822(user.apiLabels, full, addrKR) - if err != nil { - return fmt.Errorf("failed to build RFC822 draft: %w", err) + res := buildRFC822(user.apiLabels, full, addrKR) + + if res.err != nil { + logrus.WithError(err).Error("Failed to build RFC822 message") + + if err := user.vault.AddFailedMessageID(event.ID); err != nil { + user.log.WithError(err).Error("Failed to add failed message ID to vault") + } + + if err := user.reporter.ReportMessageWithContext("Failed to build message (event update)", reporter.Context{ + "messageID": res.messageID, + "error": res.err, + }); err != nil { + logrus.WithError(err).Error("Failed to report message build error") + } + + return nil + } + + if err := user.vault.RemFailedMessageID(event.ID); err != nil { + user.log.WithError(err).Error("Failed to remove failed message ID from vault") } user.updateCh[full.AddressID].Enqueue(imap.NewMessageUpdated( - buildRes.update.Message, - buildRes.update.Literal, - buildRes.update.MailboxIDs, - buildRes.update.ParsedMessage, + res.update.Message, + res.update.Literal, + res.update.MailboxIDs, + res.update.ParsedMessage, )) return nil diff --git a/internal/user/imap.go b/internal/user/imap.go index ec8de275..ba6c734b 100644 --- a/internal/user/imap.go +++ b/internal/user/imap.go @@ -18,8 +18,10 @@ package user import ( + "bytes" "context" "fmt" + "net/mail" "sync/atomic" "time" @@ -31,6 +33,7 @@ import ( "github.com/ProtonMail/proton-bridge/v3/internal/safe" "github.com/ProtonMail/proton-bridge/v3/internal/vault" "github.com/ProtonMail/proton-bridge/v3/pkg/message" + "github.com/ProtonMail/proton-bridge/v3/pkg/message/parser" "github.com/bradenaw/juniper/stream" "github.com/bradenaw/juniper/xslices" "golang.org/x/exp/slices" @@ -326,7 +329,7 @@ func (conn *imapConnector) RemoveMessagesFromMailbox(ctx context.Context, messag return err } - if mailboxID == proton.SpamLabel || mailboxID == proton.TrashLabel || mailboxID == proton.DraftsLabel { + if mailboxID == proton.TrashLabel || mailboxID == proton.DraftsLabel { var metadata []proton.MessageMetadata // There's currently no limit on how many IDs we can filter on, @@ -437,20 +440,37 @@ func (conn *imapConnector) importMessage( if err := safe.RLockRet(func() error { return withAddrKR(conn.apiUser, conn.apiAddrs[conn.addrID], conn.vault.KeyPass(), func(_, addrKR *crypto.KeyRing) error { - res, err := stream.Collect(ctx, conn.client.ImportMessages(ctx, addrKR, 1, 1, []proton.ImportReq{{ - Metadata: proton.ImportMetadata{ - AddressID: conn.addrID, - LabelIDs: labelIDs, - Unread: proton.Bool(unread), - Flags: flags, - }, - Message: literal, - }}...)) - if err != nil { - return fmt.Errorf("failed to import message: %w", err) + messageID := "" + + if slices.Contains(labelIDs, proton.DraftsLabel) { + msg, err := conn.createDraft(ctx, literal, addrKR, conn.apiAddrs[conn.addrID]) + if err != nil { + return fmt.Errorf("failed to create draft: %w", err) + } + + // apply labels + + messageID = msg.ID + } else { + res, err := stream.Collect(ctx, conn.client.ImportMessages(ctx, addrKR, 1, 1, []proton.ImportReq{{ + Metadata: proton.ImportMetadata{ + AddressID: conn.addrID, + LabelIDs: labelIDs, + Unread: proton.Bool(unread), + Flags: flags, + }, + Message: literal, + }}...)) + if err != nil { + return fmt.Errorf("failed to import message: %w", err) + } + + messageID = res[0].MessageID } - if full, err = conn.client.GetFullMessage(ctx, res[0].MessageID); err != nil { + var err error + + if full, err = conn.client.GetFullMessage(ctx, messageID); err != nil { return fmt.Errorf("failed to fetch message: %w", err) } @@ -497,6 +517,63 @@ func toIMAPMessage(message proton.MessageMetadata) imap.Message { } } +func (conn *imapConnector) createDraft(ctx context.Context, literal []byte, addrKR *crypto.KeyRing, sender proton.Address) (proton.Message, error) { //nolint:funlen + // Create a new message parser from the reader. + parser, err := parser.New(bytes.NewReader(literal)) + if err != nil { + return proton.Message{}, fmt.Errorf("failed to create parser: %w", err) + } + + message, err := message.ParseWithParser(parser) + if err != nil { + return proton.Message{}, fmt.Errorf("failed to parse message: %w", err) + } + + decBody := string(message.PlainBody) + if message.RichBody != "" { + decBody = string(message.RichBody) + } + + draft, err := conn.client.CreateDraft(ctx, addrKR, proton.CreateDraftReq{ + Message: proton.DraftTemplate{ + Subject: message.Subject, + Body: decBody, + MIMEType: message.MIMEType, + + Sender: &mail.Address{Name: sender.DisplayName, Address: sender.Email}, + ToList: message.ToList, + CCList: message.CCList, + BCCList: message.BCCList, + + ExternalID: message.ExternalID, + }, + }) + + if err != nil { + return proton.Message{}, fmt.Errorf("failed to create draft: %w", err) + } + + for _, att := range message.Attachments { + disposition := proton.AttachmentDisposition + if att.Disposition == "inline" && att.ContentID != "" { + disposition = proton.InlineDisposition + } + + if _, err := conn.client.UploadAttachment(ctx, addrKR, proton.CreateAttachmentReq{ + MessageID: draft.ID, + Filename: att.Name, + MIMEType: rfc822.MIMEType(att.MIMEType), + Disposition: disposition, + ContentID: att.ContentID, + Body: att.Data, + }); err != nil { + return proton.Message{}, fmt.Errorf("failed to add attachment to draft: %w", err) + } + } + + return draft, nil +} + func toIMAPMailbox(label proton.Label, flags, permFlags, attrs imap.FlagSet) imap.Mailbox { if label.Type == proton.LabelTypeLabel { label.Path = append([]string{labelPrefix}, label.Path...) diff --git a/internal/user/smtp.go b/internal/user/smtp.go index d827ac40..67682f37 100644 --- a/internal/user/smtp.go +++ b/internal/user/smtp.go @@ -188,19 +188,9 @@ func sendWithKey( //nolint:funlen return proton.Message{}, fmt.Errorf("unsupported MIME type: %v", message.MIMEType) } - encBody, err := addrKR.Encrypt(crypto.NewPlainMessageFromString(decBody), nil) - if err != nil { - return proton.Message{}, fmt.Errorf("failed to encrypt message body: %w", err) - } - - armBody, err := encBody.GetArmored() - if err != nil { - return proton.Message{}, fmt.Errorf("failed to get armored message body: %w", err) - } - - draft, err := createDraft(ctx, client, emails, from, to, parentID, message.InReplyTo, proton.DraftTemplate{ + draft, err := createDraft(ctx, client, addrKR, emails, from, to, parentID, message.InReplyTo, proton.DraftTemplate{ Subject: message.Subject, - Body: armBody, + Body: decBody, MIMEType: message.MIMEType, Sender: message.Sender, @@ -312,6 +302,7 @@ func getParentID( //nolint:funlen func createDraft( ctx context.Context, client *proton.Client, + addrKR *crypto.KeyRing, emails []string, from string, to []string, @@ -357,7 +348,7 @@ func createDraft( action = proton.ForwardAction } - return client.CreateDraft(ctx, proton.CreateDraftReq{ + return client.CreateDraft(ctx, addrKR, proton.CreateDraftReq{ Message: template, ParentID: parentID, Action: action, diff --git a/internal/user/sync.go b/internal/user/sync.go index c81b480d..49e8daf9 100644 --- a/internal/user/sync.go +++ b/internal/user/sync.go @@ -26,6 +26,7 @@ import ( "github.com/ProtonMail/gluon/imap" "github.com/ProtonMail/gluon/queue" + "github.com/ProtonMail/gluon/reporter" "github.com/ProtonMail/go-proton-api" "github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/ProtonMail/proton-bridge/v3/internal/events" @@ -36,6 +37,7 @@ import ( "github.com/google/uuid" "github.com/sirupsen/logrus" "golang.org/x/exp/maps" + "golang.org/x/exp/slices" ) const ( @@ -87,6 +89,7 @@ func (user *User) doSync(ctx context.Context) error { return nil } +// nolint:funlen func (user *User) sync(ctx context.Context) error { return safe.RLockRet(func() error { return withAddrKRs(user.apiUser, user.apiAddrs, user.vault.KeyPass(), func(_ *crypto.KeyRing, addrKRs map[string]*crypto.KeyRing) error { @@ -109,10 +112,32 @@ func (user *User) sync(ctx context.Context) error { if !user.vault.SyncStatus().HasMessages { user.log.Info("Syncing messages") + // Determine which messages to sync. + messageIDs, err := user.client.GetMessageIDs(ctx, "") + if err != nil { + return fmt.Errorf("failed to get message IDs to sync: %w", err) + } + + // Remove any messages that have already failed to sync. + messageIDs = xslices.Filter(messageIDs, func(messageID string) bool { + return !slices.Contains(user.vault.SyncStatus().FailedMessageIDs, messageID) + }) + + // Reverse the order of the message IDs so that the newest messages are synced first. + xslices.Reverse(messageIDs) + + // If we have a message ID that we've already synced, then we can skip all messages before it. + if idx := xslices.Index(messageIDs, user.vault.SyncStatus().LastMessageID); idx >= 0 { + messageIDs = messageIDs[idx+1:] + } + + // Sync the messages. if err := syncMessages( ctx, user.ID(), + messageIDs, user.client, + user.reporter, user.vault, user.apiLabels, addrKRs, @@ -183,7 +208,9 @@ func syncLabels(ctx context.Context, apiLabels map[string]proton.Label, updateCh func syncMessages( ctx context.Context, userID string, + messageIDs []string, client *proton.Client, + sentry reporter.Reporter, vault *vault.User, apiLabels map[string]proton.Label, addrKRs map[string]*crypto.KeyRing, @@ -194,20 +221,6 @@ func syncMessages( ctx, cancel := context.WithCancel(ctx) defer cancel() - // Determine which messages to sync. - messageIDs, err := client.GetMessageIDs(ctx, "") - if err != nil { - return fmt.Errorf("failed to get message IDs to sync: %w", err) - } - - // Reverse the order of the message IDs so that the newest messages are synced first. - xslices.Reverse(messageIDs) - - // If we have a message ID that we've already synced, then we can skip all messages before it. - if idx := xslices.Index(messageIDs, vault.SyncStatus().LastMessageID); idx >= 0 { - messageIDs = messageIDs[idx+1:] - } - // Track the amount of time to process all the messages. syncStartTime := time.Now() defer func() { logrus.WithField("duration", time.Since(syncStartTime)).Info("Message sync completed") }() @@ -222,14 +235,12 @@ func syncMessages( flushers := make(map[string]*flusher, len(updateCh)) for addrID, updateCh := range updateCh { - flusher := newFlusher(updateCh, maxUpdateSize) - - flushers[addrID] = flusher + flushers[addrID] = newFlusher(updateCh, maxUpdateSize) } // Create a reporter to report sync progress updates. - reporter := newReporter(userID, eventCh, len(messageIDs), time.Second) - defer reporter.done() + syncReporter := newSyncReporter(userID, eventCh, len(messageIDs), time.Second) + defer syncReporter.done() type flushUpdate struct { messageID string @@ -267,7 +278,7 @@ func syncMessages( return nil, ctx.Err() } - return buildRFC822(apiLabels, msg, addrKRs[msg.AddressID]) + return buildRFC822(apiLabels, msg, addrKRs[msg.AddressID]), nil }) if err != nil { errorCh <- err @@ -283,12 +294,31 @@ func syncMessages( } }() - // Goroutine in charge of converting the messages into updates and building a waitable structure for progress - // tracking. + // Goroutine which converts the messages into updates and builds a waitable structure for progress tracking. go func() { defer close(flushUpdateCh) for batch := range flushCh { for _, res := range batch { + if res.err != nil { + if err := vault.AddFailedMessageID(res.messageID); err != nil { + logrus.WithError(err).Error("Failed to add failed message ID") + } + + if err := sentry.ReportMessageWithContext("Failed to build message (sync)", reporter.Context{ + "messageID": res.messageID, + "error": res.err, + }); err != nil { + logrus.WithError(err).Error("Failed to report message build error") + } + + // We could sync a placeholder message here, but for now we skip it entirely. + continue + } else { + if err := vault.RemFailedMessageID(res.messageID); err != nil { + logrus.WithError(err).Error("Failed to remove failed message ID") + } + } + flushers[res.addressID].push(res.update) } @@ -321,7 +351,7 @@ func syncMessages( return fmt.Errorf("failed to set last synced message ID: %w", err) } - reporter.add(flushUpdate.batchLen) + syncReporter.add(flushUpdate.batchLen) } return <-errorCh @@ -333,6 +363,8 @@ func newSystemMailboxCreatedUpdate(labelID imap.MailboxID, labelName string) *im } attrs := imap.NewFlagSet(imap.AttrNoInferiors) + permanentFlags := defaultPermanentFlags + flags := defaultFlags switch labelID { case proton.TrashLabel: @@ -343,6 +375,8 @@ func newSystemMailboxCreatedUpdate(labelID imap.MailboxID, labelName string) *im case proton.AllMailLabel: attrs = attrs.Add(imap.AttrAll) + flags = imap.NewFlagSet(imap.FlagSeen, imap.FlagFlagged) + permanentFlags = imap.NewFlagSet(imap.FlagSeen, imap.FlagFlagged) case proton.ArchiveLabel: attrs = attrs.Add(imap.AttrArchive) @@ -360,8 +394,8 @@ func newSystemMailboxCreatedUpdate(labelID imap.MailboxID, labelName string) *im return imap.NewMailboxCreated(imap.Mailbox{ ID: labelID, Name: []string{labelName}, - Flags: defaultFlags, - PermanentFlags: defaultPermanentFlags, + Flags: flags, + PermanentFlags: permanentFlags, Attributes: attrs, }) } diff --git a/internal/user/sync_build.go b/internal/user/sync_build.go index 897474a0..ef5032a7 100644 --- a/internal/user/sync_build.go +++ b/internal/user/sync_build.go @@ -18,18 +18,22 @@ package user import ( - "fmt" + "bytes" + "html/template" + "time" "github.com/ProtonMail/gluon/imap" "github.com/ProtonMail/go-proton-api" "github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/ProtonMail/proton-bridge/v3/pkg/message" + "github.com/bradenaw/juniper/xslices" ) type buildRes struct { messageID string addressID string update *imap.MessageCreated + err error } func defaultJobOpts() message.JobOptions { @@ -43,22 +47,28 @@ func defaultJobOpts() message.JobOptions { } } -func buildRFC822(apiLabels map[string]proton.Label, full proton.FullMessage, addrKR *crypto.KeyRing) (*buildRes, error) { - literal, err := message.BuildRFC822(addrKR, full.Message, full.AttData, defaultJobOpts()) - if err != nil { - return nil, fmt.Errorf("failed to build message %s: %w", full.ID, err) - } +func buildRFC822(apiLabels map[string]proton.Label, full proton.FullMessage, addrKR *crypto.KeyRing) *buildRes { + var ( + update *imap.MessageCreated + err error + ) - update, err := newMessageCreatedUpdate(apiLabels, full.MessageMetadata, literal) - if err != nil { - return nil, fmt.Errorf("failed to create IMAP update for message %s: %w", full.ID, err) + if literal, buildErr := message.BuildRFC822(addrKR, full.Message, full.AttData, defaultJobOpts()); buildErr != nil { + update = newMessageCreatedFailedUpdate(apiLabels, full.MessageMetadata, buildErr) + err = buildErr + } else if created, parseErr := newMessageCreatedUpdate(apiLabels, full.MessageMetadata, literal); parseErr != nil { + update = newMessageCreatedFailedUpdate(apiLabels, full.MessageMetadata, parseErr) + err = parseErr + } else { + update = created } return &buildRes{ messageID: full.ID, addressID: full.AddressID, update: update, - }, nil + err: err, + } } func newMessageCreatedUpdate( @@ -78,3 +88,83 @@ func newMessageCreatedUpdate( ParsedMessage: parsedMessage, }, nil } + +func newMessageCreatedFailedUpdate( + apiLabels map[string]proton.Label, + message proton.MessageMetadata, + err error, +) *imap.MessageCreated { + literal := newFailedMessageLiteral(message.ID, time.Unix(message.Time, 0), message.Subject, err) + + parsedMessage, err := imap.NewParsedMessage(literal) + if err != nil { + panic(err) + } + + return &imap.MessageCreated{ + Message: toIMAPMessage(message), + MailboxIDs: mapTo[string, imap.MailboxID](wantLabels(apiLabels, message.LabelIDs)), + Literal: literal, + ParsedMessage: parsedMessage, + } +} + +func newFailedMessageLiteral( + messageID string, + date time.Time, + subject string, + syncErr error, +) []byte { + var buf bytes.Buffer + + if tmpl, err := template.New("header").Parse(failedMessageHeaderTemplate); err != nil { + panic(err) + } else if b, err := tmplExec(tmpl, map[string]any{ + "Date": date.In(time.UTC).Format(time.RFC822), + }); err != nil { + panic(err) + } else if _, err := buf.Write(b); err != nil { + panic(err) + } + + if tmpl, err := template.New("body").Parse(failedMessageBodyTemplate); err != nil { + panic(err) + } else if b, err := tmplExec(tmpl, map[string]any{ + "MessageID": messageID, + "Subject": subject, + "Error": syncErr.Error(), + }); err != nil { + panic(err) + } else if _, err := buf.Write(lineWrap(b64Encode(b))); err != nil { + panic(err) + } + + return buf.Bytes() +} + +func tmplExec(template *template.Template, data any) ([]byte, error) { + var buf bytes.Buffer + + if err := template.Execute(&buf, data); err != nil { + return nil, err + } + + return buf.Bytes(), nil +} + +func lineWrap(b []byte) []byte { + return bytes.Join(xslices.Chunk(b, 76), []byte{'\r', '\n'}) +} + +const failedMessageHeaderTemplate = `Date: {{.Date}} +Subject: Message failed to build +Content-Type: text/plain +Content-Transfer-Encoding: base64 + +` + +const failedMessageBodyTemplate = `Failed to build message: +Subject: {{.Subject}} +Error: {{.Error}} +MessageID: {{.MessageID}} +` diff --git a/internal/user/sync_build_test.go b/internal/user/sync_build_test.go new file mode 100644 index 00000000..d3406086 --- /dev/null +++ b/internal/user/sync_build_test.go @@ -0,0 +1,49 @@ +// Copyright (c) 2022 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 . + +package user + +import ( + "errors" + "testing" + "time" + + "github.com/ProtonMail/gluon/imap" + "github.com/ProtonMail/gluon/rfc822" + "github.com/stretchr/testify/require" +) + +func TestNewFailedMessageLiteral(t *testing.T) { + literal := newFailedMessageLiteral("abcd-efgh", time.Unix(123456789, 0), "subject", errors.New("oops")) + + header, err := rfc822.Parse(literal).ParseHeader() + require.NoError(t, err) + require.Equal(t, "Message failed to build", header.Get("Subject")) + require.Equal(t, "29 Nov 73 21:33 UTC", header.Get("Date")) + require.Equal(t, "text/plain", header.Get("Content-Type")) + require.Equal(t, "base64", header.Get("Content-Transfer-Encoding")) + + b, err := rfc822.Parse(literal).DecodedBody() + require.NoError(t, err) + require.Equal(t, string(b), "Failed to build message: \nSubject: subject\nError: oops\nMessageID: abcd-efgh\n") + + parsed, err := imap.NewParsedMessage(literal) + require.NoError(t, err) + require.Equal(t, `("29 Nov 73 21:33 UTC" "Message failed to build" NIL NIL NIL NIL NIL NIL NIL NIL)`, parsed.Envelope) + require.Equal(t, `("text" "plain" () NIL NIL "base64" 114 2)`, parsed.Body) + require.Equal(t, `("text" "plain" () NIL NIL "base64" 114 2 NIL NIL NIL NIL)`, parsed.Structure) +} diff --git a/internal/user/sync_reporter.go b/internal/user/sync_reporter.go index 3149e8cc..87722bb9 100644 --- a/internal/user/sync_reporter.go +++ b/internal/user/sync_reporter.go @@ -24,7 +24,7 @@ import ( "github.com/ProtonMail/proton-bridge/v3/internal/events" ) -type reporter struct { +type syncReporter struct { userID string eventCh *queue.QueuedChannel[events.Event] @@ -36,8 +36,8 @@ type reporter struct { freq time.Duration } -func newReporter(userID string, eventCh *queue.QueuedChannel[events.Event], total int, freq time.Duration) *reporter { - return &reporter{ +func newSyncReporter(userID string, eventCh *queue.QueuedChannel[events.Event], total int, freq time.Duration) *syncReporter { + return &syncReporter{ userID: userID, eventCh: eventCh, @@ -47,7 +47,7 @@ func newReporter(userID string, eventCh *queue.QueuedChannel[events.Event], tota } } -func (rep *reporter) add(delta int) { +func (rep *syncReporter) add(delta int) { rep.count += delta if time.Since(rep.last) > rep.freq { @@ -62,7 +62,7 @@ func (rep *reporter) add(delta int) { } } -func (rep *reporter) done() { +func (rep *syncReporter) done() { rep.eventCh.Enqueue(events.SyncProgress{ UserID: rep.userID, Progress: 1, diff --git a/internal/user/types.go b/internal/user/types.go index 16a8cc36..acd3fb3c 100644 --- a/internal/user/types.go +++ b/internal/user/types.go @@ -60,6 +60,15 @@ func groupBy[Key comparable, Value any](items []Value, key func(Value) Key) map[ // b64Encode returns the base64 encoding of the given byte slice. func b64Encode(b []byte) []byte { + enc := make([]byte, base64.StdEncoding.EncodedLen(len(b))) + + base64.StdEncoding.Encode(enc, b) + + return enc +} + +// b64RawEncode returns the base64 encoding of the given byte slice. +func b64RawEncode(b []byte) []byte { enc := make([]byte, base64.RawURLEncoding.EncodedLen(len(b))) base64.RawURLEncoding.Encode(enc, b) @@ -67,8 +76,8 @@ func b64Encode(b []byte) []byte { return enc } -// b64Decode returns the bytes represented by the base64 encoding of the given byte slice. -func b64Decode(b []byte) ([]byte, error) { +// b64RawDecode returns the bytes represented by the base64 encoding of the given byte slice. +func b64RawDecode(b []byte) ([]byte, error) { dec := make([]byte, base64.RawURLEncoding.DecodedLen(len(b))) n, err := base64.RawURLEncoding.Decode(dec, b) diff --git a/internal/user/user.go b/internal/user/user.go index d5c4c753..1b55a70d 100644 --- a/internal/user/user.go +++ b/internal/user/user.go @@ -29,7 +29,7 @@ import ( "github.com/ProtonMail/gluon/connector" "github.com/ProtonMail/gluon/imap" "github.com/ProtonMail/gluon/queue" - gluonReporter "github.com/ProtonMail/gluon/reporter" + "github.com/ProtonMail/gluon/reporter" "github.com/ProtonMail/go-proton-api" "github.com/ProtonMail/proton-bridge/v3/internal/async" "github.com/ProtonMail/proton-bridge/v3/internal/events" @@ -57,6 +57,7 @@ type User struct { vault *vault.User client *proton.Client + reporter reporter.Reporter eventCh *queue.QueuedChannel[events.Event] sendHash *sendRecorder @@ -72,8 +73,6 @@ type User struct { updateCh map[string]*queue.QueuedChannel[imap.Update] updateChLock safe.RWMutex - reporter gluonReporter.Reporter - tasks *async.Group abortable async.Abortable goSync func() @@ -92,9 +91,9 @@ func New( ctx context.Context, encVault *vault.User, client *proton.Client, + reporter reporter.Reporter, apiUser proton.User, crashHandler async.PanicHandler, - reporter gluonReporter.Reporter, syncWorkers int, showAllMail bool, ) (*User, error) { //nolint:funlen @@ -118,6 +117,7 @@ func New( vault: encVault, client: client, + reporter: reporter, eventCh: queue.NewQueuedChannel[events.Event](0, 0), sendHash: newSendRecorder(sendEntryExpiry), @@ -133,8 +133,6 @@ func New( updateCh: make(map[string]*queue.QueuedChannel[imap.Update]), updateChLock: safe.NewRWMutex(), - reporter: reporter, - tasks: async.NewGroup(context.Background(), crashHandler), pollAPIEventsCh: make(chan chan struct{}), @@ -357,7 +355,7 @@ func (user *User) GluonKey() []byte { // BridgePass returns the user's bridge password, used for authentication over SMTP and IMAP. func (user *User) BridgePass() []byte { - return b64Encode(user.vault.BridgePass()) + return b64RawEncode(user.vault.BridgePass()) } // UsedSpace returns the total space used by the user on the API. @@ -433,7 +431,7 @@ func (user *User) CheckAuth(email string, password []byte) (string, error) { panic("your wish is my command.. I crash") } - dec, err := b64Decode(password) + dec, err := b64RawDecode(password) if err != nil { return "", fmt.Errorf("failed to decode password: %w", err) } diff --git a/internal/user/user_test.go b/internal/user/user_test.go index 524c5297..66bba5e0 100644 --- a/internal/user/user_test.go +++ b/internal/user/user_test.go @@ -218,7 +218,7 @@ func withUser(tb testing.TB, ctx context.Context, _ *server.Server, m *proton.Ma vaultUser, err := vault.AddUser(apiUser.ID, username, apiAuth.UID, apiAuth.RefreshToken, saltedKeyPass) require.NoError(tb, err) - user, err := New(ctx, vaultUser, client, apiUser, nil, nil, vault.SyncWorkers(), true) + user, err := New(ctx, vaultUser, client, nil, apiUser, nil, vault.SyncWorkers(), true) require.NoError(tb, err) defer user.Close() diff --git a/internal/vault/types_user.go b/internal/vault/types_user.go index 9673d554..b58c76c2 100644 --- a/internal/vault/types_user.go +++ b/internal/vault/types_user.go @@ -60,9 +60,10 @@ func (mode AddressMode) String() string { } type SyncStatus struct { - HasLabels bool - HasMessages bool - LastMessageID string + HasLabels bool + HasMessages bool + LastMessageID string + FailedMessageIDs []string } func (status SyncStatus) IsComplete() bool { diff --git a/internal/vault/user.go b/internal/vault/user.go index 38ee3ce9..50867f42 100644 --- a/internal/vault/user.go +++ b/internal/vault/user.go @@ -21,6 +21,8 @@ import ( "fmt" "github.com/ProtonMail/gluon/imap" + "github.com/bradenaw/juniper/xslices" + "golang.org/x/exp/slices" ) type User struct { @@ -158,6 +160,24 @@ func (user *User) SetLastMessageID(messageID string) error { }) } +// AddFailedMessageID adds a message ID to the list of failed message IDs. +func (user *User) AddFailedMessageID(messageID string) error { + return user.vault.modUser(user.userID, func(data *UserData) { + if !slices.Contains(data.SyncStatus.FailedMessageIDs, messageID) { + data.SyncStatus.FailedMessageIDs = append(data.SyncStatus.FailedMessageIDs, messageID) + } + }) +} + +// RemFailedMessageID removes a message ID from the list of failed message IDs. +func (user *User) RemFailedMessageID(messageID string) error { + return user.vault.modUser(user.userID, func(data *UserData) { + data.SyncStatus.FailedMessageIDs = xslices.Filter(data.SyncStatus.FailedMessageIDs, func(otherID string) bool { + return otherID != messageID + }) + }) +} + // ClearSyncStatus clears the user's sync status. func (user *User) ClearSyncStatus() error { return user.vault.modUser(user.userID, func(data *UserData) { diff --git a/tests/api_test.go b/tests/api_test.go index d32f7315..530cea4e 100644 --- a/tests/api_test.go +++ b/tests/api_test.go @@ -19,7 +19,6 @@ package tests import ( "github.com/Masterminds/semver/v3" - "github.com/ProtonMail/go-proton-api" "github.com/ProtonMail/go-proton-api/server" ) @@ -36,8 +35,6 @@ type API interface { RemoveAddress(userID, addrID string) error RemoveAddressKey(userID, addrID, keyID string) error - UpdateDraft(userID, draftID string, changes proton.DraftTemplate) error - Close() } diff --git a/tests/bdd_test.go b/tests/bdd_test.go index 8073172f..25ad2228 100644 --- a/tests/bdd_test.go +++ b/tests/bdd_test.go @@ -98,7 +98,7 @@ func TestFeatures(testingT *testing.T) { ctx.Step(`^the address "([^"]*)" of account "([^"]*)" has the following messages in "([^"]*)":$`, s.theAddressOfAccountHasTheFollowingMessagesInMailbox) ctx.Step(`^the address "([^"]*)" of account "([^"]*)" has (\d+) messages in "([^"]*)"$`, s.theAddressOfAccountHasMessagesInMailbox) ctx.Step(`^the address "([^"]*)" of account "([^"]*)" has no keys$`, s.theAddressOfAccountHasNoKeys) - ctx.Step(`^the following fields where changed in draft (\d+) for address "([^"]*)" of account "([^"]*)":$`, s.addressDraftChanged) + ctx.Step(`^the following fields were changed in draft (\d+) for address "([^"]*)" of account "([^"]*)":$`, s.theFollowingFieldsWereChangedInDraftForAddressOfAccount) // ==== BRIDGE ==== ctx.Step(`^bridge starts$`, s.bridgeStarts) diff --git a/tests/ctx_test.go b/tests/ctx_test.go index 9eb4ca0e..e489714c 100644 --- a/tests/ctx_test.go +++ b/tests/ctx_test.go @@ -230,36 +230,28 @@ func (t *testCtx) getMBoxID(userID string, name string) string { // getDraftID will return the API ID of draft message with draftIndex, where // draftIndex is similar to sequential ID i.e. 1 represents the first message // of draft folder sorted by API creation time. -func (t *testCtx) getDraftID(username string, draftIndex int) string { - if draftIndex < 1 { - panic(fmt.Sprintf("draft index suppose to be non-zero positive integer, but have %d", draftIndex)) - } - +func (t *testCtx) getDraftID(username string, draftIndex int) (string, error) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() var draftID string if err := t.withClient(ctx, username, func(ctx context.Context, client *proton.Client) error { - messages, err := client.GetMessageMetadata( - ctx, proton.MessageFilter{LabelID: proton.DraftsLabel}, - ) + messages, err := client.GetMessageMetadata(ctx, proton.MessageFilter{LabelID: proton.DraftsLabel}) if err != nil { - panic(err) - } - - if len(messages) < draftIndex { - panic("draft index too high") + return fmt.Errorf("failed to get message metadata: %w", err) + } else if len(messages) < draftIndex { + return fmt.Errorf("draft index %d is out of range", draftIndex) } draftID = messages[draftIndex-1].ID return nil }); err != nil { - panic(err) + return "", err } - return draftID + return draftID, nil } func (t *testCtx) getLastCall(method, pathExp string) (server.Call, error) { diff --git a/tests/features/imap/message/create.feature b/tests/features/imap/message/create.feature index 54be10d7..fee5fb7e 100644 --- a/tests/features/imap/message/create.feature +++ b/tests/features/imap/message/create.feature @@ -27,6 +27,7 @@ Feature: IMAP create messages And IMAP client "1" eventually sees the following messages in "Drafts": | from | to | subject | body | | user@pm.me | john.doe@email.com | foo | bar | + # This fails now And IMAP client "1" eventually sees the following messages in "All Mail": | from | to | subject | body | | user@pm.me | john.doe@email.com | foo | bar | diff --git a/tests/features/imap/message/delete.feature b/tests/features/imap/message/delete.feature index 2e10764c..11df8b0d 100644 --- a/tests/features/imap/message/delete.feature +++ b/tests/features/imap/message/delete.feature @@ -16,12 +16,14 @@ Feature: IMAP remove messages from mailbox And IMAP client "1" marks message 2 as deleted Then IMAP client "1" sees that message 2 has the flag "\Deleted" When IMAP client "1" expunges + And it succeeds Then IMAP client "1" sees 9 messages in "Folders/mbox" Scenario: Mark all messages as deleted and EXPUNGE When IMAP client "1" selects "Folders/mbox" And IMAP client "1" marks all messages as deleted And IMAP client "1" expunges + And it succeeds Then IMAP client "1" sees 0 messages in "Folders/mbox" Scenario: Mark messages as undeleted and EXPUNGE @@ -30,11 +32,11 @@ Feature: IMAP remove messages from mailbox But IMAP client "1" marks message 2 as not deleted And IMAP client "1" marks message 3 as not deleted When IMAP client "1" expunges + And it succeeds Then IMAP client "1" sees 2 messages in "Folders/mbox" -# TODO(GODT-1989): Re-enable! -# Scenario: Not possible to delete from All Mail and expunge does nothing -# When IMAP client "1" selects "All Mail" -# And IMAP client "1" marks message 2 as deleted -# And IMAP client "1" expunges -# Then IMAP client "1" eventually sees 10 messages in "All Mail" \ No newline at end of file + Scenario: Not possible to delete from All Mail and expunge does nothing + When IMAP client "1" selects "All Mail" + And IMAP client "1" marks message 2 as deleted + And IMAP client "1" expunges + Then it fails \ No newline at end of file diff --git a/tests/features/imap/message/delete_from_trash.feature b/tests/features/imap/message/delete_from_trash.feature index 0ba83756..43d79265 100644 --- a/tests/features/imap/message/delete_from_trash.feature +++ b/tests/features/imap/message/delete_from_trash.feature @@ -6,8 +6,8 @@ Feature: IMAP remove messages from Trash | mbox | folder | | label | label | - Scenario Outline: Message in Trash or Spam and some other label is not permanently deleted - Given the address "user@pm.me" of account "user@pm.me" has the following messages in "": + Scenario Outline: Message in Trash and some other label is not permanently deleted + Given the address "user@pm.me" of account "user@pm.me" has the following messages in "Trash": | from | to | subject | body | | john.doe@mail.com | user@pm.me | foo | hello | | jane.doe@mail.com | name@pm.me | bar | world | @@ -15,27 +15,22 @@ Feature: IMAP remove messages from Trash And the user logs in with username "user@pm.me" and password "password" And user "user@pm.me" finishes syncing And user "user@pm.me" connects and authenticates IMAP client "1" - And IMAP client "1" selects "" - When IMAP client "1" copies the message with subject "foo" from "" to "Labels/label" + And IMAP client "1" selects "Trash" + When IMAP client "1" copies the message with subject "foo" from "Trash" to "Labels/label" Then it succeeds When IMAP client "1" marks the message with subject "foo" as deleted Then it succeeds - And IMAP client "1" sees 2 messages in "" + And IMAP client "1" sees 2 messages in "Trash" And IMAP client "1" sees 2 messages in "All Mail" And IMAP client "1" sees 1 messages in "Labels/label" When IMAP client "1" expunges Then it succeeds - And IMAP client "1" sees 1 messages in "" + And IMAP client "1" sees 1 messages in "Trash" And IMAP client "1" sees 2 messages in "All Mail" And IMAP client "1" sees 1 messages in "Labels/label" - Examples: - | mailbox | - | Spam | - | Trash | - - Scenario Outline: Message in Trash or Spam only is permanently deleted - Given the address "user@pm.me" of account "user@pm.me" has the following messages in "": + Scenario Outline: Message in Trash only is permanently deleted + Given the address "user@pm.me" of account "user@pm.me" has the following messages in "Trash": | from | to | subject | body | | john.doe@mail.com | user@pm.me | foo | hello | | jane.doe@mail.com | name@pm.me | bar | world | @@ -43,17 +38,12 @@ Feature: IMAP remove messages from Trash And the user logs in with username "user@pm.me" and password "password" And user "user@pm.me" finishes syncing And user "user@pm.me" connects and authenticates IMAP client "1" - And IMAP client "1" selects "" + And IMAP client "1" selects "Trash" When IMAP client "1" marks the message with subject "foo" as deleted Then it succeeds - And IMAP client "1" sees 2 messages in "" + And IMAP client "1" sees 2 messages in "Trash" And IMAP client "1" sees 2 messages in "All Mail" When IMAP client "1" expunges Then it succeeds - And IMAP client "1" sees 1 messages in "" - And IMAP client "1" eventually sees 1 messages in "All Mail" - - Examples: - | mailbox | - | Spam | - | Trash | + And IMAP client "1" sees 1 messages in "Trash" + And IMAP client "1" eventually sees 1 messages in "All Mail" \ No newline at end of file diff --git a/tests/features/imap/message/drafts.feature b/tests/features/imap/message/drafts.feature index 72c91ee4..721d84d8 100644 --- a/tests/features/imap/message/drafts.feature +++ b/tests/features/imap/message/drafts.feature @@ -11,10 +11,15 @@ Feature: IMAP Draft messages This is a dra """ + Then IMAP client "1" eventually sees the following messages in "Drafts": + | body | + | This is a dra | + And IMAP client "1" sees 1 messages in "Drafts" Scenario: Draft edited locally When IMAP client "1" marks message 1 as deleted And IMAP client "1" expunges + And it succeeds And IMAP client "1" appends the following message to "Drafts": """ Subject: Basic Draft @@ -30,7 +35,7 @@ Feature: IMAP Draft messages And IMAP client "1" sees 1 messages in "Drafts" Scenario: Draft edited remotely - When the following fields where changed in draft 1 for address "user@pm.me" of account "user@pm.me": + When the following fields were changed in draft 1 for address "user@pm.me" of account "user@pm.me": | to | subject | body | | someone@proton.me | Basic Draft | This is a draft body, but longer | Then IMAP client "1" eventually sees the following messages in "Drafts": diff --git a/tests/imap_test.go b/tests/imap_test.go index 0b92f8ed..f7707d1c 100644 --- a/tests/imap_test.go +++ b/tests/imap_test.go @@ -33,6 +33,7 @@ import ( "github.com/emersion/go-imap" id "github.com/emersion/go-imap-id" "github.com/emersion/go-imap/client" + "github.com/sirupsen/logrus" "golang.org/x/exp/slices" ) @@ -280,7 +281,9 @@ func (s *scenario) imapClientSeesTheFollowingMessagesInMailbox(clientID, mailbox func (s *scenario) imapClientEventuallySeesTheFollowingMessagesInMailbox(clientID, mailbox string, table *godog.Table) error { return eventually(func() error { - return s.imapClientSeesTheFollowingMessagesInMailbox(clientID, mailbox, table) + err := s.imapClientSeesTheFollowingMessagesInMailbox(clientID, mailbox, table) + logrus.WithError(err).Trace("Matching eventually") + return err }) } @@ -374,7 +377,9 @@ func (s *scenario) imapClientSeesThatMessageHasTheFlag(clientID string, seq int, func (s *scenario) imapClientExpunges(clientID string) error { _, client := s.t.getIMAPClient(clientID) - return client.Expunge(nil) + s.t.pushError(client.Expunge(nil)) + + return nil } func (s *scenario) imapClientAppendsTheFollowingMessageToMailbox(clientID string, mailbox string, docString *godog.DocString) error { diff --git a/tests/types_test.go b/tests/types_test.go index 6901d5d2..151199b0 100644 --- a/tests/types_test.go +++ b/tests/types_test.go @@ -144,7 +144,7 @@ func matchMessages(have, want []Message) error { }) if !IsSub(ToAny(have), ToAny(want)) { - return fmt.Errorf("missing messages: have %+v, want %+v", have, want) + return fmt.Errorf("missing messages: have %#v, want %#v", have, want) } return nil diff --git a/tests/user_test.go b/tests/user_test.go index 08cc3f76..eb7be069 100644 --- a/tests/user_test.go +++ b/tests/user_test.go @@ -226,7 +226,7 @@ func (s *scenario) theAddressOfAccountHasNoKeys(address, username string) error // accountDraftChanged changes the draft attributes, where draftIndex is // similar to sequential ID i.e. 1 represents the first message of draft folder // sorted by API creation time. -func (s *scenario) addressDraftChanged(draftIndex int, address, username string, table *godog.Table) error { +func (s *scenario) theFollowingFieldsWereChangedInDraftForAddressOfAccount(draftIndex int, address, username string, table *godog.Table) error { wantMessages, err := unmarshalTable[Message](table) if err != nil { return err @@ -236,35 +236,49 @@ func (s *scenario) addressDraftChanged(draftIndex int, address, username string, return fmt.Errorf("expected to have one row in table but got %d instead", len(wantMessages)) } - draftID := s.t.getDraftID(username, draftIndex) - - encBody := []byte{} - - if wantMessages[0].Body != "" { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - if err := s.t.withClient(ctx, username, func(ctx context.Context, c *proton.Client) error { - return s.t.withAddrKR(ctx, c, username, s.t.getUserAddrID(s.t.getUserID(username), address), - func(ctx context.Context, addrKR *crypto.KeyRing) error { - var err error - encBody, err = proton.EncryptRFC822(addrKR, wantMessages[0].Build()) - return err - }) - }); err != nil { - return err - } + draftID, err := s.t.getDraftID(username, draftIndex) + if err != nil { + return fmt.Errorf("failed to get draft ID: %w", err) } - changes := proton.DraftTemplate{ - Subject: wantMessages[0].Subject, - Body: string(encBody), - } - if wantMessages[0].To != "" { - changes.ToList = []*mail.Address{{Address: wantMessages[0].To}} - } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() - return s.t.api.UpdateDraft(s.t.getUserID(username), draftID, changes) + return s.t.withClient(ctx, username, func(ctx context.Context, c *proton.Client) error { + return s.t.withAddrKR(ctx, c, username, s.t.getUserAddrID(s.t.getUserID(username), address), func(_ context.Context, addrKR *crypto.KeyRing) error { + var changes proton.DraftTemplate + + if wantMessages[0].From != "" { + return fmt.Errorf("changing from address is not supported") + } + + if wantMessages[0].To != "" { + changes.ToList = []*mail.Address{{Address: wantMessages[0].To}} + } + + if wantMessages[0].CC != "" { + changes.CCList = []*mail.Address{{Address: wantMessages[0].CC}} + } + + if wantMessages[0].BCC != "" { + changes.BCCList = []*mail.Address{{Address: wantMessages[0].BCC}} + } + + if wantMessages[0].Subject != "" { + changes.Subject = wantMessages[0].Subject + } + + if wantMessages[0].Body != "" { + changes.Body = wantMessages[0].Body + } + + if _, err := c.UpdateDraft(ctx, draftID, addrKR, proton.UpdateDraftReq{Message: changes}); err != nil { + return fmt.Errorf("failed to update draft: %w", err) + } + + return nil + }) + }) } func (s *scenario) userLogsInWithUsernameAndPassword(username, password string) error {