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

Users signed out randomly #14033

Open
3 tasks done
ab-bee opened this issue Nov 25, 2024 · 27 comments
Open
3 tasks done

Users signed out randomly #14033

ab-bee opened this issue Nov 25, 2024 · 27 comments
Labels
Auth Related to Auth components/category bug Something isn't working React Native React Native related issue

Comments

@ab-bee
Copy link

ab-bee commented Nov 25, 2024

Before opening, please confirm:

JavaScript Framework

React Native

Amplify APIs

Authentication

Amplify Version

v6

Amplify Categories

auth

Backend

None

Environment information

# Put output below this line
System:
    OS: macOS 15.0.1
    CPU: (8) arm64 Apple M2
    Memory: 42.89 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.18.0 - ~/.nvm/versions/node/v18.18.0/bin/node
    Yarn: 1.22.22 - ~/.yarn/bin/yarn
    npm: 10.8.3 - ~/.nvm/versions/node/v18.18.0/bin/npm
    pnpm: 9.14.2 - ~/.nvm/versions/node/v18.18.0/bin/pnpm
  Browsers:
    Safari: 18.0.1
  npmPackages:
    @aws-amplify/react-native: ^1.1.6 => 1.1.6 
    @babel/core: ^7.22.11 => 7.24.7 
    @babel/preset-env: ^7.22.11 => 7.24.7 
    @babel/runtime: ^7.22.11 => 7.24.7 
    @react-native-async-storage/async-storage: ^1.23.1 => 1.23.1 
    @react-native-community/datetimepicker: ^8.0.0 => 8.0.1 
    @react-native-community/netinfo: ^11.0.0 => 11.3.2 
    @react-native-firebase/analytics: ^19.1.1 => 19.3.0 
    @react-native-firebase/app: ^19.1.1 => 19.3.0 
    @react-native-firebase/dynamic-links: ^19.1.1 => 19.3.0 
    @react-native-picker/picker: ^2.4.9 => 2.7.7 
    @react-native/babel-preset: 0.73.21 => 0.73.21 
    @react-native/eslint-config: 0.73.2 => 0.73.2 
    @react-native/metro-config: 0.73.5 => 0.73.5 
    @react-native/typescript-config: 0.73.1 => 0.73.1 
    @react-navigation/core: ^6.2.2 => 6.2.2 
    @react-navigation/drawer: ^6.6.2 => 6.6.15 
    @react-navigation/native: ^6.0.11 => 6.0.11 
    @react-navigation/stack: ^6.2.2 => 6.2.2 
    @redux-devtools/extension: ^3.0.0 => 3.3.0 
    @redux-saga/testing-utils: ^1.1.3 => 1.1.5 
    @reduxjs/toolkit: ^1.9.5 => 1.9.7 
    @reduxjs/toolkit-query:  1.0.0 
    @reduxjs/toolkit-query-react:  1.0.0 
    @sentry/integrations: ^7.119.0 => 7.119.0 
    @sentry/react-native: ^5.33.1 => 5.33.1 
    @testing-library/jest-native: ^5.4.3 => 5.4.3 
    @testing-library/react-native: ^12.3.2 => 12.5.1 
    @types/base-64: ^1.0.0 => 1.0.2 
    @types/jest: ^29.2.1 => 29.5.12 
    @types/react: ^18.0.24 => 18.3.5 
    @types/react-dom: ^18.0.0 => 18.3.0 
    @types/react-native-vector-icons: ^6.4.10 => 6.4.18 
    @types/react-redux: 7.1.33 => 7.1.33 
    @types/react-test-renderer: ^18.0.0 => 18.0.0 
    @types/uuid: ^10.0.0 => 10.0.0 (9.0.8)
    @types/yup: ^0.29.14 => 0.29.14 
    @typescript-eslint/eslint-plugin: ^7.18.0 => 7.18.0 (5.62.0)
    @typescript-eslint/parser: ^7.18.0 => 7.18.0 (5.62.0)
    @welldone-software/why-did-you-render: ^7.0.1 => 7.0.1 
    HelloWorld:  0.0.1 
    LocationEnable:  0.0.1 
    SimpleEnable:  0.0.1 
    aws-amplify: ^6.9.0 => 6.9.0 
    aws-amplify/adapter-core:  undefined ()
    aws-amplify/analytics:  undefined ()
    aws-amplify/analytics/kinesis:  undefined ()
    aws-amplify/analytics/kinesis-firehose:  undefined ()
    aws-amplify/analytics/personalize:  undefined ()
    aws-amplify/analytics/pinpoint:  undefined ()
    aws-amplify/api:  undefined ()
    aws-amplify/api/server:  undefined ()
    aws-amplify/auth:  undefined ()
    aws-amplify/auth/cognito:  undefined ()
    aws-amplify/auth/cognito/server:  undefined ()
    aws-amplify/auth/enable-oauth-listener:  undefined ()
    aws-amplify/auth/server:  undefined ()
    aws-amplify/data:  undefined ()
    aws-amplify/data/server:  undefined ()
    aws-amplify/datastore:  undefined ()
    aws-amplify/in-app-messaging:  undefined ()
    aws-amplify/in-app-messaging/pinpoint:  undefined ()
    aws-amplify/push-notifications:  undefined ()
    aws-amplify/push-notifications/pinpoint:  undefined ()
    aws-amplify/storage:  undefined ()
    aws-amplify/storage/s3:  undefined ()
    aws-amplify/storage/s3/server:  undefined ()
    aws-amplify/storage/server:  undefined ()
    aws-amplify/utils:  undefined ()
    axios: ^1.5.0 => 1.5.0 
    babel-jest: ^29.6.3 => 29.7.0 
    base-64: ^1.0.0 => 1.0.0 
    date-fns: 2.30.0 => 2.30.0 
    date-fns-tz: 1.3.8 => 1.3.8 
    eslint: ^8.57.0 => 8.57.0 
    eslint-config-airbnb: ^19.0.4 => 19.0.4 
    eslint-config-airbnb-typescript: ^18.0.0 => 18.0.0 
    eslint-config-prettier: 8.10.0 => 8.10.0 
    eslint-import-resolver-typescript: ^2.7.1 => 2.7.1 
    eslint-plugin-eslint-comments: ^3.2.0 => 3.2.0 
    eslint-plugin-import: ^2.29.1 => 2.29.1 
    eslint-plugin-jest: ^28.8.2 => 28.8.2 (26.9.0)
    eslint-plugin-jsx-a11y: ^6.9.0 => 6.9.0 
    eslint-plugin-react: ^7.35.0 => 7.35.0 
    eslint-plugin-react-hooks: ^4.6.2 => 4.6.2 
    eslint-plugin-react-native: ^4.1.0 => 4.1.0 
    formik: ^2.4.6 => 2.4.6 
    jest: ^29.7.0 => 29.7.0 
    jest-environment-jsdom: ^29.2.1 => 29.7.0 
    jest-mock-extended: ^3.0.1 => 3.0.7 
    jest-transform-stub: ^2.0.0 => 2.0.0 
    mockdate: ^3.0.5 => 3.0.5 
    openapi-typescript: ^6.0.0-pre.0 => 6.7.6 
    patch-package: ^8.0.0 => 8.0.0 
    postinstall-postinstall: ^2.1.0 => 2.1.0 
    prettier: 2.8.8 => 2.8.8 
    react: 18.2.0 => 18.2.0 
    react-devtools: ^4.25.0 => 4.25.0 
    react-dom: 18.2.0 => 18.2.0 
    react-native: ^0.73.6 => 0.73.6 
    react-native-android-location-enabler: ^1.2.2 => 1.2.2 
    react-native-ble-plx: ^3.1.2 => 3.1.2 
    react-native-camera: ^4.2.1 => 4.2.1 
    react-native-code-push: ^8.2.1 => 8.2.2 
    react-native-config: ^1.4.5 => 1.4.5 
    react-native-devsettings: ^1.0.5 => 1.0.5 
    react-native-gesture-handler: ^2.16.0 => 2.16.2 
    react-native-get-random-values: 1.8.0 => 1.8.0 
    react-native-inappbrowser-reborn: ^3.7.0 => 3.7.0 
    react-native-keyboard-aware-scroll-view: ^0.9.5 => 0.9.5 
    react-native-localize: ^2.2.2 => 2.2.6 
    react-native-modal-datetime-picker: ^18.0.0 => 18.0.0 
    react-native-onesignal: ^5.0.4 => 5.2.2 
    react-native-pager-view: ^6.1.4 => 6.1.4 
    react-native-paper: ^5.12.3 => 5.12.3 
    react-native-permissions: ^4.1.5 => 4.1.5 
    react-native-qrcode-scanner: ^1.5.5 => 1.5.5 
    react-native-reanimated: ^3.8.1 => 3.8.1 
    react-native-safe-area-context: ^4.3.1 => 4.7.1 
    react-native-screens: ^3.29.0 => 3.29.0 
    react-native-svg: ^13.4.0 => 13.13.0 
    react-native-svg-transformer: ^1.3.0 => 1.3.0 
    react-native-swipe-list-view: ^3.2.9 => 3.2.9 
    react-native-tab-view: ^3.5.2 => 3.5.2 
    react-native-url-polyfill: ^2.0.0 => 2.0.0 
    react-native-vector-icons: ^10.0.0 => 10.0.0 
    react-redux: ^8.1.2 => 8.1.3 
    react-test-renderer: 18.2.0 => 18.2.0 
    react-uid: ^2.3.2 => 2.3.3 
    redux: ^4.0.1 => 4.2.1 
    redux-saga: ^1.1.1 => 1.1.3 
    redux-saga/effects:  undefined ()
    semver: ^7.6.3 => 7.6.3 (6.3.1, 7.5.4, 7.3.7, 5.7.1, 7.6.2)
    typescript: 5.0.4 => 5.0.4 
    uuid: ^10.0.0 => 10.0.0 (9.0.1, 7.0.3)
    victory-native: 36.9.2 => 36.9.2 
    yup: ^0.30.0 => 0.30.0 
  npmGlobalPackages:
    corepack: 0.19.0
    npm: 10.8.3


