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

Two token refresh requests use the same refresh token #733

Closed
goflo opened this issue Mar 5, 2020 · 7 comments
Closed

Two token refresh requests use the same refresh token #733

goflo opened this issue Mar 5, 2020 · 7 comments
Labels
more-info-needed Please provide a minimal example (e.g. at stackblitz.com) which demonstrates the issue

Comments

@goflo
Copy link

goflo commented Mar 5, 2020

Describe the bug
We are using IdentityServer4 with the angular-oauth2-oidc library. Authorization is done using Code-Flow and automatic refresh is enabled calling setupAutomaticSilentRefresh method. Login is working fine. Before the token expires the automatic refresh kicks in and refreshes the token. Normally that works fine. We get a new token and everything works. But sometimes two refresh token requests (R1 and R2) are sent at the same point in time and both use the same refresh token (RT). Request R1 is successful and we get a new fresh token from IdentityServer. But R2 fails because IdentityServer already generated a new refrsh token and the old refresh token (RT) is not valid anymore! The following request are working fine again.

Anyone who knows that issue? Any ideas how to solve it?
Do I need the silent-refresh.html page and setting the silentRefreshRedirectUri (for code-flow!) as described in this example: https://github.com/jeroenheijmans/sample-auth0-angular-oauth2-oidc

To Reproduce
Steps to reproduce the behavior:
I haven't found a way to reproduce it. I just do the following steps:

  1. Login
  2. Wait some time
  3. After a few successful refresh token requests maybe the error occurs (but only maybe)

Desktop (please complete the following information):

IdentityServer Log:

dbug: IdentityServer4.Validation.TokenRequestValidator[0]
      Start validation of refresh token request
dbug: IdentityServer4.Stores.DefaultRefreshTokenStore[0]
      refresh_token grant with value: d52907154ce4ad550a96a49a57cffc5ec91b0040fa46b4530c409ecdb8c78401 not found in store.
warn: IdentityServer4.Validation.TokenValidator[0]
      Invalid refresh token
warn: IdentityServer4.Validation.TokenRequestValidator[0]
      Refresh token validation failed. aborting, {
        "ClientId": "*************************.App.Client",
        "ClientName": "*******************",
        "GrantType": "refresh_token",
        "Raw": {
          "grant_type": "refresh_token",
          "client_id": "********************.App.Client",
          "scope": "openid profile email offline_access *****_api",
          "refresh_token": "***REDACTED***"
        }
      }

Successful refresh token request (R1) with refresh token d5290....8401:
image

Failed refresh token request (R2) with refresh token d5290....8401 at "same time" as R1:
image

Log output in the console:
image

In the console "Error performing password flow" is printed as error. Why password flow? We use code-flow and never start a password flow.

Maybe related to this issue?
#722
#724

@jeroenheijmans jeroenheijmans added investigation-needed Indication that the maintainer or involved community members may need to investigate more. more-info-needed Please provide a minimal example (e.g. at stackblitz.com) which demonstrates the issue labels Mar 5, 2020
@jeroenheijmans
Copy link
Collaborator

Hmm....

Don't have any immediate thoughts or solutions, but you could try with the just released v9 of the library, that should have a lot of changes/fixes around code flow too I think?

(Haven't tried v9 yet myself though.)

@manfredsteyer
Copy link
Owner

Do you already use version 9 of this lib? We've fixed a bug regarding this in version 9.

@goflo
Copy link
Author

goflo commented Mar 6, 2020

@jeroenheijmans @manfredsteyer Thank you for your reply. No, I'm using v8.0.4. And at the moment I can't update to Angular 9 which is required for v9.

I have found this comment in an other issue: #722 (comment)

We changed the order and call now setupAutomaticSilentRefresh() before loadDiscoveryDocumentAndTryLogin() maybe that solves our problem as well. If not I hope we can migrate to Angular 9 and v9 of this library soon...

@jeroenheijmans
Copy link
Collaborator

We changed the order and call now setupAutomaticSilentRefresh() before loadDiscoveryDocumentAndTryLogin() maybe that solves our problem as well.

FWIW: I can confirm that's what I also do in all my applications.

@manfredsteyer
Copy link
Owner

@goflo : Version 9 should also work with Angular 8. I guess I should point this out in the readme.

@jeroenheijmans : This issue with the right order of these called really gives me a hard time. Can you have a short look into the new quickstart-demo. Here I have this order and it works. Can you tell me what I have to do here to run into this very issue?

@jeroenheijmans
Copy link
Collaborator

I cannot reproduce this behavior.

I think the order of things in the quickstart-demo is fine (though it doesn't include any automatic or timed refreshes, just when you hit the button). Even if I setup a timeoutFactor of 0.025 and do setupAutomaticSilentRefresh() in it I cannot reproduce the behavior.

I cannot reproduce it in my own sample repository either, unless.... you'd use a version before the commit where I compensate for #600 (no silent refresh via iframe available with code flow). In that case, my sample even for v8 will show an errorred out refresh that was happening because I prefer to try iframe-based refreshed at the start of my app.

Maybe OP was experiencing that behavior? @goflo could you check if you initiate silentRefresh() in your app somewhere? That's not supported currently and causes the behavior you describe.

Otherwise, we'll probably need a repository or a StackBlitz example to further investigate...

@jeroenheijmans jeroenheijmans removed the investigation-needed Indication that the maintainer or involved community members may need to investigate more. label Mar 8, 2020
@goflo
Copy link
Author

goflo commented Mar 23, 2020

@manfredsteyer : thanks for the info regarding v9 and Angular 8!

@jeroenheijmans : no, I don't initiate a silentRefresh().

As I wrote above we are calling setupAutomaticSilentRefresh() now before loadDiscoveryDocumentAndTryLogin() and until now it seems like that solved the issue. If it occurs again I will update to v9. For now we can close this issue.

Thanks for your time!

@goflo goflo closed this as completed Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-info-needed Please provide a minimal example (e.g. at stackblitz.com) which demonstrates the issue
Projects
None yet
Development

No branches or pull requests

3 participants