-
Notifications
You must be signed in to change notification settings - Fork 119
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(auth): Fix for checking current preferred type before setting the MFAType as enabled to ensure the current preference is not cleared out #2580
Conversation
… enabled to ensure the current preference is not cleared out
aws-auth-cognito/src/main/java/com/amplifyframework/auth/cognito/RealAWSCognitoAuthPlugin.kt
Outdated
Show resolved
Hide resolved
aws-auth-cognito/src/main/java/com/amplifyframework/auth/cognito/RealAWSCognitoAuthPlugin.kt
Outdated
Show resolved
Hide resolved
aws-auth-cognito/src/main/java/com/amplifyframework/auth/cognito/RealAWSCognitoAuthPlugin.kt
Outdated
Show resolved
Hide resolved
aws-auth-cognito/src/main/java/com/amplifyframework/auth/cognito/RealAWSCognitoAuthPlugin.kt
Outdated
Show resolved
Hide resolved
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## main #2580 +/- ##
==========================================
+ Coverage 41.72% 41.76% +0.03%
==========================================
Files 900 900
Lines 28791 28814 +23
Branches 4082 4093 +11
==========================================
+ Hits 12012 12033 +21
- Misses 15459 15462 +3
+ Partials 1320 1319 -1 |
aws-auth-cognito/src/main/java/com/amplifyframework/auth/cognito/RealAWSCognitoAuthPlugin.kt
Outdated
Show resolved
Hide resolved
aws-auth-cognito/src/main/java/com/amplifyframework/auth/cognito/RealAWSCognitoAuthPlugin.kt
Outdated
Show resolved
Hide resolved
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.
Discovered an issue that can be seen in the described scenario.'
Say TOTP was enabled and preferred.
Next, if a user were to set SMS as enabled and preferred, the update call would also attempt to set TOTP as preferred due to the previous setting.
2 MFA settings can't be preferred at the same time.
aws-auth-cognito/src/main/java/com/amplifyframework/auth/cognito/RealAWSCognitoAuthPlugin.kt
Outdated
Show resolved
Hide resolved
… MFAType as enabled to ensure the current preference is not cleared ou (#2580)
Fix for checking current preferred type before setting the MFAType as enabled to ensure the current preference is not cleared out
Issue #, if available:
Description of changes: For updating of mfa preferences we were just updating based on what customer had provided without taking into consideration what current preferences were which meant we could get into scenarios where 2 preferences were set to preferred. Hence in this PR I aim to fix that by prefetching the current preferences before updating it.
How did you test these changes?
(Please add a line here how the changes were tested)
Documentation update required?
General Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.