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

After remembering device, calling Amplify.Auth.signOut() causes next signIn() call to return CONFIRM_SIGN_IN_WITH_SMS_MFA_CODE #2295

Closed
1 task done
TylerMcCraw opened this issue Feb 17, 2023 · 28 comments · Fixed by #2614
Assignees
Labels
auth Related to the Auth category/plugins bug Something isn't working

Comments

@TylerMcCraw
Copy link

TylerMcCraw commented Feb 17, 2023

Before opening, please confirm:

Language and Async Model

Kotlin - Coroutines

Amplify Categories

Authentication

Gradle script dependencies

// From TOML file
[versions]
amplify = "2.2.1"
androidGradlePlugin = "7.4.1"
kotlin = "1.8.0"
kotlin-coroutines = "1.6.4"

[libraries]
amplify-cognito = { module = "com.amplifyframework:aws-auth-cognito", version.ref = "amplify" }
amplify-kotlin = { module = "com.amplifyframework:core-kotlin", version.ref = "amplify" }
kotlinx-coroutines-core = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "kotlin-coroutines" }
kotlinx-coroutines-android = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-android", version.ref = "kotlin-coroutines" }

Environment information

------------------------------------------------------------
Gradle 7.6
------------------------------------------------------------

Build time:   2022-11-25 13:35:10 UTC
Revision:     daece9dbc5b79370cc8e4fd6fe4b2cd400e150a8

Kotlin:       1.7.10
Groovy:       3.0.13
Ant:          Apache Ant(TM) version 1.10.11 compiled on July 10 2021
JVM:          11.0.17 (Homebrew 11.0.17+0)
OS:           Mac OS X 13.2.1 aarch64

Describe the bug

If Amplify.Auth.rememberDevice() is called after confirmSignIn(), then I would expect that the device would be remembered and the user would not be challenged for MFA if they were to sign out and sign back in.

Currently, if a device is set to be remembered after a user confirms signin and then they sign out and sign in again, we receive a CONFIRM_SIGN_IN_WITH_SMS_MFA_CODE as the signIn() call's nextStep.signInStep. Unless I'm misunderstanding the expected outcome of a signOut() call, I think the user should be receiving DONE as the nextStep.signInStep

Reproduction steps (if applicable)

  1. Call Amplify.Auth.signIn(username, password). Result should return CONFIRM_SIGN_IN_WITH_SMS_MFA_CODE as next step
  2. Call Amplify.Auth.confirmSignIn(code). Result should return DONE as next step
  3. Call Amplify.Auth.rememberDevice()
  4. Call Amplify.Auth.signOut(options = AuthSignOutOptions.builder().globalSignOut(false).build())
  5. Call Amplify.Auth.signIn(username, password). Result returns CONFIRM_SIGN_IN_WITH_SMS_MFA_CODE again as next step. Result should have returned DONE 🐛

amplifyconfiguration.json

{
  "auth": {
    "plugins": {
      "awsCognitoAuthPlugin": {
        "IdentityManager": {
          "Default": {}
        },
        "CognitoUserPool": {
          "Default": {
            "PoolId": "REDACTED",
            "AppClientId": "REDACTED",
            "Region": "us-east-1"
          }
        },
        "Auth": {
          "Default": {
            "authenticationFlowType": "USER_SRP_AUTH",
            "OAuth": {
              "WebDomain": "REDACTED",
              "AppClientId": "REDACTED",
              "SignInRedirectURI": "REDACTED",
              "SignOutRedirectURI": "REDACTED",
              "Scopes": [
                "aws.cognito.signin.user.admin"
              ]
            }
          }
        }
      }
    }
  }
}
@gpanshu gpanshu added the auth Related to the Auth category/plugins label Feb 17, 2023
@tjleing tjleing added the bug Something isn't working label Feb 20, 2023
@dengdan154 dengdan154 added p3 and removed p3 labels Feb 21, 2023
@tjleing
Copy link
Contributor

tjleing commented Feb 24, 2023

Hello @TylerMcCraw, thank you for reaching out! Have you enabled using a remembered device to suppress the second factor in MFA in your Cognito user pool? Here are the instructions to do that. On the new UI of Cognito those customization options are under user pool -> sign-in experience -> device tracking. Hope this helps!

