Merge pull request #127010 from aramase/aramase/f/kep_3331_jti

Set credential-id in userinfo.extra for jwt authenticators if jti claim present

pkg/registry/core/serviceaccount/storage/token.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/apiserver/pkg/audit"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/serviceaccount"
+	authenticationtokenjwt "k8s.io/apiserver/pkg/authentication/token/jwt"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/rest"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -203,7 +204,7 @@ func (r *TokenREST) Create(ctx context.Context, name string, obj runtime.Object,
 	}
 
 	if r.maxExpirationSeconds > 0 && req.Spec.ExpirationSeconds > r.maxExpirationSeconds {
-		//only positive value is valid
+		// only positive value is valid
 		warning.AddWarning(ctx, "", fmt.Sprintf("requested expiration of %d seconds shortened to %d seconds", req.Spec.ExpirationSeconds, r.maxExpirationSeconds))
 		req.Spec.ExpirationSeconds = r.maxExpirationSeconds
 	}
@@ -235,7 +236,7 @@ func (r *TokenREST) Create(ctx context.Context, name string, obj runtime.Object,
 		ExpirationTimestamp: metav1.Time{Time: nowTime.Add(time.Duration(out.Spec.ExpirationSeconds) * time.Second)},
 	}
 	if utilfeature.DefaultFeatureGate.Enabled(features.ServiceAccountTokenJTI) && len(sc.ID) > 0 {
-		audit.AddAuditAnnotation(ctx, serviceaccount.IssuedCredentialIDAuditAnnotationKey, serviceaccount.CredentialIDForJTI(sc.ID))
+		audit.AddAuditAnnotation(ctx, serviceaccount.IssuedCredentialIDAuditAnnotationKey, authenticationtokenjwt.CredentialIDForJTI(sc.ID))
 	}
 	return out, nil
 }

pkg/serviceaccount/claims.go

@@ -27,6 +27,7 @@ import (
 
 	"k8s.io/apiserver/pkg/audit"
 	apiserverserviceaccount "k8s.io/apiserver/pkg/authentication/serviceaccount"
+	authenticationtokenjwt "k8s.io/apiserver/pkg/authentication/token/jwt"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/apis/core"
@@ -286,6 +287,6 @@ func (v *validator) Validate(ctx context.Context, _ string, public *jwt.Claims,
 		PodUID:       podUID,
 		NodeName:     nodeName,
 		NodeUID:      nodeUID,
-		CredentialID: apiserverserviceaccount.CredentialIDForJTI(jti),
+		CredentialID: authenticationtokenjwt.CredentialIDForJTI(jti),
 	}, nil
 }

staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go

@@ -163,15 +163,6 @@ func (sa *ServiceAccountInfo) UserInfo() user.Info {
 	return info
 }
 
