feat(otp): handle test OTPs and Twilio Verify on the one_time_tokens path

internal/api/verify.go

@@ -701,7 +701,7 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams,
 	config := a.config
 
 	if config.Experimental.EnableOTTAsSourceOfTruth {
-		return verifyUserAndTokenFromOTT(conn, params, aud)
+		return a.verifyUserAndTokenFromOTT(conn, params, aud)
 	}
 
 	var user *models.User

internal/api/verify_ott.go

@@ -2,8 +2,10 @@ package api
 
 import (
 	"strings"
+	"time"
 
 	"github.com/supabase/auth/internal/api/apierrors"
+	"github.com/supabase/auth/internal/api/sms_provider"
 	mail "github.com/supabase/auth/internal/mailer"
 	"github.com/supabase/auth/internal/models"
 	"github.com/supabase/auth/internal/storage"
@@ -13,11 +15,19 @@ import (
 // challenge in the one_time_tokens table and derives the user from that row,
 // instead of finding the user by identifier and comparing the users.*_token
 // columns. A lookup miss is rejected as an expired or invalid token.
-//
-// NOTE: Test OTPs and Twilio Verify are not handled yet on this path; a follow-up PR will add them.
-func verifyUserAndTokenFromOTT(conn *storage.Connection, params *VerifyParams, aud string) (*models.User, error) {
+func (a *API) verifyUserAndTokenFromOTT(conn *storage.Connection, params *VerifyParams, aud string) (*models.User, error) {
+	config := a.config
+
+	// Twilio Verify and test OTPs are verified without a local challenge
+	if params.Type == smsVerification || params.Type == phoneChangeVerification {
+		if testOTP, ok := config.Sms.GetTestOTP(params.Phone, time.Now()); ok && params.Token == testOTP {
+			return a.findUserForTestOTP(conn, params, aud)
+		}
+		if !config.Hook.SendSMS.Enabled && config.Sms.IsTwilioVerifyProvider() {
+			return a.verifyPhoneWithTwilio(conn, params, aud)
+		}
+	}
 
-	// TODO AUTH-1553: Add support for test OTPs and Twilio Verify on this path.
 	ott, err := verifyOneTimeToken(conn, params)
 	if err != nil {
 		return nil, err
@@ -43,6 +53,84 @@ func verifyUserAndTokenFromOTT(conn *storage.Connection, params *VerifyParams, a
 	return user, nil
 }
 
+func (a *API) verifyPhoneWithTwilio(conn *storage.Connection, params *VerifyParams, aud string) (*models.User, error) {
+	tokenType, ok := verifyTypeToTokenType(params.Type)
+	if !ok {
+		// The caller only routes phone types here, so in practice this should never happen.
+		return nil, apierrors.NewInternalServerError("Twilio Verify lookup called for unknown verification type %q", params.Type)
+	}
+
+	ott, err := models.FindOneTimeTokenByRelatesTo(conn, params.Phone, tokenType)
+	if models.IsNotFoundError(err) {
+		return nil, apierrors.NewForbiddenError(apierrors.ErrorCodeOTPExpired, "Token has expired or is invalid").WithInternalError(err)
+	} else if err != nil {
+		return nil, apierrors.NewInternalServerError("Database error finding one time token").WithInternalError(err)
+	}
+
+	user, err := models.FindUserByID(conn, ott.UserID)
+	if models.IsNotFoundError(err) {
+		return nil, apierrors.NewForbiddenError(apierrors.ErrorCodeOTPExpired, "Token has expired or is invalid").WithInternalError(err)
+	} else if err != nil {
+		return nil, apierrors.NewInternalServerError("Database error finding user").WithInternalError(err)
+	}
+
+	if err := validateUserForOTT(params, ott, user, aud); err != nil {
+		return nil, err
+	}
+
+	if user.IsBanned() {
+		return nil, apierrors.NewForbiddenError(apierrors.ErrorCodeUserBanned, "User is banned")
+	}
+	if err := a.verifyOTPWithTwilio(params.Phone, params.Token); err != nil {
+		return nil, apierrors.NewForbiddenError(apierrors.ErrorCodeOTPExpired, "Token has expired or is invalid").WithInternalError(err)
+	}
+	return user, nil
+}
+
+// findUserForTestOTP resolves the user for a phone verification whose code
+// matched a configured test OTP. A test OTP has no local challenge.
+func (a *API) findUserForTestOTP(conn *storage.Connection, params *VerifyParams, aud string) (*models.User, error) {
+	var user *models.User
+	var err error
+
+	switch params.Type {
+	case phoneChangeVerification:
+		user, err = models.FindUserByPhoneChangeAndAudience(conn, params.Phone, aud)
+	case smsVerification:
+		user, err = models.FindUserByPhoneAndAudience(conn, params.Phone, aud)
+	default:
+		// The caller only routes phone types here, so in practice this should never happen.
+		return nil, apierrors.NewInternalServerError("Test OTP lookup called for non-phone verification type %q", params.Type)
+	}
+	if models.IsNotFoundError(err) {
+		return nil, apierrors.NewForbiddenError(apierrors.ErrorCodeOTPExpired, "Token has expired or is invalid").WithInternalError(err)
+	} else if err != nil {
+		return nil, apierrors.NewInternalServerError("Database error finding user").WithInternalError(err)
+	}
+
+	if user.IsBanned() {
+		return nil, apierrors.NewForbiddenError(apierrors.ErrorCodeUserBanned, "User is banned")
+	}
+	return user, nil
+}
+
+// verifyOTPWithTwilio asks Twilio Verify to check the code. Twilio generates
+// and delivers its own code, so there is no local challenge to compare.
+func (a *API) verifyOTPWithTwilio(phone, code string) error {
+	smsProvider, err := sms_provider.GetSmsProvider(*a.config)
+	if err != nil {
+		return apierrors.NewInternalServerError("Failed to get SMS provider").WithInternalError(err)
+	}
+	twilioVerify, ok := smsProvider.(*sms_provider.TwilioVerifyProvider)
+	if !ok {
+		return apierrors.NewInternalServerError("SMS provider is not Twilio Verify")
+	}
+	if err := twilioVerify.VerifyOTP(phone, code); err != nil {
+		return apierrors.NewForbiddenError(apierrors.ErrorCodeOTPExpired, "Token has expired or is invalid").WithInternalError(err)
+	}
+	return nil
+}
+
 func verifyOneTimeToken(conn *storage.Connection, params *VerifyParams) (*models.OneTimeToken, error) {
 	tokenTypes := verifyTypeToTokenTypes(params.Type)
 	if len(tokenTypes) == 0 {
@@ -63,22 +151,38 @@ func verifyOneTimeToken(conn *storage.Connection, params *VerifyParams) (*models
 	return ott, nil
 }
 
+// verifyTypeToTokenTypes returns nil for an unknown verification type.
 func verifyTypeToTokenTypes(verifyType string) []models.OneTimeTokenType {
 	switch verifyType {
 	case mail.EmailOTPVerification:
 		return []models.OneTimeTokenType{models.ConfirmationToken, models.RecoveryToken}
-	case mail.SignupVerification, mail.InviteVerification:
-		return []models.OneTimeTokenType{models.ConfirmationToken}
-	case mail.RecoveryVerification, mail.MagicLinkVerification:
-		return []models.OneTimeTokenType{models.RecoveryToken}
 	case mail.EmailChangeVerification:
 		return []models.OneTimeTokenType{models.EmailChangeTokenCurrent, models.EmailChangeTokenNew}
-	case phoneChangeVerification:
-		return []models.OneTimeTokenType{models.PhoneChangeToken}
+	}
+
+	tokenType, ok := verifyTypeToTokenType(verifyType)
+	if !ok {
+		return nil
+	}
+	return []models.OneTimeTokenType{tokenType}
+}
+
+// verifyTypeToTokenType maps a verification type that has exactly one token
+// type. ok is false for an unknown type. ConfirmationToken is the zero value of
+// OneTimeTokenType, so callers must check ok instead of the returned type.
+func verifyTypeToTokenType(verifyType string) (models.OneTimeTokenType, bool) {
+	switch verifyType {
+	case mail.SignupVerification, mail.InviteVerification:
+		return models.ConfirmationToken, true
+	case mail.RecoveryVerification, mail.MagicLinkVerification:
+		return models.RecoveryToken, true
 	case smsVerification:
-		return []models.OneTimeTokenType{models.ConfirmationToken}
+		// phone signup codes are stored as confirmation tokens
+		return models.ConfirmationToken, true
+	case phoneChangeVerification:
+		return models.PhoneChangeToken, true
 	default:
-		return nil
+		return 0, false
 	}
 }
 

internal/api/verify_ott_parity_test.go

@@ -9,8 +9,11 @@ import (
 
 	"github.com/gofrs/uuid"
 	"github.com/stretchr/testify/require"
+	"gopkg.in/h2non/gock.v1"
 
 	"github.com/supabase/auth/internal/api/apierrors"
+	"github.com/supabase/auth/internal/api/sms_provider"
+	"github.com/supabase/auth/internal/conf"
 	"github.com/supabase/auth/internal/crypto"
 	mail "github.com/supabase/auth/internal/mailer"
 	"github.com/supabase/auth/internal/models"
@@ -21,12 +24,13 @@ import (
 //
 // These tests run every flow once per store with identical seeding and ensure the outcome is equal.
 const (
-	parityOTP       = "123456"
-	parityEmail     = "test@example.com"
-	parityPhone     = "12345678"
-	parityNewEmail  = "new@example.com"
-	parityNewPhone  = "1234567890"
-	parityForbidden = "Token has expired or is invalid"
+	parityOTP        = "123456"
+	parityEmail      = "test@example.com"
+	parityPhone      = "12345678"
+	parityNewEmail   = "new@example.com"
+	parityNewPhone   = "1234567890"
+	parityForbidden  = "Token has expired or is invalid"
+	twilioServiceSid = "VA-parity-test"
 )
 
 // otpParityOutcome is everything a client or an operator can observe after a
@@ -260,6 +264,58 @@ func (ts *VerifyTestSuite) TestVerifyOTPParityPhoneFlows() {
 			requestBody: phoneOTPBody(phoneChangeVerification, parityNewPhone),
 			expected:    phoneChanged,
 		},
+		// A test OTP is accepted without any stored challenge. This is the
+		// path app store reviewers and CI rely on.
+		"sms with a test OTP succeeds with no stored challenge": {
+			configure: func() func() {
+				return ts.configureTestOTP(parityPhone, parityOTP)
+			},
+			requestBody: phoneOTPBody(smsVerification, parityPhone),
+			expected:    phoneSignedUp,
+		},
+		"sms with a wrong code falls through the test OTP check and is rejected": {
+			configure: func() func() {
+				return ts.configureTestOTP(parityPhone, "000000")
+			},
+			requestBody: phoneOTPBody(smsVerification, parityPhone),
+			expected:    forbidden,
+		},
+		// Twilio Verify generates and delivers its own code, so the locally
+		// stored hash never matches what the user types. Twilio's answer is the
+		// only thing that counts.
+		"sms with Twilio Verify accepts a code Twilio approves": {
+			configure: func() func() {
+				return ts.configureTwilioVerify(map[string]interface{}{"status": "approved", "valid": true})
+			},
+			seed: func(u *models.User) {
+				ts.seedChallenge(u, models.ConfirmationToken, parityPhone, crypto.GenerateTokenHash(parityPhone, "999999"), now, time.Hour)
+			},
+			requestBody: phoneOTPBody(smsVerification, parityPhone),
+			expected:    phoneSignedUp,
+		},
+		"sms with Twilio Verify rejects a code Twilio does not approve": {
+			configure: func() func() {
+				return ts.configureTwilioVerify(map[string]interface{}{"status": "pending", "valid": false})
+			},
+			seed: func(u *models.User) {
+				ts.seedChallenge(u, models.ConfirmationToken, parityPhone, phoneHash, now, time.Hour)
+			},
+			requestBody: phoneOTPBody(smsVerification, parityPhone),
+			expected:    forbidden,
+		},
+		// An SSO user has no local credentials to verify. Twilio approves the
+		// code here, so only the SSO check can reject the request.
+		"sms with Twilio Verify rejects an SSO user": {
+			configure: func() func() {
+				return ts.configureTwilioVerify(map[string]interface{}{"status": "approved", "valid": true})
+			},
+			seed: func(u *models.User) {
+				u.IsSSOUser = true
+				ts.seedChallenge(u, models.ConfirmationToken, parityPhone, phoneHash, now, time.Hour)
+			},
+			requestBody: phoneOTPBody(smsVerification, parityPhone),
+			expected:    forbidden,
+		},
 	}
 
 	ts.runOTPParityCases(cases)
@@ -336,8 +392,8 @@ func (ts *VerifyTestSuite) saveUser(u *models.User) {
 
 // seedChallenge stores hash in the users column and the one_time_tokens row
 // for tokenType, mirroring what the send paths write. relatesTo is the address
-// or number the code was sent to. It persists u, so it also saves any other
-// change the case made.
+// or number the code was sent to; the Twilio Verify path finds the row by it.
+// It persists u, so it also saves any other change the case made.
 func (ts *VerifyTestSuite) seedChallenge(u *models.User, tokenType models.OneTimeTokenType, relatesTo, hash string, sentAt time.Time, validity time.Duration) {
 	switch tokenType {
 	case models.ConfirmationToken:
@@ -488,6 +544,42 @@ func (ts *VerifyTestSuite) responseMsg(w *httptest.ResponseRecorder) string {
 	return body.Msg
 }
 
+func (ts *VerifyTestSuite) configureTestOTP(phone, otp string) func() {
+	previous := ts.Config.Sms.TestOTP
+	ts.Config.Sms.TestOTP = map[string]string{phone: otp}
+	return func() { ts.Config.Sms.TestOTP = previous }
+}
+
+// configureTwilioVerify switches the SMS provider to Twilio Verify and arms a
+// single mocked VerificationCheck response.
+func (ts *VerifyTestSuite) configureTwilioVerify(response map[string]interface{}) func() {
+	previousProvider := ts.Config.Sms.Provider
+	previousTwilio := ts.Config.Sms.TwilioVerify
+	previousMock := sms_provider.MockProvider
+
+	ts.Config.Sms.Provider = "twilio_verify"
+	ts.Config.Sms.TwilioVerify = conf.TwilioVerifyProviderConfiguration{
+		AccountSid:        "AC-parity-test",
+		AuthToken:         "parity-test-token",
+		MessageServiceSid: twilioServiceSid,
+	}
+	// The mock provider would short-circuit GetSmsProvider and never reach
+	// the Twilio Verify type assertion.
+	sms_provider.MockProvider = nil
+
+	gock.New("https://verify.twilio.com/v2/Services/" + twilioServiceSid + "/VerificationCheck").
+		Post("").
+		Reply(http.StatusOK).
+		JSON(response)
+
+	return func() {
+		gock.OffAll()
+		sms_provider.MockProvider = previousMock
+		ts.Config.Sms.TwilioVerify = previousTwilio
+		ts.Config.Sms.Provider = previousProvider
+	}
+}
+
 func emailOTPBody(verifyType, email string) map[string]interface{} {
 	return map[string]interface{}{
 		"type":  verifyType,