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

Set missing refresh timeout #8880

Merged
merged 1 commit into from Oct 18, 2021

Conversation

licitdev
Copy link
Member

Issue 1

When the page goes "idle", the refresh timeout is cleared.

directus/app/src/auth.ts

Lines 47 to 55 in 6db9e0c

idleTracker.on('idle', () => {
clearTimeout(refreshTimeout);
idle = true;
});
idleTracker.on('hide', () => {
clearTimeout(refreshTimeout);
idle = true;
});

However, it is not re-added when the page becomes active again.

Issue 2

isRefreshing flag is permanently true if it is valid for the following conditional check.

directus/app/src/auth.ts

Lines 81 to 88 in 6db9e0c

isRefreshing = true;
const appStore = useAppStore();
// Skip refresh if access token is still fresh
if (appStore.accessTokenExpiry && Date.now() < appStore.accessTokenExpiry - 10000) {
return;
}

Fixes

Set a new timeout if the token is still fresh.
Shifted the setting of the isRefreshing flag after the conditional check.
In addition, I have removed a check for isRefreshing as it is redundant.

if (refreshTimeout) clearTimeout(refreshTimeout);

@rijkvanzanten rijkvanzanten merged commit 7c205fd into directus:main Oct 18, 2021
@licitdev licitdev deleted the fix/missing-refresh-timeout branch October 20, 2021 18:15
@massimoliani
Copy link

@rijkvanzanten as I'm using standard npm package locally and on aws server are you planning to common this refresh change soon on rc99? Have you a timing? I need for a demo.

Thank you and sorry to disturb you

@rijkvanzanten
Copy link
Member

@massimoliani 99 is scheduled for release tomorrow (oct 21) 👍🏻

@massimoliani
Copy link

Thanks a lot

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants