AuthJWT: Fix JWT query param leak (CVE-2023-1387) (#825)

fix JWT query param leak

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
Co-authored-by: Kalle Persson <kalle.persson@grafana.com>

pkg/services/authn/clients/jwt.go

@@ -19,6 +19,8 @@ import (
 	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
+const authQueryParamName = "auth_token"
+
 var _ authn.ContextAwareClient = new(JWT)
 
 var (
@@ -50,6 +52,7 @@ func (s *JWT) Name() string {
 
 func (s *JWT) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identity, error) {
 	jwtToken := s.retrieveToken(r.HTTPRequest)
+	s.stripSensitiveParam(r.HTTPRequest)
 
 	claims, err := s.jwtService.Verify(ctx, jwtToken)
 	if err != nil {
@@ -120,6 +123,18 @@ func (s *JWT) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identi
 	return id, nil
 }
 
+// remove sensitive query param
+// avoid JWT URL login passing auth_token in URL
+func (s *JWT) stripSensitiveParam(httpRequest *http.Request) {
+	if s.cfg.JWTAuthURLLogin {
+		params := httpRequest.URL.Query()
+		if params.Has(authQueryParamName) {
+			params.Del(authQueryParamName)
+			httpRequest.URL.RawQuery = params.Encode()
+		}
+	}
+}
+
 // retrieveToken retrieves the JWT token from the request.
 func (s *JWT) retrieveToken(httpRequest *http.Request) string {
 	jwtToken := httpRequest.Header.Get(s.cfg.JWTAuthHeaderName)

pkg/services/authn/clients/jwt_test.go

@@ -307,3 +307,47 @@ func TestJWTTest(t *testing.T) {
 		})
 	}
 }
+
+func TestJWTStripParam(t *testing.T) {
+	jwtService := &jwt.FakeJWTService{
+		VerifyProvider: func(context.Context, string) (jwt.JWTClaims, error) {
+			return jwt.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,
+		JWTAuthURLLogin:                true,
+		JWTAuthRoleAttributeStrict:     false,
+		JWTAuthRoleAttributePath:       "roles",
+		JWTAuthEmailClaim:              "email",
+		JWTAuthUsernameClaim:           "preferred_username",
+	}
+
+	// #nosec G101 -- This is a dummy/test token
+	token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.XbPfbIHMI6arZ3Y922BhjWgQzWXcXNrz0ogtVhfEd2o"
+
+	httpReq := &http.Request{
+		URL: &url.URL{RawQuery: "auth_token=" + token + "&other_param=other_value"},
+	}
+	jwtClient := ProvideJWT(jwtService, cfg)
+	_, err := jwtClient.Authenticate(context.Background(), &authn.Request{
+		OrgID:       1,
+		HTTPRequest: httpReq,
+		Resp:        nil,
+	})
+	require.NoError(t, err)
+	// auth_token should be removed from the query string
+	assert.Equal(t, "other_param=other_value", httpReq.URL.RawQuery)
+}