Skip to content

Commit

Permalink
fix: drop smpt frequency and otp length from input params
Browse files Browse the repository at this point in the history
  • Loading branch information
J0 committed Mar 25, 2024
1 parent c63aaa5 commit 7bed58f
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 17 deletions.
2 changes: 1 addition & 1 deletion internal/api/external.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.
if decision.CandidateEmail.Email != "" {
referrer := utilities.GetReferrer(r, config)
externalURL := getExternalHost(ctx)
if terr = a.sendConfirmation(tx, user, config.SMTP.MaxFrequency, referrer, externalURL, config.Mailer.OtpLength, models.ImplicitFlow); terr != nil {
if terr = a.sendConfirmation(tx, user, referrer, externalURL, models.ImplicitFlow); terr != nil {
if errors.Is(terr, MaxFrequencyLimitError) {
return nil, tooManyRequestsError(ErrorCodeOverEmailSendRateLimit, "For security purposes, you can only request this once every minute")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/api/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (a *API) linkIdentityToUser(r *http.Request, ctx context.Context, tx *stora
if !userData.Metadata.EmailVerified {
referrer := utilities.GetReferrer(r, a.config)
externalURL := getExternalHost(ctx)
if terr := a.sendConfirmation(tx, targetUser, a.config.SMTP.MaxFrequency, referrer, externalURL, a.config.Mailer.OtpLength, models.ImplicitFlow); terr != nil {
if terr := a.sendConfirmation(tx, targetUser, referrer, externalURL, models.ImplicitFlow); terr != nil {
if errors.Is(terr, MaxFrequencyLimitError) {
return nil, tooManyRequestsError(ErrorCodeOverSMSSendRateLimit, "For security purposes, you can only request this once every minute")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/api/invite.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (a *API) Invite(w http.ResponseWriter, r *http.Request) error {

referrer := utilities.GetReferrer(r, config)
externalURL := getExternalHost(ctx)
if err := a.sendInvite(tx, user, referrer, externalURL, config.Mailer.OtpLength); err != nil {
if err := a.sendInvite(tx, user, referrer, externalURL); err != nil {
return internalServerError("Error inviting user").WithInternalError(err)
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/api/magic_link.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (a *API) MagicLink(w http.ResponseWriter, r *http.Request) error {
}
referrer := utilities.GetReferrer(r, config)
externalURL := getExternalHost(ctx)
return a.sendMagicLink(tx, user, config.SMTP.MaxFrequency, referrer, externalURL, config.Mailer.OtpLength, flowType)
return a.sendMagicLink(tx, user, referrer, externalURL, flowType)
})
if err != nil {
if errors.Is(err, MaxFrequencyLimitError) {
Expand Down
29 changes: 22 additions & 7 deletions internal/api/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/pkg/errors"
"github.com/sethvargo/go-password/password"
"github.com/supabase/auth/internal/api/provider"
"github.com/supabase/auth/internal/conf"
"github.com/supabase/auth/internal/crypto"
"github.com/supabase/auth/internal/models"
"github.com/supabase/auth/internal/storage"
Expand Down Expand Up @@ -262,8 +261,11 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error {
return sendJSON(w, http.StatusOK, resp)
}

func (a *API) sendConfirmation(tx *storage.Connection, u *models.User, maxFrequency time.Duration, referrerURL string, externalURL *url.URL, otpLength int, flowType models.FlowType) error {
func (a *API) sendConfirmation(tx *storage.Connection, u *models.User, referrerURL string, externalURL *url.URL, flowType models.FlowType) error {
mailer := a.Mailer()
config := a.config
otpLength := config.Mailer.OtpLength
maxFrequency := config.SMTP.MaxFrequency
var err error
if u.ConfirmationSentAt != nil && !u.ConfirmationSentAt.Add(maxFrequency).Before(time.Now()) {
return MaxFrequencyLimitError
Expand All @@ -290,8 +292,10 @@ func (a *API) sendConfirmation(tx *storage.Connection, u *models.User, maxFreque
return nil
}

func (a *API) sendInvite(tx *storage.Connection, u *models.User, referrerURL string, externalURL *url.URL, otpLength int) error {
func (a *API) sendInvite(tx *storage.Connection, u *models.User, referrerURL string, externalURL *url.URL) error {
mailer := a.Mailer()
config := a.config
otpLength := config.Mailer.OtpLength
var err error
oldToken := u.ConfirmationToken
otp, err := crypto.GenerateOtp(otpLength)
Expand All @@ -315,7 +319,10 @@ func (a *API) sendInvite(tx *storage.Connection, u *models.User, referrerURL str
return nil
}

func (a *API) sendPasswordRecovery(tx *storage.Connection, u *models.User, maxFrequency time.Duration, referrerURL string, externalURL *url.URL, otpLength int, flowType models.FlowType) error {
func (a *API) sendPasswordRecovery(tx *storage.Connection, u *models.User, referrerURL string, externalURL *url.URL, flowType models.FlowType) error {
config := a.config
maxFrequency := config.SMTP.MaxFrequency
otpLength := config.Mailer.OtpLength
mailer := a.Mailer()
var err error
if u.RecoverySentAt != nil && !u.RecoverySentAt.Add(maxFrequency).Before(time.Now()) {
Expand Down Expand Up @@ -344,7 +351,10 @@ func (a *API) sendPasswordRecovery(tx *storage.Connection, u *models.User, maxFr
return nil
}

func (a *API) sendReauthenticationOtp(tx *storage.Connection, u *models.User, maxFrequency time.Duration, otpLength int) error {
func (a *API) sendReauthenticationOtp(tx *storage.Connection, u *models.User) error {
config := a.config
maxFrequency := config.SMTP.MaxFrequency
otpLength := config.Mailer.OtpLength
mailer := a.Mailer()
var err error
if u.ReauthenticationSentAt != nil && !u.ReauthenticationSentAt.Add(maxFrequency).Before(time.Now()) {
Expand Down Expand Up @@ -372,8 +382,11 @@ func (a *API) sendReauthenticationOtp(tx *storage.Connection, u *models.User, ma
return nil
}

func (a *API) sendMagicLink(tx *storage.Connection, u *models.User, maxFrequency time.Duration, referrerURL string, externalURL *url.URL, otpLength int, flowType models.FlowType) error {
func (a *API) sendMagicLink(tx *storage.Connection, u *models.User, referrerURL string, externalURL *url.URL, flowType models.FlowType) error {
mailer := a.Mailer()
config := a.config
otpLength := config.Mailer.OtpLength
maxFrequency := config.SMTP.MaxFrequency
var err error
// since Magic Link is just a recovery with a different template and behaviour
// around new users we will reuse the recovery db timer to prevent potential abuse
Expand Down Expand Up @@ -404,7 +417,9 @@ func (a *API) sendMagicLink(tx *storage.Connection, u *models.User, maxFrequency
}

// sendEmailChange sends out an email change token to the new email.
func (a *API) sendEmailChange(tx *storage.Connection, config *conf.GlobalConfiguration, u *models.User, email, referrerURL string, externalURL *url.URL, otpLength int, flowType models.FlowType) error {
func (a *API) sendEmailChange(tx *storage.Connection, u *models.User, email, referrerURL string, externalURL *url.URL, flowType models.FlowType) error {
config := a.config
otpLength := config.Mailer.OtpLength
var err error
mailer := a.Mailer()
if u.EmailChangeSentAt != nil && !u.EmailChangeSentAt.Add(config.SMTP.MaxFrequency).Before(time.Now()) {
Expand Down
2 changes: 1 addition & 1 deletion internal/api/reauthenticate.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (a *API) Reauthenticate(w http.ResponseWriter, r *http.Request) error {
return terr
}
if email != "" {
return a.sendReauthenticationOtp(tx, user, config.SMTP.MaxFrequency, config.Mailer.OtpLength)
return a.sendReauthenticationOtp(tx, user)
} else if phone != "" {
smsProvider, terr := sms_provider.GetSmsProvider(*config)
if terr != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/api/recover.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (a *API) Recover(w http.ResponseWriter, r *http.Request) error {
}
referrer := utilities.GetReferrer(r, config)
externalURL := getExternalHost(ctx)
return a.sendPasswordRecovery(tx, user, config.SMTP.MaxFrequency, referrer, externalURL, config.Mailer.OtpLength, flowType)
return a.sendPasswordRecovery(tx, user, referrer, externalURL, flowType)
})
if err != nil {
if errors.Is(err, MaxFrequencyLimitError) {
Expand Down
4 changes: 2 additions & 2 deletions internal/api/resend.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error {
return terr
}
// PKCE not implemented yet
return a.sendConfirmation(tx, user, config.SMTP.MaxFrequency, referrer, externalURL, config.Mailer.OtpLength, models.ImplicitFlow)
return a.sendConfirmation(tx, user, referrer, externalURL, models.ImplicitFlow)
case smsVerification:
if terr := models.NewAuditLogEntry(r, tx, user, models.UserRecoveryRequestedAction, "", nil); terr != nil {
return terr
Expand All @@ -139,7 +139,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error {
}
messageID = mID
case emailChangeVerification:
return a.sendEmailChange(tx, config, user, user.EmailChange, referrer, externalURL, config.Mailer.OtpLength, models.ImplicitFlow)
return a.sendEmailChange(tx, user, user.EmailChange, referrer, externalURL, models.ImplicitFlow)
case phoneChangeVerification:
smsProvider, terr := sms_provider.GetSmsProvider(*config)
if terr != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/api/signup.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error {
}
}
externalURL := getExternalHost(ctx)
if terr = a.sendConfirmation(tx, user, config.SMTP.MaxFrequency, referrer, externalURL, config.Mailer.OtpLength, flowType); terr != nil {
if terr = a.sendConfirmation(tx, user, referrer, externalURL, flowType); terr != nil {
if errors.Is(terr, MaxFrequencyLimitError) {
now := time.Now()
left := user.ConfirmationSentAt.Add(config.SMTP.MaxFrequency).Sub(now) / time.Second
Expand Down
2 changes: 1 addition & 1 deletion internal/api/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {

}
externalURL := getExternalHost(ctx)
if terr = a.sendEmailChange(tx, config, user, params.Email, referrer, externalURL, config.Mailer.OtpLength, flowType); terr != nil {
if terr = a.sendEmailChange(tx, user, params.Email, referrer, externalURL, flowType); terr != nil {
if errors.Is(terr, MaxFrequencyLimitError) {
return tooManyRequestsError(ErrorCodeOverEmailSendRateLimit, "For security purposes, you can only request this once every 60 seconds")
}
Expand Down

0 comments on commit 7bed58f

Please sign in to comment.