-// CredentialIDForJTI converts a given JTI string into a credential identifier for use in a
-// users 'extra' info.
-func CredentialIDForJTI(jti string) string {
-	if len(jti) == 0 {
-		return ""
-	}
-	return "JTI=" + jti
-}
-
 // IsServiceAccountToken returns true if the secret is a valid api token for the service account
 func IsServiceAccountToken(secret *v1.Secret, sa *v1.ServiceAccount) bool {
 	if secret.Type != v1.SecretTypeServiceAccountToken {

staging/src/k8s.io/apiserver/pkg/authentication/token/jwt/jwt.go

@@ -0,0 +1,26 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package jwt
+
+// CredentialIDForJTI converts a given JTI string into a credential identifier for use in a
+// users 'extra' info.
+func CredentialIDForJTI(jti string) string {
+	if len(jti) == 0 {
+		return ""
+	}
+	return "JTI=" + jti
+}

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go

@@ -56,6 +56,7 @@ import (
 	apiservervalidation "k8s.io/apiserver/pkg/apis/apiserver/validation"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	authenticationcel "k8s.io/apiserver/pkg/authentication/cel"
+	authenticationtokenjwt "k8s.io/apiserver/pkg/authentication/token/jwt"
 	"k8s.io/apiserver/pkg/authentication/user"
 	certutil "k8s.io/client-go/util/cert"
 	"k8s.io/klog/v2"
@@ -726,7 +727,7 @@ func (a *jwtAuthenticator) AuthenticateToken(ctx context.Context, token string)
 		return nil, false, err
 	}
 
-	extra, err := a.getExtra(ctx, claimsUnstructured)
+	extra, err := a.getExtra(ctx, c, claimsUnstructured)
 	if err != nil {
 		return nil, false, err
 	}
@@ -914,17 +915,21 @@ func (a *jwtAuthenticator) getUID(ctx context.Context, c claims, claimsUnstructu
 	return evalResult.EvalResult.Value().(string), nil
 }
 
-func (a *jwtAuthenticator) getExtra(ctx context.Context, claimsUnstructured *unstructured.Unstructured) (map[string][]string, error) {
+func (a *jwtAuthenticator) getExtra(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) (map[string][]string, error) {
+	extra := make(map[string][]string)
+
+	if credentialID := getCredentialID(c); len(credentialID) > 0 {
+		extra[user.CredentialIDKey] = []string{credentialID}
+	}
+
 	if a.celMapper.Extra == nil {
-		return nil, nil
+		return extra, nil
 	}
 
 	evalResult, err := a.celMapper.Extra.EvalClaimMappings(ctx, claimsUnstructured)
 	if err != nil {
 		return nil, err
 	}
-
-	extra := make(map[string][]string, len(evalResult))
 	for _, result := range evalResult {
 		extraMapping, ok := result.ExpressionAccessor.(*authenticationcel.ExtraMappingExpression)
 		if !ok {
@@ -936,16 +941,25 @@ func (a *jwtAuthenticator) getExtra(ctx context.Context, claimsUnstructured *uns
 			return nil, fmt.Errorf("oidc: error evaluating extra claim expression: %s: %w", extraMapping.Expression, err)
 		}
 
-		if len(extraValues) == 0 {
+		if len(extraValues) > 0 {
-			continue
+			extra[extraMapping.Key] = extraValues
 		}
-
-		extra[extraMapping.Key] = extraValues
 	}
 
 	return extra, nil
 }
 
+func getCredentialID(c claims) string {
+	if _, ok := c["jti"]; ok {
+		var jti string
+		if err := c.unmarshalClaim("jti", &jti); err == nil {
+			return authenticationtokenjwt.CredentialIDForJTI(jti)
+		}
+	}
+
+	return ""
+}
+
 // getClaimJWT gets a distributed claim JWT from url, using the supplied access
 // token as bearer token.  If the access token is "", the authorization header
 // will not be set.

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go

@@ -3356,6 +3356,152 @@ func TestToken(t *testing.T) {
 				Name: "jane",
 			},
 		},
+		{
+			name: "credential id set in extra even when no extra claim mappings are defined",
+			options: Options{
+				JWTAuthenticator: apiserver.JWTAuthenticator{
+					Issuer: apiserver.Issuer{
+						URL:       "https://auth.example.com",
+						Audiences: []string{"my-client"},
+					},
+					ClaimMappings: apiserver.ClaimMappings{
+						Username: apiserver.PrefixedClaimOrExpression{
+							Expression: "claims.username",
+						},
+					},
+				},
+				now: func() time.Time { return now },
+			},
+			signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256),
+			pubKeys: []*jose.JSONWebKey{
+				loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256),
+			},
+			claims: fmt.Sprintf(`{
+				"iss": "https://auth.example.com",
+				"aud": "my-client",
+				"username": "jane",
+				"exp": %d,
+				"jti": "1234"
+			}`, valid.Unix()),
+			want: &user.DefaultInfo{
+				Name: "jane",
+				Extra: map[string][]string{
+					user.CredentialIDKey: {"JTI=1234"},
+				},
+			},
+		},
+		{
+			name: "credential id set in extra when extra claim mappings are defined",
+			options: Options{
+				JWTAuthenticator: apiserver.JWTAuthenticator{
+					Issuer: apiserver.Issuer{
+						URL:       "https://auth.example.com",
+						Audiences: []string{"my-client"},
+					},
+					ClaimMappings: apiserver.ClaimMappings{
+						Username: apiserver.PrefixedClaimOrExpression{
+							Expression: "claims.username",
+						},
+						Extra: []apiserver.ExtraMapping{
+							{
+								Key:             "example.org/foo",
+								ValueExpression: "claims.foo",
+							},
+							{
+								Key:             "example.org/bar",
+								ValueExpression: "claims.bar",
+							},
+						},
+					},
+				},
+				now: func() time.Time { return now },
+			},
+			signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256),
+			pubKeys: []*jose.JSONWebKey{
+				loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256),
+			},
+			claims: fmt.Sprintf(`{
+				"iss": "https://auth.example.com",
+				"aud": "my-client",
+				"username": "jane",
+				"exp": %d,
+				"jti": "1234",
+				"foo": "bar",
+				"bar": [
+					"baz",
+					"qux"
+				]
+			}`, valid.Unix()),
+			want: &user.DefaultInfo{
+				Name: "jane",
+				Extra: map[string][]string{
+					user.CredentialIDKey: {"JTI=1234"},
+					"example.org/foo":    {"bar"},
+					"example.org/bar":    {"baz", "qux"},
+				},
+			},
+		},
+		{
+			name: "non-string jti claim does not set credential id in extra or error",
+			options: Options{
+				JWTAuthenticator: apiserver.JWTAuthenticator{
+					Issuer: apiserver.Issuer{
+						URL:       "https://auth.example.com",
+						Audiences: []string{"my-client"},
+					},
+					ClaimMappings: apiserver.ClaimMappings{
+						Username: apiserver.PrefixedClaimOrExpression{
+							Expression: "claims.username",
+						},
+					},
+				},
+				now: func() time.Time { return now },
+			},
+			signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256),
+			pubKeys: []*jose.JSONWebKey{
+				loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256),
+			},
+			claims: fmt.Sprintf(`{
+				"iss": "https://auth.example.com",
+				"aud": "my-client",
+				"username": "jane",
+				"exp": %d,
+				"jti": 1234
+			}`, valid.Unix()),
+			want: &user.DefaultInfo{
+				Name: "jane",
+			},
+		},
+		{
+			name: "missing jti claim does not set credential id in extra or error",
+			options: Options{
+				JWTAuthenticator: apiserver.JWTAuthenticator{
+					Issuer: apiserver.Issuer{
+						URL:       "https://auth.example.com",
+						Audiences: []string{"my-client"},
+					},
+					ClaimMappings: apiserver.ClaimMappings{
+						Username: apiserver.PrefixedClaimOrExpression{
+							Expression: "claims.username",
+						},
+					},
+				},
+				now: func() time.Time { return now },
+			},
+			signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256),
+			pubKeys: []*jose.JSONWebKey{
+				loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256),
+			},
+			claims: fmt.Sprintf(`{
+				"iss": "https://auth.example.com",
+				"aud": "my-client",
+				"username": "jane",
+				"exp": %d
+			}`, valid.Unix()),
+			want: &user.DefaultInfo{
+				Name: "jane",
+			},
+		},
 	}
 
 	var successTestCount, failureTestCount int

