diff --git a/internal/imap/mailbox_message.go b/internal/imap/mailbox_message.go index 2899093a..42d6190b 100644 --- a/internal/imap/mailbox_message.go +++ b/internal/imap/mailbox_message.go @@ -133,9 +133,6 @@ func (im *imapMailbox) CreateMessage(flags []string, date time.Time, body imap.L // We didn't find the message in the store, so we are currently sending it. logEntry.WithField("time", date).Info("No matching UID, continuing APPEND to Sent") } - - // This is an APPEND to the Sent folder, so we will set the sent flag - m.Flags |= pmapi.FlagSent } message.ParseFlags(m, flags) diff --git a/internal/transfer/provider_pmapi_target.go b/internal/transfer/provider_pmapi_target.go index 36149e75..46cf661c 100644 --- a/internal/transfer/provider_pmapi_target.go +++ b/internal/transfer/provider_pmapi_target.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "io/ioutil" + "net/mail" "sync" pkgMsg "github.com/ProtonMail/proton-bridge/pkg/message" @@ -238,7 +239,7 @@ func (p *PMAPIProvider) generateImportMsgReq(rules transferRules, progress *Prog Body: body, Unread: unread, Time: message.Time, - Flags: computeMessageFlags(labelIDs), + Flags: computeMessageFlags(message.Header), LabelIDs: labelIDs, }, nil } @@ -257,24 +258,11 @@ func (p *PMAPIProvider) encryptMessage(msg *pmapi.Message, attachmentReaders []i return pkgMsg.BuildEncrypted(msg, attachmentReaders, p.keyRing) } -func computeMessageFlags(labels []string) (flag int64) { - for _, labelID := range labels { - switch labelID { - case pmapi.SentLabel: - flag = (flag | pmapi.FlagSent) - case pmapi.ArchiveLabel, pmapi.InboxLabel: - flag = (flag | pmapi.FlagReceived) - case pmapi.DraftLabel: - log.Error("Found draft target in non-draft import") - } +func computeMessageFlags(header mail.Header) (flag int64) { + if header.Get("received") == "" { + return pmapi.FlagSent } - - // NOTE: if the labels are custom only - if flag == 0 { - flag = pmapi.FlagReceived - } - - return flag + return pmapi.FlagReceived } func (p *PMAPIProvider) startImportWorkers(progress *Progress, preparedImportRequestsCh chan map[string]*pmapi.ImportMsgReq) *sync.WaitGroup { diff --git a/pkg/message/flags.go b/pkg/message/flags.go index 5902f711..258a5d2e 100644 --- a/pkg/message/flags.go +++ b/pkg/message/flags.go @@ -60,10 +60,12 @@ func GetFlags(m *pmapi.Message) (flags []string) { } func ParseFlags(m *pmapi.Message, flags []string) { - // Consider to use ComputeMessageFlagsByLabels to keep logic in one place. - if (m.Flags & pmapi.FlagSent) == 0 { - m.Flags |= pmapi.FlagReceived + if m.Header.Get("received") == "" { + m.Flags = pmapi.FlagSent + } else { + m.Flags = pmapi.FlagReceived } + m.Unread = 1 for _, f := range flags { switch f { diff --git a/test/api_checks_test.go b/test/api_checks_test.go index 40b6dbb5..d62280b0 100644 --- a/test/api_checks_test.go +++ b/test/api_checks_test.go @@ -24,6 +24,8 @@ import ( "strings" "time" + "github.com/ProtonMail/proton-bridge/pkg/pmapi" + "github.com/ProtonMail/proton-bridge/test/accounts" "github.com/cucumber/godog" "github.com/cucumber/godog/gherkin" "github.com/stretchr/testify/assert" @@ -32,6 +34,8 @@ import ( func APIChecksFeatureContext(s *godog.Suite) { s.Step(`^API endpoint "([^"]*)" is called with:$`, apiIsCalledWith) s.Step(`^message is sent with API call$`, messageIsSentWithAPICall) + s.Step(`^API mailbox "([^"]*)" for "([^"]*)" has (\d+) message(?:s)?$`, apiMailboxForUserHasNumberOfMessages) + s.Step(`^API mailbox "([^"]*)" for address "([^"]*)" of "([^"]*)" has (\d+) message(?:s)?$`, apiMailboxForAddressOfUserHasNumberOfMessages) s.Step(`^API mailbox "([^"]*)" for "([^"]*)" has messages$`, apiMailboxForUserHasMessages) s.Step(`^API mailbox "([^"]*)" for address "([^"]*)" of "([^"]*)" has messages$`, apiMailboxForAddressOfUserHasMessages) s.Step(`^API client manager user-agent is "([^"]*)"$`, clientManagerUserAgent) @@ -84,6 +88,35 @@ func checkAllRequiredFieldsForSendingMessage(request []byte) bool { return true } +func apiMailboxForUserHasNumberOfMessages(mailboxName, bddUserID string, countOfMessages int) error { + return apiMailboxForAddressOfUserHasNumberOfMessages(mailboxName, "", bddUserID, countOfMessages) +} + +func apiMailboxForAddressOfUserHasNumberOfMessages(mailboxName, bddAddressID, bddUserID string, countOfMessages int) error { + account := ctx.GetTestAccountWithAddress(bddUserID, bddAddressID) + if account == nil { + return godog.ErrPending + } + + start := time.Now() + for { + afterLimit := time.Since(start) > ctx.EventLoopTimeout() + pmapiMessages, err := getPMAPIMessages(account, mailboxName) + if err != nil { + return err + } + total := len(pmapiMessages) + if total == countOfMessages { + break + } + if afterLimit { + return fmt.Errorf("expected %v messages, but got %v", countOfMessages, total) + } + time.Sleep(100 * time.Millisecond) + } + return nil +} + func apiMailboxForUserHasMessages(mailboxName, bddUserID string, messages *gherkin.DataTable) error { return apiMailboxForAddressOfUserHasMessages(mailboxName, "", bddUserID, messages) } @@ -94,13 +127,7 @@ func apiMailboxForAddressOfUserHasMessages(mailboxName, bddAddressID, bddUserID return godog.ErrPending } - labelIDs, err := ctx.GetPMAPIController().GetLabelIDs(account.Username(), []string{mailboxName}) - if err != nil { - return internalError(err, "getting label %s for %s", mailboxName, account.Username()) - } - labelID := labelIDs[0] - - pmapiMessages, err := ctx.GetPMAPIController().GetMessages(account.Username(), labelID) + pmapiMessages, err := getPMAPIMessages(account, mailboxName) if err != nil { return err } @@ -122,6 +149,16 @@ func apiMailboxForAddressOfUserHasMessages(mailboxName, bddAddressID, bddUserID return nil } +func getPMAPIMessages(account *accounts.TestAccount, mailboxName string) ([]*pmapi.Message, error) { + labelIDs, err := ctx.GetPMAPIController().GetLabelIDs(account.Username(), []string{mailboxName}) + if err != nil { + return nil, internalError(err, "getting label %s for %s", mailboxName, account.Username()) + } + labelID := labelIDs[0] + + return ctx.GetPMAPIController().GetMessages(account.Username(), labelID) +} + func clientManagerUserAgent(expectedUserAgent string) error { expectedUserAgent = strings.ReplaceAll(expectedUserAgent, "[GOOS]", runtime.GOOS) diff --git a/test/fakeapi/messages.go b/test/fakeapi/messages.go index 213eb552..5771e3a5 100644 --- a/test/fakeapi/messages.go +++ b/test/fakeapi/messages.go @@ -230,7 +230,7 @@ func (api *FakePMAPI) generateMessageFromImportRequest(msgReq *pmapi.ImportMsgRe ToList: m.ToList, Subject: m.Subject, Unread: msgReq.Unread, - LabelIDs: append(msgReq.LabelIDs, pmapi.AllMailLabel), + LabelIDs: api.generateLabelIDsFromImportRequest(msgReq), Body: m.Body, Header: m.Header, Flags: msgReq.Flags, @@ -238,6 +238,27 @@ func (api *FakePMAPI) generateMessageFromImportRequest(msgReq *pmapi.ImportMsgRe }, nil } +// generateLabelIDsFromImportRequest simulates API where Sent and INBOX is the same +// mailbox but the message is shown in one or other based on the flags instead. +func (api *FakePMAPI) generateLabelIDsFromImportRequest(msgReq *pmapi.ImportMsgReq) []string { + isInSentOrInbox := false + labelIDs := []string{pmapi.AllMailLabel} + for _, labelID := range msgReq.LabelIDs { + if labelID == pmapi.InboxLabel || labelID == pmapi.SentLabel { + isInSentOrInbox = true + } else { + labelIDs = append(labelIDs, labelID) + } + } + if isInSentOrInbox && (msgReq.Flags&pmapi.FlagSent) != 0 { + labelIDs = append(labelIDs, pmapi.SentLabel) + } + if isInSentOrInbox && (msgReq.Flags&pmapi.FlagReceived) != 0 { + labelIDs = append(labelIDs, pmapi.InboxLabel) + } + return labelIDs +} + func (api *FakePMAPI) findMessage(newMsg *pmapi.Message) *pmapi.Message { if newMsg.ExternalID == "" { return nil diff --git a/test/features/bridge/imap/message/import.feature b/test/features/bridge/imap/message/import.feature index 4482963e..9a210481 100644 --- a/test/features/bridge/imap/message/import.feature +++ b/test/features/bridge/imap/message/import.feature @@ -84,3 +84,31 @@ Feature: IMAP import messages """ Then IMAP response is "OK" + Scenario: Import received message to Sent + When IMAP client imports 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 IMAP response is "OK" + And API mailbox "Sent" for "user" has 0 message + And API mailbox "INBOX" for "user" has 1 message + + Scenario: Import non-received message to Inbox + When IMAP client imports message to "INBOX" + """ + From: Foo + To: Bridge Test + Subject: Hello + + Hello + + """ + Then IMAP response is "OK" + And API mailbox "INBOX" for "user" has 0 message + And API mailbox "Sent" for "user" has 1 message diff --git a/test/features/ie/transfer/import_eml.feature b/test/features/ie/transfer/import_eml.feature index 64f69869..87cbe313 100644 --- a/test/features/ie/transfer/import_eml.feature +++ b/test/features/ie/transfer/import_eml.feature @@ -13,6 +13,7 @@ Feature: Import from EML files Subject: hello 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 hello diff --git a/test/features/ie/transfer/import_encrypted.feature b/test/features/ie/transfer/import_encrypted.feature index 8969ca0f..21d66595 100644 --- a/test/features/ie/transfer/import_encrypted.feature +++ b/test/features/ie/transfer/import_encrypted.feature @@ -6,17 +6,19 @@ Feature: Import from EML files Subject: clear 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 - secret + secret """ And there is EML file "Inbox/encrypted.eml" """ Subject: encrypted 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 -----BEGIN PGP MESSAGE----- - + hQEMA7hGUUsYs0fEAQgA10NwJSNTLm3vpxVtoYBaA9AjFI5H4hKuV3/f2NHbWb2s k5enK3tEIOYdFdrO+NLrV6weHq3Dgu4er3URTQ62tFwjSJyeXxmY0d9J+JdxibJg wqZubuSHYzQHpFqJgoUUWEVEsv0Ao8yQo8BE9iybCKoZf6KojROUuE748oxlxJnV @@ -33,6 +35,7 @@ Feature: Import from EML files Subject: encrypted mime 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 MIME-Version: 1.0 Content-Type: multipart/encrypted; protocol="application/pgp-encrypted"; boundary="WLjzd46aUAiOcuNXjWTJItBZonI56MuAk" @@ -110,13 +113,14 @@ Feature: Import from EML files Subject: signed 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 -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 - + secret -----BEGIN PGP SIGNATURE----- - + iQEzBAEBCAAdFiEENaE2ZPemenI4pZah/SJcGo7SJWIFAl+cCAgACgkQ/SJcGo7S JWKsOQf/YakNXkMNjZIu8Hf1WflxtiDXVzTugOicC05k5W64oIqSHt0xNaFKE37k //3eDMWbHvqHKFVdg7qcLsVPeVBaW3bdZUiexGM24OiGgyEitufnHQLOtEDTound @@ -132,9 +136,10 @@ Feature: Import from EML files Subject: encrypted and signed 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 -----BEGIN PGP MESSAGE----- - + hQEMA7hGUUsYs0fEAQf/dppHciWIf+o4l0gEfHeyHV/HVhG4es0aVQYrwFQlSWVx estMuyLBSMfrsQXLago7Q9ZNo/XnKszzprCXxxYH52hAg64oAsjKB3jgRmVizs8b 8lj0BRf003wUluS/0msV9SiEZBGeL8jGq6Te9vaM8OHHhIVzVjGnRdTSC0jBE6cS @@ -154,13 +159,13 @@ Feature: Import from EML files """ Scenario: Import encrypted - Given there is skip encrypted messages set to "false" + Given there is skip encrypted messages set to "false" When user "user" imports local files Then progress result is "OK" And transfer failed for 0 messages And transfer exported 5 messages And transfer imported 5 messages - And transfer skipped 0 messages + And transfer skipped 0 messages And API mailbox "INBOX" for "user" has messages | from | to | subject | | bridgetest@pm.test | test@protonmail.com | clear | @@ -170,13 +175,13 @@ Feature: Import from EML files | bridgetest@pm.test | test@protonmail.com | encrypted and signed | Scenario: Skip encrypted - Given there is skip encrypted messages set to "true" + Given there is skip encrypted messages set to "true" When user "user" imports local files Then progress result is "OK" And transfer failed for 0 messages And transfer exported 5 messages And transfer imported 2 messages - And transfer skipped 3 messages + And transfer skipped 3 messages And API mailbox "INBOX" for "user" has messages | from | to | subject | | bridgetest@pm.test | test@protonmail.com | clear | diff --git a/test/features/ie/transfer/import_imap.feature b/test/features/ie/transfer/import_imap.feature index f3766348..47e626d7 100644 --- a/test/features/ie/transfer/import_imap.feature +++ b/test/features/ie/transfer/import_imap.feature @@ -18,6 +18,7 @@ Feature: Import from IMAP server Subject: hello 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 hello diff --git a/test/features/ie/transfer/import_mbox.feature b/test/features/ie/transfer/import_mbox.feature index 1235a324..4ad66dcb 100644 --- a/test/features/ie/transfer/import_mbox.feature +++ b/test/features/ie/transfer/import_mbox.feature @@ -16,6 +16,7 @@ Feature: Import from MBOX files Subject: hello 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 hello diff --git a/test/features/ie/transfer/import_sent.feature b/test/features/ie/transfer/import_sent.feature new file mode 100644 index 00000000..0762457a --- /dev/null +++ b/test/features/ie/transfer/import_sent.feature @@ -0,0 +1,88 @@ +Feature: Import to sent + Background: + Given there is connected user "user" + And there is "user" with mailbox "Labels/label" + And there is EML file "Sent/one.eml" + """ + Subject: one + From: Foo + To: Bridge Test + Message-ID: one.integrationtest + + one + + """ + And there is EML file "Sent/two.eml" + """ + Subject: two + From: Bar + To: Bridge Test + Message-ID: two.integrationtest + + two + + """ + + Scenario: Import sent only + When user "user" imports local files + Then progress result is "OK" + And transfer exported 2 messages + And transfer imported 2 messages + And transfer failed for 0 messages + And API mailbox "INBOX" for "user" has 0 message + And API mailbox "Sent" for "user" has messages + | from | to | subject | + | foo@example.com | bridgetest@pm.test | one | + | bar@example.com | bridgetest@pm.test | two | + + Scenario: Import to sent and custom label + And there is EML file "Label/one.eml" + """ + Subject: one + From: Foo + To: Bridge Test + Message-ID: one.integrationtest + + one + + """ + When user "user" imports local files + Then progress result is "OK" + And transfer exported 3 messages + And transfer imported 3 messages + And transfer failed for 0 messages + # We had an issue that moving message to Sent automatically added + # the message also into Inbox if the message was in some custom label. + And API mailbox "INBOX" for "user" has 0 message + And API mailbox "Labels/label" for "user" has messages + | from | to | subject | + | foo@example.com | bridgetest@pm.test | one | + And API mailbox "Sent" for "user" has messages + | from | to | subject | + | foo@example.com | bridgetest@pm.test | one | + | bar@example.com | bridgetest@pm.test | two | + + Scenario: Import to sent and inbox is in both mailboxes + And there is EML file "Inbox/one.eml" + """ + Subject: one + From: Foo + To: Bridge Test + Message-ID: one.integrationtest + Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000 + + one + + """ + When user "user" imports local files + Then progress result is "OK" + And transfer exported 3 messages + And transfer imported 3 messages + And transfer failed for 0 messages + And API mailbox "INBOX" for "user" has messages + | from | to | subject | + | foo@example.com | bridgetest@pm.test | one | + And API mailbox "Sent" for "user" has messages + | from | to | subject | + | foo@example.com | bridgetest@pm.test | one | + | bar@example.com | bridgetest@pm.test | two | diff --git a/test/mocks/imap_client.go b/test/mocks/imap_client.go index 9cabeb12..1b005169 100644 --- a/test/mocks/imap_client.go +++ b/test/mocks/imap_client.go @@ -173,6 +173,9 @@ func (c *IMAPClient) AppendBody(mailboxName, subject, from, to, body string) *IM msg := fmt.Sprintf("Subject: %s\r\n", subject) msg += fmt.Sprintf("From: %s\r\n", from) msg += fmt.Sprintf("To: %s\r\n", to) + if mailboxName != "Sent" { + msg += "Received: by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000\r\n" + } msg += "\r\n" msg += body msg += "\r\n" diff --git a/test/transfer_setup_test.go b/test/transfer_setup_test.go index 10846acc..38fb4f3c 100644 --- a/test/transfer_setup_test.go +++ b/test/transfer_setup_test.go @@ -190,6 +190,7 @@ func thereIsIMAPMessage(mailboxName string, seqNum, uid int, dateValue, subject func getBodyFromDataRow(head []*gherkin.TableCell, row *gherkin.TableRow) string { body := "hello" headers := textproto.MIMEHeader{} + headers.Set("Received", "by 2002:0:0:0:0:0:0:0 with SMTP id 0123456789abcdef; Wed, 30 Dec 2020 01:23:45 0000") for n, cell := range row.Cells { switch head[n].Value { case "from": diff --git a/unreleased.md b/unreleased.md index 81ac9feb..71104689 100644 --- a/unreleased.md +++ b/unreleased.md @@ -11,3 +11,7 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) ### Changed ### Fixed +* GODT-900 Remove \Deleted flag after re-importing the message (do not delete messages by moving to local folder and back). +* GODT-908 Do not unpause event loop if other mailbox is still fetching. +* Check deprecated status code first to better determine API error. +* GODT-787 GODT-978 Fix IE and Bridge importing to sent not showing up in inbox (setting up flags properly).