Describe the bug

Users are signed out before their refresh token expires if they lose internet connection.

There are no tokens in local storage when they're signed out.

Using the latest AWS Amplify version (v6.9.0).

Expected behavior

Users remain signed in until their refresh token expires.

Reproduction steps

  1. Set access and ID token expiration to 5 minutes
  2. Set refresh token expiration to 60 minutes
  3. Sign in, disable the internet connection, and wait 5-10 minutes
  4. You should be signed out and see no tokens in local storage, if using react-native-async-storage

Log output

// Put your logs below this line

# Before user signed out:
{
    "@MemoryStorage:CognitoIdentityServiceProvider.61bvred0ajga966hutm2ahcls9.LastAuthUser": {
        "content": "<redacted-email>"
    },
    "@MemoryStorage:CognitoIdentityServiceProvider.61bvred0ajga966hutm2ahcls9.<redacted-email>": {
        "content": "<redacted-token>"
    },
    "@MemoryStorage:CognitoIdentityServiceProvider.61bvred0ajga966hutm2ahcls9.<redacted-email>.clockDrift": {
        "content": "37"
    },
    "@MemoryStorage:CognitoIdentityServiceProvider.61bvred0ajga966hutm2ahcls9.<redacted-email>.idToken": {
        "content": "<redacted-token>"
    },
    "@MemoryStorage:CognitoIdentityServiceProvider.61bvred0ajga966hutm2ahcls9.<redacted-email>.refreshToken": {
        "content": "<redacted-token>"
    },
    "@MemoryStorage:CognitoIdentityServiceProvider.61bvred0ajga966hutm2ahcls9.<redacted-email>.signInDetails": {
        "content": "{\"loginId\":\"<redacted-email>\",\"authFlowType\":\"USER_SRP_AUTH\"}"
    },
    "@RNDS/isDebuggingRemotely": {
        "content": "true"
    }
}

