fix(session): check for valid session before API calls #358
Conversation
|
Needs review |
| @@ -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) { | |||
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
|
This will likely be impacted by the introduction of Keycloak, so we'll pause development for now. |
nmcharlton
added
the
on hold
Description
Add condition session.token to execute checkSession only if session.token and other conditions are true. Otherwise 5 more API calls from getTotalUnprocessed, getTotalVerified, loadOrganizations, getTotal, and getTotalGrowerCount will fetch data.
Issue(s) addressed
What kind of change(s) does this PR introduce?
Please check if the PR fulfills these requirements
Issue
What is the current behavior?
The app does not logout properly upon 401 error. checkSession is not checking for valid session so logging in will trigger all API calls from mounting DashStat Components.
What is the new behavior?
App will logout and stop making API calls when JWT token changes.
Breaking change
Does this PR introduce a breaking change?
No
Other useful information
Currently no catch method on checkSession to logout on error. Does not handle 401 error specifically