Skip to content

Commit

Permalink
fix: send otp in email link (supabase#379)
Browse files Browse the repository at this point in the history 
* fix: send otp in email links

* fix: allow verifying otps from emails

* add tests for email_change verification

* fix: add env var to configure email link / token expiry

* docs: update README

* fix: verify phone & email before fetching user

* add test for invalid email otp
  • Loading branch information
@kangmingtay @LashaJini
kangmingtay authored and LashaJini committed Nov 13, 2024
1 parent 2cc60af commit 3ed9beb
Show file tree
Hide file tree
Showing 5 changed files with 316 additions and 131 deletions.
4 changes: 4 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,10 @@ Sets the name of the sender. Defaults to the `SMTP_ADMIN_EMAIL` if not used.

If you do not require email confirmation, you may set this to `true`. Defaults to `false`.

`MAILER_OTP_EXP` - `number`

Controls the duration an email link or otp is valid for.

`MAILER_URLPATHS_INVITE` - `string`

URL path to use in the user invite email. Defaults to `/`.
Expand Down
238 changes: 119 additions & 119 deletions api/verify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type VerifyParams struct {
Type string `json:"type"`
Token string `json:"token"`
Password string `json:"password"`
Email string `json:"email"`
Phone string `json:"phone"`
RedirectTo string `json:"redirect_to"`
}
Expand All @@ -63,7 +64,7 @@ func (a *API) Verify(w http.ResponseWriter, r *http.Request) error {
}
params.RedirectTo = a.getRedirectURLOrReferrer(r, params.RedirectTo)
default:
unprocessableEntityError("Sorry, only GET and POST methods are supported.")
unprocessableEntityError("Only GET and POST methods are supported.")
}

