diff --git a/filters/auth/oidc_introspection.go b/filters/auth/oidc_introspection.go index cdde259678..dbea2b1b5e 100644 --- a/filters/auth/oidc_introspection.go +++ b/filters/auth/oidc_introspection.go @@ -131,13 +131,13 @@ func (filter *oidcIntrospectionFilter) Request(ctx filters.FilterContext) { return } - sub, ok := token.Claims["sub"] - if ok { - authorized(ctx, sub.(string)) - } else { - sub := token.Subject - authorized(ctx, sub) + sub, ok := token.Claims["sub"].(string) + if !ok { + unauthorized(ctx, sub, invalidSub, "", "") + return } + + authorized(ctx, sub) } func (filter *oidcIntrospectionFilter) Response(filters.FilterContext) {} diff --git a/filters/auth/oidc_introspection_test.go b/filters/auth/oidc_introspection_test.go index c20a7d7482..1a4186ee0a 100644 --- a/filters/auth/oidc_introspection_test.go +++ b/filters/auth/oidc_introspection_test.go @@ -139,11 +139,12 @@ func TestCreateOIDCQueryClaimsFilter(t *testing.T) { func TestOIDCQueryClaimsFilter(t *testing.T) { for _, tc := range []struct { - msg string - path string - expected int - expectErr bool - args []interface{} + msg string + path string + expected int + expectErr bool + args []interface{} + removeClaims []string }{ { msg: "secure sub/path not permitted", @@ -165,6 +166,17 @@ func TestOIDCQueryClaimsFilter(t *testing.T) { expected: 200, expectErr: false, }, + { + msg: "secure sub/path is not permitted", + args: []interface{}{ + "/login:groups.#[==\"AppX-Test-Users\"]", + "/:@_:email%\"*@example.org\"", + }, + path: "/login/page", + expected: 401, + expectErr: false, + removeClaims: []string{"sub"}, + }, { msg: "generic user path permitted", args: []interface{}{ @@ -292,7 +304,7 @@ func TestOIDCQueryClaimsFilter(t *testing.T) { t.Errorf("Failed to parse url %s: %v", proxy.URL, err) } reqURL.Path = tc.path - oidcServer := createOIDCServer(proxy.URL+"/redirect", validClient, "mysec", jwt.MapClaims{"groups": []string{"CD-Administrators", "Purchasing-Department", "AppX-Test-Users", "white space"}}) + oidcServer := createOIDCServer(proxy.URL+"/redirect", validClient, "mysec", jwt.MapClaims{"groups": []string{"CD-Administrators", "Purchasing-Department", "AppX-Test-Users", "white space"}}, tc.removeClaims) defer oidcServer.Close() t.Logf("oidc/auth server URL: %s", oidcServer.URL) // create filter diff --git a/filters/auth/oidc_test.go b/filters/auth/oidc_test.go index 771fde01fe..0b1b3839e3 100644 --- a/filters/auth/oidc_test.go +++ b/filters/auth/oidc_test.go @@ -127,7 +127,7 @@ var testOpenIDConfig = `{ // returns a localhost instance implementation of an OpenID Connect // server with configendpoint, tokenendpoint, authenticationserver endpoint, userinfor // endpoint, jwks endpoint -func createOIDCServer(cb, client, clientsecret string, extraClaims jwt.MapClaims) *httptest.Server { +func createOIDCServer(cb, client, clientsecret string, extraClaims jwt.MapClaims, removeClaims []string) *httptest.Server { var oidcServer *httptest.Server oidcServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { @@ -233,6 +233,11 @@ func createOIDCServer(cb, client, clientsecret string, extraClaims jwt.MapClaims for k, v := range extraClaims { claims[k] = v } + + for _, k := range removeClaims { + delete(claims, k) + } + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) privKey, err := os.ReadFile(keyPath) @@ -557,7 +562,7 @@ func TestNewOidc(t *testing.T) { } func TestCreateFilterOIDC(t *testing.T) { - oidcServer := createOIDCServer("", "", "", nil) + oidcServer := createOIDCServer("", "", "", nil, nil) defer oidcServer.Close() for _, tt := range []struct { @@ -900,7 +905,7 @@ func TestOIDCSetup(t *testing.T) { t.Logf("redirect URL: %s", redirectURL.String()) - oidcServer := createOIDCServer(redirectURL.String(), "valid-client", "mysec", tc.extraClaims) + oidcServer := createOIDCServer(redirectURL.String(), "valid-client", "mysec", tc.extraClaims, nil) defer oidcServer.Close() t.Logf("oidc server URL: %s", oidcServer.URL)