From faf8eb3afb497232ad6f3220b1bdba4eba0d6193 Mon Sep 17 00:00:00 2001 From: Nicholas Wiersma Date: Fri, 9 Sep 2022 11:05:58 +0200 Subject: [PATCH] JWT: Allow conventional bearer token in Authorization header (#54821) * fix: allow JWT to accept standard bearer token * fix: linter issues * fix: linter gosec false positive * fix: refactor logic into JWT handler * fix: move bearer trimming earlier --- pkg/middleware/middleware_jwt_auth_test.go | 29 +++++++++++++++++++ pkg/middleware/middleware_test.go | 21 ++++++++++++++ pkg/services/contexthandler/auth_jwt.go | 16 ++++++++++ pkg/services/contexthandler/contexthandler.go | 2 +- 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/pkg/middleware/middleware_jwt_auth_test.go b/pkg/middleware/middleware_jwt_auth_test.go index ed854540b73..7e791b9b720 100644 --- a/pkg/middleware/middleware_jwt_auth_test.go +++ b/pkg/middleware/middleware_jwt_auth_test.go @@ -24,6 +24,11 @@ func TestMiddlewareJWTAuth(t *testing.T) { cfg.JWTAuthHeaderName = "x-jwt-assertion" } + configureAuthHeader := func(cfg *setting.Cfg) { + cfg.JWTAuthEnabled = true + cfg.JWTAuthHeaderName = "Authorization" + } + configureUsernameClaim := func(cfg *setting.Cfg) { cfg.JWTAuthUsernameClaim = "foo-username" } @@ -72,6 +77,30 @@ func TestMiddlewareJWTAuth(t *testing.T) { assert.Equal(t, myUsername, sc.context.Login) }, configure, configureUsernameClaim) + middlewareScenario(t, "Valid token with bearer in authorization header", func(t *testing.T, sc *scenarioContext) { + myUsername := "vladimir" + // We can ignore gosec G101 since this does not contain any credentials. + // nolint:gosec + myToken := "some.jwt.token" + var verifiedToken string + sc.jwtAuthService.VerifyProvider = func(ctx context.Context, token string) (models.JWTClaims, error) { + verifiedToken = myToken + return models.JWTClaims{ + "sub": myUsername, + "foo-username": myUsername, + }, nil + } + sc.userService.ExpectedSignedInUser = &user.SignedInUser{UserID: id, OrgID: orgID, Login: myUsername} + + sc.fakeReq("GET", "/").withJWTAuthHeader("Bearer " + myToken).exec() + assert.Equal(t, verifiedToken, myToken) + assert.Equal(t, 200, sc.resp.Code) + assert.True(t, sc.context.IsSignedIn) + assert.Equal(t, orgID, sc.context.OrgID) + assert.Equal(t, id, sc.context.UserID) + assert.Equal(t, myUsername, sc.context.Login) + }, configureAuthHeader, configureUsernameClaim) + middlewareScenario(t, "Valid token with valid email claim", func(t *testing.T, sc *scenarioContext) { var verifiedToken string sc.jwtAuthService.VerifyProvider = func(ctx context.Context, token string) (models.JWTClaims, error) { diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index dae1283ec74..2cd381398e1 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -81,6 +81,11 @@ func TestMiddleWareSecurityHeaders(t *testing.T) { func TestMiddlewareContext(t *testing.T) { const noCache = "no-cache" + configureJWTAuthHeader := func(cfg *setting.Cfg) { + cfg.JWTAuthEnabled = true + cfg.JWTAuthHeaderName = "Authorization" + } + middlewareScenario(t, "middleware should add context to injector", func(t *testing.T, sc *scenarioContext) { sc.fakeReq("GET", "/").exec() assert.NotNil(t, sc.context) @@ -165,6 +170,22 @@ func TestMiddlewareContext(t *testing.T) { assert.Equal(t, org.RoleEditor, sc.context.OrgRole) }) + middlewareScenario(t, "Valid API key with JWT enabled", func(t *testing.T, sc *scenarioContext) { + const orgID int64 = 12 + keyhash, err := util.EncodePassword("v5nAwpMafFP6znaS4urhdWDLS5511M42", "asd") + require.NoError(t, err) + + sc.apiKeyService.ExpectedAPIKey = &apikey.APIKey{OrgId: orgID, Role: org.RoleEditor, Key: keyhash} + + sc.fakeReq("GET", "/").withValidApiKey().exec() + + require.Equal(t, 200, sc.resp.Code) + + assert.True(t, sc.context.IsSignedIn) + assert.Equal(t, orgID, sc.context.OrgID) + assert.Equal(t, org.RoleEditor, sc.context.OrgRole) + }, configureJWTAuthHeader) + middlewareScenario(t, "Valid API key, but does not match DB hash", func(t *testing.T, sc *scenarioContext) { const keyhash = "Something_not_matching" sc.apiKeyService.ExpectedAPIKey = &apikey.APIKey{OrgId: 12, Role: org.RoleEditor, Key: keyhash} diff --git a/pkg/services/contexthandler/auth_jwt.go b/pkg/services/contexthandler/auth_jwt.go index b4356aa883a..898293a7d90 100644 --- a/pkg/services/contexthandler/auth_jwt.go +++ b/pkg/services/contexthandler/auth_jwt.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/models" @@ -32,6 +33,15 @@ func (h *ContextHandler) initContextWithJWT(ctx *models.ReqContext, orgId int64) return false } + // Strip the 'Bearer' prefix if it exists. + jwtToken = strings.TrimPrefix(jwtToken, "Bearer ") + + // The header is Authorization and the token does not look like a JWT, + // this is likely an API key. Pass it on. + if h.Cfg.JWTAuthHeaderName == "Authorization" && !looksLikeJWT(jwtToken) { + return false + } + claims, err := h.JWTAuthService.Verify(ctx.Req.Context(), jwtToken) if err != nil { ctx.Logger.Debug("Failed to verify JWT", "error", err) @@ -186,3 +196,9 @@ func searchClaimsForStringAttr(attributePath string, claims map[string]interface return "", nil } + +func looksLikeJWT(token string) bool { + // A JWT must have 3 parts separated by `.`. + parts := strings.Split(token, ".") + return len(parts) == 3 +} diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index 7b35b57f173..f343bbfdea8 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -147,11 +147,11 @@ func (h *ContextHandler) Middleware(next http.Handler) http.Handler { // then test if anonymous access is enabled switch { case h.initContextWithRenderAuth(reqContext): + case h.initContextWithJWT(reqContext, orgID): case h.initContextWithAPIKey(reqContext): case h.initContextWithBasicAuth(reqContext, orgID): case h.initContextWithAuthProxy(reqContext, orgID): case h.initContextWithToken(reqContext, orgID): - case h.initContextWithJWT(reqContext, orgID): case h.initContextWithAnonymousUser(reqContext): }