fix: patch secure email change (double confirm) response format. (#1241)

## What kind of change does this PR introduce?

There are two issues the PR aims to resolve:

1. Currently, a Token Hash can be re-used twice in place of using the
token hash send to the new email and a token has in the current mail. A
solve attempt was originally made in #1240 but a test was added in this
branch.

2. Currently, the single confirmation response is slightly misformed and
has an additional null param

<img width="1062" alt="CleanShot 2023-09-04 at 15 47 04@2x"
src="https://github.com/supabase/gotrue/assets/8011761/69da91e5-e646-4970-8e80-1659e2e3fd41">

This stems from the return in the transaction. sendJSON doesn't return
an error. Consequently, he error returned by the transaction will be
nil. This leads to

<img width="755" alt="CleanShot 2023-09-04 at 15 47 41@2x"
src="https://github.com/supabase/gotrue/assets/8011761/af583492-1aac-4cbd-aaad-856282cce808">

`sendJSON(w, http.StatusOK, token)` being run after `sendJSON` is
callled which will write the `token` (`nil` in this case) to the
existing singleConfirmationResponse. This in turn affects returned
response for the first confirmation as the client library is unable to
unpack the returned JSON with extra null leading to an error.


## What is the new behavior?

Returns response
<img width="617" alt="CleanShot 2023-09-04 at 15 50 07@2x"
src="https://github.com/supabase/gotrue/assets/8011761/e27db0ab-0489-4cda-a25f-8a650db5cab1">

## Additional context

TODO
- [x] Need to complete a test for the SecureEmailChange TokenHash to
prevent a regression

---------

Co-authored-by: [email protected] <[email protected]>

internal/api/verify.go

@@ -231,6 +231,7 @@ func (a *API) verifyPost(w http.ResponseWriter, r *http.Request, params *VerifyP
 		grantParams models.GrantParams
 		token       *AccessTokenResponse
 	)
+	var isSingleConfirmationResponse = false
 
 	err := db.Transaction(func(tx *storage.Connection) error {
 		var terr error
@@ -253,10 +254,8 @@ func (a *API) verifyPost(w http.ResponseWriter, r *http.Request, params *VerifyP
 		case emailChangeVerification:
 			user, terr = a.emailChangeVerify(r, ctx, tx, params, user)
 			if user == nil && terr == nil {
-				return sendJSON(w, http.StatusOK, map[string]string{
-					"msg":  singleConfirmationAccepted,
-					"code": strconv.Itoa(http.StatusOK),
-				})
+				isSingleConfirmationResponse = true
+				return nil
 			}
 		case smsVerification, phoneChangeVerification:
 			user, terr = a.smsVerify(r, ctx, tx, user, params.Type)
@@ -280,6 +279,12 @@ func (a *API) verifyPost(w http.ResponseWriter, r *http.Request, params *VerifyP
 	if err != nil {
 		return err
 	}
+	if isSingleConfirmationResponse {
+		return sendJSON(w, http.StatusOK, map[string]string{
+			"msg":  singleConfirmationAccepted,
+			"code": strconv.Itoa(http.StatusOK),
+		})
+	}
 	return sendJSON(w, http.StatusOK, token)
 }
 
@@ -448,9 +453,9 @@ func (a *API) emailChangeVerify(r *http.Request, ctx context.Context, conn *stor
 	if config.Mailer.SecureEmailChangeEnabled && user.EmailChangeConfirmStatus == zeroConfirmation && user.GetEmail() != "" {
 		err := conn.Transaction(func(tx *storage.Connection) error {
 			user.EmailChangeConfirmStatus = singleConfirmation
-			if params.Token == user.EmailChangeTokenCurrent {
+			if params.Token == user.EmailChangeTokenCurrent || params.TokenHash == user.EmailChangeTokenCurrent {
 				user.EmailChangeTokenCurrent = ""
-			} else if params.Token == user.EmailChangeTokenNew {
+			} else if params.Token == user.EmailChangeTokenNew || params.TokenHash == user.EmailChangeTokenNew {
 				user.EmailChangeTokenNew = ""
 			}
 			if terr := tx.UpdateOnly(user, "email_change_confirm_status", "email_change_token_current", "email_change_token_new"); terr != nil {

internal/api/verify_test.go

@@ -622,7 +622,6 @@ func (ts *VerifyTestSuite) TestVerifySignupWithredirectURLContainedPath() {
 }
 
 func (ts *VerifyTestSuite) TestVerifyPKCEOTP() {
-
 	u, err := models.FindUserByEmailAndAudience(ts.API.db, "[email protected]", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)
 	u.ConfirmationToken = "pkce_confirmation_token"
@@ -935,6 +934,83 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() {
 	}
 }
 
+func (ts *VerifyTestSuite) TestSecureEmailChangeWithTokenHash() {
+	ts.Config.Mailer.SecureEmailChangeEnabled = true
+	u, err := models.FindUserByEmailAndAudience(ts.API.db, "[email protected]", ts.Config.JWT.Aud)
+	require.NoError(ts.T(), err)
+	u.EmailChange = "[email protected]"
+	require.NoError(ts.T(), ts.API.db.Update(u))
+
+	currentEmailChangeToken := crypto.GenerateTokenHash(string(u.Email), "123456")
+	newEmailChangeToken := crypto.GenerateTokenHash(u.EmailChange, "123456")
+
+	cases := []struct {
+		desc                   string
+		firstVerificationBody  map[string]interface{}
+		secondVerificationBody map[string]interface{}
+		expectedStatus         int
+	}{
+		{
+			desc: "Secure Email Change with Token Hash (Success)",
+			firstVerificationBody: map[string]interface{}{
+				"type":       emailChangeVerification,
+				"token_hash": currentEmailChangeToken,
+			},
+			secondVerificationBody: map[string]interface{}{
+				"type":       emailChangeVerification,
+				"token_hash": newEmailChangeToken,
+			},
+			expectedStatus: http.StatusOK,
+		},
+		{
+			desc: "Secure Email Change with Token Hash. Reusing a token hash twice should fail",
+			firstVerificationBody: map[string]interface{}{
+				"type":       emailChangeVerification,
+				"token_hash": currentEmailChangeToken,
+			},
+			secondVerificationBody: map[string]interface{}{
+				"type":       emailChangeVerification,
+				"token_hash": currentEmailChangeToken,
+			},
+			expectedStatus: http.StatusUnauthorized,
+		},
+	}
+	for _, c := range cases {
+		ts.Run(c.desc, func() {
+			// Set the corresponding email change tokens
+			u.EmailChangeTokenCurrent = currentEmailChangeToken
+			u.EmailChangeTokenNew = newEmailChangeToken
+
+			currentTime := time.Now()
+			u.EmailChangeSentAt = &currentTime
+			require.NoError(ts.T(), ts.API.db.Update(u))
+
+			var buffer bytes.Buffer
+			require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(c.firstVerificationBody))
+
+			// Setup request
+			req := httptest.NewRequest(http.MethodPost, "http://localhost/verify", &buffer)
+			req.Header.Set("Content-Type", "application/json")
+
+			// Setup response recorder
+			w := httptest.NewRecorder()
+			ts.API.handler.ServeHTTP(w, req)
+			require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(c.secondVerificationBody))
+
+			// Setup second request
+			req = httptest.NewRequest(http.MethodPost, "http://localhost/verify", &buffer)
+			req.Header.Set("Content-Type", "application/json")
+
+			// Setup second response recorder
+			w = httptest.NewRecorder()
+			ts.API.handler.ServeHTTP(w, req)
+			assert.Equal(ts.T(), c.expectedStatus, w.Code)
+		})
+
+	}
+
+}
+
 func (ts *VerifyTestSuite) TestPrepRedirectURL() {
 	escapedMessage := url.QueryEscape(singleConfirmationAccepted)
 	cases := []struct {