From bc381407a7f7899d57a5b894bb8a6da29af38a01 Mon Sep 17 00:00:00 2001 From: Leander Beernaert Date: Mon, 13 Nov 2023 12:21:24 +0100 Subject: [PATCH] feat(GODT-2576): Forward and $Forward Flag Support When an IMAP client stores the `Forward` or `$Forward` flags on a message, the forwarded state is now correctly represented on the Proton servers. https://github.com/ProtonMail/go-proton-api/pull/125 https://github.com/ProtonMail/gluon/pull/400 --- go.mod | 4 +- go.sum | 8 +-- internal/services/imapservice/api_client.go | 2 + internal/services/imapservice/connector.go | 47 ++++++++++--- internal/services/imapservice/helpers.go | 4 ++ internal/services/imapservice/imap_updates.go | 12 ++-- internal/services/imapservice/mocks/mocks.go | 38 +++++++++++ tests/features/imap/message/store.feature | 32 +++++++++ tests/imap_test.go | 68 ++++++++++++------- tests/steps_test.go | 10 +-- 10 files changed, 176 insertions(+), 49 deletions(-) create mode 100644 tests/features/imap/message/store.feature diff --git a/go.mod b/go.mod index b94c494e..b3fd48e8 100644 --- a/go.mod +++ b/go.mod @@ -5,9 +5,9 @@ go 1.20 require ( github.com/0xAX/notificator v0.0.0-20220220101646-ee9b8921e557 github.com/Masterminds/semver/v3 v3.2.0 - github.com/ProtonMail/gluon v0.17.1-0.20231025125916-5c7941465df8 + github.com/ProtonMail/gluon v0.17.1-0.20231114153341-2ecbdd2739f7 github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a - github.com/ProtonMail/go-proton-api v0.4.1-0.20231108105501-fb55d3bd8bd8 + github.com/ProtonMail/go-proton-api v0.4.1-0.20231114153253-d2fbf42bb036 github.com/ProtonMail/gopenpgp/v2 v2.7.4-proton github.com/PuerkitoBio/goquery v1.8.1 github.com/abiosoft/ishell v2.0.0+incompatible diff --git a/go.sum b/go.sum index 5a1e0ff8..70076895 100644 --- a/go.sum +++ b/go.sum @@ -25,8 +25,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.17.1-0.20231025125916-5c7941465df8 h1:sG0o5pEoS2z2jNR9zK7Juq5Tr3X+GfHmQ8L99RPowaE= -github.com/ProtonMail/gluon v0.17.1-0.20231025125916-5c7941465df8/go.mod h1:Og5/Dz1MiGpCJn51XujZwxiLG7WzvvjE5PRpZBQmAHo= +github.com/ProtonMail/gluon v0.17.1-0.20231114153341-2ecbdd2739f7 h1:w+VoSAq9FQvKMm3DlH1MIEZ1KGe7LJ+81EJFVwSV4VU= +github.com/ProtonMail/gluon v0.17.1-0.20231114153341-2ecbdd2739f7/go.mod h1:Og5/Dz1MiGpCJn51XujZwxiLG7WzvvjE5PRpZBQmAHo= 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-20230321155629-9a39f2531310/go.mod h1:8TI4H3IbrackdNgv+92dI+rhpCaLqM0IfpgCgenFvRE= @@ -36,8 +36,8 @@ github.com/ProtonMail/go-message v0.13.1-0.20230526094639-b62c999c85b7 h1:+j+Kd/ github.com/ProtonMail/go-message v0.13.1-0.20230526094639-b62c999c85b7/go.mod h1:NBAn21zgCJ/52WLDyed18YvYFm5tEoeDauubFqLokM4= github.com/ProtonMail/go-mime v0.0.0-20230322103455-7d82a3887f2f h1:tCbYj7/299ekTTXpdwKYF8eBlsYsDVoggDAuAjoK66k= github.com/ProtonMail/go-mime v0.0.0-20230322103455-7d82a3887f2f/go.mod h1:gcr0kNtGBqin9zDW9GOHcVntrwnjrK+qdJ06mWYBybw= -github.com/ProtonMail/go-proton-api v0.4.1-0.20231108105501-fb55d3bd8bd8 h1:A89egSM6ODsnxLrb8ChMC/SR5yqZiGIJipEdesPOPhM= -github.com/ProtonMail/go-proton-api v0.4.1-0.20231108105501-fb55d3bd8bd8/go.mod h1:WEXJqj5DSc2YI77SgXdpMY0nk33Qy92Vu2r4tOEazA8= +github.com/ProtonMail/go-proton-api v0.4.1-0.20231114153253-d2fbf42bb036 h1:nlvJaayeMa3ZSFtPyiO1NoIQnA7MAuXFvv1ZKf6i91E= +github.com/ProtonMail/go-proton-api v0.4.1-0.20231114153253-d2fbf42bb036/go.mod h1:WEXJqj5DSc2YI77SgXdpMY0nk33Qy92Vu2r4tOEazA8= github.com/ProtonMail/go-smtp v0.0.0-20231109081432-2b3d50599865 h1:EP1gnxLL5Z7xBSymE9nSTM27nRYINuvssAtDmG0suD8= github.com/ProtonMail/go-smtp v0.0.0-20231109081432-2b3d50599865/go.mod h1:qm27SGYgoIPRot6ubfQ/GpiPy/g3PaZAVRxiO/sDUgQ= github.com/ProtonMail/go-srp v0.0.7 h1:Sos3Qk+th4tQR64vsxGIxYpN3rdnG9Wf9K4ZloC1JrI= diff --git a/internal/services/imapservice/api_client.go b/internal/services/imapservice/api_client.go index c6bf2630..45fd3cf2 100644 --- a/internal/services/imapservice/api_client.go +++ b/internal/services/imapservice/api_client.go @@ -48,4 +48,6 @@ type APIClient interface { DeleteMessage(ctx context.Context, messageIDs ...string) error MarkMessagesRead(ctx context.Context, messageIDs ...string) error MarkMessagesUnread(ctx context.Context, messageIDs ...string) error + MarkMessagesForwarded(ctx context.Context, messageIDs ...string) error + MarkMessagesUnForwarded(ctx context.Context, messageIDs ...string) error } diff --git a/internal/services/imapservice/connector.go b/internal/services/imapservice/connector.go index 6e9dc031..a09869fa 100644 --- a/internal/services/imapservice/connector.go +++ b/internal/services/imapservice/connector.go @@ -84,9 +84,9 @@ func NewConnector( identityState: identityState, addrID: addrID, showAllMail: b32(showAllMail), - flags: defaultFlags, - permFlags: defaultPermanentFlags, - attrs: defaultAttributes, + flags: defaultMailboxFlags(), + permFlags: defaultMailboxPermanentFlags(), + attrs: defaultMailboxAttributes(), client: apiClient, telemetry: telemetry, @@ -144,6 +144,18 @@ func (s *Connector) Init(ctx context.Context, cache connector.IMAPState) error { } } } + + // Retroactively apply the forwarded flags to existing mailboxes so that the IMAP clients can recognize + // that they can store these flags now. + if err := write.AddFlagsToAllMailboxes(ctx, imap.ForwardFlagList...); err != nil { + return fmt.Errorf("failed to add \\Forward flag to all mailboxes:%w", err) + } + + // Add forwarded flag as perm flags to all mailboxes. + if err := write.AddPermFlagsToAllMailboxes(ctx, imap.ForwardFlagList...); err != nil { + return fmt.Errorf("failed to add \\Forward permanent flag to all mailboxes:%w", err) + } + return nil }) } @@ -487,6 +499,14 @@ func (s *Connector) MarkMessagesFlagged(ctx context.Context, _ connector.IMAPSta return s.client.UnlabelMessages(ctx, usertypes.MapTo[imap.MessageID, string](messageIDs), proton.StarredLabel) } +func (s *Connector) MarkMessagesForwarded(ctx context.Context, _ connector.IMAPStateWrite, messageIDs []imap.MessageID, flagged bool) error { + if flagged { + return s.client.MarkMessagesForwarded(ctx, usertypes.MapTo[imap.MessageID, string](messageIDs)...) + } + + return s.client.MarkMessagesUnForwarded(ctx, usertypes.MapTo[imap.MessageID, string](messageIDs)...) +} + func (s *Connector) GetUpdates() <-chan imap.Update { return s.updateCh.GetChannel() } @@ -501,12 +521,6 @@ func (s *Connector) ShowAllMail(v bool) { atomic.StoreUint32(&s.showAllMail, b32(v)) } -var ( - defaultFlags = imap.NewFlagSet(imap.FlagSeen, imap.FlagFlagged, imap.FlagDeleted) // nolint:gochecknoglobals - defaultPermanentFlags = imap.NewFlagSet(imap.FlagSeen, imap.FlagFlagged, imap.FlagDeleted) // nolint:gochecknoglobals - defaultAttributes = imap.NewFlagSet() // nolint:gochecknoglobals -) - const ( folderPrefix = "Folders" labelPrefix = "Labels" @@ -812,3 +826,18 @@ func fixGODT3003Labels( return applied, nil } + +func defaultMailboxFlags() imap.FlagSet { + f := imap.NewFlagSet(imap.FlagSeen, imap.FlagFlagged, imap.FlagDeleted) + f.AddToSelf(imap.ForwardFlagList...) + + return f +} + +func defaultMailboxPermanentFlags() imap.FlagSet { + return defaultMailboxFlags() +} + +func defaultMailboxAttributes() imap.FlagSet { + return imap.NewFlagSet() +} diff --git a/internal/services/imapservice/helpers.go b/internal/services/imapservice/helpers.go index 327d1415..fddce51e 100644 --- a/internal/services/imapservice/helpers.go +++ b/internal/services/imapservice/helpers.go @@ -68,6 +68,10 @@ func BuildFlagSetFromMessageMetadata(message proton.MessageMetadata) imap.FlagSe flags.AddToSelf(imap.FlagAnswered) } + if message.IsForwarded { + flags.AddToSelf(imap.ForwardFlagList...) + } + return flags } diff --git a/internal/services/imapservice/imap_updates.go b/internal/services/imapservice/imap_updates.go index c7f297bd..9f8b8299 100644 --- a/internal/services/imapservice/imap_updates.go +++ b/internal/services/imapservice/imap_updates.go @@ -32,8 +32,8 @@ func newSystemMailboxCreatedUpdate(labelID imap.MailboxID, labelName string) *im } attrs := imap.NewFlagSet(imap.AttrNoInferiors) - permanentFlags := defaultPermanentFlags - flags := defaultFlags + permanentFlags := defaultMailboxPermanentFlags() + flags := defaultMailboxFlags() switch labelID { case proton.TrashLabel: @@ -86,8 +86,8 @@ func newPlaceHolderMailboxCreatedUpdate(labelName string) *imap.MailboxCreated { return imap.NewMailboxCreated(imap.Mailbox{ ID: imap.MailboxID(labelName), Name: []string{labelName}, - Flags: defaultFlags, - PermanentFlags: defaultPermanentFlags, + Flags: defaultMailboxFlags(), + PermanentFlags: defaultMailboxPermanentFlags(), Attributes: imap.NewFlagSet(imap.AttrNoSelect), }) } @@ -96,8 +96,8 @@ func newMailboxCreatedUpdate(labelID imap.MailboxID, labelName []string) *imap.M return imap.NewMailboxCreated(imap.Mailbox{ ID: labelID, Name: labelName, - Flags: defaultFlags, - PermanentFlags: defaultPermanentFlags, + Flags: defaultMailboxFlags(), + PermanentFlags: defaultMailboxPermanentFlags(), Attributes: imap.NewFlagSet(), }) } diff --git a/internal/services/imapservice/mocks/mocks.go b/internal/services/imapservice/mocks/mocks.go index 17e5b7cd..4cfe4d03 100644 --- a/internal/services/imapservice/mocks/mocks.go +++ b/internal/services/imapservice/mocks/mocks.go @@ -35,6 +35,44 @@ func (m *MockIMAPStateWrite) EXPECT() *MockIMAPStateWriteMockRecorder { return m.recorder } +// AddFlagsToAllMailboxes mocks base method. +func (m *MockIMAPStateWrite) AddFlagsToAllMailboxes(arg0 context.Context, arg1 ...string) error { + m.ctrl.T.Helper() + varargs := []interface{}{arg0} + for _, a := range arg1 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "AddFlagsToAllMailboxes", varargs...) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddFlagsToAllMailboxes indicates an expected call of AddFlagsToAllMailboxes. +func (mr *MockIMAPStateWriteMockRecorder) AddFlagsToAllMailboxes(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0}, arg1...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddFlagsToAllMailboxes", reflect.TypeOf((*MockIMAPStateWrite)(nil).AddFlagsToAllMailboxes), varargs...) +} + +// AddPermFlagsToAllMailboxes mocks base method. +func (m *MockIMAPStateWrite) AddPermFlagsToAllMailboxes(arg0 context.Context, arg1 ...string) error { + m.ctrl.T.Helper() + varargs := []interface{}{arg0} + for _, a := range arg1 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "AddPermFlagsToAllMailboxes", varargs...) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddPermFlagsToAllMailboxes indicates an expected call of AddPermFlagsToAllMailboxes. +func (mr *MockIMAPStateWriteMockRecorder) AddPermFlagsToAllMailboxes(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0}, arg1...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddPermFlagsToAllMailboxes", reflect.TypeOf((*MockIMAPStateWrite)(nil).AddPermFlagsToAllMailboxes), varargs...) +} + // CreateMailbox mocks base method. func (m *MockIMAPStateWrite) CreateMailbox(arg0 context.Context, arg1 imap.Mailbox) error { m.ctrl.T.Helper() diff --git a/tests/features/imap/message/store.feature b/tests/features/imap/message/store.feature new file mode 100644 index 00000000..bcc159b3 --- /dev/null +++ b/tests/features/imap/message/store.feature @@ -0,0 +1,32 @@ +Feature: IMAP marks messages as forwarded + Background: + Given there exists an account with username "[user:user]" and password "password" + And the account "[user:user]" has the following custom mailboxes: + | name | type | + | mbox | folder | + And the address "[user:user]@[domain]" of account "[user:user]" has 1 messages in "Folders/mbox" + Then it succeeds + When bridge starts + And the user logs in with username "[user:user]" and password "password" + And user "[user:user]" finishes syncing + And user "[user:user]" connects and authenticates IMAP client "1" + Then it succeeds + + Scenario: Mark message as forwarded + When IMAP client "1" selects "Folders/mbox" + And IMAP client "1" marks message 1 as "forwarded" + And it succeeds + Then IMAP client "1" eventually sees that message at row 1 has the flag "forwarded" + And it succeeds + + @ignore-live + Scenario: Mark message as forwarded and then revert + When IMAP client "1" selects "Folders/mbox" + And IMAP client "1" marks message 1 as "forwarded" + And it succeeds + Then IMAP client "1" eventually sees that message at row 1 has the flag "forwarded" + And it succeeds + And IMAP client "1" marks message 1 as "unforwarded" + And it succeeds + Then IMAP client "1" eventually sees that message at row 1 does not have the flag "forwarded" + And it succeeds diff --git a/tests/imap_test.go b/tests/imap_test.go index 8138a9c9..63f9fa55 100644 --- a/tests/imap_test.go +++ b/tests/imap_test.go @@ -468,10 +468,12 @@ func (s *scenario) imapClientMarksAllMessagesAsState(clientID, messageState stri return nil } -func (s *scenario) imapClientSeesThatMessageHasTheFlag(clientID string, seq int, flag string) error { - _, client := s.t.getIMAPClient(clientID) +func (s *scenario) imapClientEventuallySeesThatMessageHasTheFlag(clientID string, seq int, flag string) error { + return eventually(func() error { + _, client := s.t.getIMAPClient(clientID) - return clientIsFlagApplied(client, seq, flag, true, false) + return clientIsFlagApplied(client, seq, flag, true, false) + }) } func (s *scenario) imapClientSeesThatMessageDoesNotHaveTheFlag(clientID string, seq int, flag string) error { @@ -480,38 +482,46 @@ func (s *scenario) imapClientSeesThatMessageDoesNotHaveTheFlag(clientID string, return clientIsFlagApplied(client, seq, flag, false, false) } -func (s *scenario) imapClientSeesThatTheMessageWithSubjectHasTheFlag(clientID, subject, flag string) error { - _, client := s.t.getIMAPClient(clientID) +func (s *scenario) imapClientEventuallySeesThatTheMessageWithSubjectHasTheFlag(clientID, subject, flag string) error { + return eventually(func() error { + _, client := s.t.getIMAPClient(clientID) - uid, err := clientGetUIDBySubject(client, client.Mailbox().Name, subject) - if err != nil { - return err - } + uid, err := clientGetUIDBySubject(client, client.Mailbox().Name, subject) + if err != nil { + return err + } - return clientIsFlagApplied(client, int(uid), flag, true, false) + return clientIsFlagApplied(client, int(uid), flag, true, false) + }) } -func (s *scenario) imapClientSeesThatTheMessageWithSubjectDoesNotHaveTheFlag(clientID, subject, flag string) error { - _, client := s.t.getIMAPClient(clientID) +func (s *scenario) imapClientEventuallySeesThatTheMessageWithSubjectDoesNotHaveTheFlag(clientID, subject, flag string) error { + return eventually(func() error { + _, client := s.t.getIMAPClient(clientID) - uid, err := clientGetUIDBySubject(client, client.Mailbox().Name, subject) - if err != nil { - return err - } + uid, err := clientGetUIDBySubject(client, client.Mailbox().Name, subject) + if err != nil { + return err + } - return clientIsFlagApplied(client, int(uid), flag, false, false) + return clientIsFlagApplied(client, int(uid), flag, false, false) + }) } -func (s *scenario) imapClientSeesThatAllTheMessagesHaveTheFlag(clientID string, flag string) error { - _, client := s.t.getIMAPClient(clientID) +func (s *scenario) imapClientEventuallySeesThatAllTheMessagesHaveTheFlag(clientID string, flag string) error { + return eventually(func() error { + _, client := s.t.getIMAPClient(clientID) - return clientIsFlagApplied(client, 1, flag, true, true) + return clientIsFlagApplied(client, 1, flag, true, true) + }) } -func (s *scenario) imapClientSeesThatAllTheMessagesDoNotHaveTheFlag(clientID string, flag string) error { - _, client := s.t.getIMAPClient(clientID) +func (s *scenario) imapClientEventuallySeesThatAllTheMessagesDoNotHaveTheFlag(clientID string, flag string) error { + return eventually(func() error { + _, client := s.t.getIMAPClient(clientID) - return clientIsFlagApplied(client, 1, flag, false, true) + return clientIsFlagApplied(client, 1, flag, false, true) + }) } func (s *scenario) imapClientExpunges(clientID string) error { @@ -916,6 +926,18 @@ func clientChangeMessageState(client *client.Client, seq int, messageState strin if err != nil { return err } + + case messageState == "forwarded": + _, err := clientStore(client, seq, seq, isUID, imap.FormatFlagsOp(imap.AddFlags, true), "Forwarded") + if err != nil { + return err + } + + case messageState == "unforwarded": + _, err := clientStore(client, seq, seq, isUID, imap.FormatFlagsOp(imap.RemoveFlags, true), "Forwarded") + if err != nil { + return err + } } return nil diff --git a/tests/steps_test.go b/tests/steps_test.go index db67d42f..1ccc1b01 100644 --- a/tests/steps_test.go +++ b/tests/steps_test.go @@ -156,12 +156,12 @@ func (s *scenario) steps(ctx *godog.ScenarioContext) { ctx.Step(`^IMAP client "([^"]*)" marks message (\d+) as "([^"]*)"$`, s.imapClientMarksMessageAsState) ctx.Step(`^IMAP client "([^"]*)" marks the message with subject "([^"]*)" as "([^"]*)"$`, s.imapClientMarksTheMessageWithSubjectAsState) ctx.Step(`^IMAP client "([^"]*)" marks all messages as "([^"]*)"$`, s.imapClientMarksAllMessagesAsState) - ctx.Step(`^IMAP client "([^"]*)" eventually sees that message at row (\d+) has the flag "([^"]*)"$`, s.imapClientSeesThatMessageHasTheFlag) + ctx.Step(`^IMAP client "([^"]*)" eventually sees that message at row (\d+) has the flag "([^"]*)"$`, s.imapClientEventuallySeesThatMessageHasTheFlag) ctx.Step(`^IMAP client "([^"]*)" eventually sees that message at row (\d+) does not have the flag "([^"]*)"$`, s.imapClientSeesThatMessageDoesNotHaveTheFlag) - ctx.Step(`^IMAP client "([^"]*)" eventually sees that the message with subject "([^"]*)" has the flag "([^"]*)"`, s.imapClientSeesThatTheMessageWithSubjectHasTheFlag) - ctx.Step(`^IMAP client "([^"]*)" eventually sees that the message with subject "([^"]*)" does not have the flag "([^"]*)"`, s.imapClientSeesThatTheMessageWithSubjectDoesNotHaveTheFlag) - ctx.Step(`^IMAP client "([^"]*)" eventually sees that all the messages have the flag "([^"]*)"`, s.imapClientSeesThatAllTheMessagesHaveTheFlag) - ctx.Step(`^IMAP client "([^"]*)" eventually sees that all the messages do not have the flag "([^"]*)"`, s.imapClientSeesThatAllTheMessagesDoNotHaveTheFlag) + ctx.Step(`^IMAP client "([^"]*)" eventually sees that the message with subject "([^"]*)" has the flag "([^"]*)"`, s.imapClientEventuallySeesThatTheMessageWithSubjectHasTheFlag) + ctx.Step(`^IMAP client "([^"]*)" eventually sees that the message with subject "([^"]*)" does not have the flag "([^"]*)"`, s.imapClientEventuallySeesThatTheMessageWithSubjectDoesNotHaveTheFlag) + ctx.Step(`^IMAP client "([^"]*)" eventually sees that all the messages have the flag "([^"]*)"`, s.imapClientEventuallySeesThatAllTheMessagesHaveTheFlag) + ctx.Step(`^IMAP client "([^"]*)" eventually sees that all the messages do not have the flag "([^"]*)"`, s.imapClientEventuallySeesThatAllTheMessagesDoNotHaveTheFlag) ctx.Step(`^IMAP client "([^"]*)" appends the following message to "([^"]*)":$`, s.imapClientAppendsTheFollowingMessageToMailbox) ctx.Step(`^IMAP client "([^"]*)" appends the following messages to "([^"]*)":$`, s.imapClientAppendsTheFollowingMessagesToMailbox) ctx.Step(`^IMAP client "([^"]*)" appends "([^"]*)" to "([^"]*)"$`, s.imapClientAppendsToMailbox)