if params.Token == "" {
Expand All @@ -78,42 +79,40 @@ func (a *API) Verify(w http.ResponseWriter, r *http.Request) error {

err = a.db.Transaction(func(tx *storage.Connection) error {
var terr error
aud := a.requestAud(ctx, r)
user, terr = a.verifyUserAndToken(ctx, tx, params, aud)
if terr != nil {
var herr *HTTPError
if errors.As(terr, &herr) {
if errors.Is(herr.InternalError, redirectWithQueryError) {
rurl := a.prepErrorRedirectURL(herr, r, params.RedirectTo)
http.Redirect(w, r, rurl, http.StatusSeeOther)
return nil
}
}
return terr
}

switch params.Type {
case signupVerification, inviteVerification:
user, terr = a.signupVerify(ctx, tx, params)
user, terr = a.signupVerify(ctx, tx, user)
case recoveryVerification, magicLinkVerification:
user, terr = a.recoverVerify(ctx, tx, params)
user, terr = a.recoverVerify(ctx, tx, user)
case emailChangeVerification:
user, terr = a.emailChangeVerify(ctx, tx, params)
user, terr = a.emailChangeVerify(ctx, tx, params, user)
if user == nil && terr == nil {
// when double confirmation is required
rurl := a.prepRedirectURL("Confirmation link accepted. Please proceed to confirm link sent to the other email", params.RedirectTo)
http.Redirect(w, r, rurl, http.StatusSeeOther)
return nil
}
case smsVerification:
if params.Phone == "" {
return unprocessableEntityError("Sms Verification requires a phone number")
}
params.Phone = a.formatPhoneNumber(params.Phone)
if isValid := a.validateE164Format(params.Phone); !isValid {
return unprocessableEntityError("Invalid phone number format")
}
aud := a.requestAud(ctx, r)
user, terr = a.smsVerify(ctx, tx, params, aud)
user, terr = a.smsVerify(ctx, tx, user)
default:
return unprocessableEntityError("Verify requires a verification type")
}

if terr != nil {
var e *HTTPError
if errors.As(terr, &e) {
if errors.Is(e.InternalError, redirectWithQueryError) {
rurl := a.prepErrorRedirectURL(e, r, params.RedirectTo)
http.Redirect(w, r, rurl, http.StatusSeeOther)
return nil
}
}
return terr
}

Expand Down Expand Up @@ -152,28 +151,11 @@ func (a *API) Verify(w http.ResponseWriter, r *http.Request) error {
return nil
}

func (a *API) signupVerify(ctx context.Context, conn *storage.Connection, params *VerifyParams) (*models.User, error) {
func (a *API) signupVerify(ctx context.Context, conn *storage.Connection, user *models.User) (*models.User, error) {
instanceID := getInstanceID(ctx)
config := a.getConfig(ctx)

user, err := models.FindUserByConfirmationToken(conn, params.Token)
if err != nil {
if models.IsNotFoundError(err) {
return nil, notFoundError(err.Error()).WithInternalError(redirectWithQueryError)
}
return nil, internalServerError("Database error finding user").WithInternalError(err)
}

if user.IsBanned() {
return nil, unauthorizedError("Error confirming user").WithInternalError(redirectWithQueryError)
}

nextDay := user.ConfirmationSentAt.Add(24 * time.Hour)
if user.ConfirmationSentAt != nil && time.Now().After(nextDay) {
return nil, expiredTokenError("Confirmation token expired").WithInternalError(redirectWithQueryError)
}

err = conn.Transaction(func(tx *storage.Connection) error {
err := conn.Transaction(func(tx *storage.Connection) error {
var terr error
if user.EncryptedPassword == "" {
if user.InvitedAt != nil {
Expand Down Expand Up @@ -208,27 +190,11 @@ func (a *API) signupVerify(ctx context.Context, conn *storage.Connection, params
return user, nil
}

func (a *API) recoverVerify(ctx context.Context, conn *storage.Connection, params *VerifyParams) (*models.User, error) {
func (a *API) recoverVerify(ctx context.Context, conn *storage.Connection, user *models.User) (*models.User, error) {
instanceID := getInstanceID(ctx)
config := a.getConfig(ctx)
user, err := models.FindUserByRecoveryToken(conn, params.Token)
if err != nil {
if models.IsNotFoundError(err) {
return nil, notFoundError(err.Error()).WithInternalError(redirectWithQueryError)
}
return nil, internalServerError("Database error finding user").WithInternalError(err)
}

if user.IsBanned() {
return nil, unauthorizedError("Error confirming user").WithInternalError(redirectWithQueryError)
}

nextDay := user.RecoverySentAt.Add(24 * time.Hour)
if user.RecoverySentAt != nil && time.Now().After(nextDay) {
return nil, expiredTokenError("Recovery token expired").WithInternalError(redirectWithQueryError)
}

err = conn.Transaction(func(tx *storage.Connection) error {
err := conn.Transaction(func(tx *storage.Connection) error {
var terr error
if terr = user.Recover(tx); terr != nil {
return terr
Expand All @@ -254,30 +220,11 @@ func (a *API) recoverVerify(ctx context.Context, conn *storage.Connection, param
return user, nil
}

func (a *API) smsVerify(ctx context.Context, conn *storage.Connection, params *VerifyParams, aud string) (*models.User, error) {
func (a *API) smsVerify(ctx context.Context, conn *storage.Connection, user *models.User) (*models.User, error) {
instanceID := getInstanceID(ctx)
config := a.getConfig(ctx)
user, err := models.FindUserByPhoneAndAudience(conn, instanceID, params.Phone, aud)
if err != nil {
if models.IsNotFoundError(err) {
return nil, notFoundError(err.Error()).WithInternalError(redirectWithQueryError)
}
return nil, internalServerError("Database error finding user").WithInternalError(err)
}

if user.IsBanned() {
return nil, unauthorizedError("Error confirming user").WithInternalError(redirectWithQueryError)
}

now := time.Now()
expiresAt := user.ConfirmationSentAt.Add(time.Second * time.Duration(config.Sms.OtpExp))

// check if token has expired or is invalid
if isOtpValid := now.Before(expiresAt) && params.Token == user.ConfirmationToken; !isOtpValid {
return nil, expiredTokenError("Otp has expired or is invalid").WithInternalError(redirectWithQueryError)
}

err = conn.Transaction(func(tx *storage.Connection) error {
err := conn.Transaction(func(tx *storage.Connection) error {
var terr error
if terr = models.NewAuditLogEntry(tx, instanceID, user, models.UserSignedUpAction, nil); terr != nil {
return terr
Expand Down Expand Up @@ -318,56 +265,31 @@ func (a *API) prepRedirectURL(message string, rurl string) string {
return rurl + "#" + q.Encode()
}

func (a *API) emailChangeVerify(ctx context.Context, conn *storage.Connection, params *VerifyParams) (*models.User, error) {
func (a *API) emailChangeVerify(ctx context.Context, conn *storage.Connection, params *VerifyParams, user *models.User) (*models.User, error) {
instanceID := getInstanceID(ctx)
config := a.getConfig(ctx)
user, err := models.FindUserByEmailChangeToken(conn, params.Token)
if err != nil {
if models.IsNotFoundError(err) {
return nil, notFoundError(err.Error()).WithInternalError(redirectWithQueryError)
}
return nil, internalServerError("Database error finding user").WithInternalError(err)
}

if user.IsBanned() {
return nil, unauthorizedError("Error confirming user").WithInternalError(redirectWithQueryError)
}

nextDay := user.EmailChangeSentAt.Add(24 * time.Hour)
if user.EmailChangeSentAt != nil && time.Now().After(nextDay) {
err = a.db.Transaction(func(tx *storage.Connection) error {
user.EmailChangeConfirmStatus = zeroConfirmation
return tx.UpdateOnly(user, "email_change_confirm_status")
if config.Mailer.SecureEmailChangeEnabled && user.EmailChangeConfirmStatus == zeroConfirmation {
err := a.db.Transaction(func(tx *storage.Connection) error {
user.EmailChangeConfirmStatus = singleConfirmation
if params.Token == user.EmailChangeTokenCurrent {
user.EmailChangeTokenCurrent = ""
} else if params.Token == user.EmailChangeTokenNew {
user.EmailChangeTokenNew = ""
}
if terr := tx.UpdateOnly(user, "email_change_confirm_status", "email_change_token_current", "email_change_token_new"); terr != nil {
return terr
}
return nil
})
if err != nil {
return nil, err
}
return nil, expiredTokenError("Email change token expired").WithInternalError(redirectWithQueryError)
}

if config.Mailer.SecureEmailChangeEnabled {
if user.EmailChangeConfirmStatus == zeroConfirmation {
err = a.db.Transaction(func(tx *storage.Connection) error {
user.EmailChangeConfirmStatus = singleConfirmation
if params.Token == user.EmailChangeTokenCurrent {
user.EmailChangeTokenCurrent = ""
} else if params.Token == user.EmailChangeTokenNew {
user.EmailChangeTokenNew = ""
}
if terr := tx.UpdateOnly(user, "email_change_confirm_status", "email_change_token_current", "email_change_token_new"); terr != nil {
return terr
}
return nil
})
if err != nil {
return nil, err
}
return nil, nil
}
return nil, nil
}

// one email is confirmed at this point
err = a.db.Transaction(func(tx *storage.Connection) error {
err := a.db.Transaction(func(tx *storage.Connection) error {
var terr error

if terr = models.NewAuditLogEntry(tx, instanceID, user, models.UserModifiedAction, nil); terr != nil {
Expand All @@ -390,3 +312,81 @@ func (a *API) emailChangeVerify(ctx context.Context, conn *storage.Connection, p

return user, nil
}

// verifyUserAndToken verifies the token associated to the user based on the verify type
func (a *API) verifyUserAndToken(ctx context.Context, conn *storage.Connection, params *VerifyParams, aud string) (*models.User, error) {
instanceID := getInstanceID(ctx)
config := getConfig(ctx)

var user *models.User
var err error
if isUrlVerification(params) {
switch params.Type {
case signupVerification, inviteVerification:
user, err = models.FindUserByConfirmationToken(conn, params.Token)
case recoveryVerification, magicLinkVerification:
user, err = models.FindUserByRecoveryToken(conn, params.Token)
case emailChangeVerification:
user, err = models.FindUserByEmailChangeToken(conn, params.Token)
}
} else if params.Type == smsVerification {
if params.Phone == "" {
return nil, unprocessableEntityError("Sms Verification requires a phone number")
}
params.Phone = a.formatPhoneNumber(params.Phone)
if ok := a.validateE164Format(params.Phone); !ok {
return nil, unprocessableEntityError("Invalid phone number format")
}
user, err = models.FindUserByPhoneAndAudience(conn, instanceID, params.Phone, aud)
} else if params.Email != "" {
if err := a.validateEmail(ctx, params.Email); err != nil {
return nil, unprocessableEntityError("Invalid email format").WithInternalError(err)
}
user, err = models.FindUserByEmailAndAudience(conn, instanceID, params.Email, aud)
}

if err != nil {
if models.IsNotFoundError(err) {
return nil, notFoundError(err.Error()).WithInternalError(redirectWithQueryError)
}
return nil, internalServerError("Database error finding user").WithInternalError(err)
}

if user.IsBanned() {
return nil, unauthorizedError("Error confirming user").WithInternalError(redirectWithQueryError)
}

var isValid bool
mailerOtpExpiresAt := time.Second * time.Duration(config.Mailer.OtpExp)
smsOtpExpiresAt := time.Second * time.Duration(config.Sms.OtpExp)
switch params.Type {
case signupVerification, inviteVerification:
isValid = isOtpValid(params.Token, user.ConfirmationToken, user.ConfirmationSentAt.Add(mailerOtpExpiresAt))
case recoveryVerification, magicLinkVerification:
isValid = isOtpValid(params.Token, user.RecoveryToken, user.RecoverySentAt.Add(mailerOtpExpiresAt))
case emailChangeVerification:
expiresAt := user.EmailChangeSentAt.Add(mailerOtpExpiresAt)
isValid = isOtpValid(params.Token, user.EmailChangeTokenCurrent, expiresAt) || isOtpValid(params.Token, user.EmailChangeTokenNew, expiresAt)
if !isValid {
// reset email confirmation status
user.EmailChangeConfirmStatus = zeroConfirmation
err = conn.UpdateOnly(user, "email_change_confirm_status")
}
case smsVerification:
isValid = isOtpValid(params.Token, user.ConfirmationToken, user.ConfirmationSentAt.Add(smsOtpExpiresAt))
}

if !isValid || err != nil {
return nil, expiredTokenError("Token has expired or is invalid").WithInternalError(redirectWithQueryError)
}
return user, nil
}

// isOtpValid checks the actual otp sent against the expected otp and ensures that it's within the valid window
func isOtpValid(actual, expected string, expiresAt time.Time) bool {
return time.Now().Before(expiresAt) && (actual == expected)
}

func isUrlVerification(params *VerifyParams) bool {
return params.Type != smsVerification && params.Email == ""
}
Loading

0 comments on commit 3ed9beb

Please sign in to comment.