Merge pull request #6 from bootjp/feature/2

fix missing status code
bootjp · Nov 11, 2022 · 154dd51 · 154dd51
2 parents 5bb5362 + d8eaaa3
commit 154dd51
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 21 deletions.
diff --git a/path_auth.go b/path_auth.go
@@ -31,19 +31,7 @@ var (
 )
 
 // ErrKeyAuthMissing is error type when PathAuth middleware is unable to extract value from lookups
-type ErrKeyAuthMissing struct {
-	Err error
-}
-
-// Error returns errors text
-func (e *ErrKeyAuthMissing) Error() string {
-	return e.Err.Error()
-}
-
-// Unwrap unwraps error
-func (e *ErrKeyAuthMissing) Unwrap() error {
-	return e.Err
-}
+var ErrKeyAuthMissing = echo.NewHTTPError(http.StatusBadRequest, "Missing key in the request")
 
 // PathAuth returns an PathAuth middleware.
 //
@@ -74,20 +62,35 @@ func PathAuthWithConfig(config PathAuthConfig) echo.MiddlewareFunc {
 			if config.Skipper(c) {
 				return next(c)
 			}
-			valid, err := config.Validator(c.Param(config.Param), c)
-			if err != nil {
+
+			if !extract(config.Param, c.ParamNames()) {
 				return &echo.HTTPError{
-					Code:     http.StatusUnauthorized,
-					Message:  "Unauthorized",
-					Internal: err,
+					Code:     http.StatusBadRequest,
+					Message:  http.StatusText(http.StatusBadRequest),
+					Internal: ErrKeyAuthMissing,
 				}
 			}
 
-			if valid {
+			valid, err := config.Validator(c.Param(config.Param), c)
+			if err == nil && valid {
 				return next(c)
 			}
 
-			return echo.NewHTTPError(http.StatusBadRequest)
+			return &echo.HTTPError{
+				Code:     http.StatusUnauthorized,
+				Message:  http.StatusText(http.StatusUnauthorized),
+				Internal: err,
+			}
 		}
 	}
 }
+
+func extract(cParam string, params []string) bool {
+	for _, param := range params {
+		if cParam == param {
+			return true
+		}
+	}
+
+	return false
+}
diff --git a/path_auth_test.go b/path_auth_test.go
@@ -26,6 +26,7 @@ func TestKeyAuth(t *testing.T) {
 		handlerCalled := false
 		handler := func(c echo.Context) error {
 			handlerCalled = true
+			//nolint:wrapcheck
 			return c.String(http.StatusOK, "test")
 		}
 		middlewareChain := PathAuth("apikey", testKeyValidator)(handler)
@@ -48,6 +49,7 @@ func TestKeyAuth(t *testing.T) {
 		handlerCalled := false
 		handler := func(c echo.Context) error {
 			handlerCalled = true
+			//nolint:wrapcheck
 			return c.String(http.StatusOK, "test")
 		}
 		middlewareChain := PathAuth("apikey", testKeyValidator)(handler)
@@ -63,9 +65,55 @@ func TestKeyAuth(t *testing.T) {
 		err := middlewareChain(c)
 
 		assert.Error(t, err)
+		assert.EqualError(t, err, "code=401, message=Unauthorized, internal=some user defined error")
 		assert.False(t, handlerCalled)
 	})
+	t.Run("auth no error failed", func(t *testing.T) {
+		handlerCalled := false
+		handler := func(c echo.Context) error {
+			handlerCalled = true
+			//nolint:wrapcheck
+			return c.String(http.StatusOK, "test")
+		}
+		middlewareChain := PathAuth("apikey", testKeyValidator)(handler)
 
+		e := echo.New()
+		e.GET("/:apikey", middlewareChain)
+
+		req := httptest.NewRequest(http.MethodGet, "/", nil)
+		rec := httptest.NewRecorder()
+
+		c := e.NewContext(req, rec)
+		e.Router().Find(http.MethodGet, "/no-error", c)
+		err := middlewareChain(c)
+
+		assert.Error(t, err)
+		assert.EqualError(t, err, "code=401, message=Unauthorized")
+		assert.False(t, handlerCalled)
+	})
+	t.Run("auth nokey", func(t *testing.T) {
+		handlerCalled := false
+		handler := func(c echo.Context) error {
+			handlerCalled = true
+			//nolint:wrapcheck
+			return c.String(http.StatusOK, "test")
+		}
+		middlewareChain := PathAuth("undef", testKeyValidator)(handler)
+
+		e := echo.New()
+		e.GET("/:apikey", middlewareChain)
+
+		req := httptest.NewRequest(http.MethodGet, "/", nil)
+		rec := httptest.NewRecorder()
+
+		c := e.NewContext(req, rec)
+		e.Router().Find(http.MethodGet, "/error-key", c)
+		err := middlewareChain(c)
+
+		assert.Error(t, err)
+		assert.EqualError(t, err, "code=400, message=Bad Request, internal=code=400, message=Missing key in the request")
+		assert.False(t, handlerCalled)
+	})
 }
 
 func TestPathAuthWithConfig(t *testing.T) {
@@ -103,7 +151,7 @@ func TestPathAuthWithConfig(t *testing.T) {
 				return req
 			},
 			expectHandlerCalled: false,
-			expectError:         "code=400, message=Bad Request",
+			expectError:         "code=401, message=Unauthorized",
 		},
 	}
 
@@ -112,6 +160,7 @@ func TestPathAuthWithConfig(t *testing.T) {
 			handlerCalled := false
 			handler := func(c echo.Context) error {
 				handlerCalled = true
+				//nolint:wrapcheck
 				return c.String(http.StatusOK, "test")
 			}
 			config := PathAuthConfig{
@@ -154,6 +203,7 @@ func TestPathAuthWithConfig_panicsOnEmptyValidator(t *testing.T) {
 		"PathAuth: requires a validator function",
 		func() {
 			handler := func(c echo.Context) error {
+				//nolint:wrapcheck
 				return c.String(http.StatusOK, "test")
 			}
 			PathAuthWithConfig(PathAuthConfig{
@@ -169,6 +219,7 @@ func TestPathAuthWithConfig_panicsOnEmptyParam(t *testing.T) {
 		"PathAuth: requires a param",
 		func() {
 			handler := func(c echo.Context) error {
+				//nolint:wrapcheck
 				return c.String(http.StatusOK, "test")
 			}
 			PathAuthWithConfig(PathAuthConfig{
@@ -185,6 +236,7 @@ func TestPathAuthWithConfig_panicsOnEmptyParam(t *testing.T) {
 		"PathAuth: requires a param",
 		func() {
 			handler := func(c echo.Context) error {
+				//nolint:wrapcheck
 				return c.String(http.StatusOK, "test")
 			}
 			PathAuth("", func(auth string, c echo.Context) (bool, error) {
@@ -193,3 +245,10 @@ func TestPathAuthWithConfig_panicsOnEmptyParam(t *testing.T) {
 		},
 	)
 }
+
+func TestExtract(t *testing.T) {
+
+	assert.True(t, extract("apikey", []string{"apikey", "valid-key"}))
+	assert.False(t, extract("apikey", []string{"valid-key"}))
+
+}