From 011e6c92d0f9c75c425d491bfdc48eb4768cec1c Mon Sep 17 00:00:00 2001
From: carlganz <carlganz@gmail.com>
Date: Fri, 15 Nov 2019 14:44:16 -0800
Subject: [PATCH] WIP: pass errors from webhooks to client

---
 api/hook_test.go | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 api/hooks.go     | 28 ++++++++++++++++++++++++----
 2 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/api/hook_test.go b/api/hook_test.go
index 3cf834bb2..88f25b38b 100644
--- a/api/hook_test.go
+++ b/api/hook_test.go
@@ -9,10 +9,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/gobuffalo/uuid"
 	"github.com/netlify/gotrue/conf"
 	"github.com/netlify/gotrue/models"
 	"github.com/netlify/gotrue/storage/test"
-	"github.com/gobuffalo/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -171,4 +171,47 @@ func TestHookNoServer(t *testing.T) {
 	assert.Equal(t, http.StatusBadGateway, herr.Code)
 }
 
+func TestHookErrorParsing(t *testing.T) {
+	globalConfig, err := conf.LoadGlobal(apiTestConfig)
+	require.NoError(t, err)
+
+	conn, err := test.SetupDBConnection(globalConfig)
+	require.NoError(t, err)
+
+	iid := uuid.Must(uuid.NewV4())
+	user, err := models.NewUser(iid, "test@truth.com", "thisisapassword", "", nil)
+	require.NoError(t, err)
+
+	var callCount int
+	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		callCount++
+		defer squash(r.Body.Close)
+		raw, err := ioutil.ReadAll(r.Body)
+		require.NoError(t, err)
+		// create custom error in webhook
+		var data map[string]string
+		data["WebhookError"] = "Test Error"
+		require.NoError(t, json.Unmarshal(raw, &data))
+
+		assert.Equal(t, "Test Error", data["WebhookError"])
+		// generate error
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer svr.Close()
+
+	config := &conf.Configuration{
+		Webhook: conf.WebhookConfig{
+			Events: []string{"signup"},
+		},
+	}
+
+	ctx := context.Background()
+	ctx = withFunctionHooks(ctx, map[string]string{
+		"signup": svr.URL,
+	})
+	err = triggerHook(ctx, conn, SignupEvent, user, iid, config)
+	// check if error message was parsed correctly
+	assert.Equal(t, "Test Error", err.Error())
+}
+
 func squash(f func() error) { _ = f }
diff --git a/api/hooks.go b/api/hooks.go
index c2ac6ead0..6f37eddee 100644
--- a/api/hooks.go
+++ b/api/hooks.go
@@ -14,8 +14,8 @@ import (
 	"time"
 
 	jwt "github.com/dgrijalva/jwt-go"
-	"github.com/pkg/errors"
 	"github.com/gobuffalo/uuid"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 
 	"github.com/netlify/gotrue/conf"
@@ -54,6 +54,7 @@ type Webhook struct {
 type WebhookResponse struct {
 	AppMetaData  map[string]interface{} `json:"app_metadata,omitempty"`
 	UserMetaData map[string]interface{} `json:"user_metadata,omitempty"`
+	WebhookError string                 `json:"webhook_error,omitempty"`
 }
 
 func (w *Webhook) trigger() (io.ReadCloser, error) {
@@ -76,7 +77,8 @@ func (w *Webhook) trigger() (io.ReadCloser, error) {
 		"signed":      w.jwtSecret != "",
 		"instance_id": w.instanceID,
 	})
-
+	// will return body even if there is error so may as well initiate at top
+	var body io.ReadCloser
 	for i := 0; i < w.Retries; i++ {
 		hooklog = hooklog.WithField("attempt", i+1)
 		hooklog.Info("Starting to perform signup hook request")
@@ -120,21 +122,28 @@ func (w *Webhook) trigger() (io.ReadCloser, error) {
 			"status_code": rsp.StatusCode,
 			"dur":         dur.Nanoseconds(),
 		})
+
 		switch rsp.StatusCode {
 		case http.StatusOK, http.StatusNoContent, http.StatusAccepted:
 			rspLog.Infof("Finished processing webhook in %s", dur)
-			var body io.ReadCloser
 			if rsp.ContentLength > 0 {
 				body = rsp.Body
 			}
 			return body, nil
 		default:
 			rspLog.Infof("Bad response for webhook %d in %s", rsp.StatusCode, dur)
+			// if there is body let's return it so triggerHook can handle error parsing
+			if rsp.ContentLength > 0 {
+				body = rsp.Body
+			}
+			hooklog.Infof("Failed to process webhook for %s after %d attempts", w.URL, w.Retries)
+			// return body and non nil error which will be handled in triggerHook
+			return body, unprocessableEntityError("Failed to handle signup webhook")
 		}
 	}
 
 	hooklog.Infof("Failed to process webhook for %s after %d attempts", w.URL, w.Retries)
-	return nil, unprocessableEntityError("Failed to handle signup webhook")
+	return body, unprocessableEntityError("Failed to handle signup webhook")
 }
 
 func (w *Webhook) generateSignature() (string, error) {
@@ -260,6 +269,17 @@ func triggerHook(ctx context.Context, conn *storage.Connection, event HookEvent,
 			}
 			return nil
 		})
+	} else if err != nil && body != nil {
+		// parse error message from body
+		webhookRsp := &WebhookResponse{}
+		decoder := json.NewDecoder(body)
+		if err = decoder.Decode(webhookRsp); err != nil {
+			return internalServerError("Webhook returned malformed JSON: %v", err).WithInternalError(err)
+		}
+		// if there is error message in format we expect replace default error message with the one specified by webhook
+		if webhookRsp.WebhookError != "" {
+			return unprocessableEntityError(webhookRsp.WebhookError)
+		}
 	}
 	return err
 }