From 56c2755b3b92dcee6b3d1c8226b6e0145929174f Mon Sep 17 00:00:00 2001 From: linoman <2051016+linoman@users.noreply.github.com> Date: Thu, 19 Jan 2023 16:03:09 +0100 Subject: [PATCH] Fix JWT claims request (#61650) * Fix JWT claims request * Add test scenarios for missing config options --- pkg/services/authn/clients/jwt.go | 4 +- pkg/services/authn/clients/jwt_test.go | 111 +++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/pkg/services/authn/clients/jwt.go b/pkg/services/authn/clients/jwt.go index 9eaf0543c8d..6c9b18bb272 100644 --- a/pkg/services/authn/clients/jwt.go +++ b/pkg/services/authn/clients/jwt.go @@ -115,10 +115,10 @@ func (s *JWT) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identi } } - if id.Login == "" || id.Email == "" { + if id.Login == "" && id.Email == "" { s.log.Debug("Failed to get an authentication claim from JWT", "login", id.Login, "email", id.Email) - return nil, ErrJWTMissingClaim.Errorf("missing login or email claim in JWT") + return nil, ErrJWTMissingClaim.Errorf("missing login and email claim in JWT") } if s.cfg.JWTAuthAutoSignUp { diff --git a/pkg/services/authn/clients/jwt_test.go b/pkg/services/authn/clients/jwt_test.go index c511ac2fe4d..5a62067681c 100644 --- a/pkg/services/authn/clients/jwt_test.go +++ b/pkg/services/authn/clients/jwt_test.go @@ -85,6 +85,117 @@ func TestAuthenticateJWT(t *testing.T) { assert.EqualValues(t, wantID, id, fmt.Sprintf("%+v", id)) } +func TestJWTClaimConfig(t *testing.T) { + jwtService := &models.FakeJWTService{ + VerifyProvider: func(context.Context, string) (models.JWTClaims, error) { + return models.JWTClaims{ + "sub": "1234567890", + "email": "eai.doe@cor.po", + "preferred_username": "eai-doe", + "name": "Eai Doe", + "roles": "Admin", + }, nil + }, + } + + jwtHeaderName := "X-Forwarded-User" + + cfg := &setting.Cfg{ + JWTAuthEnabled: true, + JWTAuthHeaderName: jwtHeaderName, + JWTAuthAutoSignUp: true, + JWTAuthAllowAssignGrafanaAdmin: true, + JWTAuthRoleAttributeStrict: true, + JWTAuthRoleAttributePath: "roles", + } + + // #nosec G101 -- This is a dummy/test token + token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.XbPfbIHMI6arZ3Y922BhjWgQzWXcXNrz0ogtVhfEd2o" + + type Dictionary map[string]interface{} + + type testCase struct { + desc string + claimsConfigurations []Dictionary + valid bool + } + + testCases := []testCase{ + { + desc: "JWT configuration with email and username claims", + claimsConfigurations: []Dictionary{ + { + "JWTAuthEmailClaim": true, + "JWTAuthUsernameClaim": true, + }, + }, + valid: true, + }, + { + desc: "JWT configuration with email claim", + claimsConfigurations: []Dictionary{ + { + "JWTAuthEmailClaim": true, + "JWTAuthUsernameClaim": false, + }, + }, + valid: true, + }, + { + desc: "JWT configuration with username claim", + claimsConfigurations: []Dictionary{ + { + "JWTAuthEmailClaim": false, + "JWTAuthUsernameClaim": true, + }, + }, + valid: true, + }, + { + desc: "JWT configuration without email and username claims", + claimsConfigurations: []Dictionary{ + { + "JWTAuthEmailClaim": false, + "JWTAuthUsernameClaim": false, + }, + }, + valid: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + for _, claims := range tc.claimsConfigurations { + cfg.JWTAuthEmailClaim = "" + cfg.JWTAuthUsernameClaim = "" + + if claims["JWTAuthEmailClaim"] == true { + cfg.JWTAuthEmailClaim = "email" + } + if claims["JWTAuthUsernameClaim"] == true { + cfg.JWTAuthUsernameClaim = "preferred_username" + } + } + }) + httpReq := &http.Request{ + URL: &url.URL{RawQuery: "auth_token=" + token}, + Header: map[string][]string{ + jwtHeaderName: {token}}, + } + jwtClient := ProvideJWT(jwtService, cfg) + _, err := jwtClient.Authenticate(context.Background(), &authn.Request{ + OrgID: 1, + HTTPRequest: httpReq, + Resp: nil, + }) + if tc.valid { + require.NoError(t, err) + } else { + require.Error(t, err) + } + } +} + func TestJWTTest(t *testing.T) { jwtService := &models.FakeJWTService{} jwtHeaderName := "X-Forwarded-User"