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

RefreshToken not present on refreshSession response #10319

Closed mwenko closed 2 years ago

mwenko commented 2 years ago

Before opening, please confirm:

JavaScript Framework

Not applicable

Amplify APIs

Authentication

Amplify Categories

auth

Environment information

``` # Put output below this line System: OS: macOS 12.4 CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz Memory: 668.83 MB / 16.00 GB Shell: 5.8.1 - /bin/zsh Binaries: Node: 16.14.2 - ~/.nvm/versions/node/v16.14.2/bin/node Yarn: 1.22.18 - ~/.yarn/bin/yarn npm: 8.5.0 - ~/.nvm/versions/node/v16.14.2/bin/npm npmPackages: <%= name %>: <%= version %> @nestjs/axios: 0.1.0 => 0.1.0 @nestjs/cli: 9.0.0 => 9.0.0 @nestjs/common: 9.0.8 => 9.0.8 (8.4.4) @nestjs/core: 9.0.8 => 9.0.8 (8.4.4) @nestjs/event-emitter: ^1.2.0 => 1.3.1 @nestjs/mongoose: 9.2.0 => 9.2.0 @nestjs/passport: 9.0.0 => 9.0.0 @nestjs/platform-express: 9.0.8 => 9.0.8 @nestjs/schedule: 2.1.0 => 2.1.0 @nestjs/schematics: 9.0.1 => 9.0.1 amazon-cognito-identity-js: 5.2.10 => 5.2.10 ```

Describe the bug

I'm using amazon-cognito-identity-js to refresh the AccessToken of a user. When executing the refreshSession function (CognitoUser) of amazon-cognito-identity-js the AccessToken & IdToken gets updated, but the RefreshToken property is not present in the AuthenticationResult.

Looking at the documentation https://docs.aws.amazon.com/cognito-user-identity-pools/latest/APIReference/API_InitiateAuth.html the response definitely should include a RefreshToken as well.

I debugged the refreshSession of amazon-cognito-identity-js and I don't get any new RefreshToken from it.

This is bad because if the RefreshToken never gets updated, we need to force the user to do a login (username + password) every time it expires. This is something that nobody likes.

I found a few related issues that are describing the same issue: https://github.com/aws/aws-sdk-js/issues/4156 https://stackoverflow.com/questions/55069851/how-to-get-refresh-token-auth-request-to-return-refreshtoken https://www.reddit.com/r/aws/comments/g0pkcd/how_to_renew_refreshtoken_in_cognito/

This seems like a bug in the API?

Expected behavior

Each time refreshSession is called it should give back an updated RefreshToken with an updated expirationTime (which is configured in CognitoUserPool.

Reproduction steps

1) Installing amazon-cognito-identity-js 2) Calling refreshSession() with valid paramters 3) RefreshToken that is passed back in the result is the same as the one in the request

Code Snippet

const refreshToken = new CognitoRefreshToken({ RefreshToken: data.token });

        return await new Promise((resolve, reject) => {
            user.refreshSession(refreshToken, (err, result) => {
                if (err) {
                    this.logger.warn(
                        `Refreshing cognito session failed for username ${data.name} and token ${
                            data.token
                        }. Used user pool id ${this.userPool.getUserPoolId()}`
                    );
                    reject(new CannotRefreshTokenException(data.name, data.token, err));
                } else {
                    resolve(result);
                }
            });
        });

Log output

