From 732ee3679eae7c971bb3750f30747079e940d48a Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Mon, 25 Nov 2024 17:32:34 -0700 Subject: [PATCH 1/6] feat: add email validation function to lower bounce rates The goal is to only return an error when we have a very high confidence the email won't be deliverable. --- internal/mailer/validate.go | 107 +++++++++++++++++++++++++++++++ internal/mailer/validate_test.go | 92 ++++++++++++++++++++++++++ 2 files changed, 199 insertions(+) create mode 100644 internal/mailer/validate.go create mode 100644 internal/mailer/validate_test.go diff --git a/internal/mailer/validate.go b/internal/mailer/validate.go new file mode 100644 index 000000000..a48a29d8b --- /dev/null +++ b/internal/mailer/validate.go @@ -0,0 +1,107 @@ +package mailer + +import ( + "context" + "errors" + "net" + "net/mail" + "strings" + "time" +) + +var invalidEmailMap = map[string]bool{ + "test@gmail.com": true, + "test@test.com": true, + "test@email.com": true, +} + +// https://www.rfc-editor.org/rfc/rfc2606 +var invalidHostMap = map[string]bool{ + "test": true, + "example": true, + "invalid": true, + "example.com": true, + "example.net": true, + "example.org": true, +} + +const ( + validateEmailTimeout = 250 * time.Millisecond +) + +var ( + // We use the default resolver for this. + validateEmailResolver net.Resolver +) + +var ( + ErrInvalidEmailFormat = errors.New("invalid email format") + ErrInvalidEmailAddress = errors.New("invalid email address") +) + +// ValidateEmail returns a nil error in all cases but the following: +// - `email` cannot be parsed by mail.ParseAddress +// - `email` has a domain with no DNS configured +func ValidateEmail(ctx context.Context, email string) error { + ctx, cancel := context.WithTimeout(ctx, validateEmailTimeout) + defer cancel() + + return validateEmail(ctx, email) +} + +func validateEmail(ctx context.Context, email string) error { + ea, err := mail.ParseAddress(email) + if err != nil { + return ErrInvalidEmailFormat + } + + i := strings.LastIndex(ea.Address, "@") + if i == -1 { + return ErrInvalidEmailFormat + } + + // few static lookups that are typed constantly and known to be invalid. + if invalidEmailMap[email] { + return ErrInvalidEmailAddress + } + + host := email[i+1:] + if invalidHostMap[host] { + return ErrInvalidEmailAddress + } + return validateHostMX(ctx, host) +} + +func validateHostMX(ctx context.Context, host string) error { + _, err := validateEmailResolver.LookupMX(ctx, host) + if err == nil { + // We had no err, so we treat it as valid. We don't check the mx records + // because RFC 5321 specifies that if an empty list of MX's are returned + // the host should be treated as the MX[1]. + // + // [1] https://www.rfc-editor.org/rfc/rfc5321.html#section-5.1 + return nil + } + + // No names present, we will try to get a positive assertion that the + // domain is not configured to receive email. + var dnsError *net.DNSError + if !errors.As(err, &dnsError) { + // We will be unable to determine with absolute certainy the email was + // invalid so we will err on the side of caution and return nil. + return nil + } + + // The type of err is dnsError, inspect it to see if we can be certain + // the domain has no mx records currently. For this we require that + // the error was not temporary or a timeout. If those are both false + // we trust the value in IsNotFound. + // + // TODO(cstockton): I think that in this case, I need to then lookup the + // host to ensure I'm following the section above. I think that if the + // mx record list is empty Go will set IsNotFound here. + if !dnsError.IsTemporary && !dnsError.IsTimeout && dnsError.IsNotFound { + return ErrInvalidEmailAddress + } + return nil +} diff --git a/internal/mailer/validate_test.go b/internal/mailer/validate_test.go new file mode 100644 index 000000000..a7ed9cb38 --- /dev/null +++ b/internal/mailer/validate_test.go @@ -0,0 +1,92 @@ +package mailer + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestValidateEmail(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithTimeout(ctx, time.Second*60) + defer cancel() + + cases := []struct { + email string + timeout time.Duration + err string + }{ + // valid (has mx record) + {email: "a@supabase.io"}, + {email: "support@supabase.io"}, + {email: "chris.stockton@supabase.io"}, + + // bad format + {email: "", err: "invalid email format"}, + {email: "io", err: "invalid email format"}, + {email: "supabase.io", err: "invalid email format"}, + {email: "@supabase.io", err: "invalid email format"}, + + // invalid: valid mx records, but invalid and often typed + // (invalidEmailMap) + {email: "test@test.com", err: "invalid email address"}, + {email: "test@gmail.com", err: "invalid email address"}, + {email: "test@email.com", err: "invalid email address"}, + + // invalid: valid mx records, but invalid and often typed + // (invalidHostMap) + {email: "a@example.com", err: "invalid email address"}, + {email: "a@example.net", err: "invalid email address"}, + {email: "a@example.org", err: "invalid email address"}, + + // invalid: no mx records + {email: "a@test", err: "invalid email address"}, + {email: "test@local", err: "invalid email address"}, + {email: "test@example", err: "invalid email address"}, + {email: "test@invalid", err: "invalid email address"}, + + // valid but not actually valid and typed a lot + {email: "a@invalid", err: "invalid email address"}, + {email: "test@invalid", err: "invalid email address"}, + + // various invalid emails + {email: "test@test.localhost", err: "invalid email address"}, + {email: "test@invalid.example.com", err: "invalid email address"}, + {email: "test@no.such.email.host.supabase.io", err: "invalid email address"}, + + // this low timeout should simulate a dns timeout, which should + // not be treated as an invalid email. + {email: "test@test.localhost", timeout: time.Millisecond}, + + // likewise for a valid email + {email: "support@supabase.io", timeout: time.Millisecond}, + } + for idx, tc := range cases { + func(timeout time.Duration) { + if timeout == 0 { + timeout = validateEmailTimeout + } + + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + now := time.Now() + err := validateEmail(ctx, tc.email) + dur := time.Since(now) + if max := timeout + (time.Millisecond * 50); max < dur { + t.Fatal("timeout was not respected") + } + + t.Logf("tc #%v - email %v", idx, tc.email) + if tc.err != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.err) + return + } + require.NoError(t, err) + + }(tc.timeout) + } +} From c5f460a5f644c7f1b16cfee21637af072368ec39 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Mon, 25 Nov 2024 19:53:43 -0700 Subject: [PATCH 2/6] fix: attempt to follow spec with host fallback --- internal/mailer/validate.go | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/internal/mailer/validate.go b/internal/mailer/validate.go index a48a29d8b..6b4912e6c 100644 --- a/internal/mailer/validate.go +++ b/internal/mailer/validate.go @@ -20,6 +20,8 @@ var invalidHostMap = map[string]bool{ "test": true, "example": true, "invalid": true, + "local": true, + "localhost": true, "example.com": true, "example.net": true, "example.org": true, @@ -69,18 +71,29 @@ func validateEmail(ctx context.Context, email string) error { if invalidHostMap[host] { return ErrInvalidEmailAddress } - return validateHostMX(ctx, host) + + _, err = validateEmailResolver.LookupMX(ctx, host) + if !isHostNotFound(err) { + return nil + } + + _, err = validateEmailResolver.LookupHost(ctx, host) + if !isHostNotFound(err) { + return nil + } + + // No addrs or mx records were found + return ErrInvalidEmailAddress } -func validateHostMX(ctx context.Context, host string) error { - _, err := validateEmailResolver.LookupMX(ctx, host) +func isHostNotFound(err error) bool { if err == nil { // We had no err, so we treat it as valid. We don't check the mx records // because RFC 5321 specifies that if an empty list of MX's are returned // the host should be treated as the MX[1]. // // [1] https://www.rfc-editor.org/rfc/rfc5321.html#section-5.1 - return nil + return false } // No names present, we will try to get a positive assertion that the @@ -89,7 +102,7 @@ func validateHostMX(ctx context.Context, host string) error { if !errors.As(err, &dnsError) { // We will be unable to determine with absolute certainy the email was // invalid so we will err on the side of caution and return nil. - return nil + return false } // The type of err is dnsError, inspect it to see if we can be certain @@ -101,7 +114,7 @@ func validateHostMX(ctx context.Context, host string) error { // host to ensure I'm following the section above. I think that if the // mx record list is empty Go will set IsNotFound here. if !dnsError.IsTemporary && !dnsError.IsTimeout && dnsError.IsNotFound { - return ErrInvalidEmailAddress + return true } - return nil + return false } From 4a16841e36738383a226d040bb5aab20a15cc252 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Mon, 2 Dec 2024 15:27:45 -0700 Subject: [PATCH 3/6] fix: some final improvements to bounce detection --- internal/mailer/validate.go | 81 +++++++++++++++++++++++++++----- internal/mailer/validate_test.go | 11 ++++- 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/internal/mailer/validate.go b/internal/mailer/validate.go index 6b4912e6c..64461eb2b 100644 --- a/internal/mailer/validate.go +++ b/internal/mailer/validate.go @@ -10,25 +10,50 @@ import ( ) var invalidEmailMap = map[string]bool{ + + // People type these often enough to be special cased. "test@gmail.com": true, - "test@test.com": true, "test@email.com": true, } -// https://www.rfc-editor.org/rfc/rfc2606 +var invalidHostSuffixes = []string{ + + // These are a directly from Section 2 of RFC2606[1]. + // + // [1] https://www.rfc-editor.org/rfc/rfc2606.html#section-2 + ".test", + ".example", + ".invalid", + ".local", + ".localhost", +} + var invalidHostMap = map[string]bool{ - "test": true, - "example": true, - "invalid": true, - "local": true, - "localhost": true, + + // These exist here too for when they are typed as "test@test" + "test": true, + "example": true, + "invalid": true, + "local": true, + "localhost": true, + + // These are commonly typed and have DNS records which cause a + // large enough volume of bounce backs to special case. + "test.com": true, "example.com": true, "example.net": true, "example.org": true, + + // Hundreds of typos per day for this. + "gamil.com": true, + + // These are not email providers, but people often use them. + "anonymous.com": true, + "email.com": true, } const ( - validateEmailTimeout = 250 * time.Millisecond + validateEmailTimeout = 500 * time.Millisecond ) var ( @@ -72,7 +97,40 @@ func validateEmail(ctx context.Context, email string) error { return ErrInvalidEmailAddress } - _, err = validateEmailResolver.LookupMX(ctx, host) + for i := range invalidHostSuffixes { + if strings.HasSuffix(host, invalidHostSuffixes[i]) { + return ErrInvalidEmailAddress + } + } + + name := email[:i] + if err := validateProviders(name, host); err != nil { + return err + } + + if err := validateHost(ctx, host); err != nil { + return err + } + return nil +} + +func validateProviders(name, host string) error { + switch host { + case "gmail.com": + // Based on a sample of internal data, this reduces the number of + // bounced emails by 23%. Gmail documentation specifies that the + // min user name length is 6 characters. There may be some accounts + // from early gmail beta with shorter email addresses, but I think + // this reduces bounce rates enough to be worth adding for now. + if len(name) < 6 { + return ErrInvalidEmailAddress + } + } + return nil +} + +func validateHost(ctx context.Context, host string) error { + _, err := validateEmailResolver.LookupMX(ctx, host) if !isHostNotFound(err) { return nil } @@ -92,6 +150,7 @@ func isHostNotFound(err error) bool { // because RFC 5321 specifies that if an empty list of MX's are returned // the host should be treated as the MX[1]. // + // See section 2 and 3 of: https://www.rfc-editor.org/rfc/rfc2606 // [1] https://www.rfc-editor.org/rfc/rfc5321.html#section-5.1 return false } @@ -109,10 +168,6 @@ func isHostNotFound(err error) bool { // the domain has no mx records currently. For this we require that // the error was not temporary or a timeout. If those are both false // we trust the value in IsNotFound. - // - // TODO(cstockton): I think that in this case, I need to then lookup the - // host to ensure I'm following the section above. I think that if the - // mx record list is empty Go will set IsNotFound here. if !dnsError.IsTemporary && !dnsError.IsTimeout && dnsError.IsNotFound { return true } diff --git a/internal/mailer/validate_test.go b/internal/mailer/validate_test.go index a7ed9cb38..4c378c025 100644 --- a/internal/mailer/validate_test.go +++ b/internal/mailer/validate_test.go @@ -28,6 +28,7 @@ func TestValidateEmail(t *testing.T) { {email: "io", err: "invalid email format"}, {email: "supabase.io", err: "invalid email format"}, {email: "@supabase.io", err: "invalid email format"}, + {email: "test@.supabase.io", err: "invalid email format"}, // invalid: valid mx records, but invalid and often typed // (invalidEmailMap) @@ -35,6 +36,9 @@ func TestValidateEmail(t *testing.T) { {email: "test@gmail.com", err: "invalid email address"}, {email: "test@email.com", err: "invalid email address"}, + // very common typo + {email: "test@gamil.com", err: "invalid email address"}, + // invalid: valid mx records, but invalid and often typed // (invalidHostMap) {email: "a@example.com", err: "invalid email address"}, @@ -44,11 +48,13 @@ func TestValidateEmail(t *testing.T) { // invalid: no mx records {email: "a@test", err: "invalid email address"}, {email: "test@local", err: "invalid email address"}, + {email: "test@test.local", err: "invalid email address"}, {email: "test@example", err: "invalid email address"}, {email: "test@invalid", err: "invalid email address"}, // valid but not actually valid and typed a lot {email: "a@invalid", err: "invalid email address"}, + {email: "a@a.invalid", err: "invalid email address"}, {email: "test@invalid", err: "invalid email address"}, // various invalid emails @@ -58,7 +64,8 @@ func TestValidateEmail(t *testing.T) { // this low timeout should simulate a dns timeout, which should // not be treated as an invalid email. - {email: "test@test.localhost", timeout: time.Millisecond}, + {email: "validemail@probablyaaaaaaaanotarealdomain.com", + timeout: time.Millisecond}, // likewise for a valid email {email: "support@supabase.io", timeout: time.Millisecond}, @@ -79,7 +86,7 @@ func TestValidateEmail(t *testing.T) { t.Fatal("timeout was not respected") } - t.Logf("tc #%v - email %v", idx, tc.email) + t.Logf("tc #%v - email %q", idx, tc.email) if tc.err != "" { require.Error(t, err) require.Contains(t, err.Error(), tc.err) From 2d1fcc10948e39acc45ff763f169a302c6e0afd5 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Wed, 4 Dec 2024 17:34:36 -0700 Subject: [PATCH 4/6] fix: add email validation service support --- internal/api/errorcodes.go | 1 + internal/api/mail.go | 30 +++-- internal/conf/configuration.go | 5 + internal/mailer/mailer.go | 24 ++-- internal/mailer/mailme.go | 38 ++++-- internal/mailer/noop.go | 18 ++- internal/mailer/template.go | 49 ++++---- internal/mailer/validate.go | 159 ++++++++++++++++++++---- internal/mailer/validate_test.go | 207 +++++++++++++++++++++++++++---- 9 files changed, 429 insertions(+), 102 deletions(-) diff --git a/internal/api/errorcodes.go b/internal/api/errorcodes.go index 334a28be3..8f09901d9 100644 --- a/internal/api/errorcodes.go +++ b/internal/api/errorcodes.go @@ -91,4 +91,5 @@ const ( //#nosec G101 -- Not a secret value. ErrorCodeInvalidCredentials ErrorCode = "invalid_credentials" ErrorCodeEmailAddressNotAuthorized ErrorCode = "email_address_not_authorized" + ErrorCodeEmailAddressInvalid ErrorCode = "email_address_invalid" ) diff --git a/internal/api/mail.go b/internal/api/mail.go index 80d74f679..8a8019363 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -7,6 +7,7 @@ import ( "time" "github.com/supabase/auth/internal/hooks" + "github.com/supabase/auth/internal/mailer" mail "github.com/supabase/auth/internal/mailer" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" @@ -601,7 +602,6 @@ func (a *API) checkEmailAddressAuthorization(email string) bool { } func (a *API) sendEmail(r *http.Request, tx *storage.Connection, u *models.User, emailActionType, otp, otpNew, tokenHashWithPrefix string) error { - mailer := a.Mailer() ctx := r.Context() config := a.config referrerURL := utilities.GetReferrer(r, config) @@ -675,20 +675,34 @@ func (a *API) sendEmail(r *http.Request, tx *storage.Connection, u *models.User, return a.invokeHook(tx, r, &input, &output) } + mr := a.Mailer() + var err error switch emailActionType { case mail.SignupVerification: - return mailer.ConfirmationMail(r, u, otp, referrerURL, externalURL) + err = mr.ConfirmationMail(r, u, otp, referrerURL, externalURL) case mail.MagicLinkVerification: - return mailer.MagicLinkMail(r, u, otp, referrerURL, externalURL) + err = mr.MagicLinkMail(r, u, otp, referrerURL, externalURL) case mail.ReauthenticationVerification: - return mailer.ReauthenticateMail(r, u, otp) + err = mr.ReauthenticateMail(r, u, otp) case mail.RecoveryVerification: - return mailer.RecoveryMail(r, u, otp, referrerURL, externalURL) + err = mr.RecoveryMail(r, u, otp, referrerURL, externalURL) case mail.InviteVerification: - return mailer.InviteMail(r, u, otp, referrerURL, externalURL) + err = mr.InviteMail(r, u, otp, referrerURL, externalURL) case mail.EmailChangeVerification: - return mailer.EmailChangeMail(r, u, otpNew, otp, referrerURL, externalURL) + err = mr.EmailChangeMail(r, u, otpNew, otp, referrerURL, externalURL) + default: + err = errors.New("invalid email action type") + } + + switch { + case errors.Is(err, mailer.ErrInvalidEmailAddress), + errors.Is(err, mailer.ErrInvalidEmailFormat), + errors.Is(err, mailer.ErrInvalidEmailDNS): + return badRequestError( + ErrorCodeEmailAddressInvalid, + "Email address %q is invalid", + u.GetEmail()) default: - return errors.New("invalid email action type") + return err } } diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 5d5060523..f6d34c9e6 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -399,6 +399,11 @@ type MailerConfiguration struct { OtpLength int `json:"otp_length" split_words:"true"` ExternalHosts []string `json:"external_hosts" split_words:"true"` + + // EXPERIMENTAL: May be removed in a future release. + EmailValidationExtended bool `json:"email_validation_extended" split_words:"true" default:"false"` + EmailValidationServiceURL string `json:"email_validation_service_url" split_words:"true"` + EmailValidationServiceKey string `json:"email_validation_service_key" split_words:"true"` } type PhoneProviderConfiguration struct { diff --git a/internal/mailer/mailer.go b/internal/mailer/mailer.go index 8dabe4a56..1499960f5 100644 --- a/internal/mailer/mailer.go +++ b/internal/mailer/mailer.go @@ -18,7 +18,6 @@ type Mailer interface { MagicLinkMail(r *http.Request, user *models.User, otp, referrerURL string, externalURL *url.URL) error EmailChangeMail(r *http.Request, user *models.User, otpNew, otpCurrent, referrerURL string, externalURL *url.URL) error ReauthenticateMail(r *http.Request, user *models.User, otp string) error - ValidateEmail(email string) error GetEmailActionLink(user *models.User, actionType, referrerURL string, externalURL *url.URL) (string, error) } @@ -46,18 +45,21 @@ func NewMailer(globalConfig *conf.GlobalConfiguration) Mailer { var mailClient MailClient if globalConfig.SMTP.Host == "" { logrus.Infof("Noop mail client being used for %v", globalConfig.SiteURL) - mailClient = &noopMailClient{} + mailClient = &noopMailClient{ + EmailValidator: newEmailValidator(globalConfig.Mailer), + } } else { mailClient = &MailmeMailer{ - Host: globalConfig.SMTP.Host, - Port: globalConfig.SMTP.Port, - User: globalConfig.SMTP.User, - Pass: globalConfig.SMTP.Pass, - LocalName: u.Hostname(), - From: from, - BaseURL: globalConfig.SiteURL, - Logger: logrus.StandardLogger(), - MailLogging: globalConfig.SMTP.LoggingEnabled, + Host: globalConfig.SMTP.Host, + Port: globalConfig.SMTP.Port, + User: globalConfig.SMTP.User, + Pass: globalConfig.SMTP.Pass, + LocalName: u.Hostname(), + From: from, + BaseURL: globalConfig.SiteURL, + Logger: logrus.StandardLogger(), + MailLogging: globalConfig.SMTP.LoggingEnabled, + EmailValidator: newEmailValidator(globalConfig.Mailer), } } diff --git a/internal/mailer/mailme.go b/internal/mailer/mailme.go index 4a27f2173..20ff17740 100644 --- a/internal/mailer/mailme.go +++ b/internal/mailer/mailme.go @@ -2,6 +2,7 @@ package mailer import ( "bytes" + "context" "errors" "html/template" "io" @@ -24,22 +25,29 @@ const TemplateExpiration = 10 * time.Second // MailmeMailer lets MailMe send templated mails type MailmeMailer struct { - From string - Host string - Port int - User string - Pass string - BaseURL string - LocalName string - FuncMap template.FuncMap - cache *TemplateCache - Logger logrus.FieldLogger - MailLogging bool + From string + Host string + Port int + User string + Pass string + BaseURL string + LocalName string + FuncMap template.FuncMap + cache *TemplateCache + Logger logrus.FieldLogger + MailLogging bool + EmailValidator *EmailValidator } // Mail sends a templated mail. It will try to load the template from a URL, and // otherwise fall back to the default -func (m *MailmeMailer) Mail(to, subjectTemplate, templateURL, defaultTemplate string, templateData map[string]interface{}, headers map[string][]string, typ string) error { +func (m *MailmeMailer) Mail( + ctx context.Context, + to, subjectTemplate, templateURL, defaultTemplate string, + templateData map[string]interface{}, + headers map[string][]string, + typ string, +) error { if m.FuncMap == nil { m.FuncMap = map[string]interface{}{} } @@ -51,6 +59,12 @@ func (m *MailmeMailer) Mail(to, subjectTemplate, templateURL, defaultTemplate st } } + if m.EmailValidator != nil { + if err := m.EmailValidator.Validate(ctx, to); err != nil { + return err + } + } + tmp, err := template.New("Subject").Funcs(template.FuncMap(m.FuncMap)).Parse(subjectTemplate) if err != nil { return err diff --git a/internal/mailer/noop.go b/internal/mailer/noop.go index 17151a877..0e0e3bfcb 100644 --- a/internal/mailer/noop.go +++ b/internal/mailer/noop.go @@ -1,14 +1,28 @@ package mailer import ( + "context" "errors" ) -type noopMailClient struct{} +type noopMailClient struct { + EmailValidator *EmailValidator +} -func (m *noopMailClient) Mail(to, subjectTemplate, templateURL, defaultTemplate string, templateData map[string]interface{}, headers map[string][]string, typ string) error { +func (m *noopMailClient) Mail( + ctx context.Context, + to, subjectTemplate, templateURL, defaultTemplate string, + templateData map[string]interface{}, + headers map[string][]string, + typ string, +) error { if to == "" { return errors.New("to field cannot be empty") } + if m.EmailValidator != nil { + if err := m.EmailValidator.Validate(ctx, to); err != nil { + return err + } + } return nil } diff --git a/internal/mailer/template.go b/internal/mailer/template.go index 1075d4aa0..b150165c4 100644 --- a/internal/mailer/template.go +++ b/internal/mailer/template.go @@ -1,18 +1,37 @@ package mailer import ( + "context" "fmt" "net/http" "net/url" "strings" - "github.com/badoux/checkmail" "github.com/supabase/auth/internal/conf" "github.com/supabase/auth/internal/models" ) +type MailRequest struct { + To string + SubjectTemplate string + TemplateURL string + DefaultTemplate string + TemplateData map[string]interface{} + Headers map[string][]string + Type string +} + type MailClient interface { - Mail(string, string, string, string, map[string]interface{}, map[string][]string, string) error + Mail( + ctx context.Context, + to string, + subjectTemplate string, + templateURL string, + defaultTemplate string, + templateData map[string]interface{}, + headers map[string][]string, + typ string, + ) error } // TemplateMailer will send mail and use templates from the site for easy mail styling @@ -81,12 +100,6 @@ const defaultReauthenticateMail = `

