diff --git a/internal/hooks/hookshttp/hookshttp.go b/internal/hooks/hookshttp/hookshttp.go index 36df0977f..a06826ca7 100644 --- a/internal/hooks/hookshttp/hookshttp.go +++ b/internal/hooks/hookshttp/hookshttp.go @@ -17,6 +17,7 @@ import ( "github.com/sirupsen/logrus" "github.com/supabase/auth/internal/api/apierrors" "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/hooks/hookserrors" "github.com/supabase/auth/internal/observability" standardwebhooks "github.com/standard-webhooks/standard-webhooks/libraries/go" @@ -232,15 +233,24 @@ func (o *Dispatcher) runHTTPHook( } return nil, apierrors.NewInternalServerError( "Service currently unavailable due to hook") - case http.StatusBadRequest: - return nil, apierrors.NewInternalServerError( - "Invalid payload sent to hook") - case http.StatusUnauthorized: - return nil, apierrors.NewInternalServerError( - "Hook requires authorization token") default: - return nil, apierrors.NewInternalServerError( - "Unexpected status code returned from hook: %d", rsp.StatusCode) + body, err := io.ReadAll(io.LimitReader(rsp.Body, o.limitResponse)) + if err == nil && len(body) > 0 { + if err := hookserrors.Check(body); err != nil { + return nil, err + } + } + switch rsp.StatusCode { + case http.StatusBadRequest: + return nil, apierrors.NewInternalServerError( + "Invalid payload sent to hook") + case http.StatusUnauthorized: + return nil, apierrors.NewInternalServerError( + "Hook requires authorization token") + default: + return nil, apierrors.NewInternalServerError( + "Unexpected status code returned from hook: %d", rsp.StatusCode) + } } } diff --git a/internal/hooks/hookshttp/hookshttp_test.go b/internal/hooks/hookshttp/hookshttp_test.go index 875584211..b4f3d7c2f 100644 --- a/internal/hooks/hookshttp/hookshttp_test.go +++ b/internal/hooks/hookshttp/hookshttp_test.go @@ -187,11 +187,32 @@ func TestDispatch(t *testing.T) { }, } - // error status codes { - addCase := func(statusIn, statusOut int, msg string) { + addCaseWithCustomError := func(statusIn int, customHTTPCode int, customMsg string) { cases = append(cases, testCase{ - desc: fmt.Sprintf("fail - invalid status code %d", statusIn), + desc: fmt.Sprintf("fail - status %d with custom error propagates", statusIn), + req: M{"empty": true}, + errStr: fmt.Sprintf("%d: %s", customHTTPCode, customMsg), + hr: &mockHandler{ + status: statusIn, + ctype: "application/json", + data: M{ + "error": M{ + "http_code": customHTTPCode, + "message": customMsg, + }, + }, + }, + }) + } + addCaseWithCustomError(http.StatusBadRequest, 403, "Signups from this domain not allowed") + addCaseWithCustomError(http.StatusForbidden, 422, "Email validation failed") + addCaseWithCustomError(http.StatusUnauthorized, 401, "Custom auth error") + addCaseWithCustomError(http.StatusTeapot, 500, "Custom server error from hook") + + addCaseFallback := func(statusIn, statusOut int, msg string) { + cases = append(cases, testCase{ + desc: fmt.Sprintf("fail - status %d without error object falls back", statusIn), req: M{"empty": true}, err: &apierrors.HTTPError{ HTTPStatus: statusOut, @@ -201,41 +222,64 @@ func TestDispatch(t *testing.T) { hr: &mockHandler{ status: statusIn, ctype: "application/json", - data: M{ - "error": M{ - // This is not propagated in current implementation - "http_code": 500, - "message": "sentinel error", - }, - }, + data: M{"no_error_key": true}, }, }) } - addCase( - http.StatusServiceUnavailable, - http.StatusInternalServerError, - "Service currently unavailable due to hook", - ) - addCase( - http.StatusTooManyRequests, - http.StatusInternalServerError, - "Service currently unavailable due to hook", - ) - addCase( + addCaseFallback( http.StatusBadRequest, http.StatusInternalServerError, "Invalid payload sent to hook", ) - addCase( + addCaseFallback( http.StatusUnauthorized, http.StatusInternalServerError, "Hook requires authorization token", ) - addCase( + addCaseFallback( http.StatusTeapot, http.StatusInternalServerError, "Unexpected status code returned from hook: 418", ) + + cases = append(cases, testCase{ + desc: "fail - 503 uses retry logic", + req: M{"empty": true}, + err: &apierrors.HTTPError{ + HTTPStatus: http.StatusInternalServerError, + ErrorCode: apierrors.ErrorCodeUnexpectedFailure, + Message: "Service currently unavailable due to hook", + }, + hr: &mockHandler{ + status: http.StatusServiceUnavailable, + ctype: "application/json", + data: M{ + "error": M{ + "http_code": 500, + "message": "custom error ignored for 503", + }, + }, + }, + }) + cases = append(cases, testCase{ + desc: "fail - 429 uses retry logic", + req: M{"empty": true}, + err: &apierrors.HTTPError{ + HTTPStatus: http.StatusInternalServerError, + ErrorCode: apierrors.ErrorCodeUnexpectedFailure, + Message: "Service currently unavailable due to hook", + }, + hr: &mockHandler{ + status: http.StatusTooManyRequests, + ctype: "application/json", + data: M{ + "error": M{ + "http_code": 500, + "message": "custom error ignored for 429", + }, + }, + }, + }) } for _, tc := range cases {