Repository: supabase/gotrue
Pull request #927 (2023-02-01 14:47:09)

feat: remove `id_token` flow with freeform provider
The `POST /token?grant_type=id_token` implementation allowed any OpenID Connect compliant issuer to be used for authentication, albeit in a very limited and broken way. Given we are not supporting this at this time, the implementation is being removed.

Existing OIDC providers, importantly `google` and `apple` continue to be supported.


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..8965958c 100644
--- a/api/token.go
+++ b/api/token.go
@@ -70,8 +70,6 @@ type IdTokenGrantParams struct {
 	IdToken  string `json:"id_token"`
 	Nonce    string `json:"nonce"`
 	Provider string `json:"provider"`
-	ClientID string `json:"client_id"`
-	Issuer   string `json:"issuer"`
 }
 
 const useCookieHeader = "x-use-cookie"
@@ -125,16 +123,6 @@ func (p *IdTokenGrantParams) getVerifier(ctx context.Context, config *conf.Globa
 	return provider.Verifier(&oidc.Config{ClientID: oAuthProviderClientId}), nil
 }
 
-func (p *IdTokenGrantParams) getVerifierFromClientIDandIssuer(ctx context.Context) (*oidc.IDTokenVerifier, error) {
-	var provider *oidc.Provider
-	var err error
-	provider, err = oidc.NewProvider(ctx, p.Issuer)
-	if err != nil {
-		return nil, fmt.Errorf("issuer %s doesn't support the id_token grant flow", p.Issuer)
-	}
-	return provider.Verifier(&oidc.Config{ClientID: p.ClientID}), nil
-}
-
 func getEmailVerified(v interface{}) bool {
 	var emailVerified bool
 	var err error
@@ -204,7 +192,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)
@@ -404,18 +392,11 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R
 		return oauthError("invalid request", "id_token required")
 	}
 
-	if params.Provider == "" && (params.ClientID == "" || params.Issuer == "") {
-		return oauthError("invalid request", "provider or client_id and issuer required")
+	if params.Provider == "" {
+		return oauthError("invalid request", "provider required")
 	}
 
-	var verifier *oidc.IDTokenVerifier
-	if params.Provider != "" {
-		verifier, err = params.getVerifier(ctx, a.config)
-	} else if params.ClientID != "" && params.Issuer != "" {
-		verifier, err = params.getVerifierFromClientIDandIssuer(ctx)
-	} else {
-		return badRequestError("%v", err)
-	}
+	verifier, err := params.getVerifier(ctx, a.config)
 	if err != nil {
 		return err
 	}
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)
 		}
diff --git a/migrations/20221011041400_add_mfa_indexes.up.sql b/migrations/20221011041400_add_mfa_indexes.up.sql
index 25d3a863..def57a29 100644
--- a/migrations/20221011041400_add_mfa_indexes.up.sql
+++ b/migrations/20221011041400_add_mfa_indexes.up.sql
@@ -5,8 +5,9 @@ do $$
 begin
   if not exists
      (select constraint_name
-      from information_schema.check_constraints
-      where constraint_schema = '{{ index .Options "Namespace" }}'
+      from information_schema.table_constraints
+      where table_schema = '{{ index .Options "Namespace" }}'
+      and table_name = 'mfa_amr_claims'
       and constraint_name = 'amr_id_pk')
   then
     alter table {{ index .Options "Namespace" }}.mfa_amr_claims add constraint amr_id_pk primary key(id);
diff --git a/openapi.yaml b/openapi.yaml
index 8660dad5..280ab051 100644
--- a/openapi.yaml
+++ b/openapi.yaml
@@ -90,7 +90,7 @@ paths:
               description: |-
                 For the refresh token flow, supply only `refresh_token`.
                 For the email/phone with password flow, supply `email`, `phone` and `password` with an optional `gotrue_meta_security`.
-                For the OIDC ID token flow, supply `id_token`, `nonce`, `provider`, `client_id`, `issuer` with an optional `gotrue_meta_security`.
+                For the OIDC ID token flow, supply `id_token`, `nonce`, `provider` with an optional `gotrue_meta_security`.
               properties:
                 refresh_token:
                   type: string
@@ -111,10 +111,6 @@ paths:
                   enum:
                     - google
                     - apple
-                client_id:
-                  type: string
-                issuer:
-                  type: string
                 gotrue_meta_security:
                   $ref: '#/components/schemas/GoTrueMetaSecurity'
       responses:


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)
 }