Confirm reauthentication

Enter the code: {{ .Token }}

` -// ValidateEmail returns nil if the email is valid, -// otherwise an error indicating the reason it is invalid -func (m TemplateMailer) ValidateEmail(email string) error { - return checkmail.ValidateFormat(email) -} - func (m *TemplateMailer) Headers(messageType string) map[string][]string { originalHeaders := m.Config.SMTP.NormalizedHeaders() @@ -145,6 +158,7 @@ func (m *TemplateMailer) InviteMail(r *http.Request, user *models.User, otp, ref } return m.Mailer.Mail( + r.Context(), user.GetEmail(), withDefault(m.Config.Mailer.Subjects.Invite, "You have been invited"), m.Config.Mailer.Templates.Invite, @@ -177,6 +191,7 @@ func (m *TemplateMailer) ConfirmationMail(r *http.Request, user *models.User, ot } return m.Mailer.Mail( + r.Context(), user.GetEmail(), withDefault(m.Config.Mailer.Subjects.Confirmation, "Confirm Your Email"), m.Config.Mailer.Templates.Confirmation, @@ -197,6 +212,7 @@ func (m *TemplateMailer) ReauthenticateMail(r *http.Request, user *models.User, } return m.Mailer.Mail( + r.Context(), user.GetEmail(), withDefault(m.Config.Mailer.Subjects.Reauthentication, "Confirm reauthentication"), m.Config.Mailer.Templates.Reauthentication, @@ -263,6 +279,7 @@ func (m *TemplateMailer) EmailChangeMail(r *http.Request, user *models.User, otp "RedirectTo": referrerURL, } errors <- m.Mailer.Mail( + r.Context(), address, withDefault(m.Config.Mailer.Subjects.EmailChange, "Confirm Email Change"), template, @@ -280,7 +297,6 @@ func (m *TemplateMailer) EmailChangeMail(r *http.Request, user *models.User, otp return e } } - return nil } @@ -305,6 +321,7 @@ func (m *TemplateMailer) RecoveryMail(r *http.Request, user *models.User, otp, r } return m.Mailer.Mail( + r.Context(), user.GetEmail(), withDefault(m.Config.Mailer.Subjects.Recovery, "Reset Your Password"), m.Config.Mailer.Templates.Recovery, @@ -337,6 +354,7 @@ func (m *TemplateMailer) MagicLinkMail(r *http.Request, user *models.User, otp, } return m.Mailer.Mail( + r.Context(), user.GetEmail(), withDefault(m.Config.Mailer.Subjects.MagicLink, "Your Magic Link"), m.Config.Mailer.Templates.MagicLink, @@ -347,19 +365,6 @@ func (m *TemplateMailer) MagicLinkMail(r *http.Request, user *models.User, otp, ) } -// Send can be used to send one-off emails to users -func (m TemplateMailer) Send(user *models.User, subject, body string, data map[string]interface{}) error { - return m.Mailer.Mail( - user.GetEmail(), - subject, - "", - body, - data, - m.Headers("other"), - "other", - ) -} - // GetEmailActionLink returns a magiclink, recovery or invite link based on the actionType passed. func (m TemplateMailer) GetEmailActionLink(user *models.User, actionType, referrerURL string, externalURL *url.URL) (string, error) { var err error diff --git a/internal/mailer/validate.go b/internal/mailer/validate.go index 64461eb2b..3d20f6f22 100644 --- a/internal/mailer/validate.go +++ b/internal/mailer/validate.go @@ -1,19 +1,28 @@ package mailer import ( + "bytes" "context" + "encoding/json" "errors" + "io" "net" + "net/http" "net/mail" "strings" "time" + + "github.com/supabase/auth/internal/conf" + "golang.org/x/sync/errgroup" ) var invalidEmailMap = map[string]bool{ // People type these often enough to be special cased. - "test@gmail.com": true, - "test@email.com": true, + "test@gmail.com": true, + "example@gmail.com": true, + "someone@gmail.com": true, + "test@email.com": true, } var invalidHostSuffixes = []string{ @@ -53,7 +62,7 @@ var invalidHostMap = map[string]bool{ } const ( - validateEmailTimeout = 500 * time.Millisecond + validateEmailTimeout = 2 * time.Second ) var ( @@ -62,59 +71,165 @@ var ( ) var ( - ErrInvalidEmailFormat = errors.New("invalid email format") - ErrInvalidEmailAddress = errors.New("invalid email address") + ErrInvalidEmailAddress = errors.New("invalid_email_address") + ErrInvalidEmailFormat = errors.New("invalid_email_format") + ErrInvalidEmailDNS = errors.New("invalid_email_dns") ) -// ValidateEmail returns a nil error in all cases but the following: +type EmailValidator struct { + extended bool + serviceURL string + serviceKey string +} + +func newEmailValidator(mc conf.MailerConfiguration) *EmailValidator { + return &EmailValidator{ + extended: mc.EmailValidationExtended, + serviceURL: mc.EmailValidationServiceURL, + serviceKey: mc.EmailValidationServiceKey, + } +} + +func (ev *EmailValidator) isExtendedDisabled() bool { return !ev.extended } +func (ev *EmailValidator) isServiceDisabled() bool { + return ev.serviceURL == "" || ev.serviceKey == "" +} + +// Validate performs validation on the given email. +// +// When extended is true, returns a nil error in all cases but the following: // - `email` cannot be parsed by mail.ParseAddress // - `email` has a domain with no DNS configured -func ValidateEmail(ctx context.Context, email string) error { +// +// When serviceURL AND serviceKey are non-empty strings it uses the remote +// service to determine if the email is valid. +func (ev *EmailValidator) Validate(ctx context.Context, email string) error { + if ev.isExtendedDisabled() && ev.isServiceDisabled() { + return nil + } + + // One of the two validation methods are enabled, set a timeout. ctx, cancel := context.WithTimeout(ctx, validateEmailTimeout) defer cancel() - return validateEmail(ctx, email) + // Easier control flow here to always use errgroup, it has very little + // overhad in comparison to the network calls it makes. The reason + // we run both checks concurrently is to tighten the timeout without + // potentially missing a call to the validation service due to a + // dns timeout or something more nefarious like a honeypot dns entry. + g := new(errgroup.Group) + + // Validate the static rules first to prevent round trips on bad emails + // and to parse the host ahead of time. + if !ev.isExtendedDisabled() { + + // First validate static checks such as format, known invalid hosts + // and any other network free checks. Running this check before we + // call the service will help reduce the number of calls with known + // invalid emails. + host, err := ev.validateStatic(email) + if err != nil { + return err + } + + // Start the goroutine to validate the host. + g.Go(func() error { return ev.validateHost(ctx, host) }) + } + + // If the service check is not disabled we start a goroutine to run + // that check as well. + if !ev.isServiceDisabled() { + g.Go(func() error { return ev.validateService(ctx, email) }) + } + return g.Wait() } -func validateEmail(ctx context.Context, email string) error { +// validateStatic will validate the format and do the static checks before +// returning the host portion of the email. +func (ev *EmailValidator) validateStatic(email string) (string, error) { + if !ev.extended { + return "", nil + } + ea, err := mail.ParseAddress(email) if err != nil { - return ErrInvalidEmailFormat + return "", ErrInvalidEmailFormat } i := strings.LastIndex(ea.Address, "@") if i == -1 { - return ErrInvalidEmailFormat + return "", ErrInvalidEmailFormat } // few static lookups that are typed constantly and known to be invalid. if invalidEmailMap[email] { - return ErrInvalidEmailAddress + return "", ErrInvalidEmailAddress } host := email[i+1:] if invalidHostMap[host] { - return ErrInvalidEmailAddress + return "", ErrInvalidEmailDNS } for i := range invalidHostSuffixes { if strings.HasSuffix(host, invalidHostSuffixes[i]) { - return ErrInvalidEmailAddress + return "", ErrInvalidEmailDNS } } name := email[:i] - if err := validateProviders(name, host); err != nil { - return err + if err := ev.validateProviders(name, host); err != nil { + return "", err + } + return host, nil +} + +func (ev *EmailValidator) validateService(ctx context.Context, email string) error { + if ev.isServiceDisabled() { + return nil } - if err := validateHost(ctx, host); err != nil { - return err + reqObject := struct { + EmailAddress string `json:"email"` + }{email} + + reqData, err := json.Marshal(&reqObject) + if err != nil { + return nil } - return nil + + rdr := bytes.NewReader(reqData) + req, err := http.NewRequestWithContext(ctx, "GET", ev.serviceURL, rdr) + if err != nil { + return nil + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("apikey", ev.serviceKey) + + res, err := http.DefaultClient.Do(req) + if err != nil { + return nil + } + defer res.Body.Close() + + resObject := struct { + Valid *bool `json:"valid"` + }{} + dec := json.NewDecoder(io.LimitReader(res.Body, 1<<5)) + if err := dec.Decode(&resObject); err != nil { + return nil + } + + // If the object did not contain a valid key we consider the check as + // failed. We _must_ get a valid JSON response with a "valid" field. + if resObject.Valid == nil || *resObject.Valid { + return nil + } + + return ErrInvalidEmailAddress } -func validateProviders(name, host string) error { +func (ev *EmailValidator) validateProviders(name, host string) error { switch host { case "gmail.com": // Based on a sample of internal data, this reduces the number of @@ -129,7 +244,7 @@ func validateProviders(name, host string) error { return nil } -func validateHost(ctx context.Context, host string) error { +func (ev *EmailValidator) validateHost(ctx context.Context, host string) error { _, err := validateEmailResolver.LookupMX(ctx, host) if !isHostNotFound(err) { return nil @@ -141,7 +256,7 @@ func validateHost(ctx context.Context, host string) error { } // No addrs or mx records were found - return ErrInvalidEmailAddress + return ErrInvalidEmailDNS } func isHostNotFound(err error) bool { diff --git a/internal/mailer/validate_test.go b/internal/mailer/validate_test.go index 4c378c025..428605ce7 100644 --- a/internal/mailer/validate_test.go +++ b/internal/mailer/validate_test.go @@ -2,13 +2,162 @@ package mailer import ( "context" + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" "testing" "time" "github.com/stretchr/testify/require" + "github.com/supabase/auth/internal/conf" ) -func TestValidateEmail(t *testing.T) { +func TestEmalValidatorService(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithTimeout(ctx, time.Second*60) + defer cancel() + + testResVal := new(atomic.Value) + testResVal.Store(`{"valid": true}`) + + testHdrsVal := new(atomic.Value) + testHdrsVal.Store(map[string]string{"apikey": "test"}) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + key := r.Header.Get("apikey") + if key == "" { + fmt.Fprintln(w, `{"error": true}`) + return + } + + fmt.Fprintln(w, testResVal.Load().(string)) + })) + defer ts.Close() + + // Return nil err from service + // when svc and extended checks both report email as valid + { + testResVal.Store(`{"valid": true}`) + cfg := conf.MailerConfiguration{ + EmailValidationExtended: true, + EmailValidationServiceURL: ts.URL, + EmailValidationServiceKey: "test", + } + ev := newEmailValidator(cfg) + err := ev.Validate(ctx, "chris.stockton@supabase.io") + if err != nil { + t.Fatalf("exp nil err; got %v", err) + } + } + + // Return nil err from service when + // extended is disabled for a known invalid address + // service reports valid + { + testResVal.Store(`{"valid": true}`) + + cfg := conf.MailerConfiguration{ + EmailValidationExtended: false, + EmailValidationServiceURL: ts.URL, + EmailValidationServiceKey: "test", + } + ev := newEmailValidator(cfg) + err := ev.Validate(ctx, "test@gmail.com") + if err != nil { + t.Fatalf("exp nil err; got %v", err) + } + } + + // Return nil err from service when + // extended is disabled for a known invalid address + // service is disabled for a known invalid address + { + testResVal.Store(`{"valid": false}`) + + cfg := conf.MailerConfiguration{ + EmailValidationExtended: false, + EmailValidationServiceURL: "", + EmailValidationServiceKey: "", + } + ev := newEmailValidator(cfg) + err := ev.Validate(ctx, "test@gmail.com") + if err != nil { + t.Fatalf("exp nil err; got %v", err) + } + } + + // Return err from service when + // extended reports invalid + // service is disabled for a known invalid address + { + testResVal.Store(`{"valid": true}`) + cfg := conf.MailerConfiguration{ + EmailValidationExtended: true, + EmailValidationServiceURL: "", + EmailValidationServiceKey: "", + } + ev := newEmailValidator(cfg) + err := ev.Validate(ctx, "test@gmail.com") + if err == nil { + t.Fatal("exp non-nil err") + } + } + + // Return err from service when + // extended reports invalid + // service reports valid + { + testResVal.Store(`{"valid": true}`) + cfg := conf.MailerConfiguration{ + EmailValidationExtended: true, + EmailValidationServiceURL: ts.URL, + EmailValidationServiceKey: "test", + } + ev := newEmailValidator(cfg) + err := ev.Validate(ctx, "test@gmail.com") + if err == nil { + t.Fatal("exp non-nil err") + } + } + + // Return err from service when + // extended reports valid + // service reports invalid + { + testResVal.Store(`{"valid": false}`) + cfg := conf.MailerConfiguration{ + EmailValidationExtended: true, + EmailValidationServiceURL: ts.URL, + EmailValidationServiceKey: "test", + } + ev := newEmailValidator(cfg) + err := ev.Validate(ctx, "chris.stockton@supabase.io") + if err == nil { + t.Fatal("exp non-nil err") + } + } + + // Return err from service when + // extended reports invalid + // service reports invalid + { + testResVal.Store(`{"valid": false}`) + + cfg := conf.MailerConfiguration{ + EmailValidationExtended: false, + EmailValidationServiceURL: ts.URL, + EmailValidationServiceKey: "test", + } + ev := newEmailValidator(cfg) + err := ev.Validate(ctx, "test@gmail.com") + if err == nil { + t.Fatal("exp non-nil err") + } + } +} + +func TestValidateEmailExtended(t *testing.T) { ctx := context.Background() ctx, cancel := context.WithTimeout(ctx, time.Second*60) defer cancel() @@ -24,43 +173,43 @@ func TestValidateEmail(t *testing.T) { {email: "chris.stockton@supabase.io"}, // bad format - {email: "", err: "invalid email format"}, - {email: "io", err: "invalid email format"}, - {email: "supabase.io", err: "invalid email format"}, - {email: "@supabase.io", err: "invalid email format"}, - {email: "test@.supabase.io", err: "invalid email format"}, + {email: "", err: "invalid_email_format"}, + {email: "io", err: "invalid_email_format"}, + {email: "supabase.io", err: "invalid_email_format"}, + {email: "@supabase.io", err: "invalid_email_format"}, + {email: "test@.supabase.io", err: "invalid_email_format"}, // invalid: valid mx records, but invalid and often typed // (invalidEmailMap) - {email: "test@test.com", err: "invalid email address"}, - {email: "test@gmail.com", err: "invalid email address"}, - {email: "test@email.com", err: "invalid email address"}, + {email: "test@email.com", err: "invalid_email_address"}, + {email: "test@gmail.com", err: "invalid_email_address"}, + {email: "test@test.com", err: "invalid_email_dns"}, // very common typo - {email: "test@gamil.com", err: "invalid email address"}, + {email: "test@gamil.com", err: "invalid_email_dns"}, // invalid: valid mx records, but invalid and often typed // (invalidHostMap) - {email: "a@example.com", err: "invalid email address"}, - {email: "a@example.net", err: "invalid email address"}, - {email: "a@example.org", err: "invalid email address"}, + {email: "a@example.com", err: "invalid_email_dns"}, + {email: "a@example.net", err: "invalid_email_dns"}, + {email: "a@example.org", err: "invalid_email_dns"}, // invalid: no mx records - {email: "a@test", err: "invalid email address"}, - {email: "test@local", err: "invalid email address"}, - {email: "test@test.local", err: "invalid email address"}, - {email: "test@example", err: "invalid email address"}, - {email: "test@invalid", err: "invalid email address"}, + {email: "a@test", err: "invalid_email_dns"}, + {email: "test@local", err: "invalid_email_dns"}, + {email: "test@test.local", err: "invalid_email_dns"}, + {email: "test@example", err: "invalid_email_dns"}, + {email: "test@invalid", err: "invalid_email_dns"}, // valid but not actually valid and typed a lot - {email: "a@invalid", err: "invalid email address"}, - {email: "a@a.invalid", err: "invalid email address"}, - {email: "test@invalid", err: "invalid email address"}, + {email: "a@invalid", err: "invalid_email_dns"}, + {email: "a@a.invalid", err: "invalid_email_dns"}, + {email: "test@invalid", err: "invalid_email_dns"}, // various invalid emails - {email: "test@test.localhost", err: "invalid email address"}, - {email: "test@invalid.example.com", err: "invalid email address"}, - {email: "test@no.such.email.host.supabase.io", err: "invalid email address"}, + {email: "test@test.localhost", err: "invalid_email_dns"}, + {email: "test@invalid.example.com", err: "invalid_email_dns"}, + {email: "test@no.such.email.host.supabase.io", err: "invalid_email_dns"}, // this low timeout should simulate a dns timeout, which should // not be treated as an invalid email. @@ -70,6 +219,14 @@ func TestValidateEmail(t *testing.T) { // likewise for a valid email {email: "support@supabase.io", timeout: time.Millisecond}, } + + cfg := conf.MailerConfiguration{ + EmailValidationExtended: true, + EmailValidationServiceURL: "", + EmailValidationServiceKey: "", + } + ev := newEmailValidator(cfg) + for idx, tc := range cases { func(timeout time.Duration) { if timeout == 0 { @@ -80,7 +237,7 @@ func TestValidateEmail(t *testing.T) { defer cancel() now := time.Now() - err := validateEmail(ctx, tc.email) + err := ev.Validate(ctx, tc.email) dur := time.Since(now) if max := timeout + (time.Millisecond * 50); max < dur { t.Fatal("timeout was not respected") From 42ed14acdb5d2a9a537ce826c52d19c1e30e265d Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Wed, 4 Dec 2024 17:44:46 -0700 Subject: [PATCH 5/6] fix: double import on aliased mailer / mail --- internal/api/mail.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/api/mail.go b/internal/api/mail.go index 8a8019363..b1492dffb 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -7,7 +7,6 @@ import ( "time" "github.com/supabase/auth/internal/hooks" - "github.com/supabase/auth/internal/mailer" mail "github.com/supabase/auth/internal/mailer" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" @@ -695,9 +694,9 @@ func (a *API) sendEmail(r *http.Request, tx *storage.Connection, u *models.User, } switch { - case errors.Is(err, mailer.ErrInvalidEmailAddress), - errors.Is(err, mailer.ErrInvalidEmailFormat), - errors.Is(err, mailer.ErrInvalidEmailDNS): + case errors.Is(err, mail.ErrInvalidEmailAddress), + errors.Is(err, mail.ErrInvalidEmailFormat), + errors.Is(err, mail.ErrInvalidEmailDNS): return badRequestError( ErrorCodeEmailAddressInvalid, "Email address %q is invalid", From f51f1f8c1ba6291be835f398a97a99edab54eeec Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Thu, 5 Dec 2024 15:17:54 -0700 Subject: [PATCH 6/6] fix: some minor fixes and cleanup --- internal/conf/configuration.go | 29 +++++++++-- internal/conf/configuration_test.go | 7 +++ internal/mailer/template.go | 7 ++- internal/mailer/validate.go | 43 +++++++++------- internal/mailer/validate_test.go | 79 ++++++++++++++++++++--------- 5 files changed, 118 insertions(+), 47 deletions(-) diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index f6d34c9e6..390cb29e4 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -401,9 +401,31 @@ type MailerConfiguration struct { ExternalHosts []string `json:"external_hosts" split_words:"true"` // EXPERIMENTAL: May be removed in a future release. - EmailValidationExtended bool `json:"email_validation_extended" split_words:"true" default:"false"` - EmailValidationServiceURL string `json:"email_validation_service_url" split_words:"true"` - EmailValidationServiceKey string `json:"email_validation_service_key" split_words:"true"` + EmailValidationExtended bool `json:"email_validation_extended" split_words:"true" default:"false"` + EmailValidationServiceURL string `json:"email_validation_service_url" split_words:"true"` + EmailValidationServiceHeaders string `json:"email_validation_service_key" split_words:"true"` + + serviceHeaders map[string][]string `json:"-"` +} + +func (c *MailerConfiguration) Validate() error { + headers := make(map[string][]string) + + if c.EmailValidationServiceHeaders != "" { + err := json.Unmarshal([]byte(c.EmailValidationServiceHeaders), &headers) + if err != nil { + return fmt.Errorf("conf: SMTP headers not a map[string][]string format: %w", err) + } + } + + if len(headers) > 0 { + c.serviceHeaders = headers + } + return nil +} + +func (c *MailerConfiguration) GetEmailValidationServiceHeaders() map[string][]string { + return c.serviceHeaders } type PhoneProviderConfiguration struct { @@ -1025,6 +1047,7 @@ func (c *GlobalConfiguration) Validate() error { &c.Tracing, &c.Metrics, &c.SMTP, + &c.Mailer, &c.SAML, &c.Security, &c.Sessions, diff --git a/internal/conf/configuration_test.go b/internal/conf/configuration_test.go index d139e1ea3..c03f95495 100644 --- a/internal/conf/configuration_test.go +++ b/internal/conf/configuration_test.go @@ -25,6 +25,7 @@ func TestGlobal(t *testing.T) { os.Setenv("GOTRUE_HOOK_MFA_VERIFICATION_ATTEMPT_URI", "pg-functions://postgres/auth/count_failed_attempts") os.Setenv("GOTRUE_HOOK_SEND_SMS_SECRETS", "v1,whsec_aWxpa2VzdXBhYmFzZXZlcnltdWNoYW5kaWhvcGV5b3Vkb3Rvbw==") os.Setenv("GOTRUE_SMTP_HEADERS", `{"X-PM-Metadata-project-ref":["project_ref"],"X-SES-Message-Tags":["ses:feedback-id-a=project_ref,ses:feedback-id-b=$messageType"]}`) + os.Setenv("GOTRUE_MAILER_EMAIL_VALIDATION_SERVICE_HEADERS", `{"apikey":["test"]}`) os.Setenv("GOTRUE_SMTP_LOGGING_ENABLED", "true") gc, err := LoadGlobal("") require.NoError(t, err) @@ -34,6 +35,12 @@ func TestGlobal(t *testing.T) { assert.Equal(t, "X-Request-ID", gc.API.RequestIDHeader) assert.Equal(t, "pg-functions://postgres/auth/count_failed_attempts", gc.Hook.MFAVerificationAttempt.URI) + { + hdrs := gc.Mailer.GetEmailValidationServiceHeaders() + assert.Equal(t, 1, len(hdrs["apikey"])) + assert.Equal(t, "test", hdrs["apikey"][0]) + } + } func TestRateLimits(t *testing.T) { diff --git a/internal/mailer/template.go b/internal/mailer/template.go index b150165c4..59a485450 100644 --- a/internal/mailer/template.go +++ b/internal/mailer/template.go @@ -253,7 +253,10 @@ func (m *TemplateMailer) EmailChangeMail(r *http.Request, user *models.User, otp }) } - errors := make(chan error) + ctx, cancel := context.WithCancel(r.Context()) + defer cancel() + + errors := make(chan error, len(emails)) for _, email := range emails { path, err := getPath( m.Config.Mailer.URLPaths.EmailChange, @@ -279,7 +282,7 @@ func (m *TemplateMailer) EmailChangeMail(r *http.Request, user *models.User, otp "RedirectTo": referrerURL, } errors <- m.Mailer.Mail( - r.Context(), + ctx, address, withDefault(m.Config.Mailer.Subjects.EmailChange, "Confirm Email Change"), template, diff --git a/internal/mailer/validate.go b/internal/mailer/validate.go index 3d20f6f22..b18a08769 100644 --- a/internal/mailer/validate.go +++ b/internal/mailer/validate.go @@ -62,7 +62,7 @@ var invalidHostMap = map[string]bool{ } const ( - validateEmailTimeout = 2 * time.Second + validateEmailTimeout = 3 * time.Second ) var ( @@ -77,23 +77,21 @@ var ( ) type EmailValidator struct { - extended bool - serviceURL string - serviceKey string + extended bool + serviceURL string + serviceHeaders map[string][]string } func newEmailValidator(mc conf.MailerConfiguration) *EmailValidator { return &EmailValidator{ - extended: mc.EmailValidationExtended, - serviceURL: mc.EmailValidationServiceURL, - serviceKey: mc.EmailValidationServiceKey, + extended: mc.EmailValidationExtended, + serviceURL: mc.EmailValidationServiceURL, + serviceHeaders: mc.GetEmailValidationServiceHeaders(), } } -func (ev *EmailValidator) isExtendedDisabled() bool { return !ev.extended } -func (ev *EmailValidator) isServiceDisabled() bool { - return ev.serviceURL == "" || ev.serviceKey == "" -} +func (ev *EmailValidator) isExtendedEnabled() bool { return ev.extended } +func (ev *EmailValidator) isServiceEnabled() bool { return ev.serviceURL != "" } // Validate performs validation on the given email. // @@ -104,7 +102,7 @@ func (ev *EmailValidator) isServiceDisabled() bool { // When serviceURL AND serviceKey are non-empty strings it uses the remote // service to determine if the email is valid. func (ev *EmailValidator) Validate(ctx context.Context, email string) error { - if ev.isExtendedDisabled() && ev.isServiceDisabled() { + if !ev.isExtendedEnabled() && !ev.isServiceEnabled() { return nil } @@ -121,7 +119,7 @@ func (ev *EmailValidator) Validate(ctx context.Context, email string) error { // Validate the static rules first to prevent round trips on bad emails // and to parse the host ahead of time. - if !ev.isExtendedDisabled() { + if ev.isExtendedEnabled() { // First validate static checks such as format, known invalid hosts // and any other network free checks. Running this check before we @@ -136,9 +134,9 @@ func (ev *EmailValidator) Validate(ctx context.Context, email string) error { g.Go(func() error { return ev.validateHost(ctx, host) }) } - // If the service check is not disabled we start a goroutine to run + // If the service check is enabled we start a goroutine to run // that check as well. - if !ev.isServiceDisabled() { + if ev.isServiceEnabled() { g.Go(func() error { return ev.validateService(ctx, email) }) } return g.Wait() @@ -147,7 +145,7 @@ func (ev *EmailValidator) Validate(ctx context.Context, email string) error { // validateStatic will validate the format and do the static checks before // returning the host portion of the email. func (ev *EmailValidator) validateStatic(email string) (string, error) { - if !ev.extended { + if !ev.isExtendedEnabled() { return "", nil } @@ -185,7 +183,7 @@ func (ev *EmailValidator) validateStatic(email string) (string, error) { } func (ev *EmailValidator) validateService(ctx context.Context, email string) error { - if ev.isServiceDisabled() { + if !ev.isServiceEnabled() { return nil } @@ -204,7 +202,11 @@ func (ev *EmailValidator) validateService(ctx context.Context, email string) err return nil } req.Header.Set("Content-Type", "application/json") - req.Header.Set("apikey", ev.serviceKey) + for name, vals := range ev.serviceHeaders { + for _, val := range vals { + req.Header.Set(name, val) + } + } res, err := http.DefaultClient.Do(req) if err != nil { @@ -215,6 +217,11 @@ func (ev *EmailValidator) validateService(ctx context.Context, email string) err resObject := struct { Valid *bool `json:"valid"` }{} + + if res.StatusCode != http.StatusOK { + return nil + } + dec := json.NewDecoder(io.LimitReader(res.Body, 1<<5)) if err := dec.Decode(&resObject); err != nil { return nil diff --git a/internal/mailer/validate_test.go b/internal/mailer/validate_test.go index 428605ce7..e1a86c28a 100644 --- a/internal/mailer/validate_test.go +++ b/internal/mailer/validate_test.go @@ -24,6 +24,9 @@ func TestEmalValidatorService(t *testing.T) { testHdrsVal := new(atomic.Value) testHdrsVal.Store(map[string]string{"apikey": "test"}) + // testHeaders := map[string][]string{"apikey": []string{"test"}} + testHeaders := `{"apikey": ["test"]}` + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { key := r.Header.Get("apikey") if key == "" { @@ -40,10 +43,14 @@ func TestEmalValidatorService(t *testing.T) { { testResVal.Store(`{"valid": true}`) cfg := conf.MailerConfiguration{ - EmailValidationExtended: true, - EmailValidationServiceURL: ts.URL, - EmailValidationServiceKey: "test", + EmailValidationExtended: true, + EmailValidationServiceURL: ts.URL, + EmailValidationServiceHeaders: testHeaders, + } + if err := cfg.Validate(); err != nil { + t.Fatal(err) } + ev := newEmailValidator(cfg) err := ev.Validate(ctx, "chris.stockton@supabase.io") if err != nil { @@ -58,10 +65,14 @@ func TestEmalValidatorService(t *testing.T) { testResVal.Store(`{"valid": true}`) cfg := conf.MailerConfiguration{ - EmailValidationExtended: false, - EmailValidationServiceURL: ts.URL, - EmailValidationServiceKey: "test", + EmailValidationExtended: false, + EmailValidationServiceURL: ts.URL, + EmailValidationServiceHeaders: testHeaders, + } + if err := cfg.Validate(); err != nil { + t.Fatal(err) } + ev := newEmailValidator(cfg) err := ev.Validate(ctx, "test@gmail.com") if err != nil { @@ -76,10 +87,14 @@ func TestEmalValidatorService(t *testing.T) { testResVal.Store(`{"valid": false}`) cfg := conf.MailerConfiguration{ - EmailValidationExtended: false, - EmailValidationServiceURL: "", - EmailValidationServiceKey: "", + EmailValidationExtended: false, + EmailValidationServiceURL: "", + EmailValidationServiceHeaders: "", + } + if err := cfg.Validate(); err != nil { + t.Fatal(err) } + ev := newEmailValidator(cfg) err := ev.Validate(ctx, "test@gmail.com") if err != nil { @@ -93,10 +108,14 @@ func TestEmalValidatorService(t *testing.T) { { testResVal.Store(`{"valid": true}`) cfg := conf.MailerConfiguration{ - EmailValidationExtended: true, - EmailValidationServiceURL: "", - EmailValidationServiceKey: "", + EmailValidationExtended: true, + EmailValidationServiceURL: "", + EmailValidationServiceHeaders: "", + } + if err := cfg.Validate(); err != nil { + t.Fatal(err) } + ev := newEmailValidator(cfg) err := ev.Validate(ctx, "test@gmail.com") if err == nil { @@ -110,10 +129,14 @@ func TestEmalValidatorService(t *testing.T) { { testResVal.Store(`{"valid": true}`) cfg := conf.MailerConfiguration{ - EmailValidationExtended: true, - EmailValidationServiceURL: ts.URL, - EmailValidationServiceKey: "test", + EmailValidationExtended: true, + EmailValidationServiceURL: ts.URL, + EmailValidationServiceHeaders: testHeaders, + } + if err := cfg.Validate(); err != nil { + t.Fatal(err) } + ev := newEmailValidator(cfg) err := ev.Validate(ctx, "test@gmail.com") if err == nil { @@ -127,10 +150,14 @@ func TestEmalValidatorService(t *testing.T) { { testResVal.Store(`{"valid": false}`) cfg := conf.MailerConfiguration{ - EmailValidationExtended: true, - EmailValidationServiceURL: ts.URL, - EmailValidationServiceKey: "test", + EmailValidationExtended: true, + EmailValidationServiceURL: ts.URL, + EmailValidationServiceHeaders: testHeaders, + } + if err := cfg.Validate(); err != nil { + t.Fatal(err) } + ev := newEmailValidator(cfg) err := ev.Validate(ctx, "chris.stockton@supabase.io") if err == nil { @@ -145,10 +172,14 @@ func TestEmalValidatorService(t *testing.T) { testResVal.Store(`{"valid": false}`) cfg := conf.MailerConfiguration{ - EmailValidationExtended: false, - EmailValidationServiceURL: ts.URL, - EmailValidationServiceKey: "test", + EmailValidationExtended: false, + EmailValidationServiceURL: ts.URL, + EmailValidationServiceHeaders: testHeaders, + } + if err := cfg.Validate(); err != nil { + t.Fatal(err) } + ev := newEmailValidator(cfg) err := ev.Validate(ctx, "test@gmail.com") if err == nil { @@ -221,9 +252,9 @@ func TestValidateEmailExtended(t *testing.T) { } cfg := conf.MailerConfiguration{ - EmailValidationExtended: true, - EmailValidationServiceURL: "", - EmailValidationServiceKey: "", + EmailValidationExtended: true, + EmailValidationServiceURL: "", + EmailValidationServiceHeaders: "", } ev := newEmailValidator(cfg)