Skip to content

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

Open
yelhouti opened this issue May 15, 2020 · 21 comments
Open

Interceptor sends request with expired token #820

yelhouti opened this issue May 15, 2020 · 21 comments
Labels
investigation-needed Indication that the maintainer or involved community members may need to investigate more.

Comments

@yelhouti
Copy link

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 (when useSilentRefresh) that the token is not expired yet, and if so clear the access_token and refresh before sending the request.

Expected behavior
When useSilentRefresh the user shouldn't ensure that it's token is valid before sending requests

@jeroenheijmans
Copy link
Collaborator

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...

@jeroenheijmans jeroenheijmans added the investigation-needed Indication that the maintainer or involved community members may need to investigate more. label May 16, 2020
@pinguinosod
Copy link

pinguinosod commented Oct 2, 2020

I am having this problem when I use localStorage instead of sessionStorage.
If I have a valid access token and I close the tab, I wait until the token should be expired, and I open the tab again... Then the expired access token is still present on my localStorage and is still being sent on the requests.

@mlbiche
Copy link

mlbiche commented Nov 9, 2020

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 AUTH_STORAGE_ITEMS are all items stored by the module. (In fact, it is equivalent to a log out, but without posting the logout request)

I am OK with it except that I would prefer to have a method such as hasValidRefreshToken that returns if the refresh token is still valid or not. It would prevent having in the console a 400 HTTP error when posting on /token endpoint with an invalid refresh token.

@remkoboschker
Copy link

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.

@santosmken
Copy link

santosmken commented Nov 10, 2021

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 oauth service events? and what specific event? Another is about AUTH_STORAGE_ITEMS, should I delete everything from what I got during the acquiring of token?
Cuz, I'm trying to fix the same issue and my use case is that the user went idle for an hour then tries another http request which resulted into unauthorized. Thank you!

@mlbiche
Copy link

mlbiche commented Nov 10, 2021

It is executed when loadDiscoveryDocumentAndTryLogin is done in the initialisation method of my Authentication service.

Here is my AUTH_STORAGE_ITEMS :

private readonly AUTH_STORAGE_ITEMS: string[] = [
    'access_token',
    'access_token_stored_at',
    'expires_at',
    'granted_scopes',
    'id_token',
    'id_token_claims_obj',
    'id_token_expires_at',
    'id_token_stored_at',
    'nonce',
    'PKCE_verifier',
    'refresh_token',
    'session_state'
  ];

Once again, it is a personal implementation that works for me and may (absolutely ?) not be standard. Hope it helps 😉

@santosmken
Copy link

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, setupAutomaticSilentRefresh()will not be triggered automatically which will result into invalid token going to the api which must be renewed first supposedly.

@mlbiche
Copy link

mlbiche commented Nov 10, 2021

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.
I have not found any solution for this but, as it seems to only happen while debugging, I am ok with this.

@remkoboschker
Copy link

Hi, I clear the tokens by calling logout in a subscriber to oauthService.events. When the event.type is token_refresh_error for instance.

@santosmken
Copy link

santosmken commented Nov 16, 2021

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 loadDiscoveryDocument to have storage values. Did I missed something here? Btw, I'm using APP_INITIALIZER and it is triggered once since user need to login and oauth events is part of my APP_INITIALIZER.

Apart from that issue, the refresh_token isn't totally expired yet because of the 1 week validity but if you didn't use the application for less than a week, I am forced to login again which it shouldn't be. Do you have any idea about this issue?

Thanks guys!

Here is the GIST: https://gist.github.com/santosmken/026329bbfdb39a82d2990e9424e21d9d

@bjornharvold
Copy link

bjornharvold commented Jan 23, 2022

Having the same issue as @santosmken.

Steps:

  • Using localStorage
  • Authenticated via login with popup
  • Closed the lid of my laptop to put it in hibernate to stop work for the day
  • Opened my laptop to continue working
  • Token has expired but it's still being set as Bearer token and consequently getting a 401 from the resource server
  • Refreshing the page does nothing. Have to clear my localStorage manually.

@bjornharvold
Copy link

bjornharvold commented Jan 26, 2022

@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.

@bjornharvold
Copy link

However... @mlbiche's solution works just fine when you remove the storage items after having called loadDiscoveryDocumentAndTryLogin() and the token is still not valid.

@santosmken
Copy link

santosmken commented Jan 31, 2022

@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:

  1. loadDiscoveryDocumentAndTryLogin()
  2. !this.oauthService.hasValidAccessToken() (falsy)
  3. this.AUTH_STORAGE_ITEMS.map((item: string) => { sessionStorage.removeItem(item); });

Additional note that I also have this event for removing the storage keys.

if (event.type === 'token_refresh_error') { this.AUTH_STORAGE_ITEMS.map((item: string) => { sessionStorage.removeItem(item); }); }

@mlbiche
Copy link

mlbiche commented Jan 31, 2022

@bjornharvold I confirm that I do this after calling loadDiscoveryDocumentAndTryLogin when the token is still not valid 👍

@santosmken Smart ! I think your piece of code fixes an authentication issue I do have when I am pausing the application while debugging. Cheers 🎉

@santosmken
Copy link

Cool @mlbiche! Thank you for your assistance with this matter.

@bjornharvold
Copy link

Cheers guys! Very helpful.

@mlbiche
Copy link

mlbiche commented Feb 22, 2022

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 access_token before sending it to the server. If the token is expired, it would erase all the local state and logout the user. I am just checking if anyone has a better solution before implementing this (maybe @santosmken's is the good one, I haven't tried it yet 🤔).

@CharlieGreenman
Copy link

Does anyone have a gist for this one? Following the above thread might not be ideal for some

@MatthiasvB
Copy link

MatthiasvB commented Mar 16, 2023

I opened a PR for a partial solution to the following subset of the problem described:

When restarting the application, which would trigger loadDiscoveryDocument and all that, the access_token is successfully renewed. BUT, since the interceptor currently only checks for the presence of any Access-Token, instead of a valid one, it let's requests through attaching the old, outdated token if the refresh isn't finished, yet. This returns 401 responses, which, if your error-handling is weird (which is not untypical 😅), can even lead to redirects to error pages which delete the hash fragment that would be required to login. Welcome to login-loop.

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

@JustDoItSascha
Copy link

JustDoItSascha commented Jul 19, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigation-needed Indication that the maintainer or involved community members may need to investigate more.
Projects
None yet
Development

No branches or pull requests

10 participants