From 70244071ea3c1689bbfb3e7e20d562c2ec345118 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 30 Nov 2022 16:32:30 +0100 Subject: [PATCH 01/19] Other: Bump go-proton-api to v0.1.4 --- go.mod | 2 +- go.sum | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index a6bc393b..2ddcdb2b 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.20221129150032-c663738a6cee 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.1.4 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 3a5edec0..323bc27d 100644 --- a/go.sum +++ b/go.sum @@ -43,8 +43,10 @@ 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.1.3 h1:Y0kdBTbyxEBo4eVK1BD7V9GSTPKUiy/soYR7bg2i2TU= +github.com/ProtonMail/go-proton-api v0.1.3/go.mod h1:jqvJ2HqLHqiPJoEb+BTIB1IF7wvr6p+8ZfA6PO2NRNk= +github.com/ProtonMail/go-proton-api v0.1.4 h1:tGZlYuXlvT7AxFUPry42AsukLYm0HazlRg1iZLvlAZE= +github.com/ProtonMail/go-proton-api v0.1.4/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= From 83a569b3661c9480831ddf3588606d613254e1c9 Mon Sep 17 00:00:00 2001 From: Jakub Date: Thu, 1 Dec 2022 08:42:24 +0100 Subject: [PATCH 02/19] Other: Bridge Perth Narrows v3.0.3 --- Changelog.md | 5 +++++ Makefile | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 78d3397c..18451573 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,11 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) +## [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..084b631e 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.3+git APP_VERSION:=${BRIDGE_APP_VERSION} APP_FULL_NAME:=Proton Mail Bridge APP_VENDOR:=Proton AG From 618cb27ac19ea68a71d7826bfa5fbc748b4e9b06 Mon Sep 17 00:00:00 2001 From: Leander Beernaert Date: Fri, 2 Dec 2022 13:43:08 +0100 Subject: [PATCH 03/19] Other: Disable perma-delete for expunge on Spam folder --- internal/user/imap.go | 2 +- .../imap/message/delete_from_trash.feature | 34 +++++++------------ 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/internal/user/imap.go b/internal/user/imap.go index ec8de275..e3432713 100644 --- a/internal/user/imap.go +++ b/internal/user/imap.go @@ -326,7 +326,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, 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 From 5c3179df487681b1209e61d185cdc21d22c57217 Mon Sep 17 00:00:00 2001 From: Jakub Date: Thu, 1 Dec 2022 11:44:32 +0100 Subject: [PATCH 04/19] GODT-2170: User create draft rounte: first steps. --- internal/user/imap.go | 110 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 98 insertions(+), 12 deletions(-) diff --git a/internal/user/imap.go b/internal/user/imap.go index e3432713..527e8085 100644 --- a/internal/user/imap.go +++ b/internal/user/imap.go @@ -18,6 +18,7 @@ package user import ( + "bytes" "context" "fmt" "sync/atomic" @@ -31,6 +32,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" @@ -437,20 +439,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) + 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 +516,73 @@ func toIMAPMessage(message proton.MessageMetadata) imap.Message { } } +func (conn *imapConnector) createDraft(ctx context.Context, literal []byte, addrKR *crypto.KeyRing) (proton.Message, error) { + // 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) + } + + 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 := conn.client.CreateDraft(ctx, proton.CreateDraftReq{ + Message: proton.DraftTemplate{ + Subject: message.Subject, + Body: armBody, + MIMEType: message.MIMEType, + + Sender: message.Sender, + 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...) From 828fe0e86ec14ff27a8fb9f50d5623f48e910b9a Mon Sep 17 00:00:00 2001 From: Jakub Date: Thu, 1 Dec 2022 11:58:58 +0100 Subject: [PATCH 05/19] GODT-2170: Update draft event means delete old and create new message. --- internal/user/events.go | 33 +++++++++++++++++---------------- internal/user/imap.go | 2 +- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/internal/user/events.go b/internal/user/events.go index d027db89..4339128e 100644 --- a/internal/user/events.go +++ b/internal/user/events.go @@ -389,6 +389,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. @@ -400,16 +412,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"), @@ -485,6 +487,10 @@ func (user *User) handleUpdateDraftEvent(ctx context.Context, event proton.Messa "subject": logging.Sensitive(event.Message.Subject), }).Info("Handling draft updated event") + for _, updateCh := range user.updateCh { + updateCh.Enqueue(imap.NewMessagesDeleted(imap.MessageID(event.ID))) + } + full, err := user.client.GetFullMessage(ctx, event.Message.ID) if err != nil { return fmt.Errorf("failed to get full draft: %w", err) @@ -496,12 +502,7 @@ func (user *User) handleUpdateDraftEvent(ctx context.Context, event proton.Messa return fmt.Errorf("failed to build RFC822 draft: %w", err) } - user.updateCh[full.AddressID].Enqueue(imap.NewMessageUpdated( - buildRes.update.Message, - buildRes.update.Literal, - buildRes.update.MailboxIDs, - buildRes.update.ParsedMessage, - )) + user.updateCh[full.AddressID].Enqueue(imap.NewMessagesCreated(buildRes.update)) return nil }) diff --git a/internal/user/imap.go b/internal/user/imap.go index 527e8085..33668627 100644 --- a/internal/user/imap.go +++ b/internal/user/imap.go @@ -516,7 +516,7 @@ func toIMAPMessage(message proton.MessageMetadata) imap.Message { } } -func (conn *imapConnector) createDraft(ctx context.Context, literal []byte, addrKR *crypto.KeyRing) (proton.Message, error) { +func (conn *imapConnector) createDraft(ctx context.Context, literal []byte, addrKR *crypto.KeyRing) (proton.Message, error) { //nolint:funlen // Create a new message parser from the reader. parser, err := parser.New(bytes.NewReader(literal)) if err != nil { From 8408a5fdc00e95bc2d4253f75acb6c310a3a1c09 Mon Sep 17 00:00:00 2001 From: Jakub Date: Thu, 1 Dec 2022 15:52:42 +0100 Subject: [PATCH 06/19] GODT-2170: Improving test server behaviour. --- go.mod | 4 ++-- go.sum | 6 ++++++ internal/user/events.go | 11 ++++++----- internal/user/imap.go | 7 ++++--- tests/features/imap/message/create.feature | 1 + tests/features/imap/message/drafts.feature | 4 ++++ tests/imap_test.go | 5 ++++- tests/types_test.go | 2 +- tests/user_test.go | 12 ++++++++---- 9 files changed, 36 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index 2ddcdb2b..f1e98ef4 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.20221129150032-c663738a6cee + github.com/ProtonMail/gluon v0.14.2-0.20221201132248-f620ad183873 github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a - github.com/ProtonMail/go-proton-api v0.1.4 + github.com/ProtonMail/go-proton-api v0.1.4-0.20221201145430-b0a0286c470e 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 323bc27d..edc29813 100644 --- a/go.sum +++ b/go.sum @@ -30,6 +30,10 @@ 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.20221129150032-c663738a6cee h1:rDGqVa4CepqpJF8TDjqnBITqD8OzrLzeg66ibVDCPSc= github.com/ProtonMail/gluon v0.14.2-0.20221129150032-c663738a6cee/go.mod h1:z2AxLIiBCT1K+0OBHyaDI7AEaO5qI6/BEC2TE42vs4Q= +github.com/ProtonMail/gluon v0.14.2-0.20221201094406-fc09f2d35933 h1:kj6JBn/ASSDKdLvZPU7fFPKC5vN+UgboBvfJzAaZkrY= +github.com/ProtonMail/gluon v0.14.2-0.20221201094406-fc09f2d35933/go.mod h1:z2AxLIiBCT1K+0OBHyaDI7AEaO5qI6/BEC2TE42vs4Q= +github.com/ProtonMail/gluon v0.14.2-0.20221201132248-f620ad183873 h1:9LKxCftz374K4cse/AdIaUgGLwQlrCPIMIA7llcOck4= +github.com/ProtonMail/gluon v0.14.2-0.20221201132248-f620ad183873/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= @@ -45,6 +49,8 @@ github.com/ProtonMail/go-mime v0.0.0-20220429130430-2192574d760f h1:4IWzKjHzZxdr github.com/ProtonMail/go-mime v0.0.0-20220429130430-2192574d760f/go.mod h1:qRZgbeASl2a9OwmsV85aWwRqic0NHPh+9ewGAzb4cgM= github.com/ProtonMail/go-proton-api v0.1.3 h1:Y0kdBTbyxEBo4eVK1BD7V9GSTPKUiy/soYR7bg2i2TU= github.com/ProtonMail/go-proton-api v0.1.3/go.mod h1:jqvJ2HqLHqiPJoEb+BTIB1IF7wvr6p+8ZfA6PO2NRNk= +github.com/ProtonMail/go-proton-api v0.1.4-0.20221201145430-b0a0286c470e h1:PQIbcD4ZpHsZaLT2RcVuoWi7yl2f2x51KthTrerOpqI= +github.com/ProtonMail/go-proton-api v0.1.4-0.20221201145430-b0a0286c470e/go.mod h1:jqvJ2HqLHqiPJoEb+BTIB1IF7wvr6p+8ZfA6PO2NRNk= github.com/ProtonMail/go-proton-api v0.1.4 h1:tGZlYuXlvT7AxFUPry42AsukLYm0HazlRg1iZLvlAZE= github.com/ProtonMail/go-proton-api v0.1.4/go.mod h1:jqvJ2HqLHqiPJoEb+BTIB1IF7wvr6p+8ZfA6PO2NRNk= github.com/ProtonMail/go-rfc5322 v0.11.0 h1:o5Obrm4DpmQEffvgsVqG6S4BKwC1Wat+hYwjIp2YcCY= diff --git a/internal/user/events.go b/internal/user/events.go index 4339128e..41b47a1e 100644 --- a/internal/user/events.go +++ b/internal/user/events.go @@ -487,10 +487,6 @@ func (user *User) handleUpdateDraftEvent(ctx context.Context, event proton.Messa "subject": logging.Sensitive(event.Message.Subject), }).Info("Handling draft updated event") - for _, updateCh := range user.updateCh { - updateCh.Enqueue(imap.NewMessagesDeleted(imap.MessageID(event.ID))) - } - full, err := user.client.GetFullMessage(ctx, event.Message.ID) if err != nil { return fmt.Errorf("failed to get full draft: %w", err) @@ -502,7 +498,12 @@ func (user *User) handleUpdateDraftEvent(ctx context.Context, event proton.Messa return fmt.Errorf("failed to build RFC822 draft: %w", err) } - user.updateCh[full.AddressID].Enqueue(imap.NewMessagesCreated(buildRes.update)) + user.updateCh[full.AddressID].Enqueue(imap.NewMessageUpdated( + buildRes.update.Message, + buildRes.update.Literal, + buildRes.update.MailboxIDs, + buildRes.update.ParsedMessage, + )) return nil }) diff --git a/internal/user/imap.go b/internal/user/imap.go index 33668627..ec5f70ff 100644 --- a/internal/user/imap.go +++ b/internal/user/imap.go @@ -21,6 +21,7 @@ import ( "bytes" "context" "fmt" + "net/mail" "sync/atomic" "time" @@ -442,7 +443,7 @@ func (conn *imapConnector) importMessage( messageID := "" if slices.Contains(labelIDs, proton.DraftsLabel) { - msg, err := conn.createDraft(ctx, literal, addrKR) + msg, err := conn.createDraft(ctx, literal, addrKR, conn.apiAddrs[conn.addrID]) if err != nil { return fmt.Errorf("failed to create draft: %w", err) } @@ -516,7 +517,7 @@ func toIMAPMessage(message proton.MessageMetadata) imap.Message { } } -func (conn *imapConnector) createDraft(ctx context.Context, literal []byte, addrKR *crypto.KeyRing) (proton.Message, error) { //nolint:funlen +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 { @@ -549,7 +550,7 @@ func (conn *imapConnector) createDraft(ctx context.Context, literal []byte, addr Body: armBody, MIMEType: message.MIMEType, - Sender: message.Sender, + Sender: &mail.Address{Name: sender.DisplayName, Address: sender.Email}, ToList: message.ToList, CCList: message.CCList, BCCList: message.BCCList, 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/drafts.feature b/tests/features/imap/message/drafts.feature index 72c91ee4..1f2a8c36 100644 --- a/tests/features/imap/message/drafts.feature +++ b/tests/features/imap/message/drafts.feature @@ -11,6 +11,10 @@ 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 diff --git a/tests/imap_test.go b/tests/imap_test.go index 0b92f8ed..efeaf1bd 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 }) } 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..1985f8f8 100644 --- a/tests/user_test.go +++ b/tests/user_test.go @@ -238,7 +238,7 @@ func (s *scenario) addressDraftChanged(draftIndex int, address, username string, draftID := s.t.getDraftID(username, draftIndex) - encBody := []byte{} + encBody := "" if wantMessages[0].Body != "" { ctx, cancel := context.WithCancel(context.Background()) @@ -247,8 +247,12 @@ func (s *scenario) addressDraftChanged(draftIndex int, address, username string, 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()) + msg, err := addrKR.Encrypt(crypto.NewPlainMessage([]byte(wantMessages[0].Body)), addrKR) + if err != nil { + return err + } + + encBody, err = msg.GetArmored() return err }) }); err != nil { @@ -258,7 +262,7 @@ func (s *scenario) addressDraftChanged(draftIndex int, address, username string, changes := proton.DraftTemplate{ Subject: wantMessages[0].Subject, - Body: string(encBody), + Body: encBody, } if wantMessages[0].To != "" { changes.ToList = []*mail.Address{{Address: wantMessages[0].To}} From 01c7daaba7cbd7caaafd1de03d748eabb9246249 Mon Sep 17 00:00:00 2001 From: Leander Beernaert Date: Fri, 2 Dec 2022 11:25:54 +0100 Subject: [PATCH 07/19] Other: Update gluon to latest version --- go.mod | 2 +- go.sum | 12 ++---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index f1e98ef4..8eef9471 100644 --- a/go.mod +++ b/go.mod @@ -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.20221201132248-f620ad183873 + github.com/ProtonMail/gluon v0.14.2-0.20221202093012-ad1570c49c8c github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a github.com/ProtonMail/go-proton-api v0.1.4-0.20221201145430-b0a0286c470e github.com/ProtonMail/go-rfc5322 v0.11.0 diff --git a/go.sum b/go.sum index edc29813..3e5049c5 100644 --- a/go.sum +++ b/go.sum @@ -28,12 +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.20221129150032-c663738a6cee h1:rDGqVa4CepqpJF8TDjqnBITqD8OzrLzeg66ibVDCPSc= -github.com/ProtonMail/gluon v0.14.2-0.20221129150032-c663738a6cee/go.mod h1:z2AxLIiBCT1K+0OBHyaDI7AEaO5qI6/BEC2TE42vs4Q= -github.com/ProtonMail/gluon v0.14.2-0.20221201094406-fc09f2d35933 h1:kj6JBn/ASSDKdLvZPU7fFPKC5vN+UgboBvfJzAaZkrY= -github.com/ProtonMail/gluon v0.14.2-0.20221201094406-fc09f2d35933/go.mod h1:z2AxLIiBCT1K+0OBHyaDI7AEaO5qI6/BEC2TE42vs4Q= -github.com/ProtonMail/gluon v0.14.2-0.20221201132248-f620ad183873 h1:9LKxCftz374K4cse/AdIaUgGLwQlrCPIMIA7llcOck4= -github.com/ProtonMail/gluon v0.14.2-0.20221201132248-f620ad183873/go.mod h1:z2AxLIiBCT1K+0OBHyaDI7AEaO5qI6/BEC2TE42vs4Q= +github.com/ProtonMail/gluon v0.14.2-0.20221202093012-ad1570c49c8c h1:DzVlJERHOHDQjYz/P12VlORS4rF2Ii83cWcYHsXGdng= +github.com/ProtonMail/gluon v0.14.2-0.20221202093012-ad1570c49c8c/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= @@ -47,12 +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.3 h1:Y0kdBTbyxEBo4eVK1BD7V9GSTPKUiy/soYR7bg2i2TU= -github.com/ProtonMail/go-proton-api v0.1.3/go.mod h1:jqvJ2HqLHqiPJoEb+BTIB1IF7wvr6p+8ZfA6PO2NRNk= github.com/ProtonMail/go-proton-api v0.1.4-0.20221201145430-b0a0286c470e h1:PQIbcD4ZpHsZaLT2RcVuoWi7yl2f2x51KthTrerOpqI= github.com/ProtonMail/go-proton-api v0.1.4-0.20221201145430-b0a0286c470e/go.mod h1:jqvJ2HqLHqiPJoEb+BTIB1IF7wvr6p+8ZfA6PO2NRNk= -github.com/ProtonMail/go-proton-api v0.1.4 h1:tGZlYuXlvT7AxFUPry42AsukLYm0HazlRg1iZLvlAZE= -github.com/ProtonMail/go-proton-api v0.1.4/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= From 7bc608ce6c4c0852f157642ecb9793686c4375a3 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Fri, 2 Dec 2022 12:47:12 +0100 Subject: [PATCH 08/19] GODT-2170: Use client-side draft update in integration tests --- go.mod | 2 +- go.sum | 4 +- tests/api_test.go | 3 - tests/bdd_test.go | 2 +- tests/ctx_test.go | 22 ++---- tests/features/imap/message/drafts.feature | 2 +- tests/user_test.go | 82 ++++++++++++++-------- 7 files changed, 63 insertions(+), 54 deletions(-) diff --git a/go.mod b/go.mod index 8eef9471..3021f04a 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.20221202093012-ad1570c49c8c github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a - github.com/ProtonMail/go-proton-api v0.1.4-0.20221201145430-b0a0286c470e + github.com/ProtonMail/go-proton-api v0.1.5 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 3e5049c5..1a784fd6 100644 --- a/go.sum +++ b/go.sum @@ -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.4-0.20221201145430-b0a0286c470e h1:PQIbcD4ZpHsZaLT2RcVuoWi7yl2f2x51KthTrerOpqI= -github.com/ProtonMail/go-proton-api v0.1.4-0.20221201145430-b0a0286c470e/go.mod h1:jqvJ2HqLHqiPJoEb+BTIB1IF7wvr6p+8ZfA6PO2NRNk= +github.com/ProtonMail/go-proton-api v0.1.5 h1:6RJO3jXP3opFfGqrXNvFTefdD4MlfxKjIebT8r5ROf8= +github.com/ProtonMail/go-proton-api v0.1.5/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/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/drafts.feature b/tests/features/imap/message/drafts.feature index 1f2a8c36..bdcfb46b 100644 --- a/tests/features/imap/message/drafts.feature +++ b/tests/features/imap/message/drafts.feature @@ -34,7 +34,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/user_test.go b/tests/user_test.go index 1985f8f8..f28c17fd 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,39 +236,59 @@ 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 := "" - - 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 { - msg, err := addrKR.Encrypt(crypto.NewPlainMessage([]byte(wantMessages[0].Body)), addrKR) - if err != nil { - return err - } - - encBody, err = msg.GetArmored() - 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: 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 != "" { + enc, err := addrKR.Encrypt(crypto.NewPlainMessageFromString(wantMessages[0].Body), addrKR) + if err != nil { + return fmt.Errorf("failed to encrypt message body: %w", err) + } + + arm, err := enc.GetArmored() + if err != nil { + return fmt.Errorf("failed to get armored message: %w", err) + } + + changes.Body = arm + } + + if _, err := c.UpdateDraft(ctx, draftID, 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 { From 8990f2d1d6f3293ffb74b79a3c09f5b684823567 Mon Sep 17 00:00:00 2001 From: Leander Beernaert Date: Fri, 2 Dec 2022 14:56:29 +0100 Subject: [PATCH 09/19] Other: Ensure expunge feature test pushes to error stack --- tests/features/imap/message/delete.feature | 14 ++++++++------ tests/features/imap/message/drafts.feature | 1 + tests/imap_test.go | 4 +++- 3 files changed, 12 insertions(+), 7 deletions(-) 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/drafts.feature b/tests/features/imap/message/drafts.feature index bdcfb46b..721d84d8 100644 --- a/tests/features/imap/message/drafts.feature +++ b/tests/features/imap/message/drafts.feature @@ -19,6 +19,7 @@ Feature: IMAP Draft messages 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 diff --git a/tests/imap_test.go b/tests/imap_test.go index efeaf1bd..f7707d1c 100644 --- a/tests/imap_test.go +++ b/tests/imap_test.go @@ -377,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 { From 2cd773546864d94fba86949443259ad5855f60bb Mon Sep 17 00:00:00 2001 From: Leander Beernaert Date: Fri, 2 Dec 2022 14:59:22 +0100 Subject: [PATCH 10/19] Other: Do not list \Deleted flag for All Mail --- internal/user/sync.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/user/sync.go b/internal/user/sync.go index c81b480d..611e23d2 100644 --- a/internal/user/sync.go +++ b/internal/user/sync.go @@ -333,6 +333,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 +345,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 +364,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, }) } From 27889b8085a381be28b6cd2572b1dc85902c843a Mon Sep 17 00:00:00 2001 From: Romain LE JEUNE Date: Fri, 2 Dec 2022 15:42:11 +0100 Subject: [PATCH 11/19] Other: Bridge Perth Narrows v3.0.4 --- Changelog.md | 13 +++++++++++++ Makefile | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 18451573..dd67c0a4 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,19 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) +## [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 diff --git a/Makefile b/Makefile index 084b631e..20340c89 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.3+git +BRIDGE_APP_VERSION?=3.0.4+git APP_VERSION:=${BRIDGE_APP_VERSION} APP_FULL_NAME:=Proton Mail Bridge APP_VENDOR:=Proton AG From 990b8cda962f9d36389909cf6618f7ae2c8572d1 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Mon, 5 Dec 2022 12:05:35 +0100 Subject: [PATCH 12/19] GODT-2180: Allow login with FIDO2 The API docs didn't specify what the "integer" meant. Turns out it's a bitfield; we can't compare with equality. --- internal/bridge/user.go | 2 +- internal/frontend/cli/accounts.go | 2 +- internal/frontend/grpc/service_methods.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/bridge/user.go b/internal/bridge/user.go index e8eff23c..465c5fa6 100644 --- a/internal/bridge/user.go +++ b/internal/bridge/user.go @@ -182,7 +182,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.TOTPEnabled != 0 { logrus.WithField("userID", auth.UserID).Info("Requesting TOTP") totp, err := getTOTP() diff --git a/internal/frontend/cli/accounts.go b/internal/frontend/cli/accounts.go index 00fa69ee..10884d4a 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.TOTPEnabled != 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 d5093c53..100981e1 100644 --- a/internal/frontend/grpc/service_methods.go +++ b/internal/frontend/grpc/service_methods.go @@ -406,7 +406,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.TOTPEnabled != 0: _ = s.SendEvent(NewLoginTfaRequestedEvent(login.Username)) case auth.PasswordMode == proton.TwoPasswordMode: From 04881b9b7831978fa6216b8a310c3203811f34e7 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Mon, 5 Dec 2022 15:13:33 +0100 Subject: [PATCH 13/19] GODT-2178: Bump go-proton-api to fix drafts --- go.mod | 2 +- go.sum | 4 ++-- internal/bridge/user.go | 2 +- internal/frontend/cli/accounts.go | 2 +- internal/frontend/grpc/service_methods.go | 2 +- internal/user/imap.go | 14 ++------------ internal/user/smtp.go | 17 ++++------------- tests/user_test.go | 14 ++------------ 8 files changed, 14 insertions(+), 43 deletions(-) diff --git a/go.mod b/go.mod index 3021f04a..343a5d12 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.20221202093012-ad1570c49c8c github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a - github.com/ProtonMail/go-proton-api v0.1.5 + 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 1a784fd6..a5a0a02a 100644 --- a/go.sum +++ b/go.sum @@ -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.5 h1:6RJO3jXP3opFfGqrXNvFTefdD4MlfxKjIebT8r5ROf8= -github.com/ProtonMail/go-proton-api v0.1.5/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 465c5fa6..3e34058c 100644 --- a/internal/bridge/user.go +++ b/internal/bridge/user.go @@ -182,7 +182,7 @@ func (bridge *Bridge) LoginFull( return "", fmt.Errorf("failed to begin login process: %w", err) } - if auth.TwoFA.Enabled&proton.TOTPEnabled != 0 { + if auth.TwoFA.Enabled&proton.HasTOTP != 0 { logrus.WithField("userID", auth.UserID).Info("Requesting TOTP") totp, err := getTOTP() diff --git a/internal/frontend/cli/accounts.go b/internal/frontend/cli/accounts.go index 10884d4a..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 != 0 { + 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 100981e1..efeb7bb6 100644 --- a/internal/frontend/grpc/service_methods.go +++ b/internal/frontend/grpc/service_methods.go @@ -406,7 +406,7 @@ func (s *Service) Login(ctx context.Context, login *LoginRequest) (*emptypb.Empt s.auth = auth switch { - case auth.TwoFA.Enabled&proton.TOTPEnabled != 0: + case auth.TwoFA.Enabled&proton.HasTOTP != 0: _ = s.SendEvent(NewLoginTfaRequestedEvent(login.Username)) case auth.PasswordMode == proton.TwoPasswordMode: diff --git a/internal/user/imap.go b/internal/user/imap.go index ec5f70ff..ba6c734b 100644 --- a/internal/user/imap.go +++ b/internal/user/imap.go @@ -534,20 +534,10 @@ func (conn *imapConnector) createDraft(ctx context.Context, literal []byte, addr decBody = string(message.RichBody) } - 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 := conn.client.CreateDraft(ctx, proton.CreateDraftReq{ + draft, err := conn.client.CreateDraft(ctx, addrKR, proton.CreateDraftReq{ Message: proton.DraftTemplate{ Subject: message.Subject, - Body: armBody, + Body: decBody, MIMEType: message.MIMEType, Sender: &mail.Address{Name: sender.DisplayName, Address: sender.Email}, 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/tests/user_test.go b/tests/user_test.go index f28c17fd..eb7be069 100644 --- a/tests/user_test.go +++ b/tests/user_test.go @@ -269,20 +269,10 @@ func (s *scenario) theFollowingFieldsWereChangedInDraftForAddressOfAccount(draft } if wantMessages[0].Body != "" { - enc, err := addrKR.Encrypt(crypto.NewPlainMessageFromString(wantMessages[0].Body), addrKR) - if err != nil { - return fmt.Errorf("failed to encrypt message body: %w", err) - } - - arm, err := enc.GetArmored() - if err != nil { - return fmt.Errorf("failed to get armored message: %w", err) - } - - changes.Body = arm + changes.Body = wantMessages[0].Body } - if _, err := c.UpdateDraft(ctx, draftID, proton.UpdateDraftReq{Message: changes}); err != nil { + if _, err := c.UpdateDraft(ctx, draftID, addrKR, proton.UpdateDraftReq{Message: changes}); err != nil { return fmt.Errorf("failed to update draft: %w", err) } From d4198737a6d5e8e28e52778bfe8e803efab0286e Mon Sep 17 00:00:00 2001 From: Romain LE JEUNE Date: Mon, 5 Dec 2022 15:42:49 +0100 Subject: [PATCH 14/19] Other: Bridge Perth Narrows v3.0.5 --- Changelog.md | 6 ++++++ Makefile | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index dd67c0a4..7cc17e8f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,12 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) +## [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 diff --git a/Makefile b/Makefile index 20340c89..f5bfc9da 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.4+git +BRIDGE_APP_VERSION?=3.0.5+git APP_VERSION:=${BRIDGE_APP_VERSION} APP_FULL_NAME:=Proton Mail Bridge APP_VENDOR:=Proton AG From 01c12655b8f2ae1f96cba852a1ff90f8ea805fb6 Mon Sep 17 00:00:00 2001 From: Leander Beernaert Date: Tue, 6 Dec 2022 11:49:39 +0100 Subject: [PATCH 15/19] Other: Update Gluon to latest version Fixes: #316 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 343a5d12..b1f5321c 100644 --- a/go.mod +++ b/go.mod @@ -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.20221202093012-ad1570c49c8c + 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.2.1 github.com/ProtonMail/go-rfc5322 v0.11.0 diff --git a/go.sum b/go.sum index a5a0a02a..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.20221202093012-ad1570c49c8c h1:DzVlJERHOHDQjYz/P12VlORS4rF2Ii83cWcYHsXGdng= -github.com/ProtonMail/gluon v0.14.2-0.20221202093012-ad1570c49c8c/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= From 58d04f9693e5e654e6a7a03cd94691b84743a31c Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Tue, 6 Dec 2022 13:04:24 +0100 Subject: [PATCH 16/19] GODT-2187: Skip messages during sync that fail to build/parse --- internal/bridge/user.go | 2 +- internal/user/events.go | 14 +++---- internal/user/sync.go | 74 ++++++++++++++++++++++++---------- internal/user/sync_build.go | 49 +++++++++++++++++----- internal/user/sync_reporter.go | 10 ++--- internal/user/user.go | 10 ++--- internal/user/user_test.go | 2 +- internal/vault/types_user.go | 7 ++-- internal/vault/user.go | 20 +++++++++ 9 files changed, 133 insertions(+), 55 deletions(-) diff --git a/internal/bridge/user.go b/internal/bridge/user.go index 3e34058c..b13868c3 100644 --- a/internal/bridge/user.go +++ b/internal/bridge/user.go @@ -441,9 +441,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/user/events.go b/internal/user/events.go index 41b47a1e..adcda965 100644 --- a/internal/user/events.go +++ b/internal/user/events.go @@ -438,12 +438,12 @@ 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) + update, err := buildRFC822(user.apiLabels, full, addrKR).update.unpack() if err != nil { return fmt.Errorf("failed to build RFC822 message: %w", err) } - user.updateCh[full.AddressID].Enqueue(imap.NewMessagesCreated(buildRes.update)) + user.updateCh[full.AddressID].Enqueue(imap.NewMessagesCreated(update)) return nil }) @@ -493,16 +493,16 @@ 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) + update, err := buildRFC822(user.apiLabels, full, addrKR).update.unpack() if err != nil { return fmt.Errorf("failed to build RFC822 draft: %w", err) } user.updateCh[full.AddressID].Enqueue(imap.NewMessageUpdated( - buildRes.update.Message, - buildRes.update.Literal, - buildRes.update.MailboxIDs, - buildRes.update.ParsedMessage, + update.Message, + update.Literal, + update.MailboxIDs, + update.ParsedMessage, )) return nil diff --git a/internal/user/sync.go b/internal/user/sync.go index 611e23d2..173bbbf0 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 @@ -289,7 +300,26 @@ func syncMessages( defer close(flushUpdateCh) for batch := range flushCh { for _, res := range batch { - flushers[res.addressID].push(res.update) + if err := res.update.err(); 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 sync message", reporter.Context{ + "messageID": res.messageID, + "error": err, + }); err != nil { + logrus.WithError(err).Error("Failed to report message sync error") + } + + 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.unwrap()) + } } for _, flusher := range flushers { @@ -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 diff --git a/internal/user/sync_build.go b/internal/user/sync_build.go index 897474a0..b5cfbd58 100644 --- a/internal/user/sync_build.go +++ b/internal/user/sync_build.go @@ -26,10 +26,39 @@ import ( "github.com/ProtonMail/proton-bridge/v3/pkg/message" ) +type result[T any] struct { + v T + e error +} + +func resOk[T any](v T) result[T] { + return result[T]{v: v} +} + +func resErr[T any](e error) result[T] { + return result[T]{e: e} +} + +func (r *result[T]) unwrap() T { + if r.e != nil { + panic(r.err) + } + + return r.v +} + +func (r *result[T]) unpack() (T, error) { + return r.v, r.e +} + +func (r *result[T]) err() error { + return r.e +} + type buildRes struct { messageID string addressID string - update *imap.MessageCreated + update result[*imap.MessageCreated] } func defaultJobOpts() message.JobOptions { @@ -43,22 +72,22 @@ 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 result[*imap.MessageCreated] - 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, err := message.BuildRFC822(addrKR, full.Message, full.AttData, defaultJobOpts()); err != nil { + update = resErr[*imap.MessageCreated](fmt.Errorf("failed to build RFC822 for message %s: %w", full.ID, err)) + } else if created, err := newMessageCreatedUpdate(apiLabels, full.MessageMetadata, literal); err != nil { + update = resErr[*imap.MessageCreated](fmt.Errorf("failed to create IMAP update for message %s: %w", full.ID, err)) + } else { + update = resOk(created) } return &buildRes{ messageID: full.ID, addressID: full.AddressID, update: update, - }, nil + } } func newMessageCreatedUpdate( 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/user.go b/internal/user/user.go index 28aebc0c..ccee8d65 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{}), 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) { From bd6ae2ac2b45ed58708ecb44a7c8147378249abf Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Tue, 6 Dec 2022 15:27:28 +0100 Subject: [PATCH 17/19] GODT-2187: Placeholder for unbuildable messages --- internal/user/events.go | 20 ++--- internal/user/sync.go | 12 +-- internal/user/sync_build.go | 135 ++++++++++++++++++++++--------- internal/user/sync_build_test.go | 49 +++++++++++ internal/user/types.go | 13 ++- internal/user/user.go | 4 +- 6 files changed, 177 insertions(+), 56 deletions(-) create mode 100644 internal/user/sync_build_test.go diff --git a/internal/user/events.go b/internal/user/events.go index adcda965..494f4143 100644 --- a/internal/user/events.go +++ b/internal/user/events.go @@ -438,12 +438,13 @@ 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 { - update, err := buildRFC822(user.apiLabels, full, addrKR).update.unpack() - if err != nil { + buildRes := buildRFC822(user.apiLabels, full, addrKR) + + if buildRes.err != nil { return fmt.Errorf("failed to build RFC822 message: %w", err) } - user.updateCh[full.AddressID].Enqueue(imap.NewMessagesCreated(update)) + user.updateCh[full.AddressID].Enqueue(imap.NewMessagesCreated(buildRes.update)) return nil }) @@ -493,16 +494,17 @@ 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 { - update, err := buildRFC822(user.apiLabels, full, addrKR).update.unpack() - if err != nil { + buildRes := buildRFC822(user.apiLabels, full, addrKR) + + if buildRes.err != nil { return fmt.Errorf("failed to build RFC822 draft: %w", err) } user.updateCh[full.AddressID].Enqueue(imap.NewMessageUpdated( - update.Message, - update.Literal, - update.MailboxIDs, - update.ParsedMessage, + buildRes.update.Message, + buildRes.update.Literal, + buildRes.update.MailboxIDs, + buildRes.update.ParsedMessage, )) return nil diff --git a/internal/user/sync.go b/internal/user/sync.go index 173bbbf0..d64a494e 100644 --- a/internal/user/sync.go +++ b/internal/user/sync.go @@ -294,32 +294,32 @@ 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 err := res.update.err(); err != nil { + 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 sync message", reporter.Context{ "messageID": res.messageID, - "error": err, + "error": res.err, }); err != nil { logrus.WithError(err).Error("Failed to report message sync 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.unwrap()) } + + flushers[res.addressID].push(res.update) } for _, flusher := range flushers { diff --git a/internal/user/sync_build.go b/internal/user/sync_build.go index b5cfbd58..6e14f36a 100644 --- a/internal/user/sync_build.go +++ b/internal/user/sync_build.go @@ -18,47 +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 result[T any] struct { - v T - e error -} - -func resOk[T any](v T) result[T] { - return result[T]{v: v} -} - -func resErr[T any](e error) result[T] { - return result[T]{e: e} -} - -func (r *result[T]) unwrap() T { - if r.e != nil { - panic(r.err) - } - - return r.v -} - -func (r *result[T]) unpack() (T, error) { - return r.v, r.e -} - -func (r *result[T]) err() error { - return r.e -} - type buildRes struct { messageID string addressID string - update result[*imap.MessageCreated] + update *imap.MessageCreated + err error } func defaultJobOpts() message.JobOptions { @@ -73,20 +48,26 @@ func defaultJobOpts() message.JobOptions { } func buildRFC822(apiLabels map[string]proton.Label, full proton.FullMessage, addrKR *crypto.KeyRing) *buildRes { - var update result[*imap.MessageCreated] + var ( + update *imap.MessageCreated + err error + ) - if literal, err := message.BuildRFC822(addrKR, full.Message, full.AttData, defaultJobOpts()); err != nil { - update = resErr[*imap.MessageCreated](fmt.Errorf("failed to build RFC822 for message %s: %w", full.ID, err)) - } else if created, err := newMessageCreatedUpdate(apiLabels, full.MessageMetadata, literal); err != nil { - update = resErr[*imap.MessageCreated](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 = resOk(created) + update = created } return &buildRes{ messageID: full.ID, addressID: full.AddressID, update: update, + err: err, } } @@ -107,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.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..fdbf7ddf --- /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 22:33 CET", 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 22:33 CET" "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/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 ccee8d65..42755c66 100644 --- a/internal/user/user.go +++ b/internal/user/user.go @@ -355,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. @@ -431,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) } From 75c88eaa5598deee0a95e528ae3327b87c952b50 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Tue, 6 Dec 2022 16:52:02 +0100 Subject: [PATCH 18/19] GODT-2187: Handle unbuildable messages in event loop --- internal/user/events.go | 57 ++++++++++++++++++++++++++------ internal/user/sync.go | 4 +-- internal/user/sync_build.go | 2 +- internal/user/sync_build_test.go | 4 +-- 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/internal/user/events.go b/internal/user/events.go index 494f4143..f661e60d 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" @@ -438,13 +439,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 := buildRFC822(user.apiLabels, full, addrKR) + res := buildRFC822(user.apiLabels, full, addrKR) - if buildRes.err != nil { - return fmt.Errorf("failed to build RFC822 message: %w", err) + 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 }) @@ -494,17 +512,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 := buildRFC822(user.apiLabels, full, addrKR) + res := buildRFC822(user.apiLabels, full, addrKR) - if buildRes.err != nil { - return fmt.Errorf("failed to build RFC822 draft: %w", err) + 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/sync.go b/internal/user/sync.go index d64a494e..49e8daf9 100644 --- a/internal/user/sync.go +++ b/internal/user/sync.go @@ -304,11 +304,11 @@ func syncMessages( logrus.WithError(err).Error("Failed to add failed message ID") } - if err := sentry.ReportMessageWithContext("Failed to sync message", reporter.Context{ + 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 sync error") + logrus.WithError(err).Error("Failed to report message build error") } // We could sync a placeholder message here, but for now we skip it entirely. diff --git a/internal/user/sync_build.go b/internal/user/sync_build.go index 6e14f36a..ef5032a7 100644 --- a/internal/user/sync_build.go +++ b/internal/user/sync_build.go @@ -120,7 +120,7 @@ func newFailedMessageLiteral( if tmpl, err := template.New("header").Parse(failedMessageHeaderTemplate); err != nil { panic(err) } else if b, err := tmplExec(tmpl, map[string]any{ - "Date": date.Format(time.RFC822), + "Date": date.In(time.UTC).Format(time.RFC822), }); err != nil { panic(err) } else if _, err := buf.Write(b); err != nil { diff --git a/internal/user/sync_build_test.go b/internal/user/sync_build_test.go index fdbf7ddf..d3406086 100644 --- a/internal/user/sync_build_test.go +++ b/internal/user/sync_build_test.go @@ -33,7 +33,7 @@ func TestNewFailedMessageLiteral(t *testing.T) { 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 22:33 CET", header.Get("Date")) + 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")) @@ -43,7 +43,7 @@ func TestNewFailedMessageLiteral(t *testing.T) { parsed, err := imap.NewParsedMessage(literal) require.NoError(t, err) - require.Equal(t, `("29 Nov 73 22:33 CET" "Message failed to build" NIL NIL NIL NIL NIL NIL NIL NIL)`, parsed.Envelope) + 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) } From f070314524aac277150cb0e8e96a0747e2c82126 Mon Sep 17 00:00:00 2001 From: Romain LE JEUNE Date: Wed, 7 Dec 2022 07:38:05 +0100 Subject: [PATCH 19/19] Other: Bridge Perth Narrows v3.0.6 --- Changelog.md | 5 +++++ Makefile | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 7cc17e8f..becde52a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,11 @@ 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 diff --git a/Makefile b/Makefile index f5bfc9da..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.5+git +BRIDGE_APP_VERSION?=3.0.6+git APP_VERSION:=${BRIDGE_APP_VERSION} APP_FULL_NAME:=Proton Mail Bridge APP_VENDOR:=Proton AG