Skip to content

Commit

Permalink
fix: update MaxFrequency error message to reflect number of seconds (s…
Browse files Browse the repository at this point in the history
…upabase#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.
  • Loading branch information
J0 authored Jun 20, 2024
1 parent 1b75dfd commit 84a7f10
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 6 deletions.
7 changes: 7 additions & 0 deletions internal/api/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"
"os"
"runtime/debug"
"time"

"github.com/pkg/errors"
"github.com/supabase/auth/internal/observability"
Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion internal/api/magic_link.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
5 changes: 1 addition & 4 deletions internal/api/signup.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package api

import (
"context"
"fmt"
"net/http"
"time"

Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/api/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 84a7f10

Please sign in to comment.