chore: remove unnecessary comments & check for pkce prefix in verify

hack/test.env

@@ -15,6 +15,7 @@ GOTRUE_LOG_LEVEL=info
 GOTRUE_SITE_URL=https://example.netlify.com
 GOTRUE_URI_ALLOW_LIST="http://localhost:3000"
 GOTRUE_OPERATOR_TOKEN=foobar
+GOTRUE_API_MAX_REQUEST_DURATION="0"
 GOTRUE_EXTERNAL_APPLE_ENABLED=true
 GOTRUE_EXTERNAL_APPLE_CLIENT_ID=testclientid
 GOTRUE_EXTERNAL_APPLE_SECRET=testsecret

internal/api/verify.go

@@ -604,7 +604,7 @@ func (a *API) verifyTokenHash(conn *storage.Connection, params *VerifyParams) (*
 	case mail.EmailOTPVerification:
 		sentAt := user.ConfirmationSentAt
 		params.Type = "signup"
-		if _, err := models.FindOneTimeToken(conn, params.TokenHash, models.ReauthenticationToken); err != nil {
+		if _, err := models.FindOneTimeToken(conn, params.TokenHash, models.RecoveryToken); err != nil {
 			if !models.IsNotFoundError(err) {
 				return nil, internalServerError("Database error finding token").WithInternalError(err)
 			}
@@ -665,7 +665,6 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams,
 	switch params.Type {
 	case mail.EmailOTPVerification:
 		// if the type is emailOTPVerification, we'll check both the confirmation_token and recovery_token columns
-		// if isOtpValid(tokenHash, user.ConfirmationToken, user.ConfirmationSentAt, config.Mailer.OtpExp) {
 		isValid, otpErr = isOtpValid(conn, tokenHash, config.Mailer.OtpExp, models.ConfirmationToken)
 		if otpErr == nil {
 			params.Type = mail.SignupVerification
@@ -676,7 +675,6 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams,
 			params.Type = mail.MagicLinkVerification
 			break
 		}
-		// need some way to figure out whether to set the param type to signup or magiclink
 	case mail.SignupVerification, mail.InviteVerification:
 		isValid, otpErr = isOtpValid(conn, tokenHash, config.Mailer.OtpExp, models.ConfirmationToken)
 	case mail.RecoveryVerification, mail.MagicLinkVerification:
@@ -716,16 +714,21 @@ func (a *API) verifyUserAndToken(conn *storage.Connection, params *VerifyParams,
 func isOtpValid(tx *storage.Connection, tokenHash string, otpExp uint, tokenTypes ...models.OneTimeTokenType) (bool, error) {
 	token, err := models.FindOneTimeToken(tx, tokenHash, tokenTypes...)
 	if err != nil {
-		if models.IsNotFoundError(err) {
-			return false, nil
+		if !models.IsNotFoundError(err) {
+			return false, err
+		}
+		// try again with the pkce prefix
+		token, err = models.FindOneTimeToken(tx, "pkce_"+tokenHash, tokenTypes...)
+		if err != nil {
+			if models.IsNotFoundError(err) {
+				return false, nil
+			}
+			return false, err
 		}
-		return false, err
 	}
 	if isOtpExpired(&token.CreatedAt, otpExp) {
 		return false, nil
 	}
-
-	// return !isOtpExpired(sentAt, otpExp) && ((actual == expected) || ("pkce_"+actual == expected))
 	return true, nil
 }
 

internal/api/verify_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/gofrs/uuid"
 	mail "github.com/supabase/auth/internal/mailer"
 
 	"github.com/stretchr/testify/assert"
@@ -748,6 +749,60 @@ func (ts *VerifyTestSuite) TestVerifyPKCEOTP() {
 
 }
 
+func (ts *VerifyTestSuite) TestVerifyPostPKCE() {
+	var buffer bytes.Buffer
+	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
+		"email":                 "[email protected]",
+		"password":              "test-password",
+		"code_challenge":        "codechallengecodechallengcodechallengcodechallengcodechallenge",
+		"code_challenge_method": "plain",
+	}))
+	req := httptest.NewRequest(http.MethodPost, "http://localhost/signup", &buffer)
+	req.Header.Set("Content-Type", "application/json")
+
+	w := httptest.NewRecorder()
+	ts.API.handler.ServeHTTP(w, req)
+	require.Equal(ts.T(), http.StatusOK, w.Code)
+
+	var data map[string]interface{}
+	require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data))
+	userId, err := uuid.FromString(data["id"].(string))
+	require.NoError(ts.T(), err)
+	u, err := models.FindUserByID(ts.API.db, userId)
+	require.NoError(ts.T(), err)
+	require.NotEmpty(ts.T(), u.OneTimeTokens)
+
+	// update the token hash to a known value for subsequent verification
+	ott := u.OneTimeTokens[0]
+	ott.TokenHash = addFlowPrefixToToken(crypto.GenerateTokenHash("[email protected]", "123456"), models.PKCEFlow)
+	require.NoError(ts.T(), ts.API.db.Update(&ott))
+
+	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
+		"email": "[email protected]",
+		"token": "123456",
+		"type":  "email",
+	}))
+	req = httptest.NewRequest(http.MethodPost, "http://localhost/verify", &buffer)
+	req.Header.Set("Content-Type", "application/json")
+
+	w = httptest.NewRecorder()
+	ts.API.handler.ServeHTTP(w, req)
+	require.Equal(ts.T(), http.StatusOK, w.Code)
+
+	// check that the response contains an access and refresh token
+	require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data))
+	require.Contains(ts.T(), data, "access_token")
+	require.Contains(ts.T(), data, "refresh_token")
+
+	// check that the user in the db is confirmed
+	u, err = models.FindUserByID(ts.API.db, userId)
+	require.NoError(ts.T(), err)
+	require.True(ts.T(), u.IsConfirmed())
+
+	// // one time tokens should be cleared after successful verification
+	require.Empty(ts.T(), u.OneTimeTokens)
+}
+
 func (ts *VerifyTestSuite) TestVerifyBannedUser() {
 	u, err := models.FindUserByEmailAndAudience(ts.API.db, "[email protected]", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)