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

AutoSilentRefresh doesn't work after refresh the page #444

Closed
junimohano opened this issue Oct 1, 2018 · 17 comments
Closed

AutoSilentRefresh doesn't work after refresh the page #444

junimohano opened this issue Oct 1, 2018 · 17 comments
Labels
bug For tagging faulty or unexpected behavior.

Comments

@junimohano
Copy link

    this.oAuthService.configure(this.ntAuthConfig);
    this.oAuthService.tokenValidationHandler = new JwksValidationHandler();
    this.oAuthService.setupAutomaticSilentRefresh();
    this.oAuthService.loadDiscoveryDocumentAndLogin();

once get token, setupAutomaticSilentRefresh works well
but after refresh the page, seems like counter is reset as start from 0

for example, set access token expires 5 mins. and login then 70% of 5mins will refresh token right?
Let us say 60% time spent and refresh the page, then after 3mins again will try to refresh the token.
so that the time between when I refresh and reach the 70% will be expired.

when the user refresh the page, do we need to refresh access_token? or use same token?
if yes, how to fetch new access token when the user refresh the page?

@jdgeier
Copy link
Contributor

jdgeier commented Oct 2, 2018

The timers are all based on ngZone so when you refresh they get destroyed calling this.oAuthService.setupAutomaticSilentRefresh(); I think will refresh the timers but I think it needs to be part of the app.component.ts so that it gets called when the application is initialized.

@junimohano
Copy link
Author

@jdgeier As I tested, it doesn't re-fetch new token if valid token I still have.
and the count starts from 0 again.

Do I manually need to execute silentRefresh() after refreshing the page if no token achived?

@jdgeier
Copy link
Contributor

jdgeier commented Oct 2, 2018

I'm not sure I only started looking at this thing about a week ago to see if it will fit my needs. You might have to dig into the source to see how it works. Also if you got this from npm make sure you have the right version it seems to default to a 2.x branch if you don't specify ^4.0.0

@junimohano
Copy link
Author

thank you for replying, I am using 4.0.3,

I wonder how other developers resolve this.

@jdgeier
Copy link
Contributor

jdgeier commented Oct 2, 2018

I went back and looked and yeah I think you need to manually call silentRefresh()

@junimohano
Copy link
Author

Let me know if you find something.
I couldn't find proper events to call silentRefresh() after refreshing the page.

once I called loadDiscoveryDocumentAndLogin(), receive two events of discovery_document_loaded and token_received if new token received.

and only discovery_document_loaded event if current token is still valid.

but I don't know how to verify token_received event called or not on certain point of time.

@jdgeier
Copy link
Contributor

jdgeier commented Oct 2, 2018

Like I said you might have to look at the code:

https://manfredsteyer.github.io/angular-oauth2-oidc/docs/injectables/OAuthService.html#silentRefresh

If you click on the Defined In Line: XXX you'll see the relevant stuff.

@junimohano
Copy link
Author

junimohano commented Oct 2, 2018

um... check setupRefreshTimer() and setupExpirationTimers() it seems like it supposed to re-calculate expire time.

private setupAccessTokenTimer(): void {
    const expiration = this.getAccessTokenExpiration();
    const storedAt = this.getAccessTokenStoredAt();
    const timeout = this.calcTimeout(storedAt, expiration);

    this.ngZone.runOutsideAngular(() => {
      this.accessTokenTimeoutSubscription = of(
        new OAuthInfoEvent('token_expires', 'access_token')
      )
        .pipe(delay(timeout))
        .subscribe(e => {
          this.ngZone.run(() => {
            this.eventsSubject.next(e);
          });
        });
    });
  }

@jdgeier
Copy link
Contributor

jdgeier commented Oct 2, 2018

Correct, but they appear to be private. You'll have to modify the source to switch their accessibility.

@junimohano
Copy link
Author

This method is called on constructor if token is valid.

hmm.. I don't get it so far

@jdgeier
Copy link
Contributor

jdgeier commented Oct 2, 2018

Ah, I didn't notice that.

@junimohano
Copy link
Author

junimohano commented Oct 2, 2018

private setupAccessTokenTimer(): void {
    const expiration = this.getAccessTokenExpiration();
    const storedAt = this.getAccessTokenStoredAt();
    const timeout = this.calcTimeout(storedAt, expiration);

    this.ngZone.runOutsideAngular(() => {
      this.accessTokenTimeoutSubscription = of(
        new OAuthInfoEvent('token_expires', 'access_token')
      )
        .pipe(delay(timeout))
        .subscribe(e => {
          this.ngZone.run(() => {
            this.eventsSubject.next(e);
          });
        });
    });
  }

I think this method needs to be changed.
according to this method, try to calculate (expiration - storeAt) * 0.75(default) which is always fetch 75% of expiration time.
but what we need to is we have to calculate current time as well, if current time is past than storedAt, we need to use current time instead of storedAt.

same as setupIdTokenTimer()

@jdgeier
Copy link
Contributor

jdgeier commented Oct 2, 2018

How are you setting up OAuthStorage? I just remembered when I was debugging an issue a few days ago if you do not set the value for OAuthStorage in the providers when the constructor runs the first time it may not be able to read the tokens.

Under app.modules.ts or in your own ngmodule make sure you've added this line to the providers:

providers[ ... { provide: OAuthStorage, useValue: localStorage },
or sessionStorage as you choose.

@baconcutter
Copy link

I have also encountered this issue and I think it is a bug that was introduced with the fix for #103.
I agree with @junimohano that the methods setupAccessTokenTimer and setupIdTokenTimer should take the current time into account when calculating the refresh time.

As a current workaround, I make sure that the user is logged out before the page is refreshed. This is not ideal, because this will increase the startup time (because the user always has to log in first) and it will not work when you use local storage for storing the tokens.

Hopefully this will be fixed soon.

@lanovoy
Copy link

lanovoy commented Dec 12, 2018

Are there any ETAs on fix for this issue? Workarounds?

@orditeck
Copy link

orditeck commented Dec 13, 2018

I'm experiencing the same issue.

I ended up doing this on the page load, after configure, setupAutomaticSilentRefresh and loadDiscoveryDocumentAndTryLogin:

    private async refreshTokenOnLoadIfNeeded(): Promise<void> {
        const expiration = this.oauthService.getAccessTokenExpiration();
        const storedAt = parseInt(sessionStorage.getItem('access_token_stored_at'), 10);
        const timeout = (expiration - storedAt) * this.oauthService.timeoutFactor;
        const refreshAt = timeout + storedAt;

        if (now >= refreshAt) {
            await this.oauthService.silentRefresh();
        }
    }

@danmana
Copy link

danmana commented Oct 11, 2019

This bug is not longer present in version 8.0.0

setupAutomaticSilentRefresh()
calls this.restartRefreshTimerIfStillLoggedIn();
which calls this.setupExpirationTimers();
which calls this.setupAccessTokenTimer()
which uses const timeout = this.calcTimeout(storedAt, expiration);
which calculates the timeout as:

const delta = (expiration - storedAt) * this.timeoutFactor - (now - storedAt);
return Math.max(0, delta);

in case the token is past it's timoutFactor (75%) the timeout will be 0 => an instant refresh will be triggered

@jeroenheijmans jeroenheijmans added the bug For tagging faulty or unexpected behavior. label Nov 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For tagging faulty or unexpected behavior.
Projects
None yet
Development

No branches or pull requests

8 participants