Skip to content

Commit

Permalink
Merge pull request #123431 from aramase/aramase/f/kep_3331_multiple_j…
Browse files Browse the repository at this point in the history
…wt_authenticator

Support multiple JWT authenticators with structured authn config
  • Loading branch information
k8s-ci-robot committed Mar 7, 2024
2 parents 05cb0a5 + 39e1c91 commit c726b2b
Show file tree
Hide file tree
Showing 3 changed files with 211 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,32 +45,33 @@ func ValidateAuthenticationConfiguration(c *api.AuthenticationConfiguration, dis
root := field.NewPath("jwt")
var allErrs field.ErrorList

// This stricter validation is solely based on what the current implementation supports.
// TODO(aramase): when StructuredAuthenticationConfiguration feature gate is added and wired up,
// relax this check to allow 0 authenticators. This will allow us to support the case where
// API server is initially configured with no authenticators and then authenticators are added
// later via dynamic config.
if len(c.JWT) == 0 {
allErrs = append(allErrs, field.Required(root, fmt.Sprintf(atLeastOneRequiredErrFmt, root)))
return allErrs
}
// We allow 0 authenticators in the authentication configuration.
// This allows us to support scenarios where the API server is initially set up without
// any authenticators and then authenticators are added later via dynamic config.

// This stricter validation is because the --oidc-* flag option is singular.
// TODO(aramase): when StructuredAuthenticationConfiguration feature gate is added and wired up,
// remove the 1 authenticator limit check and add set the limit to 64.
if len(c.JWT) > 1 {
allErrs = append(allErrs, field.TooMany(root, len(c.JWT), 1))
if len(c.JWT) > 64 {
allErrs = append(allErrs, field.TooMany(root, len(c.JWT), 64))
return allErrs
}

// TODO(aramase): right now we only support a single JWT authenticator as
// this is wired to the --oidc-* flags. When StructuredAuthenticationConfiguration
// feature gate is added and wired up, we will remove the 1 authenticator limit
// check and add validation for duplicate issuers.
seenIssuers := sets.New[string]()
seenDiscoveryURLs := sets.New[string]()
for i, a := range c.JWT {
fldPath := root.Index(i)
_, errs := validateJWTAuthenticator(a, fldPath, sets.New(disallowedIssuers...), utilfeature.DefaultFeatureGate.Enabled(features.StructuredAuthenticationConfiguration))
allErrs = append(allErrs, errs...)

if seenIssuers.Has(a.Issuer.URL) {
allErrs = append(allErrs, field.Duplicate(fldPath.Child("issuer").Child("url"), a.Issuer.URL))
}
seenIssuers.Insert(a.Issuer.URL)

if len(a.Issuer.DiscoveryURL) > 0 {
if seenDiscoveryURLs.Has(a.Issuer.DiscoveryURL) {
allErrs = append(allErrs, field.Duplicate(fldPath.Child("issuer").Child("discoveryURL"), a.Issuer.DiscoveryURL))
}
seenDiscoveryURLs.Insert(a.Issuer.DiscoveryURL)
}
}

return allErrs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,97 @@ func TestValidateAuthenticationConfiguration(t *testing.T) {
{
name: "jwt authenticator is empty",
in: &api.AuthenticationConfiguration{},
want: "jwt: Required value: at least one jwt is required",
want: "",
},
{
name: ">1 jwt authenticator",
name: "duplicate issuer across jwt authenticators",
in: &api.AuthenticationConfiguration{
JWT: []api.JWTAuthenticator{
{Issuer: api.Issuer{URL: "https://issuer-url", Audiences: []string{"audience"}}},
{Issuer: api.Issuer{URL: "https://issuer-url", Audiences: []string{"audience"}}},
{
Issuer: api.Issuer{
URL: "https://issuer-url",
Audiences: []string{"audience"},
},
ClaimValidationRules: []api.ClaimValidationRule{
{
Claim: "foo",
RequiredValue: "bar",
},
},
ClaimMappings: api.ClaimMappings{
Username: api.PrefixedClaimOrExpression{
Claim: "sub",
Prefix: pointer.String("prefix"),
},
},
},
{
Issuer: api.Issuer{
URL: "https://issuer-url",
Audiences: []string{"audience"},
},
ClaimValidationRules: []api.ClaimValidationRule{
{
Claim: "foo",
RequiredValue: "bar",
},
},
ClaimMappings: api.ClaimMappings{
Username: api.PrefixedClaimOrExpression{
Claim: "sub",
Prefix: pointer.String("prefix"),
},
},
},
},
},
want: `jwt[1].issuer.url: Duplicate value: "https://issuer-url"`,
},
{
name: "duplicate discoveryURL across jwt authenticators",
in: &api.AuthenticationConfiguration{
JWT: []api.JWTAuthenticator{
{
Issuer: api.Issuer{
URL: "https://issuer-url",
DiscoveryURL: "https://discovery-url/.well-known/openid-configuration",
Audiences: []string{"audience"},
},
ClaimValidationRules: []api.ClaimValidationRule{
{
Claim: "foo",
RequiredValue: "bar",
},
},
ClaimMappings: api.ClaimMappings{
Username: api.PrefixedClaimOrExpression{
Claim: "sub",
Prefix: pointer.String("prefix"),
},
},
},
{
Issuer: api.Issuer{
URL: "https://different-issuer-url",
DiscoveryURL: "https://discovery-url/.well-known/openid-configuration",
Audiences: []string{"audience"},
},
ClaimValidationRules: []api.ClaimValidationRule{
{
Claim: "foo",
RequiredValue: "bar",
},
},
ClaimMappings: api.ClaimMappings{
Username: api.PrefixedClaimOrExpression{
Claim: "sub",
Prefix: pointer.String("prefix"),
},
},
},
},
},
want: "jwt: Too many: 2: must have at most 1 items",
want: `jwt[1].issuer.discoveryURL: Duplicate value: "https://discovery-url/.well-known/openid-configuration"`,
},
{
name: "failed issuer validation",
Expand Down
107 changes: 107 additions & 0 deletions test/integration/apiserver/oidc/oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1057,6 +1057,113 @@ jwt:
}
}

func TestMultipleJWTAuthenticators(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthenticationConfiguration, true)()

caCertContent1, _, caFilePath1, caKeyFilePath1 := generateCert(t)
signingPrivateKey1, publicKey1 := rsaGenerateKey(t)
oidcServer1 := utilsoidc.BuildAndRunTestServer(t, caFilePath1, caKeyFilePath1, "")

caCertContent2, _, caFilePath2, caKeyFilePath2 := generateCert(t)
signingPrivateKey2, publicKey2 := rsaGenerateKey(t)
oidcServer2 := utilsoidc.BuildAndRunTestServer(t, caFilePath2, caKeyFilePath2, "https://example.com")

authenticationConfig := fmt.Sprintf(`
apiVersion: apiserver.config.k8s.io/v1alpha1
kind: AuthenticationConfiguration
jwt:
- issuer:
url: %s
audiences:
- foo
audienceMatchPolicy: MatchAny
certificateAuthority: |
%s
claimMappings:
username:
expression: "'k8s-' + claims.sub"
claimValidationRules:
- expression: 'claims.hd == "example.com"'
message: "the hd claim must be set to example.com"
- issuer:
url: "https://example.com"
discoveryURL: %s/.well-known/openid-configuration
audiences:
- bar
audienceMatchPolicy: MatchAny
certificateAuthority: |
%s
claimMappings:
username:
expression: "'k8s-' + claims.sub"
groups:
expression: '(claims.roles.split(",") + claims.other_roles.split(",")).map(role, "system:" + role)'
uid:
expression: "claims.uid"
`, oidcServer1.URL(), indentCertificateAuthority(string(caCertContent1)), oidcServer2.URL(), indentCertificateAuthority(string(caCertContent2)))

oidcServer1.JwksHandler().EXPECT().KeySet().AnyTimes().DoAndReturn(utilsoidc.DefaultJwksHandlerBehavior(t, publicKey1))
oidcServer2.JwksHandler().EXPECT().KeySet().AnyTimes().DoAndReturn(utilsoidc.DefaultJwksHandlerBehavior(t, publicKey2))

apiServer := startTestAPIServerForOIDC(t, apiServerOIDCConfig{authenticationConfigYAML: authenticationConfig}, publicKey1)

idTokenLifetime := time.Second * 1200
oidcServer1.TokenHandler().EXPECT().Token().Times(1).DoAndReturn(utilsoidc.TokenHandlerBehaviorReturningPredefinedJWT(
t,
signingPrivateKey1,
map[string]interface{}{
"iss": oidcServer1.URL(),
"sub": defaultOIDCClaimedUsername,
"aud": "foo",
"exp": time.Now().Add(idTokenLifetime).Unix(),
"hd": "example.com",
},
defaultStubAccessToken,
defaultStubRefreshToken,
))

oidcServer2.TokenHandler().EXPECT().Token().Times(1).DoAndReturn(utilsoidc.TokenHandlerBehaviorReturningPredefinedJWT(
t,
signingPrivateKey2,
map[string]interface{}{
"iss": "https://example.com",
"sub": "not_john_doe",
"aud": "bar",
"roles": "role1,role2",
"other_roles": "role3,role4",
"exp": time.Now().Add(idTokenLifetime).Unix(),
"uid": "1234",
},
defaultStubAccessToken,
defaultStubRefreshToken,
))

tokenURL1, err := oidcServer1.TokenURL()
require.NoError(t, err)

tokenURL2, err := oidcServer2.TokenURL()
require.NoError(t, err)

client1 := configureClientFetchingOIDCCredentials(t, apiServer.ClientConfig, caCertContent1, caFilePath1, oidcServer1.URL(), tokenURL1)
client2 := configureClientFetchingOIDCCredentials(t, apiServer.ClientConfig, caCertContent2, caFilePath2, oidcServer2.URL(), tokenURL2)

ctx := testContext(t)
res, err := client1.AuthenticationV1().SelfSubjectReviews().Create(ctx, &authenticationv1.SelfSubjectReview{}, metav1.CreateOptions{})
require.NoError(t, err)
assert.Equal(t, authenticationv1.UserInfo{
Username: "k8s-john_doe",
Groups: []string{"system:authenticated"},
}, res.Status.UserInfo)

res, err = client2.AuthenticationV1().SelfSubjectReviews().Create(ctx, &authenticationv1.SelfSubjectReview{}, metav1.CreateOptions{})
require.NoError(t, err)
assert.Equal(t, authenticationv1.UserInfo{
Username: "k8s-not_john_doe",
Groups: []string{"system:role1", "system:role2", "system:role3", "system:role4", "system:authenticated"},
UID: "1234",
}, res.Status.UserInfo)
}

func rsaGenerateKey(t *testing.T) (*rsa.PrivateKey, *rsa.PublicKey) {
t.Helper()

Expand Down

0 comments on commit c726b2b

Please sign in to comment.