Validate structured authn feature is enabled for discovery url/multiple

audiences

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
This commit is contained in:
Anish Ramasekar 2024-07-23 15:04:02 -07:00
parent 2a372a99bc
commit f80c73248f
No known key found for this signature in database
GPG Key ID: E96F745A34A409C2
2 changed files with 110 additions and 74 deletions

View File

@ -104,7 +104,7 @@ func validateJWTAuthenticator(authenticator api.JWTAuthenticator, fldPath *field
compiler := authenticationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))
state := &validationState{}
allErrs = append(allErrs, validateIssuer(authenticator.Issuer, disallowedIssuers, fldPath.Child("issuer"))...)
allErrs = append(allErrs, validateIssuer(authenticator.Issuer, disallowedIssuers, fldPath.Child("issuer"), structuredAuthnFeatureEnabled)...)
allErrs = append(allErrs, validateClaimValidationRules(compiler, state, authenticator.ClaimValidationRules, fldPath.Child("claimValidationRules"), structuredAuthnFeatureEnabled)...)
allErrs = append(allErrs, validateClaimMappings(compiler, state, authenticator.ClaimMappings, fldPath.Child("claimMappings"), structuredAuthnFeatureEnabled)...)
allErrs = append(allErrs, validateUserValidationRules(compiler, state, authenticator.UserValidationRules, fldPath.Child("userValidationRules"), structuredAuthnFeatureEnabled)...)
@ -118,12 +118,12 @@ type validationState struct {
usesEmailVerifiedClaim bool
}
func validateIssuer(issuer api.Issuer, disallowedIssuers sets.Set[string], fldPath *field.Path) field.ErrorList {
func validateIssuer(issuer api.Issuer, disallowedIssuers sets.Set[string], fldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList {
var allErrs field.ErrorList
allErrs = append(allErrs, validateIssuerURL(issuer.URL, disallowedIssuers, fldPath.Child("url"))...)
allErrs = append(allErrs, validateIssuerDiscoveryURL(issuer.URL, issuer.DiscoveryURL, fldPath.Child("discoveryURL"))...)
allErrs = append(allErrs, validateAudiences(issuer.Audiences, issuer.AudienceMatchPolicy, fldPath.Child("audiences"), fldPath.Child("audienceMatchPolicy"))...)
allErrs = append(allErrs, validateIssuerDiscoveryURL(issuer.URL, issuer.DiscoveryURL, fldPath.Child("discoveryURL"), structuredAuthnFeatureEnabled)...)
allErrs = append(allErrs, validateAudiences(issuer.Audiences, issuer.AudienceMatchPolicy, fldPath.Child("audiences"), fldPath.Child("audienceMatchPolicy"), structuredAuthnFeatureEnabled)...)
allErrs = append(allErrs, validateCertificateAuthority(issuer.CertificateAuthority, fldPath.Child("certificateAuthority"))...)
return allErrs
@ -137,13 +137,17 @@ func validateIssuerURL(issuerURL string, disallowedIssuers sets.Set[string], fld
return validateURL(issuerURL, disallowedIssuers, fldPath)
}
func validateIssuerDiscoveryURL(issuerURL, issuerDiscoveryURL string, fldPath *field.Path) field.ErrorList {
func validateIssuerDiscoveryURL(issuerURL, issuerDiscoveryURL string, fldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList {
var allErrs field.ErrorList
if len(issuerDiscoveryURL) == 0 {
return nil
}
if !structuredAuthnFeatureEnabled {
allErrs = append(allErrs, field.Invalid(fldPath, issuerDiscoveryURL, "discoveryURL is not supported when StructuredAuthenticationConfiguration feature gate is disabled"))
}
if len(issuerURL) > 0 && strings.TrimRight(issuerURL, "/") == strings.TrimRight(issuerDiscoveryURL, "/") {
allErrs = append(allErrs, field.Invalid(fldPath, issuerDiscoveryURL, "discoveryURL must be different from URL"))
}
@ -181,7 +185,7 @@ func validateURL(issuerURL string, disallowedIssuers sets.Set[string], fldPath *
return allErrs
}
func validateAudiences(audiences []string, audienceMatchPolicy api.AudienceMatchPolicyType, fldPath, audienceMatchPolicyFldPath *field.Path) field.ErrorList {
func validateAudiences(audiences []string, audienceMatchPolicy api.AudienceMatchPolicyType, fldPath, audienceMatchPolicyFldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList {
var allErrs field.ErrorList
if len(audiences) == 0 {
@ -189,6 +193,10 @@ func validateAudiences(audiences []string, audienceMatchPolicy api.AudienceMatch
return allErrs
}
if len(audiences) > 1 && !structuredAuthnFeatureEnabled {
allErrs = append(allErrs, field.Invalid(fldPath, audiences, "multiple audiences are not supported when StructuredAuthenticationConfiguration feature gate is disabled"))
}
seenAudiences := sets.NewString()
for i, audience := range audiences {
fldPath := fldPath.Index(i)

View File

@ -693,80 +693,98 @@ func TestValidateIssuerDiscoveryURL(t *testing.T) {
fldPath := field.NewPath("issuer", "discoveryURL")
testCases := []struct {
name string
in string
issuerURL string
want string
name string
in string
issuerURL string
want string
structuredAuthnFeatureEnabled bool
}{
{
name: "url is empty",
in: "",
want: "",
name: "url is empty",
in: "",
want: "",
structuredAuthnFeatureEnabled: true,
},
{
name: "url parse error",
in: "https://oidc.oidc-namespace.svc:invalid-port",
want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc:invalid-port": parse "https://oidc.oidc-namespace.svc:invalid-port": invalid port ":invalid-port" after host`,
name: "url parse error",
in: "https://oidc.oidc-namespace.svc:invalid-port",
want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc:invalid-port": parse "https://oidc.oidc-namespace.svc:invalid-port": invalid port ":invalid-port" after host`,
structuredAuthnFeatureEnabled: true,
},
{
name: "url is not https",
in: "http://oidc.oidc-namespace.svc",
want: `issuer.discoveryURL: Invalid value: "http://oidc.oidc-namespace.svc": URL scheme must be https`,
name: "url is not https",
in: "http://oidc.oidc-namespace.svc",
want: `issuer.discoveryURL: Invalid value: "http://oidc.oidc-namespace.svc": URL scheme must be https`,
structuredAuthnFeatureEnabled: true,
},
{
name: "url user info is not allowed",
in: "https://user:pass@oidc.oidc-namespace.svc",
want: `issuer.discoveryURL: Invalid value: "https://user:pass@oidc.oidc-namespace.svc": URL must not contain a username or password`,
name: "url user info is not allowed",
in: "https://user:pass@oidc.oidc-namespace.svc",
want: `issuer.discoveryURL: Invalid value: "https://user:pass@oidc.oidc-namespace.svc": URL must not contain a username or password`,
structuredAuthnFeatureEnabled: true,
},
{
name: "url raw query is not allowed",
in: "https://oidc.oidc-namespace.svc?query",
want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc?query": URL must not contain a query`,
name: "url raw query is not allowed",
in: "https://oidc.oidc-namespace.svc?query",
want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc?query": URL must not contain a query`,
structuredAuthnFeatureEnabled: true,
},
{
name: "url fragment is not allowed",
in: "https://oidc.oidc-namespace.svc#fragment",
want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc#fragment": URL must not contain a fragment`,
name: "url fragment is not allowed",
in: "https://oidc.oidc-namespace.svc#fragment",
want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc#fragment": URL must not contain a fragment`,
structuredAuthnFeatureEnabled: true,
},
{
name: "valid url",
name: "valid url",
in: "https://oidc.oidc-namespace.svc",
want: "",
structuredAuthnFeatureEnabled: true,
},
{
name: "valid url with path",
in: "https://oidc.oidc-namespace.svc/path",
want: "",
structuredAuthnFeatureEnabled: true,
},
{
name: "discovery url same as issuer url",
issuerURL: "https://issuer-url",
in: "https://issuer-url",
want: `issuer.discoveryURL: Invalid value: "https://issuer-url": discoveryURL must be different from URL`,
structuredAuthnFeatureEnabled: true,
},
{
name: "discovery url same as issuer url, with trailing slash",
issuerURL: "https://issuer-url",
in: "https://issuer-url/",
want: `issuer.discoveryURL: Invalid value: "https://issuer-url/": discoveryURL must be different from URL`,
structuredAuthnFeatureEnabled: true,
},
{
name: "discovery url same as issuer url, with multiple trailing slashes",
issuerURL: "https://issuer-url",
in: "https://issuer-url///",
want: `issuer.discoveryURL: Invalid value: "https://issuer-url///": discoveryURL must be different from URL`,
structuredAuthnFeatureEnabled: true,
},
{
name: "discovery url same as issuer url, issuer url with trailing slash",
issuerURL: "https://issuer-url/",
in: "https://issuer-url",
want: `issuer.discoveryURL: Invalid value: "https://issuer-url": discoveryURL must be different from URL`,
structuredAuthnFeatureEnabled: true,
},
{
name: "discovery url set but structured authn feature disabled",
in: "https://oidc.oidc-namespace.svc",
want: "",
},
{
name: "valid url with path",
in: "https://oidc.oidc-namespace.svc/path",
want: "",
},
{
name: "discovery url same as issuer url",
issuerURL: "https://issuer-url",
in: "https://issuer-url",
want: `issuer.discoveryURL: Invalid value: "https://issuer-url": discoveryURL must be different from URL`,
},
{
name: "discovery url same as issuer url, with trailing slash",
issuerURL: "https://issuer-url",
in: "https://issuer-url/",
want: `issuer.discoveryURL: Invalid value: "https://issuer-url/": discoveryURL must be different from URL`,
},
{
name: "discovery url same as issuer url, with multiple trailing slashes",
issuerURL: "https://issuer-url",
in: "https://issuer-url///",
want: `issuer.discoveryURL: Invalid value: "https://issuer-url///": discoveryURL must be different from URL`,
},
{
name: "discovery url same as issuer url, issuer url with trailing slash",
issuerURL: "https://issuer-url/",
in: "https://issuer-url",
want: `issuer.discoveryURL: Invalid value: "https://issuer-url": discoveryURL must be different from URL`,
want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc": discoveryURL is not supported when StructuredAuthenticationConfiguration feature gate is disabled`,
},
}
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
got := validateIssuerDiscoveryURL(tt.issuerURL, tt.in, fldPath).ToAggregate()
got := validateIssuerDiscoveryURL(tt.issuerURL, tt.in, fldPath, tt.structuredAuthnFeatureEnabled).ToAggregate()
if d := cmp.Diff(tt.want, errString(got)); d != "" {
t.Fatalf("URL validation mismatch (-want +got):\n%s", d)
}
@ -779,10 +797,11 @@ func TestValidateAudiences(t *testing.T) {
audienceMatchPolicyFldPath := field.NewPath("issuer", "audienceMatchPolicy")
testCases := []struct {
name string
in []string
matchPolicy string
want string
name string
in []string
matchPolicy string
want string
structuredAuthnFeatureEnabled bool
}{
{
name: "audiences is empty",
@ -812,27 +831,36 @@ func TestValidateAudiences(t *testing.T) {
want: "",
},
{
name: "duplicate audience",
in: []string{"audience", "audience"},
matchPolicy: "MatchAny",
want: `issuer.audiences[1]: Duplicate value: "audience"`,
name: "duplicate audience",
in: []string{"audience", "audience"},
matchPolicy: "MatchAny",
want: `issuer.audiences[1]: Duplicate value: "audience"`,
structuredAuthnFeatureEnabled: true,
},
{
name: "match policy not set with multiple audiences",
in: []string{"audience1", "audience2"},
want: `issuer.audienceMatchPolicy: Invalid value: "": audienceMatchPolicy must be MatchAny for multiple audiences`,
name: "match policy not set with multiple audiences",
in: []string{"audience1", "audience2"},
want: `issuer.audienceMatchPolicy: Invalid value: "": audienceMatchPolicy must be MatchAny for multiple audiences`,
structuredAuthnFeatureEnabled: true,
},
{
name: "valid multiple audiences",
name: "valid multiple audiences",
in: []string{"audience1", "audience2"},
matchPolicy: "MatchAny",
want: "",
structuredAuthnFeatureEnabled: true,
},
{
name: "multiple audiences set when structured authn feature is disabled",
in: []string{"audience1", "audience2"},
matchPolicy: "MatchAny",
want: "",
want: `issuer.audiences: Invalid value: []string{"audience1", "audience2"}: multiple audiences are not supported when StructuredAuthenticationConfiguration feature gate is disabled`,
},
}
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
got := validateAudiences(tt.in, api.AudienceMatchPolicyType(tt.matchPolicy), fldPath, audienceMatchPolicyFldPath).ToAggregate()
got := validateAudiences(tt.in, api.AudienceMatchPolicyType(tt.matchPolicy), fldPath, audienceMatchPolicyFldPath, tt.structuredAuthnFeatureEnabled).ToAggregate()
if d := cmp.Diff(tt.want, errString(got)); d != "" {
t.Fatalf("Audiences validation mismatch (-want +got):\n%s", d)
}