Here is the result that `refreshSession()` gets from calling `API_InitiateAuth`, which should contain a RefreshToken property ![image](https://user-images.githubusercontent.com/22257587/190154186-2a135016-7e30-4dc0-8600-1b1c9da9fc66.png) Because no `RefreshToken` is present, the library always gives back the old `RefreshToken`: ![image](https://user-images.githubusercontent.com/22257587/190154307-69f4abde-0d4e-49a9-851a-18dfe943c6c4.png)

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

nadetastic commented 2 years ago

Hi @mwenko 👋,

Thanks for opening this issue. Out of curiosity, is there a requirement/reason to use amazon-cognito-identity-js and not Amplify Auth?

mwenko commented 2 years ago

Hi @mwenko 👋,

Thanks for opening this issue. Out of curiosity, is there a requirement/reason to use amazon-cognito-identity-js and not Amplify Auth?

Hi @nadetastic

There is no specific reason. But as Amplify Auth uses amazon-cognito-identity-js under the hood, using it probably wouldn't solve this issue.

nadetastic commented 2 years ago

@mwenko We have created a streamlined experience for our developers needing to use Authentication with Cognito using the Authentication library from Amplify, and recommend using Amplify Auth instead of the cognito-identity-js library.

While we do provide some occasional fixes/upgrades to the cognito-identity-js library, we are planning on deprecating that package in the near future.

With Amplify Auth, you can use Auth.currentSession() [1] to automatically refresh sessions.

You can opt in to use just the JS library without the Amplify CLI (https://docs.amplify.aws/lib/auth/start/q/platform/js/#re-use-existing-authentication-resource) if you feel like that would be better for your use case.

blwinters commented 2 years ago

@mwenko It seems that you are expecting the refreshToken to be refreshed, which to my understanding is not how OAuth works. The refreshToken has a permanent expiration date and when it expires, the user has to re-authenticate before they can receive a new refreshToken with a new expiration date. During the period when the refreshToken is not yet expired, calling refreshSession will generate a new accessToken with an updated expiration date, but the refreshToken never changes.

nadetastic commented 2 years ago

@mwenko following up on this, do you have any additional questions?

mwenko commented 2 years ago

@mwenko We have created a streamlined experience for our developers needing to use Authentication with Cognito using the Authentication library from Amplify, and recommend using Amplify Auth instead of the cognito-identity-js library.

While we do provide some occasional fixes/upgrades to the cognito-identity-js library, we are planning on deprecating that package in the near future.

With Amplify Auth, you can use Auth.currentSession() [1] to automatically refresh sessions.

You can opt in to use just the JS library without the Amplify CLI (https://docs.amplify.aws/lib/auth/start/q/platform/js/#re-use-existing-authentication-resource) if you feel like that would be better for your use case.

I'm gonna try Amplify Auth and see if this solves my issue. Thanks also to @nadetastic for providing further information. I just don't wanna logout my users if they are active on a daily basis. I expect them to stay logged-in basically forever, like in all other common apps, like facebook, whatsapp, ...

nadetastic commented 2 years ago

@mwenko Sounds good, I'll go ahead and close this issue out. If you have any questions with using Amplify Auth, feel free open a new issue.

flodaniel commented 2 years ago

Amplify Auth does not resolve the described issue, as it uses the same API (https://docs.aws.amazon.com/cognito-user-identity-pools/latest/APIReference/API_InitiateAuth.html with AuthFlow=REFRESH_TOKEN_AUTH. This is another issue that is reported in the github issues of client facing libraries (such as amplify-js), but is a server-side bug.

I suspect that this bug is forcing many developers to extend the lifetime of the refresh token to multiple users. Does the AWS/Cognito team not perceive this as a security threat for their customers? Especially, as one can now assume that most apps using cognito work with multi-year refresh tokens? I am not a security expert, so some guidance on the implications of using a multi-year refresh token would be greatly appreciated!

Should we report this bug once again directly in the AWS console, or how do we get a timeline or any insight on a potential update? Usually any cognito bugs die silently with a comment of "we are forwarding this to the cognito team", such as this (unrelated) ticket from a while back.

leonardomerlin commented 2 years ago

I'll write down just for the record. I believe this issue end up updating LocalStorage with empty value, and later on, when Auth.currentSession() is called, the app crashes.

I am in the middle of an investigation right now and couldn't make time to create reproducible example. Just wanted to share in case someone else is also facing this.

Log:

[DEBUG] 02:40.377 AuthClass - Getting current session
ConsoleLogger.ts:125 [DEBUG] 02:40.378 AuthClass - Failed to get the user session Error: Cannot retrieve a new session. Please authenticate.
    at CognitoUser2.getSession (CognitoUser.js:1320:25)
    at AuthClass2.<anonymous> (Auth.ts:1634:11)
    at step (Errors.ts:117:29)
    at Object.next (Errors.ts:117:29)
    at Errors.ts:117:29
    at new Promise (<anonymous>)
    at __awaiter3 (Errors.ts:117:29)
    at Auth.ts:1590:11
ConsoleLogger.ts:125 [DEBUG] 02:40.379 AuthClass - Failed to get the current user Error: Cannot retrieve a new session. Please authenticate.
    at CognitoUser2.getSession (CognitoUser.js:1320:25)
    at AuthClass2.<anonymous> (Auth.ts:1634:11)
    at step (Errors.ts:117:29)
    at Object.next (Errors.ts:117:29)
    at Errors.ts:117:29
    at new Promise (<anonymous>)
    at __awaiter3 (Errors.ts:117:29)
    at Auth.ts:1590:11

Workaround

try {
    await Auth.currentSession()
} catch (error) {
    // handle the issue here. For example: 
    Auth.signOut()
}