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.

Sign up for GitHub

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

fix(session): check for valid session before API calls #358

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

fix(session): check for valid session before API calls #358

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/context/AppContext.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,11 @@ export const AppProvider = (props) => {
function checkSession() {
const localToken = JSON.parse(localStorage.getItem('token'));
const localUser = JSON.parse(localStorage.getItem('user'));
if (localToken && localUser) {

if (localToken && localUser && session.token) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this doesn't fix the underlying problem, and has the undesirable side effect of logging the user out every time they refresh the page.

We should call /auth/check_session if there's a local token and local user, but should ideally suspend other API calls until this check has finished and the user has been logged out (if the response is 401).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.. I thought about putting a condition on the rendering of Dash Stat components. Should I change something in that API call function itself or its useEffect, or something in the error handling that is catching the API 401 errors?

By /auth/check_session do you mean the checkSession function that uses that path?

  • checkSession is currently called only once in the entire app in AppContext. Are you saying that checkSession should be called as it is below:

if (!user || !token) {
checkSession();
}

or that we should call checkSession if somewhere in the app there is local token and local user?
because that means we have to export checkSession as an app wide function with context

My solution to stop the API calls was to put a condition in the DashStats (the component responsible for making the API call upon mounting) The last time I tried, the API calls stopped. I'm not sure if that's the ideal way to suspend the API calls.

Does anyone have suggestions or want to try this ticket? I'm not sure the timeline. I would love to know the solution too.

// Temporarily log in with the localStorage credentials while
// we check that the session is still valid
login(localUser, localToken);

axios
.get(
`${process.env.REACT_APP_API_ROOT}/auth/check_session?id=${localUser.id}`,
Expand All @@ -232,6 +232,7 @@ export const AppProvider = (props) => {
logout();
}
});

return true;
}
return false;
Expand Down