From 84a7f1070f04171e02fe39a6cacde3b5b7b95bd4 Mon Sep 17 00:00:00 2001 From: Joel Lee Date: Thu, 20 Jun 2024 13:54:37 +0200 Subject: [PATCH] fix: update MaxFrequency error message to reflect number of seconds (#1540) ## What kind of change does this PR introduce? Currently we use a constant value on number of seconds left before a developer can send a follow up email confirmation. This can prove confusing if developer has a custom setting for `MaxFrequency` on `Sms` or `SMTP` We change some core email related routes to show the exact number of seconds. This includes `signup`, `magic_link` and `email_change` The rest will follow should this change roll out smoothly. Tested the three flows locally by triggering the max frequency limit and checking that the number of seconds show up as expected. --- internal/api/errors.go | 7 +++++++ internal/api/magic_link.go | 2 +- internal/api/signup.go | 5 +---- internal/api/user.go | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/internal/api/errors.go b/internal/api/errors.go index 2d40a53f4..e821409a7 100644 --- a/internal/api/errors.go +++ b/internal/api/errors.go @@ -6,6 +6,7 @@ import ( "net/http" "os" "runtime/debug" + "time" "github.com/pkg/errors" "github.com/supabase/auth/internal/observability" @@ -312,3 +313,9 @@ func HandleResponseError(err error, w http.ResponseWriter, r *http.Request) { } } } + +func generateFrequencyLimitErrorMessage(timeStamp *time.Time, maxFrequency time.Duration) string { + now := time.Now() + left := timeStamp.Add(maxFrequency).Sub(now) / time.Second + return fmt.Sprintf("For security purposes, you can only request this after %d seconds.", left) +} diff --git a/internal/api/magic_link.go b/internal/api/magic_link.go index e197d72f6..eeabafd39 100644 --- a/internal/api/magic_link.go +++ b/internal/api/magic_link.go @@ -142,7 +142,7 @@ func (a *API) MagicLink(w http.ResponseWriter, r *http.Request) error { }) if err != nil { if errors.Is(err, MaxFrequencyLimitError) { - return tooManyRequestsError(ErrorCodeOverEmailSendRateLimit, "For security purposes, you can only request this once every 60 seconds") + return tooManyRequestsError(ErrorCodeOverEmailSendRateLimit, generateFrequencyLimitErrorMessage(user.RecoverySentAt, config.SMTP.MaxFrequency)) } return internalServerError("Error sending magic link").WithInternalError(err) } diff --git a/internal/api/signup.go b/internal/api/signup.go index 75c287cff..b396178db 100644 --- a/internal/api/signup.go +++ b/internal/api/signup.go @@ -2,7 +2,6 @@ package api import ( "context" - "fmt" "net/http" "time" @@ -246,9 +245,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { } if terr = a.sendConfirmation(r, tx, user, flowType); terr != nil { if errors.Is(terr, MaxFrequencyLimitError) { - now := time.Now() - left := user.ConfirmationSentAt.Add(config.SMTP.MaxFrequency).Sub(now) / time.Second - return tooManyRequestsError(ErrorCodeOverEmailSendRateLimit, fmt.Sprintf("For security purposes, you can only request this after %d seconds.", left)) + return tooManyRequestsError(ErrorCodeOverEmailSendRateLimit, generateFrequencyLimitErrorMessage(user.ConfirmationSentAt, config.SMTP.MaxFrequency)) } return internalServerError("Error sending confirmation mail").WithInternalError(terr) } diff --git a/internal/api/user.go b/internal/api/user.go index 33c35aa57..e76b77453 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -214,7 +214,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { } if terr = a.sendEmailChange(r, tx, user, params.Email, flowType); terr != nil { if errors.Is(terr, MaxFrequencyLimitError) { - return tooManyRequestsError(ErrorCodeOverEmailSendRateLimit, "For security purposes, you can only request this once every 60 seconds") + return tooManyRequestsError(ErrorCodeOverEmailSendRateLimit, generateFrequencyLimitErrorMessage(user.EmailChangeSentAt, config.SMTP.MaxFrequency)) } return internalServerError("Error sending change email").WithInternalError(terr) }