fix(authenticator): Move autoSignIn logic out of signUp actor (#1910)

wlee221 · slaymance · web-flow · commit 766bf302c4d5 · 2022-05-20T10:59:49.000-07:00
* Remove `Auth.signIn` from `confirmSignUp` service This should fail because we aren't setting intents yet. * Implement "autoSignIn" redirect intent * Fix incorrect json keys * Remove verification step in autoSignIn * Remove unused var * Add autoSignIn route * Add test for sms-mfa redirect after autosignin * Revert "Fix incorrect json keys" This reverts commit f0d954e. * Create eight-insects-mix.md * Create famous-vans-smile.md * Edit wording * Update eight-insects-mix.md * Update packages/ui/src/machines/authenticator/actors/signIn.ts Co-authored-by: Shane Laymance <shane.laymance@gmail.com> Co-authored-by: Shane Laymance <shane.laymance@gmail.com>
diff --git a/.changeset/eight-insects-mix.md b/.changeset/eight-insects-mix.md
@@ -0,0 +1,6 @@
+---
+"@aws-amplify/ui-react": patch
+"@aws-amplify/ui": patch
+---
+
+refactor(internal): Move `autoSignIn` logic out of `signUp` actor
diff --git a/.changeset/famous-vans-smile.md b/.changeset/famous-vans-smile.md
@@ -0,0 +1,8 @@
+---
+"@aws-amplify/ui-react": patch
+"@aws-amplify/ui": patch
+---
+
+On userpools with sms mfa required, authenticator will now automatically redirect user to sms mfa page after successful sign up.
+
+Previously, end users needed to sign in again to go to the sms mfa page ([#1660](https://github.com/aws-amplify/amplify-ui/issues/1660)).
diff --git a/packages/e2e/cypress/fixtures/Auth.signIn-sms-mfa.json b/packages/e2e/cypress/fixtures/Auth.signIn-sms-mfa.json
@@ -0,0 +1,8 @@
+{
+  "challengeName": "SMS_MFA",
+  "challengeParameters": {
+    "CODE_DELIVERY_DELIVERY_MEDIUM": "SMS",
+    "CODE_DELIVERY_DESTINATION": "+*******0839"
+  },
+  "session": "••••••"
+}
diff --git a/packages/e2e/features/ui/components/authenticator/sign-up-sms-mfa.feature b/packages/e2e/features/ui/components/authenticator/sign-up-sms-mfa.feature
@@ -0,0 +1,27 @@
+Feature: Sign up with SMS MFA
+
+  If your backend has SMS MFA required, Authenticator will redirect end users to 
+  SMS confirmation screen when they successfully sign up.
+
+  Background:
+    Given I'm running the example "ui/components/authenticator/sign-in-sms-mfa"
+    And I intercept '{ "headers": { "X-Amz-Target": "AWSCognitoIdentityProviderService.SignUp" } }' with fixture "sign-up-with-phone"
+    When I click the "Create Account" tab 
+
+  @angular @react @vue
+  Scenario: Successful sign up redirects user to sms mfa route
+    When I select my country code with status "UNCONFIRMED"
+    And I type my "phone number" with status "UNCONFIRMED"
+    And I type my password
+    And I confirm my password
+    And I type my "email" with status "UNCONFIRMED"
+    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-sms-mfa"
+    And I mock 'Amplify.Auth.currentAuthenticatedUser' with fixture "Auth.currentAuthenticatedUser-verified-email"
+    And I click the "Confirm" button
+    Then I see "Confirm SMS Code"
+    
diff --git a/packages/react/src/components/Authenticator/Router/Router.tsx b/packages/react/src/components/Authenticator/Router/Router.tsx
@@ -16,6 +16,7 @@ const getRouteComponent = (route: string) => {
     case 'authenticated':
     case 'idle':
     case 'setup':
+    case 'autoSignIn':
       return () => null;
     case 'confirmSignUp':
       return ConfirmSignUp;
diff --git a/packages/ui/src/helpers/authenticator/facade.ts b/packages/ui/src/helpers/authenticator/facade.ts
@@ -73,6 +73,8 @@ export const getServiceContextFacade = (state: AuthMachineState) => {
         return 'signOut';
       case state.matches('authenticated'):
         return 'authenticated';
+      case actorState?.matches('autoSignIn'):
+        return 'autoSignIn';
       case actorState?.matches('confirmSignUp'):
         return 'confirmSignUp';
       case actorState?.matches('confirmSignIn'):
diff --git a/packages/ui/src/machines/authenticator/actors/signIn.ts b/packages/ui/src/machines/authenticator/actors/signIn.ts
@@ -41,7 +41,10 @@ export function signInActor({ services }: SignInMachineOptions) {
       id: 'signInActor',
       states: {
         init: {
-          always: [{ target: 'signIn' }],
+          always: [
+            { target: 'autoSignIn', cond: 'shouldAutoSignIn' },
+            { target: 'signIn' },
+          ],
         },
         signIn: {
           initial: 'edit',
@@ -141,6 +144,62 @@ export function signInActor({ services }: SignInMachineOptions) {
             rejected: { always: '#signInActor.rejected' },
           },
         },
+        autoSignIn: {
+          initial: 'submit',
+          states: {
+            submit: {
+              tags: ['pending'],
+              entry: ['clearError', 'sendUpdate'],
+              invoke: {
+                src: 'signIn',
+                onDone: [
+                  {
+                    cond: 'shouldSetupTOTP',
+                    actions: ['setUser', 'setChallengeName'],
+                    target: '#signInActor.setupTOTP',
+                  },
+                  {
+                    cond: 'shouldConfirmSignIn',
+                    actions: ['setUser', 'setChallengeName'],
+                    target: '#signInActor.confirmSignIn',
+                  },
+                  {
+                    cond: 'shouldForceChangePassword',
+                    actions: [
+                      'setUser',
+                      'setChallengeName',
+                      'setRequiredAttributes',
+                    ],
+                    target: '#signInActor.forceNewPassword',
+                  },
+                  {
+                    actions: 'setUser',
+                    target: '#signInActor.resolved',
+                  },
+                ],
+                onError: [
+                  {
+                    cond: 'shouldRedirectToConfirmSignUp',
+                    actions: ['setCredentials', 'setConfirmSignUpIntent'],
+                    target: '#signInActor.rejected',
+                  },
+                  {
+                    cond: 'shouldRedirectToConfirmResetPassword',
+                    actions: [
+                      'setUsernameAuthAttributes',
+                      'setConfirmResetPasswordIntent',
+                    ],
+                    target: '#signInActor.rejected',
+                  },
+                  {
+                    actions: 'setRemoteError',
+                    target: '#signInActor.signIn',
+                  },
+                ],
+              },
+            },
+          },
+        },
         confirmSignIn: {
           initial: 'edit',
           exit: ['clearFormValues', 'clearError', 'clearTouched'],
@@ -414,6 +473,9 @@ export function signInActor({ services }: SignInMachineOptions) {
 
           return validChallengeNames.includes(challengeName);
         },
+        shouldAutoSignIn: (context) => {
+          return context?.intent === 'autoSignIn';
+        },
         shouldRedirectToConfirmSignUp: (_, event): boolean => {
           return event.data.code === 'UserNotConfirmedException';
         },
@@ -438,7 +500,15 @@ export function signInActor({ services }: SignInMachineOptions) {
       },
       services: {
         async signIn(context) {
-          const { username, password } = context.formValues;
+          /**
+           * `authAttributes` are any username/password combo we remembered in
+           * memory. This is used in autoSignIn flow usually to pass username/pw
+           * from `confirmSignUp`.
+           */
+          const { authAttributes = {}, formValues = {} } = context;
+
+          const credentials = { ...authAttributes, ...formValues };
+          const { username, password } = credentials;
 
           return await services.handleSignIn({
             username,
diff --git a/packages/ui/src/machines/authenticator/index.ts b/packages/ui/src/machines/authenticator/index.ts
@@ -133,8 +133,9 @@ export function createAuthenticatorMachine() {
           on: {
             SIGN_IN: 'signIn',
             'done.invoke.signUpActor': {
-              target: 'signUp.autoSignIn',
+              target: '#authenticator.signIn',
               actions: 'setActorDoneData',
+              cond: 'shouldAutoSignIn',
             },
           },
         },
@@ -293,7 +294,7 @@ export function createAuthenticatorMachine() {
           actorRef: (context, _) => {
             const { services } = context;
             const actor = signInActor({ services }).withContext({
-              authAttributes: context.actorDoneData?.authAttributes,
+              authAttributes: context.actorDoneData?.authAttributes ?? {},
               user: context.user,
               intent: context.actorDoneData?.intent,
               country_code: DEFAULT_COUNTRY_CODE,
@@ -373,6 +374,7 @@ export function createAuthenticatorMachine() {
           event.data?.intent === 'confirmSignUp',
         shouldRedirectToResetPassword: (_, event) =>
           event.data?.intent === 'confirmPasswordReset',
+        shouldAutoSignIn: (_, event) => event.data?.intent === 'autoSignIn',
         shouldSetup: (context) => context.hasSetup === false,
         // other context guards
         hasActor: (context) => !!context.actorRef,
diff --git a/packages/ui/src/machines/authenticator/signUp.ts b/packages/ui/src/machines/authenticator/signUp.ts
@@ -1,7 +1,7 @@
 import { Auth } from 'aws-amplify';
 import get from 'lodash/get';
 import pickBy from 'lodash/pickBy';
-import { createMachine, sendUpdate } from 'xstate';
+import { assign, createMachine, sendUpdate } from 'xstate';
 
 import { AuthEvent, SignUpContext } from '../../types';
 import { runValidators } from '../../validators';
@@ -132,16 +132,9 @@ export function createSignUpMachine({ services }: SignUpMachineOptions) {
                   },
                 },
                 skipConfirm: {
-                  invoke: {
-                    src: 'signIn',
-                    onDone: {
-                      target: '#signUpActor.resolved',
-                      actions: 'setUser',
-                    },
-                    onError: {
-                      target: 'idle',
-                      actions: 'setRemoteError',
-                    },
+                  always: {
+                    target: '#signUpActor.resolved',
+                    actions: 'setAutoSignInIntent',
                   },
                 },
 
@@ -174,7 +167,7 @@ export function createSignUpMachine({ services }: SignUpMachineOptions) {
                 onError: [
                   {
                     target: '#signUpActor.resolved',
-                    actions: 'setUser',
+                    actions: 'setAutoSignInIntent',
                     cond: 'isUserAlreadyConfirmed',
                   },
                   { target: 'edit', actions: 'setRemoteError' },
@@ -188,7 +181,7 @@ export function createSignUpMachine({ services }: SignUpMachineOptions) {
                 src: 'confirmSignUp',
                 onDone: {
                   target: '#signUpActor.resolved',
-                  actions: ['setUser'],
+                  actions: 'setAutoSignInIntent',
                 },
                 onError: { target: 'edit', actions: 'setRemoteError' },
               },
@@ -203,6 +196,7 @@ export function createSignUpMachine({ services }: SignUpMachineOptions) {
             return {
               user: get(event, 'data.user') || context.user,
               authAttributes: { username, password },
+              intent: context.intent,
             };
           },
         },
@@ -247,28 +241,17 @@ export function createSignUpMachine({ services }: SignUpMachineOptions) {
         setCodeDeliveryDetails,
         setUser,
         sendUpdate: sendUpdate(), // sendUpdate is a HOC
+        setAutoSignInIntent: assign({ intent: (_) => 'autoSignIn' }),
       },
       services: {
-        async signIn(context, event) {
-          const { user, authAttributes, formValues } = context;
-
-          const username =
-            get(user, 'username') || get(authAttributes, 'username');
-          const password = get(formValues, 'password');
-
-          return await Auth.signIn(username, password);
-        },
         async confirmSignUp(context, event) {
           const { user, authAttributes, formValues } = context;
           const { confirmation_code: code } = formValues;
 
           const username =
             get(user, 'username') || get(authAttributes, 'username');
-          const { password } = authAttributes;
-
-          await services.handleConfirmSignUp({ username, code });
 
-          return await Auth.signIn(username, password);
+          return await services.handleConfirmSignUp({ username, code });
         },
         async resendConfirmationCode(context, event) {
           const { user, authAttributes } = context;
