fix(auth): passwordless pr feedback (#22)

* callout in ts docs for password requirement * unify callback and store reset for autosignin * comment for clarity
aws-amplify · Nov 22, 2024 · b253166 · b253166
1 parent 6fadcdf
commit b253166
Show file tree

Hide file tree

Showing 9 changed files with 21 additions and 25 deletions.
diff --git a/packages/auth/__tests__/providers/cognito/autoSignIn.test.ts b/packages/auth/__tests__/providers/cognito/autoSignIn.test.ts
@@ -84,7 +84,6 @@ describe('autoSignIn()', () => {
 			mockCreateSignUpClient.mockClear();
 			handleUserSRPAuthFlowSpy.mockClear();
 
-			autoSignInStore.dispatch({ type: 'RESET' });
 			resetAutoSignIn();
 		});
 
@@ -164,7 +163,6 @@ describe('autoSignIn()', () => {
 			mockHandleUserAuthFlow.mockClear();
 			mockCreateConfirmSignUpClient.mockClear();
 
-			autoSignInStore.dispatch({ type: 'RESET' });
 			resetAutoSignIn();
 		});
 

diff --git a/packages/auth/src/providers/cognito/apis/autoSignIn.ts b/packages/auth/src/providers/cognito/apis/autoSignIn.ts
@@ -1,6 +1,7 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
+import { autoSignInStore } from '../../../client/utils/store';
 import { AuthError } from '../../../errors/AuthError';
 import { AUTO_SIGN_IN_EXCEPTION } from '../../../errors/constants';
 import { AutoSignInCallback } from '../../../types/models';
@@ -114,6 +115,9 @@ export function setAutoSignIn(callback: AutoSignInCallback) {
  *
  * @internal
  */
-export function resetAutoSignIn() {
-	autoSignIn = initialAutoSignIn;
+export function resetAutoSignIn(resetCallback = true) {
+	if (resetCallback) {
+		autoSignIn = initialAutoSignIn;
+	}
+	autoSignInStore.dispatch({ type: 'RESET' });
 }
diff --git a/packages/auth/src/providers/cognito/apis/confirmSignUp.ts b/packages/auth/src/providers/cognito/apis/confirmSignUp.ts
@@ -93,7 +93,6 @@ export async function confirmSignUp(
 				autoSignInStoreState.username !== username
 			) {
 				resolve(signUpOut);
-				autoSignInStore.dispatch({ type: 'RESET' });
 				resetAutoSignIn();
 
 				return;

diff --git a/packages/auth/src/providers/cognito/apis/signIn.ts b/packages/auth/src/providers/cognito/apis/signIn.ts
@@ -8,13 +8,13 @@ import {
 import { assertUserNotAuthenticated } from '../utils/signInHelpers';
 import { SignInInput, SignInOutput } from '../types';
 import { AuthValidationErrorCode } from '../../../errors/types/validation';
-import { autoSignInStore } from '../../../client/utils/store';
 
 import { signInWithCustomAuth } from './signInWithCustomAuth';
 import { signInWithCustomSRPAuth } from './signInWithCustomSRPAuth';
 import { signInWithSRP } from './signInWithSRP';
 import { signInWithUserPassword } from './signInWithUserPassword';
 import { signInWithUserAuth } from './signInWithUserAuth';
+import { resetAutoSignIn } from './autoSignIn';
 
 /**
  * Signs a user in
@@ -28,7 +28,12 @@ import { signInWithUserAuth } from './signInWithUserAuth';
  * @throws AuthTokenConfigException - Thrown when the token provider config is invalid.
  */
 export async function signIn(input: SignInInput): Promise<SignInOutput> {
-	autoSignInStore.dispatch({ type: 'RESET' });
+	// Here we want to reset the store but not reassign the callback.
+	// The callback is reset when the underlying promise resolves or rejects.
+	// With the advent of session based sign in, this guarantees that the signIn API initiates a new auth flow,
+	// regardless of whether it is called for a user currently engaged in an active auto sign in session.
+	resetAutoSignIn(false);
+
 	const authFlowType = input.options?.authFlowType;
 	await assertUserNotAuthenticated();
 	switch (authFlowType) {

diff --git a/packages/auth/src/providers/cognito/apis/signInWithSRP.ts b/packages/auth/src/providers/cognito/apis/signInWithSRP.ts
@@ -28,7 +28,6 @@ import {
 	SignInWithSRPOutput,
 } from '../types';
 import {
-	autoSignInStore,
 	cleanActiveSignInState,
 	setActiveSignInState,
 } from '../../../client/utils/store';
@@ -93,8 +92,6 @@ export async function signInWithSRP(
 		});
 		if (AuthenticationResult) {
 			cleanActiveSignInState();
-			autoSignInStore.dispatch({ type: 'RESET' });
-			resetAutoSignIn();
 			await cacheCognitoTokens({
 				username: activeUsername,
 				...AuthenticationResult,
@@ -109,6 +106,8 @@ export async function signInWithSRP(
 
 			await dispatchSignedInHubEvent();
 
+			resetAutoSignIn();
+
 			return {
 				isSignedIn: true,
 				nextStep: { signInStep: 'DONE' },
@@ -121,7 +120,6 @@ export async function signInWithSRP(
 		});
 	} catch (error) {
 		cleanActiveSignInState();
-		autoSignInStore.dispatch({ type: 'RESET' });
 		resetAutoSignIn();
 		assertServiceError(error);
 		const result = getSignInResultFromError(error.name);

diff --git a/packages/auth/src/providers/cognito/apis/signInWithUserAuth.ts b/packages/auth/src/providers/cognito/apis/signInWithUserAuth.ts
@@ -101,8 +101,6 @@ export async function signInWithUserAuth(
 
 		if (response.AuthenticationResult) {
 			cleanActiveSignInState();
-			autoSignInStore.dispatch({ type: 'RESET' });
-			resetAutoSignIn();
 			await cacheCognitoTokens({
 				username: activeUsername,
 				...response.AuthenticationResult,
@@ -116,6 +114,8 @@ export async function signInWithUserAuth(
 			});
 			await dispatchSignedInHubEvent();
 
+			resetAutoSignIn();
+
 			return {
 				isSignedIn: true,
 				nextStep: { signInStep: 'DONE' },
@@ -132,7 +132,6 @@ export async function signInWithUserAuth(
 		});
 	} catch (error) {
 		cleanActiveSignInState();
-		autoSignInStore.dispatch({ type: 'RESET' });
 		resetAutoSignIn();
 		assertServiceError(error);
 		const result = getSignInResultFromError(error.name);

diff --git a/packages/auth/src/providers/cognito/apis/signInWithUserPassword.ts b/packages/auth/src/providers/cognito/apis/signInWithUserPassword.ts
@@ -26,7 +26,6 @@ import {
 	SignInWithUserPasswordOutput,
 } from '../types';
 import {
-	autoSignInStore,
 	cleanActiveSignInState,
 	setActiveSignInState,
 } from '../../../client/utils/store';
@@ -87,6 +86,7 @@ export async function signInWithUserPassword(
 			signInDetails,
 		});
 		if (AuthenticationResult) {
+			cleanActiveSignInState();
 			await cacheCognitoTokens({
 				...AuthenticationResult,
 				username: activeUsername,
@@ -98,12 +98,11 @@ export async function signInWithUserPassword(
 				}),
 				signInDetails,
 			});
-			cleanActiveSignInState();
-			autoSignInStore.dispatch({ type: 'RESET' });
-			resetAutoSignIn();
 
 			await dispatchSignedInHubEvent();
 
+			resetAutoSignIn();
+
 			return {
 				isSignedIn: true,
 				nextStep: { signInStep: 'DONE' },
@@ -116,7 +115,6 @@ export async function signInWithUserPassword(
 		});
 	} catch (error) {
 		cleanActiveSignInState();
-		autoSignInStore.dispatch({ type: 'RESET' });
 		resetAutoSignIn();
 		assertServiceError(error);
 		const result = getSignInResultFromError(error.name);

diff --git a/packages/auth/src/providers/cognito/utils/signUpHelpers.ts b/packages/auth/src/providers/cognito/utils/signUpHelpers.ts
@@ -10,7 +10,6 @@ import { AutoSignInCallback } from '../../../types/models';
 import { AuthError } from '../../../errors/AuthError';
 import { resetAutoSignIn, setAutoSignIn } from '../apis/autoSignIn';
 import { AUTO_SIGN_IN_EXCEPTION } from '../../../errors/constants';
-import { autoSignInStore } from '../../../client/utils/store';
 import { signInWithUserAuth } from '../apis/signInWithUserAuth';
 
 const MAX_AUTOSIGNIN_POLLING_MS = 3 * 60 * 1000;
@@ -37,7 +36,6 @@ export function handleCodeAutoSignIn(signInInput: SignInInput) {
 	// This will stop the listener if confirmSignUp is not resolved.
 	const timeOutId = setTimeout(() => {
 		stopHubListener();
-		autoSignInStore.dispatch({ type: 'RESET' });
 		clearTimeout(timeOutId);
 		resetAutoSignIn();
 	}, MAX_AUTOSIGNIN_POLLING_MS);
@@ -84,20 +82,17 @@ function handleAutoSignInWithLink(
 				}),
 			);
 			resetAutoSignIn();
-			autoSignInStore.dispatch({ type: 'RESET' });
 		} else {
 			try {
 				const signInOutput = await signIn(signInInput);
 				if (signInOutput.nextStep.signInStep !== 'CONFIRM_SIGN_UP') {
 					resolve(signInOutput);
 					clearInterval(autoSignInPollingIntervalId);
-					autoSignInStore.dispatch({ type: 'RESET' });
 					resetAutoSignIn();
 				}
 			} catch (error) {
 				clearInterval(autoSignInPollingIntervalId);
 				reject(error);
-				autoSignInStore.dispatch({ type: 'RESET' });
 				resetAutoSignIn();
 			}
 		}

diff --git a/packages/auth/src/types/inputs.ts b/packages/auth/src/types/inputs.ts
@@ -75,7 +75,7 @@ export interface AuthSignInWithRedirectInput {
  * The parameters for constructing a Sign Up input.
  *
  * @param username - a standard username, potentially an email/phone number
- * @param password - the user's password
+ * @param password - the user's password, may be required depending on your Cognito User Pool configuration
  * @param options - optional parameters for the Sign Up process, including user attributes
  */
 export interface AuthSignUpInput<