Fixes issue with sub in token claims

Signed-off-by: wassafshahzad <wassafshahzad@gmail.com>
zalando · Nov 18, 2024 · 3cfb9fa · 3cfb9fa
1 parent 628aae3
commit 3cfb9fa
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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)