fix: MFA TOTP sign up - user is not set correctly #2287
Conversation
🦋 Changeset detectedLatest commit: 0157f16 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @angular @react @vue | ||
| Scenario: Successful sign up shows correct email from authenticated user | ||
| When I click the "Create Account" tab | ||
| And I type a new "email" | ||
| And I type my password | ||
| And I confirm my password | ||
| And I click the "Create Account" button | ||
| Then I see "Confirmation Code" | ||
| And I type a valid confirmation code | ||
| And I intercept '{ "headers": { "X-Amz-Target": "AWSCognitoIdentityProviderService.ConfirmSignUp" } }' with fixture "confirm-sign-up-with-email" | ||
| # Mocking these two calls is much easier than intercepting 6+ network calls with tokens that are validated & expire within the hour | ||
| And I mock 'Amplify.Auth.signIn' with fixture "Auth.signIn-mfa-setup" | ||
| And I mock 'Amplify.Auth.currentAuthenticatedUser' with fixture "Auth.currentAuthenticatedUser-setup-TOTP" | ||
| And I click the "Confirm" button | ||
| Then I see "Setup TOTP" | ||
| Then I see "Code" | ||
| And I type a valid confirmation code | ||
| And I mock 'Amplify.Auth.verifyTotpToken' with fixture "Auth.verifyTOTP" | ||
| And I click the "Confirm" button | ||
| Then I see "test@example.com" |
There was a problem hiding this comment.
This mocks the setup totp flow, and verifies the email address being sent back is from he Auth.currentAuthenticatedUser not the values from Auth.verifyTotopToken.
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "email": "test@example.com" | |||
There was a problem hiding this comment.
In the real Auth.currentAuthenticatedUser method there are many different values being returned. For simplicity sake, all I care is that this is being called and it returns this value for email.
There was a problem hiding this comment.
*changed this to username instead.
calebpollman
left a comment
calebpollman
left a comment
There was a problem hiding this comment.
Left some questions/feedback, but it seems like this might be a bug on the Amplify JS side from reading the initial ticket?
examples/next/pages/ui/components/authenticator/sign-in-totp-mfa/index.page.tsx
Outdated
Show resolved
Hide resolved
...gular/src/pages/ui/components/authenticator/sign-in-totp-mfa/sign-in-totp-mfa.component.html
Outdated
Show resolved
Hide resolved
|
This pull request introduces 1 alert when merging 491a2eb into 660d9b6 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 2461f5c into 660d9b6 - view on LGTM.com new alerts:
|
examples/next/pages/ui/components/authenticator/sign-in-totp-mfa/index.page.tsx
Outdated
Show resolved
Hide resolved
| @angular @react @vue | ||
| Scenario: Successful sign up shows correct username from authenticated user |
3 participants
Description of changes
When a user signs up and has MFA TOTP enabled, the user object is not set correctly. To correct this, instead of returing the values from
Auth.verifyTOTPI send back the values fromAuth.currentAuthenticatedUserIssue #, if available
closes #2274
Description of how you validated changes
Created additional e2e test
Checklist
yarn testpassessideEffectsfield updatedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.