Skip to content

Commit fb9f9b9

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #2771 from ShazaAldawamneh/CNTRLPLANE-3010
 CNTRLPLANE-3010: Add API-side validation to enforce prefixPolicy is not set when username expression is used
2 parents 8689398 + 43a92bf commit fb9f9b9

18 files changed

Lines changed: 238 additions & 823 deletions

‎config/v1/tests/authentications.config.openshift.io/ExternalOIDCWithUpstreamParity.yaml‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,107 @@ tests:
617617
claim: "preferred_username"
618618
groups:
619619
claim: ""
620+
- name: Cannot set prefixPolicy to Prefix when using username expression
621+
initial: |
622+
apiVersion: config.openshift.io/v1
623+
kind: Authentication
624+
spec:
625+
type: OIDC
626+
oidcProviders:
627+
- name: myoidc
628+
issuer:
629+
issuerURL: https://meh.tld
630+
audiences: ['openshift-aud']
631+
claimMappings:
632+
username:
633+
expression: "claims.sub"
634+
prefixPolicy: Prefix
635+
prefix:
636+
prefixString: "myoidc:"
637+
expectedError: "prefixPolicy must not be set to 'Prefix' when expression is set"
638+
639+
- name: Can set prefixPolicy to NoPrefix when using username expression
640+
initial: |
641+
apiVersion: config.openshift.io/v1
642+
kind: Authentication
643+
spec:
644+
type: OIDC
645+
oidcProviders:
646+
- name: myoidc
647+
issuer:
648+
issuerURL: https://meh.tld
649+
audiences: ['openshift-aud']
650+
claimMappings:
651+
username:
652+
expression: "claims.sub"
653+
prefixPolicy: NoPrefix
654+
expected: |
655+
apiVersion: config.openshift.io/v1
656+
kind: Authentication
657+
spec:
658+
type: OIDC
659+
oidcProviders:
660+
- name: myoidc
661+
issuer:
662+
issuerURL: https://meh.tld
663+
audiences: ['openshift-aud']
664+
claimMappings:
665+
username:
666+
expression: "claims.sub"
667+
prefixPolicy: NoPrefix
668+
669+
- name: Cannot set prefix to non-empty value when using groups expression
670+
initial: |
671+
apiVersion: config.openshift.io/v1
672+
kind: Authentication
673+
spec:
674+
type: OIDC
675+
oidcProviders:
676+
- name: myoidc
677+
issuer:
678+
issuerURL: https://meh.tld
679+
audiences: ['openshift-aud']
680+
claimMappings:
681+
username:
682+
claim: "preferred_username"
683+
groups:
684+
expression: "claims.groups"
685+
prefix: "oidc-group:"
686+
expectedError: "prefix must not be set to a non-empty value when expression is set"
687+
688+
- name: Can set prefix to empty string when using groups expression
689+
initial: |
690+
apiVersion: config.openshift.io/v1
691+
kind: Authentication
692+
spec:
693+
type: OIDC
694+
oidcProviders:
695+
- name: myoidc
696+
issuer:
697+
issuerURL: https://meh.tld
698+
audiences: ['openshift-aud']
699+
claimMappings:
700+
username:
701+
claim: "preferred_username"
702+
groups:
703+
expression: "claims.groups"
704+
prefix: ""
705+
expected: |
706+
apiVersion: config.openshift.io/v1
707+
kind: Authentication
708+
spec:
709+
type: OIDC
710+
oidcProviders:
711+
- name: myoidc
712+
issuer:
713+
issuerURL: https://meh.tld
714+
audiences: ['openshift-aud']
715+
claimMappings:
716+
username:
717+
claim: "preferred_username"
718+
groups:
719+
expression: "claims.groups"
720+
prefix: ""
620721
onUpdate:
621722
- name: Should allow updating other fields if existing username claim mapping is longer than 256 characters
622723
initialCRDPatches:

