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.44k stars 2.13k forks source link

oauthTokenRefreshException thrown but not using oauth flow #13168

Closed clement-faure closed 7 months ago

clement-faure commented 7 months ago

Before opening, please confirm:

JavaScript Framework

Next.js

Amplify APIs

Authentication

Amplify Version

v6

Amplify Categories

auth

Backend

None

Environment information

``` # Put output below this line ```

Describe the bug

This error is thrown on my production environment, but i have never use oauth flow.

Capture d’écran 2024-03-25 à 10 36 14

Expected behavior

Token refresh should refresh normally.

Reproduction steps

1) Upgrade from amplify v5 to v6 on authentication flow 2) Randomly get refresh token error regarding oauth flow

Code Snippet

// Put your code below this line.

Log output

Capture d’écran 2024-03-25 à 10 52 12

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

israx commented 7 months ago

hello @clement-faure . Can you please expand how you are able to reproduce this issue ? Plase share,

israx commented 7 months ago

the OAuthTokenRefreshException is usually thrown when storage doesn't have a refresh_token . Can you make sure tokens are stored in the cookies ?

clement-faure commented 7 months ago

Thank you for your quick response !

Amplify configuration :

Capture d’écran 2024-03-25 à 14 09 45

Amplify storage mode (default localeStorage) :

Capture d’écran 2024-03-25 à 14 10 26

When use click on remember checkbox true, storage in persisted in local storage, otherwise stored in cookies for session

clement-faure commented 7 months ago

I was not able to reproduce it on my own, i have a production environment with +1000 customer, and i got this error in my Sentry. This error happened after migrating to aws amplify v6

israx commented 7 months ago

I think the library might not being able to pull in the tokens from the current storage mechanism. Hence the error showed. Can you run your application and do the following,

  1. authenticate.
  2. call fetchAuthSession with the forceRefresh flag enabled.
  3. check if the error was thrown
  4. check where the tokens are stored

In addition to that, we added additional support for Next-JS as well and the configuration requires to pass the SSR flag.

Amplify.configure(config, {
  ssr: true // required when using Amplify with Next.js
});
clement-faure commented 7 months ago

I'm gonna try that right away. I'm not using SSR on my application, do i have to enabled SSR flag necessarily ? Amplify is only used on client side environment.

clement-faure commented 7 months ago

I confirm that all is working fine when reproducing your steps. Tokens are stored in local storage when "remember me" checkbox is checked, otherwise in cookies (like i've implement in my updateCognitoTokenSavingMechanism)

I think the error is linked to aws-amplify upgrade, my customer probably logged in on aws amplify v5, and when reloading app and refreshing token on aws amplify v6, the error has been thrown ? But it's not happening to all my customers.. very strange.

israx commented 7 months ago

If Amplify is used only on the client side, then you wouldn't need the SSR flag.

Strange, we kept the same key format between v5 and v6, so the refresh token should be there. Probably asking the end user to delete their keys and authenticate again might help.

clement-faure commented 7 months ago

Thank you for your response. I will let you know in the incoming weeks if happening regularly !

cwomack commented 7 months ago

@clement-faure, I'll put the pending-response label on this issue for now until we hear back from you on implementing @israx's guidance above.

cwomack commented 7 months ago

@clement-faure, wanted to check in and see if you're still experiencing this. Let us know if you are!

clement-faure commented 7 months ago

put the pending-response label on this issue for now until we hear back from you on implementing @israx's guidance above.

The issue didn't happened again so i think we can close this issue safely !

cwomack commented 7 months ago

@clement-faure, appreciate the quick reply! I'll close this issue then for now, but feel free to leave further comments or context if this persists.