fix(Authenticator): update state machine to include onDone in federat…

…ed sign in flow (#4978)

.changeset/orange-falcons-act.md

@@ -0,0 +1,7 @@
+---
+"@aws-amplify/ui-react-core": patch
+"@aws-amplify/ui-react-native": patch
+"@aws-amplify/ui": patch
+---
+
+fix(Authenticator): update state machine to include onDone in federated sign in flow

examples/react-native/ios/Podfile.lock

@@ -547,4 +547,4 @@ SPEC CHECKSUMS:
 
 PODFILE CHECKSUM: 86255707601fa0f502375c1c40775dbd535ed624
 
-COCOAPODS: 1.12.1
+COCOAPODS: 1.14.3

packages/react-core/src/Authenticator/hooks/useAuthenticatorRoute/__tests__/__snapshots__/useAuthenticatorRoute.spec.ts.snap

@@ -162,6 +162,7 @@ exports[`useAuthenticatorRoute returns the expected values for the signUp route
     "hasValidationErrors": false,
     "isPending": false,
     "socialProviders": [],
+    "toFederatedSignIn": [MockFunction],
     "toSignIn": [MockFunction],
     "validationErrors": {},
   },

packages/react-core/src/Authenticator/hooks/useAuthenticatorRoute/__tests__/utils.spec.ts

@@ -115,6 +115,7 @@ describe('getRouteMachineSelector', () => {
         ...commonSelectorProps,
         hasValidationErrors,
         socialProviders,
+        toFederatedSignIn,
         toSignIn,
         validationErrors,
         route,
@@ -186,6 +187,7 @@ describe('props resolver functions', () => {
         hasValidationErrors,
         isPending,
         socialProviders,
+        toFederatedSignIn,
         toSignIn,
         validationErrors,
       },

packages/react-core/src/Authenticator/hooks/useAuthenticatorRoute/constants.ts

@@ -76,6 +76,7 @@ const SIGN_UP_MACHINE_KEYS: SignUpMachineKey[] = [
   ...COMMON_ROUTE_MACHINE_KEYS,
   'hasValidationErrors',
   'socialProviders',
+  'toFederatedSignIn',
   'toSignIn',
   'validationErrors',
 ];

packages/react-native/src/Authenticator/common/DefaultContent/DefaultContent.tsx

@@ -14,7 +14,7 @@ function useThemedStyles<Style>(getStyle: (theme: StrictTheme) => Style) {
 }
 
 export default function DefaultContent<
-  FieldsType extends TextFieldOptionsType | RadioFieldOptions
+  FieldsType extends TextFieldOptionsType | RadioFieldOptions,
 >({
   body,
   buttons: { primary, links, secondary },

packages/react-native/src/Authenticator/common/FederatedProviderButtons/FederatedProviderButtons.tsx

@@ -1,6 +1,5 @@
 import React, { useMemo } from 'react';
 import { View } from 'react-native';
-import { signInWithRedirect } from 'aws-amplify/auth';
 
 import {
   SocialProvider,
@@ -17,21 +16,13 @@ import { getThemedStyles } from './styles';
 
 const { getSignInWithFederationText, getOrText } = authenticatorTextUtil;
 
-// use `signInWithRedirect` directly instead of `toFederatedSignIn`
-// exposed on `useAuthenticator` for RN. `@aws-amplify/rtn-web-browser`
-// does not emit an event on federated sign in flow cancellation,
-// preventing the `Authenticator` from updating state and leaving the
-// UI in a "pending" state
-const handleSignInWithRedirect = (
-  provider: 'amazon' | 'apple' | 'facebook' | 'google'
-) => signInWithRedirect({ provider: capitalize(provider) });
-
 export default function FederatedProviderButtons({
   buttonStyle,
   dividerLabelStyle,
   route,
   socialProviders,
   style,
+  toFederatedSignIn,
 }: FederatedProviderButtonsProps): JSX.Element | null {
   const theme = useTheme();
   const themedStyle = getThemedStyles(theme);
@@ -42,7 +33,7 @@ export default function FederatedProviderButtons({
         const providerIconSource = icons[`${provider}Logo`];
 
         const handlePress = () => {
-          handleSignInWithRedirect(provider);
+          toFederatedSignIn({ provider: capitalize(provider) });
         };
 
         return (
@@ -56,7 +47,7 @@ export default function FederatedProviderButtons({
           </FederatedProviderButton>
         );
       }),
-    [route, socialProviders, themedStyle, buttonStyle]
+    [route, socialProviders, themedStyle, buttonStyle, toFederatedSignIn]
   );
 
   return providerButtons?.length ? (

packages/react-native/src/Authenticator/common/FederatedProviderButtons/__tests__/FederatedProviderButtons.spec.tsx

@@ -1,6 +1,5 @@
 import React from 'react';
 import { fireEvent, render } from '@testing-library/react-native';
-import * as AuthModule from 'aws-amplify/auth';
 
 import {
   AuthenticatorRoute,
@@ -44,13 +43,7 @@ describe('FederatedProviderButtons', () => {
     expect(toJSON()).toBe(null);
   });
 
-  it('calls signInWithRedirect with the expected provider on press', () => {
-    const signInWithRedirectSpy = jest
-      .spyOn(AuthModule, 'signInWithRedirect')
-      .mockImplementation((_?: AuthModule.SignInWithRedirectInput) =>
-        Promise.resolve(undefined)
-      );
-
+  it('calls toFederatedSignIn with the expected provider on press', () => {
     const { getByText } = render(
       <FederatedProviderButtons {...defaultProps} />
     );
@@ -62,7 +55,7 @@ describe('FederatedProviderButtons', () => {
 
     fireEvent.press(providerButton);
 
-    expect(signInWithRedirectSpy).toHaveBeenCalledWith({
+    expect(toFederatedSignIn).toHaveBeenCalledWith({
       provider: capitalize(provider),
     });
   });

packages/ui/src/helpers/authenticator/__tests__/getRoute.test.ts

@@ -24,15 +24,15 @@ describe('getRoute', () => {
     } as AuthActorState;
   };
 
-  it(`should return null when the current state is unknown`, () => {
+  it('should return `null` when the current state is unknown', () => {
     const state = getState();
     const actorState = getActorState();
 
     expect(getRoute(state, actorState)).toBe(null);
   });
 
   it.each(['idle', 'setup'])(
-    `should return the '%s' route when the current state is '%s'`,
+    "should return the '%s' route when the current state is '%s'",
     (route: string) => {
       const state = getState(route);
       const actorState = getActorState(); // actor state is undefined here
@@ -41,22 +41,22 @@ describe('getRoute', () => {
     }
   );
 
-  it(`should return the 'signOut' route when the current state is 'signOut'`, () => {
+  it("should return the 'signOut' route when the current state is 'signOut'", () => {
     const state = getState('signOut');
     const actorState = getActorState('pending');
 
     expect(getRoute(state, actorState)).toBe('signOut');
   });
 
-  it(`should return the 'authenticated' route when the current state is 'authenticated'`, () => {
+  it("should return the 'authenticated' route when the current state is 'authenticated'", () => {
     const state = getState('authenticated');
     const actorState = getActorState('resolved');
 
     expect(getRoute(state, actorState)).toBe('authenticated');
   });
 
   it.each(['confirmSignUp', 'resendSignUpCode'])(
-    `should return the 'confirmSignUp' route when the current actorState is '%s'`,
+    "should return the 'confirmSignUp' route when the current actorState is '%s'",
     (route: string) => {
       const state = getState('signInActor.runActor');
       const actorState = getActorState(route);
@@ -65,15 +65,15 @@ describe('getRoute', () => {
     }
   );
 
-  it(`should return the 'confirmSignIn' route when the current actorState is 'confirmSignIn'`, () => {
+  it("should return the 'confirmSignIn' route when the current actorState is 'confirmSignIn'", () => {
     const state = getState('signInActor.runActor');
     const actorState = getActorState('confirmSignIn');
 
     expect(getRoute(state, actorState)).toBe('confirmSignIn');
   });
 
   it.each(['setupTotp.edit', 'setupTotp.submit'])(
-    `should return the 'setupTotp' route when the current actorState is '%s'`,
+    "should return the 'setupTotp' route when the current actorState is '%s'",
     (route: string) => {
       const state = getState('signInActor.runActor');
       const actorState = getActorState(route);
@@ -82,18 +82,25 @@ describe('getRoute', () => {
     }
   );
 
-  it.each(['signIn', 'federatedSignIn'])(
-    `should return the 'signIn' route when actorState is '%s'`,
+  it.each(['signIn', 'signUp'])(
+    "should return the '%s' route when actorState is 'federatedSignIn",
     (route: string) => {
-      const state = getState('signInActor.runActor');
-      const actorState = getActorState(route);
+      const state = getState(`${route}Actor`);
+      const actorState = getActorState('federatedSignIn');
 
-      expect(getRoute(state, actorState)).toBe('signIn');
+      expect(getRoute(state, actorState)).toBe(route);
     }
   );
 
+  it("should return the 'signIn' route when actorState is 'signIn'", () => {
+    const state = getState('signInActor.runActor');
+    const actorState = getActorState('signIn');
+
+    expect(getRoute(state, actorState)).toBe('signIn');
+  });
+
   it.each(['signUp', 'autoSignIn'])(
-    `should return the 'signUp' route when actorState is '%s'`,
+    "should return the 'signUp' route when actorState is '%s'",
     (route: string) => {
       const state = getState('signUpActor.runActor');
       const actorState = getActorState(route);
@@ -102,49 +109,49 @@ describe('getRoute', () => {
     }
   );
 
-  it(`should return the 'forceNewPassword' route when the current actorState is 'forceChangePassword'`, () => {
+  it("should return the 'forceNewPassword' route when the current actorState is 'forceChangePassword'", () => {
     const state = getState('signInActor.runActor');
     const actorState = getActorState('forceChangePassword');
 
     expect(getRoute(state, actorState)).toBe('forceNewPassword');
   });
 
-  it(`should return the 'forgotPassword' route when the current actorState is 'forgotPassword'`, () => {
+  it("should return the 'forgotPassword' route when the current actorState is 'forgotPassword'", () => {
     const state = getState('forgotPasswordActor.runActor');
     const actorState = getActorState('forgotPassword');
 
     expect(getRoute(state, actorState)).toBe('forgotPassword');
   });
 
-  it(`should return the 'confirmResetPassword' route when the current actorState is 'confirmResetPassword'`, () => {
+  it("should return the 'confirmResetPassword' route when the current actorState is 'confirmResetPassword'", () => {
     const state = getState('forgotPasswordActor.runActor');
     const actorState = getActorState('confirmResetPassword');
 
     expect(getRoute(state, actorState)).toBe('confirmResetPassword');
   });
 
-  it(`should return the 'verifyUser' route when the current actorState is 'selectUserAttributes'`, () => {
+  it("should return the 'verifyUser' route when the current actorState is 'selectUserAttributes'", () => {
     const state = getState('verifyUserAttributesActor.runActor');
     const actorState = getActorState('selectUserAttributes');
 
     expect(getRoute(state, actorState)).toBe('verifyUser');
   });
 
-  it(`should return the 'confirmVerifyUser' route when the current actorState is 'confirmVerifyUserAttribute'`, () => {
+  it("should return the 'confirmVerifyUser' route when the current actorState is 'confirmVerifyUserAttribute'", () => {
     const state = getState('verifyUserAttributesActor.runActor');
     const actorState = getActorState('confirmVerifyUserAttribute');
 
     expect(getRoute(state, actorState)).toBe('confirmVerifyUser');
   });
 
-  it(`should return the 'transition' route when the current state is 'getCurrentUser'`, () => {
+  it("should return the 'transition' route when the current state is 'getCurrentUser'", () => {
     const state = getState('getCurrentUser');
     const actorState = getActorState('resolved');
 
     expect(getRoute(state, actorState)).toBe('transition');
   });
 
-  it(`should return the 'transition' route when the current actorState is 'fetchUserAttributes'`, () => {
+  it("should return the 'transition' route when the current actorState is 'fetchUserAttributes'", () => {
     const state = getState('signInActor.runActor');
     const actorState = getActorState('fetchUserAttributes');
 

packages/ui/src/helpers/authenticator/getRoute.ts

@@ -7,6 +7,18 @@ export const getRoute = (
   state: AuthMachineState,
   actorState: AuthActorState
 ) => {
+  // 'federatedSignIn' exists as a state on both the 'signInActor' and 'signUpActor',
+  // match against the `actorState` initially to determine if the federated sign in flow
+  // has begun, then which actor has begun the flow and return the corresponding `route`
+  if (actorState?.matches('federatedSignIn')) {
+    if (state.matches('signUpActor')) {
+      return 'signUp';
+    }
+    if (state.matches('signInActor')) {
+      return 'signIn';
+    }
+  }
+
   switch (true) {
     case state.matches('idle'):
       return 'idle';
@@ -25,7 +37,6 @@ export const getRoute = (
     case actorState?.matches('setupTotp.submit'):
       return 'setupTotp';
     case actorState?.matches('signIn'):
-    case actorState?.matches('federatedSignIn'):
       return 'signIn';
     case actorState?.matches('signUp'):
     case actorState?.matches('autoSignIn'):

packages/ui/src/machines/authenticator/actors/__tests__/__snapshots__/utils.test.ts.snap

@@ -0,0 +1,39 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`getFederatedSignInState returns the expected 'federatedSignIn\` state values for the 'signIn' route 1`] = `
+{
+  "entry": [
+    "sendUpdate",
+    "clearError",
+  ],
+  "invoke": {
+    "onDone": {
+      "target": "signIn",
+    },
+    "onError": {
+      "actions": "setRemoteError",
+      "target": "signIn",
+    },
+    "src": "signInWithRedirect",
+  },
+}
+`;
+
+exports[`getFederatedSignInState returns the expected 'federatedSignIn\` state values for the 'signUp' route 1`] = `
+{
+  "entry": [
+    "sendUpdate",
+    "clearError",
+  ],
+  "invoke": {
+    "onDone": {
+      "target": "signUp",
+    },
+    "onError": {
+      "actions": "setRemoteError",
+      "target": "signUp",
+    },
+    "src": "signInWithRedirect",
+  },
+}
+`;

packages/ui/src/machines/authenticator/actors/__tests__/utils.test.ts

@@ -0,0 +1,11 @@
+import { getFederatedSignInState } from '../utils';
+
+describe('getFederatedSignInState', () => {
+  it.each(['signIn', 'signUp'])(
+    "returns the expected 'federatedSignIn` state values for the '%s' route",
+    (route: 'signIn' | 'signUp') => {
+      const output = getFederatedSignInState(route);
+      expect(output).toMatchSnapshot();
+    }
+  );
+});

packages/ui/src/machines/authenticator/actors/signIn.ts

@@ -15,6 +15,8 @@ import guards from '../guards';
 
 import { AuthEvent, ActorDoneData, SignInContext } from '../types';
 
+import { getFederatedSignInState } from './utils';
+
 export interface SignInMachineOptions {
   services?: Partial<typeof defaultServices>;
 }
@@ -101,13 +103,7 @@ export function signInActor({ services }: SignInMachineOptions) {
             { target: 'signIn' },
           ],
         },
-        federatedSignIn: {
-          entry: ['sendUpdate', 'clearError'],
-          invoke: {
-            src: 'signInWithRedirect',
-            onError: { actions: 'setRemoteError' },
-          },
-        },
+        federatedSignIn: getFederatedSignInState('signIn'),
         fetchUserAttributes: {
           invoke: {
             src: 'fetchUserAttributes',