@TylerMcCraw
Copy link
Author

Yes, we have. Devices are being remembered.

And if a session timeout happens, which causes our sign-in screen to show, then on subsequent signin calls the user isn't required to be challenged (i.e. their next step is DONE).

@TylerMcCraw
Copy link
Author

This issue is causing a serious detriment to our user experience and negates our ability to remember devices. Could we better understand what may need to be done to fix this?
I've tried reading through the entire codebase to understand why this is happening, but I can't seem to figure it out.

It seems like the device credential is getting cleared here:

credentialStoreClient.clearCredentials(CredentialType.Device(username))

But, I'm not sure if this is the cause of this particular issue.

@TylerMcCraw
Copy link
Author

I've had our iOS team verify if this behavior is similar with using the amplify-swift library.
They are NOT receiving a CONFIRM_SIGN_IN_WITH_SMS_MFA_CODE as the next step in step #5 of my reproduction steps above.

@div5yesh div5yesh assigned div5yesh and tjleing and unassigned tjleing Mar 9, 2023
@TylerMcCraw
Copy link
Author

@div5yesh @tjleing Have you all been able to read my recent notes here and compare the behavior with the Amplify Swift repository?

@div5yesh
Copy link
Contributor

@TylerMcCraw Thanks for the update. We are working to reproduce this issue on our end so that we can determine the root cause.

@div5yesh
Copy link
Contributor

div5yesh commented Apr 7, 2023

@TylerMcCraw We were not able to replicate the issue. Some more info would help us debug this if this is still an issue for you.

  1. Can you share your user pool device tracking and MFA settings?
  2. Can you confirm whether rememberDevice call was a success?
  3. You comment here suggests that it is working as expected (that is, user is not prompted with challenge after remember device). Can you confirm if this is case?

@div5yesh div5yesh added the pending-community-response Issue is pending response from the issue requestor label Apr 7, 2023
@TylerMcCraw
Copy link
Author

  1. I will have to get back to you on this, but these settings are working fine for our iOS and Web applications which use their respective versions of Amplify SDKs
  2. rememberDevice is successful, yes.
  3. It is remembering the device yes, but the issue lies in the user flow after the rememberDevice() call that I mentioned in my original post.
  1. Call Amplify.Auth.rememberDevice()
  2. Call Amplify.Auth.signOut(options = AuthSignOutOptions.builder().globalSignOut(false).build())
  3. Call Amplify.Auth.signIn(username, password). Result returns CONFIRM_SIGN_IN_WITH_SMS_MFA_CODE again as next step. Result should have returned DONE 🐛

@div5yesh
Copy link
Contributor

div5yesh commented May 4, 2023

Can you enable the Android debugger Amplify.addPlugin(AndroidLoggingPlugin(LogLevel.VERBOSE) as the top level plugin and perform the steps you mentioned in the issue?

Paste the logs here with sensitive info removed. This would help us understand the issue better and reproduce it.

@TylerMcCraw
Copy link
Author

@div5yesh I'll get logs to you asap!

@TylerMcCraw
Copy link
Author

@div5yesh I've got a working reproduction of the issue with this project that I created for you here: https://github.com/TylerMcCraw/androidamplifyissue.
Please check it out when you can and follow the steps mentioned there.
Logging is turned on already for you.

@TylerMcCraw
Copy link
Author

@div5yesh @tylerjroach Hoping to ping you here so you know that there's a working project I posted above to reproduce this issue. The Issue has a pending-response label, but that's not necessarily true since I've provided all the info to reproduce.

@tylerjroach tylerjroach removed the pending-community-response Issue is pending response from the issue requestor label May 22, 2023
@tylerjroach
Copy link
Member

@TylerMcCraw Thanks for the ping. I know Divyesh has his own sample app that he was attempting to replicate this. We can investigate the sample provided to see if there are any differences that stand out.

In the meantime, the easiest way to investigate this issue may still be to provide the logs that @div5yesh requested so we can see exactly what is happening on your devices. It's possible the issue could exist in the service configuration and we may still not be able to easily replicate with the sample.

@TylerMcCraw
Copy link
Author

@tylerjroach @div5yesh
After spending a considerable amount of time in the debugger, I believe I've tracked down the cause of this issue.

Particularly, at this line of code.
The username in the initial signin event is the same as the username that's passed to Amplify.Auth.signin (in our case, an email address from the signin screen's "Username" input field).
When SRPCognitoActions.initiateSRPAuthAction() is eventually called, the username (still an email address, in our case) is used to look up the device metadata (and more importantly, the device key).
This device metadata is used to determine if the user ought to be challenged for MFA. If the device key is null, they will likely be challenged. And in our case, it's always null.
The reason the device key is always null is because at this line the USERNAME parameter returned is a UUID string (e.g. "7n1h6e06-7503-4b21-80ad-d52ae3769dv8") and NOT the email address from the original signin call. This UUID string username is then used for all credential store calls going forward, including the one that is used to store the device key after the user confirms with their 2FA code and rememberDevice() is called.