test/integration/apiserver/oidc/oidc_test.go

@@ -629,6 +629,7 @@ jwt:
 						"sub": defaultOIDCClaimedUsername,
 						"aud": defaultOIDCClientID,
 						"exp": time.Now().Add(idTokenLifetime).Unix(),
+						"jti": "0123456789",
 					},
 					defaultStubAccessToken,
 					defaultStubRefreshToken,
@@ -641,6 +642,10 @@ jwt:
 			wantUser: &authenticationv1.UserInfo{
 				Username: "k8s-john_doe",
 				Groups:   []string{"system:authenticated"},
+				Extra: map[string]authenticationv1.ExtraValue{
+					// validates credential id is set correctly when jti claim is present
+					"authentication.kubernetes.io/credential-id": {"JTI=0123456789"},
+				},
 			},
 		},
 		{
@@ -777,6 +782,7 @@ jwt:
 						"aud": defaultOIDCClientID,
 						"exp": time.Now().Add(idTokenLifetime).Unix(),
 						"baz": "qux",
+						"jti": "0123456789",
 					},
 					defaultStubAccessToken,
 					defaultStubRefreshToken,
@@ -790,6 +796,8 @@ jwt:
 				Username: "k8s-john_doe",
 				Groups:   []string{"system:authenticated"},
 				Extra: map[string]authenticationv1.ExtraValue{
+					// validates credential id is set correctly and other extra fields are set
+					"authentication.kubernetes.io/credential-id": {"JTI=0123456789"},
 					"example.org/foo": {"bar"},
 					"example.org/baz": {"qux"},
 				},
@@ -945,6 +953,52 @@ jwt:
 				UID:      "1234",
 			},
 		},
+		{
+			name: "non-string jti claim doesn't result in authentication error",
+			authConfigFn: func(t *testing.T, issuerURL, caCert string) string {
+				return fmt.Sprintf(`
+apiVersion: apiserver.config.k8s.io/v1alpha1
+kind: AuthenticationConfiguration
+jwt:
+- issuer:
+    url: %s
+    audiences:
+    - %s
+    - another-audience
+    audienceMatchPolicy: MatchAny
+    certificateAuthority: |
+        %s
+  claimMappings:
+    username:
+      expression: "'k8s-' + claims.sub"
+`, issuerURL, defaultOIDCClientID, indentCertificateAuthority(caCert))
+			},
+			configureInfrastructure: configureTestInfrastructure[*rsa.PrivateKey, *rsa.PublicKey],
+			configureOIDCServerBehaviour: func(t *testing.T, oidcServer *utilsoidc.TestServer, signingPrivateKey *rsa.PrivateKey) {
+				idTokenLifetime := time.Second * 1200
+				oidcServer.TokenHandler().EXPECT().Token().RunAndReturn(utilsoidc.TokenHandlerBehaviorReturningPredefinedJWT(
+					t,
+					signingPrivateKey,
+					map[string]interface{}{
+						"iss": oidcServer.URL(),
+						"sub": defaultOIDCClaimedUsername,
+						"aud": defaultOIDCClientID,
+						"exp": time.Now().Add(idTokenLifetime).Unix(),
+						"jti": 1234,
+					},
+					defaultStubAccessToken,
+					defaultStubRefreshToken,
+				)).Times(1)
+			},
+			configureClient: configureClientFetchingOIDCCredentials,
+			assertErrFn: func(t *testing.T, errorToCheck error) {
+				assert.NoError(t, errorToCheck)
+			},
+			wantUser: &authenticationv1.UserInfo{
+				Username: "k8s-john_doe",
+				Groups:   []string{"system:authenticated"},
+			},
+		},
 	}
 
 	for _, tt := range tests {