Skip to content
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, authenticator): prevent fetch session during sign up #3175

Closed
wants to merge 2 commits into from

Conversation

Jordan-Nelson
Copy link
Member

Issue #, if available: #3122

Description of changes:

  • prevent Auth.signIn() from fetching the session by using loadCredentials() insead of getUserPoolTokens() and by using an AnonymousCredentialsProvider on sign in calls.
  • Update the Authenticator to use getCurrentUser to check the sign in state prior to calling sign in. This aligns with the updated behavior in the auth library and will prevent unnecessary network calls to fetch the session.

Background:

  • The sign in flow can result in unnecessary network calls. At multiple points in the sign in flow there is an attempt to retrieve credentials. If there is no unauthenticated identity at this point, the auth lib attempts to create an unauthenticated identity. This results in 2 extra calls in the case where unuathenticated identities are allowed (one to get ID and one to get credentials) and results in several additional calls in the case that unuathenticated identities are not allowed (each attempt will call get ID which will fail).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Jordan-Nelson Jordan-Nelson requested a review from a team as a code owner June 10, 2023 14:53
@Jordan-Nelson Jordan-Nelson force-pushed the feat/auth/sign-in-fetch-session branch from 3b67a3f to 9037474 Compare June 10, 2023 14:55
final initResponse = await cognitoIdentityProvider
.initiateAuth(
initRequest,
// initiateAuth does not require credentials
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// initiateAuth does not require credentials
// initiateAuth does not require credentials and using the default fetchAuthSession
// credentials provider will trigger unnecessary calls to Cognito

);

fetchAuthSessionStateMachine.stream.listen(
(_) => throw StateError('.signIn() should not fetch auth session.'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(_) => throw StateError('.signIn() should not fetch auth session.'),
(_) => fail('.signIn() should not fetch auth session.'),

@Jordan-Nelson
Copy link
Member Author

Closing in favor of #3671

@Jordan-Nelson Jordan-Nelson deleted the feat/auth/sign-in-fetch-session branch October 20, 2023 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants