aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.42k stars 2.12k forks source link

AWS cognito tokens not persisting if "remember device" = Always or user opt IN #3409

Closed hamzakhawaja27 closed 4 years ago

hamzakhawaja27 commented 5 years ago

We've identified a bug in the Cognito services and/or the Amplify SDK for JavaScript that we'd like to bring to your attention. Here is a GitHub repository with a minimal reproduction of the issue https://github.com/alwaysai/cognito-signin-bug-nodejs . The script src/index.ts invokes Amplify's Auth.signIn once with each of two user pools us-west-2_rEKRyJubT, which I'll henceforth refer to as the "bad" user pool, and us-west-2_0uQVvAYKk, which I'll refer to as the "good" user pool. Those two user pools are ~almost~ identical. Both were created today from scratch using our automation tools. The only difference between the pools is that on the "good" pool I went to Cognito web console > General settings > Devices and set "Do you want to remember your user's devices?" to "No". (Our automated script creates the pool with that attribute set to "User Opt In".)

For the "bad" pool, user auth state persistence does not work. The script outputs:

User pool us-west-2_rEKRyJubT has persisted the following keys:
 - amplify-signin-with-hostedUI

For the "good" pool, user auth state persistence works as expected. The script outputs:

User pool us-west-2_0uQVvAYKk has persisted the following keys:
 - CognitoIdentityServiceProvider.1ebs5shhsl190bmdllpbj6e7kb.chris.arnesen@gmail.com.idToken
 - CognitoIdentityServiceProvider.1ebs5shhsl190bmdllpbj6e7kb.chris.arnesen@gmail.com.accessToken
 - CognitoIdentityServiceProvider.1ebs5shhsl190bmdllpbj6e7kb.chris.arnesen@gmail.com.refreshToken
 - CognitoIdentityServiceProvider.1ebs5shhsl190bmdllpbj6e7kb.chris.arnesen@gmail.com.clockDrift
 - CognitoIdentityServiceProvider.1ebs5shhsl190bmdllpbj6e7kb.LastAuthUser
 - CognitoIdentityServiceProvider.1ebs5shhsl190bmdllpbj6e7kb.chris.arnesen@gmail.com.userData
 - amplify-signin-with-hostedUI

In both cases, the Auth.signIn operation succeeds and all the correct keys get written into storage. But in the "bad" case, something internal to the SDK subsequently removes all the keys in storage except amplify-signin-with-hostedUI before the process exits. I saw this by adding "debug" statements to the mocked "removeItem" function https://github.com/alwaysai/cognito-signin-bug-nodejs/blob/f83654e56f3e023cf6eb305c18c762da869bf17d/src/index.ts#L24 .

So that's the bug. We'd expect that auth state persistence should work properly or the SDK should throw an error. Instead it adds then silently removes the keys if we set "Do you want to remember your user's devices?" to anything but "No". We are mostly ok with the workaround of setting that to "No", but we wanted to bring this to your attention since it was pretty confusing and difficult to pinpoint.

carnesen commented 5 years ago

@hamzakhawaja27 Thank you for filing this issue! I (@carnesen) had posed this to AWS support and Hamza identified this repo as the best place to address the bug. To clarify one thing, the sentence "Those two user pools are ~almost~ identical" should read "Those two user pools are almost identical". I use markdown in emails sometimes and don't always get it right :)

rogueturnip commented 5 years ago

I was seeing something like this also but only when working on iOS. The amplify tokens all get removed when the app is restarted/shutdown. It doesn't happen with android or anything else I set with AsyncStorage. I should mention I'm using the new react-native-community Asyncstorage for other items and those work fine.

carnesen commented 5 years ago

FYI I'm going to delete the repo https://github.com/alwaysai/cognito-signin-bug-nodejs. For posterity the minimal reproduction code now lives here https://gist.github.com/carnesen/5a09fd4899f2c1b7c4121307e487c644 I'm also going to delete the user pools referenced in the reproduction code because I'm a lil OCD about digital organization, and I don't like having usable credentials for one of our AWS resources published on the internet.

dhordeyev commented 5 years ago

Any update on this issue? We get the same problem with device tracking on

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

carnesen commented 5 years ago

AFAIK this issue still exists in the software.

sammartinez commented 4 years ago

@hamzakhawaja27 In clicking on the link for that was sent, it looks like the repo is no longer available. Are you still looking for this use case? Please let us know

carnesen commented 4 years ago

I deleted the repo and instead consolidated the reproduction code to https://gist.github.com/carnesen/5a09fd4899f2c1b7c4121307e487c644 . I'm actually no longer working at the company at which we were using Cognito for authentication. So I no longer have a vested interest in a solution. This was more one of those things where I spent many hours trying to pin down and work around the bug and figured y'all might appreciate a heads up. As far as I'm concerned you can close it.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

haverchuck commented 4 years ago

Closing this issue due to inactivity.

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.