# After user signed out:
{
    "@RNDS/isDebuggingRemotely": {
        "content": "true"
    }
}

Manual configuration

{
    Auth: {
      Cognito: {
        userPoolId: <redacted>,
        userPoolClientId: <redacted>,
      },
    },
    Geo: { LocationService: { region: 'ap-southeast-2' } },
}

Mobile Operating System

Android

@github-actions github-actions bot added pending-triage Issue is pending triage pending-maintainer-response Issue is pending a response from the Amplify team. labels Nov 25, 2024
@chrisbonifacio chrisbonifacio added Auth Related to Auth components/category React Native React Native related issue labels Nov 25, 2024
@cwomack cwomack self-assigned this Nov 25, 2024
@HuiSF
Copy link
Member

HuiSF commented Nov 25, 2024

Hi @ab-bee, thanks for reporting this issue we will start investigation.

Also could you clarify:

see no tokens in local storage

by local storage, you mean the underlying AsyncStorage of your react-native app?

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Nov 25, 2024
@HuiSF
Copy link
Member

HuiSF commented Nov 25, 2024

Hi @ab-bee I couldn't reproduce this error in a react-native app that's running on Android.
I also verified that when no network connection, the underlying refresh tokens service calls failed due to NetworkError, but the expired tokens remain in the AsyncStorage along with the effective refresh token. And when the device regain network, refresh token worked correctly.

