From 5be8a7a7ea96d962d0f0b9f09b967e403a227698 Mon Sep 17 00:00:00 2001 From: Forest Johnson Date: Mon, 31 Jan 2022 10:46:20 +0000 Subject: [PATCH] [bug] Send plaintext emails to fix "message refused: Message is not RFC 2822 compliant" (#366) * trying to fix "message refused: Message is not RFC 2822 compliant" * fix "message refused: Message is not RFC 2822 compliant" 550 5.7.1 Delivery not authorized, message refused: Message is not RFC 2822 compliant * remove silly regex * lint * fix tests * we should use text/template instead of html/template now --- internal/email/confirm.go | 14 ++++-- internal/email/noopsender.go | 12 +++-- internal/email/reset.go | 9 ++-- internal/email/sender.go | 2 +- internal/email/util.go | 45 ++++++++++++------- internal/email/util_test.go | 4 +- internal/processing/user/emailconfirm_test.go | 2 +- ...l_confirm.tmpl => email_confirm_html.tmpl} | 0 web/template/email_confirm_text.tmpl | 9 ++++ ...email_reset.tmpl => email_reset_html.tmpl} | 0 web/template/email_reset_text.tmpl | 9 ++++ 11 files changed, 78 insertions(+), 28 deletions(-) rename web/template/{email_confirm.tmpl => email_confirm_html.tmpl} (100%) create mode 100644 web/template/email_confirm_text.tmpl rename web/template/{email_reset.tmpl => email_reset_html.tmpl} (100%) create mode 100644 web/template/email_reset_text.tmpl diff --git a/internal/email/confirm.go b/internal/email/confirm.go index 4503137b..34e2fb66 100644 --- a/internal/email/confirm.go +++ b/internal/email/confirm.go @@ -21,11 +21,15 @@ package email import ( "bytes" "net/smtp" + + "github.com/sirupsen/logrus" + "github.com/spf13/viper" + "github.com/superseriousbusiness/gotosocial/internal/config" ) const ( - confirmTemplate = "email_confirm.tmpl" - confirmSubject = "Subject: GoToSocial Email Confirmation" + confirmTemplate = "email_confirm_text.tmpl" + confirmSubject = "GoToSocial Email Confirmation" ) func (s *sender) SendConfirmEmail(toAddress string, data ConfirmData) error { @@ -35,7 +39,11 @@ func (s *sender) SendConfirmEmail(toAddress string, data ConfirmData) error { } confirmBody := buf.String() - msg := assembleMessage(confirmSubject, confirmBody, toAddress, s.from) + msg, err := assembleMessage(confirmSubject, confirmBody, toAddress, s.from) + if err != nil { + return err + } + logrus.WithField("func", "SendConfirmEmail").Trace(s.hostAddress + "\n" + viper.GetString(config.Keys.SMTPUsername) + ":password" + "\n" + s.from + "\n" + toAddress + "\n\n" + string(msg) + "\n") return smtp.SendMail(s.hostAddress, s.auth, s.from, []string{toAddress}, msg) } diff --git a/internal/email/noopsender.go b/internal/email/noopsender.go index efec303f..9f587f31 100644 --- a/internal/email/noopsender.go +++ b/internal/email/noopsender.go @@ -20,7 +20,7 @@ package email import ( "bytes" - "html/template" + "text/template" "github.com/sirupsen/logrus" "github.com/spf13/viper" @@ -57,7 +57,10 @@ func (s *noopSender) SendConfirmEmail(toAddress string, data ConfirmData) error } confirmBody := buf.String() - msg := assembleMessage(confirmSubject, confirmBody, toAddress, "test@example.org") + msg, err := assembleMessage(confirmSubject, confirmBody, toAddress, "test@example.org") + if err != nil { + return err + } logrus.Tracef("NOT SENDING confirmation email to %s with contents: %s", toAddress, msg) @@ -74,7 +77,10 @@ func (s *noopSender) SendResetEmail(toAddress string, data ResetData) error { } resetBody := buf.String() - msg := assembleMessage(resetSubject, resetBody, toAddress, "test@example.org") + msg, err := assembleMessage(resetSubject, resetBody, toAddress, "test@example.org") + if err != nil { + return err + } logrus.Tracef("NOT SENDING reset email to %s with contents: %s", toAddress, msg) diff --git a/internal/email/reset.go b/internal/email/reset.go index 7a08ebda..b646ef99 100644 --- a/internal/email/reset.go +++ b/internal/email/reset.go @@ -24,8 +24,8 @@ import ( ) const ( - resetTemplate = "email_reset.tmpl" - resetSubject = "Subject: GoToSocial Password Reset" + resetTemplate = "email_reset_text.tmpl" + resetSubject = "GoToSocial Password Reset" ) func (s *sender) SendResetEmail(toAddress string, data ResetData) error { @@ -35,7 +35,10 @@ func (s *sender) SendResetEmail(toAddress string, data ResetData) error { } resetBody := buf.String() - msg := assembleMessage(resetSubject, resetBody, toAddress, s.from) + msg, err := assembleMessage(resetSubject, resetBody, toAddress, s.from) + if err != nil { + return err + } return smtp.SendMail(s.hostAddress, s.auth, s.from, []string{toAddress}, msg) } diff --git a/internal/email/sender.go b/internal/email/sender.go index 97bbcd23..f4462749 100644 --- a/internal/email/sender.go +++ b/internal/email/sender.go @@ -20,8 +20,8 @@ package email import ( "fmt" - "html/template" "net/smtp" + "text/template" "github.com/spf13/viper" "github.com/superseriousbusiness/gotosocial/internal/config" diff --git a/internal/email/util.go b/internal/email/util.go index db95128f..52290dbe 100644 --- a/internal/email/util.go +++ b/internal/email/util.go @@ -19,15 +19,12 @@ package email import ( + "errors" "fmt" - "html/template" "os" "path/filepath" -) - -const ( - mime = `MIME-version: 1.0; -Content-Type: text/html;` + "strings" + "text/template" ) func loadTemplates(templateBaseDir string) (*template.Template, error) { @@ -41,16 +38,34 @@ func loadTemplates(templateBaseDir string) (*template.Template, error) { return template.ParseGlob(tmPath) } -func assembleMessage(mailSubject string, mailBody string, mailTo string, mailFrom string) []byte { - from := fmt.Sprintf("From: GoToSocial <%s>", mailFrom) - to := fmt.Sprintf("To: %s", mailTo) +// https://datatracker.ietf.org/doc/html/rfc2822 +// I did not read the RFC, I just copy and pasted from +// https://pkg.go.dev/net/smtp#SendMail +// and it did seem to work. +func assembleMessage(mailSubject string, mailBody string, mailTo string, mailFrom string) ([]byte, error) { + + if strings.Contains(mailSubject, "\r") || strings.Contains(mailSubject, "\n") { + return nil, errors.New("email subject must not contain newline characters") + } + + if strings.Contains(mailFrom, "\r") || strings.Contains(mailFrom, "\n") { + return nil, errors.New("email from address must not contain newline characters") + } + + if strings.Contains(mailTo, "\r") || strings.Contains(mailTo, "\n") { + return nil, errors.New("email to address must not contain newline characters") + } + + // normalize the message body to use CRLF line endings + mailBody = strings.ReplaceAll(mailBody, "\r\n", "\n") + mailBody = strings.ReplaceAll(mailBody, "\n", "\r\n") msg := []byte( - mailSubject + "\r\n" + - from + "\r\n" + - to + "\r\n" + - mime + "\r\n" + - mailBody + "\r\n") + "To: " + mailTo + "\r\n" + + "Subject: " + mailSubject + "\r\n" + + "\r\n" + + mailBody + "\r\n", + ) - return msg + return msg, nil } diff --git a/internal/email/util_test.go b/internal/email/util_test.go index b5c7a985..8895785f 100644 --- a/internal/email/util_test.go +++ b/internal/email/util_test.go @@ -39,7 +39,7 @@ func (suite *UtilTestSuite) TestTemplateConfirm() { suite.sender.SendConfirmEmail("user@example.org", confirmData) suite.Len(suite.sentEmails, 1) - suite.Equal("Subject: GoToSocial Email Confirmation\r\nFrom: GoToSocial \r\nTo: user@example.org\r\nMIME-version: 1.0;\nContent-Type: text/html;\r\n\n\n \n \n
\n

\n Hello test!\n

\n
\n
\n

\n You are receiving this mail because you've requested an account on Test Instance.\n

\n

\n We just need to confirm that this is your email address. To confirm your email, click here or paste the following in your browser's address bar:\n

\n

\n \n https://example.org/confirm_email?token=ee24f71d-e615-43f9-afae-385c0799b7fa\n \n

\n
\n
\n

\n If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of Test Instance.\n

\n
\n \n\r\n", suite.sentEmails["user@example.org"]) + suite.Equal("To: user@example.org\r\nSubject: GoToSocial Email Confirmation\r\n\r\nHello test!\r\n\r\nYou are receiving this mail because you've requested an account on https://example.org.\r\n\r\nWe just need to confirm that this is your email address. To confirm your email, paste the following in your browser's address bar:\r\n\r\nhttps://example.org/confirm_email?token=ee24f71d-e615-43f9-afae-385c0799b7fa\r\n\r\nIf you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of https://example.org\r\n\r\n", suite.sentEmails["user@example.org"]) } func (suite *UtilTestSuite) TestTemplateReset() { @@ -52,7 +52,7 @@ func (suite *UtilTestSuite) TestTemplateReset() { suite.sender.SendResetEmail("user@example.org", resetData) suite.Len(suite.sentEmails, 1) - suite.Equal("Subject: GoToSocial Password Reset\r\nFrom: GoToSocial \r\nTo: user@example.org\r\nMIME-version: 1.0;\nContent-Type: text/html;\r\n\n\n \n \n
\n

\n Hello test!\n

\n
\n
\n

\n You are receiving this mail because a password reset has been requested for your account on Test Instance.\n

\n

\n To reset your password, click here or paste the following in your browser's address bar:\n

\n

\n \n https://example.org/reset_email?token=ee24f71d-e615-43f9-afae-385c0799b7fa\n \n

\n
\n
\n

\n If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of Test Instance.\n

\n
\n \n\r\n", suite.sentEmails["user@example.org"]) + suite.Equal("To: user@example.org\r\nSubject: GoToSocial Password Reset\r\n\r\nHello test!\r\n\r\nYou are receiving this mail because a password reset has been requested for your account on https://example.org.\r\n\r\nTo reset your password, paste the following in your browser's address bar:\r\n\r\nhttps://example.org/reset_email?token=ee24f71d-e615-43f9-afae-385c0799b7fa\r\n\r\nIf you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of https://example.org.\r\n\r\n", suite.sentEmails["user@example.org"]) } func TestUtilTestSuite(t *testing.T) { diff --git a/internal/processing/user/emailconfirm_test.go b/internal/processing/user/emailconfirm_test.go index 58836d40..6f22306a 100644 --- a/internal/processing/user/emailconfirm_test.go +++ b/internal/processing/user/emailconfirm_test.go @@ -54,7 +54,7 @@ func (suite *EmailConfirmTestSuite) TestSendConfirmEmail() { suite.NotEmpty(token) // email should contain the token - emailShould := fmt.Sprintf("Subject: GoToSocial Email Confirmation\r\nFrom: GoToSocial \r\nTo: some.email@example.org\r\nMIME-version: 1.0;\nContent-Type: text/html;\r\n\n\n \n \n
\n

\n Hello the_mighty_zork!\n

\n
\n
\n

\n You are receiving this mail because you've requested an account on localhost:8080.\n

\n

\n We just need to confirm that this is your email address. To confirm your email, click here or paste the following in your browser's address bar:\n

\n

\n \n http://localhost:8080/confirm_email?token=%s\n \n

\n
\n
\n

\n If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of localhost:8080.\n

\n
\n \n\r\n", token, token) + emailShould := fmt.Sprintf("To: some.email@example.org\r\nSubject: GoToSocial Email Confirmation\r\n\r\nHello the_mighty_zork!\r\n\r\nYou are receiving this mail because you've requested an account on http://localhost:8080.\r\n\r\nWe just need to confirm that this is your email address. To confirm your email, paste the following in your browser's address bar:\r\n\r\nhttp://localhost:8080/confirm_email?token=%s\r\n\r\nIf you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of http://localhost:8080\r\n\r\n", token) suite.Equal(emailShould, email) // confirmationSentAt should be recent diff --git a/web/template/email_confirm.tmpl b/web/template/email_confirm_html.tmpl similarity index 100% rename from web/template/email_confirm.tmpl rename to web/template/email_confirm_html.tmpl diff --git a/web/template/email_confirm_text.tmpl b/web/template/email_confirm_text.tmpl new file mode 100644 index 00000000..8682cc49 --- /dev/null +++ b/web/template/email_confirm_text.tmpl @@ -0,0 +1,9 @@ +Hello {{.Username}}! + +You are receiving this mail because you've requested an account on {{.InstanceURL}}. + +We just need to confirm that this is your email address. To confirm your email, paste the following in your browser's address bar: + +{{.ConfirmLink}} + +If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of {{.InstanceURL}} diff --git a/web/template/email_reset.tmpl b/web/template/email_reset_html.tmpl similarity index 100% rename from web/template/email_reset.tmpl rename to web/template/email_reset_html.tmpl diff --git a/web/template/email_reset_text.tmpl b/web/template/email_reset_text.tmpl new file mode 100644 index 00000000..ccac3bf7 --- /dev/null +++ b/web/template/email_reset_text.tmpl @@ -0,0 +1,9 @@ +Hello {{.Username}}! + +You are receiving this mail because a password reset has been requested for your account on {{.InstanceURL}}. + +To reset your password, paste the following in your browser's address bar: + +{{.ResetLink}} + +If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of {{.InstanceURL}}.