-
Notifications
You must be signed in to change notification settings - Fork 372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(cognito/userpool)!: MFA not always working #1533
fix(cognito/userpool)!: MFA not always working #1533
Conversation
588201c
to
1624b0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please prettify the comments and then we are ready to merge.
1624b0a
to
af13bb0
Compare
@wotolom can you rebase ? we fixed the CI issue with check-diff |
af13bb0
to
beb2253
Compare
@MisterMX you requested changes so i unable to merge the PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for implementation
-add field SoftwareTokenMFAConfiguration -remove deprecated UnusedAccountValidityDays -enhance several controller parts Signed-off-by: Charel Baum <charel.baum@accenture.com>
beb2253
to
83bfe4f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks a lot @wotolom
Description of your changes
SoftwareTokenMFAConfiguration
(+ handling throughSetUserPoolMfaConfigWithContext
)AdminCreateUserConfig.UnusedAccountValidityDays
isUptoDate
-checks for not updatable fieldslateInitialize
some fieldsFixes #1499
While fixing the MFA issue, I came across some smaller, missed edge cases, which I fixed/implemented.
One of these is a Breaking Change: the removal of the deprecated field
adminCreateUserConfig.unusedAccountValidityDays
.Terraform already eliminated the deprecated field. (hashicorp/terraform-provider-aws#14294)
Users using
adminCreateUserConfig.unusedAccountValidityDays
will have to usepolicies.passwordPolicy.temporaryPasswordValidityDays
instead.I have:
make reviewable test
to ensure this PR is ready for review.How has this code been tested
Manually with the relating example.
Minimal addition to setup_test.go.