So, the lookup from EncryptedSharedPreferences via AuthEnvironment.getDeviceMetadata(username) to get the device key always uses the username passed in the Amplify.Auth.signin call (e.g. [email protected])
And the credentialStoreClient.storeCredentials call via SignInCognitoActions.confirmDevice to store the device key always uses the AWS username UUID string (e.g. "7n1h6e06-7503-4b21-80ad-d52ae3769dv8").

To sum it up, looking up the device key with "[email protected]" returns null because the device key is stored with "7n1h6e06-7503-4b21-80ad-d52ae3769dv8", therefore Amplify can't fetch the device key and therefore Amplify responds with a challenge step every time requiring the user to enter their 2FA code.

Possible Solution

If amplify-android were to use the AWS username UUID to look up the device metadata here instead of the username that's passed along from the Amplify.Auth.signin() call, then this step would likely be able to get the device key and then the user wouldn't be challenged again for the 2FA code.
I would open up a PR for this fix, but I can't think of a way to get the AWS username UUID string from the credentials that were passed in the original signin call.

@div5yesh
Copy link
Contributor

div5yesh commented Jul 12, 2023

@TylerMcCraw Thanks for the deep dive. 🙌

@TylerMcCraw
Copy link
Author

If you all could help me figure out a way to fetch the username UUID string that corresponds to the user attempting to signin with their username(non-UUID string)/password as described in the possible solution I mentioned above, then I'll put up a PR for this fix.

@nautilux2
Copy link

@div5yesh Is there any news from this? Or you can put a specific line of code and I can edit my sdk for hotfix?

@tungdagem
Copy link

I am also having the same problem as above, remembering the device on android is successful, but the next time I enter it, it still goes to the sms case

@nautilux2
Copy link

@TylerMcCraw Do you have any workaround solution for this case?

@tungdagem
Copy link

Unfortunately I'm also trying different methods and it still doesn't work, I see that every time I remember on the same machine, the device key is different.

@TylerMcCraw
Copy link
Author

TylerMcCraw commented Jul 25, 2023

@nautilux2
You can try to use the UDID of the user on subsequent logins to ensure the AWS SDK is looking up the device key using the right kind of username.

  • After the first login, store the AWS username UDID (Amplify.Auth.getCurrentUser().username) in sharedprefs (or some alternative local storage) mapped to the user's entered username.
  • And then on future logins, get the matching AWS UDID username to the user's entered username from sharedprefs when calling Amplify.Auth.signin() so that when SRPCognitoActions.initiateSRPAuthAction() is called in the SDK, it will find the device key because it's now using the username that was used to store the device key.

@tungdagem
Copy link

thanks you, I will try

@tungdagem
Copy link

it was work, thanks you @tylerjroach , @nautilux2

@gpanshu gpanshu closed this as completed Jul 25, 2023
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@TylerMcCraw
Copy link
Author

@gpanshu why was this ticket closed?
This is still an active bug.

@gpanshu gpanshu reopened this Jul 25, 2023
@tungdagem
Copy link

tungdagem commented Jul 26, 2023

i have called forget device function on my device but it failed error saying device key is null

@div5yesh
Copy link
Contributor

@TylerMcCraw This is the location in the code where user's device metadata is stored with username as the key. Note that signedInData contains both username and userId (UUID), so most probably this is where the fix would live.

Feel free to create a PR for any possible solution you have in mind.

@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth Related to the Auth category/plugins bug Something isn't working
Projects
None yet
8 participants