-
Notifications
You must be signed in to change notification settings - Fork 47
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
PXP-4219 Fix/session #806
PXP-4219 Fix/session #806
Conversation
@@ -168,5 +168,4 @@ module.exports = { | |||
typescript: {}, // this loads <rootdir>/tsconfig.json to eslint | |||
}, | |||
}, | |||
ignorePatterns: ['src/params.js'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have to put this into .eslintignore
since my vscode eslint doesn't like this somehow and it breaks my local eslint 🤷♂️
const newState = Object.assign({}, initialState); | ||
|
||
if (!newState.authenticated) { | ||
return Promise.resolve(newState); | ||
} | ||
if (nowMs - lastTokenRefreshMs < 41000) { | ||
return Promise.resolve(newState); | ||
} | ||
return store.dispatch(fetchProjects()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The if-blocks in this area appear to be unnecessary -- the conditions all result in return Promise.resolve(newState);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifically this snippet:
const projects = store.getState().submission.projects;
if (projects) {
// user already has a valid token
return Promise.resolve(newState);
} else if (info.status !== 403 || info.status !== 401) {
// do not authenticate unless we have a 403 or 401
// should only check 401 after we fix fence to return correct
// error code for all cases
// there may be no tables at startup time,
// or some other weirdness ...
// The oauth dance below is only relevant for legacy commons - pre jwt
return Promise.resolve(newState);
}
return Promise.resolve(newState);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the original logic of marking user as not authed and redirecting to /login
is still valid, so I updated with that
Jira Ticket: PXP-4219
Improved token refresh and session monitor logic, see comments in
SessionMonitor/index.js
TL;DR: Portal will now tracking user's activity interval and actively logging out inactive users, rather than passively rely on waiting for Fence's token expiration.
To support this, Fence must be able to update access token in cookie before the current one expires as well, see uc-cdis/fence#874
Also removed a good chunk of old codes in portal (mostly relevent to the old
submissionApiOauth
, which is before Fence)Deployed in: https://qa-mickey.planx-pla.net/
Improvements
Dependency updates