Repository: supabase/gotrue
Pull request #391 (2022-02-23 16:12:37)

fix: add nullif to auth functions
## What kind of change does this PR introduce?
* Fixes https://github.com/supabase/supabase/issues/4794 
* Related to https://github.com/supabase/supabase/pull/4871 


Patch:
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index 9fa241b2..29253b21 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -16,6 +16,7 @@ jobs:
       status: ${{ steps.pre-release.outputs.release != steps.post-release.outputs.release }}
 
     runs-on: ubuntu-18.04
+    if: ${{ github.event.workflow_run.conclusion == 'success' }}
     steps:
       - uses: actions/checkout@v2
 
diff --git a/api/recover.go b/api/recover.go
index 05de55d2..2853e754 100644
--- a/api/recover.go
+++ b/api/recover.go
@@ -32,11 +32,12 @@ func (a *API) Recover(w http.ResponseWriter, r *http.Request) error {
 
 	aud := a.requestAud(ctx, r)
 	user, err := models.FindUserByEmailAndAudience(a.db, instanceID, params.Email, aud)
+	recoverErrorMessage := "If a user exists, you will receive an email with instructions on how to reset your password."
 	if err != nil {
 		if models.IsNotFoundError(err) {
 			return notFoundError(err.Error())
 		}
-		return internalServerError("Database error finding user").WithInternalError(err)
+		return internalServerError(recoverErrorMessage).WithInternalError(err)
 	}
 
 	err = a.db.Transaction(func(tx *storage.Connection) error {
@@ -52,7 +53,7 @@ func (a *API) Recover(w http.ResponseWriter, r *http.Request) error {
 		if errors.Is(err, MaxFrequencyLimitError) {
 			return tooManyRequestsError("For security purposes, you can only request this once every 60 seconds")
 		}
-		return internalServerError("Error recovering user").WithInternalError(err)
+		return internalServerError(recoverErrorMessage).WithInternalError(err)
 	}
 
 	return sendJSON(w, http.StatusOK, &map[string]string{})
diff --git a/api/verify.go b/api/verify.go
index 3c5f8b69..a48aca9f 100644
--- a/api/verify.go
+++ b/api/verify.go
@@ -82,14 +82,6 @@ func (a *API) Verify(w http.ResponseWriter, r *http.Request) error {
 		aud := a.requestAud(ctx, r)
 		user, terr = a.verifyUserAndToken(ctx, tx, params, aud)
 		if terr != nil {
-			var herr *HTTPError
-			if errors.As(terr, &herr) {
-				if errors.Is(herr.InternalError, redirectWithQueryError) {
-					rurl := a.prepErrorRedirectURL(herr, r, params.RedirectTo)
-					http.Redirect(w, r, rurl, http.StatusSeeOther)
-					return nil
-				}
-			}
 			return terr
 		}
 
@@ -126,8 +118,20 @@ func (a *API) Verify(w http.ResponseWriter, r *http.Request) error {
 		}
 		return nil
 	})
+
 	if err != nil {
-		return err
+		if r.Method == http.MethodPost {
+			// do not redirect POST requests
+			return err
+		}
+		var herr *HTTPError
+		if errors.As(err, &herr) {
+			if errors.Is(herr.InternalError, redirectWithQueryError) {
+				rurl := a.prepErrorRedirectURL(herr, r, params.RedirectTo)
+				http.Redirect(w, r, rurl, http.StatusSeeOther)
+				return nil
+			}
+		}
 	}
 
 	// GET requests should return to the app site after confirmation
diff --git a/migrations/20220224000811_update_auth_functions.up.sql b/migrations/20220224000811_update_auth_functions.up.sql
new file mode 100644
index 00000000..689b5440
--- /dev/null
+++ b/migrations/20220224000811_update_auth_functions.up.sql
@@ -0,0 +1,34 @@
+-- update auth functions
+
+create or replace function auth.uid() 
+returns uuid 
+language sql stable
+as $$
+  select 
+  	coalesce(
+		nullif(current_setting('request.jwt.claim.sub', true), ''),
+		(nullif(current_setting('request.jwt.claims', true), '')::jsonb ->> 'sub')
+	)::uuid
+$$;
+
+create or replace function auth.role() 
+returns text 
+language sql stable
+as $$
+  select 
+  	coalesce(
+		nullif(current_setting('request.jwt.claim.role', true), ''),
+		(nullif(current_setting('request.jwt.claims', true), '')::jsonb ->> 'role')
+	)::text
+$$;
+
+create or replace function auth.email() 
+returns text 
+language sql stable
+as $$
+  select 
+  	coalesce(
+		nullif(current_setting('request.jwt.claim.email', true), ''),
+		(nullif(current_setting('request.jwt.claims', true), '')::jsonb ->> 'email')
+	)::text
+$$;


Test patch:
diff --git a/api/verify_test.go b/api/verify_test.go
index 979b53b4..65c4dc7f 100644
--- a/api/verify_test.go
+++ b/api/verify_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -173,16 +174,9 @@ func (ts *VerifyTestSuite) TestExpiredConfirmationToken() {
 	u.ConfirmationSentAt = &sentTime
 	require.NoError(ts.T(), ts.API.db.Update(u))
 
-	// Request body
-	var buffer bytes.Buffer
-	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
-		"type":  signupVerification,
-		"token": u.ConfirmationToken,
-	}))
-
 	// Setup request
-	req := httptest.NewRequest(http.MethodPost, "http://localhost/verify", &buffer)
-	req.Header.Set("Content-Type", "application/json")
+	reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", signupVerification, u.ConfirmationToken)
+	req := httptest.NewRequest(http.MethodGet, reqURL, nil)
 
 	// Setup response recorder
 	w := httptest.NewRecorder()
@@ -203,21 +197,21 @@ func (ts *VerifyTestSuite) TestInvalidOtp() {
 	u.ConfirmationSentAt = &sentTime
 	require.NoError(ts.T(), ts.API.db.Update(u))
 
-	type expected struct {
-		code      int
-		fragments string
+	type ResponseBody struct {
+		Code int    `json:"code"`
+		Msg  string `json:"msg"`
 	}
 
-	expectedResponse := expected{
-		code:      http.StatusSeeOther,
-		fragments: "error_code=410&error_description=Token+has+expired+or+is+invalid",
+	expectedResponse := ResponseBody{
+		Code: http.StatusGone,
+		Msg:  "Token has expired or is invalid",
 	}
 
 	cases := []struct {
 		desc     string
 		sentTime time.Time
 		body     map[string]interface{}
-		expected
+		expected ResponseBody
 	}{
 		{
 			desc:     "Expired Sms OTP",
@@ -268,11 +262,15 @@ func (ts *VerifyTestSuite) TestInvalidOtp() {
 			// Setup response recorder
 			w := httptest.NewRecorder()
 			ts.API.handler.ServeHTTP(w, req)
-			assert.Equal(ts.T(), c.expected.code, w.Code)
 
-			url, err := w.Result().Location()
+			b, err := ioutil.ReadAll(w.Body)
 			require.NoError(ts.T(), err)
-			assert.Equal(ts.T(), c.expected.fragments, url.Fragment)
+			var resp ResponseBody
+			err = json.Unmarshal(b, &resp)
+			require.NoError(ts.T(), err)
+			assert.Equal(ts.T(), c.expected.Code, resp.Code)
+			assert.Equal(ts.T(), c.expected.Msg, resp.Msg)
+
 		})
 	}
 }
@@ -285,16 +283,9 @@ func (ts *VerifyTestSuite) TestExpiredRecoveryToken() {
 	u.RecoverySentAt = &sentTime
 	require.NoError(ts.T(), ts.API.db.Update(u))
 
-	// Request body
-	var buffer bytes.Buffer
-	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
-		"type":  recoveryVerification,
-		"token": u.RecoveryToken,
-	}))
-
 	// Setup request
-	req := httptest.NewRequest(http.MethodPost, "http://localhost/verify", &buffer)
-	req.Header.Set("Content-Type", "application/json")
+	reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", "signup", u.RecoveryToken)
+	req := httptest.NewRequest(http.MethodGet, reqURL, nil)
 
 	// Setup response recorder
 	w := httptest.NewRecorder()
@@ -528,11 +519,11 @@ func (ts *VerifyTestSuite) TestVerifyBannedUser() {
 
 			w := httptest.NewRecorder()
 			ts.API.handler.ServeHTTP(w, req)
-			assert.Equal(ts.T(), http.StatusSeeOther, w.Code)
+			assert.Equal(ts.T(), http.StatusUnauthorized, w.Code)
 
-			url, err := w.Result().Location()
+			b, err := ioutil.ReadAll(w.Body)
 			require.NoError(ts.T(), err)
-			assert.Equal(ts.T(), "error=unauthorized_client&error_code=401&error_description=Error+confirming+user", url.Fragment)
+			assert.Equal(ts.T(), "{\"code\":401,\"msg\":\"Error confirming user\"}", string(b))
 		})
 	}
 }