We recently fixed (v6.7.0) a bug that disturbed the above flows and causing the tokens to be cleared. If you upgraded aws-amplify from an older version, could you try to clean all build cache and try again?

@ab-bee
Copy link
Author

ab-bee commented Nov 25, 2024

Hi @ab-bee, thanks for reporting this issue we will start investigation.

Also could you clarify:

see no tokens in local storage

by local storage, you mean the underlying AsyncStorage of your react-native app?

Thank you!
Yes, that's right. We're using @react-native-async-storage/async-storage for this.

@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Nov 25, 2024
@HuiSF
Copy link
Member

HuiSF commented Nov 25, 2024

Thank you for confirming @ab-bee. Were you able to verify the suggestion I gave above?

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Nov 25, 2024
@HuiSF HuiSF added question General question and removed pending-triage Issue is pending triage labels Nov 25, 2024
@ab-bee
Copy link
Author

ab-bee commented Nov 25, 2024

Thank you for confirming @ab-bee. Were you able to verify the suggestion I gave above?

Hi again, I have cleared out the node_modules and build cache and can't reproduce this again. Will close the issue, thank you.

@ab-bee ab-bee closed this as completed Nov 25, 2024
@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Nov 25, 2024
@ab-bee
Copy link
Author

ab-bee commented Nov 26, 2024

Re-opening this as one of our testers just got logged out, except they didn't lose internet connection so that may have been a red herring.

@ab-bee ab-bee reopened this Nov 26, 2024
@HuiSF
Copy link
Member

HuiSF commented Nov 26, 2024

Hi @ab-bee could you describe the preconditions of the situation described in your last comment without losing network connection? Is this consistently reproducible and what are the reproducing steps?

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Nov 26, 2024
@cwomack cwomack added the pending-community-response Issue is pending a response from the author or community. label Nov 26, 2024
@ab-bee
Copy link
Author

ab-bee commented Nov 26, 2024

Hi @ab-bee could you describe the preconditions of the situation described in your last comment without losing network connection? Is this consistently reproducible and what are the reproducing steps?

Hi @HuiSF,

This is what we did:

  • Set the refresh token expiry to a long time (10 years)
  • Set the access and ID token expiry period to an hour
  • Had three people try logging into their accounts (two on Android, one on iPhone)
  • Waited an hour
  • Had those people check if they were still logged in

Only one user (one of the Android users) had been logged out, so it's not consistently reproducible unfortunately :(

@github-actions github-actions bot added pending-maintainer-response Issue is pending a response from the Amplify team. and removed pending-community-response Issue is pending a response from the author or community. labels Nov 26, 2024
@HuiSF
Copy link
Member

HuiSF commented Nov 26, 2024

Thanks for the follow @ab-bee

Had those people check if they were still logged in

Do you know what exactly did they do to validate this? (for example actions triggered an Amplify library API call, and which API etc.)

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Nov 26, 2024
@ab-bee
Copy link
Author

ab-bee commented Nov 27, 2024

Thanks for the follow @ab-bee

Had those people check if they were still logged in

Do you know what exactly did they do to validate this? (for example actions triggered an Amplify library API call, and which API etc.)

They opened the app from the background and that triggers an API call to our back-end with the user's access token as authorization. We call AWS Amplify's Auth.fetchAuthSession() to get the access token.

Also one of the other people just tried to open the app to check again and they were also logged out.

Other details:

@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Nov 27, 2024
@ab-bee ab-bee changed the title Users signed out if internet connection lost Users signed out randomly Nov 27, 2024
@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Nov 28, 2024
@ab-bee
Copy link
Author

ab-bee commented Nov 28, 2024

TY for the additional details, we will attempt to reproduce this issue.
Updated: been testing continuously in the past 4 hour or so - couldn't reproduce this when calling fetchAuthSession() after the access token became expired.

Hi again,

We've just had to revert the library back to v5 because we previously weren't experiencing the issue on that version, but we are now experiencing the issue on v5 as well 😓

I think I just reproduced the issue with the Amplify logging on (Amplify.Logger.LOG_LEVEL = 'DEBUG';). This is on Amplify v5.
These logs look to be of particular relevance:

 LOG  [DEBUG] 15:38.645 AuthClass - getting current authenticated user
 LOG  [DEBUG] 15:39.413 AuthClass - Getting current session
 LOG  [DEBUG] 15:39.628 AuthClass - cannot load federated user from auth storage
 LOG  [DEBUG] 15:39.629 AuthClass - get current authenticated userpool user

We found it strange that the log referred to a 'federated user', because we don't use federated identities in our app.

There's then a log saying 'Succeed to get the user session' (with the session data included below), however, an API call that used the token failed with an 'unauthenticated' error, so it seems that the token is actually invalid.

This error was thrown from the cognito-express library which is being used on our backend to validate the token against the user pool.

{
  "accessToken": {
    "jwtToken": "<redacted>",
    "payload": {
      "auth_time": 1732760460,
      "client_id": "<redacted>",
      "cognito:groups": [Array],
      "event_id": "<redacted>",
      "exp": 1732763718,
      "iat": 1732763418,
      "iss": "https://cognito-idp.ap-southeast-2.amazonaws.com/<redacted>",
      "jti": "<redacted>",
      "origin_jti": "<redacted>",
      "scope": "aws.cognito.signin.user.admin",
      "sub": "<redacted>",
      "token_use": "access",
      "username": "<redacted>"
    }
  },
  "clockDrift": 61151,
  "idToken": {
    "jwtToken": "<redacted>",
    "payload": {
      "aud": "<redacted>",
      "auth_time": 1732760460,
      "cognito:groups": [Array],
      "cognito:username": "<redacted>",
      "email": "<redacted>",
      "email_verified": true,
      "event_id": "42580feb-21eb-4ec8-be68-cc833fe61376",
      "exp": 1732763718,
      "iat": 1732763418,
      "iss": "https://cognito-idp.ap-southeast-2.amazonaws.com/<redacted>",
      "jti": "<redacted>",
      "origin_jti": "<redacted>",
      "sub": "<redacted>",
      "token_use": "id"
    }
  },
  "refreshToken": {
    "token": "<redacted>"
  }
}

I also just noticed that when the access token expiry period is set to 5 minutes, after 5 minutes it successfully updates to the next 5 minutes and I don't get logged out. It's only after an hour that I get logged out.

The cognito-express library has it's own separate access token expiry period for some reason, which is an hour. I thought this might've been why I was getting logged out, but I changed this to 5 minutes and was still only logged out after over an hour.

@adithyavis
Copy link

@HuiSF We have been experiencing this in our project as well. We are unable to reproduce it, but we get around 40-50 cases every day, which adds up quickly.

We are using v6.6.5 of the package and the package defaults to async-storage for persisting tokens.

"aws-amplify": "6.6.5",
 "@aws-amplify/react-native": "^1.1.6",
"react-native-get-random-values": "^1.11.0",
"@react-native-async-storage/async-storage": "^1.24.0",
"@react-native-community/netinfo": "^11.3.2",

For the above package version configuration, we noticed that the values stored in async storage are either empty or incomplete (for eg, has idToken but no refreshToken, or has refreshToken and no idToken).
We also persist the idToken manually in mmkv, and this gets persisted without issues.

I wonder if this is an issue with how aws-amplify handles the storage of tokens to async-storage, or is it an issue with how async-storage manages the stored values.

@ab-bee
Copy link
Author

ab-bee commented Nov 28, 2024

I'm trying to reproduce the issue again and noticed that this time, while I got the log 'AuthClass - cannot load federated user from auth storage', I have not been logged out.

I then got a tokenRefresh from the Amplify Hub events, so it seems that this refresh token process was successful.

So I'm not sure if the 'AuthClass - cannot load federated user from auth storage' log is relevant to why I was logged out last time.

@ab-bee
Copy link
Author

ab-bee commented Dec 2, 2024

