Repository: labstack/echo
Pull request #1967 (2021-08-19 13:00:49)

Add custom jwt extractor to jwt config
This PR help users to extract their JWT tokens in which way they want.

codecov[bot]: # [Codecov](https://codecov.io/gh/labstack/echo/pull/1967?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) Report
> Merging [#1967](https://codecov.io/gh/labstack/echo/pull/1967?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) (b037644) into [master](https://codecov.io/gh/labstack/echo/commit/7d41537e70ce8e3e25c2e8199c798c7cfe3d4299?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) (7d41537) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/labstack/echo/pull/1967/graphs/tree.svg?width=650&height=150&src=pr&token=vqnT76l79i&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack)](https://codecov.io/gh/labstack/echo/pull/1967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack)

```diff
@@           Coverage Diff           @@
##           master    #1967   +/-   ##
=======================================
  Coverage   91.03%   91.03%           
=======================================
  Files          32       32           
  Lines        2800     2800           
=======================================
  Hits         2549     2549           
  Misses        160      160           
  Partials       91       91           
```


| [Impacted Files](https://codecov.io/gh/labstack/echo/pull/1967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) | Coverage Δ | |
|---|---|---|
| [middleware/jwt.go](https://codecov.io/gh/labstack/echo/pull/1967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack#diff-bWlkZGxld2FyZS9qd3QuZ28=) | `90.26% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/labstack/echo/pull/1967?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/labstack/echo/pull/1967?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack). Last update [d793521...b037644](https://codecov.io/gh/labstack/echo/pull/1967?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack).

RashadAnsari: @aldas Could you please review these changes?

RashadAnsari: Does no one want to review this PR?
aldas: I have seen this but I am not very keen to accept it. I do not know the use-case. Currently JWT middleware allows to extract query, param, cookie, form, header values. I do not understand is there case these 5 variants does no cover?


Note: absolute minimal custom JWT middleware would be and extractor part can be whatever needed
```go
e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
	return func(c echo.Context) error {
		authHeader := c.Request().Header.Get(echo.HeaderAuthorization)
		rawToken := strings.TrimPrefix(authHeader, "Bearer ")
		if len(authHeader) == len(rawToken) {
			return middleware.ErrJWTMissing
		}
		jwtToken, err := jwt.Parse(rawToken, func(t *jwt.Token) (interface{}, error) {
			if t.Method.Alg() != "HS256" {
				return nil, fmt.Errorf("unexpected jwt signing method=%v", t.Header["alg"])
			}
			return "MySuperSecretSigningKey", nil
		})
		if err != nil {
			return err
		}
		if !jwtToken.Valid {
			return errors.New("invalid token")
		}
		c.Set("user", jwtToken)
		return next(c)
	}
})
```
RashadAnsari: > I have seen this but I am not very keen to accept it. I do not know the use-case. Currently JWT middleware allows to extract query, param, cookie, form, header values. I do not understand is there case these 5 variants does no cover?
> 
> Note: absolute minimal custom JWT middleware would be and extractor part can be whatever needed
> 
> ```go
> e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
> 	return func(c echo.Context) error {
> 		authHeader := c.Request().Header.Get(echo.HeaderAuthorization)
> 		rawToken := strings.TrimPrefix(authHeader, "Bearer ")
> 		if len(authHeader) == len(rawToken) {
> 			return middleware.ErrJWTMissing
> 		}
> 		jwtToken, err := jwt.Parse(rawToken, func(t *jwt.Token) (interface{}, error) {
> 			if t.Method.Alg() != "HS256" {
> 				return nil, fmt.Errorf("unexpected jwt signing method=%v", t.Header["alg"])
> 			}
> 			return "MySuperSecretSigningKey", nil
> 		})
> 		if err != nil {
> 			return err
> 		}
> 		if !jwtToken.Valid {
> 			return errors.New("invalid token")
> 		}
> 		c.Set("user", jwtToken)
> 		return next(c)
> 	}
> })
> ```

If you see the test case I wrote, you will understand why I made this change. The standard header extractor does not handle this case because the auth scheme always is Bearer but, if you look at the tests with a custom extractor, you can implement whatever you want.
Actually, the default values that you assumed for some variables like auth scheme will make a problem for me.
aldas: Still. This seems to be shortcoming of `JWTConfig.TokenLookup` and `JWTConfig.AuthScheme` as usecase where token needs to be extracted from header does not work properly.

Ala it should be possible to:
```go
	e.Use(JWTWithConfig(JWTConfig{
		TokenLookup: "header:X-API-Key",
		AuthScheme:  "", // maybe for empty somehow
		SigningKey:  []byte("secret"),
	}))
```
RashadAnsari: > Still. This seems to be shortcoming of `JWTConfig.TokenLookup` and `JWTConfig.AuthScheme` as usecase where token needs to be extracted from header does not work properly.
> 
> Ala it should be possible to:
> 
> ```go
> 	e.Use(JWTWithConfig(JWTConfig{
> 		TokenLookup: "header:X-API-Key",
> 		AuthScheme:  "", // maybe force empty somehow
> 		SigningKey:  []byte("secret"),
> 	}))
> ```

This doesn't work because for initializing the middleware, we have this condition:

```go
if config.AuthScheme == "" {
    config.AuthScheme = DefaultJWTConfig.AuthScheme
}
```
aldas: I think we can modify "header" to support custom authscheme for each header with these changes

https://github.com/labstack/echo/blob/6b5e62b27ea0bc459843e67014360dd35ae8147b/middleware/jwt.go#L195
```go
		case "header":
			prefix := config.AuthScheme
			if len(parts) > 2 {
				prefix = parts[2]
			}
			extractors = append(extractors, jwtFromHeader(parts[1], prefix))
```

https://github.com/labstack/echo/blob/6b5e62b27ea0bc459843e67014360dd35ae8147b/middleware/jwt.go#L297
```go
		l := len(authScheme)
		if l == 0 {
			return auth, nil
		}
		if len(auth) > l+1 && strings.EqualFold(auth[:l], authScheme) {
```

With this we can have
```go
	e.Use(JWTWithConfig(JWTConfig{
		TokenLookup: "header:X-API-Key:", // auth scheme will be empty
		SigningKey:  []byte("secret"),
	}))

	e.Use(JWTWithConfig(JWTConfig{
		TokenLookup: "header:X-API-Key:MyAuth", // auth cheme will be "MyAuth"
		SigningKey:  []byte("secret"),
	}))

	e.Use(JWTWithConfig(JWTConfig{
		TokenLookup: "header:X-API-Key", // auth scheme will be "Bearer"
		SigningKey:  []byte("secret"),
	}))
```

RashadAnsari: > 

Actually, for me is better to make sense to add the ability to extract the token by a custom extractor. In this case, anyone can quickly write his own extractor if he needs and supports almost all varieties of custom use cases.
This is just an idea but, I think your proposed solution makes it a little complex and unreadable.
aldas: Fair enough,  but change it so that:
1. name`JWTExtractors` to `TokenLookupFuncs` - lets call things with same names. We already have `TokenLookup` and other stuff has "*Func" prefixes.
2.  `TokenLookup` default value and `TokenLookupFuncs` are mutually exclusive - when you provide your own extractor funcs `TokenLookup` will not default to `"header:Authorization"`. Atm extractors take the priority but that "Authorization: bearer xxxx" is also there at the end, added by the defaults.
3. And add note to fields that you can mix `TokenLookup` (when you explicitly provide something) with funcs and funcs are executed first


RashadAnsari: Great, I will make the changes.
RashadAnsari: @aldas, Are you ok with the changes that I made?
RashadAnsari: @aldas, Thank you for your approval.
Do you know when this PR will get merged?

Patch:
diff --git a/middleware/jwt.go b/middleware/jwt.go
index 21e33ab..43605e3 100644
--- a/middleware/jwt.go
+++ b/middleware/jwt.go
@@ -68,9 +68,14 @@ type (
 		// - "form:<name>"
 		// Multiply sources example:
 		// - "header: Authorization,cookie: myowncookie"
-
 		TokenLookup string
 
+		// TokenLookupFuncs defines a list of user-defined functions that extract JWT token from the given context.
+		// This is one of the two options to provide a token extractor.
+		// The order of precedence is user-defined TokenLookupFuncs, and TokenLookup.
+		// You can also provide both if you want.
+		TokenLookupFuncs []TokenLookupFunc
+
 		// AuthScheme to be used in the Authorization header.
 		// Optional. Default value "Bearer".
 		AuthScheme string
@@ -103,7 +108,8 @@ type (
 	// JWTErrorHandlerWithContext is almost identical to JWTErrorHandler, but it's passed the current context.
 	JWTErrorHandlerWithContext func(error, echo.Context) error
 
-	jwtExtractor func(echo.Context) (string, error)
+	// TokenLookupFunc defines a function for extracting JWT token from the given context.
+	TokenLookupFunc func(echo.Context) (string, error)
 )
 
 // Algorithms
@@ -120,13 +126,14 @@ var (
 var (
 	// DefaultJWTConfig is the default JWT auth middleware config.
 	DefaultJWTConfig = JWTConfig{
-		Skipper:       DefaultSkipper,
-		SigningMethod: AlgorithmHS256,
-		ContextKey:    "user",
-		TokenLookup:   "header:" + echo.HeaderAuthorization,
-		AuthScheme:    "Bearer",
-		Claims:        jwt.MapClaims{},
-		KeyFunc:       nil,
+		Skipper:          DefaultSkipper,
+		SigningMethod:    AlgorithmHS256,
+		ContextKey:       "user",
+		TokenLookup:      "header:" + echo.HeaderAuthorization,
+		TokenLookupFuncs: nil,
+		AuthScheme:       "Bearer",
+		Claims:           jwt.MapClaims{},
+		KeyFunc:          nil,
 	}
 )
 
@@ -163,7 +170,7 @@ func JWTWithConfig(config JWTConfig) echo.MiddlewareFunc {
 	if config.Claims == nil {
 		config.Claims = DefaultJWTConfig.Claims
 	}
-	if config.TokenLookup == "" {
+	if config.TokenLookup == "" && len(config.TokenLookupFuncs) == 0 {
 		config.TokenLookup = DefaultJWTConfig.TokenLookup
 	}
 	if config.AuthScheme == "" {
@@ -179,7 +186,7 @@ func JWTWithConfig(config JWTConfig) echo.MiddlewareFunc {
 	// Initialize
 	// Split sources
 	sources := strings.Split(config.TokenLookup, ",")
-	var extractors []jwtExtractor
+	var extractors = config.TokenLookupFuncs
 	for _, source := range sources {
 		parts := strings.Split(source, ":")
 
@@ -290,8 +297,8 @@ func (config *JWTConfig) defaultKeyFunc(t *jwt.Token) (interface{}, error) {
 	return config.SigningKey, nil
 }
 
-// jwtFromHeader returns a `jwtExtractor` that extracts token from the request header.
-func jwtFromHeader(header string, authScheme string) jwtExtractor {
+// jwtFromHeader returns a `TokenLookupFunc` that extracts token from the request header.
+func jwtFromHeader(header string, authScheme string) TokenLookupFunc {
 	return func(c echo.Context) (string, error) {
 		auth := c.Request().Header.Get(header)
 		l := len(authScheme)
@@ -302,8 +309,8 @@ func jwtFromHeader(header string, authScheme string) jwtExtractor {
 	}
 }
 
-// jwtFromQuery returns a `jwtExtractor` that extracts token from the query string.
-func jwtFromQuery(param string) jwtExtractor {
+// jwtFromQuery returns a `TokenLookupFunc` that extracts token from the query string.
+func jwtFromQuery(param string) TokenLookupFunc {
 	return func(c echo.Context) (string, error) {
 		token := c.QueryParam(param)
 		if token == "" {
@@ -313,8 +320,8 @@ func jwtFromQuery(param string) jwtExtractor {
 	}
 }
 
-// jwtFromParam returns a `jwtExtractor` that extracts token from the url param string.
-func jwtFromParam(param string) jwtExtractor {
+// jwtFromParam returns a `TokenLookupFunc` that extracts token from the url param string.
+func jwtFromParam(param string) TokenLookupFunc {
 	return func(c echo.Context) (string, error) {
 		token := c.Param(param)
 		if token == "" {
@@ -324,8 +331,8 @@ func jwtFromParam(param string) jwtExtractor {
 	}
 }
 
-// jwtFromCookie returns a `jwtExtractor` that extracts token from the named cookie.
-func jwtFromCookie(name string) jwtExtractor {
+// jwtFromCookie returns a `TokenLookupFunc` that extracts token from the named cookie.
+func jwtFromCookie(name string) TokenLookupFunc {
 	return func(c echo.Context) (string, error) {
 		cookie, err := c.Cookie(name)
 		if err != nil {
@@ -335,8 +342,8 @@ func jwtFromCookie(name string) jwtExtractor {
 	}
 }
 
-// jwtFromForm returns a `jwtExtractor` that extracts token from the form field.
-func jwtFromForm(name string) jwtExtractor {
+// jwtFromForm returns a `TokenLookupFunc` that extracts token from the form field.
+func jwtFromForm(name string) TokenLookupFunc {
 	return func(c echo.Context) (string, error) {
 		field := c.FormValue(name)
 		if field == "" {


Test patch:
diff --git a/middleware/jwt_test.go b/middleware/jwt_test.go
index 5f36ce0..18454d0 100644
--- a/middleware/jwt_test.go
+++ b/middleware/jwt_test.go
@@ -603,3 +603,27 @@ func TestJWTConfig_custom_ParseTokenFunc_Keyfunc(t *testing.T) {
 
 	assert.Equal(t, http.StatusTeapot, res.Code)
 }
+
+func TestJWTConfig_TokenLookupFuncs(t *testing.T) {
+	e := echo.New()
+
+	e.GET("/", func(c echo.Context) error {
+		return c.String(http.StatusOK, "test")
+	})
+
+	e.Use(JWTWithConfig(JWTConfig{
+		TokenLookupFuncs: []TokenLookupFunc{
+			func(c echo.Context) (string, error) {
+				return c.Request().Header.Get("X-API-Key"), nil
+			},
+		},
+		SigningKey: []byte("secret"),
+	}))
+
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	req.Header.Set("X-API-Key", "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWV9.TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ")
+	res := httptest.NewRecorder()
+	e.ServeHTTP(res, req)
+
+	assert.Equal(t, http.StatusOK, res.Code)
+}