From edd326efd97fda320d82e27ab44bb1281ae08036 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Thu, 6 Oct 2022 13:00:09 +0200 Subject: [PATCH] GODT-1650: Mixed case and failure sending tests --- go.mod | 2 +- go.sum | 2 + internal/user/smtp.go | 41 ++++++++++++--- tests/features/smtp/send/failures.feature | 55 +++++++++++++++++++++ tests/features/smtp/send/mixed_case.feature | 43 ++++++++++++++++ 5 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 tests/features/smtp/send/failures.feature create mode 100644 tests/features/smtp/send/mixed_case.feature diff --git a/go.mod b/go.mod index c5f9b7dc..a82df419 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,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.33.2-0.20221006095946-fc4061f2140b + gitlab.protontech.ch/go/liteapi v0.33.2-0.20221006105817-e76abecc140a 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 2dfa6fdc..17d5da67 100644 --- a/go.sum +++ b/go.sum @@ -475,6 +475,8 @@ gitlab.protontech.ch/go/liteapi v0.33.1 h1:Ks8YdojRwYTLTUmGLG9MzFvxNeiwYJYQEaTHU gitlab.protontech.ch/go/liteapi v0.33.1/go.mod h1:9nsslyEJn7Utbielp4c+hc7qT6hqIJ52aGFR/tX+tYk= gitlab.protontech.ch/go/liteapi v0.33.2-0.20221006095946-fc4061f2140b h1:Obu2CCCYdVi3NJmoYf/iJco1mat6EzJezInKoUTo+Dc= gitlab.protontech.ch/go/liteapi v0.33.2-0.20221006095946-fc4061f2140b/go.mod h1:9nsslyEJn7Utbielp4c+hc7qT6hqIJ52aGFR/tX+tYk= +gitlab.protontech.ch/go/liteapi v0.33.2-0.20221006105817-e76abecc140a h1:dt6BahWRcy88dcXnEbm9m1X1W+RCZaVlo3W45V+vReQ= +gitlab.protontech.ch/go/liteapi v0.33.2-0.20221006105817-e76abecc140a/go.mod h1:9nsslyEJn7Utbielp4c+hc7qT6hqIJ52aGFR/tX+tYk= 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/smtp.go b/internal/user/smtp.go index 9464062b..46a5d1e2 100644 --- a/internal/user/smtp.go +++ b/internal/user/smtp.go @@ -22,6 +22,7 @@ import ( "github.com/emersion/go-smtp" "github.com/sirupsen/logrus" "gitlab.protontech.ch/go/liteapi" + "golang.org/x/exp/maps" "golang.org/x/exp/slices" ) @@ -194,6 +195,7 @@ func (session *smtpSession) Data(r io.Reader) error { session.settings, sanitizeEmail(session.emails[session.fromAddrID]), session.to, + maps.Values(session.emails), parser, ) if err != nil { @@ -219,7 +221,7 @@ func sendWithKey( userKR, addrKR *crypto.KeyRing, settings liteapi.MailSettings, from string, - to []string, + to, emails []string, parser *parser.Parser, ) (liteapi.Message, error) { ctx, cancel := context.WithCancel(context.Background()) @@ -244,7 +246,7 @@ func sendWithKey( return liteapi.Message{}, fmt.Errorf("failed to parse message: %w", err) } - if err := sanitizeParsedMessage(&message, from, to); err != nil { + if err := sanitizeParsedMessage(&message, from, to, emails); err != nil { return liteapi.Message{}, fmt.Errorf("failed to sanitize message: %w", err) } @@ -276,7 +278,7 @@ func sendWithKey( return res, nil } -func sanitizeParsedMessage(message *message.Message, from string, to []string) error { +func sanitizeParsedMessage(message *message.Message, from string, to, emails []string) error { // Check sender: set the sender in the parsed message if it's missing. if message.Sender == nil { message.Sender = &mail.Address{Address: from} @@ -284,6 +286,15 @@ func sanitizeParsedMessage(message *message.Message, from string, to []string) e message.Sender.Address = from } + // Check that the sending address is owned by the user, and if so, properly capitalize it. + if idx := xslices.IndexFunc(emails, func(email string) bool { + return strings.EqualFold(email, sanitizeEmail(message.Sender.Address)) + }); idx < 0 { + return fmt.Errorf("address %q is not owned by user", message.Sender.Address) + } else { + message.Sender.Address = constructEmail(message.Sender.Address, emails[idx]) + } + // Check ToList: ensure that ToList only contains addresses we actually plan to send to. message.ToList = xslices.Filter(message.ToList, func(addr *mail.Address) bool { return slices.Contains(to, addr.Address) @@ -563,7 +574,25 @@ func sanitizeEmail(email string) string { if len(splitAt) != 2 { return email } - splitPlus := strings.Split(splitAt[0], "+") - email = splitPlus[0] + "@" + splitAt[1] - return email + + return strings.Split(splitAt[0], "+")[0] + "@" + splitAt[1] +} + +func constructEmail(headerEmail string, addressEmail string) string { + splitAtHeader := strings.Split(headerEmail, "@") + if len(splitAtHeader) != 2 { + return addressEmail + } + + splitPlus := strings.Split(splitAtHeader[0], "+") + if len(splitPlus) != 2 { + return addressEmail + } + + splitAtAddress := strings.Split(addressEmail, "@") + if len(splitAtAddress) != 2 { + return addressEmail + } + + return splitAtAddress[0] + "+" + splitPlus[1] + "@" + splitAtAddress[1] } diff --git a/tests/features/smtp/send/failures.feature b/tests/features/smtp/send/failures.feature new file mode 100644 index 00000000..ab1ce9ee --- /dev/null +++ b/tests/features/smtp/send/failures.feature @@ -0,0 +1,55 @@ +Feature: SMTP wrong messages + Background: + Given there exists an account with username "user@pm.me" and password "password" + And there exists an account with username "bridgetest@protonmail.com" and password "password" + And bridge starts + And the user logs in with username "user@pm.me" and password "password" + And user "user@pm.me" connects and authenticates SMTP client "1" + + Scenario: Message with attachment and wrong boundaries + When SMTP client "1" sends the following message from "user@pm.me" to "bridgetest@protonmail.com": + """ + From: Bridge Test + To: Internal Bridge + Subject: With attachment (wrong boundaries) + Content-Type: multipart/related; boundary=bc5bd30245232f31b6c976adcd59bb0069c9b13f986f9e40c2571bb80aa16606 + + --bc5bd30245232f31b6c976adcd59bb0069c9b13f986f9e40c2571bb80aa16606 + Content-Disposition: inline + Content-Transfer-Encoding: quoted-printable + Content-Type: text/plain; charset=utf-8 + + This is body of mail with attachment + + --bc5bd30245232f31b6c976adcd59bb0069c9b13f986f9e40c2571bb80aa16606 + Content-Disposition: attachment; filename=outline-light-instagram-48.png + Content-Id: <9114fe6f0adfaf7fdf7a@protonmail.com> + Content-Transfer-Encoding: base64 + Content-Type: image/png + + iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAALVBMVEUAAAD///////////////// + //////////////////////////////////////+hSKubAAAADnRSTlMAgO8QQM+/IJ9gj1AwcIQd + OXUAAAGdSURBVDjLXJC9SgNBFIVPXDURTYhgIQghINgowyLYCAYtRFAIgtYhpAjYhC0srCRW6YIg + WNpoHVSsg/gEii+Qnfxq4DyDc3cyMfrBwl2+O+fOHTi8p7LS5RUf/9gpMKL7iT9sK47Q95ggpkzv + 1cvRcsGYNMYsmP+zKN27NR2vcDyTNVdfkOuuniNPMWafvIbljt+YoMEvW8y7lt+ARwhvrgPjhA0I + BTng7S1GLPlypBvtIBPidY4YBDJFdtnkscQ5JGaGqxC9i7jSDwcwnB8qHWBaQjw1ABI8wYgtVoG6 + 9pFkH8iZIiJeulFt4JLvJq8I5N2GMWYbHWDWzM3JZTMdeSWla0kW86FcuI0mfStiNKQ/AhEeh8h0 + YUTffFwrMTT5oSwdojIQ0UKcocgAKRH1HiqhFQmmJa5qRaYHNbRiSsOgslY0NdixItUTUWlZkedP + HXVyAgAIA1F0wP5btQZPIyTwvAqa/Fl4oacuP+e4XHAjSYpkQkxSiMX+T7FPoZJToSStzED70HCy + KE3NGCg4jJrC6Ti7AFwZLhnW0gMbzFZc0RmmeAAAAABJRU5ErkJggg== + --bc5bd30245232f31b6c976adcd59bb0069c9b13f986f9e40c2571bb80aa16606 + + + """ + Then it fails + + Scenario: Invalid from + When SMTP client "1" sends the following message from "bridgetest@pm.test" to "bridgetest@protonmail.com": + """ + From: Bridge Test + To: Internal Bridge + + hello + + """ + Then it fails diff --git a/tests/features/smtp/send/mixed_case.feature b/tests/features/smtp/send/mixed_case.feature new file mode 100644 index 00000000..336fbfa2 --- /dev/null +++ b/tests/features/smtp/send/mixed_case.feature @@ -0,0 +1,43 @@ +Feature: SMTP sending with mixed case address + Background: + Given there exists an account with username "user@pm.me" and password "password" + And there exists an account with username "bridgetest@protonmail.com" and password "password" + And bridge starts + And the user logs in with username "user@pm.me" and password "password" + And user "user@pm.me" connects and authenticates SMTP client "1" + + Scenario: Mixed sender case in sender address + When SMTP client "1" sends the following message from "user@pm.me" to "bridgetest@protonmail.com": + """ + From: Bridge Test + To: Internal Bridge + + hello + + """ + Then it succeeds + When user "user@pm.me" connects and authenticates IMAP client "1" + Then IMAP client "1" eventually sees the following messages in "Sent": + | from | to | subject | + | user@pm.me | bridgetest@protonmail.com | | + And the body in the "POST" request to "/mail/v4/messages" is: + """ + { + "Message": { + "Subject": "", + "Sender": { + "Name": "Bridge Test", + "Address": "user@pm.me" + }, + "ToList": [ + { + "Address": "bridgetest@protonmail.com", + "Name": "Internal Bridge" + } + ], + "CCList": [], + "BCCList": [], + "MIMEType": "text/plain" + } + } + """