Validate transforms examples in federation_domain_watcher.go

Also changes the transformation pipeline code to sort and uniq
the transformed group names at the end of the pipeline. This makes
the results more predicable without changing the semantics.
This commit is contained in:
Ryan Richard
2023-07-14 16:50:43 -07:00
parent 52925a2a46
commit c771328bb1
7 changed files with 427 additions and 82 deletions

View File

@@ -45,6 +45,7 @@ const (
typeIdentityProvidersObjectRefKindValid = "IdentityProvidersObjectRefKindValid"
typeTransformsConstantsNamesUnique = "TransformsConstantsNamesUnique"
typeTransformsExpressionsValid = "TransformsExpressionsValid"
typeTransformsExamplesPassed = "TransformsExamplesPassed"
reasonSuccess = "Success"
reasonNotReady = "NotReady"
@@ -61,6 +62,7 @@ const (
reasonKindUnrecognized = "KindUnrecognized"
reasonDuplicateConstantsNames = "DuplicateConstantsNames"
reasonInvalidTransformsExpressions = "InvalidTransformsExpressions"
reasonTransformsExamplesFailed = "TransformsExamplesFailed"
kindLDAPIdentityProvider = "LDAPIdentityProvider"
kindOIDCIdentityProvider = "OIDCIdentityProvider"
@@ -328,10 +330,12 @@ func (c *federationDomainWatcherController) makeLegacyFederationDomainIssuer(
conditions = appendIdentityProviderObjectRefKindCondition(c.sortedAllowedKinds(), []string{}, conditions)
conditions = appendTransformsConstantsNamesUniqueCondition(sets.Set[string]{}, conditions)
conditions = appendTransformsExpressionsValidCondition([]string{}, conditions)
conditions = appendTransformsExamplesPassedCondition([]string{}, conditions)
return federationDomainIssuer, conditions, nil
}
//nolint:funlen
func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplicitIDPs(
ctx context.Context,
federationDomain *configv1alpha1.FederationDomain,
@@ -490,7 +494,7 @@ func (c *federationDomainWatcherController) makeTransformationPipelineAndEvaluat
return nil, false, nil, err
}
allExamplesPassed, conditions := c.evaluateExamples(ctx, idp, pipeline, conditions)
allExamplesPassed, conditions := c.evaluateExamples(ctx, idp, idpIndex, pipeline, conditions)
return pipeline, allExamplesPassed, conditions, nil
}
@@ -580,83 +584,76 @@ func (c *federationDomainWatcherController) makeTransformationPipeline(
func (c *federationDomainWatcherController) evaluateExamples(
ctx context.Context,
idp configv1alpha1.FederationDomainIdentityProvider,
idpIndex int,
pipeline *idtransform.TransformationPipeline,
conditions []*configv1alpha1.Condition,
) (bool, []*configv1alpha1.Condition) {
const errorFmt = ".spec.identityProviders[%d].transforms.examples[%d] example failed:\nexpected: %s\nactual: %s"
examplesErrors := []string{}
if pipeline == nil {
// TODO cannot evaluate examples, but still need to write a condition for it
// Unable to evaluate the conditions where the pipeline of expressions was invalid.
conditions = appendTransformsExamplesPassedCondition(nil, conditions)
return false, conditions
}
// Run all the provided transform examples. If any fail, put errors on the FederationDomain status.
examplesErrors := []string{}
for examplesIndex, e := range idp.Transforms.Examples {
result, _ := pipeline.Evaluate(ctx, e.Username, e.Groups)
// TODO: handle err
for exIndex, e := range idp.Transforms.Examples {
result, err := pipeline.Evaluate(ctx, e.Username, e.Groups)
if err != nil {
examplesErrors = append(examplesErrors, fmt.Sprintf(errorFmt, idpIndex, exIndex,
"no transformation errors",
fmt.Sprintf("transformations resulted in an unexpected error %q", err.Error())))
continue
}
resultWasAuthRejected := !result.AuthenticationAllowed
if e.Expects.Rejected && !resultWasAuthRejected { //nolint:gocritic,nestif
// TODO: handle this failed example
examplesErrors = append(examplesErrors, "TODO")
plog.Warning("FederationDomain identity provider transformations example failed: expected authentication to be rejected but it was not",
"idpDisplayName", idp.DisplayName,
"exampleIndex", examplesIndex,
"expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message,
"actualMessageResult", result.RejectedAuthenticationMessage,
)
} else if !e.Expects.Rejected && resultWasAuthRejected {
// TODO: handle this failed example
examplesErrors = append(examplesErrors, "TODO")
plog.Warning("FederationDomain identity provider transformations example failed: expected authentication not to be rejected but it was rejected",
"idpDisplayName", idp.DisplayName,
"exampleIndex", examplesIndex,
"expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message,
"actualMessageResult", result.RejectedAuthenticationMessage,
)
} else if e.Expects.Rejected && resultWasAuthRejected && e.Expects.Message != result.RejectedAuthenticationMessage {
// TODO: when expected message is blank, then treat it like it expects the default message
// TODO: handle this failed example
examplesErrors = append(examplesErrors, "TODO")
plog.Warning("FederationDomain identity provider transformations example failed: expected a different authentication rejection message",
"idpDisplayName", idp.DisplayName,
"exampleIndex", examplesIndex,
"expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message,
"actualMessageResult", result.RejectedAuthenticationMessage,
)
} else if result.AuthenticationAllowed {
if e.Expects.Rejected && !resultWasAuthRejected {
examplesErrors = append(examplesErrors,
fmt.Sprintf(errorFmt, idpIndex, exIndex, "authentication to be rejected", "authentication was not rejected"))
continue
}
if !e.Expects.Rejected && resultWasAuthRejected {
examplesErrors = append(examplesErrors, fmt.Sprintf(errorFmt, idpIndex, exIndex,
"authentication not to be rejected",
fmt.Sprintf("authentication was rejected with message %q", result.RejectedAuthenticationMessage)))
continue
}
expectedRejectionMessage := e.Expects.Message
if len(expectedRejectionMessage) == 0 {
expectedRejectionMessage = celtransformer.DefaultPolicyRejectedAuthMessage
}
if e.Expects.Rejected && resultWasAuthRejected && expectedRejectionMessage != result.RejectedAuthenticationMessage {
examplesErrors = append(examplesErrors, fmt.Sprintf(errorFmt, idpIndex, exIndex,
fmt.Sprintf("authentication rejection message %q", expectedRejectionMessage),
fmt.Sprintf("authentication rejection message %q", result.RejectedAuthenticationMessage)))
continue
}
if result.AuthenticationAllowed {
// In the case where the user expected the auth to be allowed and it was allowed, then compare
// the expected username and group names to the actual username and group names.
// TODO: when both of these fail, put both errors onto the status (not just the first one)
if e.Expects.Username != result.Username {
// TODO: handle this failed example
examplesErrors = append(examplesErrors, "TODO")
plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed username",
"idpDisplayName", idp.DisplayName,
"exampleIndex", examplesIndex,
"expectedUsername", e.Expects.Username,
"actualUsernameResult", result.Username,
)
examplesErrors = append(examplesErrors, fmt.Sprintf(errorFmt, idpIndex, exIndex,
fmt.Sprintf("username %q", e.Expects.Username),
fmt.Sprintf("username %q", result.Username)))
}
if !stringSlicesEqual(e.Expects.Groups, result.Groups) {
// TODO: Do we need to make this insensitive to ordering, or should the transformations evaluator be changed to always return sorted group names at the end of the pipeline?
// TODO: What happens if the user did not write any group expectation? Treat it like expecting an empty list of groups?
// TODO: handle this failed example
examplesErrors = append(examplesErrors, "TODO")
plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed groups list",
"idpDisplayName", idp.DisplayName,
"exampleIndex", examplesIndex,
"expectedGroups", e.Expects.Groups,
"actualGroupsResult", result.Groups,
)
expectedGroups := e.Expects.Groups
if expectedGroups == nil {
expectedGroups = []string{}
}
if !stringSetsEqual(expectedGroups, result.Groups) {
examplesErrors = append(examplesErrors, fmt.Sprintf(errorFmt, idpIndex, exIndex,
fmt.Sprintf("groups [%s]", strings.Join(sortAndQuote(expectedGroups), ", ")),
fmt.Sprintf("groups [%s]", strings.Join(sortAndQuote(result.Groups), ", "))))
}
}
}
conditions = appendTransformsExamplesPassedCondition(examplesErrors, conditions)
return len(examplesErrors) == 0, conditions
}
@@ -724,6 +721,34 @@ func appendTransformsExpressionsValidCondition(errors []string, conditions []*co
return conditions
}
func appendTransformsExamplesPassedCondition(errors []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
switch {
case errors == nil:
conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsExamplesPassed,
Status: configv1alpha1.ConditionUnknown,
Reason: reasonUnableToValidate,
Message: "unable to check if the examples specified by .spec.identityProviders[].transforms.examples[] had errors because an expression was invalid",
})
case len(errors) > 0:
conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsExamplesPassed,
Status: configv1alpha1.ConditionFalse,
Reason: reasonTransformsExamplesFailed,
Message: fmt.Sprintf("the examples specified by .spec.identityProviders[].transforms.examples[] had errors:\n\n%s",
strings.Join(errors, "\n\n")),
})
default:
conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsExamplesPassed,
Status: configv1alpha1.ConditionTrue,
Reason: reasonSuccess,
Message: "the examples specified by .spec.identityProviders[].transforms.examples[] had no errors",
})
}
return conditions
}
func appendIdentityProviderDuplicateDisplayNamesCondition(duplicateDisplayNames sets.Set[string], conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
if duplicateDisplayNames.Len() > 0 {
conditions = append(conditions, &configv1alpha1.Condition{
@@ -948,14 +973,8 @@ func hadErrorCondition(conditions []*configv1alpha1.Condition) bool {
return false
}
func stringSlicesEqual(a []string, b []string) bool {
if len(a) != len(b) {
return false
}
for i, itemFromA := range a {
if b[i] != itemFromA {
return false
}
}
return true
func stringSetsEqual(a []string, b []string) bool {
aSet := sets.New(a...)
bSet := sets.New(b...)
return aSet.Equal(bSet)
}

View File

@@ -442,6 +442,39 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
}
}
happyTransformationExamplesCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "TransformsExamplesPassed",
Status: "True",
ObservedGeneration: observedGeneration,
LastTransitionTime: time,
Reason: "Success",
Message: "the examples specified by .spec.identityProviders[].transforms.examples[] had no errors",
}
}
sadTransformationExamplesCondition := func(errorMessages string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "TransformsExamplesPassed",
Status: "False",
ObservedGeneration: observedGeneration,
LastTransitionTime: time,
Reason: "TransformsExamplesFailed",
Message: fmt.Sprintf("the examples specified by .spec.identityProviders[].transforms.examples[] had errors:\n\n%s", errorMessages),
}
}
unknownTransformationExamplesCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "TransformsExamplesPassed",
Status: "Unknown",
ObservedGeneration: observedGeneration,
LastTransitionTime: time,
Reason: "UnableToValidate",
Message: "unable to check if the examples specified by .spec.identityProviders[].transforms.examples[] had errors because an expression was invalid",
}
}
happyAPIGroupSuffixCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "IdentityProvidersObjectRefAPIGroupSuffixValid",
@@ -511,6 +544,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
allHappyConditionsSuccess := func(issuer string, time metav1.Time, observedGeneration int64) []configv1alpha1.Condition {
return sortConditionsByType([]configv1alpha1.Condition{
happyTransformationExamplesCondition(frozenMetav1Now, 123),
happyTransformationExpressionsCondition(frozenMetav1Now, 123),
happyConstNamesUniqueCondition(frozenMetav1Now, 123),
happyKindCondition(frozenMetav1Now, 123),
@@ -1330,7 +1364,248 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
spec.identityProvider[0].transforms.expressions[3].expression was invalid:
CEL expression compile error: ERROR: <input>:1:7: Syntax error: mismatched input 'not' expecting <EOF>
| still not a valid cel expression
| ......^`),
| ......^`,
),
frozenMetav1Now, 123),
unknownTransformationExamplesCondition(frozenMetav1Now, 123),
sadReadyCondition(frozenMetav1Now, 123),
}),
),
},
},
{
name: "the federation domain has transformation examples which don't pass",
inputObjects: []runtime.Object{
oidcIdentityProvider,
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
Spec: configv1alpha1.FederationDomainSpec{
Issuer: "https://issuer1.com",
IdentityProviders: []configv1alpha1.FederationDomainIdentityProvider{
{
DisplayName: "name1",
ObjectRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String(apiGroupSupervisor),
Kind: "OIDCIdentityProvider",
Name: oidcIdentityProvider.Name,
},
Transforms: configv1alpha1.FederationDomainTransforms{
Expressions: []configv1alpha1.FederationDomainTransformsExpression{
{Type: "policy/v1", Expression: `username == "ryan" || username == "rejectMeWithDefaultMessage"`, Message: "only ryan allowed"},
{Type: "policy/v1", Expression: `username != "rejectMeWithDefaultMessage"`}, // no message specified
{Type: "username/v1", Expression: `"pre:" + username`},
{Type: "groups/v1", Expression: `groups.map(g, "pre:" + g)`},
},
Examples: []configv1alpha1.FederationDomainTransformsExample{
{ // this example should pass
Username: "ryan",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Username: "pre:ryan",
Groups: []string{"pre:b", "pre:a", "pre:b", "pre:a"}, // order and repeats don't matter, treated like a set
Rejected: false,
},
},
{ // this example should pass
Username: "other",
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Rejected: true,
Message: "only ryan allowed",
},
},
{ // this example should fail because it expects the user to be rejected but the user was actually not rejected
Username: "ryan",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Rejected: true,
Message: "this input is ignored in this case",
},
},
{ // this example should fail because it expects the user not to be rejected but they were actually rejected
Username: "other",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Username: "pre:other",
Groups: []string{"pre:a", "pre:b"},
Rejected: false,
},
},
{ // this example should fail because it expects the wrong rejection message
Username: "other",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Rejected: true,
Message: "wrong message",
},
},
{ // this example should pass even though it does not make any assertion about the rejection message
// because the message assertions defaults to asserting the default rejection message
Username: "rejectMeWithDefaultMessage",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Rejected: true,
},
},
{ // this example should fail because it expects both the wrong username and groups
Username: "ryan",
Groups: []string{"b", "a"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Username: "wrong",
Groups: []string{},
Rejected: false,
},
},
{ // this example should fail because it expects the wrong username only
Username: "ryan",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Username: "wrong",
Groups: []string{"pre:b", "pre:a"},
Rejected: false,
},
},
{ // this example should fail because it expects the wrong groups only
Username: "ryan",
Groups: []string{"b", "a"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Username: "pre:ryan",
Groups: []string{"wrong2", "wrong1"},
Rejected: false,
},
},
{ // this example should fail because it does not expect anything but the auth actually was successful
Username: "ryan",
Groups: []string{"b", "a"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{},
},
},
},
},
},
},
},
},
wantFDIssuers: []*federationdomainproviders.FederationDomainIssuer{},
wantStatusUpdates: []*configv1alpha1.FederationDomain{
expectedFederationDomainStatusUpdate(
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
},
configv1alpha1.FederationDomainPhaseError,
replaceConditions(
allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123),
[]configv1alpha1.Condition{
sadTransformationExamplesCondition(
here.Doc(
`.spec.identityProviders[0].transforms.examples[2] example failed:
expected: authentication to be rejected
actual: authentication was not rejected
.spec.identityProviders[0].transforms.examples[3] example failed:
expected: authentication not to be rejected
actual: authentication was rejected with message "only ryan allowed"
.spec.identityProviders[0].transforms.examples[4] example failed:
expected: authentication rejection message "wrong message"
actual: authentication rejection message "only ryan allowed"
.spec.identityProviders[0].transforms.examples[6] example failed:
expected: username "wrong"
actual: username "pre:ryan"
.spec.identityProviders[0].transforms.examples[6] example failed:
expected: groups []
actual: groups ["pre:a", "pre:b"]
.spec.identityProviders[0].transforms.examples[7] example failed:
expected: username "wrong"
actual: username "pre:ryan"
.spec.identityProviders[0].transforms.examples[8] example failed:
expected: groups ["wrong1", "wrong2"]
actual: groups ["pre:a", "pre:b"]
.spec.identityProviders[0].transforms.examples[9] example failed:
expected: username ""
actual: username "pre:ryan"
.spec.identityProviders[0].transforms.examples[9] example failed:
expected: groups []
actual: groups ["pre:a", "pre:b"]`,
),
frozenMetav1Now, 123),
sadReadyCondition(frozenMetav1Now, 123),
}),
),
},
},
{
name: "the federation domain has transformation expressions that return illegal values with examples which exercise them",
inputObjects: []runtime.Object{
oidcIdentityProvider,
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
Spec: configv1alpha1.FederationDomainSpec{
Issuer: "https://issuer1.com",
IdentityProviders: []configv1alpha1.FederationDomainIdentityProvider{
{
DisplayName: "name1",
ObjectRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String(apiGroupSupervisor),
Kind: "OIDCIdentityProvider",
Name: oidcIdentityProvider.Name,
},
Transforms: configv1alpha1.FederationDomainTransforms{
Expressions: []configv1alpha1.FederationDomainTransformsExpression{
{Type: "username/v1", Expression: `username == "ryan" ? "" : username`}, // not allowed to return an empty string as the transformed username
},
Examples: []configv1alpha1.FederationDomainTransformsExample{
{ // every example which encounters an unexpected error should fail because the transformation pipeline returned an error
Username: "ryan",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{},
},
{ // every example which encounters an unexpected error should fail because the transformation pipeline returned an error
Username: "ryan",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{},
},
{ // this should pass
Username: "other",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Username: "other",
Groups: []string{"a", "b"},
Rejected: false,
},
},
},
},
},
},
},
},
},
wantFDIssuers: []*federationdomainproviders.FederationDomainIssuer{},
wantStatusUpdates: []*configv1alpha1.FederationDomain{
expectedFederationDomainStatusUpdate(
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
},
configv1alpha1.FederationDomainPhaseError,
replaceConditions(
allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123),
[]configv1alpha1.Condition{
sadTransformationExamplesCondition(
here.Doc(
`.spec.identityProviders[0].transforms.examples[0] example failed:
expected: no transformation errors
actual: transformations resulted in an unexpected error "identity transformation returned an empty username, which is not allowed"
.spec.identityProviders[0].transforms.examples[1] example failed:
expected: no transformation errors
actual: transformations resulted in an unexpected error "identity transformation returned an empty username, which is not allowed"`,
),
frozenMetav1Now, 123),
sadReadyCondition(frozenMetav1Now, 123),
}),