From 83bfe4fe227a8c7e156a85fa28d6f29fa941cfca Mon Sep 17 00:00:00 2001 From: Charel Baum Date: Wed, 19 Oct 2022 11:26:52 +0200 Subject: [PATCH] fix(cognito/userpool)!: MFA not always working -add field SoftwareTokenMFAConfiguration -remove deprecated UnusedAccountValidityDays -enhance several controller parts Signed-off-by: Charel Baum --- .../generator-config.yaml | 6 + .../v1alpha1/zz_generated.deepcopy.go | 10 +- .../v1alpha1/zz_types.go | 2 - .../v1alpha1/zz_user_pool.go | 2 + examples/cognito/userpool.yaml | 29 ++ ...yprovider.aws.crossplane.io_userpools.yaml | 9 +- .../cognitoidentityprovider/fake/fake.go | 70 +++++ .../cognitoidentityprovider/userpool/setup.go | 290 +++++++++++++++--- .../userpool/setup_test.go | 120 +++++--- .../userpool/zz_controller.go | 3 - .../userpool/zz_conversions.go | 9 - 11 files changed, 443 insertions(+), 107 deletions(-) create mode 100644 pkg/clients/cognitoidentityprovider/fake/fake.go diff --git a/apis/cognitoidentityprovider/generator-config.yaml b/apis/cognitoidentityprovider/generator-config.yaml index 4d3e187e60..03888a441c 100644 --- a/apis/cognitoidentityprovider/generator-config.yaml +++ b/apis/cognitoidentityprovider/generator-config.yaml @@ -13,8 +13,14 @@ ignore: - CreateIdentityProviderInput.UserPoolId - CreateIdentityProviderInput.ProviderDetails - CreateIdentityProviderOutput.IdentityProvider.ProviderDetails + - CreateUserPoolInput.AdminCreateUserConfig.UnusedAccountValidityDays resources: UserPool: + fields: + SoftwareTokenMfaConfiguration: + from: + operation: SetUserPoolMfaConfig + path: SoftwareTokenMfaConfiguration exceptions: errors: # In the API this is a 400 error, but we have to define a 404 error here, diff --git a/apis/cognitoidentityprovider/v1alpha1/zz_generated.deepcopy.go b/apis/cognitoidentityprovider/v1alpha1/zz_generated.deepcopy.go index 933979d3bd..64bdbc5749 100644 --- a/apis/cognitoidentityprovider/v1alpha1/zz_generated.deepcopy.go +++ b/apis/cognitoidentityprovider/v1alpha1/zz_generated.deepcopy.go @@ -65,11 +65,6 @@ func (in *AdminCreateUserConfigType) DeepCopyInto(out *AdminCreateUserConfigType *out = new(MessageTemplateType) (*in).DeepCopyInto(*out) } - if in.UnusedAccountValidityDays != nil { - in, out := &in.UnusedAccountValidityDays, &out.UnusedAccountValidityDays - *out = new(int64) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AdminCreateUserConfigType. @@ -3035,6 +3030,11 @@ func (in *UserPoolParameters) DeepCopyInto(out *UserPoolParameters) { *out = new(string) **out = **in } + if in.SoftwareTokenMFAConfiguration != nil { + in, out := &in.SoftwareTokenMFAConfiguration, &out.SoftwareTokenMFAConfiguration + *out = new(SoftwareTokenMFAConfigType) + (*in).DeepCopyInto(*out) + } if in.UserPoolAddOns != nil { in, out := &in.UserPoolAddOns, &out.UserPoolAddOns *out = new(UserPoolAddOnsType) diff --git a/apis/cognitoidentityprovider/v1alpha1/zz_types.go b/apis/cognitoidentityprovider/v1alpha1/zz_types.go index 2d166d4712..4b6d4281f3 100644 --- a/apis/cognitoidentityprovider/v1alpha1/zz_types.go +++ b/apis/cognitoidentityprovider/v1alpha1/zz_types.go @@ -37,8 +37,6 @@ type AdminCreateUserConfigType struct { AllowAdminCreateUserOnly *bool `json:"allowAdminCreateUserOnly,omitempty"` // The message template structure. InviteMessageTemplate *MessageTemplateType `json:"inviteMessageTemplate,omitempty"` - - UnusedAccountValidityDays *int64 `json:"unusedAccountValidityDays,omitempty"` } // +kubebuilder:skipversion diff --git a/apis/cognitoidentityprovider/v1alpha1/zz_user_pool.go b/apis/cognitoidentityprovider/v1alpha1/zz_user_pool.go index 54c148f41d..4a3a6c7de5 100644 --- a/apis/cognitoidentityprovider/v1alpha1/zz_user_pool.go +++ b/apis/cognitoidentityprovider/v1alpha1/zz_user_pool.go @@ -90,6 +90,8 @@ type UserPoolParameters struct { SmsConfiguration *SmsConfigurationType `json:"smsConfiguration,omitempty"` // A string representing the SMS verification message. SmsVerificationMessage *string `json:"smsVerificationMessage,omitempty"` + // The software token MFA configuration. + SoftwareTokenMFAConfiguration *SoftwareTokenMFAConfigType `json:"softwareTokenMFAConfiguration,omitempty"` // Enables advanced security risk detection. Set the key AdvancedSecurityMode // to the value "AUDIT". UserPoolAddOns *UserPoolAddOnsType `json:"userPoolAddOns,omitempty"` diff --git a/examples/cognito/userpool.yaml b/examples/cognito/userpool.yaml index b81c3fa795..a2541b30fc 100644 --- a/examples/cognito/userpool.yaml +++ b/examples/cognito/userpool.yaml @@ -6,5 +6,34 @@ spec: forProvider: region: us-east-1 poolName: examplePool + # "After you create a user pool, you can't change or remove sign-in options.": + # concerns aliasAttributes, usernameAttributes, UsernameConfiguration + # "Only one of the aliasAttributes or usernameAttributes can be set in a User pool." + # if aliasAttributes used, seems to add these additionally to username as sign-in options. + # In contrast using usernameAttributes seems to replace username as sign-in option! + aliasAttributes: + - email + - phone_number + - preferred_username + usernameConfiguration: # default is true + caseSensitive: false + adminCreateUserConfig: + allowAdminCreateUserOnly: true + inviteMessageTemplate: + emailMessage: | + Your username is: +
{username}
+
and your temporary password is: +
{####}
+ # ensure that for email subject line a single-line string is provided + emailSubject: "Your temporary password" + mfaConfiguration: 'ON' + # to make MFA usable, either softwareTokenMFAConfiguration + # and/or smsConfiguration (with externalID) needs to be provided + softwareTokenMFAConfiguration: + enabled: true + userPoolTags: + pool: big + users: best providerConfigRef: name: example diff --git a/package/crds/cognitoidentityprovider.aws.crossplane.io_userpools.yaml b/package/crds/cognitoidentityprovider.aws.crossplane.io_userpools.yaml index 2d027d8512..f17f6e72a3 100644 --- a/package/crds/cognitoidentityprovider.aws.crossplane.io_userpools.yaml +++ b/package/crds/cognitoidentityprovider.aws.crossplane.io_userpools.yaml @@ -98,9 +98,6 @@ spec: sMSMessage: type: string type: object - unusedAccountValidityDays: - format: int64 - type: integer type: object aliasAttributes: description: 'Attributes supported as an alias for this user pool. @@ -287,6 +284,12 @@ spec: smsVerificationMessage: description: A string representing the SMS verification message. type: string + softwareTokenMFAConfiguration: + description: The software token MFA configuration. + properties: + enabled: + type: boolean + type: object userPoolAddOns: description: Enables advanced security risk detection. Set the key AdvancedSecurityMode to the value "AUDIT". diff --git a/pkg/clients/cognitoidentityprovider/fake/fake.go b/pkg/clients/cognitoidentityprovider/fake/fake.go new file mode 100644 index 0000000000..f058adece6 --- /dev/null +++ b/pkg/clients/cognitoidentityprovider/fake/fake.go @@ -0,0 +1,70 @@ +/* +Copyright 2021 The Crossplane 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 fake + +import ( + "context" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/request" + "github.com/aws/aws-sdk-go/service/cognitoidentityprovider" + "github.com/aws/aws-sdk-go/service/cognitoidentityprovider/cognitoidentityprovideriface" +) + +// MockCognitoIdentityProviderClient for testing +type MockCognitoIdentityProviderClient struct { + cognitoidentityprovideriface.CognitoIdentityProviderAPI + + MockGetUserPoolMfaConfig func(*cognitoidentityprovider.GetUserPoolMfaConfigInput) (*cognitoidentityprovider.GetUserPoolMfaConfigOutput, error) + MockSetUserPoolMfaConfigWithContext func(context.Context, *cognitoidentityprovider.SetUserPoolMfaConfigInput, []request.Option) (*cognitoidentityprovider.SetUserPoolMfaConfigOutput, error) + + Called MockCognitoIdentityProviderClientCall +} + +// CallGetUserPoolMfaConfig to log call +type CallGetUserPoolMfaConfig struct { + Ctx aws.Context + I *cognitoidentityprovider.GetUserPoolMfaConfigInput + Opts []request.Option +} + +// GetUserPoolMfaConfig calls MockGetUserPoolMfaConfig +func (m *MockCognitoIdentityProviderClient) GetUserPoolMfaConfig(i *cognitoidentityprovider.GetUserPoolMfaConfigInput) (*cognitoidentityprovider.GetUserPoolMfaConfigOutput, error) { + m.Called.GetUserPoolMfaConfig = append(m.Called.GetUserPoolMfaConfig, &CallGetUserPoolMfaConfig{I: i}) + + return m.MockGetUserPoolMfaConfig(i) +} + +// CallSetUserPoolMfaConfigWithContext to log call +type CallSetUserPoolMfaConfigWithContext struct { + Ctx aws.Context + I *cognitoidentityprovider.SetUserPoolMfaConfigInput + Opts []request.Option +} + +// SetUserPoolMfaConfigWithContext calls MockSetUserPoolMfaConfigWithContext +func (m *MockCognitoIdentityProviderClient) SetUserPoolMfaConfigWithContext(ctx context.Context, i *cognitoidentityprovider.SetUserPoolMfaConfigInput, opts ...request.Option) (*cognitoidentityprovider.SetUserPoolMfaConfigOutput, error) { + m.Called.SetUserPoolMfaConfigWithContext = append(m.Called.SetUserPoolMfaConfigWithContext, &CallSetUserPoolMfaConfigWithContext{Ctx: ctx, I: i, Opts: opts}) + + return m.MockSetUserPoolMfaConfigWithContext(ctx, i, opts) +} + +// MockCognitoIdentityProviderClientCall to log calls +type MockCognitoIdentityProviderClientCall struct { + GetUserPoolMfaConfig []*CallGetUserPoolMfaConfig + SetUserPoolMfaConfigWithContext []*CallSetUserPoolMfaConfigWithContext +} diff --git a/pkg/controller/cognitoidentityprovider/userpool/setup.go b/pkg/controller/cognitoidentityprovider/userpool/setup.go index 7ba66ce83b..a315a095ac 100644 --- a/pkg/controller/cognitoidentityprovider/userpool/setup.go +++ b/pkg/controller/cognitoidentityprovider/userpool/setup.go @@ -18,9 +18,11 @@ import ( "reflect" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/pkg/errors" ctrl "sigs.k8s.io/controller-runtime" svcsdk "github.com/aws/aws-sdk-go/service/cognitoidentityprovider" + svcsdkapi "github.com/aws/aws-sdk-go/service/cognitoidentityprovider/cognitoidentityprovideriface" "github.com/crossplane/crossplane-runtime/pkg/connection" "github.com/crossplane/crossplane-runtime/pkg/controller" "github.com/crossplane/crossplane-runtime/pkg/event" @@ -34,18 +36,27 @@ import ( "github.com/crossplane-contrib/provider-aws/pkg/features" ) +const ( + errMissingMFAStuff = "no SoftwareTokenMfaConfiguration or SmsConfiguration given, unable to make MFA ON/OPTIONAL" + errFailedGetMFARequest = "failed GetUserPoolMfaConfig request. Could not check MFAConfiguration isUptoDate-state" + errFailedSetMFARequest = "failed SetUserPoolMfaConfig request. Could not update UserPool" + errConflictingFields = "fields conflicting! Please only use one of them or both with the same value" +) + // SetupUserPool adds a controller that reconciles UserPool. func SetupUserPool(mgr ctrl.Manager, o controller.Options) error { name := managed.ControllerName(svcapitypes.UserPoolGroupKind) opts := []option{ func(e *external) { + h := &hooks{client: e.client} e.preObserve = preObserve e.postObserve = postObserve - e.preUpdate = preUpdate + e.preUpdate = h.preUpdate e.preDelete = preDelete + e.preCreate = preCreate e.postCreate = postCreate - e.isUpToDate = isUpToDate + e.isUpToDate = h.isUpToDate e.lateInitialize = lateInitialize }, } @@ -69,6 +80,10 @@ func SetupUserPool(mgr ctrl.Manager, o controller.Options) error { managed.WithConnectionPublishers(cps...))) } +type hooks struct { + client svcsdkapi.CognitoIdentityProviderAPI +} + func preObserve(_ context.Context, cr *svcapitypes.UserPool, obj *svcsdk.DescribeUserPoolInput) error { if meta.GetExternalName(cr) != "" { obj.UserPoolId = awsclients.String(meta.GetExternalName(cr)) @@ -86,9 +101,13 @@ func postObserve(_ context.Context, cr *svcapitypes.UserPool, obj *svcsdk.Descri return obs, nil } -func preUpdate(_ context.Context, cr *svcapitypes.UserPool, obj *svcsdk.UpdateUserPoolInput) error { +func (e *hooks) preUpdate(ctx context.Context, cr *svcapitypes.UserPool, obj *svcsdk.UpdateUserPoolInput) error { obj.UserPoolId = awsclients.String(meta.GetExternalName(cr)) - return nil + + // "Cannot turn MFA functionality ON, once the user pool has been created" + // -> concerns UpdateUserPool, not SetUserPoolMfaConfig + // therefore, before Update request, set MFA configuration + return e.setMfaConfiguration(ctx, cr) } func preDelete(_ context.Context, cr *svcapitypes.UserPool, obj *svcsdk.DeleteUserPoolInput) (bool, error) { @@ -96,45 +115,63 @@ func preDelete(_ context.Context, cr *svcapitypes.UserPool, obj *svcsdk.DeleteUs return false, nil } +func preCreate(_ context.Context, cr *svcapitypes.UserPool, obj *svcsdk.CreateUserPoolInput) error { + // for Creation need to set MFA to OFF, + // bc if MFA ON and no SmsConfiguration provided, AWS throws error + // in first Update, we can use SetUserPoolMfaConfig to set all MFA stuff (e.g. Token) + obj.MfaConfiguration = awsclients.String(svcsdk.UserPoolMfaTypeOff) + + return nil +} + func postCreate(_ context.Context, cr *svcapitypes.UserPool, obj *svcsdk.CreateUserPoolOutput, obs managed.ExternalCreation, err error) (managed.ExternalCreation, error) { if err != nil { return managed.ExternalCreation{}, err } - meta.SetExternalName(cr, awsclients.StringValue(obj.UserPool.Id)) + + // we cannot do a SetUserPoolMfaConfig-call here, but have to wait until first Update, + // bc in zz_controller.go/Create all cr.specs.forProvider are set to obj.Userpool values + // (->so no knowledge here of actual user input) return managed.ExternalCreation{ExternalNameAssigned: true}, nil } -func isUpToDate(cr *svcapitypes.UserPool, resp *svcsdk.DescribeUserPoolOutput) (bool, error) { +func (e *hooks) isUpToDate(cr *svcapitypes.UserPool, resp *svcsdk.DescribeUserPoolOutput) (bool, error) { pool := resp.UserPool + spec := cr.Spec.ForProvider switch { - case !areAccountRecoverySettingEqual(cr.Spec.ForProvider.AccountRecoverySetting, pool.AccountRecoverySetting), - !areAdminCreateUserConfigEqual(cr.Spec.ForProvider.AdminCreateUserConfig, pool.AdminCreateUserConfig), - !reflect.DeepEqual(cr.Spec.ForProvider.AliasAttributes, pool.AliasAttributes), - !reflect.DeepEqual(cr.Spec.ForProvider.AutoVerifiedAttributes, pool.AutoVerifiedAttributes), - !areDeviceConfigurationEqual(cr.Spec.ForProvider.DeviceConfiguration, pool.DeviceConfiguration), - !areEmailConfigurationEqual(cr.Spec.ForProvider.EmailConfiguration, pool.EmailConfiguration), - awsclients.StringValue(cr.Spec.ForProvider.EmailVerificationMessage) != awsclients.StringValue(pool.EmailVerificationMessage), - awsclients.StringValue(cr.Spec.ForProvider.EmailVerificationSubject) != awsclients.StringValue(pool.EmailVerificationSubject), - !areLambdaConfigEqual(cr.Spec.ForProvider.LambdaConfig, pool.LambdaConfig), - awsclients.StringValue(cr.Spec.ForProvider.MFAConfiguration) != awsclients.StringValue(pool.MfaConfiguration), - !arePoliciesEqual(cr.Spec.ForProvider.Policies, pool.Policies), - !areSchemaEqual(cr.Spec.ForProvider.Schema, pool.SchemaAttributes), - awsclients.StringValue(cr.Spec.ForProvider.SmsAuthenticationMessage) != awsclients.StringValue(pool.SmsAuthenticationMessage), - !areSmsConfigurationEqual(cr.Spec.ForProvider.SmsConfiguration, pool.SmsConfiguration), - awsclients.StringValue(cr.Spec.ForProvider.SmsVerificationMessage) != awsclients.StringValue(pool.SmsVerificationMessage), - !areUserPoolAddOnsEqual(cr.Spec.ForProvider.UserPoolAddOns, pool.UserPoolAddOns), - !reflect.DeepEqual(cr.Spec.ForProvider.UsernameAttributes, pool.UsernameAttributes), - !areUsernameConfigurationEqual(cr.Spec.ForProvider.UsernameConfiguration, pool.UsernameConfiguration), - !areVerificationMessageTemplateEqual(cr.Spec.ForProvider.VerificationMessageTemplate, pool.VerificationMessageTemplate): + case !areAccountRecoverySettingEqual(spec.AccountRecoverySetting, pool.AccountRecoverySetting), + !areAdminCreateUserConfigEqual(spec.AdminCreateUserConfig, pool.AdminCreateUserConfig), + !reflect.DeepEqual(spec.AutoVerifiedAttributes, pool.AutoVerifiedAttributes), + !areDeviceConfigurationEqual(spec.DeviceConfiguration, pool.DeviceConfiguration), + !areEmailConfigurationEqual(spec.EmailConfiguration, pool.EmailConfiguration), + !areLambdaConfigEqual(spec.LambdaConfig, pool.LambdaConfig), + !arePoliciesEqual(spec.Policies, pool.Policies), + !areSchemaEqual(spec.Schema, pool.SchemaAttributes), + awsclients.StringValue(spec.SmsAuthenticationMessage) != awsclients.StringValue(pool.SmsAuthenticationMessage), + !areSmsConfigurationEqual(spec.SmsConfiguration, pool.SmsConfiguration), + !areUserPoolAddOnsEqual(spec.UserPoolAddOns, pool.UserPoolAddOns), + !areVerificationMessageTemplateEqual(spec.VerificationMessageTemplate, pool.VerificationMessageTemplate), + !reflect.DeepEqual(spec.UserPoolTags, pool.UserPoolTags): return false, nil } - return true, nil + + // check the conflicting fields for isUpToDate + conflicts + fieldsUpToDate, err := conflictingFieldsEqual(spec, pool) + if err != nil || !fieldsUpToDate { + return false, err + } + + // check MFA stuff + return e.areMFAConfigEqual(cr) } func areAccountRecoverySettingEqual(spec *svcapitypes.AccountRecoverySettingType, current *svcsdk.AccountRecoverySettingType) bool { - if spec != nil && current != nil { + if spec != nil { + if current == nil { + return false + } if len(spec.RecoveryMechanisms) > 0 && len(spec.RecoveryMechanisms) != len(current.RecoveryMechanisms) { return false } @@ -147,7 +184,6 @@ func areAccountRecoverySettingEqual(spec *svcapitypes.AccountRecoverySettingType } } } - return true } @@ -155,8 +191,7 @@ func areAdminCreateUserConfigEqual(spec *svcapitypes.AdminCreateUserConfigType, if spec != nil && current != nil { switch { case awsclients.BoolValue(spec.AllowAdminCreateUserOnly) != awsclients.BoolValue(current.AllowAdminCreateUserOnly), - !areInviteMessageTemplateEqual(spec.InviteMessageTemplate, current.InviteMessageTemplate), - awsclients.Int64Value(spec.UnusedAccountValidityDays) != awsclients.Int64Value(current.UnusedAccountValidityDays): + !areInviteMessageTemplateEqual(spec.InviteMessageTemplate, current.InviteMessageTemplate): return false } } @@ -164,7 +199,10 @@ func areAdminCreateUserConfigEqual(spec *svcapitypes.AdminCreateUserConfigType, } func areInviteMessageTemplateEqual(spec *svcapitypes.MessageTemplateType, current *svcsdk.MessageTemplateType) bool { - if spec != nil && current != nil { + if spec != nil { + if current == nil { + return false + } switch { case awsclients.StringValue(spec.EmailMessage) != awsclients.StringValue(current.EmailMessage), awsclients.StringValue(spec.EmailSubject) != awsclients.StringValue(current.EmailSubject), @@ -176,7 +214,10 @@ func areInviteMessageTemplateEqual(spec *svcapitypes.MessageTemplateType, curren } func areDeviceConfigurationEqual(spec *svcapitypes.DeviceConfigurationType, current *svcsdk.DeviceConfigurationType) bool { - if spec != nil && current != nil { + if spec != nil { + if current == nil { + return false + } switch { case awsclients.BoolValue(spec.ChallengeRequiredOnNewDevice) != awsclients.BoolValue(current.ChallengeRequiredOnNewDevice), awsclients.BoolValue(spec.DeviceOnlyRememberedOnUserPrompt) != awsclients.BoolValue(current.DeviceOnlyRememberedOnUserPrompt): @@ -285,7 +326,10 @@ func areSchemaEqual(spec []*svcapitypes.SchemaAttributeType, current []*svcsdk.S } func areSmsConfigurationEqual(spec *svcapitypes.SmsConfigurationType, current *svcsdk.SmsConfigurationType) bool { - if spec != nil && current != nil { + if spec != nil { + if current == nil { + return false + } switch { case awsclients.StringValue(spec.ExternalID) != awsclients.StringValue(current.ExternalId), awsclients.StringValue(spec.SNSCallerARN) != awsclients.StringValue(current.SnsCallerArn): @@ -302,31 +346,193 @@ func areUserPoolAddOnsEqual(spec *svcapitypes.UserPoolAddOnsType, current *svcsd return true } -func areUsernameConfigurationEqual(spec *svcapitypes.UsernameConfigurationType, current *svcsdk.UsernameConfigurationType) bool { - if spec != nil && current != nil { - return awsclients.BoolValue(spec.CaseSensitive) == awsclients.BoolValue(current.CaseSensitive) +func conflictingFieldsEqual(params svcapitypes.UserPoolParameters, pool *svcsdk.UserPoolType) (bool, error) { + // conflicting fields, that require the user to + // either set them both with exactly the same value + // or to just provide one of them and leave the other on empty + + // should never be nil, bc set in lateInit, but just to be safe + if params.VerificationMessageTemplate != nil { + // check for conflicts and isUpTodates + fieldUpToDate, err := conflictingFieldsHelper(params.EmailVerificationMessage, params.VerificationMessageTemplate.EmailMessage, pool.EmailVerificationMessage) + if err != nil { + return true, errors.Wrap(err, "EmailVerificationMessage and verificationMessageTemplate.EmailMessage") + } + if !fieldUpToDate { + return false, nil + } + fieldUpToDate, err = conflictingFieldsHelper(params.EmailVerificationSubject, params.VerificationMessageTemplate.EmailSubject, pool.EmailVerificationSubject) + if err != nil { + return true, errors.Wrap(err, "EmailVerificationSubject and verificationMessageTemplate.EmailSubject") + } + if !fieldUpToDate { + return false, nil + } + fieldUpToDate, err = conflictingFieldsHelper(params.SmsVerificationMessage, params.VerificationMessageTemplate.SmsMessage, pool.SmsVerificationMessage) + if err != nil { + return true, errors.Wrap(err, "SmsVerificationMessage and verificationMessageTemplate.SmsMessage") + } + if !fieldUpToDate { + return false, nil + } } - return true + + return true, nil +} + +// conflictingFieldsHelper checks if 2 *string fields conflict and if their value isUpToDate +func conflictingFieldsHelper(field1 *string, field2 *string, fieldAWS *string) (bool, error) { + // both of them nil => all fine + if field1 == nil && field2 == nil { + return true, nil + } + if field1 != nil && field2 != nil { + if awsclients.StringValue(field1) != awsclients.StringValue(field2) { + // both of them non-nil and different => means conflict + return true, errors.New(errConflictingFields) + } + // both of them non-nil, but same => check if value isUpToDate + if awsclients.StringValue(field1) != awsclients.StringValue(fieldAWS) { + + return false, nil + } + + return true, nil + } + // check which one is non-nil and if its value isUpToDate + if field1 != nil { + return awsclients.StringValue(field1) == awsclients.StringValue(fieldAWS), nil + } + return awsclients.StringValue(field2) == awsclients.StringValue(fieldAWS), nil } func areVerificationMessageTemplateEqual(spec *svcapitypes.VerificationMessageTemplateType, current *svcsdk.VerificationMessageTemplateType) bool { if spec != nil && current != nil { - switch { + switch { // EmailMessage, EmailSubject, SmsMessage are checked for in conflictingFieldsEqual case awsclients.StringValue(spec.DefaultEmailOption) != awsclients.StringValue(current.DefaultEmailOption), - awsclients.StringValue(spec.EmailMessage) != awsclients.StringValue(current.EmailMessage), awsclients.StringValue(spec.EmailMessageByLink) != awsclients.StringValue(current.EmailMessageByLink), - awsclients.StringValue(spec.EmailSubject) != awsclients.StringValue(current.EmailSubject), - awsclients.StringValue(spec.EmailSubjectByLink) != awsclients.StringValue(current.EmailSubjectByLink), - awsclients.StringValue(spec.SmsMessage) != awsclients.StringValue(current.SmsMessage): + awsclients.StringValue(spec.EmailSubjectByLink) != awsclients.StringValue(current.EmailSubjectByLink): return false } } return true } +func (e *hooks) areMFAConfigEqual(cr *svcapitypes.UserPool) (bool, error) { + + out, err := e.client.GetUserPoolMfaConfig(&svcsdk.GetUserPoolMfaConfigInput{UserPoolId: awsclients.String(meta.GetExternalName(cr))}) + if err != nil { + return true, errors.Wrap(err, errFailedGetMFARequest) + } + + // out should not be nil, bc we set MFA to OFF in preCreate + if awsclients.StringValue(cr.Spec.ForProvider.MFAConfiguration) != awsclients.StringValue(out.MfaConfiguration) { + return false, nil + } + + // only check MFAConfig stuff, if MFAConfiguration is ON/OPTIONAL, + // bc AWS doesn't allow setting stuff in SetUserPoolMfaConfig, if MFA is OFF + // (-> e.g. Token enabled in specs with MFA OFF) + if awsclients.StringValue(cr.Spec.ForProvider.MFAConfiguration) != svcsdk.UserPoolMfaTypeOff { + if cr.Spec.ForProvider.SoftwareTokenMFAConfiguration != nil && out.SoftwareTokenMfaConfiguration != nil { + return awsclients.BoolValue(cr.Spec.ForProvider.SoftwareTokenMFAConfiguration.Enabled) == awsclients.BoolValue(out.SoftwareTokenMfaConfiguration.Enabled), nil + } + // no need to check SmsMfaConfiguration here, + // bc currently it is 100% overlapping with SmsConfiguration and SmsAuthenticationMessage, + // which are checked in other places + // if in future there is change in this API structure (e.g. fields separation), + // here would then be potentially the place for SmsMfaConfiguration check + } + + return true, nil +} + func lateInitialize(cr *svcapitypes.UserPoolParameters, resp *svcsdk.DescribeUserPoolOutput) error { instance := resp.UserPool cr.MFAConfiguration = awsclients.LateInitializeStringPtr(cr.MFAConfiguration, instance.MfaConfiguration) + + if instance.AdminCreateUserConfig != nil { + if cr.AdminCreateUserConfig == nil { + cr.AdminCreateUserConfig = &svcapitypes.AdminCreateUserConfigType{} + } + cr.AdminCreateUserConfig.AllowAdminCreateUserOnly = awsclients.LateInitializeBoolPtr(cr.AdminCreateUserConfig.AllowAdminCreateUserOnly, instance.AdminCreateUserConfig.AllowAdminCreateUserOnly) + } + + if instance.EmailConfiguration != nil { + if cr.EmailConfiguration == nil { + cr.EmailConfiguration = &svcapitypes.EmailConfigurationType{} + } + cr.EmailConfiguration.EmailSendingAccount = awsclients.LateInitializeStringPtr(cr.EmailConfiguration.EmailSendingAccount, instance.EmailConfiguration.EmailSendingAccount) + } + + if instance.Policies != nil { + if cr.Policies == nil { + cr.Policies = &svcapitypes.UserPoolPolicyType{PasswordPolicy: &svcapitypes.PasswordPolicyType{}} + } + if instance.Policies.PasswordPolicy != nil { + cr.Policies.PasswordPolicy.MinimumLength = awsclients.LateInitializeInt64Ptr(cr.Policies.PasswordPolicy.MinimumLength, instance.Policies.PasswordPolicy.MinimumLength) + cr.Policies.PasswordPolicy.RequireLowercase = awsclients.LateInitializeBoolPtr(cr.Policies.PasswordPolicy.RequireLowercase, instance.Policies.PasswordPolicy.RequireLowercase) + cr.Policies.PasswordPolicy.RequireNumbers = awsclients.LateInitializeBoolPtr(cr.Policies.PasswordPolicy.RequireNumbers, instance.Policies.PasswordPolicy.RequireNumbers) + cr.Policies.PasswordPolicy.RequireSymbols = awsclients.LateInitializeBoolPtr(cr.Policies.PasswordPolicy.RequireSymbols, instance.Policies.PasswordPolicy.RequireSymbols) + cr.Policies.PasswordPolicy.RequireUppercase = awsclients.LateInitializeBoolPtr(cr.Policies.PasswordPolicy.RequireUppercase, instance.Policies.PasswordPolicy.RequireUppercase) + cr.Policies.PasswordPolicy.TemporaryPasswordValidityDays = awsclients.LateInitializeInt64Ptr(cr.Policies.PasswordPolicy.TemporaryPasswordValidityDays, instance.Policies.PasswordPolicy.TemporaryPasswordValidityDays) + } + } + + if instance.VerificationMessageTemplate != nil { + if cr.VerificationMessageTemplate == nil { + cr.VerificationMessageTemplate = &svcapitypes.VerificationMessageTemplateType{} + } + cr.VerificationMessageTemplate.DefaultEmailOption = awsclients.LateInitializeStringPtr(cr.VerificationMessageTemplate.DefaultEmailOption, instance.VerificationMessageTemplate.DefaultEmailOption) + } + + // Info: to avoid redundancy+problems, do not lateInit conflicting fields + // (e.g. VerificationMessageTemplate.SmsMessage & SmsVerificationMessage) + + return nil +} + +// setMfaConfiguration sets the MFA configuration with a SetUserPoolMfaConfigWithContext-Request +func (e *hooks) setMfaConfiguration(ctx context.Context, cr *svcapitypes.UserPool) error { + // set MFA configuration (only allowed by AWS when MFA not OFF: + // -> "Invalid MFA configuration given, can't turn off MFA and configure an MFA together") + if awsclients.StringValue(cr.Spec.ForProvider.MFAConfiguration) != svcsdk.UserPoolMfaTypeOff { + mfaConfig := &svcsdk.SetUserPoolMfaConfigInput{ + MfaConfiguration: cr.Spec.ForProvider.MFAConfiguration, + UserPoolId: awsclients.String(meta.GetExternalName(cr)), + } + + // even without setting it here, + // if smsConfiguration is provided and MFA is turned ON/OPTIONAL, + // AWS will automatically use SMS as a possible MFA method + if cr.Spec.ForProvider.SmsConfiguration != nil { + mfaConfig.SmsMfaConfiguration = &svcsdk.SmsMfaConfigType{ + SmsAuthenticationMessage: cr.Spec.ForProvider.SmsAuthenticationMessage, + SmsConfiguration: &svcsdk.SmsConfigurationType{ + ExternalId: cr.Spec.ForProvider.SmsConfiguration.ExternalID, + SnsCallerArn: cr.Spec.ForProvider.SmsConfiguration.SNSCallerARN, + }, + } + } + + if cr.Spec.ForProvider.SoftwareTokenMFAConfiguration != nil { + mfaConfig.SoftwareTokenMfaConfiguration = &svcsdk.SoftwareTokenMfaConfigType{ + Enabled: cr.Spec.ForProvider.SoftwareTokenMFAConfiguration.Enabled, + } + } + + // custom error here needed, + // bc of our setting/handling of MfaConfiguration (SetUserPoolMfaConfig + UpdateUserPool) + if mfaConfig.SmsMfaConfiguration == nil && mfaConfig.SoftwareTokenMfaConfiguration == nil { + return errors.New(errMissingMFAStuff) + } + + _, err := e.client.SetUserPoolMfaConfigWithContext(ctx, mfaConfig) + if err != nil { + return errors.Wrap(err, errFailedSetMFARequest) + } + } + return nil } diff --git a/pkg/controller/cognitoidentityprovider/userpool/setup_test.go b/pkg/controller/cognitoidentityprovider/userpool/setup_test.go index f783aca8bb..260f6b33f1 100644 --- a/pkg/controller/cognitoidentityprovider/userpool/setup_test.go +++ b/pkg/controller/cognitoidentityprovider/userpool/setup_test.go @@ -12,6 +12,7 @@ import ( "github.com/pkg/errors" svcapitypes "github.com/crossplane-contrib/provider-aws/apis/cognitoidentityprovider/v1alpha1" + "github.com/crossplane-contrib/provider-aws/pkg/clients/cognitoidentityprovider/fake" ) type functionModifier func(*svcapitypes.UserPool) @@ -48,12 +49,17 @@ var ( testNumberChanged int64 = 2 testBool1 bool = true testBool2 bool = false + testTagKey string = "test-key" + testTagValue string = "test-value" + testOtherTagKey string = "test-other-key" + testOtherTagValue string = "test-other-value" ) func TestIsUpToDate(t *testing.T) { type args struct { - cr *svcapitypes.UserPool - resp *svcsdk.DescribeUserPoolOutput + cr *svcapitypes.UserPool + resp *svcsdk.DescribeUserPoolOutput + resp2 *svcsdk.GetUserPoolMfaConfigOutput } type want struct { @@ -67,8 +73,9 @@ func TestIsUpToDate(t *testing.T) { }{ "UpToDate": { args: args{ - cr: userPool(withSpec(svcapitypes.UserPoolParameters{})), - resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{}}, + cr: userPool(withSpec(svcapitypes.UserPoolParameters{})), + resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{}}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: true, @@ -95,6 +102,7 @@ func TestIsUpToDate(t *testing.T) { }, }, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -117,20 +125,7 @@ func TestIsUpToDate(t *testing.T) { }, }, }}, - }, - want: want{ - result: false, - err: nil, - }, - }, - "ChangedAliasAttributes": { - args: args{ - cr: userPool(withSpec(svcapitypes.UserPoolParameters{ - AliasAttributes: []*string{&testString1}, - })), - resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{ - AliasAttributes: []*string{&testString2}, - }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -145,6 +140,7 @@ func TestIsUpToDate(t *testing.T) { resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{ AutoVerifiedAttributes: []*string{&testString2}, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -163,6 +159,7 @@ func TestIsUpToDate(t *testing.T) { ChallengeRequiredOnNewDevice: &testBool2, }, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -181,6 +178,7 @@ func TestIsUpToDate(t *testing.T) { From: &testString2, }, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -189,12 +187,17 @@ func TestIsUpToDate(t *testing.T) { }, "ChangedEmailVerificationMessage": { args: args{ - cr: userPool(withSpec(svcapitypes.UserPoolParameters{ - EmailVerificationMessage: &testString1, - })), + cr: userPool( + withSpec(svcapitypes.UserPoolParameters{ + EmailVerificationMessage: &testString1, + VerificationMessageTemplate: &svcapitypes.VerificationMessageTemplateType{ + DefaultEmailOption: &testString1, + }, + })), resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{ EmailVerificationMessage: &testString2, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -205,10 +208,14 @@ func TestIsUpToDate(t *testing.T) { args: args{ cr: userPool(withSpec(svcapitypes.UserPoolParameters{ EmailVerificationSubject: &testString1, + VerificationMessageTemplate: &svcapitypes.VerificationMessageTemplateType{ + DefaultEmailOption: &testString1, + }, })), resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{ EmailVerificationSubject: &testString2, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -231,6 +238,7 @@ func TestIsUpToDate(t *testing.T) { }, }, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -245,6 +253,32 @@ func TestIsUpToDate(t *testing.T) { resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{ MfaConfiguration: &testString2, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{ + MfaConfiguration: &testString2, + }, + }, + want: want{ + result: false, + err: nil, + }, + }, + "ChangedMFAToken": { + args: args{ + cr: userPool(withSpec(svcapitypes.UserPoolParameters{ + MFAConfiguration: &testString1, + SoftwareTokenMFAConfiguration: &svcapitypes.SoftwareTokenMFAConfigType{ + Enabled: &testBool1, + }, + })), + resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{ + MfaConfiguration: &testString1, + }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{ + MfaConfiguration: &testString1, + SoftwareTokenMfaConfiguration: &svcsdk.SoftwareTokenMfaConfigType{ + Enabled: &testBool2, + }, + }, }, want: want{ result: false, @@ -267,6 +301,7 @@ func TestIsUpToDate(t *testing.T) { }, }, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -293,6 +328,7 @@ func TestIsUpToDate(t *testing.T) { }, }, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -307,6 +343,7 @@ func TestIsUpToDate(t *testing.T) { resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{ SmsAuthenticationMessage: &testString2, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -325,6 +362,7 @@ func TestIsUpToDate(t *testing.T) { ExternalId: &testString2, }, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -335,10 +373,14 @@ func TestIsUpToDate(t *testing.T) { args: args{ cr: userPool(withSpec(svcapitypes.UserPoolParameters{ SmsVerificationMessage: &testString1, + VerificationMessageTemplate: &svcapitypes.VerificationMessageTemplateType{ + DefaultEmailOption: &testString1, + }, })), resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{ SmsVerificationMessage: &testString2, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -357,38 +399,22 @@ func TestIsUpToDate(t *testing.T) { AdvancedSecurityMode: &testString2, }, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, err: nil, }, }, - "ChangedUsernameAttributes": { + "ChangedUserPoolTags": { args: args{ cr: userPool(withSpec(svcapitypes.UserPoolParameters{ - UsernameAttributes: []*string{&testString1}, + UserPoolTags: map[string]*string{testTagKey: &testTagValue}, })), resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{ - UsernameAttributes: []*string{&testString2}, - }}, - }, - want: want{ - result: false, - err: nil, - }, - }, - "ChangedUsernameConfiguration": { - args: args{ - cr: userPool(withSpec(svcapitypes.UserPoolParameters{ - UsernameConfiguration: &svcapitypes.UsernameConfigurationType{ - CaseSensitive: &testBool1, - }, - })), - resp: &svcsdk.DescribeUserPoolOutput{UserPool: &svcsdk.UserPoolType{ - UsernameConfiguration: &svcsdk.UsernameConfigurationType{ - CaseSensitive: &testBool2, - }, + UserPoolTags: map[string]*string{testOtherTagKey: &testOtherTagValue}, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -407,6 +433,7 @@ func TestIsUpToDate(t *testing.T) { DefaultEmailOption: &testString2, }, }}, + resp2: &svcsdk.GetUserPoolMfaConfigOutput{}, }, want: want{ result: false, @@ -417,8 +444,15 @@ func TestIsUpToDate(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { + h := &hooks{ + client: &fake.MockCognitoIdentityProviderClient{ + MockGetUserPoolMfaConfig: func(in *svcsdk.GetUserPoolMfaConfigInput) (*svcsdk.GetUserPoolMfaConfigOutput, error) { + return tc.resp2, nil + }, + }, + } // Act - result, err := isUpToDate(tc.args.cr, tc.args.resp) + result, err := h.isUpToDate(tc.args.cr, tc.args.resp) // Assert if diff := cmp.Diff(tc.want.result, result, test.EquateConditions()); diff != "" { diff --git a/pkg/controller/cognitoidentityprovider/userpool/zz_controller.go b/pkg/controller/cognitoidentityprovider/userpool/zz_controller.go index 94e90076aa..48f7c573e0 100644 --- a/pkg/controller/cognitoidentityprovider/userpool/zz_controller.go +++ b/pkg/controller/cognitoidentityprovider/userpool/zz_controller.go @@ -153,9 +153,6 @@ func (e *external) Create(ctx context.Context, mg cpresource.Managed) (managed.E } f1.InviteMessageTemplate = f1f1 } - if resp.UserPool.AdminCreateUserConfig.UnusedAccountValidityDays != nil { - f1.UnusedAccountValidityDays = resp.UserPool.AdminCreateUserConfig.UnusedAccountValidityDays - } cr.Spec.ForProvider.AdminCreateUserConfig = f1 } else { cr.Spec.ForProvider.AdminCreateUserConfig = nil diff --git a/pkg/controller/cognitoidentityprovider/userpool/zz_conversions.go b/pkg/controller/cognitoidentityprovider/userpool/zz_conversions.go index 08a827c283..b69e4c89f9 100644 --- a/pkg/controller/cognitoidentityprovider/userpool/zz_conversions.go +++ b/pkg/controller/cognitoidentityprovider/userpool/zz_conversions.go @@ -79,9 +79,6 @@ func GenerateUserPool(resp *svcsdk.DescribeUserPoolOutput) *svcapitypes.UserPool } f1.InviteMessageTemplate = f1f1 } - if resp.UserPool.AdminCreateUserConfig.UnusedAccountValidityDays != nil { - f1.UnusedAccountValidityDays = resp.UserPool.AdminCreateUserConfig.UnusedAccountValidityDays - } cr.Spec.ForProvider.AdminCreateUserConfig = f1 } else { cr.Spec.ForProvider.AdminCreateUserConfig = nil @@ -476,9 +473,6 @@ func GenerateCreateUserPoolInput(cr *svcapitypes.UserPool) *svcsdk.CreateUserPoo } f1.SetInviteMessageTemplate(f1f1) } - if cr.Spec.ForProvider.AdminCreateUserConfig.UnusedAccountValidityDays != nil { - f1.SetUnusedAccountValidityDays(*cr.Spec.ForProvider.AdminCreateUserConfig.UnusedAccountValidityDays) - } res.SetAdminCreateUserConfig(f1) } if cr.Spec.ForProvider.AliasAttributes != nil { @@ -783,9 +777,6 @@ func GenerateUpdateUserPoolInput(cr *svcapitypes.UserPool) *svcsdk.UpdateUserPoo } f1.SetInviteMessageTemplate(f1f1) } - if cr.Spec.ForProvider.AdminCreateUserConfig.UnusedAccountValidityDays != nil { - f1.SetUnusedAccountValidityDays(*cr.Spec.ForProvider.AdminCreateUserConfig.UnusedAccountValidityDays) - } res.SetAdminCreateUserConfig(f1) } if cr.Spec.ForProvider.AutoVerifiedAttributes != nil {