‎config/v1/types_authentication.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,7 @@ type OIDCClientReference struct {
618618
// +openshift:validation:FeatureGateAwareXValidation:featureGate=ExternalOIDC,rule="has(self.claim)",message="claim is required"
619619
// +openshift:validation:FeatureGateAwareXValidation:featureGate=ExternalOIDCWithUIDAndExtraClaimMappings,rule="has(self.claim)",message="claim is required"
620620
// +openshift:validation:FeatureGateAwareXValidation:featureGate=ExternalOIDCWithUpstreamParity,rule="has(self.claim) ? !has(self.expression) : has(self.expression)",message="precisely one of claim or expression must be set"
621+
// +openshift:validation:FeatureGateAwareXValidation:featureGate=ExternalOIDCWithUpstreamParity,rule="has(self.expression) && size(self.expression) > 0 ? !has(self.prefixPolicy) || self.prefixPolicy != 'Prefix' : true",message="prefixPolicy must not be set to 'Prefix' when expression is set"
621622
type UsernameClaimMapping struct {
622623
// claim is an optional field that configures the JWT token claim whose value is assigned to the cluster identity field associated with this mapping.
623624
// claim is required when the ExternalOIDCWithUpstreamParity feature gate is not enabled.
@@ -650,11 +651,9 @@ type UsernameClaimMapping struct {
650651
// Allowed values are 'Prefix', 'NoPrefix', and omitted (not provided or an empty string).
651652
//
652653
// When set to 'Prefix', the value specified in the prefix field will be prepended to the value of the JWT claim.
653-
//
654654
// The prefix field must be set when prefixPolicy is 'Prefix'.
655-
//
655+
// Must not be set to 'Prefix' when expression is set.
656656
// When set to 'NoPrefix', no prefix will be prepended to the value of the JWT claim.
657-
//
658657
// When omitted, this means no opinion and the platform is left to choose any prefixes that are applied which is subject to change over time.
659658
// Currently, the platform prepends `{issuerURL}#` to the value of the JWT claim when the claim is not 'email'.
660659
//
@@ -710,12 +709,14 @@ type UsernamePrefix struct {
710709

711710
// PrefixedClaimMapping configures a claim mapping
712711
// that allows for an optional prefix.
712+
// +openshift:validation:FeatureGateAwareXValidation:featureGate=ExternalOIDCWithUpstreamParity,rule="has(self.expression) && size(self.expression) > 0 ? (!has(self.prefix) || size(self.prefix) == 0) : true",message="prefix must not be set to a non-empty value when expression is set"
713713
type PrefixedClaimMapping struct {
714714
TokenClaimMapping `json:",inline"`
715715

716716
// prefix is an optional field that configures the prefix that will be applied to the cluster identity attribute during the process of mapping JWT claims to cluster identity attributes.
717717
//
718-
// When omitted (""), no prefix is applied to the cluster identity attribute.
718+
// When omitted or set to an empty string (""), no prefix is applied to the cluster identity attribute.
719+
// Must not be set to a non-empty value when expression is set.
719720
//
720721
// Example: if `prefix` is set to "myoidc:" and the `claim` in JWT contains an array of strings "a", "b" and "c", the mapping will result in an array of string "myoidc:a", "myoidc:b" and "myoidc:c".
721722
//

‎config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-CustomNoUpgrade.crd.yaml‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,18 @@ spec:
212212
description: |-
213213
prefix is an optional field that configures the prefix that will be applied to the cluster identity attribute during the process of mapping JWT claims to cluster identity attributes.
214214
215-
When omitted (""), no prefix is applied to the cluster identity attribute.
215+
When omitted or set to an empty string (""), no prefix is applied to the cluster identity attribute.
216+
Must not be set to a non-empty value when expression is set.
216217
217218
Example: if `prefix` is set to "myoidc:" and the `claim` in JWT contains an array of strings "a", "b" and "c", the mapping will result in an array of string "myoidc:a", "myoidc:b" and "myoidc:c".
218219
type: string
219220
type: object
220221
x-kubernetes-validations:
222+
- message: prefix must not be set to a non-empty value when
223+
expression is set
224+
rule: 'has(self.expression) && size(self.expression) >
225+
0 ? (!has(self.prefix) || size(self.prefix) == 0) :
226+
true'
221227
- message: expression must not be set if claim is specified
222228
and is not an empty string
223229
rule: '(size(self.?claim.orValue("")) > 0) ? !has(self.expression)
@@ -316,11 +322,9 @@ spec:
316322
Allowed values are 'Prefix', 'NoPrefix', and omitted (not provided or an empty string).
317323
318324
When set to 'Prefix', the value specified in the prefix field will be prepended to the value of the JWT claim.
319-
320325
The prefix field must be set when prefixPolicy is 'Prefix'.
321-
326+
Must not be set to 'Prefix' when expression is set.
322327
When set to 'NoPrefix', no prefix will be prepended to the value of the JWT claim.
323-
324328
When omitted, this means no opinion and the platform is left to choose any prefixes that are applied which is subject to change over time.
325329
Currently, the platform prepends `{issuerURL}#` to the value of the JWT claim when the claim is not 'email'.
326330
@@ -341,6 +345,11 @@ spec:
341345
- message: precisely one of claim or expression must be
342346
set
343347
rule: 'has(self.claim) ? !has(self.expression) : has(self.expression)'
348+
- message: prefixPolicy must not be set to 'Prefix' when
349+
expression is set
350+
rule: 'has(self.expression) && size(self.expression) >
351+
0 ? !has(self.prefixPolicy) || self.prefixPolicy !=
352+
''Prefix'' : true'
344353
- message: prefix must be set if prefixPolicy is 'Prefix',
345354
but must remain unset otherwise
346355
rule: 'has(self.prefixPolicy) && self.prefixPolicy ==

‎config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-Default.crd.yaml‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ spec:
199199
description: |-
200200
prefix is an optional field that configures the prefix that will be applied to the cluster identity attribute during the process of mapping JWT claims to cluster identity attributes.
201201
202-
When omitted (""), no prefix is applied to the cluster identity attribute.
202+
When omitted or set to an empty string (""), no prefix is applied to the cluster identity attribute.
203+
Must not be set to a non-empty value when expression is set.
203204
204205
Example: if `prefix` is set to "myoidc:" and the `claim` in JWT contains an array of strings "a", "b" and "c", the mapping will result in an array of string "myoidc:a", "myoidc:b" and "myoidc:c".
205206
type: string
@@ -288,11 +289,9 @@ spec:
288289
Allowed values are 'Prefix', 'NoPrefix', and omitted (not provided or an empty string).
289290
290291
When set to 'Prefix', the value specified in the prefix field will be prepended to the value of the JWT claim.
291-
292292
The prefix field must be set when prefixPolicy is 'Prefix'.
293-
293+
Must not be set to 'Prefix' when expression is set.
294294
When set to 'NoPrefix', no prefix will be prepended to the value of the JWT claim.
295-
296295
When omitted, this means no opinion and the platform is left to choose any prefixes that are applied which is subject to change over time.
297296
Currently, the platform prepends `{issuerURL}#` to the value of the JWT claim when the claim is not 'email'.
298297

‎config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-DevPreviewNoUpgrade.crd.yaml‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,18 @@ spec:
212212
description: |-
213213
prefix is an optional field that configures the prefix that will be applied to the cluster identity attribute during the process of mapping JWT claims to cluster identity attributes.
214214
215-
When omitted (""), no prefix is applied to the cluster identity attribute.
215+
When omitted or set to an empty string (""), no prefix is applied to the cluster identity attribute.
216+
Must not be set to a non-empty value when expression is set.
216217
217218
Example: if `prefix` is set to "myoidc:" and the `claim` in JWT contains an array of strings "a", "b" and "c", the mapping will result in an array of string "myoidc:a", "myoidc:b" and "myoidc:c".
218219
type: string
219220
type: object
220221
x-kubernetes-validations:
222+
- message: prefix must not be set to a non-empty value when
223+
expression is set
224+
rule: 'has(self.expression) && size(self.expression) >
225+
0 ? (!has(self.prefix) || size(self.prefix) == 0) :
226+
true'
221227
- message: expression must not be set if claim is specified
222228
and is not an empty string
223229
rule: '(size(self.?claim.orValue("")) > 0) ? !has(self.expression)
@@ -316,11 +322,9 @@ spec:
316322
Allowed values are 'Prefix', 'NoPrefix', and omitted (not provided or an empty string).
317323
318324
When set to 'Prefix', the value specified in the prefix field will be prepended to the value of the JWT claim.
319-
320325
The prefix field must be set when prefixPolicy is 'Prefix'.
321-
326+
Must not be set to 'Prefix' when expression is set.
322327
When set to 'NoPrefix', no prefix will be prepended to the value of the JWT claim.
323-
324328
When omitted, this means no opinion and the platform is left to choose any prefixes that are applied which is subject to change over time.
325329
Currently, the platform prepends `{issuerURL}#` to the value of the JWT claim when the claim is not 'email'.
326330
@@ -341,6 +345,11 @@ spec:
341345
- message: precisely one of claim or expression must be
342346
set
343347
rule: 'has(self.claim) ? !has(self.expression) : has(self.expression)'
348+
- message: prefixPolicy must not be set to 'Prefix' when
349+
expression is set
350+
rule: 'has(self.expression) && size(self.expression) >
351+
0 ? !has(self.prefixPolicy) || self.prefixPolicy !=
352+
''Prefix'' : true'
344353
- message: prefix must be set if prefixPolicy is 'Prefix',
345354
but must remain unset otherwise
346355
rule: 'has(self.prefixPolicy) && self.prefixPolicy ==

‎config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-OKD.crd.yaml‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ spec:
199199
description: |-
200200
prefix is an optional field that configures the prefix that will be applied to the cluster identity attribute during the process of mapping JWT claims to cluster identity attributes.
201201
202-
When omitted (""), no prefix is applied to the cluster identity attribute.
202+
When omitted or set to an empty string (""), no prefix is applied to the cluster identity attribute.
203+
Must not be set to a non-empty value when expression is set.
203204
204205
Example: if `prefix` is set to "myoidc:" and the `claim` in JWT contains an array of strings "a", "b" and "c", the mapping will result in an array of string "myoidc:a", "myoidc:b" and "myoidc:c".
205206
type: string
@@ -288,11 +289,9 @@ spec:
288289
Allowed values are 'Prefix', 'NoPrefix', and omitted (not provided or an empty string).
289290
290291
When set to 'Prefix', the value specified in the prefix field will be prepended to the value of the JWT claim.
291-
292292
The prefix field must be set when prefixPolicy is 'Prefix'.
293-
293+
Must not be set to 'Prefix' when expression is set.
294294
When set to 'NoPrefix', no prefix will be prepended to the value of the JWT claim.
295-
296295
When omitted, this means no opinion and the platform is left to choose any prefixes that are applied which is subject to change over time.
297296
Currently, the platform prepends `{issuerURL}#` to the value of the JWT claim when the claim is not 'email'.
298297

‎config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-TechPreviewNoUpgrade.crd.yaml‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,18 @@ spec:
212212
description: |-
213213
prefix is an optional field that configures the prefix that will be applied to the cluster identity attribute during the process of mapping JWT claims to cluster identity attributes.
214214
215-
When omitted (""), no prefix is applied to the cluster identity attribute.
215+
When omitted or set to an empty string (""), no prefix is applied to the cluster identity attribute.
216+
Must not be set to a non-empty value when expression is set.
216217
217218
Example: if `prefix` is set to "myoidc:" and the `claim` in JWT contains an array of strings "a", "b" and "c", the mapping will result in an array of string "myoidc:a", "myoidc:b" and "myoidc:c".
218219
type: string
219220
type: object
220221
x-kubernetes-validations:
222+
- message: prefix must not be set to a non-empty value when
223+
expression is set
224+
rule: 'has(self.expression) && size(self.expression) >
225+
0 ? (!has(self.prefix) || size(self.prefix) == 0) :
226+
true'
221227
- message: expression must not be set if claim is specified
222228
and is not an empty string
223229
rule: '(size(self.?claim.orValue("")) > 0) ? !has(self.expression)
@@ -316,11 +322,9 @@ spec:
316322
Allowed values are 'Prefix', 'NoPrefix', and omitted (not provided or an empty string).
317323
318324
When set to 'Prefix', the value specified in the prefix field will be prepended to the value of the JWT claim.
319-
320325
The prefix field must be set when prefixPolicy is 'Prefix'.
321-
326+
Must not be set to 'Prefix' when expression is set.
322327
When set to 'NoPrefix', no prefix will be prepended to the value of the JWT claim.
323-
324328
When omitted, this means no opinion and the platform is left to choose any prefixes that are applied which is subject to change over time.
325329
Currently, the platform prepends `{issuerURL}#` to the value of the JWT claim when the claim is not 'email'.
326330
@@ -341,6 +345,11 @@ spec:
341345
- message: precisely one of claim or expression must be
342346
set
343347
rule: 'has(self.claim) ? !has(self.expression) : has(self.expression)'
348+
- message: prefixPolicy must not be set to 'Prefix' when
349+
expression is set
350+
rule: 'has(self.expression) && size(self.expression) >
351+
0 ? !has(self.prefixPolicy) || self.prefixPolicy !=
352+
''Prefix'' : true'
344353
- message: prefix must be set if prefixPolicy is 'Prefix',
345354
but must remain unset otherwise
346355
rule: 'has(self.prefixPolicy) && self.prefixPolicy ==

0 commit comments

Comments
 (0)