From f446eabfd8a1b82c5548e28bdd9888ebd82cc65e Mon Sep 17 00:00:00 2001 From: Stefan Kostic Date: Sat, 2 Apr 2022 21:34:57 +0200 Subject: [PATCH 1/5] Simple way to filter out bad auth from sentry --- lib/responses/errors.go | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/responses/errors.go b/lib/responses/errors.go index e35a88c..6172be6 100644 --- a/lib/responses/errors.go +++ b/lib/responses/errors.go @@ -1,6 +1,7 @@ package responses import ( + "encoding/json" "net/http" "github.com/getsentry/sentry-go" @@ -43,7 +44,7 @@ func HTTPErrorHandler(err error, c echo.Context) { return } c.Logger().Error(err) - if hub := sentryecho.GetHubFromContext(c); hub != nil { + if hub := sentryecho.GetHubFromContext(c); hub != nil && isErrResponseAllowedForSentry(errToErrorResponse(err)) { hub.WithScope(func(scope *sentry.Scope) { scope.SetExtra("UserID", c.Get("UserID")) hub.CaptureException(err) @@ -58,3 +59,30 @@ func HTTPErrorHandler(err error, c echo.Context) { } // TODO: use an error matching the error code } + +// this is a simple way to try to convert err.Message interface to ErrorResponse +// without external packages +func errToErrorResponse(err error) *ErrorResponse { + he, ok := err.(*echo.HTTPError) + if !ok { + return nil + } + + heJson, err := json.Marshal(he.Message) + if err != nil { + return nil + } + + heBadResponse := &ErrorResponse{} + err = json.Unmarshal(heJson, heBadResponse) + if err != nil { + return nil + } + + return heBadResponse +} + +// currently only bad auth errors are not allowed +func isErrResponseAllowedForSentry(errResponse *ErrorResponse) bool { + return errResponse != nil && errResponse.Code != BadAuthError.Code +} From 29d045f7005752a01e584e60636624dfad99918e Mon Sep 17 00:00:00 2001 From: Stefan Kostic Date: Tue, 5 Apr 2022 20:12:00 +0200 Subject: [PATCH 2/5] Cleanup --- lib/responses/errors.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/responses/errors.go b/lib/responses/errors.go index 6172be6..56bee5d 100644 --- a/lib/responses/errors.go +++ b/lib/responses/errors.go @@ -44,7 +44,7 @@ func HTTPErrorHandler(err error, c echo.Context) { return } c.Logger().Error(err) - if hub := sentryecho.GetHubFromContext(c); hub != nil && isErrResponseAllowedForSentry(errToErrorResponse(err)) { + if hub := sentryecho.GetHubFromContext(c); hub != nil && isErrAllowedForSentry(err) { hub.WithScope(func(scope *sentry.Scope) { scope.SetExtra("UserID", c.Get("UserID")) hub.CaptureException(err) @@ -60,29 +60,31 @@ func HTTPErrorHandler(err error, c echo.Context) { // TODO: use an error matching the error code } -// this is a simple way to try to convert err.Message interface to ErrorResponse -// without external packages +// this is a simple way to try to convert err.Message interface +// to ErrorResponse without external packages func errToErrorResponse(err error) *ErrorResponse { - he, ok := err.(*echo.HTTPError) + httpError, ok := err.(*echo.HTTPError) if !ok { return nil } - heJson, err := json.Marshal(he.Message) + responseJson, err := json.Marshal(httpError.Message) if err != nil { return nil } - heBadResponse := &ErrorResponse{} - err = json.Unmarshal(heJson, heBadResponse) + errorResponse := &ErrorResponse{} + err = json.Unmarshal(responseJson, errorResponse) if err != nil { return nil } - return heBadResponse + return errorResponse } // currently only bad auth errors are not allowed -func isErrResponseAllowedForSentry(errResponse *ErrorResponse) bool { - return errResponse != nil && errResponse.Code != BadAuthError.Code +func isErrAllowedForSentry(err error) bool { + errResponse := errToErrorResponse(err) + + return errResponse == nil || errResponse.Code != BadAuthError.Code } From 82c36eba9792716428e0f1346669b35274a94ba4 Mon Sep 17 00:00:00 2001 From: Stefan Kostic Date: Tue, 5 Apr 2022 20:23:33 +0200 Subject: [PATCH 3/5] Add tests --- lib/responses/errors_test.go | 39 ++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 lib/responses/errors_test.go diff --git a/lib/responses/errors_test.go b/lib/responses/errors_test.go new file mode 100644 index 0000000..16ee4c3 --- /dev/null +++ b/lib/responses/errors_test.go @@ -0,0 +1,39 @@ +package responses + +import ( + "errors" + "net/http" + "testing" + + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" +) + +func TestBadAuthErrorsNotAllowedForSentry(t *testing.T) { + badAuthErrResponse := echo.NewHTTPError(http.StatusBadRequest, echo.Map{ + "error": true, + "code": 1, + "message": "bad auth", + }) + + isAllowed := isErrAllowedForSentry(badAuthErrResponse) + assert.False(t, isAllowed) +} + +func TestNotBadAuthErrorsAllowedForSentry(t *testing.T) { + notBadAuthErrResponse := echo.NewHTTPError(http.StatusBadRequest, echo.Map{ + "error": true, + "code": 2, + "message": "not bad auth", + }) + + isAllowed := isErrAllowedForSentry(notBadAuthErrResponse) + assert.True(t, isAllowed) +} + +func TestNonErrorResponseErrorsAllowedForSentry(t *testing.T) { + err := errors.New("random error") + + isAllowed := isErrAllowedForSentry(err) + assert.True(t, isAllowed) +} From e5c2e5337f620a87591cbf4de619b7f87d7bbbfc Mon Sep 17 00:00:00 2001 From: Stefan Kostic Date: Wed, 6 Apr 2022 17:15:17 +0200 Subject: [PATCH 4/5] Add ignoreErrors to sentry --- controllers/auth.ctrl.go | 2 +- lib/responses/errors.go | 32 +---------------------------- lib/responses/errors_test.go | 39 ------------------------------------ lib/tokens/jwt.go | 2 +- main.go | 3 ++- 5 files changed, 5 insertions(+), 73 deletions(-) delete mode 100644 lib/responses/errors_test.go diff --git a/controllers/auth.ctrl.go b/controllers/auth.ctrl.go index 0780566..618fae5 100644 --- a/controllers/auth.ctrl.go +++ b/controllers/auth.ctrl.go @@ -44,7 +44,7 @@ func (controller *AuthController) Auth(c echo.Context) error { accessToken, refreshToken, err := controller.svc.GenerateToken(c.Request().Context(), body.Login, body.Password, body.RefreshToken) if err != nil { - return c.JSON(http.StatusBadRequest, responses.BadAuthError) + return c.JSON(http.StatusUnauthorized, responses.BadAuthError) } return c.JSON(http.StatusOK, &AuthResponseBody{ diff --git a/lib/responses/errors.go b/lib/responses/errors.go index 56bee5d..e35a88c 100644 --- a/lib/responses/errors.go +++ b/lib/responses/errors.go @@ -1,7 +1,6 @@ package responses import ( - "encoding/json" "net/http" "github.com/getsentry/sentry-go" @@ -44,7 +43,7 @@ func HTTPErrorHandler(err error, c echo.Context) { return } c.Logger().Error(err) - if hub := sentryecho.GetHubFromContext(c); hub != nil && isErrAllowedForSentry(err) { + if hub := sentryecho.GetHubFromContext(c); hub != nil { hub.WithScope(func(scope *sentry.Scope) { scope.SetExtra("UserID", c.Get("UserID")) hub.CaptureException(err) @@ -59,32 +58,3 @@ func HTTPErrorHandler(err error, c echo.Context) { } // TODO: use an error matching the error code } - -// this is a simple way to try to convert err.Message interface -// to ErrorResponse without external packages -func errToErrorResponse(err error) *ErrorResponse { - httpError, ok := err.(*echo.HTTPError) - if !ok { - return nil - } - - responseJson, err := json.Marshal(httpError.Message) - if err != nil { - return nil - } - - errorResponse := &ErrorResponse{} - err = json.Unmarshal(responseJson, errorResponse) - if err != nil { - return nil - } - - return errorResponse -} - -// currently only bad auth errors are not allowed -func isErrAllowedForSentry(err error) bool { - errResponse := errToErrorResponse(err) - - return errResponse == nil || errResponse.Code != BadAuthError.Code -} diff --git a/lib/responses/errors_test.go b/lib/responses/errors_test.go deleted file mode 100644 index 16ee4c3..0000000 --- a/lib/responses/errors_test.go +++ /dev/null @@ -1,39 +0,0 @@ -package responses - -import ( - "errors" - "net/http" - "testing" - - "github.com/labstack/echo/v4" - "github.com/stretchr/testify/assert" -) - -func TestBadAuthErrorsNotAllowedForSentry(t *testing.T) { - badAuthErrResponse := echo.NewHTTPError(http.StatusBadRequest, echo.Map{ - "error": true, - "code": 1, - "message": "bad auth", - }) - - isAllowed := isErrAllowedForSentry(badAuthErrResponse) - assert.False(t, isAllowed) -} - -func TestNotBadAuthErrorsAllowedForSentry(t *testing.T) { - notBadAuthErrResponse := echo.NewHTTPError(http.StatusBadRequest, echo.Map{ - "error": true, - "code": 2, - "message": "not bad auth", - }) - - isAllowed := isErrAllowedForSentry(notBadAuthErrResponse) - assert.True(t, isAllowed) -} - -func TestNonErrorResponseErrorsAllowedForSentry(t *testing.T) { - err := errors.New("random error") - - isAllowed := isErrAllowedForSentry(err) - assert.True(t, isAllowed) -} diff --git a/lib/tokens/jwt.go b/lib/tokens/jwt.go index 46b7c67..7e0129a 100644 --- a/lib/tokens/jwt.go +++ b/lib/tokens/jwt.go @@ -25,7 +25,7 @@ func Middleware(secret []byte) echo.MiddlewareFunc { config.SigningKey = secret config.ErrorHandlerWithContext = func(err error, c echo.Context) error { c.Logger().Error(err) - return echo.NewHTTPError(http.StatusBadRequest, echo.Map{ + return echo.NewHTTPError(http.StatusUnauthorized, echo.Map{ "error": true, "code": 1, "message": "bad auth", diff --git a/main.go b/main.go index cb35f92..2ab4366 100644 --- a/main.go +++ b/main.go @@ -93,7 +93,8 @@ func main() { // Setup exception tracking with Sentry if configured if c.SentryDSN != "" { if err = sentry.Init(sentry.ClientOptions{ - Dsn: c.SentryDSN, + Dsn: c.SentryDSN, + IgnoreErrors: []string{"401"}, }); err != nil { logger.Errorf("sentry init error: %v", err) } From 843cb06bd452699d253082cb306846b4189ed19a Mon Sep 17 00:00:00 2001 From: Stefan Kostic Date: Thu, 7 Apr 2022 15:44:46 +0200 Subject: [PATCH 5/5] Fix tests --- integration_tests/auth_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/integration_tests/auth_test.go b/integration_tests/auth_test.go index 43c562f..3e0045b 100644 --- a/integration_tests/auth_test.go +++ b/integration_tests/auth_test.go @@ -128,7 +128,7 @@ func (suite *UserAuthTestSuite) TestAuthWithExpiredRefreshToken() { controller = controllers.NewAuthController(suite.Service) assert.NoError(suite.T(), controller.Auth(c)) errorResponse := &responses.ErrorResponse{} - assert.Equal(suite.T(), http.StatusBadRequest, rec.Code) + assert.Equal(suite.T(), http.StatusUnauthorized, rec.Code) assert.NoError(suite.T(), json.NewDecoder(rec.Body).Decode(errorResponse)) assert.Equal(suite.T(), responses.BadAuthError.Code, errorResponse.Code) assert.Equal(suite.T(), responses.BadAuthError.Message, errorResponse.Message) @@ -171,7 +171,7 @@ func (suite *UserAuthTestSuite) TestAuthWithInvalidSecretRefreshToken() { controller = controllers.NewAuthController(suite.Service) assert.NoError(suite.T(), controller.Auth(c)) errorResponse := &responses.ErrorResponse{} - assert.Equal(suite.T(), http.StatusBadRequest, rec.Code) + assert.Equal(suite.T(), http.StatusUnauthorized, rec.Code) assert.NoError(suite.T(), json.NewDecoder(rec.Body).Decode(errorResponse)) assert.Equal(suite.T(), responses.BadAuthError.Code, errorResponse.Code) assert.Equal(suite.T(), responses.BadAuthError.Message, errorResponse.Message) @@ -213,7 +213,7 @@ func (suite *UserAuthTestSuite) TestAuthWithInvalidUserIdRefreshToken() { controller = controllers.NewAuthController(suite.Service) assert.NoError(suite.T(), controller.Auth(c)) errorResponse := &responses.ErrorResponse{} - assert.Equal(suite.T(), http.StatusBadRequest, rec.Code) + assert.Equal(suite.T(), http.StatusUnauthorized, rec.Code) assert.NoError(suite.T(), json.NewDecoder(rec.Body).Decode(errorResponse)) assert.Equal(suite.T(), responses.BadAuthError.Code, errorResponse.Code) assert.Equal(suite.T(), responses.BadAuthError.Message, errorResponse.Message) @@ -249,7 +249,7 @@ func (suite *UserAuthTestSuite) TestAuthWithAccessToken() { controller = controllers.NewAuthController(suite.Service) assert.NoError(suite.T(), controller.Auth(c)) errorResponse := &responses.ErrorResponse{} - assert.Equal(suite.T(), http.StatusBadRequest, rec.Code) + assert.Equal(suite.T(), http.StatusUnauthorized, rec.Code) assert.NoError(suite.T(), json.NewDecoder(rec.Body).Decode(errorResponse)) assert.Equal(suite.T(), responses.BadAuthError.Code, errorResponse.Code) assert.Equal(suite.T(), responses.BadAuthError.Message, errorResponse.Message) @@ -270,7 +270,7 @@ func (suite *UserAuthTestSuite) TestAuthWithNotParseableRefreshToken() { controller := controllers.NewAuthController(suite.Service) assert.NoError(suite.T(), controller.Auth(c)) errorResponse := &responses.ErrorResponse{} - assert.Equal(suite.T(), http.StatusBadRequest, rec.Code) + assert.Equal(suite.T(), http.StatusUnauthorized, rec.Code) assert.NoError(suite.T(), json.NewDecoder(rec.Body).Decode(errorResponse)) assert.Equal(suite.T(), responses.BadAuthError.Code, errorResponse.Code) assert.Equal(suite.T(), responses.BadAuthError.Message, errorResponse.Message)