From 04b6571cb8bd78fcfdc4509db2530e3b26b03f9c Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 19 Oct 2022 08:00:45 +0200 Subject: [PATCH] Other: Handle Seen/Flagged IMAP flags when APPENDing a message When an IMAP client appends a message to a mailbox, it can specify which flags it wants the appended message to have. We need to handle these in a proton-specific way; not-seen messages need to be imported with the Unread bool set to true, and flagged messages need to additionally be imported with the Starred label. --- go.mod | 2 +- go.sum | 4 +- internal/user/imap.go | 86 +++++++---- pkg/message/build.go | 2 +- pkg/mime/encoding.go | 9 ++ tests/api_test.go | 4 - tests/bdd_test.go | 2 + tests/features/imap/message/import.feature | 147 +++++++++++++++++++ tests/imap_test.go | 24 +++ tests/testdata/text_plain_latin1.eml | 6 + tests/testdata/text_plain_unknown_latin1.eml | 6 + tests/testdata/text_plain_wrong_latin1.eml | 6 + 12 files changed, 259 insertions(+), 39 deletions(-) create mode 100644 tests/features/imap/message/import.feature create mode 100644 tests/testdata/text_plain_latin1.eml create mode 100644 tests/testdata/text_plain_unknown_latin1.eml create mode 100644 tests/testdata/text_plain_wrong_latin1.eml diff --git a/go.mod b/go.mod index 8af371dc..237beffa 100644 --- a/go.mod +++ b/go.mod @@ -39,7 +39,7 @@ require ( github.com/sirupsen/logrus v1.9.0 github.com/stretchr/testify v1.8.0 github.com/urfave/cli/v2 v2.16.3 - gitlab.protontech.ch/go/liteapi v0.34.0 + gitlab.protontech.ch/go/liteapi v0.34.4-0.20221019124718-218de25bbfd6 golang.org/x/exp v0.0.0-20220921164117-439092de6870 golang.org/x/net v0.1.0 golang.org/x/sys v0.1.0 diff --git a/go.sum b/go.sum index 6bab74e0..0c27d6f0 100644 --- a/go.sum +++ b/go.sum @@ -401,8 +401,8 @@ github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsr github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/zclconf/go-cty v1.11.0 h1:726SxLdi2SDnjY+BStqB9J1hNp4+2WlzyXLuimibIe0= github.com/zclconf/go-cty v1.11.0/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA= -gitlab.protontech.ch/go/liteapi v0.34.0 h1:IPj08EgqcCq+LrKGD/d5INDrPntd1ULD3kB59pSko/Q= -gitlab.protontech.ch/go/liteapi v0.34.0/go.mod h1:VCEA83UCi9f3XCP9W/XUIFnJKwokGB46lKUHBNzPWsQ= +gitlab.protontech.ch/go/liteapi v0.34.4-0.20221019124718-218de25bbfd6 h1:D6tMRUg6C1VtnDVjqJkm10QHZJG5TLOPAslegS46nVg= +gitlab.protontech.ch/go/liteapi v0.34.4-0.20221019124718-218de25bbfd6/go.mod h1:VCEA83UCi9f3XCP9W/XUIFnJKwokGB46lKUHBNzPWsQ= go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8= diff --git a/internal/user/imap.go b/internal/user/imap.go index 93e0989b..6da12e93 100644 --- a/internal/user/imap.go +++ b/internal/user/imap.go @@ -24,11 +24,12 @@ import ( "github.com/ProtonMail/gluon/imap" "github.com/ProtonMail/gluon/queue" + "github.com/ProtonMail/gluon/rfc822" "github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/ProtonMail/proton-bridge/v2/internal/safe" "github.com/ProtonMail/proton-bridge/v2/internal/vault" - "github.com/google/uuid" - "github.com/sirupsen/logrus" + "github.com/ProtonMail/proton-bridge/v2/pkg/message" + "github.com/bradenaw/juniper/stream" "gitlab.protontech.ch/go/liteapi" "golang.org/x/exp/slices" ) @@ -214,67 +215,90 @@ func (conn *imapConnector) GetMessage(ctx context.Context, messageID imap.Messag // CreateMessage creates a new message on the remote. func (conn *imapConnector) CreateMessage( ctx context.Context, - labelID imap.MailboxID, + mailboxID imap.MailboxID, literal []byte, flags imap.FlagSet, date time.Time, -) (imap.Message, []byte, error) { +) (imap.Message, []byte, error) { // nolint:funlen var msgFlags liteapi.MessageFlag - switch labelID { - case liteapi.SentLabel: - msgFlags |= liteapi.MessageFlagSent + if mailboxID != liteapi.DraftsLabel { + header, _ := rfc822.Split(literal) - default: - msgFlags |= liteapi.MessageFlagReceived + parsed, err := rfc822.NewHeader(header) + if err != nil { + return imap.Message{}, nil, err + } + + if parsed.Has("Received") { + msgFlags |= liteapi.MessageFlagReceived + } else { + msgFlags |= liteapi.MessageFlagSent + } } - var importResult liteapi.ImportRes - if err := conn.withAddrKR(conn.addrID, func(ring *crypto.KeyRing) error { - requestName := uuid.NewString() + var labelIDs []imap.MailboxID - importReq := []liteapi.ImportReq{{ - Name: requestName, + if flags.Contains(imap.FlagFlagged) { + labelIDs = append(labelIDs, liteapi.StarredLabel) + } + + if flags.Contains(imap.FlagAnswered) { + msgFlags |= liteapi.MessageFlagReplied + } + + var ( + messageID string + imported []byte + ) + + if err := conn.withAddrKR(conn.addrID, func(addrKR *crypto.KeyRing) error { + res, err := stream.Collect(ctx, conn.client.ImportMessages(ctx, addrKR, 1, 1, []liteapi.ImportReq{{ Metadata: liteapi.ImportMetadata{ AddressID: conn.addrID, - LabelIDs: []string{string(labelID)}, + LabelIDs: mapTo[imap.MailboxID, string](append(labelIDs, mailboxID)), + Unread: !liteapi.Bool(flags.Contains(imap.FlagSeen)), Flags: msgFlags, }, Message: literal, - }} - - r, err := conn.client.ImportMessages(ctx, ring, importReq) + }}...)) if err != nil { - return err + return fmt.Errorf("failed to import message: %w", err) } - importResult = r[requestName] + full, err := conn.client.GetFullMessage(ctx, res[0].MessageID) + if err != nil { + return fmt.Errorf("failed to fetch imported message: %w", err) + } + + b, err := message.BuildRFC822(addrKR, full.Message, full.AttData, defaultJobOpts()) + if err != nil { + return fmt.Errorf("failed to build message: %w", err) + } + + messageID = full.ID + imported = b return nil }); err != nil { return imap.Message{}, nil, err } - if importResult.Code != liteapi.SuccessCode { - logrus.Errorf("Failed to import message: %v", importResult.Message) - return imap.Message{}, nil, fmt.Errorf("failed to create message: %08x", importResult.Code) - } - return imap.Message{ - ID: imap.MessageID(importResult.MessageID), + ID: imap.MessageID(messageID), Flags: flags, Date: date, - }, literal, nil + }, imported, nil } // AddMessagesToMailbox labels the given messages with the given label ID. -func (conn *imapConnector) AddMessagesToMailbox(ctx context.Context, messageIDs []imap.MessageID, labelID imap.MailboxID) error { - return conn.client.LabelMessages(ctx, mapTo[imap.MessageID, string](messageIDs), string(labelID)) +func (conn *imapConnector) AddMessagesToMailbox(ctx context.Context, messageIDs []imap.MessageID, mailboxID imap.MailboxID) error { + return conn.client.LabelMessages(ctx, mapTo[imap.MessageID, string](messageIDs), string(mailboxID)) } // RemoveMessagesFromMailbox unlabels the given messages with the given label ID. -func (conn *imapConnector) RemoveMessagesFromMailbox(ctx context.Context, messageIDs []imap.MessageID, labelID imap.MailboxID) error { - return conn.client.UnlabelMessages(ctx, mapTo[imap.MessageID, string](messageIDs), string(labelID)) +func (conn *imapConnector) RemoveMessagesFromMailbox(ctx context.Context, messageIDs []imap.MessageID, mailboxID imap.MailboxID) error { + return conn.client.UnlabelMessages(ctx, mapTo[imap.MessageID, string](messageIDs), string(mailboxID)) } // MoveMessages removes the given messages from one label and adds them to the other label. diff --git a/pkg/message/build.go b/pkg/message/build.go index 615b7fbe..fecede2b 100644 --- a/pkg/message/build.go +++ b/pkg/message/build.go @@ -234,7 +234,7 @@ func buildPGPRFC822(kr *crypto.KeyRing, msg liteapi.Message, opts JobOptions) ([ hdr := getMessageHeader(msg, opts) - sigs, err := msg.ExtractSignatures(kr) + sigs, err := liteapi.ExtractSignatures(kr, msg.Body) if err != nil { log.WithError(err).WithField("id", msg.ID).Warn("Extract signature failed") } diff --git a/pkg/mime/encoding.go b/pkg/mime/encoding.go index 2a9a2509..3398d0c9 100644 --- a/pkg/mime/encoding.go +++ b/pkg/mime/encoding.go @@ -25,13 +25,22 @@ import ( "strings" "unicode/utf8" + "github.com/ProtonMail/gluon/rfc822" + "github.com/emersion/go-message" "github.com/pkg/errors" "github.com/sirupsen/logrus" + "gitlab.protontech.ch/go/liteapi" "golang.org/x/net/html/charset" "golang.org/x/text/encoding" "golang.org/x/text/encoding/htmlindex" ) +func init() { + rfc822.ParseMediaType = ParseMediaType + liteapi.CharsetReader = CharsetReader + message.CharsetReader = CharsetReader +} + func CharsetReader(charset string, input io.Reader) (io.Reader, error) { dec, err := SelectDecoder(charset) if err != nil { diff --git a/tests/api_test.go b/tests/api_test.go index 853caba7..573c20bf 100644 --- a/tests/api_test.go +++ b/tests/api_test.go @@ -37,10 +37,6 @@ type API interface { GetLabels(userID string) ([]liteapi.Label, error) CreateLabel(userID, name string, labelType liteapi.LabelType) (string, error) - CreateMessage(userID, addrID string, literal []byte, flags liteapi.MessageFlag, unread, starred bool) (string, error) - LabelMessage(userID, messageID, labelID string) error - UnlabelMessage(userID, messageID, labelID string) error - Close() } diff --git a/tests/bdd_test.go b/tests/bdd_test.go index 028cf2d4..a4a7900c 100644 --- a/tests/bdd_test.go +++ b/tests/bdd_test.go @@ -170,6 +170,8 @@ func TestFeatures(testingT *testing.T) { ctx.Step(`^IMAP client "([^"]*)" marks all messages as deleted$`, s.imapClientMarksAllMessagesAsDeleted) ctx.Step(`^IMAP client "([^"]*)" sees that message (\d+) has the flag "([^"]*)"$`, s.imapClientSeesThatMessageHasTheFlag) ctx.Step(`^IMAP client "([^"]*)" expunges$`, s.imapClientExpunges) + ctx.Step(`^IMAP client "([^"]*)" appends the following message to "([^"]*)":$`, s.imapClientAppendsTheFollowingMessageToMailbox) + ctx.Step(`^IMAP client "([^"]*)" appends "([^"]*)" to "([^"]*)"$`, s.imapClientAppendsToMailbox) // ==== SMTP ==== ctx.Step(`^user "([^"]*)" connects SMTP client "([^"]*)"$`, s.userConnectsSMTPClient) diff --git a/tests/features/imap/message/import.feature b/tests/features/imap/message/import.feature new file mode 100644 index 00000000..5f34711a --- /dev/null +++ b/tests/features/imap/message/import.feature @@ -0,0 +1,147 @@ +Feature: IMAP import messages + Background: + Given there exists an account with username "user@pm.me" and password "password" + And bridge starts + 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" + + Scenario: Basic message import + When IMAP client "1" appends the following message to "INBOX": + """ + From: Bridge Test + To: Internal Bridge + Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000 + Subject: Basic text/plain message + Content-Type: text/plain + + Hello + """ + Then it succeeds + And IMAP client "1" eventually sees the following messages in "INBOX": + | from | to | subject | body | + | bridgetest@pm.test | bridgetest@protonmail.com | Basic text/plain message | Hello | + + Scenario: Import message with double charset in content type + When IMAP client "1" appends the following message to "INBOX": + """ + From: Bridge Test + To: Internal Bridge + Subject: Message with double charset in content type + Content-Type: text/plain; charset=utf-8; charset=utf-8 + Content-Disposition: inline + Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000 + + Hello + """ + Then it succeeds + And IMAP client "1" eventually sees the following messages in "INBOX": + | from | to | subject | body | + | bridgetest@pm.test | bridgetest@protonmail.com | Message with double charset in content type | Hello | + + # The message is imported as UTF-8 and the content type is determined at build time. + Scenario: Import message as latin1 without content type + When IMAP client "1" appends "text_plain_unknown_latin1.eml" to "INBOX" + Then it succeeds + And IMAP client "1" eventually sees the following messages in "INBOX": + | from | to | body | + | sender@pm.me | receiver@pm.me | ééééééé | + + # The message is imported and the body is converted to UTF-8. + Scenario: Import message as latin1 with content type + When IMAP client "1" appends "text_plain_latin1.eml" to "INBOX" + Then it succeeds + And IMAP client "1" eventually sees the following messages in "INBOX": + | from | to | body | + | sender@pm.me | receiver@pm.me | ééééééé | + + # The message is imported anad the body is wrongly converted (body is corrupted). + Scenario: Import message as latin1 with wrong content type + When IMAP client "1" appends "text_plain_wrong_latin1.eml" to "INBOX" + Then it succeeds + And IMAP client "1" eventually sees the following messages in "INBOX": + | from | to | + | sender@pm.me | receiver@pm.me | + + Scenario: Import received message to Sent + When IMAP client "1" appends the following message to "Sent": + """ + From: Foo + To: Bridge Test + Subject: Hello + Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000 + + Hello + """ + Then it succeeds + And IMAP client "1" eventually sees the following messages in "INBOX": + | from | to | subject | body | + | foo@example.com | bridgetest@pm.test | Hello | Hello | + And IMAP client "1" sees 0 messages in "Sent" + + Scenario: Import non-received message to Inbox + When IMAP client "1" appends the following message to "Inbox": + """ + From: Foo + To: Bridge Test + Subject: Hello + + Hello + """ + Then it succeeds + And IMAP client "1" eventually sees the following messages in "Sent": + | from | to | subject | body | + | foo@example.com | bridgetest@pm.test | Hello | Hello | + And IMAP client "1" sees 0 messages in "Inbox" + + Scenario Outline: Import message without sender + When IMAP client "1" appends the following message to "": + """ + To: Lionel Richie + Subject: RE: Hello, is it me you looking for? + + Nope. + """ + Then it succeeds + And IMAP client "1" eventually sees the following messages in "": + | to | subject | body | + | lionel@richie.com | RE: Hello, is it me you looking for? | Nope. | + + Examples: + | mailbox | + | Drafts | + | Archive | + | Sent | + + Scenario: Import embedded message + When IMAP client "1" appends the following message to "INBOX": + """ + From: Foo + To: Bridge Test + Subject: Embedded message + Content-Type: multipart/mixed; boundary="boundary" + Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000 + + This is a multi-part message in MIME format. + --boundary + Content-Type: text/plain; charset=utf-8 + Content-Transfer-Encoding: 7bit + + + --boundary + Content-Type: message/rfc822; name="embedded.eml" + Content-Transfer-Encoding: 7bit + Content-Disposition: attachment; filename="embedded.eml" + + From: Bar + To: Bridge Test + Subject: (No Subject) + Content-Type: text/plain; charset=utf-8 + Content-Transfer-Encoding: quoted-printable + + hello + + --boundary-- + + """ + Then it succeeds \ No newline at end of file diff --git a/tests/imap_test.go b/tests/imap_test.go index 6f01fc09..685ac421 100644 --- a/tests/imap_test.go +++ b/tests/imap_test.go @@ -19,7 +19,10 @@ package tests import ( "fmt" + "os" + "path/filepath" "strings" + "time" "github.com/bradenaw/juniper/iterator" "github.com/bradenaw/juniper/xslices" @@ -380,6 +383,23 @@ func (s *scenario) imapClientExpunges(clientID string) error { return client.Expunge(nil) } +func (s *scenario) imapClientAppendsTheFollowingMessageToMailbox(clientID string, mailbox string, docString *godog.DocString) error { + _, client := s.t.getIMAPClient(clientID) + + return clientAppend(client, mailbox, docString.Content) +} + +func (s *scenario) imapClientAppendsToMailbox(clientID string, file, mailbox string) error { + _, client := s.t.getIMAPClient(clientID) + + b, err := os.ReadFile(filepath.Join("testdata", file)) + if err != nil { + return err + } + + return clientAppend(client, mailbox, string(b)) +} + func clientList(client *client.Client) []*imap.MailboxInfo { resCh := make(chan *imap.MailboxInfo) @@ -490,3 +510,7 @@ func clientStore(client *client.Client, from, to int, item imap.StoreItem, flags return iterator.Collect(iterator.Chan(resCh)), nil } + +func clientAppend(client *client.Client, mailbox string, literal string) error { + return client.Append(mailbox, []string{}, time.Now(), strings.NewReader(literal)) +} diff --git a/tests/testdata/text_plain_latin1.eml b/tests/testdata/text_plain_latin1.eml new file mode 100644 index 00000000..c8b79c90 --- /dev/null +++ b/tests/testdata/text_plain_latin1.eml @@ -0,0 +1,6 @@ +From: Sender +To: Receiver +Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000 +Content-Type: text/plain; charset=ISO-8859-1 + +ééééééé diff --git a/tests/testdata/text_plain_unknown_latin1.eml b/tests/testdata/text_plain_unknown_latin1.eml new file mode 100644 index 00000000..c31fcc3e --- /dev/null +++ b/tests/testdata/text_plain_unknown_latin1.eml @@ -0,0 +1,6 @@ +From: Sender +To: Receiver +Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000 +Content-Type: text/plain + +ééééééé diff --git a/tests/testdata/text_plain_wrong_latin1.eml b/tests/testdata/text_plain_wrong_latin1.eml new file mode 100644 index 00000000..bc77e17a --- /dev/null +++ b/tests/testdata/text_plain_wrong_latin1.eml @@ -0,0 +1,6 @@ +From: Sender +To: Receiver +Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000 +Content-Type: text/plain; charset=KOI8R + +ééééééé