From 765db08582669a1b7f054217fa8f0ed45804c0b5 Mon Sep 17 00:00:00 2001 From: Joel Lee Date: Tue, 2 Apr 2024 18:23:14 +0900 Subject: [PATCH] fix: move all EmailActionTypes to mailer package (#1510) ## What kind of change does this PR introduce? Moves the mail related refactors from #1496 into a new PR so the hook related PR is easier to review. This PR moves all EmailActionTypes to mailer package to establish an explicit link between mailer and the packages it is used in. The changes are cosmetic and should not affect underlying functionality. --- internal/api/mail.go | 15 ++++++------ internal/api/resend.go | 13 +++++----- internal/api/resend_test.go | 7 +++--- internal/api/signup_test.go | 3 ++- internal/api/verify.go | 49 +++++++++++++++++-------------------- internal/api/verify_test.go | 33 +++++++++++++------------ internal/mailer/template.go | 12 +++++++++ 7 files changed, 72 insertions(+), 60 deletions(-) diff --git a/internal/api/mail.go b/internal/api/mail.go index db280a9c4b..9b7d8dbff6 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -1,6 +1,7 @@ package api import ( + mail "github.com/supabase/auth/internal/mailer" "net/http" "strings" "time" @@ -64,8 +65,8 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { if err != nil { if models.IsNotFoundError(err) { switch params.Type { - case magicLinkVerification: - params.Type = signupVerification + case mail.MagicLinkVerification: + params.Type = mail.SignupVerification params.Password, err = password.Generate(64, 10, 1, false, true) if err != nil { // password generation must always succeed @@ -91,7 +92,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { hashedToken := crypto.GenerateTokenHash(params.Email, otp) var signupUser *models.User - if params.Type == signupVerification && user == nil { + if params.Type == mail.SignupVerification && user == nil { signupParams := &SignupParams{ Email: params.Email, Password: params.Password, @@ -113,7 +114,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { err = db.Transaction(func(tx *storage.Connection) error { var terr error switch params.Type { - case magicLinkVerification, recoveryVerification: + case mail.MagicLinkVerification, mail.RecoveryVerification: if terr = models.NewAuditLogEntry(r, tx, user, models.UserRecoveryRequestedAction, "", nil); terr != nil { return terr } @@ -123,7 +124,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { if terr != nil { terr = errors.Wrap(terr, "Database error updating user for recovery") } - case inviteVerification: + case mail.InviteVerification: if user != nil { if user.IsConfirmed() { return unprocessableEntityError(ErrorCodeEmailExists, DuplicateEmailMsg) @@ -170,7 +171,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { if terr != nil { terr = errors.Wrap(terr, "Database error updating user for invite") } - case signupVerification: + case mail.SignupVerification: if user != nil { if user.IsConfirmed() { return unprocessableEntityError(ErrorCodeEmailExists, DuplicateEmailMsg) @@ -202,7 +203,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { if terr != nil { terr = errors.Wrap(terr, "Database error updating user for confirmation") } - case "email_change_current", "email_change_new": + case mail.EmailChangeCurrentVerification, mail.EmailChangeNewVerification: if !config.Mailer.SecureEmailChangeEnabled && params.Type == "email_change_current" { return badRequestError(ErrorCodeValidationFailed, "Enable secure email change to generate link for current email") } diff --git a/internal/api/resend.go b/internal/api/resend.go index cb85c1d5fd..4093d3405c 100644 --- a/internal/api/resend.go +++ b/internal/api/resend.go @@ -7,6 +7,7 @@ import ( "github.com/supabase/auth/internal/api/sms_provider" "github.com/supabase/auth/internal/conf" + mail "github.com/supabase/auth/internal/mailer" "github.com/supabase/auth/internal/models" "github.com/supabase/auth/internal/storage" ) @@ -20,14 +21,14 @@ type ResendConfirmationParams struct { func (p *ResendConfirmationParams) Validate(config *conf.GlobalConfiguration) error { switch p.Type { - case signupVerification, emailChangeVerification, smsVerification, phoneChangeVerification: + case mail.SignupVerification, mail.EmailChangeVerification, smsVerification, phoneChangeVerification: break default: // type does not match one of the above return badRequestError(ErrorCodeValidationFailed, "Missing one of these types: signup, email_change, sms, phone_change") } - if p.Email == "" && p.Type == signupVerification { + if p.Email == "" && p.Type == mail.SignupVerification { return badRequestError(ErrorCodeValidationFailed, "Type provided requires an email address") } if p.Phone == "" && p.Type == smsVerification { @@ -91,7 +92,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error { } switch params.Type { - case signupVerification: + case mail.SignupVerification: if user.IsConfirmed() { // if the user's email is confirmed already, we don't need to send a confirmation email again return sendJSON(w, http.StatusOK, map[string]string{}) @@ -101,7 +102,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error { // if the user's phone is confirmed already, we don't need to send a confirmation sms again return sendJSON(w, http.StatusOK, map[string]string{}) } - case emailChangeVerification: + case mail.EmailChangeVerification: // do not resend if user doesn't have a new email address if user.EmailChange == "" { return sendJSON(w, http.StatusOK, map[string]string{}) @@ -116,7 +117,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error { messageID := "" err = db.Transaction(func(tx *storage.Connection) error { switch params.Type { - case signupVerification: + case mail.SignupVerification: if terr := models.NewAuditLogEntry(r, tx, user, models.UserConfirmationRequestedAction, "", nil); terr != nil { return terr } @@ -135,7 +136,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error { return terr } messageID = mID - case emailChangeVerification: + case mail.EmailChangeVerification: return a.sendEmailChange(r, tx, user, user.EmailChange, models.ImplicitFlow) case phoneChangeVerification: smsProvider, terr := sms_provider.GetSmsProvider(*config) diff --git a/internal/api/resend_test.go b/internal/api/resend_test.go index 0b589addae..122415dd78 100644 --- a/internal/api/resend_test.go +++ b/internal/api/resend_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/supabase/auth/internal/conf" + mail "github.com/supabase/auth/internal/mailer" "github.com/supabase/auth/internal/models" ) @@ -194,15 +195,15 @@ func (ts *ResendTestSuite) TestResendSuccess() { require.Equal(ts.T(), http.StatusOK, w.Code) switch c.params["type"] { - case signupVerification, emailChangeVerification: + case mail.SignupVerification, mail.EmailChangeVerification: dbUser, err := models.FindUserByID(ts.API.db, c.user.ID) require.NoError(ts.T(), err) require.NotEmpty(ts.T(), dbUser) - if c.params["type"] == signupVerification { + if c.params["type"] == mail.SignupVerification { require.NotEqual(ts.T(), dbUser.ConfirmationToken, c.user.ConfirmationToken) require.NotEqual(ts.T(), dbUser.ConfirmationSentAt, c.user.ConfirmationSentAt) - } else if c.params["type"] == emailChangeVerification { + } else if c.params["type"] == mail.EmailChangeVerification { require.NotEqual(ts.T(), dbUser.EmailChangeTokenNew, c.user.EmailChangeTokenNew) require.NotEqual(ts.T(), dbUser.EmailChangeSentAt, c.user.EmailChangeSentAt) } diff --git a/internal/api/signup_test.go b/internal/api/signup_test.go index 2978710c0f..36b8feb66a 100644 --- a/internal/api/signup_test.go +++ b/internal/api/signup_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "fmt" + mail "github.com/supabase/auth/internal/mailer" "net/http" "net/http/httptest" "net/url" @@ -132,7 +133,7 @@ func (ts *SignupTestSuite) TestVerifySignup() { require.NoError(ts.T(), err) // Setup request - reqUrl := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", signupVerification, u.ConfirmationToken) + reqUrl := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.SignupVerification, u.ConfirmationToken) req := httptest.NewRequest(http.MethodGet, reqUrl, nil) // Setup response recorder diff --git a/internal/api/verify.go b/internal/api/verify.go index d6d5dc5414..5d1a43aed5 100644 --- a/internal/api/verify.go +++ b/internal/api/verify.go @@ -14,6 +14,7 @@ import ( "github.com/supabase/auth/internal/api/provider" "github.com/supabase/auth/internal/api/sms_provider" "github.com/supabase/auth/internal/crypto" + mail "github.com/supabase/auth/internal/mailer" "github.com/supabase/auth/internal/models" "github.com/supabase/auth/internal/observability" "github.com/supabase/auth/internal/storage" @@ -21,15 +22,9 @@ import ( ) const ( - signupVerification = "signup" - recoveryVerification = "recovery" - inviteVerification = "invite" - magicLinkVerification = "magiclink" - emailChangeVerification = "email_change" smsVerification = "sms" phoneChangeVerification = "phone_change" // includes signupVerification and magicLinkVerification - emailOTPVerification = "email" ) const ( @@ -150,11 +145,11 @@ func (a *API) verifyGet(w http.ResponseWriter, r *http.Request, params *VerifyPa return terr } switch params.Type { - case signupVerification, inviteVerification: + case mail.SignupVerification, mail.InviteVerification: user, terr = a.signupVerify(r, ctx, tx, user) - case recoveryVerification, magicLinkVerification: + case mail.RecoveryVerification, mail.MagicLinkVerification: user, terr = a.recoverVerify(r, tx, user) - case emailChangeVerification: + case mail.EmailChangeVerification: user, terr = a.emailChangeVerify(r, tx, params, user) if user == nil && terr == nil { // when double confirmation is required @@ -254,11 +249,11 @@ func (a *API) verifyPost(w http.ResponseWriter, r *http.Request, params *VerifyP } switch params.Type { - case signupVerification, inviteVerification: + case mail.SignupVerification, mail.InviteVerification: user, terr = a.signupVerify(r, ctx, tx, user) - case recoveryVerification, magicLinkVerification: + case mail.RecoveryVerification, mail.MagicLinkVerification: user, terr = a.recoverVerify(r, tx, user) - case emailChangeVerification: + case mail.EmailChangeVerification: user, terr = a.emailChangeVerify(r, tx, params, user) if user == nil && terr == nil { isSingleConfirmationResponse = true @@ -555,14 +550,14 @@ func (a *API) verifyTokenHash(conn *storage.Connection, params *VerifyParams) (* var user *models.User var err error switch params.Type { - case emailOTPVerification: + case mail.EmailOTPVerification: // need to find user by confirmation token or recovery token with the token hash user, err = models.FindUserByConfirmationOrRecoveryToken(conn, params.TokenHash) - case signupVerification, inviteVerification: + case mail.SignupVerification, mail.InviteVerification: user, err = models.FindUserByConfirmationToken(conn, params.TokenHash) - case recoveryVerification, magicLinkVerification: + case mail.RecoveryVerification, mail.MagicLinkVerification: user, err = models.FindUserByRecoveryToken(conn, params.TokenHash) - case emailChangeVerification: + case mail.EmailChangeVerification: user, err = models.FindUserByEmailChangeToken(conn, params.TokenHash) default: return nil, badRequestError(ErrorCodeValidationFailed, "Invalid email verification type") @@ -581,7 +576,7 @@ func (a *API) verifyTokenHash(conn *storage.Connection, params *VerifyParams) (* var isExpired bool switch params.Type { - case emailOTPVerification: + case mail.EmailOTPVerification: sentAt := user.ConfirmationSentAt params.Type = "signup" if user.RecoveryToken == params.TokenHash { @@ -589,11 +584,11 @@ func (a *API) verifyTokenHash(conn *storage.Connection, params *VerifyParams) (* params.Type = "magiclink" } isExpired = isOtpExpired(sentAt, config.Mailer.OtpExp) - case signupVerification, inviteVerification: + case mail.SignupVerification, mail.InviteVerification: isExpired = isOtpExpired(user.ConfirmationSentAt, config.Mailer.OtpExp) - case recoveryVerification, magicLinkVerification: + case mail.RecoveryVerification, mail.MagicLinkVerification: isExpired = isOtpExpired(user.RecoverySentAt, config.Mailer.OtpExp) - case emailChangeVerification: + case mail.EmailChangeVerification: isExpired = isOtpExpired(user.EmailChangeSentAt, config.Mailer.OtpExp) } @@ -617,7 +612,7 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams, user, err = models.FindUserByPhoneChangeAndAudience(conn, params.Phone, aud) case smsVerification: user, err = models.FindUserByPhoneAndAudience(conn, params.Phone, aud) - case emailChangeVerification: + case mail.EmailChangeVerification: // Since the email change could be trigger via the implicit or PKCE flow, // the query used has to also check if the token saved in the db contains the pkce_ prefix user, err = models.FindUserForEmailChange(conn, params.Email, tokenHash, aud, config.Mailer.SecureEmailChangeEnabled) @@ -640,22 +635,22 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams, smsProvider, _ := sms_provider.GetSmsProvider(*config) switch params.Type { - case emailOTPVerification: + case mail.EmailOTPVerification: // if the type is emailOTPVerification, we'll check both the confirmation_token and recovery_token columns if isOtpValid(tokenHash, user.ConfirmationToken, user.ConfirmationSentAt, config.Mailer.OtpExp) { isValid = true - params.Type = signupVerification + params.Type = mail.SignupVerification } else if isOtpValid(tokenHash, user.RecoveryToken, user.RecoverySentAt, config.Mailer.OtpExp) { isValid = true - params.Type = magicLinkVerification + params.Type = mail.MagicLinkVerification } else { isValid = false } - case signupVerification, inviteVerification: + case mail.SignupVerification, mail.InviteVerification: isValid = isOtpValid(tokenHash, user.ConfirmationToken, user.ConfirmationSentAt, config.Mailer.OtpExp) - case recoveryVerification, magicLinkVerification: + case mail.RecoveryVerification, mail.MagicLinkVerification: isValid = isOtpValid(tokenHash, user.RecoveryToken, user.RecoverySentAt, config.Mailer.OtpExp) - case emailChangeVerification: + case mail.EmailChangeVerification: isValid = isOtpValid(tokenHash, user.EmailChangeTokenCurrent, user.EmailChangeSentAt, config.Mailer.OtpExp) || isOtpValid(tokenHash, user.EmailChangeTokenNew, user.EmailChangeSentAt, config.Mailer.OtpExp) case phoneChangeVerification, smsVerification: diff --git a/internal/api/verify_test.go b/internal/api/verify_test.go index 2f0a859072..4e7cbdd0d9 100644 --- a/internal/api/verify_test.go +++ b/internal/api/verify_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + mail "github.com/supabase/auth/internal/mailer" "io" "net/http" "net/http/httptest" @@ -105,7 +106,7 @@ func (ts *VerifyTestSuite) TestVerifyPasswordRecovery() { assert.WithinDuration(ts.T(), time.Now(), *u.RecoverySentAt, 1*time.Second) assert.False(ts.T(), u.IsConfirmed()) - reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", recoveryVerification, u.RecoveryToken) + reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.RecoveryVerification, u.RecoveryToken) req = httptest.NewRequest(http.MethodGet, reqURL, nil) w = httptest.NewRecorder() @@ -199,7 +200,7 @@ func (ts *VerifyTestSuite) TestVerifySecureEmailChange() { assert.False(ts.T(), u.IsConfirmed()) // Verify new email - reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", emailChangeVerification, u.EmailChangeTokenNew) + reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.EmailChangeVerification, u.EmailChangeTokenNew) req = httptest.NewRequest(http.MethodGet, reqURL, nil) w = httptest.NewRecorder() @@ -228,7 +229,7 @@ func (ts *VerifyTestSuite) TestVerifySecureEmailChange() { assert.Equal(ts.T(), singleConfirmation, u.EmailChangeConfirmStatus) // Verify old email - reqURL = fmt.Sprintf("http://localhost/verify?type=%s&token=%s", emailChangeVerification, u.EmailChangeTokenCurrent) + reqURL = fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.EmailChangeVerification, u.EmailChangeTokenCurrent) req = httptest.NewRequest(http.MethodGet, reqURL, nil) w = httptest.NewRecorder() @@ -270,7 +271,7 @@ func (ts *VerifyTestSuite) TestExpiredConfirmationToken() { require.NoError(ts.T(), ts.API.db.Update(u)) // Setup request - reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", signupVerification, u.ConfirmationToken) + reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.SignupVerification, u.ConfirmationToken) req := httptest.NewRequest(http.MethodGet, reqURL, nil) // Setup response recorder @@ -350,7 +351,7 @@ func (ts *VerifyTestSuite) TestInvalidOtp() { desc: "Invalid Email OTP", sentTime: time.Now(), body: map[string]interface{}{ - "type": signupVerification, + "type": mail.SignupVerification, "token": "invalid_otp", "email": u.GetEmail(), }, @@ -806,7 +807,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { desc: "Valid Confirmation OTP", sentTime: time.Now(), body: map[string]interface{}{ - "type": signupVerification, + "type": mail.SignupVerification, "tokenHash": crypto.GenerateTokenHash(u.GetEmail(), "123456"), "token": "123456", "email": u.GetEmail(), @@ -820,7 +821,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { desc: "Valid Recovery OTP", sentTime: time.Now(), body: map[string]interface{}{ - "type": recoveryVerification, + "type": mail.RecoveryVerification, "tokenHash": crypto.GenerateTokenHash(u.GetEmail(), "123456"), "token": "123456", "email": u.GetEmail(), @@ -834,7 +835,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { desc: "Valid Email OTP", sentTime: time.Now(), body: map[string]interface{}{ - "type": emailOTPVerification, + "type": mail.EmailOTPVerification, "tokenHash": crypto.GenerateTokenHash(u.GetEmail(), "123456"), "token": "123456", "email": u.GetEmail(), @@ -848,7 +849,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { desc: "Valid Email Change OTP", sentTime: time.Now(), body: map[string]interface{}{ - "type": emailChangeVerification, + "type": mail.EmailChangeVerification, "tokenHash": crypto.GenerateTokenHash(u.EmailChange, "123456"), "token": "123456", "email": u.EmailChange, @@ -876,7 +877,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { desc: "Valid Signup Token Hash", sentTime: time.Now(), body: map[string]interface{}{ - "type": signupVerification, + "type": mail.SignupVerification, "token_hash": crypto.GenerateTokenHash(u.GetEmail(), "123456"), }, expected: expected{ @@ -888,7 +889,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { desc: "Valid Email Change Token Hash", sentTime: time.Now(), body: map[string]interface{}{ - "type": emailChangeVerification, + "type": mail.EmailChangeVerification, "token_hash": crypto.GenerateTokenHash(u.EmailChange, "123456"), }, expected: expected{ @@ -900,7 +901,7 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() { desc: "Valid Email Verification Type", sentTime: time.Now(), body: map[string]interface{}{ - "type": emailOTPVerification, + "type": mail.EmailOTPVerification, "token_hash": crypto.GenerateTokenHash(u.GetEmail(), "123456"), }, expected: expected{ @@ -958,11 +959,11 @@ func (ts *VerifyTestSuite) TestSecureEmailChangeWithTokenHash() { { desc: "Secure Email Change with Token Hash (Success)", firstVerificationBody: map[string]interface{}{ - "type": emailChangeVerification, + "type": mail.EmailChangeVerification, "token_hash": currentEmailChangeToken, }, secondVerificationBody: map[string]interface{}{ - "type": emailChangeVerification, + "type": mail.EmailChangeVerification, "token_hash": newEmailChangeToken, }, expectedStatus: http.StatusOK, @@ -970,11 +971,11 @@ func (ts *VerifyTestSuite) TestSecureEmailChangeWithTokenHash() { { desc: "Secure Email Change with Token Hash. Reusing a token hash twice should fail", firstVerificationBody: map[string]interface{}{ - "type": emailChangeVerification, + "type": mail.EmailChangeVerification, "token_hash": currentEmailChangeToken, }, secondVerificationBody: map[string]interface{}{ - "type": emailChangeVerification, + "type": mail.EmailChangeVerification, "token_hash": currentEmailChangeToken, }, expectedStatus: http.StatusForbidden, diff --git a/internal/mailer/template.go b/internal/mailer/template.go index 9ecc749f15..cd1aacf95e 100644 --- a/internal/mailer/template.go +++ b/internal/mailer/template.go @@ -34,6 +34,18 @@ func encodeRedirectURL(referrerURL string) string { return referrerURL } +const ( + SignupVerification = "signup" + RecoveryVerification = "recovery" + InviteVerification = "invite" + MagicLinkVerification = "magiclink" + EmailChangeVerification = "email_change" + EmailOTPVerification = "email" + EmailChangeCurrentVerification = "email_change_current" + EmailChangeNewVerification = "email_change_new" + ReauthenticationVerification = "reauthentication" +) + const defaultInviteMail = `

You have been invited

You have been invited to create a user on {{ .SiteURL }}. Follow this link to accept the invite: