Repository: supabase/gotrue
Pull request #928 (2023-02-02 08:14:30)

refactor: update validateEmail & validatePhone
## What kind of change does this PR introduce?
* make `validateEmail` and `validatePhone` private methods in the `api` package instead of a struct method of the API struct 
* `validateEmail` used to rely on the `ValidateEmail` method in the `Mailer` interface. However, this is not necessary since we can just use the underlying lib directly 


Patch:
diff --git a/api/admin.go b/api/admin.go
index 68f54d73..d143a25a 100644
--- a/api/admin.go
+++ b/api/admin.go
@@ -148,14 +148,14 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error {
 	}
 
 	if params.Email != "" {
-		params.Email, err = a.validateEmail(ctx, params.Email)
+		params.Email, err = validateEmail(params.Email)
 		if err != nil {
 			return err
 		}
 	}
 
 	if params.Phone != "" {
-		params.Phone, err = a.validatePhone(params.Phone)
+		params.Phone, err = validatePhone(params.Phone)
 		if err != nil {
 			return err
 		}
@@ -275,7 +275,7 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error {
 	}
 
 	if params.Email != "" {
-		params.Email, err = a.validateEmail(ctx, params.Email)
+		params.Email, err = validateEmail(params.Email)
 		if err != nil {
 			return err
 		}
@@ -287,7 +287,7 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error {
 	}
 
 	if params.Phone != "" {
-		params.Phone, err = a.validatePhone(params.Phone)
+		params.Phone, err = validatePhone(params.Phone)
 		if err != nil {
 			return err
 		}
diff --git a/api/invite.go b/api/invite.go
index 699fcaed..ae0cebd4 100644
--- a/api/invite.go
+++ b/api/invite.go
@@ -33,7 +33,7 @@ func (a *API) Invite(w http.ResponseWriter, r *http.Request) error {
 		return badRequestError("Could not read Invite params: %v", err)
 	}
 
-	params.Email, err = a.validateEmail(ctx, params.Email)
+	params.Email, err = validateEmail(params.Email)
 	if err != nil {
 		return err
 	}
diff --git a/api/magic_link.go b/api/magic_link.go
index fdd35bfc..8dd5f6e9 100644
--- a/api/magic_link.go
+++ b/api/magic_link.go
@@ -43,7 +43,7 @@ func (a *API) MagicLink(w http.ResponseWriter, r *http.Request) error {
 	if params.Email == "" {
 		return unprocessableEntityError("Password recovery requires an email")
 	}
-	params.Email, err = a.validateEmail(ctx, params.Email)
+	params.Email, err = validateEmail(params.Email)
 	if err != nil {
 		return err
 	}
diff --git a/api/mail.go b/api/mail.go
index c2b5bb60..9d1df8df 100644
--- a/api/mail.go
+++ b/api/mail.go
@@ -1,7 +1,6 @@
 package api
 
 import (
-	"context"
 	"crypto/sha256"
 	"encoding/json"
 	"fmt"
@@ -9,6 +8,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/badoux/checkmail"
 	"github.com/fatih/structs"
 	"github.com/netlify/gotrue/api/provider"
 	"github.com/netlify/gotrue/conf"
@@ -60,7 +60,7 @@ func (a *API) GenerateLink(w http.ResponseWriter, r *http.Request) error {
 		return badRequestError("Could not parse JSON: %v", err)
 	}
 
-	params.Email, err = a.validateEmail(ctx, params.Email)
+	params.Email, err = validateEmail(params.Email)
 	if err != nil {
 		return err
 	}
@@ -178,7 +178,7 @@ func (a *API) GenerateLink(w http.ResponseWriter, r *http.Request) error {
 			if !config.Mailer.SecureEmailChangeEnabled && params.Type == "email_change_current" {
 				return unprocessableEntityError("Enable secure email change to generate link for current email")
 			}
-			params.NewEmail, terr = a.validateEmail(ctx, params.NewEmail)
+			params.NewEmail, terr = validateEmail(params.NewEmail)
 			if terr != nil {
 				return unprocessableEntityError("The new email address provided is invalid")
 			}
@@ -372,12 +372,11 @@ func (a *API) sendEmailChange(tx *storage.Connection, config *conf.GlobalConfigu
 	), "Database error updating user for email change")
 }
 
-func (a *API) validateEmail(ctx context.Context, email string) (string, error) {
+func validateEmail(email string) (string, error) {
 	if email == "" {
 		return "", unprocessableEntityError("An email address is required")
 	}
-	mailer := a.Mailer(ctx)
-	if err := mailer.ValidateEmail(email); err != nil {
+	if err := checkmail.ValidateFormat(email); err != nil {
 		return "", unprocessableEntityError("Unable to validate email address: " + err.Error())
 	}
 	return strings.ToLower(email), nil
diff --git a/api/otp.go b/api/otp.go
index 8e4e1606..7af296bd 100644
--- a/api/otp.go
+++ b/api/otp.go
@@ -87,7 +87,7 @@ func (a *API) SmsOtp(w http.ResponseWriter, r *http.Request) error {
 		params.Data = make(map[string]interface{})
 	}
 
-	params.Phone, err = a.validatePhone(params.Phone)
+	params.Phone, err = validatePhone(params.Phone)
 	if err != nil {
 		return err
 	}
@@ -179,13 +179,13 @@ func (a *API) shouldCreateUser(r *http.Request, params *OtpParams) (bool, error)
 		aud := a.requestAud(ctx, r)
 		var err error
 		if params.Email != "" {
-			params.Email, err = a.validateEmail(ctx, params.Email)
+			params.Email, err = validateEmail(params.Email)
 			if err != nil {
 				return false, err
 			}
 			_, err = models.FindUserByEmailAndAudience(db, params.Email, aud)
 		} else if params.Phone != "" {
-			params.Phone, err = a.validatePhone(params.Phone)
+			params.Phone, err = validatePhone(params.Phone)
 			if err != nil {
 				return false, err
 			}
diff --git a/api/phone.go b/api/phone.go
index f776dd6d..4b684787 100644
--- a/api/phone.go
+++ b/api/phone.go
@@ -23,23 +23,23 @@ const (
 	phoneReauthenticationOtp = "reauthentication"
 )
 
-func (a *API) validatePhone(phone string) (string, error) {
-	phone = a.formatPhoneNumber(phone)
-	if isValid := a.validateE164Format(phone); !isValid {
+func validatePhone(phone string) (string, error) {
+	phone = formatPhoneNumber(phone)
+	if isValid := validateE164Format(phone); !isValid {
 		return "", unprocessableEntityError("Invalid phone number format")
 	}
 	return phone, nil
 }
 
 // validateE164Format checks if phone number follows the E.164 format
-func (a *API) validateE164Format(phone string) bool {
+func validateE164Format(phone string) bool {
 	// match should never fail as long as regexp is valid
 	matched, _ := regexp.Match(e164Format, []byte(phone))
 	return matched
 }
 
 // formatPhoneNumber removes "+" and whitespaces in a phone number
-func (a *API) formatPhoneNumber(phone string) string {
+func formatPhoneNumber(phone string) string {
 	return strings.ReplaceAll(strings.Trim(phone, "+"), " ", "")
 }
 
diff --git a/api/recover.go b/api/recover.go
index b8e0c694..d5c5a95b 100644
--- a/api/recover.go
+++ b/api/recover.go
@@ -37,7 +37,7 @@ func (a *API) Recover(w http.ResponseWriter, r *http.Request) error {
 	var user *models.User
 	aud := a.requestAud(ctx, r)
 
-	params.Email, err = a.validateEmail(ctx, params.Email)
+	params.Email, err = validateEmail(params.Email)
 	if err != nil {
 		return err
 	}
diff --git a/api/signup.go b/api/signup.go
index 25a9cc6e..8d6dbe65 100644
--- a/api/signup.go
+++ b/api/signup.go
@@ -75,7 +75,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error {
 		if !config.External.Email.Enabled {
 			return badRequestError("Email signups are disabled")
 		}
-		params.Email, err = a.validateEmail(ctx, params.Email)
+		params.Email, err = validateEmail(params.Email)
 		if err != nil {
 			return err
 		}
@@ -84,7 +84,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error {
 		if !config.External.Phone.Enabled {
 			return badRequestError("Phone signups are disabled")
 		}
-		params.Phone, err = a.validatePhone(params.Phone)
+		params.Phone, err = validatePhone(params.Phone)
 		if err != nil {
 			return err
 		}
diff --git a/api/token.go b/api/token.go
index db763425..8acc8bb1 100644
--- a/api/token.go
+++ b/api/token.go
@@ -204,7 +204,7 @@ func (a *API) ResourceOwnerPasswordGrant(ctx context.Context, w http.ResponseWri
 		if !config.External.Phone.Enabled {
 			return badRequestError("Phone logins are disabled")
 		}
-		params.Phone = a.formatPhoneNumber(params.Phone)
+		params.Phone = formatPhoneNumber(params.Phone)
 		user, err = models.FindUserByPhoneAndAudience(db, params.Phone, aud)
 	} else {
 		return oauthError("invalid_grant", InvalidLoginMessage)
diff --git a/api/user.go b/api/user.go
index 62e4b1ef..153ba20d 100644
--- a/api/user.go
+++ b/api/user.go
@@ -124,7 +124,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
 		}
 
 		if params.Email != "" && params.Email != user.GetEmail() {
-			params.Email, terr = a.validateEmail(ctx, params.Email)
+			params.Email, terr = validateEmail(params.Email)
 			if terr != nil {
 				return terr
 			}
@@ -144,7 +144,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
 		}
 
 		if params.Phone != "" && params.Phone != user.GetPhone() {
-			params.Phone, err = a.validatePhone(params.Phone)
+			params.Phone, err = validatePhone(params.Phone)
 			if err != nil {
 				return err
 			}
diff --git a/api/verify.go b/api/verify.go
index 98e19b25..b10a717c 100644
--- a/api/verify.go
+++ b/api/verify.go
@@ -450,7 +450,7 @@ func (a *API) verifyUserAndToken(ctx context.Context, conn *storage.Connection,
 	var err error
 	var tokenHash string
 	if isPhoneOtpVerification(params) {
-		params.Phone, err = a.validatePhone(params.Phone)
+		params.Phone, err = validatePhone(params.Phone)
 		if err != nil {
 			return nil, err
 		}
@@ -464,7 +464,7 @@ func (a *API) verifyUserAndToken(ctx context.Context, conn *storage.Connection,
 			return nil, badRequestError("Invalid sms verification type")
 		}
 	} else if isEmailOtpVerification(params) {
-		params.Email, err = a.validateEmail(ctx, params.Email)
+		params.Email, err = validateEmail(params.Email)
 		if err != nil {
 			return nil, unprocessableEntityError("Invalid email format").WithInternalError(err)
 		}


Test patch:
diff --git a/api/phone_test.go b/api/phone_test.go
index 7057746e..63fe361f 100644
--- a/api/phone_test.go
+++ b/api/phone_test.go
@@ -56,12 +56,12 @@ func (ts *PhoneTestSuite) SetupTest() {
 }
 
 func (ts *PhoneTestSuite) TestValidateE164Format() {
-	isValid := ts.API.validateE164Format("0123456789")
+	isValid := validateE164Format("0123456789")
 	assert.Equal(ts.T(), false, isValid)
 }
 
 func (ts *PhoneTestSuite) TestFormatPhoneNumber() {
-	actual := ts.API.formatPhoneNumber("+1 23456789 ")
+	actual := formatPhoneNumber("+1 23456789 ")
 	assert.Equal(ts.T(), "123456789", actual)
 }