Just an update, I logged the access token and refresh token that were being stored in async storage, and tried decoding them.
The access token was valid and decodable but the refresh token was not valid, and so couldn't be decoded.
Just thought I'd add this information to the ticket as it seems like it might be relevant.

edit: Just checked and I have the same invalid refresh token in one of our other apps where we're not experiencing the random logouts. So maybe that's a red herring 😭

In the app that does have the random signouts issue, we use AsyncStorage for a few other things, whereas in the other app (that doesn't have the issue) we only use it for AWS Amplify. Again, not sure if that's relevant but thought it was worth mentioning in case it's a clue.

@HuiSF
Copy link
Member

HuiSF commented Dec 2, 2024

Hi @adithyavis if you are experience the same issue on Android, please upgrade Amplify library to the latest version. We fixed a known issue that refresh token may fail without retry since version 6.7.0. The unexpected failure may wrongly clear tokens.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Dec 2, 2024
@HuiSF
Copy link
Member

HuiSF commented Dec 2, 2024

Hi @ab-bee when you reverted back to v5, what version did you use? We fix the same issue described above with version 5.3.25.

The access token was valid and decodable but the refresh token was not valid, and so couldn't be decoded.

To clarify, refresh token is service encrypted by Cognito, you are unable to decode it by yourself.

@ab-bee
Copy link
Author

ab-bee commented Dec 2, 2024

Hi @ab-bee when you reverted back to v5, what version did you use? We fix the same issue described above with version 5.3.25.

The access token was valid and decodable but the refresh token was not valid, and so couldn't be decoded.

To clarify, refresh token is service encrypted by Cognito, you are unable to decode it by yourself.

Interesting, I reverted back to the version we were previously on which was v5.3.19.
I will try updating to v5.3.25 and see if we still get the issue. Thank you for the update!

@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Dec 2, 2024
@HuiSF HuiSF removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Dec 3, 2024
@itsramiel
Copy link
Contributor

Jumping in here to not create a duplicate issue and validate my findings with others.

In my team, we are also experiencing this and I believe I know why.

The situation is similar to what others have experienced. It happens when the access token is expired and is being refreshed. In our app, we call getCurrentUser when the app comes to the foreground and we also fire consequent network requests which call fetchAuthSession to retrieve the access tokens to send in the request's Auth headers.

tl;dr, Both getCurrentUser and fetchAuthSession refresh the tokens consequently(Race Condition).
-> First methods gets new tokens, and when setting the new ones, it first clears the old ones -> calls await methods allowing other code to run
-> Second reads the cleared tokens before the first was able to set them -> calls await, letting other code run
-> First method actually sets the new tokens
-> Second throws an error that there are no tokens and clears the store tokens

Detailed Step by Step

getCurrentUser would get the tokens to validate the current user:

const tokens = await amplify.Auth.getTokens({ forceRefresh: false });

Finding out that the tokens are expired and attempt to refresh the token, once the new tokens are available, the tokens are then stored in local storage.

Now here comes the issue

When storing the new tokens, the first thing done is all tokens are cleared <--- BUG:

async storeTokens(tokens: CognitoAuthTokens): Promise<void> {
assert(tokens !== undefined, TokenProviderErrorCode.InvalidAuthTokens);
await this.clearTokens();

Followed by await call to the the lastAuthUser:

const lastAuthUser = tokens.username;
await this.getKeyValueStorage().setItem(
this.getLastAuthUserKey(),
lastAuthUser,
);

Now because this is an async call, it now allows other code to run -> the fetchAuthSession which again gets the tokens:

const tokens = await this.getTokens(options);

Now when the tokens are trying to be retrieved, the refreshToken would not be in local storage because of the earlier clearing of the tokens

Down the road would be an await call which would allow execution to go back to setting the tokens from the getCurrentUser and tokens are saved correctly but too late because another method now thinks there are no tokens.

Because there is no refresh token, the following is called:

private handleErrors(err: unknown) {
assertServiceError(err);
if (err.name !== AmplifyErrorCode.NetworkError) {
// TODO(v6): Check errors on client
this.clearTokens();

Which resets the correctly stored tokens -> Now the user is in a logged out state

Solution

There has been a pr submitted to address a different issue but is also applicable here, where basically tokens are not being completely reset when storing them, but instead only update/delete updated/removed values

@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Dec 6, 2024
@HuiSF
Copy link
Member

HuiSF commented Dec 6, 2024

Hi @itsramiel thanks for the detail analysis - we are looking into this.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Dec 6, 2024
@cshfang
Copy link
Member

cshfang commented Dec 19, 2024

@itsramiel - Really appreciate the deep dive and it gave us a great starting point into our investigation.

We are evaluating the PR mentioned above but would like to also root cause this issue and I'm finding it difficult to reproduce and would appreciate it if we could get some additional info about exactly which error is causing this token clearance to be triggered. The issue I'm running into in my repro is related to this comment:

Now when the tokens are trying to be retrieved, the refreshToken would not be in local storage because of the earlier clearing of the tokens

During this fetchAuthSession call - if the tokens are no longer in the store (due to the first call clearing them out and having not yet repopulated them yet) - it should not itself try to refresh tokens (and therefore not throw any sort of refresh errors) due to an early return:

tokens = await this.getTokenStore().loadTokens();
const username = await this.getTokenStore().getLastAuthUser();
if (tokens === null) {
return null;
}

I'm wondering if you can help confirm that the clearTokens() here:

private handleErrors(err: unknown) {
assertServiceError(err);
if (err.name !== AmplifyErrorCode.NetworkError) {
// TODO(v6): Check errors on client
this.clearTokens();

Is indeed being called by attaching a Hub listener for the failure event associated below:

Hub.dispatch(
'auth',
{
event: 'tokenRefresh_failure',
data: { error: err },
},
'Auth',
AMPLIFY_SYMBOL,
);

And could you also tell me if you are ever calling fetchAuthSession with the forceRefresh option enabled?

Edit: FWIW, I do believe there could be a race condition in all this, I am just trying to lock down exactly where

@itsramiel
Copy link
Contributor

itsramiel commented Dec 20, 2024

Hi @cshfang

Yes you are right to think that this will early return:

tokens = await this.getTokenStore().loadTokens();
const username = await this.getTokenStore().getLastAuthUser();
if (tokens === null) {
return null;
}

but actually the error doesnt occur here. The tokens here are being successfully fetched before the call to clearTokens.

The problem is that by the time that the return values are returned, processed, and figured out that the accessToken is expired, then refreshToken then is fetched. And by that time, the tokens were cleared already.

Yes the tokenRefresh_failure was called after a successfull tokenRefresh. I was logging the events from the Hub and that's what helped me narrow it:
image

And could you also tell me if you are ever calling fetchAuthSession with the forceRefresh option enabled?

Never. I let amplify decide when to do it.

Edit: FWIW, I do believe there could be a race condition in all this, I am just trying to lock down exactly where

Yes I am 100% confident. I reproduced it multiple times. You can't reproduce it every time because it all depends on the async nature of js and how sync function are called when async functions are awaited

@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Dec 20, 2024
@cwomack
Copy link
Member

cwomack commented Dec 20, 2024

@itsramiel, appreciate the responses and details here. Upgrading this issue to a bug as we look into it further.

@cwomack cwomack added bug Something isn't working and removed question General question labels Dec 20, 2024
@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Dec 20, 2024
@cwomack cwomack removed their assignment Dec 20, 2024
@cshfang
Copy link
Member

cshfang commented Dec 20, 2024

@itsramiel Gotcha, I think your reply was super helpful and got me thinking that, crucially, the loadTokens function itself is peppered with awaits which could result in a fetchAuthSession call to obtain the same expired access token as a previous call but be missing the refresh token.

I am now able to reproduce such a case reliably and will be working with the team on addressing the root cause. In the meantime, I can see how the proposed PR would also serve to mitigate this and we'll continue working to get that through.

@itsramiel
Copy link
Contributor

Is the local storage methods async because of react native async storage? I know web's is sync. Because in react native, the kinda new standard in react-native-mmkv which is sync. If rn was the only reason why storage was async then it can be changed into sync and all the peppered await would be removed

@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Dec 21, 2024
@cwomack
Copy link
Member

cwomack commented Dec 23, 2024

@itsramiel we use async storage operations to support React Native's AsyncStorage, but we're still looking into the root cause of the race condition. We will update this as soon as we can and appreciate the patience.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auth Related to Auth components/category bug Something isn't working React Native React Native related issue
Projects
None yet
Development

No branches or pull requests

7 participants