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

Refresh token not loaded from storage if access token is expired #154

Closed
leonluc-dev opened this issue Sep 15, 2022 · 14 comments
Closed

Refresh token not loaded from storage if access token is expired #154

leonluc-dev opened this issue Sep 15, 2022 · 14 comments

Comments

@leonluc-dev
Copy link
Contributor

leonluc-dev commented Sep 15, 2022

v0.9.0 now checks if the accesstoken in tokenResponse is expired before it loads the tokenResponse from storage.
This introduced a bug that the refresh token (which is part of the same tokenResponse) is not loaded if the access token is expired. This means any call to refreshToken() (to renew the expired access token) will fail.

@richardkshergold
Copy link

richardkshergold commented Sep 15, 2022

Do you think that could in any way link to the problem I have been seeing with getValidToken() with version 0.9.0? #151

@leonluc-dev
Copy link
Contributor Author

leonluc-dev commented Sep 15, 2022

@richardkshergold This seems to be related yes. In your case the access tokens seem to expire after 24 hours or so.
The getValidToken() method is supposed to refresh the access token if it's expired, but since the refreshToken is not loaded from the storage, it fails and you won't be able to get a valid token.

@leonluc-dev
Copy link
Contributor Author

I've created a PR #155 that moves the expiration check.
This ensures the TokenResponse is still properly loaded to refresh the token, but the isAuthenticated is false when the token is expired (as PR #123 intended)

@richardshergold
Copy link

Hey, sorry to post this in here @leonluc-dev but it’s the only way I had of contacting you? I just wondered if you had come across this problem on any Android 11 devices? https://forum.ionicframework.com/t/android-11-capacitor-secure-storage-plugin-issue/224413

@leonluc-dev
Copy link
Contributor Author

@richardshergold I haven't done any recent testing with Android 11 so I wouldn't be able to tell you.
At the very least it seems unrelated to the tokenResponse issue discussed here and seems more related to something in the Capacitor framework or native parts of Android itself.

@richardkshergold
Copy link

richardkshergold commented Sep 17, 2022

@mraible and @leonluc-dev - I've updated to the latest version, well I assume I have. I have this in my package.json:

"ionic-appauth": "github:wi3land/ionic-appauth"

and I ran

npm install

Early results (I think) show that the app is still occasionally unable to get the token. On app startup I have some code that checks to see if a valid token can be obtained. If it cannot, the user is presented with the Okta login screen, otherwise the app continues.

I installed the new version initially on an iPhone SE (iOS 16). I installed and logged around 16:00. Then I closed and re-opened the app every hour or so until 22.30 when it suddenly re-presented the login page. I logged in again and the following day checked every hour again. It worked continuously until around 14:00 when it failed again and presented the login screen.

Note that this problem NEVER occurs when the app restarts with the onResume event - in that situation the app always re-opens and gets the token ok. This problem ONLY occurs when the app is opened and instead of onResume, it starts up from scratch i.e when the operating system has closed down the app (presumably due to memory resources etc etc etc).

On our Okta config we have tokens that expire after 6 months so (unless I am misunderstanding something) the above should not happen - not until the 6 months is up anyway!

I have just installed the app on a few other devices:

  • iPhone 11 running iOS 15
  • Pixel 4 running Android 13
  • Samsung Galaxy A10 running Android 11

I will report back with results. Note that on Android 11 if you exit the app rather than close it (move it into background) the app ALWAYS loses the token i.e getValidToken fails on startup EVERY time I do this. I can only assume this is due to some problem with ionic/capacitor storage on Android 11 (or certain devices) which is why I posted this in the Ionic forum.

Thanks for all your help with this.

@richardshergold
Copy link

Update. I still have this issue which I am ONLY seeing on an iPhone SE (my iPhone 11 is fine). This issue occurs when the app is restarted but ONLY when it is restarted because the OS has decided a restart is required. IMPORTANT this error has nothing to do with the latest version of this library. I did some tests on our live app (which uses the previous Capacitor 3 version) and the same issue occurs on there (again only on the iPhone SE, not on the iPhone 11). Another question asked over here! https://forum.ionicframework.com/t/difference-between-normal-ios-app-startup-and-o-s-forced-startup/226978

@richardkshergold
Copy link

richardkshergold commented Sep 20, 2022

Additional Update: my iPhone SE error occurs on the refreshToken call I think because if I shut down my phone and re-start the app everything works ok. But if I let the refresh token expire (our refresh tokens last 2 hours) and then repeat the same test I get the "Unable To Obtain Valid Token". I've not tried passing a buffer value to this call - does anyone have any idea what the purpose of the buffer value is (default value is -600) ?

getValidToken(buffer = AUTH_EXPIRY_BUFFER) {
        return __awaiter(this, void 0, void 0, function* () {
            if (this._tokenSubject.value) {
                if (!this._tokenSubject.value.isValid(buffer)) {
                    yield this.refreshToken();
                    if (this._tokenSubject.value) {
                        return this._tokenSubject.value;
                    }
                }
                else {
                    return this._tokenSubject.value;
                }
            }
            throw new Error("Unable To Obtain Valid Token");
        });
    }

Worth remembering - if I close and open the app myself after the 2 hour period then there is no error and also if I kill the app manually there is no error - the error only ever occurs if the app is being opened again after the OS has automatically closed the app down forcing an app re-start

@leonluc-dev
Copy link
Contributor Author

@richardshergold Your issue doesn't seem to be related to the issue being tracked here, so it's probably better to open a separate issue for this as to not clutter up this one. But still:

As far as I know, the buffer value is the "strictness/leniency" for checking the expiry time on the access token.
For example, if a token expires at 16:00 and the buffer is set to 600 (600 seconds/10 minutes) it means it will still consider the token valid until 16:10. A value of -600 will do the reverse (make the token valid until 15:50). The latter is probably the default in GetValidToken to make sure you don't get a token that will expire within just 10 minutes.

There seems to be an issue with your Okta configuration as well. As you stated earlier your access tokens are valid for 6 months, yet you say the refresh token is valid for only 2 hours. Refresh tokens are supposed to be valid longer than the access tokens (since they are meant to refresh expired access tokens). This can cause issues in the refresh cycle. In most cases access tokens are valid for only an hour or so, while refresh tokens tend to be valid for multiple weeks or even months.

All that said however, it seems your access token is not loaded from storage properly and therefore the refresh token flow is started. Since the refresh token is expired now, it will fail.
This can either mean Init is not called properly before you try to retrieve a token, or there is a issue with the storage provider.

@richardkshergold
Copy link

@leonluc-dev thank you for that - yes I did wonder if these comments are probably in the wrong place now, sorry.

Thanks for your explanation of the buffer. I will check with our Okta Admin guys re the refresh and access tokens.

And yes the app calls init() as soon as it starts and in fact I also check that the service is initialises and (in logs prior to the error) I can see that it is. So I'm wondering - like you - if this is some storage issue which only occurs in this scenario (and maybe only on the iPhone SE)

@richardkshergold
Copy link

richardkshergold commented Sep 20, 2022

I think I probably got the two terms muddled up and it's the access token that lasts 2 hours and the refresh token that lasts 6 months, in which case my earlier comment was probably incorrect about the refreshToken call failing. Thanks again.

@leonluc-dev
Copy link
Contributor Author

leonluc-dev commented Sep 20, 2022

@richardshergold Also, you might want to wait till this issue's fix is properly published as a package version.

Currently importing the package like this: ionic-appauth": "github:wi3land/ionic-appauth does not take the most recent typescript changes into account. Since most of the changes needed to fix this issue involve typescript, the fixes might not have processed for you.

@richardkshergold
Copy link

Ahh! Ok, I had not realised that. Do we have any idea when that might be?

@leonluc-dev
Copy link
Contributor Author

@richardshergold Not sure. But in the meantime you can do the following:

  1. Clone the repostory using git clone https://github.com/wi3land/ionic-appauth.git
  2. Run the npm i and npm run build commands in the cloned repository so that the typescript is properly build
  3. Reference the local appauth folder from your project "ionic-appauth": "file:/full/path/to/ionic-appauth"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants