-
Notifications
You must be signed in to change notification settings - Fork 694
Interceptor sends request with expired token #820
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
Comments
I think ever since PR #515 the default interceptor is supposed to get a new access token if the existing one can't be used anymore? Could possibly improved upon though... |
I am having this problem when I use localStorage instead of sessionStorage. |
To be more precise, this happens when the refresh token is expired so the refreshing fails. Here is my temporary fix : if (this.oauthService.getRefreshToken()) {
try {
await this.oauthService.refreshToken();
} catch(err) {
// Access and refresh token are expired
this.AUTH_STORAGE_ITEMS.map((item: string) => { localStorage.removeItem(item); })
return ...;
}
// Refresh token has succeeded
} where I am OK with it except that I would prefer to have a method such as |
I am seeing this issue after using silentRefresh in the initial login sequence from https://github.com/jeroenheijmans/sample-angular-oauth2-oidc-with-auth-guards/blob/master/src/app/core/auth.service.ts#L111 and the session on the idp is no longer valid. So the refresh fails, but the access token is not removed automatically and used in a subsequent call. Removing them as suggested by @mlbiche solves the issue, but I think this should be part of the lib. |
Hello @mlbiche @remkoboschker can you guys provide more details about where you put the code snippet above? Was it in your custom interceptor before the actual api call or was it inside |
It is executed when Here is my
Once again, it is a personal implementation that works for me and may (absolutely ?) not be standard. Hope it helps 😉 |
Thanks @mlbiche! I would like to know if your solution is applicable with my problem? Seems like its not because my APP_INITIALIZER that runs the loginSequence only triggered once? And once the user went idle like sleep mode their machine, |
I think I am facing a similar situation when I am debugging and the application is stopped at a breakpoint for a while, preventing the silent refresh to run. When I am running the application then, the access token is expired and the silent refresh is not executing resulting into an invalid token. |
Hi, I clear the tokens by calling logout in a subscriber to oauthService.events. When the event.type is token_refresh_error for instance. |
Hi @mlbiche @remkoboschker I tried guys your solutions but there is an issue that I've encountered where if I delete all the storage items, the http call will be invalid because there is no token and somehow I need to trigger again Apart from that issue, the Thanks guys! Here is the GIST: https://gist.github.com/santosmken/026329bbfdb39a82d2990e9424e21d9d |
Having the same issue as @santosmken. Steps:
|
@mlbiche This actually didn't work for me. If you clear out storage, you will also clear out storage on the return path of a successful auth server session and the error will be "invalid_nonce_in_state". If I try to not remove the nonce property, I get a refresh_token_error etc. I do this check as the first step of my rememberMe workflow. I would like to know if there is a way to only clear the session under certain circumstances. You would not want to clear it out when you are having a successful return from the auth server for example. |
However... @mlbiche's solution works just fine when you remove the storage items after having called loadDiscoveryDocumentAndTryLogin() and the token is still not valid. |
@bjornharvold I can confirm that it works. I just missed before having an if statement to check if the access token is still valid. And that is the correct place to remove the storage keys. Steps will be:
Additional note that I also have this event for removing the storage keys.
|
@bjornharvold I confirm that I do this after calling @santosmken Smart ! I think your piece of code fixes an authentication issue I do have when I am pausing the application while debugging. Cheers 🎉 |
Cool @mlbiche! Thank you for your assistance with this matter. |
Cheers guys! Very helpful. |
Just recently added Sentry to my project and I can see a bunch of 401 errors popping, even though my fix is in place... I think I am going to add an interceptor that checks the validity of the |
Does anyone have a gist for this one? Following the above thread might not be ideal for some |
I opened a PR for a partial solution to the following subset of the problem described: When restarting the application, which would trigger I can't solve silent refresh not working, but this is easy, simply make the interceptor check the validity of the token before sending it. https://github.com/manfredsteyer/angular-oauth2-oidc/pull/1317/files |
I just stumbled across the same issue and I just wanted to add a note I think is overseen often: There are many machines out there where the clocks are not correct. I have seen it many times in other projects because they had similar problems. Especially Windows machines tend to be a couple of minutes behind. So you can not assume, that an automatic silent refresh is enough. You cannot trust the expiry. There should be fallback where requests which will get an 401 will automatically get a new access token and then be retried ONCE. If 401 is still the response, THEN we can assume something is wrong. |
Describe the bug
The interceptor sends requests with expired tokens even when
useSilentRefresh
is seems like the token expired event is never triggered (tab in the background), to avoid this, we could check (whenuseSilentRefresh
) that the token is not expired yet, and if so clear theaccess_token
and refresh before sending the request.Expected behavior
When
useSilentRefresh
the user shouldn't ensure that it's token is valid before sending requestsThe text was updated successfully, but these errors were encountered: